fix(generator): avoid masking non-secret CLI metadata

[thomhurst]

CLI metadata such as MaxResults, token durations, password lengths, and public enum choices was inferred to be secret. Passing a page size of 1 could register the common digit 1 for log masking; documented choices could similarly hide ordinary words.

The generator now recognizes count descriptions and complete metadata suffixes such as Length, Duration, and Size. Identifier boundaries keep actual secret names such as SecretMessage from being mistaken for age metadata. Type enhancement clears stale inferred masking for these fields and public enum choices. Actual credential values and explicit secret overrides retain their masking.

Regression coverage includes all 31 numeric metadata fields found in the AWS CLI 2.36.46 snapshot, including the original seven pagination cases, eight additional newly generated fields, and 16 existing fields. The tests exercise parsing, metadata enhancement, and generated attributes. Separate cases protect real credentials, identifier boundaries, and explicit overrides for numeric and enum options.

Validation: the expanded baseline failed 24 of 31 metadata cases before the follow-up. All 506 focused tests now pass. Scoped formatting and the Release generator solution build pass with zero warnings/errors.

Refs #5169 and closed generated PR #5168. Fresh AWS/gcloud generation will run from main after this source fix passes CI/reviews and merges.

Summary by CodeRabbit

• Bug Fixes
  • Numeric and metadata-related options, such as counts, sizes, durations, and validity periods, are no longer incorrectly masked as secret values.
  • Credential-related options and explicitly marked secrets continue to receive secret protection.
  • Documented enum choices remain visible unless an option is explicitly identified as secret.
  • AWS CLI-generated numeric options retain their numeric types and standard command-line option metadata.

[chatgpt-codex-connector]

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-16T08:20:08.784404Z	716cec4	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]

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: ca98d6c0-4510-4b32-85d0-c78f3ed073f3

📥 Commits

Reviewing files that changed from the base of the PR and between 9242018 and 716cec4.

📒 Files selected for processing (1)

• test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs

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

---

📝 Walkthrough

Walkthrough

The generator now classifies numeric metadata and documented enum choices separately from secret values. It adds AWS CLI fixtures and tests, and strengthens a distributed cache lookup concurrency test.

Changes

Secret classification updates

Layer / File(s)	Summary
Metadata classification tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Generators/GeneratorUtils.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Generators/GeneratorUtilsTests.cs	IsSecretOption now excludes numeric metadata and matching count descriptions. Tests cover metadata descriptions, numeric secret values, and credential metadata names.
Secret inference rules tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/OptionTypeEnhancer.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/TypeDetection/OptionTypeEnhancerTests.cs	Secret inference excludes enum-backed and metadata options. Tests verify explicit secret overrides and documented enum choices.
AWS option validation tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/AwsCli/aws-2.36.46-numeric-secret-options.json, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.NumericSecrets.cs	AWS numeric option fixtures and scraper tests verify int? types, absent [SecretValue] attributes, and present [CliOption] attributes.

Cache concurrency test

Layer / File(s)	Summary
Cache lookup ordering test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs	The test controls release of the first cache lookup and asserts that execution remains incomplete until that lookup is released.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AwsCliScraper
  participant OptionTypeEnhancer
  participant GeneratedOption
  AwsCliScraper->>OptionTypeEnhancer: Parse numeric option metadata
  OptionTypeEnhancer->>OptionTypeEnhancer: Exclude metadata from secret inference
  OptionTypeEnhancer->>GeneratedOption: Generate int? option with CliOption and without SecretValue

Loading

Merge Risk: ⚪ Minimal · up to 716ce

The change is supported by focused regression coverage and does not show a concrete merge-blocking failure.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing masking of non-secret CLI metadata in the generator.
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.

• 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-5169-pagination-secret-detection

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.}

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

[thomhurst]

The docstring-coverage warning in comment 5693600709 covers named regression tests and private detector helpers. The tests follow the surrounding TUnit convention; comments document fixture provenance and why count descriptions must not inherit secrecy from later token/password references. No public library API changed, and the Release build has zero documentation warnings.

CodeRabbit also reports that clone-backed analysis was skipped. Repository fetch and the isolated checkout succeeded locally; this PR's focused tests, formatting, and source build have been validated there. CodeRabbit's diff review reports no actionable findings, and Codex completed its review at 8b316eb. Claude review and platform CI are still running.

[thomhurst]

The first Claude review job completed without posting a review. Its log records 19 denied tool calls and no buffered inline comments. Retried run 35068018122 so the review result can be published before merge; this does not change the source head.

[thomhurst]

Expanded the fix in 9242018, confirmed on the remote PR head. The AWS snapshot audit found 31 numeric metadata fields carrying SecretValue: seven pagination counts, eight additional new model-token/session-size fields, and 16 existing credential metadata fields. Public enum choices also carried inferred masking.

The updated detector handles complete metadata suffixes and count descriptions; type enhancement clears stale inferred flags and excludes public enum choices. Actual credential values and explicit secret overrides remain masked. All 506 focused tests pass, including all 31 captured numeric cases and explicit overrides. Scoped formatting and the Release generator build pass with zero warnings/errors.

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 9242018445

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

