Skip to content

feat(derive): say that a command cannot be run alone, which it knew and did not write - #937

Merged
jdx merged 1 commit into
mainfrom
claude/derive-subcommand-required
Aug 17, 2026
Merged

feat(derive): say that a command cannot be run alone, which it knew and did not write#937
jdx merged 1 commit into
mainfrom
claude/derive-subcommand-required

Conversation

@jdx

@jdx jdx commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Stacked on #923 — review that first; this PR's diff is the last commit. One of the two gaps #936 found by converting usage-cli itself to the derive; the other one gets its own PR.

A bare T subcommand field requires a subcommand and an Option<T> does not. The parser has always refused the invocation accordingly — that half was never missing — but Spec::to_kdl wrote nothing, so the emitted KDL described usage generate as a command a user could type on its own.

That reaches past help text: docs, manpages, completions and the SDK generators all read the emitted spec rather than the Rust type, so every one of them was offering a command that cannot run.

cmd "settings" help="Manage settings" subcommand_required=#true {

Shape

Cold metadata rather than a parse table — subcommand_required is not how a word binds, and the hot path never reads it. CommandMeta gains a bool, the derive fills it from the field's type, and the writer emits it only where there is something to require: the property on a command with no subcommands is a spec the linter reports and nothing could satisfy, so both conditions are held.

gen-shadow already read subcommand_required from a KDL and chose T against Option<T> accordingly, so mise's shadow round-trips it now instead of losing it on the way back out.

Verification

  • conformance/tests/subcommand_required.rs — four tests: a strict command says it, a loose one stays quiet and reads back as false, a leaf never claims it, and the parser and the spec agree about which invocations exist.
  • nesting's snapshot picks the change up on a fixture that was already shaped this way, which is the check that this is emission rather than a new rule.
  • Workspace suite green, clippy clean.

Not fixed here

The spec accepts subcommand_required only inside a cmd block, so a root that cannot run alone still has no way to say so — the same restriction as the root mount, recorded next to it in PLAN.md. The clap bridge could not say it either, so nothing regressed.

This PR was generated by Claude Code.


Note

Low Risk
Cold-path metadata and KDL emission only; parsing behavior was already correct, with added compile-time validation for an invalid flatten shape.

Overview
Emits subcommand_required in specs so docs, completions, and generators match parser behavior: a non-optional #[usage(subcommand)] field sets cold metadata and writes subcommand_required=#true only when the command actually has subcommands; optional fields stay omitted and read back as false.

Compile-time guard via flatten_checks: a #[usage(flatten)] type that still declares subcommands fails const evaluation, because flatten would leave the parent unable to express required subcommands while the nested build still demands one.

Tests and docs: new conformance/tests/subcommand_required.rs, updated nesting snapshot (settings gains the flag), and PLAN.md marks the derive/spec gap done while noting root-level subcommand_required remains unsupported in KDL.

Reviewed by Cursor Bugbot for commit 8ca7f41. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features

    • Command specifications now identify when a subcommand is required.
    • Help, documentation, completions, and generated specifications consistently reflect required versus optional subcommands.
    • Nested required-subcommand behavior is supported.
  • Bug Fixes

    • Improved parsing and specification output for commands with required or optional subcommands.
  • Documentation

    • Documented the remaining limitation that root-level required subcommands cannot currently be represented.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 16dd7ed0-8776-4459-97a8-01749cb48ab2

📥 Commits

Reviewing files that changed from the base of the PR and between eb35780 and a2b02d4.

