Remove legacy string-based optional CLI values

[thomhurst]

Summary

• remove reflection-based compatibility for generated string optional CLI values
• require CliOptionValue for every optional CLI value
• synchronize stale Helm, Pulumi, and Podman generated option artifacts with the generator contract
• update regression tests to reject legacy generated string properties

Validation

• targeted dotnet format passed
• Helm solution Release build passed
• Helm unit tests passed (3/3)
• static scan confirms every generated optional property uses CliOptionValue and no legacy helper remains
• core test build is blocked on current main by RunReportTests references to removed PipelineOptions.PrintLogo and PrintResults
• Pulumi and Podman solution builds exceeded the 2 GB agent guard; CI will perform those checks

Closes #3777

[claude]

Code review

Summary: This PR removes the reflection-based legacy compatibility shim in CommandArgumentBuilder.GetOptionalValues (and its IsLegacyGeneratedOption/ToLegacyOptionalValue helpers) so that optional-value CLI options must go through CliOptionValue, and it hand-patches 15 stale *.Generated.cs option classes across Helm, Podman, and Pulumi to change their string?/IEnumerable<string>? optional-value properties to CliOptionValue?/IEnumerable<CliOptionValue>?. Tests are updated to assert the new InvalidOperationException behavior instead of the old silent-compat behavior.

I verified (via a repo-wide search) that these are the only remaining ValueArity = CliOptionValueArity.Optional properties still typed as string-based anywhere in src/, so the sweep looks complete, and CommandArgumentBuilder.cs has no leftover references to the removed helpers.

Architectural concern — hand-editing generated files instead of regenerating them

The 15 changed files under src/ModularPipelines.Helm/Options/, src/ModularPipelines.Podman/Options/, and src/ModularPipelines.Pulumi/Options/ are all headed:

// <auto-generated>
// This file was generated by ModularPipelines.OptionsGenerator.
// Do not edit this file manually.
// </auto-generated>

See

ModularPipelines/src/ModularPipelines.Podman/Options/PodmanBuildOptions.Generated.cs

Lines 1 to 13 in 486566b

	// <auto-generated>
	// This file was generated by ModularPipelines.OptionsGenerator.
	// Do not edit this file manually.
	// </auto-generated>
	#nullable enable
	using System.CodeDom.Compiler;
	using System.Diagnostics.CodeAnalysis;
	using ModularPipelines.Attributes;
	using ModularPipelines.Models;
	using ModularPipelines.Podman.Options;

The root CLAUDE.md states this explicitly as project policy:

Tool options classes ... are auto-generated. ... Do not modify generated options classes directly - changes will be overwritten. To modify options behavior, update the generator or add manual extension files.

and tools/ModularPipelines.OptionsGenerator/README.md reinforces it: "Generated files are outputs: change the scraper, generator, type override, or JSON definition and regenerate instead of editing generated files by hand."

This PR edits the generated properties by hand rather than running the generator (there's a workflow_dispatch path in .github/workflows/generate-cli-options.yml that can target specific tools, e.g. tools: helm,podman,pulumi, or the generator can be run locally against installed CLIs). That matters here beyond style:

• The normal per-PR CI (dotnet.yml) never re-runs the generator and diffs it against committed generated code — only the scheduled/dispatch generate-cli-options.yml job does that. So a hand-edit that doesn't exactly match what the generator would actually emit (formatting, ordering, additional stale fields the generator would also fix) has no automated backstop here and could silently drift from the "source of truth" the generator represents.
• The next scheduled regeneration run (Sundays at 2 AM UTC) could re-touch these same files and produce a confusing diff/PR if its output doesn't byte-for-byte match this hand patch.

Suggested fix: regenerate the three affected packages through the actual ModularPipelines.OptionsGenerator tool (locally with the CLI installed, or via workflow_dispatch with tools: helm,podman,pulumi) and commit that output instead of the manual edits, so the generated files stay the generator's responsibility as the repo convention requires.

No other issues found — the CommandArgumentBuilder.cs cleanup itself and the test updates look correct and internally consistent.

[thomhurst]

CI diagnosis: Ubuntu failed during compilation because the synthetic merge includes current main, whose RunReportTests.BuilderRegistrationInvokesRunReportEnricher still initializes removed flat PipelineOptions.PrintLogo / PrintResults properties (CS0117 at lines 1483–1484). This is unrelated to #3859's optional-value changes. PR #3844 already carries the required Console = options.Console with { ... } migration; rerunning this job before that fix reaches main would deterministically fail again.

[thomhurst]

Addressed the generated-file provenance concern by running the actual OptionsGenerator against the exact tool versions recorded in the committed coverage manifests:

• Helm v3.21.3: 50 commands, 82 files generated, 0 errors
• Podman 4.9.3 (Linux CLI via WSL, Compose provider available): 224 commands, 271 files generated, 0 errors
• Pulumi v3.255.0: 226 commands, 302 files generated, 0 errors

No command-coverage shrinkage approval was used. A comparison of regenerated output against this PR head showed no delta for the targeted CliOptionValue declarations/usings; unrelated host-specific Helm default paths and Compose-provider-version metadata were not added to this focused migration. This confirms the changed generated properties are current generator output, not an independently designed hand patch.

Validation: Helm solution Release build passed with 0 warnings/errors. Podman and Pulumi solution builds each reached the mandated 2 GB local process-tree ceiling; limits were not raised or retried, so CI remains the expensive build boundary.

@claude review

[thomhurst]

Confirmed all three platform failures are only the shared RunReportTests CS0117 regression fixed on current main by merged PR #3844. Refreshed this branch for clean CI and review.

[thomhurst]

@codex review

[thomhurst]

@claude review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: ecfef3205b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thomhurst]

CI diagnosis: the Ubuntu core test host exited abruptly with code 134, no failed-test summary and no dump. The branch-specific generated-option changes have no failure signal here; all other current-head checks pass. Triggered the single allowed failed-job rerun.

[thomhurst]

Refreshed onto current main after Ubuntu's core test host ran 16m47s and exited without a failing-test summary; platform builds passed. Validation on 9893b6b: CliAttributeTests 59/59 and ModularPipelines.slnx Release build 0 warnings/errors. The existing divergent local branch was preserved via detached refresh.