refactor(cli): bucket per-command --help by moving cross-cutting flags off global by jdx · Pull Request #505 · endevco/aube

jdx · 2026-05-03T19:20:19Z

Summary

Moves --frozen-lockfile/--no-frozen-lockfile/--prefer-frozen-lockfile, --registry + --fetch-*, and --disable/--enable-global-virtual-store off global = true and into per-command Args groups (crates/aube/src/cli_args.rs) with next_help_heading directives. They now appear only on commands that consume them, under a "Lockfile" / "Network" / "Virtual store" heading.
Hides 7 pnpm-compat noop flags (--workspace-packages, --ignore-workspace, --include-workspace-root, --aggregate-output, --stream, --use-stderr, --yes) — still parseable, no longer cluttering --help.
Keeps pre-subcommand placement working (aube --frozen-lockfile install, aube --registry=URL install, etc.) via a small lift_per_subcommand_flags argv pre-pass that shifts the moved-off-global flag tokens past the subcommand position before clap sees argv. Mirrors the existing extract_config_overrides rewriter.
Workspace selection (-F/-r/--filter-prod/--workspace-root/--fail-if-no-match) deliberately stays on Cli. aube -r run build muscle memory is load-bearing and the per-command move would have touched ~15 more command files for a smaller win.

Result

Before, aube ls --help showed ~30 alphabetically-interleaved flags including registry tuning, lockfile policy, retry timeouts, and pnpm-compat shims. After:

```
$ aube install --help
…
Lockfile:
--frozen-lockfile
--no-frozen-lockfile
--prefer-frozen-lockfile
Network:
--fetch-retries
--fetch-retry-factor
--fetch-retry-maxtimeout
--fetch-retry-mintimeout
--fetch-timeout
--registry
Virtual store:
--disable-global-virtual-store
--enable-global-virtual-store
```

aube ls --help no longer shows any of those — list doesn't flatten the install-trigger groups.

Caveat

Implicit-script invocations like aube --frozen-lockfile dev (where dev is a package.json script) no longer behave the same: the rewriter now lifts --frozen-lockfile past dev and clap's External catch-all captures it as a script arg. Users should write aube run --frozen-lockfile dev instead. Documented in the rewriter's docstring.

Test plan

cargo build
cargo clippy --all-targets -- -D warnings
cargo fmt --check
cargo test (all 399 unit + integration tests pass, including a new pre_subcommand_registry_lifts_to_install covering the rewriter)
Manual aube install --help, aube ls --help inspection
bats suite (mise run test:bats) — runs in CI

Closes #345

Note

Medium Risk
Touches CLI argument parsing and adds an argv-rewrite pre-pass, so regressions could cause flags to be misrouted or no longer accepted in some invocation forms (especially pre-subcommand and implicit-script cases). No core install/registry logic changes beyond how overrides are plumbed in.

Overview
Refactors the CLI flag surface so cross-cutting options (lockfile policy, registry/fetch tuning, and global-virtual-store toggles) are defined as reusable clap::Args groups in cli_args.rs and flattened only into commands that actually consume them, keeping per-command --help focused.

Adds a lift_per_subcommand_flags argv pre-pass in main.rs to keep aube --registry=... <cmd> / aube --frozen-lockfile <cmd> working after those flags moved off global=true, and updates affected commands to call the new install_overrides() wiring for registry/fetch/lockfile/virtual-store overrides.

Hides several pnpm-compatibility/no-op flags (e.g. --aggregate-output, --stream, --use-stderr, --yes, and workspace-compat toggles) so they remain parseable but no longer clutter help output, and removes the old scoped registry override guard in favor of the new override plumbing.

^{Reviewed by Cursor Bugbot for commit 17cd38d. Bugbot is set up for automated code reviews on this repo. Configure here.}

greptile-apps · 2026-05-03T19:26:25Z

Greptile Summary

This PR refactors the CLI flag surface by moving lockfile, network/fetch, and virtual-store flags from global = true on Cli into per-command clap::Args groups, and adds a lift_per_subcommand_flags argv pre-pass to preserve pre-subcommand placement (aube --frozen-lockfile install, etc.).

P1 \u2014 argv rewriter gap with combined short flags: lift_per_subcommand_flags handles -F and -F=val but not -rF <space-separated-val>. When it sees -rF, it treats the whole token as a boolean, so the filter value is mistaken for the subcommand position and the loop exits before scanning the LIFTED flag that follows. The PR description explicitly calls out -rF as muscle memory that should keep working.

Confidence Score: 4/5

Safe to merge modulo the combined -rF short-flag gap in the argv rewriter, which is a regression for a documented usage pattern

One confirmed P1 regression: aube -rF --frozen-lockfile breaks post-merge. All per-command install_overrides() wiring looks correct, scoped_registry_override removal is properly replaced, and tests cover happy paths. Score capped at 4 by the P1.

crates/aube/src/main.rs — lift_per_subcommand_flags short-flag handling block (lines ~458–487)

Important Files Changed

Filename	Overview
crates/aube/src/main.rs	Core of the refactor; argv rewriter has a gap with combined short flags like -rF
crates/aube/src/cli_args.rs	New LockfileArgs/NetworkArgs/VirtualStoreArgs groups with install_overrides() wiring; clean
crates/aube/src/commands/add.rs	Properly propagates all three groups through run_global -> run_global_inner; fixes prior --registry-drop
crates/aube/src/commands/install/mod.rs	Struct fields added; install::run() relies on run_install_command to call install_overrides()
crates/aube/src/commands/run.rs	RunArgs and ScriptArgs both get all three groups; overrides wired correctly
crates/aube/src/commands/npm_fallback.rs	FallbackArgs owns NetworkArgs; registry propagated correctly; trailing_var_arg interaction benign
crates/aube/src/commands/mod.rs	scoped_registry_override guard removed cleanly; replaced by per-command install_overrides
test/cli.bats	Updated to verify hidden pnpm-compat flags are still parseable

Comments Outside Diff (1)

crates/aube/src/main.rs, line 458-487 (link)

Combined -rF <val> short flag breaks the pre-subcommand lift

When a user writes aube -rF pattern --frozen-lockfile install (or any combined short flag where -F is the last character and its value follows as a separate token), the rewriter treats -rF as an opaque boolean short flag (i += 1) and then sees pattern as the subcommand position. The loop breaks before ever scanning --frozen-lockfile, so no tokens are lifted and clap gets the original argv — where --frozen-lockfile is no longer on Cli — causing a parse error.

The three forms behave differently after this PR:
- aube -r -F pattern --frozen-lockfile install → lift works ✓
- aube -rFpattern --frozen-lockfile install → lift works (inline value) ✓
- aube -rF pattern --frozen-lockfile install → parse error ✗ (combined flag + space-separated value)
The fix is to consume the next token as -F's value when the combined flag ends in F:
```
// Combined short ending in -F, e.g. -rF (r=bool, F=takes-value)
if s.ends_with('F') && s.starts_with('-') && s.len() > 2 {
    i += 1; // skip the combined token
    if i < args.len() && !token_looks_like_flag(&args, i) {
        i += 1; // consume F's separate value
    }
    continue;
}
```
The PR description highlights -rF muscle memory as load-bearing; this edge case regresses exactly that pattern when combined with a pre-subcommand lifted flag.

_{Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile}

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 6b997f0. Configure here.}

…s off global Every subcommand's --help used to show 30+ alphabetically interleaved flags because lockfile, network, and virtual-store options were declared `global = true` on the top-level Cli. The signal-to-noise ratio on `aube ls --help` was the chief complaint in #345. This change pulls three groups (LockfileArgs, NetworkArgs, VirtualStoreArgs) into `crates/aube/src/cli_args.rs`, each with `next_help_heading`, and flattens them only into the commands that actually consume them (install-trigger paths, network-touching paths, and auto-installers like `run`/`exec`). The pnpm-compat noops (`--workspace-packages`, `--ignore-workspace`, `--include-workspace-root`, `--aggregate-output`, `--stream`, `--use-stderr`, `--yes`) get `hide = true` — still parseable, just not in --help. Pre-subcommand placement (`aube --frozen-lockfile install`, `aube --registry=URL install`, …) keeps working through a small `lift_per_subcommand_flags` argv pre-pass that shifts moved-off-global flag tokens past the subcommand position before clap parses argv. Same trick we already use for `--config.<key>=<value>`. Workspace selection (`-F`, `-r`, `--filter-prod`, `--workspace-root`, `--fail-if-no-match`) stays on Cli — `aube -r run build` muscle memory is load-bearing for pnpm users and the cost/benefit of moving it doesn't justify the per-command flatten. The cli_ordering test gets a heading-aware variant: long-only flags must sort within each help-heading bucket rather than across the whole flat namespace. Closes #345 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Mechanical regen via `mise run render` after the per-command flag move. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- aube add -g --registry=URL silently dropped --registry: run_global_inner built a throwaway AddArgs with NetworkArgs::default(), and the inner run()'s install_overrides() then RwLock-overwrote the registry slot the outer run() had just set. Lockfile/virtual-store didn't have the same bug because their slots are OnceLock-backed and idempotent. Plumb the caller's three flag groups through run_global -> run_global_inner so the inner overrides are no-op resets. - aube recursive --registry=URL install failed clap parsing on the rebuilt argv: lift_per_subcommand_flags ran on the outer argv only, not on the argv reconstructed by recursive::argv. Apply the same lift to nested_argv before Cli::try_parse_from. - test/cli.bats "aube parses pnpm global workspace/output flags" still asserted --workspace-packages / --aggregate-output / --use-stderr appear in `aube --help`; we deliberately hid those. Retarget the test to verify they parse silently while keeping the visible-flag assertions for --dir / --filter-prod / --workspace-root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…to dlx - Lifter consumed flag-shaped tokens as kept-flag values: `aube --dir --frozen-lockfile install` would silently drop `--frozen-lockfile` because the rewriter ate it as the value of `--dir`. Same problem for the lifted-flag value path (`--registry --frozen-lockfile`). Add a `token_looks_like_flag` peek that gates value-consumption on whether the next token looks like another flag. Cursor Bugbot flagged this. - Drop the redundant `rest.is_empty()` arm after `s == "-"` already caught the bare-dash case earlier in the loop. Cursor Bugbot dead-code flag. - `aube dlx --enable-gvs <pkg>` regressed: dlx uses `trailing_var_arg + allow_hyphen_values`, so flags after the subcommand get swallowed into `params` unless declared on `DlxArgs`. Flatten LockfileArgs and VirtualStoreArgs alongside the existing NetworkArgs so all three groups parse before the trailing capture; bats `aube dlx accepts the global gvs override after the subcommand` (test/dlx.bats) was failing. Forward Default::default() through `create.rs`'s synthetic `DlxArgs` construction. - Add cli_spec_tests::lifter_does_not_eat_lifted_flag_as_kept_flag_value regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

greptile-apps Bot reviewed May 3, 2026

View reviewed changes

Comment thread crates/aube/src/commands/add.rs Outdated

cursor Bot reviewed May 3, 2026

View reviewed changes

Comment thread crates/aube/src/main.rs

Comment thread crates/aube/src/main.rs Outdated

jdx and others added 4 commits May 3, 2026 19:39

docs(cli): regenerate from new help structure

81ac19f

Mechanical regen via `mise run render` after the per-command flag move. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jdx force-pushed the refactor/cli-help-grouping branch from 6b997f0 to 17cd38d Compare May 3, 2026 19:40

[autofix.ci] apply automated fixes

34699ba

jdx enabled auto-merge (squash) May 3, 2026 19:51

jdx merged commit 8a19e6c into main May 3, 2026
16 checks passed

jdx deleted the refactor/cli-help-grouping branch May 3, 2026 19:52

greptile-apps Bot mentioned this pull request May 3, 2026

chore: release v1.8.0 #491

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(cli): bucket per-command --help by moving cross-cutting flags off global#505

refactor(cli): bucket per-command --help by moving cross-cutting flags off global#505
jdx merged 5 commits intomainfrom
refactor/cli-help-grouping

jdx commented May 3, 2026 •

edited by cursor Bot

Loading

Uh oh!

greptile-apps Bot commented May 3, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented May 3, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Result

Caveat

Test plan

Uh oh!

greptile-apps Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Comments Outside Diff (1)

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented May 3, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented May 3, 2026 •

edited

Loading