Skip to content

fix(generator): preserve gcloud certificate revocation alternatives - #5173

Merged
thomhurst merged 2 commits into
mainfrom
issue-5172-gcloud-revoke
Sep 16, 2026
Merged

thomhurst merged 2 commits into
mainfrom
issue-5172-gcloud-revoke

Conversation

@thomhurst

@thomhurst thomhurst commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Problem and behavior

Gcloud SDK 585 generation passed coverage but failed to compile GcloudPrivatecaCertificatesRevokeOptions: the parser made SerialNumber a required integer and generated a null comparison against that value. The CLI instead requires exactly one of a certificate name or a hexadecimal serial identifier.

Anchor a nested choice just below its heading so differently indented resource and flag branches remain alternatives, while heading-level peers remain outside the choice. Preserve the existing depth for Or branches. Treat serial-number switches as textual identifiers. Box scalar presence checks so required value types compile, and omit redundant conditional validation for single-member bundles.

Captured-help tests compile the generated options and verify both valid identifier paths, neither/both rejection, and issuer-pool dependencies. A separate regression checks nullable and required integer presence, including zero.

Validation

  • Baseline: captured help fails requiredness; required integer validation reproduces CS0037; nullable control passes.
  • 672 parser tests passed.
  • 14 focused generated-validation and nested-group tests passed, including the review regression for heading-level peers.
  • Scoped formatting at severity info passed.
  • Release generator solution: zero warnings/errors.

Fixes #5172. Generated source is regenerated separately after merge.

Failure evidence: https://github.com/thomhurst/ModularPipelines/actions/runs/35076388294/job/104730191342

@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-16T10:09:20.804617Z 69e5c75 Manual request
ℹ️ 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: cfa51715-3159-4e5c-9136-69bab3a3ee40

📥 Commits

Reviewing files that changed from the base of the PR and between 424d0a0 and 69e5c75.

📒 Files selected for processing (2)
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/NestedArgumentGroupParsingTests.CertificateRevocation.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliArgumentGroupParser.cs

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


📝 Walkthrough

Walkthrough

The change fixes gcloud certificate revocation option generation. It preserves certificate and serial-number alternatives, treats serial numbers as text, adjusts nested group parsing, fixes generated presence checks, and adds focused fixtures and validation tests.

Changes

Certificate revocation generation

Layer / File(s) Summary
CLI alternative and identifier parsing
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/Gcloud/585.0.0/gcloud-privateca-certificates-revoke.txt, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliArgumentGroupParser.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs
The fixture documents certificate revocation alternatives. Nested choice groups use adjusted indentation. Switches ending in -serial-number are treated as textual identifiers.
Generated presence and group validation
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Generators/OptionsClassGenerator.cs
Per-member required checks are limited to non-choice groups with multiple properties. Presence checks box value types before testing for null.
Generated validation coverage
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/NestedArgumentGroupParsingTests.CertificateRevocation.cs
Tests cover valid and invalid certificate-revocation combinations, zero-valued integer members, optional values, and the generated gcloud tool definition.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 69e5c

The generator changes and focused regression coverage do not show an unresolved merge-blocking issue.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 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 The PR addresses the coding requirements in #5172. The captured help and tests cover the Certificate/SerialNumber exactly-one alternative, including neither and both cases. `GcloudCliScraper.IsTex…
Out of Scope Changes check ✅ Passed The changed fixture, regression tests, parser logic, gcloud scraper logic, and options generator logic directly support #5172. The supplied evidence identifies no unrelated implementation or test chan…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing generator handling to preserve gcloud certificate revocation alternatives.
  • 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-5172-gcloud-revoke

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 reads the revoke command bright
Certificate or serial, one takes flight
Hex strings hop as text through the gate
Zero values now validate straight
Nested choices land in order right
Tests guard the path through the night

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

@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: 424d0a04d8

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

Copy link
Copy Markdown
Owner Author

CodeRabbit's docstring warning concerns existing private implementation helpers and the new named tests. The changed parser and boxing behavior have adjacent explanatory comments; no public library API or documentation warning was added. Local scoped formatting and the Release generator solution pass with zero warnings/errors, alongside 672 parser tests and 13 focused validation/group tests. The completed diff review reports no actionable findings.

The AWS generation hold remains in place; this source PR will merge only after its checks/reviews pass and the existing AWS output is handled.

@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: 69e5c756b4

ℹ️ 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 10:14 — with GitHub Actions Active
@thomhurst
thomhurst merged commit ef66105 into main Sep 16, 2026
17 checks passed
@thomhurst
thomhurst deleted the issue-5172-gcloud-revoke branch September 16, 2026 11:05
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.

Generator: preserve gcloud certificate revocation alternatives and serial identifiers

1 participant