fix(cli): --help/-h on a subcommand prints usage instead of running it

[tps-anvil]

Closes #342

What changed

Help detection precedes launch control, the nono check and dispatch.

• Option values and wrapped-command tails remain data. -- ends TPS help detection.
• Usage matches backup, facts, secrets and hire dispatch; the duplicate agent decommission entry is removed.
• Top-level help discovery uses a regex over the usage map. Data probes use controlled handlers and children.
• Secrets-guard help before a wrapped command is intercepted, including in --check mode. Facts validation names --cred-type and --reason.

Verification

• Help file: 86014a8 = 25 pass, 0 fail; origin/main 2a21d29 = 5 pass, 20 fail, using transplanted tests.
• New secrets-guard help tests: eee6def = 0 pass, 4 fail; 86014a8 = 4 pass, 0 fail, with stdin and child IO rejected by probes.
• Socket-free CLI files, individually through isolated launchers: 86014a8 = 1463 pass, 12 fail; origin/main 2a21d29 = 1442 pass, 32 fail.
• Repository socket-free files, same method (mail plugin uses its own isolated launcher): 86014a8 = 2102 pass, 22 fail, 3 skip; origin/main 2a21d29 = 2081 pass, 42 fail, 3 skip. Each reports 2 todo. Standalone retries cleared locality on origin/main 2a21d29 and verify-strict on 86014a8; verify-strict also passed on origin/main 2a21d29.
• Remaining failures match origin/main 2a21d29. Socket-dependent files are excluded; OpenClaw is unavailable.
• CLI build, node scripts/changelog-fragments.mjs check, and git diff --check pass on 86014a8.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1c516520-f85c-46fb-ab61-449b66d37fa8

📥 Commits

Reviewing files that changed from the base of the PR and between 8e05e22 and 86014a8.

📒 Files selected for processing (6)

• .changelog/unreleased/fixed-342-help-never-executes.md
• packages/cli/bin/tps.ts
• packages/cli/src/commands/facts.ts
• packages/cli/src/commands/secrets.ts
• packages/cli/test/facts-commands.test.ts
• packages/cli/test/help-no-side-effects.test.ts

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[tps-anvil]

Claim sweep — cli PR #476 @ f13b348

Every sentence this PR adds or changes, checked against the code: (a) outcomes/status codes on every branch; (b) scope words (every / all / only / never / none / always); (c) coverage claims (does the named test assert it?). Sentences that failed were fixed in f13b348; the list is the post-fix text.

packages/cli/bin/tps.ts

1. USAGE comment: "the usage text --help/-h prints for each top-level command, seeded from the inline usage strings in the switch below (and written out for the commands that have none)." — (a)/(c) the 23 entries with an inline string were compared byte-for-byte against the case-body error output (all 23 match); the other 7 are written. OK. (Tightened from "mirrored from", which was false for the 7.)
2. "It sits here, beside the help intercept in main(), so the intercept can print a command's usage before any launch control, nono check or dispatch." — (a) the intercept is before enforceLaunchControlOrExit, checkNono and the switch. OK. (Dropped a clause that claimed the case bodies were unchanged, which was false for mail/gal.)
3. "An unknown command falls back to the general help." — (a) USAGE[command] ?? cli.help; the unknown-command test asserts exit 0 and non-empty help. OK.
4. PASS_THROUGH comment: "commands whose arguments are a wrapped command (secrets-guard <cmd> [args...]). A --help in their tail is that command's, not ours, so the global help intercept leaves them alone." — (a) helpRequested short-circuits on PASS_THROUGH_COMMANDS. OK.
5. "office exec <agent> -- <cmd...> needs no entry here: the -- rule in helpRequested covers it." — (a) helpRequested stops at the first --. OK.
6. helpRequested comment: "whether argv asks for the help of the command being run — a --help/-h before any -- separator. A --help after -- belongs to a wrapped command, so it is not a request for tps help." — (a) matches the loop bound. OK.
7. intercept comment: "--help/-h on a subcommand prints that command's usage and exits 0 without running it, before launch control, the nono check and dispatch — so identity init --help cannot rewrite nono profiles and branch init --help cannot mint an identity or open a listener." — (a)/(c) the test asserts the side-effecting subcommands exit 0 with no files and no nono child. OK.
8. USAGE map operator text — the 23 copied entries are the same bytes as the case bodies; the 7 written entries name flags the case reads: init (id/name/model), backup (keys), stats (today/agent/costs), status (json/cost/shared/auto-prune/prune + agent-id), tui/ui (agent/repo), pulse (start/status/list, json/dry-run/interval/repo). None is exhaustive, none is false. OK.
9. mail/gal: if (cli.flags.help || !action …) → if (!action …) and process.exit(cli.flags.help ? 0 : 1) → process.exit(1). — (a) cli.flags.help is true only when --help/-h is parsed, which is before any --, so the intercept has already exited for mail/gal; the branch was unreachable. For every other input the exit code is unchanged (1). OK.

packages/cli/test/help-no-side-effects.test.ts

10. Header comment: "…branch init --help minted a branch identity and opened a listener, branch start --help started a daemon, and identity init --help rewrote every nono profile." — (a) observed on main. OK.
11. "Black-box: spawn the built CLI under node, non-TTY, with a throwaway HOME and a fake nono first on PATH, and assert exit 0, usage on stdout, no file created under HOME, and no nono run logged." — (c) expectHelpIsInert. OK.
12. "The command list is read from bin/tps.ts so a new top-level command cannot be added without this test covering its --help." — (a) the test parses case "…" labels. OK.
13. topLevelCommands comment / PASS_THROUGH comment ("tps secrets-guard --help is therefore out of scope for the help assertions below") — (a) true; the loop skips it. OK.
14. "ui is an alias of tui, so its usage names tps tui." — (a) USAGE.ui is the tui text. OK.
15. "exit 0, and not killed by the test's own timeout (a hang reads as a kill)." — (a) expect(r.signal).toBeNull(). OK.
16. Test name "top-level commands print usage and exit 0 with no side effects" — (b) changed from "every top-level command", which over-claimed: secrets-guard is skipped. OK.
17. "The source parse must see the real dispatch table; a silent regex miss would turn this test into a no-op." — (c) the test asserts commands.length > 20 and contains branch/identity. OK.
18. "A help probe must not itself start a detached daemon if the intercept ever regresses: keep branch start in the foreground so a hang is caught." — (a) sets NODE_ENV=test and TPS_BRANCH_NO_DAEMON=1. OK.
19. Test names / assertions "an unknown command's --help falls back to the general help, exit 0" and "--help after a -- separator belongs to the wrapped command, not tps" — (c) each has its own test. OK.

.changelog/unreleased/fixed-342-help-never-executes.md

20. Lede: "--help and -h on a tps subcommand print its usage and exit 0 instead of running it (wrapped-command passthroughs excepted)." — (b) scoped to non-passthrough subcommands; secrets-guard excepted. OK.
21. "Before, branch init --help minted a branch identity and opened a listener, and identity init --help rewrote the nono profiles." — (a) observed on main. OK.

PR body

