feat(spec): support flag relationships - #793
Conversation
📝 WalkthroughWalkthroughThe PR adds conditional requirements and mutually overriding flags to flag specifications. Parsing applies conditional validation and last-use-wins behavior. Overridden flags do not receive defaults or environment values or trigger required checks. Fixtures and documentation are updated. ChangesFlag specification contracts
Parser behavior
Documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UserInput
participant FlagParser
participant ParseOutput
participant DefaultsAndEnvironment
UserInput->>FlagParser: provide long or short flags
FlagParser->>ParseOutput: remove mutually overriding flags
FlagParser->>ParseOutput: record overridden flag names
DefaultsAndEnvironment->>ParseOutput: skip overridden defaults and environment values
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Greptile SummaryAdds end-to-end support for relationships between flags.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Important Files Changed
Reviews (4): Last reviewed commit: "fix(parse): address flag relationship ed..." | Re-trigger Greptile |
66be83c to
1287658
Compare
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/src/spec/cmd.rs (1)
980-980: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that the fixture preserves
overrides.Line 980 adds the new KDL key, but the round-trip test only compares the original and reparsed serialized values. If both the parser and serializer ignore
overrides, the test still passes. Add an assertion that the serialized--purgeflag containsoverrideswith"-f".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/src/spec/cmd.rs` at line 980, Update the round-trip fixture test for the serialized --purge flag to explicitly assert that its overrides value is "-f". Keep the existing original-versus-reparsed comparison, but add a direct check on the serialized flag so parser and serializer omissions cannot both go undetected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/parse.rs`:
- Around line 1061-1078: Update apply_flag_overrides to accept the pending
flag-value state (flag_awaiting_value) and remove entries whose names are in
overridden_names when displaced flags are removed from parsed_flags. Update
every caller accordingly, and add a regression test covering “--file --stdin
input.txt” that verifies the displaced --file does not consume or reappear with
input.txt.
---
Nitpick comments:
In `@lib/src/spec/cmd.rs`:
- Line 980: Update the round-trip fixture test for the serialized --purge flag
to explicitly assert that its overrides value is "-f". Keep the existing
original-versus-reparsed comparison, but add a direct check on the serialized
flag so parser and serializer omissions cannot both go undetected.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b539bd6-b285-4d0c-94a8-19c9b6036cc8
📒 Files selected for processing (5)
docs/spec/reference/flag.mdlib/src/parse.rslib/src/spec/builder.rslib/src/spec/cmd.rslib/src/spec/flag.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/spec/reference/flag.md
1287658 to
7b00c07
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7b00c07. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/parse.rs`:
- Around line 1067-1073: Update flag_has_env so that when custom_env is
provided, it checks only that map and never falls back to std::env; retain
process-environment lookup only when custom_env is None, keeping
Parser::with_env replacement semantics consistent with Parser::parse.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 63919785-1d47-4bed-b2d5-cffb9598dd24
📒 Files selected for processing (5)
docs/spec/reference/flag.mdlib/src/parse.rslib/src/spec/builder.rslib/src/spec/cmd.rslib/src/spec/flag.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/src/spec/cmd.rs
- docs/spec/reference/flag.md
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|

Summary
overrides,required_if, andrequired_unlessto the flag spec model, KDL parser, serializer, and builderconfigexample from the flag referenceRelationship targets may use a flag's spec name or any short, long, or negated alias. Each property accepts one target, while child nodes support multiple targets.
required_ifactivates when any target is explicitly supplied.required_unlessrequires the flag unless any target is explicitly supplied. Environment-backed targets count as explicit; defaults do not activate conditions, though a default still satisfies the conditionally required flag itself.Root cause
The flag reference documented these relationship keys, but
SpecFlaghad no corresponding fields and the KDL parser rejected them before argument parsing began.Closes #792.
Validation
cargo test --all --all-featurescargo clippy --all --all-features -- -D warningsmise run lint:prettierAI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.
Note
Medium Risk
Changes core CLI parsing (defaults, env, required flags, mount prefix forwarding); behavior is well-tested but any consumer relying on old override/env semantics could see different outcomes.
Overview
Implements flag relationships that were documented but not wired through the spec or parser:
overrides,required_if, andrequired_unlessonSpecFlag, with KDL parse/serialize, builder helpers, and reference doc tweaks (mutual override semantics; drops unsupportedconfigexample).Parsing applies mutual, last-wins overrides when a flag is seen: conflicting flags are removed from parsed state and pending value queues; an internal
overridden_flagsset stops env/defaults from reviving losers and feeds conditional requirement checks. Mount prefix scanning applies the same override rules so globals forwarded to mounts stay consistent (--filedropping--stdin).Conditional requirements extend the missing-flag pass:
required_ifwhen any selector is explicitly present (CLI or env, not defaults);required_unlesswhen none of the selectors are explicit. Overridden flags count as absent for those selectors. Custom env maps no longer fall back to process env when checking env-backed flags/args.Reviewed by Cursor Bugbot for commit 00ca1b0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation