refactor(cli): let help resolve command aliases itself and retire R12 - #2293
Conversation
bin.ts's --help fast path composed buildCommandUsageText(normalizeCliCommandAlias(helpTarget)) inline, which let a future edit call buildCommandUsageText raw without anyone noticing until an alias's help silently dropped back to a full CLI bootstrap (the regression #1641 fixed). Move the composition into cli-schema/cli-help.ts as resolveHelpTargetUsageText, so bin.ts just calls one function that owns its own alias normalization; bin.ts no longer imports the alias registry at all. Retargets cli-help-alias-fast-path.test.ts at the new function (same three cases) and adds a process-level smoke test asserting `tap --help`/`launch --help` stdout is byte-identical to `press --help`/`open --help`. Seen red by temporarily removing the `tap` alias from CLI_COMMAND_ALIASES (both fast and slow paths lose the alias, producing an "Unknown command: tap" mismatch); green again after restoring it. Verified manually: `node --experimental-strip-types src/bin.ts tap --help` stays byte-identical to `press --help`, and `launch --help` to `open --help`; `rotate --help` still falls through to the retired-command error.
bin.ts can no longer compose buildCommandUsageText and normalizeCliCommandAlias incorrectly because it doesn't hold either import any more — resolveHelpTargetUsageText in cli-schema/cli-help.ts is the only call site, and cli-help-alias-fast-path.test.ts plus the new smoke-cli process test pin it. The static R12 checker existed only to prove that composition from source text; delete it along with its rule wiring in check.ts (rule function, import, LAYERING_RULE_IDS/LAYERING_RULES entries, header comment, summary string). Drops scripts/layering/bin-alias-fast-path.ts (352 lines) and its test (311 lines). Updates the two stale references left behind: record-runtime-mechanics-policy.ts's comparison to R12's "delegate to your single owner" shape, and check-wiring.test.ts's header, which named bin-alias-fast-path.test.ts as the seam it protects. rule-ids.ts discovers rule ids by scanning source text rather than a hand-maintained list, so no entry there needed updating. Verified: pnpm check:layering green (175/175), including check-wiring.test.ts and rule-ids.test.ts; pnpm check:quick (lint + typecheck) clean; scripts/__tests__/eager-closure-budgets.test.ts (418/418) unaffected, since neither bin.ts nor cli-help.ts sits in any HUB_ENTRY_FILES or facade closure — both files reach cli-help.ts only through a dynamic import.
The byte-identical stdout test cannot fail when the fast path is bypassed: src/cli.ts's slow path resolves the same alias and writes the identical string, so a reintroduced hand-written table in bin.ts (the exact shape of #1641) would still pass it. Add a second process-level test that runs `tap`/`launch --help` and `rotate --help` with NODE_V8_COVERAGE set and reads the subprocess's own coverage report for src/cli/process-entry.ts, the one module runCli's slow path loads and the fast path never does. Seen red: forcing the fast path to always fall through to runCli (simulating the reintroduced-table bug) failed this test (bootstrappedFullCli true where false was expected) while the byte-identical test stayed green; reverted and confirmed both green.
The canonical side of "alias help output matches its canonical command" also
called resolveHelpTargetUsageText, so the assertion became self-consistency:
a degenerate normalizer that maps every input to one canonical command would
make aliasHelp and canonicalHelp equal for every case. Compare
resolveHelpTargetUsageText(alias) against buildCommandUsageText(canonical)
(no alias normalization on the canonical side) instead, restoring the
original two-source oracle.
Seen red: pointing resolveHelpTargetUsageText at a degenerate
`return buildCommandUsageText('press')` failed this test
("launch --help" no longer byte-identical to "open --help"); reverted and
confirmed green.
server-guide.ts's help tool composed buildCommandUsageText(normalizeCliCommandAlias(topic)) inline, the same composition bin.ts held before this PR moved it into cli-help.ts. That left a second hand-written call site the R12 gate's own kill criterion said had to be gone before retirement was moot. Call resolveHelpTargetUsageText(topic) instead; behavior is unchanged (manually confirmed tap/press and rotate topics still match) since it's the same composition, and no closure/layering change since server-guide.ts already imports cli-help.ts statically.
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
|
One P2 remains at exact head Derive the process cases from the alias registry/catalog (or at minimum cover every current alias), assert each alias bypasses |
|
Addressed at |
|
Re-reviewed exact head |
thymikee
added
the
ready-for-human
bin.ts ran its dispatch at import time, so the only way to prove that an alias --help never loads the full CLI was to spawn the process under NODE_V8_COVERAGE and grep the report for process-entry.ts. That oracle needed a paragraph to justify; the code was wrong, not the comment. The dispatch now lives in src/cli/entry.ts as runEntry(argv, modules, io), with the five lazy imports injected by bin.ts. entry.test.ts drives it with recording loaders and the real help module: every registry alias prints its canonical help with only the help module loaded, an unknown topic falls through to the CLI loader, --version, bare usage, mcp, and startup failures each have one case. The subprocess coverage machinery, the alias table pin, and the multi-line comments are gone; the smoke test keeps one registry-derived byte-identical alias --help check against the real bin.ts. Seen red: hand-routing long-press and relaunch to the CLI loader inside entry.ts failed "every registered alias prints its canonical help without loading the CLI"; restored.
|
Reworked at |
|
* origin/main: perf: bundle runtime dependencies and report full install size (#2310) ci: avoid unrelated Apple runner cache invalidation (#2303) fix(web): preserve the backend ref so snapshot refs match actionable refs (#2283) test(daemon): session-open-url-prewarm through the request seam (#2304) test(daemon): session-devices-batch-runtime through the request seam (#2305) chore(gates): layering baselines ratchet against merge-base (#2299) test(daemon): one typed conformance helper for the daemon runtime suites (#2298) chore(layering): derive the contracts export inventory from package.json (#2297) perf: bundle tar-stream to reduce install footprint (#2286) docs: simplify agent context and resolve conflicting guidance (#2287) refactor(cli): let help resolve command aliases itself and retire R12 (#2293)
* origin/main: perf: bundle runtime dependencies and report full install size (#2310) ci: avoid unrelated Apple runner cache invalidation (#2303) fix(web): preserve the backend ref so snapshot refs match actionable refs (#2283) test(daemon): session-open-url-prewarm through the request seam (#2304) test(daemon): session-devices-batch-runtime through the request seam (#2305) chore(gates): layering baselines ratchet against merge-base (#2299) test(daemon): one typed conformance helper for the daemon runtime suites (#2298) chore(layering): derive the contracts export inventory from package.json (#2297) perf: bundle tar-stream to reduce install footprint (#2286) docs: simplify agent context and resolve conflicting guidance (#2287) refactor(cli): let help resolve command aliases itself and retire R12 (#2293) refactor(commands): retire the navigation-only type projection (#2294) feat(runtime): route managed leases through contained transports (#2285) refactor(contracts): build unavailable runtime facts once (#2291) refactor(cli): derive the common flag readers from the common-field table (#2292) feat(daemon): add managed allocation operation journal (#2284)
1 participant
Summary
bin.ts's--helpfast path composedbuildCommandUsageText(normalizeCliCommandAlias(helpTarget))inline, which is what let it drift once already (#1641). The composition now lives incli-help.tsasresolveHelpTargetUsageText, used by bothbin.tsand the MCP help tool, and the R12 static checker that only existed to prove the composition was not bypassed is retired.The process entry itself is now importable:
src/cli/entry.tsexportsrunEntry(argv, modules, io)andbin.tsshrinks to the five lazyimport()calls it injects. That is what makes the fast path testable without spawning a process or reading V8 coverage reports.Before:
bin.tsandmcp/server-guide.tseach composed alias normalization by hand, and nothing could import the dispatch. After: both callresolveHelpTargetUsageText, andentry.test.tsdrives the dispatch directly.Validation
Tested commit: 0803173 —
pnpm check:affected --rungreen locally (scripts/ changed, so the full check set ran).src/cli/entry.test.ts: every registry alias prints its canonical help with only the help module loaded; an unknown topic falls through to the CLI loader;--version, bare usage,mcp, and startup failure each have a case. Seen red: hand-routinglong-press/relaunchto the CLI loader failed the alias case.cli-help-alias-fast-path.test.ts: registry-derived alias parity againstbuildCommandUsageText(independent oracle);rotateresolves to null.test/integration/smoke-cli.test.ts: one registry-derivedalias --helpbyte-identical check against the realbin.ts;--help,--version,-V, no-command unchanged.pnpm check:layeringgreen after retiring R12; eager-closure budgets unchanged.