Skip to content

fix(generator): preserve gcloud command and resource semantics - #5181

Open
thomhurst wants to merge 8 commits into
mainfrom
issue-5180-gcloud-semantics
Open

thomhurst wants to merge 8 commits into
mainfrom
issue-5180-gcloud-semantics

Conversation

@thomhurst

@thomhurst thomhurst commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Problem

Review of gcloud snapshot #5179 found shared enums with incompatible command choices, unconditional requirements across provider branches, resource switches inferred as boolean flags, and incorrect secret masking. That snapshot is closed; these fixes update generator source before regeneration.

Changes

  • Scope gcloud enums to their command through OptionEnumFactory, preserving exact CLI values.
  • Ignore wrapped resource-help references when locating flag declarations.
  • Preserve named provider/authentication branches and use optional resource bundles in the synopsis to scope conditional requirements.
  • Mask inline private-key content even when help says it is not a file path; keep resource identifiers visible and retain explicit secret overrides.
  • Add six unmodified SDK 585.0.0 help captures and runtime validation covering all eight Developer Connect providers, incomplete configurations, mutually exclusive providers, HTTP authentication, and optional resource selectors.

Validation

  • Guarded generator solution Release build: zero warnings/errors.
  • All 3,174 OptionsGenerator tests passed in CI on 0ec5abf; 1,095 selected parser/masking tests also passed locally.
  • 26 focused group/runtime regressions passed, including certificate revocation and existing mandatory resource groups.
  • Scoped dotnet format applied and verified (existing SYSLIB1045/CA1861 excluded); no generated integration files or integration API baselines changed. The handwritten generator baseline includes the new Optional enum member.

After this merges, regenerate gcloud from main and carry the four handwritten test constructor adaptations from e868123 into the fresh replacement snapshot.

Part of #5180. Keep #5180 and #5178 open until the fresh snapshot, constructor adaptations, and full command coverage are verified.

CI dependency

Includes the test-only commit from #5182 to validate the distributed-cache timeout correction concurrently. That separate PR retains the five-second overlap deadline and adds completion diagnostics. #5182 is merged; #5101 tracks the recurrence.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-16T14:55:34.847114Z 24d56ab New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d4f2dc2c-4e10-486d-af8b-8f06c7da8c33

📥 Commits

Reviewing files that changed from the base of the PR and between 10a6f65 and 3961ab4.

📒 Files selected for processing (2)
  • test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The generator updates gcloud secret classification, provider-group parsing, optional requirements, and command-specific enum generation. New SDK 585 fixtures and regression tests cover these changes. A distributed executor test adds bounded timing and failure diagnostics.

Changes

Gcloud semantic preservation

