fix(parse): don't leak the mounting CLI's flags into mounted commands; scan past non-global flags - #738
Conversation
…mands A mounted command describes another program, so the flags of the commands it is mounted under are not part of it. They were still inherited on descent, which caused two problems for a CLI like mise (jdx/mise#11282), where everything after a mounted task name is forwarded to the task: 1. Completing after the mounted command offered the mounting CLI's globals (`--env`, `--silent`, `--jobs`, …), which the mounted program rejects. 2. A flag the mounted command declares itself was shadowed by a global of the same name, so its choices/completer were replaced by the global's — mise task flags named `--env` fell back to file completion, while renaming them to `--environment` worked. Commands merged in by a mount are now marked `mounted`, and descending into one keeps the inherited globals recognized for parsing — they may legitimately appear before the mounted command, and Phase 2 re-parses those tokens — while recording them in `ParseOutput::inherited_flag_keys` so completions skip them. `ParseOutput::completion_flags()` returns the set a completion should offer; without a mount in play it is identical to `available_flags`. A global consumed before the mounted command keeps owning its key, so `mycli --env prod run task` still parses as the global even when the task declares `--env` too.
📝 WalkthroughWalkthroughMounted command parsing now tracks mount provenance, preserves flag bindings across parsing phases, and computes completion candidates that honor mounted flag ownership over inherited global flags. Tests, fixtures, and reference documentation cover these rules. ChangesMounted flag completion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ParseOutput
participant SpecCommand
participant MountedCommand
CLI->>ParseOutput: parse partial command line
ParseOutput->>SpecCommand: descend through mount boundary
SpecCommand->>MountedCommand: merge mounted command flags
MountedCommand-->>ParseOutput: provide mounted-owned flag set
ParseOutput-->>CLI: return completion_flags
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 SummaryThis PR adjusts mounted-command parsing and completion behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failures remain. Important Files Changed
Reviews (5): Last reviewed commit: "fix(parse): scope the mount boundary to ..." | Re-trigger Greptile |
`ParseOutput` is constructible with a struct literal, so adding a public `inherited_flag_keys` field was a major-version break (cargo-semver-checks `constructible_struct_adds_field`). The same information is already in `ParseOutput::cmds`: the offered set is the merge of the flags declared from the first mounted command down, so compute it in `completion_flags()` instead of tracking it during the descent. The API is additive again, and `merge_subcommand_flags` keeps its previous shape for the non-mount path.
Phase 1 stopped its subcommand scan at the first non-global flag, so a
subcommand behind one was never reached — and neither was its mount. For mise
that broke completion outright for every non-global `run` flag:
$ usage complete-word ... -- mise run --force build --bump ''
Error: × unexpected word: build
The stated reason was that `cmd --local-flag run` might read `run` as the flag's
value, but that is what the existing "does this flag take an argument" check
handles, and it already governs global flags in the same position. Known
non-global flags are now consumed the same way, minus being forwarded to mounts,
which they never belonged in: they are scoped to the command that declared them.
An unknown flag still stops the scan, since its arity is unknown.
Phase 2 re-parses the words Phase 1 skipped, and the recognized flags change in
between: each descent drops the parent's non-global flags, and a mounted command
may declare the same name as a global seen earlier. Phase 1 therefore records
which flag each skipped word was read as, and Phase 2 resolves those words
through that binding. This replaces the special case that let a consumed global
keep its key through a mount, so a mounted command's flag now always owns its own
name — `mycli --env prod run task --env <TAB>` completes the task's choices while
the leading `--env prod` still parses as the global.
`run`/`tasks run` flags that redeclare a root global and add a short it lacks (`-r`/`--raw`, `-S`/`--silent`) were promoted back to global in the completion spec so the parser would still recognize them before a task name (#10069). jdx/usage#738 scans for the subcommand across any known flag, global or not, and binds each word to the flag it was read as, so the promotion is no longer needed — and it was the reason `--raw`/`--silent` leaked into the flags offered after a task name. Dropping it also fixes the purely-local `run` flags, which no promotion could cover: `mise run --force <task> <TAB>` (and `-f`, `-o <mode>`, `-n`, `-s`, `-t`, `--timeout`, …) used to fail with `unexpected word: <task>`, offering nothing. Covered in the #10069 e2e test.
… flags Two nested-mount cases from review: Marking every command in a mounted tree as `mounted` made each descent inside the tree a mount crossing, where a command's flags override an inherited global of the same name. Within one program that is the wrong rule: a nested command re-declaring a parent's global as non-global shadowed it, and the next descent's `retain(global)` then dropped it entirely. Only the transition from an unmounted command into a mounted one is a boundary now; below it the mounted program's commands are ordinary commands relative to each other, and `completion_flags()` replays the descent through `merge_subcommand_flags` so it follows exactly the same rules as the parse. A mounted spec's root flags are merged into the command the mount sits on, which left them above the boundary and therefore filtered out of completions even though they describe the mounted program. `SpecCommand::flags_from_mount` records that, and the replay starts one level up when it is set.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/src/parse.rs (1)
512-519: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated mount-boundary descent logic.
The
crossing_mountcomputation +merge_subcommand_flagscall is now duplicated between the subcommand-found branch and thedefault_subcommandbranch. This mirrors a pre-existing duplication (both branches already calledmount()/merge_subcommand_flagsbefore this PR), but this change grows it further. A shared helper (e.g.fn descend(out: &mut ParseOutput, subcommand: SpecCommand)) encapsulating mount + crossing_mount + merge + push/assign would reduce the risk of the two copies drifting on this subtle mount-boundary rule in the future.♻️ Sketch of a shared helper
+fn descend_into(out: &mut ParseOutput, mut subcommand: SpecCommand, prefix_words: &[String]) -> Result<(), UsageErr> { + subcommand.mount(prefix_words)?; + let crossing_mount = subcommand.mounted && !out.cmd.mounted; + merge_subcommand_flags(&mut out.available_flags, gather_flags(&subcommand), crossing_mount); + out.cmds.push(subcommand.clone()); + out.cmd = subcommand; + Ok(()) +}Also applies to: 573-578
🤖 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/parse.rs` around lines 512 - 519, Extract the duplicated descent logic from both the subcommand-found and default_subcommand branches into a shared helper, such as descend. The helper should perform mount handling, compute crossing_mount using the mounted-state boundary rule, merge flags, and then push or assign the descended command as required. Replace both existing inline sequences with this helper so the branches cannot drift.
🤖 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.
Nitpick comments:
In `@lib/src/parse.rs`:
- Around line 512-519: Extract the duplicated descent logic from both the
subcommand-found and default_subcommand branches into a shared helper, such as
descend. The helper should perform mount handling, compute crossing_mount using
the mounted-state boundary rule, merge flags, and then push or assign the
descended command as required. Replace both existing inline sequences with this
helper so the branches cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a9f2028f-2ba5-4061-9838-3ce319cad385
📒 Files selected for processing (7)
cli/src/cli/complete_word.rscli/tests/complete_word.rsdocs/spec/reference/cmd.mddocs/spec/reference/flag.mdexamples/mounted-global-flag-leak.shlib/src/parse.rslib/src/spec/cmd.rs
usage 3.5.7 is the release carrying jdx/usage#738, so `min_usage_version` asks for that rather than the 3.6 this PR guessed at before the release was cut. demand 2.0.4 is an unrelated patch bump.
⚠️ **CAUTION: this is a major update, indicating a breaking change!**⚠️ This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [usage](https://github.com/jdx/usage) | tools | major | `3.5.6` → `5.1.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jdx/usage (usage)</summary> ### [`v5.1.0`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#510---2026-08-09) [Compare Source](jdx/usage@v5.0.0...v5.1.0) ##### 🚀 Features - **(spec)** parse usage comments from strings by [@​jdx](https://github.com/jdx) in [#​782](jdx/usage#782) ##### 🐛 Bug Fixes - **(spec)** avoid inferred metadata from included specs by [@​jdx](https://github.com/jdx) in [#​786](jdx/usage#786) ##### 🧪 Testing - **(windows)** make the suite runnable on Windows by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​771](jdx/usage#771) ##### 📦️ Dependency Updates - update rust crate rmcp to v3 by [@​renovate\[bot\]](https://github.com/renovate\[bot]) in [#​780](jdx/usage#780) ### [`v5.0.0`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#500---2026-08-02) [Compare Source](jdx/usage@v4.1.0...v5.0.0) ##### 🚀 Features - **(cli)** allow overriding the shell program with USAGE\_SHELL\_<SHELL> by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​767](jdx/usage#767) ##### 🐛 Bug Fixes - **(cli)** forward parsed args to WSL bash via WSLENV on windows by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​764](jdx/usage#764) - **(cli)** let generate markdown write to stdout by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​766](jdx/usage#766) - **(complete)** use `type -P` so the CLI-presence guard ignores shell functions by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​760](jdx/usage#760) - **(parse)** enforce double\_dash="required" for positional args by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​762](jdx/usage#762) - **(windows)** run `run=` scripts with sh when available by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​765](jdx/usage#765) ##### 🎨 Styling - fix clippy and deprecation warnings in test and bench targets by [@​JamBalaya56562](https://github.com/JamBalaya56562) in [#​763](jdx/usage#763) ### [`v4.1.0`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#410---2026-07-30) [Compare Source](jdx/usage@v4.0.0...v4.1.0) ##### 🚀 Features - **(cli)** declare what each usage command does to the world by [@​jdx](https://github.com/jdx) in [#​751](jdx/usage#751) - **(mcp)** serve a usage spec to an agent over stdio by [@​jdx](https://github.com/jdx) in [#​746](jdx/usage#746) - **(spec)** add a top-level `repository` field by [@​jdx](https://github.com/jdx) in [#​747](jdx/usage#747) ##### 🐛 Bug Fixes - **(parse)** keep a re-declared global's aliases on one flag by [@​jdx](https://github.com/jdx) in [#​752](jdx/usage#752) - complete repeated variadic args by [@​Jai-JAP](https://github.com/Jai-JAP) in [#​753](jdx/usage#753) ##### New Contributors - [@​Jai-JAP](https://github.com/Jai-JAP) made their first contribution in [#​753](jdx/usage#753) ### [`v4.0.0`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#400---2026-07-25) [Compare Source](jdx/usage@v3.6.0...v4.0.0) ##### 🚀 Features - **(spec)** allow effect= on flags and args by [@​jdx](https://github.com/jdx) in [#​742](jdx/usage#742) ### [`v3.6.0`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#360---2026-07-25) [Compare Source](jdx/usage@v3.5.7...v3.6.0) ##### 🚀 Features - **(spec)** add effect= to declare what a command does to the world by [@​jdx](https://github.com/jdx) in [#​739](jdx/usage#739) ##### 🚜 Refactor - **(spec)** make missed SpecCommand fields a compile error, and fix the four that were already missed by [@​jdx](https://github.com/jdx) in [#​740](jdx/usage#740) ### [`v3.5.7`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#357---2026-07-25) [Compare Source](jdx/usage@v3.5.6...v3.5.7) ##### 🐛 Bug Fixes - **(parse)** don't leak the mounting CLI's flags into mounted commands; scan past non-global flags by [@​jdx](https://github.com/jdx) in [#​738](jdx/usage#738) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWFqb3IiXX0=-->
Fixes the parser-side causes of jdx/mise#11282, plus the two follow-ups that turned up while fixing it. mise side: jdx/mise#11284.
Three related defects, all in how Phase 1's subcommand scan and Phase 2's re-parse interact with mounted commands.
1. The mounting CLI's globals leaked into mounted commands
A mounted command describes another program, but the flags of the commands it is mounted under were inherited into it on descent. For a CLI like mise — where everything after a task name is forwarded to the task — that meant:
mise run mytask --<TAB>offered--cd --env --jobs --locked --quiet --raw --silent --verbose --yes(and viamise tasks run,--all --extended --global --json …). Using any of them fails:mise run mytask --silent→ERROR unexpected word: --silent.--envwithchoices "dev" "stage" "prod"completed as file names. Renaming it to--environmentworked — the reporter's workaround.Fix: commands merged in by a
mountare markedSpecCommand::mounted(runtime-only, propagated to their subcommands). Crossing into the mounted tree keeps the inherited globals inavailable_flags— they may legitimately appear before the mounted command — but the mounted command's own flags now take precedence for their own names. Only that crossing is a boundary: inside the mounted tree its commands are ordinary commands relative to each other, so descents there use the normal merge, including #649's rule. A mount can also merge flags from its spec's root onto the command it sits on;SpecCommand::flags_from_mountrecords that so those flags count as the mounted program's. NewParseOutput::completion_flags()returns what a completion should offer: everything recognized, or, once a mounted command is reached, only the flags declared from the mount boundary down. It needs no extra parse state (ParseOutput::cmdsalready records the descent chain), so the public API stays additive andcargo semver-checkspasses. With no mount in play it returnsavailable_flagsunchanged.2. A non-global flag hid the subcommand behind it
Phase 1 stopped scanning at the first non-global flag, so the subcommand — and any mount on it — was never reached, and its name fell through to Phase 2 as a positional:
That hit every non-global
runflag in mise (-f/--force,-n/--dry-run,-o/--output,-s/--shell,-t/--tool,--timeout, …), and mise'spromote_orphan_shortsworkaround couldn't help: those flags have no root global to promote onto.The old rationale was that
cmd --local-flag runmight readrunas the flag's value — but that's what the existing "does this flag take an argument" check handles, and it already governs global flags in the same position. Fix: known non-global flags are consumed like globals and the scan continues; they are just not forwarded to mounts, since they're scoped to the command that declared them. An unknown flag still stops the scan, since its arity is unknown.3. Re-parsed prefix words could bind to the wrong flag
Phase 1 deliberately leaves the words it skips in
inputso Phase 2 records them inout.flags/as_env(). But by then the recognized flags have changed — each descent drops non-globals, and (after fix 1) a mounted command can own a name a global used. Fix: Phase 1 records which flag each skipped word was read as, and Phase 2 resolves those words through that binding. This removed the special case I'd first written for it, so a mounted command's flag now always owns its own name:$ mycli --env prod run task --env <TAB> # task's choices, and --env prod still parses as the globalVerified against a real
mise usagespecmise run mytask --<TAB>--bump --cd --env --jobs --locked --output-dir --quiet --raw --silent --verbose --yes--bump --env --output-dirmise run mytask --env <TAB>mise-tasks/ mise.toml …(files)dev stage prodmise tasks run mytask --<TAB>tasksflags--bump --env --output-dirmise mytask --<TAB>(naked)--bump --env --output-dirmise run --force mytask --bump <TAB>Error: unexpected word: mytaskauto major minor patchmise run -o interleave mytask --env <TAB>Error: unexpected word: mytaskdev stage prodmise --env prod run mytask --env <TAB>dev stage prodmise -C . run mytask --bump <TAB>auto major minor patchmise run -r/-S mytask <TAB>Tests
examples/mounted-global-flag-leak.sh— new fixture: root globals-E/--env,--silent;runwith a non-global-f/--forceand a mount; mountedmytaskdeclaring a colliding--envwith choices.complete_word_mounted_does_not_offer_mounting_cli_flags— end-to-end through a real mount: the offered flag list,--env/--bumpvalue completion, flags still offered before the mount, a prefix global still parsing, the rejected-by-choices prefix value, andrun --force mytask/run -f mytask. Verified failing onmain.complete_word_non_global_flags_stop_search→..._do_not_stop_search: now asserts the mounted task is found behind--local, and that an unknown flag still stops the scan.=forms, plus the after-the-mount case), non-global flags not hiding a subcommand (boolean, short, value-taking, and unknown-flag control), and a non-mounted control assertingcompletion_flags() == available_flags.test_mount_boundary_does_not_apply_inside_the_mounted_tree(three levels, nested command re-declaring the mounted program's own global) andtest_mount_flags_merged_into_the_mounting_cmd_are_offered.mise run lint(incl.cargo semver-checks),mise run render, and the full suite are clean; the only failures locally are two pre-existingnode-dependent cases intests/examples.rs, which also fail onmainin my sandbox.This PR was prepared by an AI coding assistant.
Note
Medium Risk
Touches core partial-parse and flag-merge logic used by shell completion and dynamic mounts; behavior changes are intentional but broad, with heavy test coverage rather than isolated call sites.
Overview
Fixes mounted-command completion and partial-parse behavior (jdx/mise#11282 and related).
Mounted commands: Merged mount subcommands are marked
mounted(and mount-root flagsflags_from_mount). Crossing a mount boundary keeps parent globals recognized for tokens before the task, but mounted flags win on name collisions. NewParseOutput::completion_flags()is what completions should offer—only flags from the mount boundary down—whilecomplete-wordswitches fromavailable_flagsto that API.Phase 1 subcommand scan: Known non-global flags before a subcommand no longer stop the scan (e.g.
run --force mytask), so mounts and task names are still found; only globals are forwarded to mount scripts. Unknown flags still stop the scan.Prefix re-parse: Phase 1 records
prefix_bindingsso Phase 2 keeps early tokens bound to the flag they were first read as when a mounted command later owns the same name.Docs cover global vs non-global flags around mounts; new example script and broad parser/integration tests.
Reviewed by Cursor Bugbot for commit 6eaaf53. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Documentation