⛔ Files ignored due to path filters (1)
  • conformance/tests/snapshots/nesting__the_emitted_spec_reads_like_a_handwritten_one.snap is excluded by !**/*.snap
📒 Files selected for processing (4)
  • PLAN.md
  • argv/src/spec.rs
  • conformance/tests/subcommand_required.rs
  • derive/src/codegen.rs

Included review availability: Your plan includes up to 4 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The derive code now computes required-subcommand metadata for root and nested commands. CommandMeta stores the value, and KDL serialization emits it for applicable commands. Conformance tests cover required, optional, and leaf command hierarchies.

Changes

Subcommand requirement metadata

Layer / File(s) Summary
Command metadata and KDL serialization
argv/src/spec.rs
CommandMeta includes subcommand_required. KDL output emits the property only for commands with required subcommands.
Derived requirement generation
derive/src/codegen.rs
Root and nested command generation detect non-optional subcommand fields and pass the result into CommandMeta.
Conformance coverage and milestone tracking
conformance/tests/subcommand_required.rs, PLAN.md
Tests verify required, optional, and leaf command behavior. The plan records nested support and the root-level specification limitation.

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

Merge Risk: 🔵 Low · up to a2b02

The change adds a public CommandMeta field, which can break downstream code using complete struct literals; merge is appropriate with explicit release-version or migration guidance.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI declaration
  participant DeriveCodegen
  participant CommandMeta
  participant KDLSerializer
  CLI->>DeriveCodegen: define subcommand fields
  DeriveCodegen->>CommandMeta: emit subcommand_required
  CommandMeta->>KDLSerializer: provide command metadata
  KDLSerializer-->>CLI: serialize required-subcommand flag
Loading

Possibly related PRs

  • jdx/usage#905: Both changes update derive code generation and CommandMeta emission for command-level metadata.

Poem

A rabbit hops through nested commands,
Marking which paths must run below.
Metadata blooms in KDL,
Tests check every route they know.
Root-level limits remain in tow.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the derive change that emits metadata for commands that require a subcommand, although its wording is somewhat verbose.

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 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR aligns derived command metadata with parser behavior by emitting subcommand_required for commands containing a required subcommand field and rejecting flattened groups whose subcommands cannot be routed.

  • Adds subcommand_required to cold command metadata and conditionally serializes it to KDL.
  • Derives requiredness from bare versus optional subcommand fields.
  • Adds a compile-time guard against flattened groups that declare subcommands.
  • Adds conformance coverage and updates the nesting snapshot and implementation plan.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported flattened-subcommand mismatch is prevented by a compile-time assertion on each flattened group, and no blocking failure remains.

Important Files Changed

Filename Overview
derive/src/codegen.rs Derive output now records required subcommands and the compile-time flatten guard resolves the previously reported metadata/parser mismatch.
argv/src/spec.rs Cold command metadata gains a default-false requiredness field that is emitted only for commands with satisfiable subcommands.
conformance/tests/subcommand_required.rs End-to-end tests cover strict, optional, leaf, parsing, and emitted-spec behavior.
conformance/tests/snapshots/nesting__the_emitted_spec_reads_like_a_handwritten_one.snap Snapshot correctly records requiredness for the existing nested strict command.
PLAN.md Documents the completed nested-command fix and the intentionally deferred root-command limitation.

Reviews (5): Last reviewed commit: "feat(derive): say that a command cannot ..." | Re-trigger Greptile

Comment thread derive/src/codegen.rs
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▁████████ 180,301,068 → 180,465,907 +0.09% 17.50 → 16.49ms -5.79%
startup ▁▆▂▂▅█▆██ 1,226,319 → 1,226,289 -0.00% 1.00 → 1.01ms +0.83%

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.

Shadow comparison

Parsing mise use -g node@20 against a shadow of mise's committed spec.
Reported, not gated: the shadow grows as the derive learns to express more, so
what to watch is the ratio rather than either column.

usage clap ratio
instructions, cold parse 60439 5893640 97x
usage: argv -> struct                            1053 ns      1.05 µs
clap: build tree + parse -> struct             500627 ns    500.63 µs
clap: parse -> struct, tree reused              22862 ns     22.86 µs
clap: build tree only                          310992 ns    310.99 µs

8ca7f41e0745 vs 02e459bc675b · measured on the runner, not pushed to the history.

@jdx
jdx force-pushed the claude/derive-subcommand-required branch from ae69396 to eb78f7b Compare August 17, 2026 00:33
An error occurred while trying to automatically change base from agent/unit-variants to agent/msrv August 17, 2026 01:19
@jdx
jdx changed the base branch from agent/unit-variants to main August 17, 2026 01:30
@jdx
jdx force-pushed the claude/derive-subcommand-required branch from eb78f7b to a2b02d4 Compare August 17, 2026 01:34
@jdx
jdx force-pushed the claude/derive-subcommand-required branch from a2b02d4 to 4332d6f Compare August 17, 2026 01:54

jdx commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Addressed the P1, and it was real — worse than reported, in fact.

A #[usage(flatten)] group that declares subcommands of its own does not merely report subcommand_required=false: the group's subcommands never reach the parent's table at all, so the command is unreachable in both directions. Reduced to a probe on the branch before this fix:

cmd "parent" {
}

bare parent parses: false

The spec says parent is a runnable leaf; the parser rejects it; and no word could satisfy it, because leaf is in no table. That is flatten's own hole rather than something this PR introduced — this PR would just have printed a confident false over it.

Refused at compile time, the way Option<T> flatten already is: joining two commands' subcommand sets needs a rule for which enum a word selects from, and nothing in the fleet asks for one. flatten_checks asserts the group's COMMAND has no subcommands, during const evaluation in the parent's expansion — the same trick find_subcommand uses for default_subcommand, and what lets one expansion check a fact about another it cannot see:

error[E0080]: evaluation panicked: a flattened group cannot declare subcommands: flatten
joins flags and arguments into the parent's tables and leaves subcommands behind, so the
command would require one that no word could select. Declare the `subcommand` field on the
command's own struct instead.
  --> src/main.rs:10:10
   |
10 | #[derive(Args)]
   |          ^^^^

It points at the parent's derive, which is where the flatten is written.

The legitimate shape is untouched — conformance/tests/flatten.rs has a command with both a subcommand field and a flattened group, and mise's ten flattens are all flags and arguments. Workspace suite green, clippy clean under --all-targets, gate green.

There is no compile-fail harness in this repo, so the refusal is verified by hand and documented at the top of conformance/tests/subcommand_required.rs and in PLAN.md, next to the entry it protects.

This comment was generated by Claude Code.

…nd did not write

A bare `T` subcommand field requires a subcommand and an `Option<T>` does not.
The parser has always refused the invocation accordingly — that half was never
missing — but `Spec::to_kdl` wrote nothing, so the emitted KDL described
`usage generate` as a command a user could type on its own.

That reaches past help text. Docs, manpages, completions and the SDK generators
all read the emitted spec rather than the Rust type, so every one of them was
offering a command that cannot run.

Cold metadata, not a parse table: `subcommand_required` is not how a word binds,
and the hot path never reads it. Written only where there is something to
require, since `subcommand_required` on a command with no subcommands is a spec
the linter reports and nothing could satisfy.

Found by converting usage-cli itself to the derive and diffing the spec it
prints against the one the clap bridge printed. `nesting`'s snapshot picks the
change up on a fixture that was already shaped this way, which is the check that
this is emission rather than a new rule.

Not fixed here, and recorded in PLAN.md beside the root mount it resembles: the
spec accepts the property only inside a `cmd` block, so a root that cannot run
alone still has no way to say so. The clap bridge could not say it either, so
nothing regressed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jdx
jdx force-pushed the claude/derive-subcommand-required branch from 4332d6f to 8ca7f41 Compare August 17, 2026 02:00
@jdx
jdx merged commit 2541fab into main Aug 17, 2026
11 checks passed
@jdx
jdx deleted the claude/derive-subcommand-required branch August 17, 2026 04:26
pull Bot pushed a commit to rozsazoltan-forks/usage-rust-cli that referenced this pull request Aug 17, 2026
…a tree (jdx#939)

Stacked on jdx#937 (and so on jdx#923) — review those first; this PR's diff is
the last commit. The second of the two gaps jdx#936 found by converting
usage-cli itself to the derive.

usage-lib resolves this by walking outward from the command that ran and
falling back to the spec's own setting — `effective_unknown_flags` in
`lib/src/parse.rs`. usage-argv held the effective value per command
instead, on the theory that whoever built the tables would resolve it.

A derive cannot. It expands one struct at a time and cannot see the
command above, so `#[usage(unknown_flags = "error")]` reached the root
alone — and on an `Args` it was parsed and then **ignored**, a
declaration that compiled and did nothing. A CLI could not make a
subcommand strict, or a single command lenient, whatever it wrote.

```rust
#[usage(bin = "usage", unknown_flags = "error")]  // now reaches the whole tree
struct Cli { … }

#[usage(unknown_flags = "value")]                 // now means something on an Args
struct Bash { … }                                 // `usage bash x.sh --script-flag`
```

Without it, `usage lint --nope f.kdl` offers `--nope` to the positional,
so the *file* becomes `--nope` and the real file is the unexpected word
— where clap named the flag.

## Shape

`Command::unknown_flags` is an `Option` now: `None` means inherit. The
parser carries the effective value down as it descends rather than
reading it off the command in scope — one branch on descent, and a field
read instead of a pointer chase on the two sites that ask.

The KDL writer follows: a command that states nothing writes nothing,
one that restates what it inherited writes nothing, and one that differs
says so. Same output as before for every spec that already
round-tripped.

**The corpus's table builder stops resolving inheritance on the way
in**, which is the part worth noticing: `conformance/src/argv.rs` had
its own correct implementation of the rule, and that is exactly why no
vector ever caught the parser not having one. One implementation now, in
the parser, where the reference has it.

## Cost

**160 instructions per parse — 72,272 against 72,112** at mise's scale,
measured twice against the parent branch on one machine. cachegrind is
deterministic; both figures repeated.

## Verification

- `conformance/tests/unknown_flags_inherit.rs` — five tests: a
subcommand inherits the root's answer, it reaches two levels down, a
command that declares its own keeps it, the emitted spec says what the
parser does, and **usage-lib reads the emitted spec the same way**,
which is the claim that matters.
- `a_subcommand_writes_unknown_flags_only_where_it_differs` gains a
command that declares nothing at all, which is the case the `Option`
adds.
- Workspace suite green, clippy clean, corpus green.

_This PR was generated by Claude Code._

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes core argv binding for unrecognized flags across the whole CLI
tree; behavior fixes are intentional but affect every parse path that
relied on the old pre-resolved tables.
> 
> **Overview**
> **`unknown_flags` now inherits down the command tree** instead of
being baked into each command table. `Command::unknown_flags` is
`Option<UnknownFlags>` (`None` = inherit); the parser keeps an effective
mode on descent and only updates it when a subcommand sets one.
> 
> The derive emits `None` for commands that do not declare the attribute
(fixing root-only `error` and ignored `#[usage(unknown_flags)]` on
`Args`). KDL emission and the conformance table builder stop flattening
inheritance on build—only the parser applies the rule, matching
usage-lib’s outward walk.
> 
> New `conformance/tests/unknown_flags_inherit.rs` covers inheritance
depth, per-command `value`, spec round-trip, and usage-lib parity on
emitted KDL.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
2ff6307. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
jdx added a commit that referenced this pull request Aug 17, 2026
…#965)

Stacked on #963, the `usage-rs` facade.

`usage` is now its own first adopter. Its command structs, root, and
command enums use the `usage-rs` facade instead of Clap, and
`--usage-spec` prints `Cli::to_kdl()` from the same tables that parsed
the command line.

## What goes away

- `clap`, `clap_usage`, and the `clap-sort` dev dependency
- `tests/clap_sort.rs`; declaration order is held by the spec
- the duplicated command-effect tables; effects now live on the commands
they describe
- the empty `Sponsors` struct; unit subcommands are supported directly
- runtime checks for relationships the spec can express

## Migration shape

The declarations remain close to their former Clap layout:

- inferred shorts use `#[usage(short, long)]`
- command aliases live on their command structs, e.g. `#[usage(alias =
"c", alias_hidden("complete", "completions"))]`
- positive dependencies use `requires`, including `--cache-key` →
`--usage-cmd` and `--out-dir` → `--multi`
- aliases written on enum variants still merge with struct aliases for
compatibility
- deliberately formatted docs keep `#[usage(verbatim_doc_comment)]`
- path hints keep `value_hint = usage_rs::ValueHint::FilePath` or
`DirPath` from usage’s own runtime type
- subcommand payloads stay unboxed as they were under Clap; `Box<T>`
remains an optional size optimization

The four shell commands are separate derived structs flattening a shared
group. A single destination struct cannot identify which enum variant
selected it, so a macro keeps their common declaration in one place.

## Parity gaps closed by the stack

- required subcommands survive KDL generation (#937)
- strict unknown-flag behavior is inherited by subcommands (#939)
- command-owned visible and hidden aliases are supported (#946)
- duplicate non-repeatable flags are rejected (#945)
- a missing required subcommand prints the available command choices
(#947)
- verbatim doc-comment layout is preserved on commands, fields, and
variants (#949)
- file and directory value hints reach native and emitted completions
(#951)

The emitted spec still intentionally gains metadata the Clap bridge
dropped, including `JDX_USAGE_BIN` on `--usage-bin`, preserved multiline
long help, the manpage description, and the correct binary name `usage`.

## Verification

- `mise run render`
- `cargo test --all --all-features`
- `cargo clippy --all --all-features --all-targets -- -D warnings`
- direct binary checks for no-argument help, duplicate flags, command
aliases, and positive requirements

_This PR was generated by Codex._

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes how the primary `usage` CLI is parsed and documented
(including `--completions` UX), but behavior is covered by updated
integration tests and is an intentional migration to the shipped parser.
> 
> **Overview**
> The **`usage` binary dogfoods `usage-rs`**: command structs use
`#[derive(usage_rs::Cli)]` / `#[usage(...)]` instead of Clap, and
**`--usage-spec` emits `Cli::to_kdl()`** from the same parse tables that
handle argv. **Clap, `clap_usage`, and `clap-sort` are removed**, along
with the post-hoc **`command_effects` patch table**—`effect`,
`requires`, `overrides`, and similar metadata now live on the
declarations.
> 
> **Root CLI shape is corrected in spec and docs**: **`--completions
<SHELL>` is a long flag** (not a positional), with an **explicit
multi-line `usage` synopsis** for `usage <COMMAND>` / `--completions` /
`--usage-spec`. Shell runner commands (`bash`, `fish`, `zsh`,
`powershell`, `exec`) use **`unknown_flags = "value"`** so script
arguments can include unknown flags.
> 
> **Help and manpages** gain support for an optional root **`usage`
string** on `Spec` (derive + KDL emission); root help/man synopsis
prefer that over generated lines. Generated assets (`usage.usage.kdl`,
`usage.1`, Fig spec, CLI reference markdown, snapshots) are refreshed to
match.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
3aba1aa. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
  * Added `--completions <SHELL>` for selecting a completion shell.
* Improved command validation, aliases, defaults, environment settings,
and file-output completion hints.
  * Shell commands now forward unrecognized arguments as script values.
  * Added clearer help text and descriptions for generation commands.

* **Documentation**
* Updated CLI references, manpages, usage specifications, and shell
documentation to reflect revised options and behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Replaces #936 after flipping the facade and CLI layers.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant