diff --git a/scripts/layering/bin-alias-fast-path.test.ts b/scripts/layering/bin-alias-fast-path.test.ts deleted file mode 100644 index f8fb38bcd2..0000000000 --- a/scripts/layering/bin-alias-fast-path.test.ts +++ /dev/null @@ -1,311 +0,0 @@ -// R12 bin-alias-fast-path, tested directly: what each pure function reports for a fixture, -// independently of the check.ts wiring that turns it into a violation. - -import assert from 'node:assert/strict'; -import { readFileSync } from 'node:fs'; -import path from 'node:path'; -import { test } from 'node:test'; -import { - ALIAS_REGISTRY_FILE, - aliasResolverLocalName, - BIN_FILE, - countLocalBindings, - helpTargetBindingName, - importsAliasResolver, - localAliasLiterals, - registryAliasTokens, - usageTextDelegationFailure, -} from './bin-alias-fast-path.ts'; - -/** - * The shape of bin.ts's real `--help` fast path, minus everything R12 does not read. Fixtures - * below vary one thing against this baseline, so a test's subject is the line it changed. - */ -function binFixture(fastPathBody: string, prelude = ''): string { - return ` -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; -${prelude} -function runHelpFastPath(argv) { - const helpTarget = resolveSimpleHelpTarget(argv); - if (helpTarget === undefined) return false; -${fastPathBody} - return true; -} -`; -} - -/** `usageTextDelegationFailure` for a fixture, resolving the local name the way check.ts does. */ -function delegationFailure(source: string): string | null { - return usageTextDelegationFailure(source, aliasResolverLocalName(source)!); -} - -const REGISTRY_FIXTURE = ` -import type { CliFlags } from '@agent-device/contracts/command'; -const CLI_COMMAND_ALIASES = [ - { alias: 'long-press', command: 'longpress' }, - { alias: 'tap', command: 'press' }, - { alias: 'launch', command: 'open' }, - { alias: 'relaunch', command: 'open', impliedFlags: ['relaunch'] }, -]; -export function normalizeCliCommandAlias(command) { return command; } -`; - -test('registryAliasTokens reads every alias property value out of the registry source', () => { - assert.deepEqual(registryAliasTokens(REGISTRY_FIXTURE), [ - 'launch', - 'long-press', - 'relaunch', - 'tap', - ]); -}); - -test('registryAliasTokens is not fooled by an unrelated `alias` string elsewhere in the file', () => { - // Only a `{ alias: '' }` object-property VALUE counts. A same-named local variable, or - // the word appearing inside a comment, must not contribute a token. - const source = "const alias = 'not-a-token';\n// alias: also not a token\n"; - assert.deepEqual(registryAliasTokens(source), []); -}); - -test('importsAliasResolver is true only for a real VALUE import of the resolver', () => { - assert.equal( - importsAliasResolver( - "import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts';\n", - ), - true, - ); - // A renamed local binding still delegates to the real function — the registry specifier and - // the imported name are what matter, not what the caller calls it locally. - assert.equal( - importsAliasResolver( - "import { normalizeCliCommandAlias as resolve } from './commands/cli-command-aliases.ts';\n", - ), - true, - ); -}); - -test('importsAliasResolver is false for a type-only import', () => { - // Erased at compile time — no runtime delegation at all, which is exactly the STOP condition - // the original plan called out: importing the registry as a type only would look wired - // without actually being wired. - assert.equal( - importsAliasResolver( - "import type { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts';\n", - ), - false, - ); -}); - -test('importsAliasResolver is false when the import is missing or from the wrong module', () => { - assert.equal(importsAliasResolver('const x = 1;\n'), false); - assert.equal( - importsAliasResolver("import { normalizeCliCommandAlias } from './wrong-file.ts';\n"), - false, - ); - assert.equal( - importsAliasResolver("import { somethingElse } from './commands/cli-command-aliases.ts';\n"), - false, - ); -}); - -test('aliasResolverLocalName resolves the LOCAL binding, following an `as` alias', () => { - assert.equal( - aliasResolverLocalName( - "import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts';\n", - ), - 'normalizeCliCommandAlias', - ); - assert.equal( - aliasResolverLocalName( - "import { normalizeCliCommandAlias as resolveAlias } from './commands/cli-command-aliases.ts';\n", - ), - 'resolveAlias', - ); -}); - -test('aliasResolverLocalName is null when there is no matching value import', () => { - assert.equal(aliasResolverLocalName('const x = 1;\n'), null); - assert.equal( - aliasResolverLocalName( - "import type { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts';\n", - ), - null, - ); -}); - -test('helpTargetBindingName reads the binding resolveSimpleHelpTarget produces', () => { - assert.equal( - helpTargetBindingName(binFixture(' buildCommandUsageText(normalizeCliCommandAlias(x));')), - 'helpTarget', - ); - // Renaming the local re-points the guard rather than disarming it — the name is never assumed. - const renamed = ` -function runHelpFastPath(argv) { - const target = resolveSimpleHelpTarget(argv); -} -`; - assert.equal(helpTargetBindingName(renamed), 'target'); -}); - -test('helpTargetBindingName is null when the fast path no longer produces one', () => { - assert.equal(helpTargetBindingName('const helpTarget = argv[0];\n'), null); -}); - -test('countLocalBindings counts value declarations only, not the import or type positions', () => { - const source = ` -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; -function f(helpTarget: normalizeCliCommandAlias) { const other = 1; } -`; - // The import itself is not a shadow, and a type annotation naming the resolver binds nothing. - assert.equal(countLocalBindings(source, 'normalizeCliCommandAlias'), 0); - assert.equal(countLocalBindings(source, 'helpTarget'), 1); - assert.equal(countLocalBindings('const x = 1;\nfunction x() {}\n', 'x'), 2); -}); - -test('usageTextDelegationFailure accepts the real composition, by local name', () => { - assert.equal( - delegationFailure( - binFixture( - ' const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget));', - ), - ), - null, - ); - // Binds by whatever LOCAL name the import resolved to — an aliased import's local name must - // still be found at the call site, since that is the only name available to call it by. - const aliased = ` -import { normalizeCliCommandAlias as resolveAlias } from './commands/cli-command-aliases.ts'; -function runHelpFastPath(argv) { - const helpTarget = resolveSimpleHelpTarget(argv); - const commandHelp = buildCommandUsageText(resolveAlias(helpTarget)); -} -`; - assert.equal(delegationFailure(aliased), null); -}); - -test('usageTextDelegationFailure rejects a raw call, with no wrapping resolver call', () => { - const failure = delegationFailure( - binFixture(' const commandHelp = buildCommandUsageText(helpTarget);'), - ); - assert.match(failure ?? '', /buildCommandUsageText\(helpTarget\)/); -}); - -// #P2 (first maintainer review of R12): import presence and literal absence both still pass a -// bin.ts that imports the resolver and never calls it, or calls it on something unrelated, while -// buildCommandUsageText runs on the raw, unresolved helpTarget. -test('usageTextDelegationFailure rejects a present-but-unused import', () => { - const source = binFixture(' const commandHelp = buildCommandUsageText(helpTarget);'); - assert.equal(importsAliasResolver(source), true); - assert.notEqual(delegationFailure(source), null); -}); - -test('usageTextDelegationFailure rejects an import used only unrelated to buildCommandUsageText', () => { - const source = binFixture( - ' const commandHelp = buildCommandUsageText(helpTarget);', - 'void normalizeCliCommandAlias;', - ); - assert.equal(importsAliasResolver(source), true); - assert.notEqual(delegationFailure(source), null); -}); - -// #P2 (second maintainer review of R12): the fixture below is the reviewer's own, verbatim in -// shape. An EXISTENTIAL fact 3 — "some buildCommandUsageText call somewhere wraps the resolver" — -// accepts it, because the decoy on the first line satisfies the quantifier while the line that -// actually ships resolves nothing. This is the regression that motivated making fact 3 universal -// and value-bound, and it must be rejected for BOTH reasons independently. -test('usageTextDelegationFailure rejects a decoy wrapped call beside a raw shipped call', () => { - const source = binFixture( - ` void buildCommandUsageText(normalizeCliCommandAlias('open')); - const commandHelp = buildCommandUsageText(helpTarget);`, - ); - assert.equal(importsAliasResolver(source), true); - const failure = delegationFailure(source); - assert.match(failure ?? '', /every buildCommandUsageText call must receive/); -}); - -test('usageTextDelegationFailure rejects the resolver applied to anything but the help target', () => { - // The decoy alone, with no raw call at all: the only usage-text call in the file wraps the - // resolver, so a universal-but-not-value-bound fact 3 would still pass it. - const source = binFixture( - " const commandHelp = buildCommandUsageText(normalizeCliCommandAlias('open'));", - ); - assert.match(delegationFailure(source) ?? '', /normalizeCliCommandAlias\("open"\)/); -}); - -test('usageTextDelegationFailure rejects a local shadow of the imported resolver', () => { - // Fact 3 binds by NAME, so a same-named local would otherwise let the composition read as - // delegation while calling something that resolves nothing. - const source = binFixture( - ' const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget));', - 'const normalizeCliCommandAlias = (command) => command;', - ); - assert.match(delegationFailure(source) ?? '', /shadowing the imported resolver/); -}); - -test('usageTextDelegationFailure rejects an ambiguous second help-target binding', () => { - const source = binFixture( - ` const helpTarget = 'open'; - const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget));`, - ); - assert.match(delegationFailure(source) ?? '', /declares helpTarget more than once/); -}); - -test('usageTextDelegationFailure reports a fast path that no longer builds usage text at all', () => { - const gone = ` -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; -function runHelpFastPath(argv) { - const helpTarget = resolveSimpleHelpTarget(argv); - void normalizeCliCommandAlias(helpTarget); -} -`; - assert.match(delegationFailure(gone) ?? '', /never calls buildCommandUsageText/); - // …and one whose help-target producer is gone, so the guard says so instead of passing blind. - const untraceable = ` -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; -const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(argv[1])); -`; - assert.match(delegationFailure(untraceable) ?? '', /has no variable initialized by/); -}); - -test('localAliasLiterals reports every requested token present as a string literal', () => { - // A stale bin.ts shape with a hand-written alias table. - const preFixBinSource = ` -function normalizeHelpTarget(command) { - if (command === 'long-press') return 'longpress'; - return command; -} -`; - assert.deepEqual( - localAliasLiterals(preFixBinSource, ['long-press', 'tap', 'launch', 'relaunch']), - ['long-press'], - ); -}); - -test('localAliasLiterals ignores tokens that only appear as identifiers, not string literals', () => { - const source = 'const tap = 1;\nfunction launch() {}\n'; - assert.deepEqual(localAliasLiterals(source, ['tap', 'launch']), []); -}); - -test('localAliasLiterals reports nothing when the fixed bin.ts delegates and holds no literals', () => { - const fixedBinSource = ` -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; -const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget)); -`; - assert.deepEqual( - localAliasLiterals(fixedBinSource, ['long-press', 'tap', 'launch', 'relaunch']), - [], - ); -}); - -const repoRoot = path.resolve(import.meta.dirname, '../..'); - -test('the real tree imports the resolver, calls it into buildCommandUsageText, holds no local alias literals, and passes R12', () => { - const registrySource = readFileSync(path.join(repoRoot, ALIAS_REGISTRY_FILE), 'utf8'); - const binSource = readFileSync(path.join(repoRoot, BIN_FILE), 'utf8'); - const tokens = registryAliasTokens(registrySource); - assert.deepEqual(tokens, ['launch', 'long-press', 'relaunch', 'tap']); - const localName = aliasResolverLocalName(binSource); - assert.equal(localName, 'normalizeCliCommandAlias'); - assert.equal(helpTargetBindingName(binSource), 'helpTarget'); - assert.equal(usageTextDelegationFailure(binSource, localName!), null); - assert.deepEqual(localAliasLiterals(binSource, tokens), []); -}); diff --git a/scripts/layering/bin-alias-fast-path.ts b/scripts/layering/bin-alias-fast-path.ts deleted file mode 100644 index 7a3793ddfb..0000000000 --- a/scripts/layering/bin-alias-fast-path.ts +++ /dev/null @@ -1,352 +0,0 @@ -// Catches: bin.ts's --help fast path re-declaring its own alias table instead of calling the -// real registry — the exact silent-drift bug #1618-adjacent produced, where tap/launch/ -// relaunch fell out of a hand-written table and paid a full CLI bootstrap for static help -// text. bin.ts runs unconditionally on import, so no unit test can import and exercise it -// directly; only reading its source text structurally can catch a regression. -// Evidence: d85072d935 (#1641) routed command aliases through the help fast path; 74a70f1764 -// (#2046) removed next-major compatibility surfaces bin.ts once carried alongside it. -// Cost: 650 LOC (339 rule + 311 test). -// Kill criterion: none enforced today; retire only by maintainer decision that bin.ts's --help -// fast path no longer needs to delegate to the real alias registry — moot once the fast path -// is deleted or its resolution is inlined into commands/cli-command-aliases.ts, leaving no -// second call site. -// -// R12 bin-alias-fast-path. -// -// `bin.ts`'s `--help` fast path resolves a command alias (`tap`, `launch`, …) to its canonical -// command before looking up static help text. #1618-adjacent: bin.ts once carried its own -// hand-written two-entry table (`long-press`, `launch`) instead of calling the real alias -// registry, `commands/cli-command-aliases.ts` (five entries). The table silently fell out of -// sync — `tap`, `launch`, `relaunch` missed the fast path entirely and paid a full CLI bootstrap -// just to print static help text — and nothing failed, because bin.ts's own top-level dispatch -// runs unconditionally on import (see the module comment on `check.ts`'s R7 for the same -// "cannot safely unit-import this file" constraint) and is deliberately excluded from coverage -// (`vitest.config.ts`), so no unit test can call into it directly. -// -// Three structural facts, read from bin.ts's source text rather than by importing and running -// it, close the gap without needing to import it: -// 1. bin.ts holds a VALUE import of `normalizeCliCommandAlias` from the registry — so it is -// wired to delegate. -// 2. bin.ts never itself contains one of the registry's OWN alias tokens as a string literal — -// so it cannot be re-declaring a parallel mapping instead of actually calling the import. -// 3. EVERY call to `buildCommandUsageText` in bin.ts receives `()` — the -// LOCAL binding fact 1 imported, applied to the binding the fast path's own -// `resolveSimpleHelpTarget(...)` produced. This is the actual composition the fast path -// needs (`buildCommandUsageText(normalizeCliCommandAlias(helpTarget))`), not merely the -// import's presence. Facts 1 and 2 alone still pass if bin.ts imports the resolver and never -// calls it, or calls it on something unrelated (`void normalizeCliCommandAlias`) while -// `buildCommandUsageText(helpTarget)` runs raw — a real gap a maintainer review caught -// (the guard's own P2 follow-up). -// -// Fact 3 is deliberately UNIVERSAL and VALUE-BOUND, not existential, which is the second P2 from -// the same review. An "is there any `buildCommandUsageText(resolver(...))` somewhere" phrasing is -// satisfied by a decoy that never runs on the help target: -// -// void buildCommandUsageText(normalizeCliCommandAlias('press')); // decoy, satisfies ∃ -// const commandHelp = buildCommandUsageText(helpTarget); // what actually ships -// -// Requiring every usage-text call to receive the resolver applied to the help-target binding -// rejects both lines: the decoy resolves a literal rather than the fast path's own value, and the -// shipped call is raw. The help-target binding is discovered from bin.ts's source (the variable -// initialized by `resolveSimpleHelpTarget(...)`) rather than hard-coded, so renaming the local -// does not silently disarm the guard — it re-points it. -// -// Fact 3 binds by the import's LOCAL name, following any `as` alias, so -// `import { normalizeCliCommandAlias as resolveAlias }` still passes. Because that is a -// name-based claim about binding identity, fact 3 additionally requires that no local -// declaration in bin.ts SHADOWS either name: a local `const normalizeCliCommandAlias = (c) => c` -// would otherwise let the composition read correctly while calling something else entirely, and -// a second `helpTarget` declaration would let the resolver run on an unrelated value. -// -// All three were false on the pre-fix bin.ts (no import; both 'long-press' and 'launch' present -// as literals; no composition to find), so the set is a real regression pin, not just a -// description of intent. -// -// AST-based (`oxc-parser`, the standing precedent in this directory — session-state.ts, -// facade-exports.ts, contracts-implementation-policy.ts), not a line scan: a line scan reading -// raw text for "'tap'" would mistake this very comment, or a fixture string in a test file, for -// the real thing — precisely the false-positive failure mode that turned this directory to -// `parseSync(...).module`/`.program` in the first place. - -import { parseSync } from 'oxc-parser'; - -export const BIN_FILE = 'src/bin.ts'; -export const ALIAS_REGISTRY_FILE = 'src/commands/cli-command-aliases.ts'; -// The specifier bin.ts must use to reach the registry — relative to BIN_FILE's own directory -// (src/), not to the repo root, since that is how bin.ts's own import statement writes it. -const ALIAS_REGISTRY_SPECIFIER = './commands/cli-command-aliases.ts'; -const ALIAS_RESOLVER_EXPORT = 'normalizeCliCommandAlias'; - -/** Depth-first walk over an oxc-parser AST subtree (or `.module` entry list). */ -function visit(node: unknown, onNode: (record: Record) => void): void { - if (node === null || typeof node !== 'object') return; - if (Array.isArray(node)) { - for (const child of node) visit(child, onNode); - return; - } - const record = node as Record; - onNode(record); - for (const key of Object.keys(record)) visit(record[key], onNode); -} - -/** - * The alias tokens the registry declares — `CLI_COMMAND_ALIASES`'s `alias:` property values, - * read out of the registry's own source text rather than imported and executed. Every other - * gate in this directory treats its target as data to parse, not a module to run (session-state - * .ts reads `daemon/types.ts` the same way); staying consistent means R12 needs no `pnpm build` - * and cannot be fooled by import side effects. `CLI_COMMAND_ALIASES` itself is deliberately - * unexported (a façade names only what it means to share) — this reads its literal values - * directly out of the array-literal declaration instead, so a future sixth alias is picked up - * automatically and this list never needs hand-maintaining in a second place. - */ -export function registryAliasTokens(registrySource: string): string[] { - const parsed = parseSync(ALIAS_REGISTRY_FILE, registrySource); - const tokens = new Set(); - visit(parsed.program, (record) => { - if (record['type'] !== 'Property') return; - const key = record['key'] as Record | undefined; - if (key?.['type'] !== 'Identifier' || key['name'] !== 'alias') return; - const value = record['value'] as Record | undefined; - if (value?.['type'] === 'Literal' && typeof value['value'] === 'string') { - tokens.add(value['value'] as string); - } - }); - return [...tokens].sort(); -} - -/** - * The LOCAL binding name `binSource` imports `normalizeCliCommandAlias` as — following any `as` - * alias — for a VALUE (not type-only) import from the alias registry, or `null` if there is no - * such import. Reads `oxc-parser`'s own resolved import-entry table (`module.staticImports`), - * the same source `contracts-implementation-policy.ts`'s `moduleSpecifiers` uses — not a regex, - * so `import type { normalizeCliCommandAlias as x }` (erased at compile time, no runtime delegation - * at all) cannot pass as a real import the way a line match on the specifier text would. - * - * Reporting the LOCAL name (not just a boolean) is what lets `usageTextDelegationFailure` below - * bind by the name bin.ts actually calls, so a renamed import (`... as resolveAlias`) still - * verifies, while a same-named unrelated local cannot be mistaken for it (that one is enforced, - * not assumed — see the shadow check there). - */ -export function aliasResolverLocalName(binSource: string): string | null { - const parsed = parseSync(BIN_FILE, binSource); - for (const entry of parsed.module.staticImports) { - if (entry.moduleRequest.value !== ALIAS_REGISTRY_SPECIFIER) continue; - for (const specifier of entry.entries) { - if ( - !specifier.isType && - specifier.importName.kind === 'Name' && - specifier.importName.name === ALIAS_RESOLVER_EXPORT - ) { - return specifier.localName.value; - } - } - } - return null; -} - -/** Whether `binSource` holds a VALUE import of `normalizeCliCommandAlias` from the registry. */ -export function importsAliasResolver(binSource: string): boolean { - return aliasResolverLocalName(binSource) !== null; -} - -const USAGE_TEXT_CALLEE = 'buildCommandUsageText'; -const HELP_TARGET_PRODUCER = 'resolveSimpleHelpTarget'; - -function isCallTo(node: unknown, calleeName: string): boolean { - if (node === null || typeof node !== 'object') return false; - const record = node as Record; - if (record['type'] !== 'CallExpression') return false; - const callee = record['callee'] as Record | undefined; - return callee?.['type'] === 'Identifier' && callee['name'] === calleeName; -} - -function isIdentifierNamed(node: unknown, name: string): boolean { - if (node === null || typeof node !== 'object') return false; - const record = node as Record; - return record['type'] === 'Identifier' && record['name'] === name; -} - -/** A short, quotable rendering of an argument expression, for the violation message. */ -function describeArgument(node: unknown): string { - if (node === null || typeof node !== 'object') return String(node); - const record = node as Record; - if (record['type'] === 'Identifier') return String(record['name']); - if (record['type'] === 'Literal') return JSON.stringify(record['value']); - if (record['type'] === 'CallExpression') { - const callee = record['callee'] as Record | undefined; - const calleeName = callee?.['type'] === 'Identifier' ? String(callee['name']) : ''; - const args = Array.isArray(record['arguments']) ? record['arguments'] : []; - return `${calleeName}(${args.map(describeArgument).join(', ')})`; - } - return `<${String(record['type'])}>`; -} - -/** - * The LOCAL name of the fast path's help-target binding — the variable initialized by - * `resolveSimpleHelpTarget(...)` — or `null` if bin.ts no longer produces one that way. - * - * Read from the source rather than hard-coded so that renaming the local re-points the guard - * instead of disarming it, and so `helpTarget` never has to be maintained as a magic string in - * two places. - */ -export function helpTargetBindingName(binSource: string): string | null { - const parsed = parseSync(BIN_FILE, binSource); - let name: string | null = null; - visit(parsed.program, (record) => { - if (name !== null || record['type'] !== 'VariableDeclarator') return; - if (!isCallTo(record['init'], HELP_TARGET_PRODUCER)) return; - const id = record['id'] as Record | undefined; - if (id?.['type'] === 'Identifier') name = String(id['name']); - }); - return name; -} - -/** - * Every VALUE binding `binSource` declares locally under `name` — variable declarators, function - * and class declarations, function parameters, and catch clauses. - * - * This is what makes fact 3's binding-identity claim real rather than nominal: the composition - * `buildCommandUsageText(normalizeCliCommandAlias(helpTarget))` reads as delegation whether the - * callee is the import or a local shadow that happens to share its name, and only a declaration - * scan can tell those apart. Over-collection is the safe direction here — a false positive on - * these two specific names fails the gate loudly rather than passing a shadowed call silently — - * so patterns are walked whole, with type annotations skipped (a type named `helpTarget` binds - * nothing at runtime and must not count as a shadow). - */ -export function countLocalBindings(binSource: string, name: string): number { - const parsed = parseSync(BIN_FILE, binSource); - let count = 0; - const scanPattern = (node: unknown): void => { - visitSkippingTypes(node, (record) => { - if (isIdentifierNamed(record, name)) count += 1; - }); - }; - visit(parsed.program, (record) => { - switch (record['type']) { - case 'VariableDeclarator': - scanPattern(record['id']); - return; - case 'FunctionDeclaration': - case 'FunctionExpression': - case 'ArrowFunctionExpression': - case 'ClassDeclaration': - case 'ClassExpression': - if (isIdentifierNamed(record['id'], name)) count += 1; - scanPattern(record['params']); - return; - case 'CatchClause': - scanPattern(record['param']); - return; - default: - } - }); - return count; -} - -/** `visit`, minus type-position subtrees — type names bind nothing at runtime. */ -function visitSkippingTypes( - node: unknown, - onNode: (record: Record) => void, -): void { - if (node === null || typeof node !== 'object') return; - if (Array.isArray(node)) { - for (const child of node) visitSkippingTypes(child, onNode); - return; - } - const record = node as Record; - onNode(record); - for (const key of Object.keys(record)) { - if (key === 'typeAnnotation' || key === 'returnType' || key === 'typeParameters') continue; - visitSkippingTypes(record[key], onNode); - } -} - -/** - * Why `binSource` fails fact 3, or `null` if it holds. - * - * Fact 3 is universal and value-bound: EVERY `buildCommandUsageText(...)` call in bin.ts must - * receive `resolverLocalName()`, where `` is the binding - * `resolveSimpleHelpTarget(...)` produced. The existential phrasing this replaces ("some call - * somewhere wraps the resolver") is satisfied by a decoy that resolves an unrelated value while - * the shipped call runs raw — see the module comment for that exact fixture. - * - * Returning the reason rather than a boolean lets the gate say which of the several distinct - * ways to fail actually happened; a bare `false` sent a maintainer back to re-derive it. - */ -export function usageTextDelegationFailure( - binSource: string, - resolverLocalName: string, -): string | null { - if (countLocalBindings(binSource, resolverLocalName) > 0) { - return ( - `declares a local binding named ${resolverLocalName}, shadowing the imported resolver — ` + - `a call to ${resolverLocalName}(...) then proves nothing about delegating to ` + - `${ALIAS_REGISTRY_FILE}. Remove the shadow (or import the resolver under a different name).` - ); - } - - const helpTarget = helpTargetBindingName(binSource); - if (helpTarget === null) { - return ( - `has no variable initialized by ${HELP_TARGET_PRODUCER}(...), so the --help fast path's ` + - 'help-target binding cannot be located and its delegation cannot be checked. Keep the ' + - 'fast path resolving its target through that helper, or re-point this rule at its ' + - 'replacement.' - ); - } - if (countLocalBindings(binSource, helpTarget) > 1) { - return ( - `declares ${helpTarget} more than once, so "${USAGE_TEXT_CALLEE}(${resolverLocalName}(` + - `${helpTarget}))" no longer names one value — the resolver could be running on an ` + - 'unrelated binding that shares the name.' - ); - } - - const parsed = parseSync(BIN_FILE, binSource); - const calls: Record[] = []; - visit(parsed.program, (record) => { - if (isCallTo(record, USAGE_TEXT_CALLEE)) calls.push(record); - }); - - if (calls.length === 0) { - return ( - `never calls ${USAGE_TEXT_CALLEE} — the --help fast path that alias resolution exists to ` + - 'serve is gone, so this rule is checking nothing. Restore the fast path or retire R12.' - ); - } - - for (const call of calls) { - const args = Array.isArray(call['arguments']) ? call['arguments'] : []; - const first = args[0]; - const wraps = isCallTo(first, resolverLocalName); - const resolverArgs = - wraps && Array.isArray((first as Record)['arguments']) - ? ((first as Record)['arguments'] as unknown[]) - : []; - if (wraps && isIdentifierNamed(resolverArgs[0], helpTarget)) continue; - return ( - `calls ${USAGE_TEXT_CALLEE}(${describeArgument(first)}) — every ${USAGE_TEXT_CALLEE} call ` + - `must receive ${resolverLocalName}(${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.' - ); - } - return null; -} - -/** - * Which of `tokens` appear as a string-literal VALUE anywhere in `binSource` — not a substring - * match on the raw text, so a token that only shows up inside an unrelated identifier or this - * module's own doc comment does not count. - */ -export function localAliasLiterals(binSource: string, tokens: readonly string[]): string[] { - const wanted = new Set(tokens); - const parsed = parseSync(BIN_FILE, binSource); - const found = new Set(); - visit(parsed.program, (record) => { - if (record['type'] !== 'Literal') return; - const value = record['value']; - if (typeof value === 'string' && wanted.has(value)) found.add(value); - }); - return [...found].sort(); -} diff --git a/scripts/layering/check-wiring.test.ts b/scripts/layering/check-wiring.test.ts index 7658768b49..e8d90489da 100644 --- a/scripts/layering/check-wiring.test.ts +++ b/scripts/layering/check-wiring.test.ts @@ -1,7 +1,8 @@ -// The seam bin-alias-fast-path.test.ts calls out as untested — "the check.ts wiring that turns it -// into a violation." The registry makes duplicate registration unrepresentable on its own (an -// object holds a key once, and oxlint's no-dupe-keys rejects the attempt), so what is left to check -// is the other direction: that the catalog and the registry still describe the same set of rules. +// A rule policy module can compute violations correctly and still never run, if its entry in +// LAYERING_RULES is missing or its id is missing from LAYERING_RULE_IDS. The registry makes +// duplicate registration unrepresentable on its own (an object holds a key once, and oxlint's +// no-dupe-keys rejects the attempt), so what is left to check is the other direction: that the +// catalog and the registry still describe the same set of rules. // scripts/ is outside tsconfig.json's `include`, so the Record's exhaustiveness is an editor // signal, not a CI gate — this test is what fails the build when wiring goes missing. diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index c9a95d40e1..ebbaef4e14 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -31,9 +31,6 @@ // engine files, and planned logical modules start with zero forbidden/internal imports (R10). // - Over the WORKSPACE PACKAGES: no root back-imports, no relative tunnelling past // an exports map, and every workspace specifier declared + exports-named (R11). -// - Over BIN.TS'S ALIAS RESOLUTION: it must delegate to the one alias registry instead of -// re-declaring a parallel mapping of its own (R12) — the same "delegate to your single -// owner" shape as R7's SessionState ownership, applied to bin.ts's `--help` fast path. // - Over PLATFORM PACKAGE COMPOSITION: six private metadata façades meet at the exact root // composition file; premature implementation loading and forbidden cross-boundary edges fail (R13). // - Over REQUEST-BOUND RUNTIME EXECUTION: facts remain the only admission authority and daemon @@ -56,14 +53,6 @@ import { SESSION_STATE_FIELD_OWNERS, STORE_OWNED_SESSION_STATE_FIELDS, } from './session-state.ts'; -import { - ALIAS_REGISTRY_FILE, - aliasResolverLocalName, - BIN_FILE, - localAliasLiterals, - registryAliasTokens, - usageTextDelegationFailure, -} from './bin-alias-fast-path.ts'; import { backEdgePair, findValueImportCycles, @@ -447,75 +436,6 @@ function checkSessionStateOwnership(sources: ReadonlyMap): Layer return violations; } -/** - * R12: bin.ts's `--help` fast path must delegate command-alias resolution to the one alias - * registry instead of re-declaring its own mapping. See bin-alias-fast-path.ts for why the - * three facts below, together, are what closes the gap the original drift exploited — 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, which is exactly the P2 a maintainer review caught. Fact 3 is what closes that: EVERY - * `buildCommandUsageText` call must receive the imported resolver applied to the fast path's own - * help-target binding, with neither name shadowed by a local declaration. The universal - * quantifier is the follow-up P2 — an existential one is satisfied by a decoy call that resolves - * an unrelated literal while the shipped call still runs raw. - */ -function checkBinAliasFastPath(sources: ReadonlyMap): LayeringViolation[] { - const registrySource = sources.get(ALIAS_REGISTRY_FILE); - const binSource = sources.get(BIN_FILE); - if (!registrySource || !binSource) { - const missing = !registrySource ? ALIAS_REGISTRY_FILE : BIN_FILE; - return [ - { - rule: 'R12 bin-alias-fast-path', - file: missing, - line: 1, - message: `${missing} is missing, so bin.ts's alias delegation cannot be checked.`, - }, - ]; - } - - const violations: LayeringViolation[] = []; - const resolverLocalName = aliasResolverLocalName(binSource); - if (resolverLocalName === null) { - violations.push({ - rule: 'R12 bin-alias-fast-path', - file: BIN_FILE, - line: 1, - message: - 'does not hold a value import of normalizeCliCommandAlias from ' + - `${ALIAS_REGISTRY_FILE} — the --help fast path cannot delegate alias resolution to the ` + - 'registry without it.', - }); - } else { - const delegationFailure = usageTextDelegationFailure(binSource, resolverLocalName); - if (delegationFailure !== null) { - violations.push({ - rule: 'R12 bin-alias-fast-path', - file: BIN_FILE, - line: 1, - message: - `imports normalizeCliCommandAlias (locally ${resolverLocalName}) but ` + - `${delegationFailure}`, - }); - } - } - - const localLiterals = localAliasLiterals(binSource, registryAliasTokens(registrySource)); - if (localLiterals.length > 0) { - violations.push({ - rule: 'R12 bin-alias-fast-path', - file: BIN_FILE, - line: 1, - message: - `contains the registry's own alias token(s) (${localLiterals.join(', ')}) as string ` + - 'literals — a local alias-mapping table, hand-rolled instead of delegated to ' + - `${ALIAS_REGISTRY_FILE}. Delegate through normalizeCliCommandAlias instead of ` + - 're-declaring the mapping.', - }); - } - return violations; -} - function report( files: readonly string[], violations: readonly LayeringViolation[], @@ -532,11 +452,8 @@ function report( `(R9); ${daemonModularitySummary()}; ` + `${packageBoundariesSummary(repoRoot)}; ${platformPackagePolicySummary()}; ` + `runtime facts remain the only device-command admission authority and daemon code cannot ` + - `manufacture narrowed runtime proof (R66); R65 keeps production src/daemon free of concrete ` + - `platform imports in every executable and type-only form; and bin.ts imports ` + - `normalizeCliCommandAlias, ` + - `actually passes it into buildCommandUsageText, and holds no local alias literals ` + - `(R12).\n`, + `manufacture narrowed runtime proof (R66); and R65 keeps production src/daemon free of ` + + `concrete platform imports in every executable and type-only form.\n`, ); return 0; } @@ -598,7 +515,6 @@ export const LAYERING_RULE_IDS = [ 'session-state-ownership', 'daemon-modularity-ratchets', 'daemon-platform-boundary', - 'bin-alias-fast-path', 'package-boundaries', 'platform-package-policy', 'retired-platforms-zone', @@ -640,7 +556,6 @@ export const LAYERING_RULES: Readonly> = { ], 'daemon-platform-boundary': (context) => checkDaemonPlatformBoundary([...context.sources].map(([path, source]) => ({ path, source }))), - 'bin-alias-fast-path': (context) => checkBinAliasFastPath(context.sources), 'package-boundaries': () => checkPackageBoundaries(repoRoot), 'platform-package-policy': (context) => checkPlatformPackagePolicy( diff --git a/scripts/layering/record-runtime-mechanics-policy.ts b/scripts/layering/record-runtime-mechanics-policy.ts index 44e1e2ad35..cf80728d1a 100644 --- a/scripts/layering/record-runtime-mechanics-policy.ts +++ b/scripts/layering/record-runtime-mechanics-policy.ts @@ -1,6 +1,6 @@ // Catches: the daemon record owner reaching past its declared coordinators to spawn or poll a -// native process/timer directly — the same "delegate to your single owner" shape as R7 and -// R12, applied to record's runtime mechanics; a type check cannot see this because runCmd and +// native process/timer directly — the same "delegate to your single owner" shape as R7, +// applied to record's runtime mechanics; a type check cannot see this because runCmd and // setInterval are both fully typed, legal calls from anywhere. // Evidence: 1b2e786128 (#1724) moved screen recording onto the platform runtime, the migration // this ownership boundary protects against regressing. diff --git a/src/bin.ts b/src/bin.ts index 1be4250272..34d30c9f19 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -1,126 +1,24 @@ -import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; - -const argv = process.argv.slice(2); +import { runEntry } from './cli/entry.ts'; declare const __AGENT_DEVICE_VERSION__: string; -if (runFastPath(argv)) { - // Fast path owns process output and exit behavior. -} else if (argv[0] === 'mcp' && !argv.includes('--help') && !argv.includes('-h')) { - import('./mcp/server.ts') - .then(({ runAgentDeviceMcpServer }) => runAgentDeviceMcpServer()) - .catch(handleStartupError); -} else { - runCli(argv); -} - -function runFastPath(argv: string[]): boolean { - return runVersionFastPath(argv) || runNoCommandFastPath(argv) || runHelpFastPath(argv); -} - -function runVersionFastPath(argv: string[]): boolean { - if (argv.length !== 1 || !isVersionFlag(argv[0])) return false; - if (typeof __AGENT_DEVICE_VERSION__ === 'string') { - process.stdout.write(`${__AGENT_DEVICE_VERSION__}\n`); - return true; - } - import('@agent-device/host-kit/version') - .then(({ readVersion }) => { - process.stdout.write(`${readVersion()}\n`); - }) - .catch(handleStartupError); - return true; -} - -function runNoCommandFastPath(argv: string[]): boolean { - if (argv.length !== 0) return false; - import('./cli-schema/cli-help.ts') - .then(async ({ buildUsageText }) => { - process.stdout.write(`${buildUsageText()}\n`); - // #1596: exitAfterFlush (not a bare process.exit) so the full usage - // text reaches a piped caller before the process terminates. - const { exitAfterFlush } = await import('./cli/process-exit.ts'); - await exitAfterFlush(1); - }) - .catch(handleStartupError); - return true; -} - -function runHelpFastPath(argv: string[]): boolean { - const helpTarget = resolveSimpleHelpTarget(argv); - if (helpTarget === undefined) return false; - - import('./cli-schema/cli-help.ts') - .then(({ buildCommandUsageText, buildUsageText }) => { - if (helpTarget === null) { - process.stdout.write(`${buildUsageText()}\n`); - return; - } - const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget)); - if (commandHelp) { - process.stdout.write(commandHelp); - return; - } - // Unknown help topics still need full CLI parsing for the normal error path. - runCli(argv); - }) - .catch(handleStartupError); - return true; -} - -function resolveSimpleHelpTarget(argv: string[]): string | null | undefined { - switch (argv.length) { - case 1: - return resolveSingleArgHelpTarget(argv[0]); - case 2: - return resolveTwoArgHelpTarget(argv[0], argv[1]); - default: - return undefined; - } -} - -function resolveSingleArgHelpTarget(arg: string | undefined): null | undefined { - if (arg === 'help') return null; - return isHelpFlag(arg) ? null : undefined; -} - -function resolveTwoArgHelpTarget( - command: string | undefined, - helpArg: string | undefined, -): string | undefined { - if (isHelpCommand(command)) return helpArg; - return resolveTrailingHelpTarget(command, helpArg); -} - -function resolveTrailingHelpTarget( - command: string | undefined, - helpArg: string | undefined, -): string | undefined { - return isHelpFlag(helpArg) ? command : undefined; -} - -function isHelpCommand(command: string | undefined): boolean { - return command === 'help'; -} - -function isHelpFlag(arg: string | undefined): boolean { - return arg === '--help' || arg === '-h'; -} - -function isVersionFlag(arg: string | undefined): boolean { - return arg === '--version' || arg === '-V'; -} - -function runCli(argv: string[]): void { - import('./cli/process-entry.ts') - .then(({ runCliProcess }) => runCliProcess(argv)) - .catch(handleStartupError); -} - -function handleStartupError(error: unknown): void { - process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); - // #1596: exitAfterFlush so the message above isn't dropped on a piped stderr. - import('./cli/process-exit.ts') - .then(({ exitAfterFlush }) => exitAfterFlush(1)) - .catch(() => process.exit(1)); -} +runEntry( + process.argv.slice(2), + { + help: () => import('./cli-schema/cli-help.ts'), + cli: () => import('./cli/process-entry.ts'), + mcp: () => import('./mcp/server.ts'), + version: () => import('@agent-device/host-kit/version'), + processExit: () => import('./cli/process-exit.ts'), + }, + { + bundledVersion: + typeof __AGENT_DEVICE_VERSION__ === 'string' ? __AGENT_DEVICE_VERSION__ : undefined, + stdout: (text) => { + process.stdout.write(text); + }, + stderr: (text) => { + process.stderr.write(text); + }, + }, +).catch(() => process.exit(1)); diff --git a/src/cli-schema/cli-help-alias-fast-path.test.ts b/src/cli-schema/cli-help-alias-fast-path.test.ts index 891bafcd77..b723686488 100644 --- a/src/cli-schema/cli-help-alias-fast-path.test.ts +++ b/src/cli-schema/cli-help-alias-fast-path.test.ts @@ -1,62 +1,22 @@ -// Pins the exact composition `bin.ts`'s `--help` fast path relies on: -// `buildCommandUsageText(normalizeCliCommandAlias(helpTarget))`. Before this -// fix, `bin.ts` used its own hand-written two-entry table instead of this -// composition, so `tap`, `launch`, and `relaunch` silently missed the fast -// path and fell through to a full CLI bootstrap just to print static help -// text. `bin.ts` runs unguarded top-level dispatch on import (and is -// deliberately excluded from coverage — see vitest.config.ts), so it cannot -// be imported directly in a test; these tests instead pin the registry -// composition it calls. That makes them a real regression pin for a *future* -// alias missing help text (test 2 is durable for that), but not a substitute -// for the manual proof, run outside this suite, that bin.ts itself calls -// this composition (see the plan's execution report for the red/green -// evidence: with the stale table, `tap --help` loads `src/cli.ts`; with this -// fix, it does not). import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { buildCommandUsageText } from './cli-help.ts'; -import { cliAliasesForCommand, normalizeCliCommandAlias } from '../commands/cli-command-aliases.ts'; +import { buildCommandUsageText, resolveHelpTargetUsageText } from './cli-help.ts'; +import { cliAliasesForCommand } from '../commands/cli-command-aliases.ts'; import { listCliCommandNames } from '../command-catalog.ts'; -test('alias help output matches its canonical command', () => { - const cases: ReadonlyArray = [ - ['tap', 'press'], - ['launch', 'open'], - ['relaunch', 'open'], - ['long-press', 'longpress'], - ]; - for (const [alias, canonical] of cases) { - const aliasHelp = buildCommandUsageText(normalizeCliCommandAlias(alias)); - const canonicalHelp = buildCommandUsageText(canonical); - assert.notEqual(aliasHelp, null, `expected help text for alias "${alias}"`); - assert.equal( - aliasHelp, - canonicalHelp, - `expected "${alias} --help" to be byte-identical to "${canonical} --help"`, - ); - } -}); +const ALIASES = listCliCommandNames().flatMap((command) => + cliAliasesForCommand(command).map((entry) => [entry.alias, command] as const), +); -test('every CLI alias resolves to a command with help text', () => { - // Derive the alias list from the registry itself (via the canonical - // commands it targets) rather than hard-coding the five current alias - // names — a hard-coded list would silently stop covering a future sixth - // alias, reintroducing exactly the drift this test exists to catch. - const aliases = listCliCommandNames().flatMap((command) => - cliAliasesForCommand(command).map((entry) => entry.alias), - ); - assert.ok(aliases.length > 0, 'expected at least one alias to exercise this test'); - for (const alias of aliases) { - const help = buildCommandUsageText(normalizeCliCommandAlias(alias)); - assert.notEqual(help, null, `expected buildCommandUsageText to resolve alias "${alias}"`); +test('every registered alias resolves to its canonical command help', () => { + assert.ok(ALIASES.length > 0); + for (const [alias, canonical] of ALIASES) { + const help = resolveHelpTargetUsageText(alias); + assert.notEqual(help, null, alias); + assert.equal(help, buildCommandUsageText(canonical), alias); } }); -test('rotate still has no fast-path help', () => { - // `rotate` is not in the alias registry, so it must fall through to the - // slow path (`src/cli/parser/args.ts`'s `normalizeCommandAlias`), which is - // where the "renamed to orientation" migration error is raised. The fast - // path must never special-case `rotate` itself. - const help = buildCommandUsageText(normalizeCliCommandAlias('rotate')); - assert.equal(help, null); +test('a retired command has no help text and is left to the full CLI', () => { + assert.equal(resolveHelpTargetUsageText('rotate'), null); }); diff --git a/src/cli-schema/cli-help.ts b/src/cli-schema/cli-help.ts index 8f075211dc..da5d3c00b1 100644 --- a/src/cli-schema/cli-help.ts +++ b/src/cli-schema/cli-help.ts @@ -4,6 +4,7 @@ import { MAESTRO_COMPATIBILITY_ADR_URL, MAESTRO_COMPATIBILITY_ISSUE_URL, } from '@agent-device/maestro'; +import { normalizeCliCommandAlias } from '../commands/cli-command-aliases.ts'; import { helpBody } from '../commands/command-text.ts'; import { DEVICE_SELECTION_FLAG_KEYS, @@ -1212,6 +1213,11 @@ ${helpBody(schema.text)}${flagsSections} `; } +/** `--help` text for a command name or one of its aliases; `null` when neither has any. */ +export function resolveHelpTargetUsageText(helpTarget: string): string | null { + return buildCommandUsageText(normalizeCliCommandAlias(helpTarget)); +} + /** * Topic-id registry view for conformance tooling: the help benchmark's topic * coverage gate enumerates this instead of a hand-maintained list, so adding a diff --git a/src/cli/entry.test.ts b/src/cli/entry.test.ts new file mode 100644 index 0000000000..eba5688fc1 --- /dev/null +++ b/src/cli/entry.test.ts @@ -0,0 +1,111 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import * as cliHelp from '../cli-schema/cli-help.ts'; +import { listCliCommandNames } from '../command-catalog.ts'; +import { cliAliasesForCommand } from '../commands/cli-command-aliases.ts'; +import { runEntry, type EntryModules } from './entry.ts'; + +const ALIASES = listCliCommandNames().flatMap((command) => + cliAliasesForCommand(command).map((entry) => [entry.alias, command] as const), +); + +function harness(options: { bundledVersion?: string; cli?: EntryModules['cli'] } = {}) { + const loaded: string[] = []; + const stdout: string[] = []; + const stderr: string[] = []; + const cliArgv: string[][] = []; + const exits: number[] = []; + const modules: EntryModules = { + help: async () => { + loaded.push('help'); + return cliHelp; + }, + cli: + options.cli ?? + (async () => { + loaded.push('cli'); + return { + runCliProcess: async (argv) => { + cliArgv.push(argv); + }, + }; + }), + mcp: async () => { + loaded.push('mcp'); + return { runAgentDeviceMcpServer: async () => {} }; + }, + version: async () => { + loaded.push('version'); + return { readVersion: () => '9.9.9-source' }; + }, + processExit: async () => ({ + exitAfterFlush: async (code) => { + exits.push(code); + }, + }), + }; + const run = (argv: string[]) => + runEntry(argv, modules, { + bundledVersion: options.bundledVersion, + stdout: (text) => stdout.push(text), + stderr: (text) => stderr.push(text), + }); + return { run, loaded, stdout, stderr, cliArgv, exits }; +} + +test('every registered alias prints its canonical help without loading the CLI', async () => { + assert.ok(ALIASES.length > 0); + for (const [alias, canonical] of ALIASES) { + const entry = harness(); + await entry.run([alias, '--help']); + assert.equal(entry.stdout.join(''), cliHelp.buildCommandUsageText(canonical), alias); + assert.deepEqual(entry.loaded, ['help'], alias); + } +}); + +test('an unknown help topic falls through to the full CLI', async () => { + const entry = harness(); + await entry.run(['rotate', '--help']); + assert.deepEqual(entry.stdout, []); + assert.deepEqual(entry.loaded, ['help', 'cli']); + assert.deepEqual(entry.cliArgv, [['rotate', '--help']]); +}); + +test('--version prints the bundled version, or reads it when running from source', async () => { + const bundled = harness({ bundledVersion: '1.2.3' }); + await bundled.run(['--version']); + assert.deepEqual([bundled.stdout, bundled.loaded], [['1.2.3\n'], []]); + + const source = harness(); + await source.run(['-V']); + assert.deepEqual([source.stdout, source.loaded], [['9.9.9-source\n'], ['version']]); +}); + +test('no command prints usage and exits 1 after flushing', async () => { + const entry = harness(); + await entry.run([]); + assert.equal(entry.stdout.join(''), `${cliHelp.buildUsageText()}\n`); + assert.deepEqual(entry.exits, [1]); +}); + +test('mcp starts the server unless help is requested', async () => { + const server = harness(); + await server.run(['mcp']); + assert.deepEqual(server.loaded, ['mcp']); + + const help = harness(); + await help.run(['mcp', '--help']); + assert.deepEqual(help.loaded, ['help']); + assert.equal(help.stdout.join(''), cliHelp.buildCommandUsageText('mcp')); +}); + +test('a startup failure is reported on stderr and exits 1', async () => { + const entry = harness({ + cli: async () => { + throw new Error('boom'); + }, + }); + await entry.run(['press', 'Sign in']); + assert.deepEqual(entry.stderr, ['boom\n']); + assert.deepEqual(entry.exits, [1]); +}); diff --git a/src/cli/entry.ts b/src/cli/entry.ts new file mode 100644 index 0000000000..71f00154e0 --- /dev/null +++ b/src/cli/entry.ts @@ -0,0 +1,86 @@ +/** The lazily loaded halves of the process; `bin.ts` supplies the real `import()` calls. */ +export type EntryModules = Readonly<{ + help: () => Promise<{ + buildUsageText(): string; + resolveHelpTargetUsageText(target: string): string | null; + }>; + cli: () => Promise<{ runCliProcess(argv: string[]): Promise }>; + mcp: () => Promise<{ runAgentDeviceMcpServer(): Promise }>; + version: () => Promise<{ readVersion(): string }>; + processExit: () => Promise<{ exitAfterFlush(code: number): Promise }>; +}>; + +export type EntryIo = Readonly<{ + /** The version the bundler baked in; absent when running from source. */ + bundledVersion: string | undefined; + stdout: (text: string) => void; + stderr: (text: string) => void; +}>; + +/** Process entry: the three help/version fast paths, else the MCP server or the full CLI. */ +export async function runEntry(argv: string[], modules: EntryModules, io: EntryIo): Promise { + try { + await dispatch(argv, modules, io); + } catch (error) { + io.stderr(`${error instanceof Error ? error.message : String(error)}\n`); + // #1596: flush before exiting so a piped caller sees the message. + await (await modules.processExit()).exitAfterFlush(1); + } +} + +async function dispatch(argv: string[], modules: EntryModules, io: EntryIo): Promise { + if (argv.length === 0) return await printUsageAndExit(modules, io); + if (isVersionRequest(argv)) return await printVersion(modules, io); + const helpTarget = simpleHelpTarget(argv); + if (helpTarget !== undefined) return await printHelp(helpTarget, argv, modules, io); + if (isMcpServerRequest(argv)) return await (await modules.mcp()).runAgentDeviceMcpServer(); + await runCli(argv, modules); +} + +async function printUsageAndExit(modules: EntryModules, io: EntryIo): Promise { + io.stdout(`${(await modules.help()).buildUsageText()}\n`); + await (await modules.processExit()).exitAfterFlush(1); +} + +async function printVersion(modules: EntryModules, io: EntryIo): Promise { + io.stdout(`${io.bundledVersion ?? (await modules.version()).readVersion()}\n`); +} + +/** An unknown help topic is left to the full CLI, which owns the error path. */ +async function printHelp( + target: string | null, + argv: string[], + modules: EntryModules, + io: EntryIo, +): Promise { + const help = await modules.help(); + const text = + target === null ? `${help.buildUsageText()}\n` : help.resolveHelpTargetUsageText(target); + if (text === null) return await runCli(argv, modules); + io.stdout(text); +} + +async function runCli(argv: string[], modules: EntryModules): Promise { + await (await modules.cli()).runCliProcess(argv); +} + +function isVersionRequest(argv: string[]): boolean { + return argv.length === 1 && (argv[0] === '--version' || argv[0] === '-V'); +} + +function isMcpServerRequest(argv: string[]): boolean { + return argv[0] === 'mcp' && !argv.some(isHelpFlag); +} + +/** `null` = general usage, a string = one command's help, `undefined` = not a help request. */ +function simpleHelpTarget(argv: string[]): string | null | undefined { + const [first, second] = argv; + if (argv.length === 1) return first === 'help' || isHelpFlag(first) ? null : undefined; + if (argv.length !== 2) return undefined; + if (first === 'help') return second; + return isHelpFlag(second) ? first : undefined; +} + +function isHelpFlag(arg: string | undefined): boolean { + return arg === '--help' || arg === '-h'; +} diff --git a/src/mcp/server-guide.ts b/src/mcp/server-guide.ts index 8e81c4f8ef..6db7eb21a2 100644 --- a/src/mcp/server-guide.ts +++ b/src/mcp/server-guide.ts @@ -1,7 +1,10 @@ import type { JsonSchema } from '../commands/command-contract.ts'; -import { buildCommandUsageText, buildUsageText, helpTopicIds } from '../cli-schema/cli-help.ts'; +import { + buildUsageText, + helpTopicIds, + resolveHelpTargetUsageText, +} from '../cli-schema/cli-help.ts'; import { listCliCommandNames } from '../command-catalog.ts'; -import { normalizeCliCommandAlias } from '../commands/cli-command-aliases.ts'; import { listMcpExposedCommandNames } from '../core/command-descriptor/registry.ts'; import type { ToolResult } from './command-tools.ts'; @@ -62,7 +65,7 @@ export function callHelpTool(input: Record): ToolResult { if (topic !== undefined && typeof topic !== 'string') { return textResult('Expected topic to be a string.', true); } - const text = topic ? buildCommandUsageText(normalizeCliCommandAlias(topic)) : buildUsageText(); + const text = topic ? resolveHelpTargetUsageText(topic) : buildUsageText(); if (text === null) { return textResult(`Unknown help topic: ${topic}. ${HELP_TOOL.description}`, true); } diff --git a/test/integration/smoke-cli.test.ts b/test/integration/smoke-cli.test.ts index 8d98b19bc2..7d4710cb05 100644 --- a/test/integration/smoke-cli.test.ts +++ b/test/integration/smoke-cli.test.ts @@ -1,6 +1,8 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { runCmdSync } from '@agent-device/host-kit/command'; +import { cliAliasesForCommand } from '../../src/commands/cli-command-aliases.ts'; +import { listCliCommandNames } from '../../src/command-catalog.ts'; function runCli(args: string[]): { status: number; stdout: string; stderr: string } { const result = runCmdSync( @@ -39,3 +41,15 @@ test('cli without command prints usage and exits 1', () => { assert.equal(result.status, 1, result.stderr); assert.match(result.stdout, /agent-device /i); }); + +test('alias --help matches the canonical command help', () => { + const aliases = listCliCommandNames().flatMap((command) => + cliAliasesForCommand(command).map((entry) => [entry.alias, command] as const), + ); + assert.ok(aliases.length > 0); + for (const [alias, canonical] of aliases) { + const result = runCli([alias, '--help']); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, runCli([canonical, '--help']).stdout, alias); + } +});