Correct generated CLI enum values

[thomhurst]

Closes #3503

Summary

• migrate generated CLI enum members from DescriptionAttribute to runtime EnumValueAttribute
• replace Docker duration-suffix pseudo-enums with string options and Pulumi repeatability pseudo-enums with IEnumerable<string>
• prevent documentation scraping from treating duration units as complete enum values
• add a CI gate rejecting stale generated enum attributes

Validation

• OptionsGenerator focused tests: 3/3 passed
• OptionsGenerator Release build: 0 warnings, 0 errors
• Docker Release build: 0 errors (233 pre-existing warnings)
• Docker generated-options smoke test: 1/1 passed
• Minikube Release build: 0 warnings, 0 errors
• Minikube generated-options smoke test: 1/1 passed
• generated enum metadata gate: passed; zero stale matches
• git diff --check: passed

Local validation limits

• Pulumi Release build exceeded the mandated 2 GB agent process-tree limit at 2067 MB; not retried per repository policy
• format verification reports existing solution-wide diagnostics outside this change

[thomhurst]

Implementation pushed at 7016c5d04f741dcd2f6dce4c21ed7c2d9e536b95.

Validation summary:

• generator guard tests: 3/3 passed
• OptionsGenerator Release build: 0 warnings/errors
• Docker Release build: 0 errors; generated-options smoke test passed
• Minikube Release build: 0 warnings/errors; generated-options smoke test passed
• stale generated [Description] gate: passed (zero matches)
• git diff --check: passed
• Pulumi build reached the mandated 2 GB local agent cap and was not retried

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 7016c5d04f

ℹ️ 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".

[claude]

Review: Correct generated CLI enum values (#3572)

Verified the core bug is real and the fix is correct. CommandArgumentBuilder.ParseEnum (src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs:344-354) only reflects for EnumValueAttribute — it never looked at [Description]. So every generated enum still decorated with [Description("azure-native")] etc. was silently falling back to rawValue.ToString() (e.g. "AzureNative", "Rawjson", "__82540em") when building CLI args. That's a genuine, previously-silent correctness bug affecting real Docker/Minikube/Pulumi command invocations, not a cosmetic rename. Confirmed the source generator (Generators/EnumGenerator.cs:109) already emits [EnumValue(...)] for new output, so this PR is just bringing stale checked-in generated files back in sync with current generator behavior, plus adding a gate so they can't drift again. Good catch, good fix.

CI gate (Test-GeneratedEnumAttributes.ps1) looks solid — git grep -l --fixed-strings '[Description(' -- 'src/**/*.Generated.cs' correctly distinguishes "no matches" (exit 1) from a real grep failure (exit >1), and I confirmed 0 remaining [Description( hits in generated files in the current tree.

Duration-suffix fix is well targeted and tested — IsDurationUnit + the new CliDocumentationScraperBaseTests cover the "ms|s|m|h" case that produced the bogus DockerContainerCreateHealthInterval-style enums, and confirm real enums like plain|json|tty still detect correctly.

One gap worth closing: the Pulumi "repeatable/comma-separated" fix has no regression guard

The three deleted Pulumi enums (PulumiDeploymentSettingsEditOidcAwsPolicyArn, ...PathFilter, ...RemoveEnv) had values Repeatable/CommaSeparated — clearly the CLI help's parenthetical annotation "(repeatable, comma-separated)" being misread as the option's accepted value set. Pulumi is scraped via PulumiCliScraper : CobraCliScraper, not CliDocumentationScraperBase — so this bug came from a different detection path (CobraHelpTypeDetector / HeuristicTypeDetector / DescriptionEnumValueParser), which this PR doesn't touch at all. The only enum-quality fix and test added (IsDurationUnit) lives exclusively in CliDocumentationScraperBase, which Pulumi doesn't use.

Concretely: nothing stops a future re-scrape of Pulumi (or any other CobraCliScraper-based tool) from reproducing the exact same "annotation phrase treated as enum values" bug that was just manually fixed by deleting these three files. Right now the fix is "delete the bad generated output," not "prevent the generator from producing it again," which is inconsistent with how the Docker duration-unit case was handled (root-caused in the scraper + regression test).

Suggest either:

• Adding an equivalent denylist/heuristic in DescriptionEnumValueParser.TryParseValues (or wherever the Cobra-path enum candidates get validated) that rejects annotation-only words like repeatable/comma-separated/optional the same way IsDurationUnit rejects unit suffixes, with a test mirroring CliDocumentationScraperBaseTests, or
• If the actual originating detector is elsewhere and I've mis-traced it, at minimum a comment/test pinning down where these three enums actually came from, so the fix and the regression test live next to the real root cause rather than only covering the Docker-specific path.

Everything else — the IEnumerable<string> conversion for the repeatable Pulumi options, the string? conversion for Docker's free-form duration options, the workflow addition of the new gate step — looks correct and appropriately scoped. No other issues found.

[thomhurst]

Addressed the Pulumi regeneration gap at head bda47b1ef7d0d28c7cd939e23855132cf443765e.

The current Cobra path already routes descriptions through DescriptionEnumValueParser, whose contextual-parentheses pattern rejects annotation-only text. The deleted enums were stale output from before that parser hardening. Added exact regression coverage for all three Pulumi descriptions at both boundaries:

• parser returns no enum match
• HeuristicTypeDetector returns StringList with no enum values

Validation:

• DescriptionEnumValueParserTests: 19/19
• HeuristicTypeDetectorTests: 149/149
• patch-equivalent rebase onto current main
• git diff --check passed

The prior Ubuntu pipeline failure occurred in the full pipeline step while that run was still active; this updated head starts fresh CI.

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: bda47b1ef7

ℹ️ 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".

[claude]

Code review

Reviewed the follow-up commit (bda47b1e) that addresses the gap flagged in my earlier review.

Previous concern: the Pulumi "repeatable, comma-separated" annotation bug (which produced the Repeatable/CommaSeparated pseudo-enum values) came from the Cobra scraping path (DescriptionEnumValueParser / HeuristicTypeDetector), which the original fix didn't touch or add regression coverage for — only the Docker duration-unit path (CliDocumentationScraperBase.IsDurationUnit) got a test.

Resolution verified: the new tests are correct and pin down real behavior, not just documentation:

• DescriptionEnumValueParserTests.TryParse_Rejects_Repeatability_Annotations — traced through DescriptionEnumValueParser.TryParse: none of the three sample descriptions match ExplicitValuesPattern, QuotedOneOfValuesPattern, or ContextualParenthesizedValuesPattern (the last requires a preceding format(s)/type(s)/mode(s)/level(s) keyword within 40 chars, which none of these have), so TryParse genuinely returns null for these inputs today.
• HeuristicTypeDetectorTests.DetectType_Treats_Repeatability_Annotations_As_StringList — traced through HeuristicTypeDetector.AnalyzeOption: "repeatable" in the description sets acceptsMultipleValues = true, TryDetectEnumFromDescription returns null (per above), and the code falls through to the acceptsMultipleValues branch, correctly returning CliOptionType.StringList with no enum values.

So this isn't just "delete the bad enums and hope" — it's now backed by regression tests against the actual code path that produced the original bug, consistent with how the Docker duration-unit case was handled. Good close-out.

No other issues found in the rest of the diff (generated enum migration to EnumValueAttribute, the new Test-GeneratedEnumAttributes.ps1 CI gate — confirmed the git grep -- 'src/**/*.Generated.cs' pathspec does match nested generated files as intended — and the string/IEnumerable<string> option type changes).

[thomhurst]

Fixed the Ubuntu Docker compatibility failure by updating the regression expectation to the canonical generated CLI value (--progress=plain). Focused Docker compatibility tests pass (4/4); Docker solution builds with 0 errors. Branch updated from main. @codex review

[claude]

Re-review (head 47cfdff9)

Checked this against my previous review comment, which flagged one gap: the Pulumi "repeatable, comma-separated" pseudo-enum bug came from the Cobra scraping path (DescriptionEnumValueParser / HeuristicTypeDetector), not the CliDocumentationScraperBase.IsDurationUnit path that got the only regression test in the original fix.

That gap is resolved. Commit bda47b1e added:

• DescriptionEnumValueParserTests.TryParse_Rejects_Repeatability_Annotations — traced through TryParse: none of the three sample descriptions match ExplicitValuesPattern, QuotedOneOfValuesPattern, or ContextualParenthesizedValuesPattern, so it correctly returns null today.
• HeuristicTypeDetectorTests.DetectType_Treats_Repeatability_Annotations_As_StringList — traced through AnalyzeOption: "repeatable" sets acceptsMultipleValues = true, the enum lookup returns null, and it falls through to CliOptionType.StringList with no enum values.

This is real regression coverage against the code path that produced the original bug, not just deleting the bad output and hoping — consistent with how the Docker duration-unit case was handled.

Since then, 45a8392f fixed the one remaining loose end: DockerBuilderCompatibilityTests.BuilderEnumOptionRendersCanonicalSwitch expected --progress=Plain (the old broken ToString() fallback value) and now correctly expects --progress=plain (the canonical [EnumValue("plain")] value). Confirmed this matches DockerBuilderHistoryLogsProgress.Generated.cs and is consistent with the rest of the file, which already asserted lowercase values elsewhere. The merge commit on top (5512111f) only brings in unrelated main changes to HTTP test infra — nothing from this PR's own diff.

No new issues found. All three review rounds' concerns are addressed:

• Stale [Description] → [EnumValue] migration: correct, verified ParseEnum only reflects on EnumValueAttribute.
• Docker duration-suffix and Pulumi repeatability pseudo-enums: root-caused and tested at the actual detection code paths, not just patched at the generated-file level.
• Test-GeneratedEnumAttributes.ps1 CI gate: confirmed zero remaining [Description( hits in generated files.

One optional, non-blocking thought for the future: the new CI gate is a git grep --fixed-strings '[Description(' textual check. It works today because the generator only ever emits the short attribute form, but it's a heuristic that could silently stop catching regressions if someone ever changes that emission style (e.g. a fully-qualified [System.ComponentModel.Description(...)]). The repo already has a stronger pattern for this class of problem — Test-StageGeneratedChanges.ps1 regenerates and diffs against the checked-in output. If this gate ever needs hardening, folding the enum-attribute check into that regenerate-and-diff flow (rather than a second, independent grep heuristic) would make it structurally impossible to drift, instead of relying on the grep pattern staying in sync with the generator's output format. Not asking for a change now — just flagging it as the more robust shape if this gate ever needs to evolve.

Approving.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 5512111f09

ℹ️ 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".

[claude]

Re-review (head b47bddd0)

Diffed the current PR head against the previously-approved commit (5512111f, approved 2026-07-30T19:33:39Z) — the only change since is b47bddd0, a merge of main (adds an unrelated PipelineCommand/module-selection feature and a dependency bump). It touches none of this PR's own files; the enum/generator diff is byte-identical to what was already reviewed.

Recapping the three rounds already resolved on this PR:

• Root cause confirmed correct: CommandArgumentBuilder.ParseEnum only reflects [EnumValueAttribute], so generated enums still carrying [Description] silently fell back to rawValue.ToString() at the CLI boundary (e.g. AzureNative instead of azure-native). Migrating stale generated files to [EnumValue(...)] fixes real, previously-silent command-arg bugs, not just a rename.
• Docker duration-suffix pseudo-enums: correctly root-caused in CliDocumentationScraperBase.IsDurationUnit, with regression tests (CliDocumentationScraperBaseTests) pinning the fix at the actual detection path.
• Pulumi repeatability pseudo-enums (the gap flagged in my first review round): resolved in bda47b1e with regression tests at both the DescriptionEnumValueParser.TryParse and HeuristicTypeDetector.AnalyzeOption boundaries — traced through and confirmed these assert real behavior (annotation phrases don't match any of the enum-detection regexes, and acceptsMultipleValues correctly routes to StringList), not just documentation of intent.
• CI gate (Test-GeneratedEnumAttributes.ps1): confirmed zero remaining [Description( hits in generated files; the exit-code handling correctly distinguishes "no matches" from a real grep failure.
• Docker compatibility test fix (45a8392f): updated a stale assertion (--progress=Plain → --progress=plain) to match the now-canonical [EnumValue] output — verified against DockerBuilderHistoryLogsProgress.Generated.cs.

No new issues. This PR remains approved — nothing outstanding.

[claude]

Code review — follow-up on latest commit

Re-reviewed the state of the PR since my last pass (bda47b1e). The only substantive change since then is 45a8392f9 ("test(docker): expect canonical enum value"), which fixes the Ubuntu CI failure by updating DockerBuilderCompatibilityTests to expect --progress=plain instead of --progress=Plain.

Verified correct: traced this through CommandArgumentBuilder.ParseEnum (src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs:343) — it resolves enum arguments via the field's EnumValueAttribute, and the current generated DockerBuilderHistoryLogsProgress.Generated.cs declares [EnumValue("plain")] Plain. So at runtime the argument really does serialize to plain, not Plain. The test was asserting stale, pre-fix behavior (accidentally matching the C# enum member name rather than the CLI's actual accepted value) — updating the expectation to the canonical lowercase value is the right fix, not a workaround.

Everything else in the diff (generated enum migration from DescriptionAttribute to EnumValueAttribute, the Docker/Pulumi pseudo-enum → string/IEnumerable<string> conversions, the DescriptionEnumValueParser/HeuristicTypeDetector regression tests for the repeatability-annotation bug, and the new Test-GeneratedEnumAttributes.ps1 CI gate) was already covered and confirmed correct in my earlier review comment — no new concerns there.

No issues found. This is a solid close-out of #3503.