Claude's fresh-head workflow 35070617303 completed successfully but again produced no review comment or inline review. Both attempts on the prior head also completed without publishing a review. I have checked all comment/review surfaces; there is no Claude finding to address and no review job still running.

The available reviews on 9242018 are Codex's completed review with no findings (5694010557) and CodeRabbit's completed diff review with no actionable findings (5693600709, with its documented clone limitation). Local validation passes 506 tests and a clean Release build. Merge will wait for the remaining platform checks and any new findings.

[thomhurst]

Linux CI failed in the existing distributed cache concurrency test (#5101): both lookups started and release was signaled, but the test-forced asynchronous continuation did not resume before timeout. I am addressing its release-gate synchronization separately, then will bring the fix into this branch and run fresh CI. Generator checks passed.

[thomhurst]

Applied the separate test fix from #5171 in 716cec4, now confirmed on this remote PR head. It removes the forced thread-pool hop from the held cache lookup's release gate, retaining the overlap handshake, 50 repetitions, cancellation cleanup, and five-second timeout. The distributed suite passes all 254 tests with coverage, and the Release core build has zero warnings/errors.

Fresh CI is running. The superseded failing run 35070619377 was cancelled so its remaining macOS job does not consume capacity. #5171 must merge first; the generator fix will then have its original scope against main.

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 716cec4e18

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

[github-actions]

Code Review

Traced the secret-masking changes (GeneratorUtils.IsSecretOption/IsSecretMetadataOption, OptionTypeEnhancer.ApplySecretMetadata) line-by-line against the new and existing unit tests, including all 31 AWS snapshot fixture entries and the new GeneratorUtilsTests/OptionTypeEnhancerTests cases. The precedence rules (documented enum choices win over metadata suffixes, explicit overrides win over inference) are internally consistent and match the described behavior. Also reviewed the DistributedModuleExecutorTests concurrency test change for #5101. I did not find a correctness bug that should block this PR. Two small maintainability nits worth a look, non-blocking:

1. Boundary-check helpers have diverged. GeneratorUtils.EndsWithIdentifierSegment (used for the SecretMetadataSuffixes check, e.g. Age/Duration/Seconds) treats any character after an underscore as a valid segment start:

   && (start == 0 || propertyName[start - 1] == '_'
       || (char.IsUpper(propertyName[start]) && (...)))

   while the sibling ContainsIdentifierSegment (used for the Otp/Pwd checks) always requires the segment itself to start with an uppercase letter, even after an underscore. In practice this looks unreachable today — every propertyName reaching these methods comes through ToPascalCase/ToPascalCasePreservingWordBoundaries, which never emit underscores — so it's not a live bug. But the two helpers encode the same concept ("is this suffix/segment at a real identifier-word boundary?") with different rules, which makes it easy for a future suffix addition to silently pick up the looser behavior. Consider extracting one shared boundary predicate (e.g. IsIdentifierBoundaryStart(string s, int index)) that both EndsWithIdentifierSegment and ContainsIdentifierSegment call, so there's a single definition of "valid segment boundary" instead of two that can drift.

2. IsSecretMetadataOption is computed twice per option in OptionTypeEnhancer (tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/OptionTypeEnhancer.cs:225-239): once transitively inside hasSecretKeyword's call to IsSecretOption, and again explicitly in the inferredSecret expression with identical arguments. Not a behavior bug (both calls get the same inputs), but it's wasted work every generator run and, more importantly, the duplication is exactly the kind of thing that lets the two call sites drift apart later (one gets updated with an extra argument/condition, the other doesn't). Computing var isMetadataOption = GeneratorUtils.IsSecretMetadataOption(option.PropertyName, option.Description); once and threading it into both checks (or having IsSecretOption accept the precomputed flag) would remove the duplication and the drift risk in one move.

On the test side, Cache_Lookups_For_Ready_Modules_Run_Concurrently's releaseFirst TaskCompletionSource intentionally drops RunContinuationsAsynchronously (per its inline comment and the PR history referencing #5101/#5171), so TrySetResult resumes the held cache lookup inline instead of hopping through the thread pool. That's a deliberate, documented fix for the original flakiness, not an oversight — flagging only so it stays on the radar if the test flakes again, since inline continuations mean the exact yield point still depends on whatever the executor's cache-completion path awaits downstream (e.g. await using scope disposal in TryRestoreCachedResultAsync).

Nice test coverage on the metadata-vs-secret edge cases (enum choices, count/duration suffixes, description-based inference) — the fixture-driven AWS snapshot audit in particular gives good confidence the reclassification didn't regress the 16 existing credential fields while fixing the 15 metadata false positives.

[thomhurst]

The cache synchronization fix is now on main through #5171 (6c1341e). The final tree difference between main and this head contains only the six generator/fixture files.

Linux CI passed on 716cec4, including all 254 distributed tests. Codex (5694360812) and CodeRabbit (5693600709) completed their reviews of this head with no actionable findings. The existing docstring-warning disposition applies to the internal helpers and tests; no new public library API is involved. Claude job 35072805004 completed without publishing a review, consistent with the previously documented limitation. No review threads or review-body findings remain open.

Waiting for the final macOS check, then merge and fresh AWS/gcloud generation.