Layer / File(s) Summary
Secret and resource metadata
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Generators/GeneratorUtils.cs, .../TypeDetection/OptionTypeEnhancer.cs, .../Tests/*
Secret detection excludes file paths and resource identifiers while preserving explicit secret overrides. Tests cover private keys, token values, and resource references.
Provider group parsing and validation
.../Models/CliArgumentGroup.cs, .../Scrapers/Cli/*, .../Tests/Scrapers/Cli/*
The parser recognizes named bundles and wrapped switch references. Optional resource groups do not receive presence requirements. Developer Connect configurations remain separate conditional branches.
Command-specific enum generation
.../Scrapers/Cli/GcloudCliScraper.cs, .../Tests/Fixtures/Gcloud/585.0.0/*, .../Tests/Scrapers/Cli/*
Enum creation uses command and switch context. Firestore and SQL Edition values remain distinct, and generated enum names use command-specific types.

Distributed executor test diagnostics

Layer / File(s) Summary
Concurrency test timing and diagnostics
test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs
The test uses a longer overall timeout and a separate overlap deadline. Scheduler events, cleanup completion, execution status, thread-pool count, and pending work are recorded.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GcloudHelpFixture
  participant UsageSynopsisParser
  participant CliArgumentGroupParser
  participant GcloudCliScraper
  participant GeneratedValidation
  GcloudHelpFixture->>UsageSynopsisParser: parse command synopsis
  UsageSynopsisParser->>GcloudCliScraper: optional resource option groups
  GcloudHelpFixture->>CliArgumentGroupParser: parse provider declarations
  CliArgumentGroupParser-->>GcloudCliScraper: named provider groups
  GcloudCliScraper->>GeneratedValidation: create optional alternative constraints
  GeneratedValidation-->>GcloudCliScraper: validate selected provider branch
Loading

Merge Risk: ⚪ Minimal · up to 3961a

The generator and test changes have no established merge-blocking failure in the supplied evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main generator changes that preserve gcloud command and resource semantics.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-5180-gcloud-semantics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks each flag in line
Resource names stay non-secret by design
Provider branches form a careful chain
Enum values keep their proper name
Tests watch the scheduler run
Diagnostics record when work is done

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

Reviewed the generator/scraper source changes (not the fixture captures or generated integration files) against base main. This is a well-scoped, surgical bug-fix PR that addresses each issue named in #5180:

1. Shared enum collisions (GcloudCliScraper.cs)TryDetectEnum now builds the enum via OptionEnumFactory.TryCreate(className, propertyName, switchName, values) where className includes the full command path, replacing the old CreateEnumDefinition that produced a single Gcloud{PropertyName} enum shared across every command. Confirmed by the updated fixtures in GcloudCapturedSemanticsTests.Captured_Command_Enums_Preserve_Different_Edition_Values, which shows firestore databases create --edition and sql instances create --edition now get distinct enum types with their own (previously colliding) value sets. Existing tests (GcloudDelimitedListTests, GcloudGroupDescriptionTests, NestedArgumentGroupParsingTests) were correctly updated to the new scoped enum names rather than weakened.

2. Wrapped resource-help references (CliArgumentGroupParser.cs) — new WrappedArgumentReferencePattern skips a flag declaration when the preceding line is a bare "provide the argument" bullet, preventing a wrapped long-switch reference from being misread as a boolean flag declaration. Covered by Captured_Resource_References_Do_Not_Declare_Boolean_Flags.

3. Provider/authentication branch + optional resource scoping (UsageSynopsisParser.GetOptionalResourceOptionGroups, GcloudCliScraper.MarkOptionalResourceGroups, CliArgumentGroupParser.BeginNamedBundle) — the new Optional CliArgumentGroupKind flag is derived from the synopsis (colon-delimited resource bundles nested in [...]) and threaded through ApplyRequiredGroups/CreateAlternativeConstraint so a bundle that is optional in the synopsis is no longer forced mandatory just because its prose says "must be specified." BeginNamedBundle keeps named provider branches (e.g. GitHub vs GitLab vs Bitbucket configs) as distinct groups instead of merging them. The new developer-connect connections create fixture plus Captured_Provider_Configurations_Remain_Separate_Optional_Branches and the runtime Captured_Provider_Requirements_Are_Conditional_At_Runtime test (28 validation cases across all 8 providers) give this good end-to-end coverage, including the CMEK crypto-key resource group that stays a real required-if-touched bundle.

4. Secret masking (GeneratorUtils.cs, OptionTypeEnhancer.cs)IsFilePathOption now matches a path to regex after stripping a not ... path to negation, so a description like "not the path to the file" no longer short-circuits secret detection via the old naive Contains("path to") check (this was the actual root cause of PrivateKey content being treated as a non-secret file path). New IsResourceIdentifierOption keeps "ID of the X or fully qualified identifier for X"-style resource selectors visible even though they end in Id. Both directions are covered in GeneratorUtilsTests.IsSecretOption_Distinguishes_Material_From_Resource_Identifiers_And_Paths and OptionTypeEnhancerTests.Resource_Identifiers_Clear_Inferred_Secrets_But_Respect_Overrides (the latter also confirms an explicit manual-override secret classification still wins over the inferred resource-identifier exemption).

Repository conventions — no hand-edited generated options, no PublicAPI.*.txt churn, changes are confined to generator/scraper source, new unmodified SDK fixture captures, and tests, consistent with CLAUDE.md. No outstanding review threads or requested-changes reviews are present on the PR to reconcile.

I did not execute the build/test suite myself in this review; the PR description reports a clean Release build and the full generator test suite (1,095 + 26 tests) passing, and the diffs are internally consistent with the referenced OptionEnumFactory/GeneratorUtils APIs.

No actionable findings.

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed CodeFactor's Complex Method finding in af13552, confirmed at the remote head. Secret inference now has a separate IsInferredSecret helper; ApplySecretMetadata retains explicit overrides, key-filtered secrets, and boolean handling. All 225 masking/enhancer/captured-help tests pass, and scoped formatting was applied. CodeFactor will reassess the new commit; no finding was suppressed.

@thomhurst

Copy link
Copy Markdown
Owner Author

@coderabbitai The linked-issue warning describes the required next phase. Repository guidance requires the generator fix on main before regeneration, so this source PR must land first. I removed the automatic closing keyword: #5180 and #5178 stay open until fresh workflow output preserves the command coverage, all five fixes are verified in generated output, and the four handwritten constructor adaptations are included. The saved test commit is e868123.

The docstring percentage includes private parser helpers and regression tests. Existing repository style uses descriptive test names and comments for non-obvious parsing rules; I retained that style rather than adding boilerplate XML comments solely to reach the advisory metric. No public library API documentation changed. Your completed source review reports no actionable findings.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@thomhurst Thanks for the clarification.

The source PR can merge before the regeneration phase. Keeping #5180 and #5178 open until fresh generated output verifies the required coverage is correct.

The documentation metric is advisory in this case. The affected code is private parser and test code, and no public API documentation changed. No XML documentation changes are required.


✏️ Learnings added
Learnt from: thomhurst
URL: https://github.com/thomhurst/ModularPipelines/pull/5181

Timestamp: 2026-09-16T13:27:03.593Z
Learning: For the ModularPipelines OptionsGenerator regeneration work, the generic generator fixes must merge to `main` before regenerated workflow output is produced. Keep linked issues `#5180` and `#5178` open until the fresh workflow output preserves command coverage, verifies all five fixes in generated output, and includes the four handwritten resource-test constructor adaptations.

Learnt from: thomhurst
URL: https://github.com/thomhurst/ModularPipelines/pull/5181

Timestamp: 2026-09-16T13:27:03.593Z
Learning: In the ModularPipelines OptionsGenerator codebase, documentation-percentage metrics include private parser helpers and regression tests. Use descriptive test names and comments for non-obvious parsing rules. Do not add boilerplate XML documentation solely to satisfy this advisory metric when public library API documentation is unchanged.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@github-actions

Copy link
Copy Markdown
Contributor

test 1 2 3

@thomhurst

Copy link
Copy Markdown
Owner Author

The Linux failure occurred after both cache lookups had overlapped and the held lookup resumed within 13.4 ms. Separate PR #5182 retains the five-second overlap deadline while giving executor shutdown the normal test budget, with additional completion diagnostics. Its test-only commit is included here as 10a6f65 so CI can validate the generator changes concurrently. All 254 distributed tests pass locally; a serialized-executor negative control fails all 50 repetitions at the retained five-second deadline. #5101 remains open until Linux validation completes.

@github-actions

Copy link
Copy Markdown
Contributor

Review

Reviewed the generator-source changes (GeneratorUtils.cs, CliArgumentGroup.cs, CliArgumentGroupParser.cs, GcloudCliScraper.cs, UsageSynopsisParser.cs, OptionTypeEnhancer.cs) plus the new/updated fixtures and tests. This looks like a solid, well-targeted fix set:

  • Enum scoping via OptionEnumFactory: gcloud now builds enum names from GenerateClassName([ToolName, .. commandParts]) instead of the old flat Gcloud{propertyName} scheme, so commands with the same option name but different choice sets (e.g. --edition on firestore databases create vs sql instances create) no longer collide. Captured_Command_Enums_Preserve_Different_Edition_Values in GcloudCapturedSemanticsTests.cs directly asserts the distinct names/values, which is the right way to lock this in.
  • Optional resource bundles: MarkOptionalResourceGroups + UsageSynopsisParser.GetOptionalResourceOptionGroups correctly cross-reference the synopsis's bracket/pipe structure against the FLAGS-section argument groups to stop treating optional resource selectors as unconditionally required, while still leaving ArgumentIsConditionallyRequired members as an alternative (non-required) constraint. The new Captured_Provider_Requirements_Are_Conditional_At_Runtime table (24 cases covering all 8 Developer Connect providers, nested "at most one"/"at least one" sub-bundles, and the crypto-key-config resource) is a good way to pin down this logic at the validation-attribute level rather than just the parse tree.
  • Secret/path/resource-identifier heuristics: replacing the naive description.Contains("path to") with a negation-aware regex (stripping not (the|a) path to before matching) fixes the mismasking case where certificate/key content descriptions explicitly say they are not a path. Pairing that with the new IsResourceIdentifierOption check (and threading it into both IsSecretOption and OptionTypeEnhancer.IsInferredSecret) correctly keeps resource IDs (e.g. --private-connection, TokenAuthUser) visible while still masking real secret material (--private-key). The parametrized IsSecretOption_Distinguishes_Material_From_Resource_Identifiers_And_Paths test and OptionTypeEnhancerTests.Resource_Identifiers_Clear_Inferred_Secrets_But_Respect_Overrides both exercise this well, including the explicit-override precedence case.
  • Wiring IsSecret onto positional arguments (ParsePositionalArguments) reuses the existing CliPositionalArgument.IsSecretOptionsClassGenerator masking path rather than inventing a new one, which is the right call.

Nothing blocking. One very minor nit: in GcloudCliScraper.ApplyRequiredGroups, the removed // Optional bundles still require their mandatory members when any member is supplied. comment explained non-obvious behavior (why a conditionally-required member still contributes a non-required alternative constraint) — worth keeping for future readers, but not worth blocking on.

The unrelated DistributedModuleExecutorTests.cs timeout/diagnostics change is called out in the PR description as the test-only commit merged in from #5182 for concurrent CI validation, so it's expected to be here and isn't reviewed as part of this PR's own scope.

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed the comment-retention suggestion in review #5181 (comment) with de90e73, confirmed in the remote head. Restored the explanation that an optional bundle still requires its mandatory members when supplied. This is a one-line comment change; scoped generator formatting passed.

@github-actions

Copy link
Copy Markdown
Contributor

Review: PR #5181 — fix(generator): preserve gcloud command and resource semantics

I reviewed the generator/scraper source changes (GeneratorUtils.cs, CliArgumentGroup.cs, CliArgumentGroupParser.cs, GcloudCliScraper.cs, UsageSynopsisParser.cs, OptionTypeEnhancer.cs) against the base branch, cross-checked them against the new/updated tests, and confirmed no other reviewer has an open actionable finding on this PR (CodeRabbit reports "No actionable comments"; Codex review completed with no blocking comments).

What the diff actually does

  • Enum scoping (GcloudCliScraper.CreateOptionDefinition): enum creation now goes through the existing OptionEnumFactory.TryCreate(className, propertyName, switchName, values) instead of the old CreateEnumDefinition(propertyName, values) that produced a bare Gcloud{PropertyName} name. The className passed in (GenerateClassName([ToolName, .. commandParts])) reconstructs the exact same class name used for the owning command (commandPath at line 133, since commandParts = commandPath.Skip(1)), so the enum name is now correctly scoped per-command. This mirrors the identical convention already used by AwsCliScraper, CobraCliScraper, ChocolateyCliScraper, DotNetCliScraper, TerraformCliScraper, and YarnCliScraper, so it's consistent with the rest of the codebase rather than a one-off pattern. Test updates in GcloudDelimitedListTests.cs / GcloudGroupDescriptionTests.cs confirm the rename (GcloudTestTypeGcloudStorageDiagnoseTestType).

  • Secret vs. file-path vs. resource-identifier classification (GeneratorUtils.IsSecretOption / IsFilePathOption / new IsResourceIdentifierOption): IsFilePathOption previously did a naive description.Contains("path to"), which misfired on gcloud descriptions like "needs to be the content of the certificate file, not the path to the file" — such text would be (wrongly) treated as "this is a file path" and left unmasked, even though it documents inline secret material. The fix strips a not (the|a) path to negation before testing for path to, and adds a separate IsResourceIdentifierOption regex to keep resource-reference descriptions ("ID of the X or fully qualified identifier...") visible without conflating them with the file-path check. GeneratorUtilsTests.cs adds direct table-driven cases for exactly this ambiguity, which is the kind of case that's easy to silently regress.

  • Optional resource bundles (UsageSynopsisParser.GetOptionalResourceOptionGroups + GcloudCliScraper.MarkOptionalResourceGroups): a new synopsis walk marks argument groups whose switches exactly match an optional bracketed resource bundle in the synopsis, and ApplyRequiredGroups / CreateAlternativeConstraint now suppress the mandatory/required-alternative constraint for groups flagged Optional. This is what lets developer-connect connections create treat --private-connection/--psc-service-attachment/--static-ip-connectivity as optional instead of forcing a "must choose one" validation. The new NestedArgumentGroupParsingTests.ProviderConfigurations.cs test is a strong regression check here: it round-trips a real captured developer-connect connections create fixture through the actual generator and exercises IValidatableObject.Validate against ~20 property combinations across all 8 providers, mutually-exclusive branches, and the HTTP auth sub-choice.

  • Named provider/authentication bundles (CliArgumentGroupParser.BeginNamedBundle, WrappedArgumentReferencePattern, NamedBundleHeadingPattern): handles two related help-text quirks — long switch names that wrap the "+ provide the argument" bullet onto its own line (so the wrapped continuation isn't mistaken for a new flag declaration), and headings like "Defines the configuration for..."/"Bearer token authentication with..." that should start a new named branch inside an existing choice group without merging into a sibling's indentation. Logic is intricate but consistent with this file's existing heuristic style, and it's covered end-to-end by the same Developer Connect fixture/test plus the other NestedArgumentGroupParsingTests.

  • Positional-argument secrets: GcloudCliScraper now also runs GeneratorUtils.IsSecretOption for positional arguments (previously only named options got this treatment), closing a gap where a secret-bearing positional would not have been masked.

  • DistributedModuleExecutorTests.Cache_Lookups_For_Ready_Modules_Run_Concurrently: unrelated to the generator fix, pulled in as a CI dependency per the PR description. It keeps the original 5s deadline for the actual concurrency assertion (via a linked CancellationTokenSource) but gives the post-assertion shutdown/drain phase the standard test timeout budget, and adds progress/diagnostic recording on failure. Reasonable fix for flakiness under CI load (Distributed cache concurrency test times out in Linux CI #5101); doesn't weaken the assertion it was protecting.

Observations (non-blocking)

CliArgumentGroupParser's heuristic surface (regex-driven heading classification, BeginNamedBundle, indentation bumping in BeginArgumentGroup) keeps growing in a way that's hard to reason about locally — e.g. BeginArgumentGroup's indentation++ special case for IsNamedBundle groups is only obvious once you've read BeginNamedBundle first. This isn't a regression introduced by this PR (the file already had this style), and the new behavior is well covered by the Developer Connect end-to-end test, but if more gcloud commands keep needing bespoke heading patterns, it may be worth pulling the heading→group-kind mapping into a small documented table so the next fix doesn't require re-deriving the indentation state machine from scratch.

No correctness issues found in the reviewed diff, and no unaddressed prior review feedback remains open on this PR.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3961ab4d00

ℹ️ 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
thomhurst deployed to Pull Requests September 16, 2026 14:57 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

Review

I reviewed the generator/scraper source changes (GeneratorUtils.cs, CliArgumentGroup.cs, CliArgumentGroupParser.cs, GcloudCliScraper.cs, UsageSynopsisParser.cs, OptionTypeEnhancer.cs), the new Optional bundling logic, and the accompanying fixtures/tests, against base main. This is a well-scoped fix that addresses each issue named in #5180, and the previous review rounds on this PR (CodeRabbit, Codex, and three prior automated passes) are already resolved — I independently re-verified rather than just trusting those summaries.

Enum scoping (GcloudCliScraper.TryDetectEnumOptionEnumFactory.TryCreate(className, propertyName, switchName, values)): the className is built from the full command path, so --edition on firestore databases create and sql instances create no longer collide on a shared GcloudEdition type. This mirrors the convention already used by AwsCliScraper/CobraCliScraper/etc., so it's consistent rather than a one-off. Locked in by Captured_Command_Enums_Preserve_Different_Edition_Values and the renamed expectations in GcloudDelimitedListTests/NestedArgumentGroupParsingTests.

Optional resource bundles (UsageSynopsisParser.GetOptionalResourceOptionGroups, GcloudCliScraper.MarkOptionalResourceGroups, ApplyRequiredGroups/CreateAlternativeConstraint): the new CliArgumentGroupKind.Optional flag (16, correctly non-colliding with the existing 1/2/4/8 bit flags) is derived from the synopsis's bracketed/colon-delimited resource bundles and threaded through so a bundle that's optional in the synopsis is no longer forced mandatory by prose alone. Captured_Provider_Requirements_Are_Conditional_At_Runtime is a strong test here — it exercises the generated IValidatableObject.Validate() at runtime across all 8 Developer Connect providers, mutually-exclusive branches, and partial-configuration cases, not just the parse tree.

Secret/path/resource-identifier classification (GeneratorUtils.IsFilePathOption/IsResourceIdentifierOption, OptionTypeEnhancer.IsInferredSecret): replacing the naive Contains("path to") with a negation-stripping regex fixes descriptions like "content of the certificate file, not the path to the file". I traced the refactored IsInferredSecret helper against the original inline inferredSecret expression in ApplySecretMetadata — same boolean semantics, just with the added !IsResourceIdentifierOption guard plus the complexity fix already flagged by CodeFactor and addressed in a follow-up commit. GcloudCapturedSemanticsTests and the new OptionTypeEnhancerTests case correctly distinguish resource-identifier text ("ID of the X or fully qualified identifier...") from actual secret material.

Wrapped resource-help references (CliArgumentGroupParser, WrappedArgumentReferencePattern): skipping a flag declaration when preceded by a bare "provide the argument" bullet correctly stops a wrapped long-switch reference from being misread as a boolean flag; covered by Captured_Resource_References_Do_Not_Declare_Boolean_Flags.

One non-blocking observation: ResourceIdentifierDescriptionPattern/NamedBundleHeadingPattern are fairly narrow, phrase-specific regexes tailored to the captured gcloud fixtures in this PR. That's consistent with this repo's "current tool output is authoritative" approach and is well-covered by targeted tests, but future gcloud help-text rewordings outside these exact phrasings will likely need similar follow-up patches rather than being caught generically — worth keeping in mind for anyone regenerating gcloud again later, not something to change here.

The bundled test-only commit in DistributedModuleExecutorTests.cs (from #5182, to validate the distributed-cache timeout fix concurrently) is unrelated to the generator changes but is transparently disclosed in the PR description as a CI dependency, and the diff itself (separating the 5s overlap deadline from a 30s completion/cleanup budget) looks correct in isolation.

No actionable findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant