fix(requestflag): preserve CLI precedence for inner flags - #116
fix(requestflag): preserve CLI precedence for inner flags#116sylvesterkaczmarek wants to merge 2 commits into
Conversation
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
26ef370 to
0dfa7a8
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dfa7a8a3a
ℹ️ 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".
| func (f *InnerFlag[T]) IsSet() bool { | ||
| return false | ||
| return f.hasBeenSet |
There was a problem hiding this comment.
Keep set state scoped to the current array element
When a repeatable array-of-objects flag creates a second element, an inner flag used on the first element remains globally set. For example, after setting context-management.compact-threshold on the first entry and starting a second entry with another context-management.type, piped context_management.compact_threshold is now skipped because applyStdinDataToFlags checks IsSet() before innerFieldIsSet() can see that the trailing element lacks the field. This regresses the existing per-trailing-element merge behavior; track explicit state per element or retain the innerFieldIsSet decision for array-backed inner flags.
AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 65a9973. Inner flags now check per-element outer state before the generic IsSet() gate, so stdin can fill an unset field on a later array element without overriding an explicitly set field on that element. Added a two-element regression; focused requestflag tests pass.
Summary
Track whether an
InnerFlaghas been explicitly set so piped stdin data cannot overwrite a value the user already supplied on the command line.Problem
ApplyStdinDataToFlagsintentionally skips flags whoseIsSet()method reports true, preserving CLI-over-stdin precedence for ordinary request flags.InnerFlag.IsSet(), however, currently always returns false.That means a command-line value such as
--address.city cli-valuecan be applied successfully to its outer flag and then be overwritten later by piped data containingaddress.city: piped-value.Fix
InnerFlag.Setcalls with per-flag state;InnerFlag.IsSet();The state is tracked per inner flag rather than inferred from the outer flag, so setting one nested field does not incorrectly suppress stdin values for sibling fields.
Regression coverage
Added a focused regression that:
The test also asserts the inner flag now reports itself as set after a successful explicit assignment.
Validation
The branch is based directly on current upstream
main(ee92673a416c0851a5fe8907a2453db7bd450633) and contains one signed commit touching only the inner-flag implementation and its focused regression test. Full Go test execution is left to repository CI.Risk
Low. Only successfully assigned inner flags change
IsSet()from false to true; unset inner flags and ordinary request flags retain their existing behavior.