Skip to content

feat(spec): support flag relationships - #793

Merged
jdx merged 2 commits into
mainfrom
agent/docs-remove-unsupported-flag-keys
Aug 10, 2026
Merged

feat(spec): support flag relationships#793
jdx merged 2 commits into
mainfrom
agent/docs-remove-unsupported-flag-keys

Conversation

@jdx

@jdx jdx commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • add overrides, required_if, and required_unless to the flag spec model, KDL parser, serializer, and builder
  • apply mutual, last-occurrence-wins override semantics during argument parsing
  • enforce presence-based conditional requirements after overrides are resolved
  • account for command-line flags, environment values, and defaults consistently
  • remove the remaining unsupported config example from the flag reference

Relationship 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_if activates when any target is explicitly supplied. required_unless requires 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 SpecFlag had no corresponding fields and the KDL parser rejected them before argument parsing began.

Closes #792.

Validation

  • cargo test --all --all-features
  • cargo clippy --all --all-features -- -D warnings
  • mise run lint:prettier

AI-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, and required_unless on SpecFlag, with KDL parse/serialize, builder helpers, and reference doc tweaks (mutual override semantics; drops unsupported config example).

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_flags set 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 (--file dropping --stdin).

Conditional requirements extend the missing-flag pass: required_if when any selector is explicitly present (CLI or env, not defaults); required_unless when 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

    • Added support for defining mutually overriding flags, including single or multiple overrides.
    • Explicitly selected flags now take precedence over overridden flags, preventing conflicting defaults, environment values, and required-flag errors.
    • Added conditional requirements and exceptions for flags.
    • Added support for reading and writing flag override and conditional requirement settings in specifications.
  • Documentation

    • Updated override examples to clarify that the last-used flag takes precedence.
    • Removed outdated configuration and flag relationship examples.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Flag specification contracts

Layer / File(s) Summary
Declare and serialize flag relationships
lib/src/spec/flag.rs, lib/src/spec/builder.rs, lib/src/spec/cmd.rs
Adds required_if, required_unless, and overrides fields, builder methods, KDL parsing, serialization, Clap initialization, and round-trip tests.

Parser behavior

Layer / File(s) Summary
Resolve relationships during parsing
lib/src/parse.rs
Tracks overridden flags, applies last-use-wins behavior for long and short flags, filters mount prefixes, evaluates conditional requirements, and suppresses displaced defaults, environment values, and required checks. Tests cover these cases.

Documentation

Layer / File(s) Summary
Update flag reference
docs/spec/reference/flag.md
Documents last-use-wins behavior and removes unsupported examples.

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
Loading

Possibly related PRs

  • jdx/usage#746: Changes related flag-resolution behavior in lib/src/parse.rs.
  • jdx/usage#762: Changes ParseOutput and parser exit-state propagation.

Suggested reviewers: jambalaya56562

Poem

A rabbit sets conditions clear,
The latest flag stays near.
Old flags leave the parsing trail,
Defaults skip the flags that fail.
The spec records each rule with care.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes support the documented overrides key and implement mutual, last-occurrence-wins behavior required by issue #792.
Out of Scope Changes check ✅ Passed The changes remain within the stated flag-relationship objectives, including parsing, serialization, builders, validation, and documentation updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding support for flag relationships in the specification and parser.

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

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

@greptile-apps

greptile-apps Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds end-to-end support for relationships between flags.

  • Extends SpecFlag, its builder, and KDL parsing/serialization with overrides, required_if, and required_unless.
  • Implements last-occurrence-wins override handling across command-line parsing, environment values, defaults, and mounted-command prefixes.
  • Enforces presence-based conditional requirements and adds regression coverage and reference documentation.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
lib/src/parse.rs Implements override resolution, conditional requirement checks, environment-presence handling, and relationship-aware mount-prefix forwarding with focused tests.
lib/src/spec/flag.rs Adds relationship fields to the flag model and supports their KDL parsing, serialization, clap conversion defaults, and round-trip tests.
lib/src/spec/builder.rs Adds fluent builder methods for override and conditional-requirement relationships.
lib/src/spec/cmd.rs Extends the serialization fixture to exercise and verify the new relationship fields.
docs/spec/reference/flag.md Updates the flag reference to describe mutual last-occurrence-wins overrides and removes an unsupported config example.

Reviews (4): Last reviewed commit: "fix(parse): address flag relationship ed..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot previously approved these changes Aug 9, 2026
@jdx
jdx force-pushed the agent/docs-remove-unsupported-flag-keys branch from 66be83c to 1287658 Compare August 9, 2026 22:19
@greptile-apps
greptile-apps Bot dismissed their stale review August 9, 2026 22:19

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@jdx jdx changed the title docs(spec): remove unsupported flag keys feat(spec): support flag overrides Aug 9, 2026
Comment thread lib/src/parse.rs

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
lib/src/spec/cmd.rs (1)

980-980: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert 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 --purge flag contains overrides with "-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

📥 Commits

Reviewing files that changed from the base of the PR and between 66be83c and 1287658.

📒 Files selected for processing (5)
  • docs/spec/reference/flag.md
  • lib/src/parse.rs
  • lib/src/spec/builder.rs
  • lib/src/spec/cmd.rs
  • lib/src/spec/flag.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/spec/reference/flag.md

Comment thread lib/src/parse.rs
@jdx
jdx force-pushed the agent/docs-remove-unsupported-flag-keys branch from 1287658 to 7b00c07 Compare August 9, 2026 22:24
@jdx jdx changed the title feat(spec): support flag overrides feat(spec): support flag relationships Aug 9, 2026

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ 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.

Comment thread lib/src/parse.rs

@coderabbitai coderabbitai 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.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 1287658 and 7b00c07.

📒 Files selected for processing (5)
  • docs/spec/reference/flag.md
  • lib/src/parse.rs
  • lib/src/spec/builder.rs
  • lib/src/spec/cmd.rs
  • lib/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

Comment thread lib/src/parse.rs Outdated
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▁▆▆▆▇▆▆█▇▇▇▆▆▆▇▇ 110,556,714 → 110,556,053 -0.00% 10.82 → 11.02ms +1.87%
startup ▁▁▂▁▃▃▁▄▅██▅▅▆▆▄ 1,202,172 → 1,198,906 -0.27% 0.89 → 0.94ms +4.91%

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.

00ca1b0af1dd vs f274af850209 · measured on the runner, not pushed to the history.

@jdx
jdx merged commit 63bc8d3 into main Aug 10, 2026
9 checks passed
@jdx
jdx deleted the agent/docs-remove-unsupported-flag-keys branch August 10, 2026 00:07
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.

Documented but unsupported flag key overrides

1 participant