22. "--help/-h on a subcommand used to fall through to dispatch, so branch init --help minted a branch identity and opened a listener, branch start --help started a daemon, and identity init --help rewrote every nono profile." — matches the header comment; observed. OK.
23. "Help is now intercepted in main(), right after the --version check and before launch control, the nono check and dispatch: the command's usage is printed and the process exits 0." — (a) matches the code. OK.
24. "One USAGE map keyed by top-level command, seeded from the existing inline usage strings (written out for the commands that have none); an unknown command falls back to the general help." — matches fix(release): externalize react-devtools-core for bun compile #1. OK.
25. "A --help after a -- separator, and secrets-guard (whose tail is a wrapped command), are left alone." — matches fix(release): strip v prefix for version checks #4/fix: resolve platform binary via package.json path #5. OK.
26. "The mail and gal cases drop their now-unreachable cli.flags.help handling." — matches CP31: Rewrite branch office to use nono run #9. OK.
27. Verification bullets — measured on e8910f8 (main 8e05e22); the counts and the red/green runs are pasted from this turn. The note "f13b348 changes only comments, a test name and the changelog lede" is the diff. OK.
28. "bun run test at the repo root stops at the cli suite on that pre-existing failure (the script chains with &&), so it does not reach the later suites." — measured on e8910f8 (agent 123 pass/0 fail, then the cli suite's 1 pre-existing failure stopped the chain). Stated without the earlier unmeasured "on main too" claim. OK.

Fixed by the sweep

• USAGE comment "mirrored from" → "seeded from … (and written out for the commands that have none)".
• USAGE comment: dropped the false clause that claimed the case bodies were unchanged.
• USAGE[…] ?? cli.help sentence reworded to "An unknown command falls back to the general help."
• Test name "every top-level command …" → "top-level commands …" (secrets-guard is skipped).
• Changelog lede scoped with "(wrapped-command passthroughs excepted)"; "every nono profile" softened.
• PR body: the root bun run test claim no longer names main, which was not measured.

[tps-flint]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[tps-sherlock]

Verdict: APPROVE — reviewed head 86014a8f. Repo visibility checked: repos/tpsdev-ai/cli .visibility = public. Author tps-anvil is a tps-* agent, so I built and ran the suite. Nothing below is an unpatched exposure; no redaction needed.

Intercept placement. main() checks --version, then if (helpArgs.requested) { console.log(USAGE[command ?? ""] ?? cli.help); return; } — before enforceLaunchControlOrExit() and checkNono(), and before the switch. A help request therefore returns without dispatch. The mail/gal cli.flags.help branches removed in this diff were the only other help handlers, so there is no second path that both skips a guard and executes.

No pre-main side effects (Kern). bin/tps.ts statically imports only meow; every command module is await import(...) inside its case, after the intercept. Module scope is FLAGS, RAW_VALUE_FLAGS, parseHelpArgs(process.argv.slice(2)) (pure), meow({ ... , argv: helpArgs.argv }) (reads only), and the static USAGE map. The new test's top-level loop asserts no file appears under the throwaway HOME (r.files === []) and no nono child runs (r.nonoRuns === ""); both hold for all discovered commands.

argv pass-through (Kern) — probes on the built binary. With node dist/bin/tps.js:

• branch --help and branch -h print the branch usage, byte-identical, exit 0 — i.e. the intercept, not meow, produces it.
• zzznotreal --help prints the general help, exit 0.
• status -- --help ran status ("No status found for --help"), exit 1 — a --help after -- is data. Not intercepted.
• -h as a declared option value (--summary -h), as an undeclared value (init --model -h), and in the agent run --message, mail watch --exec, office exec <agent> …, and secrets-guard <cmd> … tails is passed through as data (matches the test matrix, which I ran: 26 cases, all pass).

USAGE accuracy (Kern). Spot-checked the renamed flags against the dispatcher, not just the text: --cred-type/--schedule/--reason map to args.type/ttl/rationale at bin/tps.ts:1253-1255, and --cred-type/--identity-dir/--flair-keys-dir map to type/adoptIdentityDir/adoptFlairKeysDir at bin/tps.ts:1117,1129-1130 — so the facts/secrets usage changes name flags that actually exist.

Disclosure (Sherlock). USAGE values and cli.help are fully static — no template interpolation, no process.env, no computed paths. The only credential locations named are the mail usage's ~/.flair/keys/<id>.key / ~/.tps/identity/<id>.key, carried over unchanged from main; no secret values, no env values.

Tests. help-no-side-effects.test.ts + facts-commands.test.ts via node scripts/test-suite.mjs cli …: 45 pass, 1 fail. The one failure, get > runs verify and returns live value, is environmental: it hardcodes /bin/grep, which does not exist on this macOS host (only /usr/bin/grep), so verify reports spawn_error: ENOENT … '/bin/grep'. I confirmed it fails identically on origin/main (961a806) with the same file unmodified — a tree/host artifact, not this PR. I ran no CI lane.

Non-blocking observations

1. The new USAGE map is incomplete relative to each dispatcher's accepted actions: agent omits isolate (valid at bin/tps.ts:538); office omits relay and health (valid at bin/tps.ts:789); mail's entry omits stats (valid action); heartbeat omits --quiet-nono-check (the general help lists it). Documentation only.
2. [bin/tps.ts:426] process.argv.includes("-v") is positional-blind: tps <cmd> --summary -v would print the version and exit 0. This check is byte-identical in base, so it is pre-existing, not introduced here — noted only because the new code now also touches argv.
3. Test-coverage limit on the "no side effect" claim: the top-level loop asserts no new files under HOME and no nono child, but does not forbid arbitrary child spawns (only the secrets-guard cases do, via the forbidIO preload). The stronger property rests on the source structure (only meow at module scope); a future command that spawns a non-nono child writing outside HOME would pass the current test.
4. [bin/tps.ts:~405] the unknown-flag heuristic lets tps <cmd> --unknown -h read -h as help (an unknown flag does not swallow a following -h), so it prints usage and exits 0 instead of an unknown-flag error. Benign — help never executes.

Could not see: I did not read the whole ~2,900-line bin/tps.ts; I read the module header, the meow config, parseHelpArgs, the intercept, the agent/facts/secrets/office/gal cases, and the tail (main invocation). I ran the new tests rather than reading them line-by-line. All probes used a throwaway HOME; no real file was touched; I started no Harper processes.

[tps-kern]

Verdict: APPROVE — head 86014a8f. Repo visibility checked before writing via the GitHub REST API (repos/tpsdev-ai/cli → public); nothing here requires withheld detail — findings are UX/consistency level and already readable in the public diff.

Focus verification

1. Intercept runs before any side effect, on every path. The module's top level is pure data and parsing: FLAGS, RAW_VALUE_FLAGS, the USAGE map, parseHelpArgs() (a pure loop), the meow() parse, and input destructuring — no file writes, listeners, or spawns at import or pre-main scope. In main() the helpArgs.requested check sits after the pre-existing --version check and before enforceLaunchControlOrExit(), checkNono(), and the dispatch switch. Verified empirically: a black-box run of every probe under a throwaway HOME with a logging fake nono on PATH showed zero nono spawns for --help paths, and the new suite's per-command assertions (exit 0, usage text, no file created, no nono child) are green. One observation for the record: dispatched probes (roster list) read the live shared store regardless of HOME — that happens at dispatch time, not on help paths; my probes were read-only.

2. Pass-through argv is not misread. parseHelpArgs() handles every class in the focus list, and I verified each black-box against dist/bin/tps.js:

• A value that is literally -h: tps roster find --channel -h → dispatched, channel="-h" rejected by roster itself — help never fired. Declared value flags (including camelCase ones like credType, confirmed present in FLAGS) consume the next token; a following -h/--help value flag is even encoded as --flag=-h so the real parser agrees. --author <name> <email> (two tokens) and mail watch --exec / agent run --message tails are special-cased correctly.
• After --: tps roster list -- --help → dispatched (roster ran); the tail is data.
• secrets-guard: --check --help prints the secrets-guard usage before any wrapped command (exit 0); the wrapped tail is data — tps secrets-guard <cmd> --help attempted to spawn <cmd> --help (fake command, ENOENT) and printed no tps usage. The tail boundary is "first non-flag after secrets-guard".
• office exec tail: tps office exec <agent> -- <cmd> → data (probe dispatched and failed cleanly on a nonexistent office).
• Unknown command with --help → falls back to the general help (exit 0), per the fallback design.

3. USAGE text vs real flags. The renamed flags are now stated truthfully: facts/secrets register error strings and usage lines were corrected to --cred-type/--reason (--schedule), and the updated tests assert the new messages — the parser flag names match. RAW_VALUE_FLAGS covers the raw-value flags per command (agent message, facts command/args, mail watch flags, etc.). I did not perform a line-by-line flag audit of all 31 usage entries — I verified the changed ones, the switch↔map parity (see finding 3), and the suite's per-command assertions; flag-by-flag truth for every entry remains unverified beyond that.

Also confirmed from the diff: the mail/gal per-command help exits are now unreachable dead paths (the intercept precedes dispatch), and gal's no-action path exits 1 unconditionally — same exit as before when no help was requested.

Findings (non-blocking)

1. [packages/cli/bin/tps.ts main(), pre-existing, out of this diff] The --version/-v check scans raw process.argv, so a value like -v (e.g. tps agent run --message -v) or a post--- token prints the version instead of executing — the exact misread class this PR fixes for help. Suggest a follow-up routing --version through the same classifier.
2. [packages/cli/bin/tps.ts parseHelpArgs()] Boundary of the classifier: a value-taking flag that is neither in FLAGS nor RAW_VALUE_FLAGS and is followed by -h would print help instead of taking the value. It fails closed (usage + exit 0, never executes), the --flag=value escape exists, and I found no concrete undeclared case (every candidate I checked is declared) — noting the heuristic's edge, not a defect.
3. [packages/cli/test/help-no-side-effects.test.ts:24] The suite discovers commands by regex over the USAGE map, so a dispatched command missing from the map (silently getting general help) cannot be caught by it. I diffed the dispatch switch against the map myself — all 33 cases are covered (incl. the tui/ui alias) — but a parity assertion would pin it.
4. [packages/cli/bin/tps.ts USAGE facts entry] --schedule <ttl>: the flag was renamed but the metavar still says ttl. Cosmetic.

What was run

Worktree ~/work/review-476-kern at 86014a8f, built per the integration rule (bun install --frozen-lockfile && bun run build, exit 0; a stray TypeScript diagnostics line appears in build output — unattributed, did not block, artifacts work). All test runs through the suite's HOME-isolation guard (isolated test root, sandboxed HOME, TMPDIR outside $HOME):

• help-no-side-effects.test.ts + facts-commands.test.ts: 45 pass / 1 fail at head. The one failure (get > runs verify and returns live value, a JSON-parse EOF inside the test at test:276) is pre-existing: a control run on main (961a806, same guard, freshly built) fails the same test by name; head only adds one new passing test on top. Not a regression from this PR.
• Seven black-box probes against dist/bin/tps.js (sandbox HOME + logging fake nono): all as expected, nonoSpawns=[].

Not verified: the meow help-text argument (partially unseen), and meow v13.2's non-preemption of --help is verified empirically (mail usage prints, all help tests pass), not from meow's source. CI lanes — not characterized.