perf(cli): route command aliases through the help fast path

[thymikee]

Summary

agent-device <command> --help has a fast path in src/bin.ts that prints static help without booting the full CLI. It resolved aliases through a hand-written two-entry table while the real registry (src/commands/cli-command-aliases.ts) has five, so tap, launch, and relaunch silently missed it and fell through to a full runCli() bootstrap just to print static text.

It now delegates to normalizeCliCommandAlias, so every alias the registry knows about gets the fast path automatically.

Measured on the built CLI, 3 runs each, warmed:

--help	before	after
press (control)	49 ms	49 ms
long-press (control, was in the old table)	43 ms	49 ms
tap	156 ms	49 ms
launch	157 ms	50 ms
relaunch	156 ms	53 ms

long-press vs tap is the controlled comparison: both are aliases printing identical help, differing only by whether the stale table knew about them. Output is byte-identical to each alias's canonical command (diff clean on all four pairs).

rotate deliberately still misses the fast path, so its rename migration error keeps rendering through the slow path. No carve-out was added to bin.ts for it — it simply isn't in the alias registry.

Regression coverage: a structural guard, not just a unit test

bin.ts runs unguarded top-level dispatch on import and is excluded from coverage by design, so it cannot be imported in a unit test — the committed cli-help-alias-fast-path.test.ts pins the registry composition bin.ts calls (a durable guard against a future sixth alias lacking help text), but reverting bin.ts alone does not fail it. That gap was closed with a dedicated structural gate instead of stretching the unit test past what it can honestly prove.

R12 (scripts/layering/bin-alias-fast-path.ts + .test.ts, wired into scripts/layering/check.ts and check:layering) reads src/bin.ts's source with oxc-parser — the same AST-based approach as session-state.ts, facade-exports.ts, and zero-dep-jobs.ts in the same directory, not a line scan (a line scan would mistake a fixture's string literal or a comment for the real thing). It asserts three facts:

1. bin.ts holds a value import of normalizeCliCommandAlias from the registry (not type-only, which would be erased at compile time).
2. bin.ts's call to buildCommandUsageText receives, as its argument, an actual call to the local binding that import resolved to — not merely both names appearing somewhere in the file. Binds by local name, so a renamed import (as resolveAlias) still verifies and an unrelated same-named local does not.
3. bin.ts contains none of the registry's own alias tokens as string literals (no local hand-rolled table sitting beside the delegation).

Fact 2 was added after a maintainer review of the first pass: import-presence and literal-absence alone still pass a bin.ts that imports the resolver and never calls it (or calls it on something unrelated) while buildCommandUsageText(helpTarget) runs raw — exactly the regression the fast path actually had. Verified by reverting locally to precisely that shape (import left in place, call reverted to the raw form) and confirming R12 now reports a violation; restored after.

Precision about that proof, since this PR is partly about not overclaiming test strength: the run demonstrated the three-fact guard failing on that fixture. That the two-fact version would have stayed silent on it was established by reading both predicates against the fixture (import present satisfies fact 1; no alias literals satisfies fact 3), not by re-running the older guard. The reasoning is checkable from the predicates themselves, but it is an argument rather than a measurement.

The alias-token list (fact 3) is read out of the registry's own CLI_COMMAND_ALIASES array-literal declaration rather than hard-coded, so a future sixth alias is covered automatically without touching this file.

Validation

• pnpm check:tooling — green (format, lint, typecheck, check:layering with R12, depgraph, production-exports, tmpdir-leaks, mcp-metadata, build, bundle-owner-files, check:package).
• pnpm check:affected --run — green.
• check:layering: 76 tests pass (15 in the new bin-alias-fast-path.test.ts), guard reports OK including R12.
• Both revert-and-watch-it-fail proofs performed and captured verbatim in the implementation history: (1) the original two-fact R12 against the pre-fix bin.ts (stale table, no import) — 2 violations; (2) the composition fact against a bin.ts with the import left in place but the call reverted to raw buildCommandUsageText(helpTarget) — 1 violation, exactly the case the two-fact version missed.
• The size report on the batch PR this was split from showed CLI --version 29.4 → 28.4 ms, confirming the one static import added to bin.ts costs nothing measurable — tsdown inlines the alias table into bin.js rather than emitting a chunk.
• The genuine red/green proof for the ORIGINAL fast-path-vs-slow-path bug (output is identical on both paths, so only the loaded modules differ) was done out-of-suite via the module graph, not as a committed test (spawning real node subprocesses in unit-core would violate the suite's "unit tests must not wait real time" budget and has no precedent in the repo):

# stale table, rebuilt dist:
NODE_V8_COVERAGE=<dir> node bin/agent-device.mjs tap --help
grep 'dist/src/cli\.js' <dir>/*.json   → MATCH      (slow path taken)
NODE_V8_COVERAGE=<dir> node bin/agent-device.mjs press --help
grep 'dist/src/cli\.js' <dir>/*.json   → no match   (control stayed fast)

# after the fix:
NODE_V8_COVERAGE=<dir> node bin/agent-device.mjs tap --help
grep 'dist/src/cli\.js' <dir>/*.json   → no match   (fast path confirmed)

Scope

6 files, +523/−9: src/bin.ts and its unit test (the fix), plus scripts/layering/bin-alias-fast-path.ts, its test, scripts/layering/check.ts, and package.json (the R12 structural guard, added across two review rounds). Not device-facing, so no simulator/emulator evidence applies.

Split out of #1639 per review. Found by a read-only codebase audit.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.99 MB	1.99 MB	+217 B
JS gzip	635.8 kB	635.8 kB	+69 B
npm tarball	769.4 kB	769.2 kB	-126 B
npm unpacked	2.69 MB	2.69 MB	-139 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.2 ms	26.7 ms	-0.6 ms
CLI --help	63.8 ms	64.9 ms	+1.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/cli.js	-293 B	-85 B
dist/src/internal/daemon.js	+235 B	+49 B

[thymikee]

Reviewed exact head 9418f11. The implementation correctly routes help aliases through the shared registry, but one P2 regression-test gap remains: src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts never exercises src/bin.ts, so reverting src/bin.ts:59 to the stale two-entry table leaves all three new tests green. Per the repository’s red-before-green rule, add a deterministic bin-entrypoint or structural guard that pins this production call and demonstrate it failing on the prior code. The out-of-suite module-graph proof establishes the current behavior, but it does not prevent recurrence. Deterministic CI is otherwise green. iOS Smoke is red on the same unrelated testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden failure reproduced on #1640/main; that is separate owner-action CI, but still blocks readiness until resolved or rerun green.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-06 22:31 UTC

[thymikee]

Re-reviewed exact head 7dc6f0f1. The production fix is correct, but the new R12 guard still does not pin its wiring:

• P2 — R12 never proves the resolver is called on the help target. It checks only that normalizeCliCommandAlias is value-imported and that local alias literals are absent. src/bin.ts can regress to buildCommandUsageText(helpTarget) while the import is used harmlessly elsewhere (or as void normalizeCliCommandAlias), and the real-tree gate stays green. Assert via AST that the imported local binding is invoked in the argument feeding buildCommandUsageText, and add a negative fixture with a present-but-unused/unrelated import.

The current PR body is also stale relative to the six-file structural-guard diff. Current head is mergeable and completed checks are green, with required lanes still pending; no ready-for-human label.

[thymikee]

Fixed at d2c3b1be5, and the PR body is refreshed against the current six-file diff.

R12 now asserts a third fact via AST: bin.ts's call to buildCommandUsageText must receive, as its argument, an actual call to the local binding the import resolved to. Binding by local name rather than the literal export string means import { normalizeCliCommandAlias as resolveAlias } still verifies, while an unrelated same-named local cannot satisfy it.

Red run for exactly the regression you named — import left in place, call site reverted to raw:

$ grep -n "buildCommandUsageText\|normalizeCliCommandAlias" src/bin.ts
1:import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts';
59:      const commandHelp = buildCommandUsageText(helpTarget);

$ node --experimental-strip-types scripts/layering/check.ts
Layering guard: 1 violation(s)

  [R12 bin-alias-fast-path] 1 violation(s):
    src/bin.ts:1 — imports normalizeCliCommandAlias (locally normalizeCliCommandAlias) but never
    passes normalizeCliCommandAlias(...) as the argument to buildCommandUsageText — the import
    alone does not prove the --help fast path actually delegates; call
    buildCommandUsageText(normalizeCliCommandAlias(helpTarget)) at the fast-path call site.

Both negative fixtures you asked for are committed: present-but-unused import, and an import used only unrelatedly (void normalizeCliCommandAlias) alongside a raw buildCommandUsageText(helpTarget). Guard tests 70 → 76.

One honest caveat, since this PR is partly about not overclaiming test strength: the run above proves the three-fact guard fails on that fixture. That the two-fact version would have stayed silent on it was established by reading both predicates against the fixture, not by re-running the older guard — an argument, not a measurement. Noted in the body too.

Also worth recording: the first push attempt was blocked by its own gate on test/integration/smoke-daemon-clean.test.ts — an assertion failure, so outside the timeouts-only retry policy. Since this PR adds the first static import bin.ts has ever had, and bin.ts spawns the daemon, that looked like a plausible real interaction. It is not: the same test passes on this branch (4.9s) and on main (1.4s), and the failing run took 15.3s inside a heavily parallel gate. Load-sensitive starvation, and the durations track machine load rather than code. Flagging because, like session-test-reporter-values.test.ts on #1645, it fails under load with an assertion and so cannot be covered by CONTENTION_RETRY_FILES by design.

pnpm check:affected --run green; rebased onto current main.

[thymikee]

Re-reviewed exact head d2c3b1be. The production alias fast path is correct, but R12 still permits the precise wiring regression it claims to prevent.

P2 — the AST guard existentially accepts any unrelated wrapped usage. usageTextCallsResolver returns true when any buildCommandUsageText(resolver(...)) exists anywhere in bin.ts; it does not identify the fast-path call consuming helpTarget, require the resolver argument to be helpTarget, or scope-resolve the imported binding. This bad fixture therefore passes all three facts while the shipped fast path is raw:

import { normalizeCliCommandAlias } from "./commands/cli-command-aliases.ts";
void buildCommandUsageText(normalizeCliCommandAlias("press"));
const commandHelp = buildCommandUsageText(helpTarget);

Add this negative fixture, then require the relevant/all usage-text call(s) to receive the imported local binding invoked specifically with helpTarget (or locate the call inside runHelpFastPath). Also cover a same-named shadow if the guard continues claiming binding identity. Current Linux/Swift/Replay failures all died resolving GitHub action downloads before checkout, so they are infrastructure rerun items, not this finding.

[thymikee]

Re-reviewed exact head f469d6c. The production alias fast path is correct, and the latest R12 guard closes the previous existential and decoy bypass: every buildCommandUsageText call must receive the imported resolver applied to the binding produced by resolveSimpleHelpTarget, with negative fixtures for raw, decoy, wrong-argument, and shadow cases. No actionable code findings. This is host-side static help routing, so device evidence does not apply. Code review is clean; remaining CI is still in progress.

[thymikee]

Fixed at f469d6c00. You were right that fact 3 was existential, and this time I have a measurement rather than an argument — which is the thing I owed you from the last round.

Fact 3 is now universal and value-bound. Every buildCommandUsageText call in bin.ts must receive <resolver>(<helpTarget>), where <helpTarget> is the binding resolveSimpleHelpTarget(...) produced — read out of bin.ts rather than hard-coded, so renaming the local re-points the guard instead of disarming it. Your fixture fails for two independent reasons: the decoy resolves a literal rather than the fast path's value, and the shipped call is raw.

Red run on your exact fixture, planted in the real tree:

$ grep -n "buildCommandUsageText" src/bin.ts
59:      void buildCommandUsageText(normalizeCliCommandAlias('open'));
60:      const commandHelp = buildCommandUsageText(helpTarget);

$ node --experimental-strip-types scripts/layering/check.ts
Layering guard: 1 violation(s)

  [R12 bin-alias-fast-path] 1 violation(s):
    src/bin.ts:1 — imports normalizeCliCommandAlias (locally normalizeCliCommandAlias) but calls
    buildCommandUsageText(normalizeCliCommandAlias("open")) — every buildCommandUsageText call
    must receive normalizeCliCommandAlias(helpTarget), the imported resolver applied to the fast
    path's own help target. A call that resolves something else (or nothing) leaves the shipped
    path un-delegated while looking wired.

I used 'open' rather than 'press' deliberately: press is a canonical command but tap is the alias token, and I wanted a literal fact 2 cannot catch, so the run isolates fact 3. Note fact 2 stayed silent — only one violation reported.

And the old predicate, measured against the same planted tree (checked out from HEAD, run against the same src/bin.ts):

OLD (existential) usageTextCallsResolver(planted bin.ts) = true

So the gap was real and is now closed, and that sentence is a run rather than a reading.

Same-named shadow covered too, as you asked. Since fact 3 claims binding identity by name, it now also rejects a local declaration of the resolver's name (const normalizeCliCommandAlias = (c) => c beside a correct-looking composition), and an ambiguous second helpTarget declaration. The declaration scan skips type positions — a type named helpTarget binds nothing at runtime and must not count.

The predicate returns the reason instead of a boolean, so the gate names which of the several distinct failures happened rather than sending you back to re-derive it. Guard tests 23, all green; pnpm check:affected --run green except one coverage failure I do not attribute to this change: ENOTEMPTY inside removeProviderScenarioTempDir during teardown of coordinate.contract.test.ts, which passes in isolation (6/6) and is not reachable from a scripts/layering diff.

---

Generated by Claude Code