chore: introduce CommandRoutingHub and migrate phase-command-router (PoC) (#3828)
* feat(routing): add CommandRoutingHub with behavioral test suite (#3788) Introduces createHub({ mode, sdkLoader, cjsRegistry, manifest }) and hub.dispatch({ family, subcommand, args, cwd, raw }) -> Result with a closed 6-value ERROR_KINDS frozen enum. Hub never throws, never prints, and enforces no transparent fallback between sdk/cjs modes. 34 behavioral tests cover all errorKind values, mode fixation, and the no-throw contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(routing): migrate phase-command-router to CommandRoutingHub (#3788) Rewrites phase-command-router.cjs to dispatch through CommandRoutingHub. Public entry point routePhaseCommand({ phase, args, cwd, raw, error }) is unchanged. The adapter determines mode (sdk/cjs) from env + tryLoadSdk(), constructs a hub, dispatches, and translates the pure Result back to output()/error() calls. New behavioral test suite (23 tests) replaces the old mock-heavy approach and includes two integration tests through the real hub. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(routing): ADR + glossary + changeset for CommandRoutingHub (#3788) Adds ADR-3788 documenting the hub's design contract (pure result, fixed mode, closed 6-value errorKind enum, no transparent fallback). Adds Command Routing Hub glossary entry to CONTEXT.md and a one-paragraph reference to ARCHITECTURE.md. Changeset fragment records the Changed entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(docs): rename ADR to sequential convention 0012 (#3788) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(inventory): register CommandRoutingHub in INVENTORY (#3788) Add command-routing-hub.cjs row to docs/INVENTORY.md CLI Modules table, bump headline count from 72 to 73, and regenerate INVENTORY-MANIFEST.json via scripts/gen-inventory-manifest.cjs --write. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(adr): add 0012 to ADR index (#3788) Add entry for 0012-command-routing-hub.md to the index table in docs/adr/README.md so the enh-3271-sdk-adr-structure lint passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(lint): bump phase test-file ceiling to accommodate command-router suite (#3788) phase-command-router.test.cjs added by the CommandRoutingHub migration pushes the phase prefix cluster from 4 to 5 test files. Bump the allowlist ceiling from 4 to 5 (issue 3788) so lint-test-file-count passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(routing): preserve phase.mvp-mode JSON error and ROADMAP scan through hub (#3788) mvp-mode was never registered in the SDK; the pre-#3788 CJS router always dispatched it via the CJS handler even when sdkAvailable was true. After the hub migration, SDK-mode hubs (Docker, where the SDK build exists) sent mvp-mode to the SDK bridge, which returned SdkDispatchFailed with reason 'unknown' instead of the expected 'usage' code, and failed ROADMAP lookups. Fix by short-circuiting mvp-mode to the CJS handler before hub construction, matching the pre-migration observable behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(adr): note SDK-incomplete subcommand limitation in ADR-0012 (#3788) * fix(inventory): bump CLI Modules headline to 74 after rebase onto main (#3788) Upstream added code-review-flags.cjs (72→73) at the same time our branch added command-routing-hub.cjs. After rebase both modules exist (74 total) but the headline stayed at 73; bump to 74. * fix(routing): remove dead mvp-mode handler from cjsRegistry (#3788) The cjsRegistry['phase']['mvp-mode'] handler (previously lines 65–68) was unreachable: the early-return bypass at line 56 intercepts mvp-mode before hub construction in CJS mode, and in SDK mode cjsRegistry is passed as undefined. Remove the dead handler; all 57 tests still pass. * docs(adr): correct router count in ADR-0012 (#3788) The context section cited "eight" routers including "frontmatter" but there is no frontmatter-command-router.cjs. The actual count is seven: phase, phases, roadmap, state, verify, validate, init. * fix(routing): guard missing subcommand + use ERROR_KINDS constant (#3788) Two fixes in phase-command-router.cjs: 1. Add early-return for missing subcommand before hub construction. Pre-#3788 the routeCjsCommandFamily fell through to error() for undefined args[1]; post-#3788 the hub's manifest check skips falsy subcommands, which would have sent bare 'phase' into SDK dispatch in SDK mode instead of the expected "Available: ..." error message. 2. Switch on ERROR_KINDS.UnknownCommand instead of bare 'UnknownCommand' string, per ADR-0012's closed-enum contract ("callers switch on ERROR_KINDS values, not bare string literals"). * docs(routing): fix factual errors in ARCHITECTURE, ADR-0012, changeset (#3788) Three corrections: 1. ARCHITECTURE.md: softened "All CJS command family routers dispatch through CommandRoutingHub" — only phase-command-router.cjs is migrated in this PR; remaining routers still use routeCjsCommandFamily and migrate in follow-up issues. 2. ADR-0012: corrected the SDK mvp-mode claim. The ADR said "the SDK has no equivalent entry" but sdk/src/query/command-static-catalog- domain.ts:104-105 registers phase.mvp-mode. The actual reason for the early-return bypass is divergent ROADMAP scan behaviour and error reason codes, not SDK absence. 3. .changeset/mellow-tigers-gather.md: corrected pr: 1 → pr: 3828. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
548
tests/command-routing-hub.test.cjs
Normal file
548
tests/command-routing-hub.test.cjs
Normal file
@@ -0,0 +1,548 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Behavioral contract tests for the CommandRoutingHub (issue #3788).
|
||||
*
|
||||
* Testing rules in force (CONTRIBUTING.md § Testing Standards):
|
||||
* 1. No readFileSync of source files. All assertions are on return values
|
||||
* from the hub's dispatch() function.
|
||||
* 2. Stub sdkLoader / cjsRegistry / manifest — the hub is the unit under test.
|
||||
* No real SDK load, no real CJS handler invocation (except one integration
|
||||
* path in the phase-command-router migration tests).
|
||||
* 3. ERROR_KINDS is a frozen enum. Tests switch on its values, not string literals.
|
||||
* 4. Hub must never throw. Every error surface arrives as { ok: false, ... }.
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const { createHub, ERROR_KINDS } = require('../get-shit-done/bin/lib/command-routing-hub.cjs');
|
||||
|
||||
// ─── Frozen taxonomy lock ─────────────────────────────────────────────────────
|
||||
// If the closed errorKind set drifts, this test fails before any behavioral
|
||||
// test runs — making the taxonomy shift visible at the seam.
|
||||
const EXPECTED_ERROR_KINDS = Object.freeze(new Set([
|
||||
'UnknownCommand',
|
||||
'InvalidArgs',
|
||||
'HandlerRefusal',
|
||||
'HandlerFailure',
|
||||
'SdkLoadFailed',
|
||||
'SdkDispatchFailed',
|
||||
]));
|
||||
|
||||
describe('CommandRoutingHub — ERROR_KINDS taxonomy', () => {
|
||||
test('exports a frozen ERROR_KINDS object', () => {
|
||||
assert.ok(Object.isFrozen(ERROR_KINDS), 'ERROR_KINDS must be frozen');
|
||||
});
|
||||
|
||||
test('ERROR_KINDS contains exactly the 6 documented values', () => {
|
||||
const actual = new Set(Object.values(ERROR_KINDS));
|
||||
assert.deepStrictEqual(actual, EXPECTED_ERROR_KINDS);
|
||||
});
|
||||
|
||||
test('ERROR_KINDS keys match their values (self-documenting enum)', () => {
|
||||
for (const [key, value] of Object.entries(ERROR_KINDS)) {
|
||||
assert.equal(key, value, `ERROR_KINDS.${key} should equal '${key}' but got '${value}'`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── createHub validation ──────────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — createHub validation', () => {
|
||||
test('throws synchronously on invalid mode (not sdk/cjs)', () => {
|
||||
assert.throws(() => createHub({ mode: 'invalid' }), /mode must be/);
|
||||
});
|
||||
|
||||
test('throws on missing mode', () => {
|
||||
assert.throws(() => createHub({}), /mode must be/);
|
||||
});
|
||||
|
||||
test('accepts mode: sdk', () => {
|
||||
const hub = createHub({ mode: 'sdk', sdkLoader: () => null });
|
||||
assert.ok(typeof hub.dispatch === 'function');
|
||||
});
|
||||
|
||||
test('accepts mode: cjs', () => {
|
||||
const hub = createHub({ mode: 'cjs', cjsRegistry: {} });
|
||||
assert.ok(typeof hub.dispatch === 'function');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Happy path — mode: sdk ───────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — happy path, mode: sdk', () => {
|
||||
test('dispatch returns { ok: true, data } when SDK succeeds', () => {
|
||||
const sdkExecute = (_input) => ({ ok: true, data: { phases: ['01'] }, exitCode: 0 });
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => sdkExecute,
|
||||
manifest: { phase: ['add', 'remove', 'complete'] },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: ['My phase'], cwd: '/tmp/proj', raw: false });
|
||||
|
||||
assert.ok(result.ok);
|
||||
assert.deepEqual(result.data, { phases: ['01'] });
|
||||
});
|
||||
|
||||
test('dispatch passes registryCommand as family.subcommand to SDK', () => {
|
||||
const calls = [];
|
||||
const sdkExecute = (input) => {
|
||||
calls.push(input);
|
||||
return { ok: true, data: 'done', exitCode: 0 };
|
||||
};
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => sdkExecute,
|
||||
manifest: { phase: ['next-decimal'] },
|
||||
});
|
||||
|
||||
hub.dispatch({ family: 'phase', subcommand: 'next-decimal', args: ['--raw'], cwd: '/proj', raw: true });
|
||||
|
||||
assert.equal(calls.length, 1);
|
||||
assert.equal(calls[0].registryCommand, 'phase.next-decimal');
|
||||
assert.equal(calls[0].projectDir, '/proj');
|
||||
assert.equal(calls[0].mode, 'raw');
|
||||
});
|
||||
|
||||
test('dispatch uses mode:json when raw is false', () => {
|
||||
const calls = [];
|
||||
const sdkExecute = (input) => { calls.push(input); return { ok: true, data: null, exitCode: 0 }; };
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => sdkExecute,
|
||||
});
|
||||
|
||||
hub.dispatch({ family: 'state', subcommand: 'load', args: [], cwd: '/p', raw: false });
|
||||
|
||||
assert.equal(calls[0].mode, 'json');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Happy path — mode: cjs ───────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — happy path, mode: cjs', () => {
|
||||
test('dispatch returns { ok: true, data } from CJS handler result', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
complete: (_ctx) => ({ ok: true, data: { completed: true } }),
|
||||
},
|
||||
},
|
||||
manifest: { phase: ['complete'] },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'complete', args: ['01'], cwd: '/tmp', raw: false });
|
||||
|
||||
assert.ok(result.ok);
|
||||
assert.deepEqual(result.data, { completed: true });
|
||||
});
|
||||
|
||||
test('dispatch passes full context to CJS handler', () => {
|
||||
const received = [];
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
roadmap: {
|
||||
analyze: (ctx) => { received.push(ctx); return { ok: true, data: null }; },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
hub.dispatch({ family: 'roadmap', subcommand: 'analyze', args: ['--verbose'], cwd: '/myproj', raw: true });
|
||||
|
||||
assert.equal(received.length, 1);
|
||||
assert.equal(received[0].family, 'roadmap');
|
||||
assert.equal(received[0].subcommand, 'analyze');
|
||||
assert.deepEqual(received[0].args, ['--verbose']);
|
||||
assert.equal(received[0].cwd, '/myproj');
|
||||
assert.equal(received[0].raw, true);
|
||||
});
|
||||
|
||||
test('handler returning undefined is treated as ok:true with data:null', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
state: {
|
||||
load: (_ctx) => undefined,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'state', subcommand: 'load', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(result.ok);
|
||||
assert.equal(result.data, null);
|
||||
});
|
||||
|
||||
test('handler returning a plain value wraps it as data payload', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
verify: {
|
||||
check: (_ctx) => 'all-good',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'verify', subcommand: 'check', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(result.ok);
|
||||
assert.equal(result.data, 'all-good');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: UnknownCommand ────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: UnknownCommand', () => {
|
||||
test('unknown family in manifest returns UnknownCommand', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {},
|
||||
manifest: { phase: ['add'] },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'bogus', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.UnknownCommand);
|
||||
});
|
||||
|
||||
test('unknown subcommand in manifest returns UnknownCommand', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {},
|
||||
manifest: { phase: ['add'] },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'nonexistent', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.UnknownCommand);
|
||||
});
|
||||
|
||||
test('missing family in cjsRegistry returns UnknownCommand (no manifest)', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: { state: { load: () => ({ ok: true, data: null }) } },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'bogus-family', subcommand: 'sub', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.UnknownCommand);
|
||||
});
|
||||
|
||||
test('missing subcommand in cjsRegistry returns UnknownCommand', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: { phase: { add: () => ({ ok: true, data: null }) } },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'not-there', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.UnknownCommand);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: InvalidArgs ───────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: InvalidArgs', () => {
|
||||
test('handler returning InvalidArgs result propagates it', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
insert: (_ctx) => ({
|
||||
ok: false,
|
||||
errorKind: ERROR_KINDS.InvalidArgs,
|
||||
message: 'phase insert requires a phase number',
|
||||
}),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'insert', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.InvalidArgs);
|
||||
assert.ok(result.message.includes('phase number'));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: HandlerRefusal ────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: HandlerRefusal', () => {
|
||||
test('handler returning HandlerRefusal result propagates it', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
'list-plans': (_ctx) => ({
|
||||
ok: false,
|
||||
errorKind: ERROR_KINDS.HandlerRefusal,
|
||||
message: 'phase list-plans is SDK-only',
|
||||
}),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'list-plans', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.HandlerRefusal);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: HandlerFailure ────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: HandlerFailure', () => {
|
||||
test('hub does not throw when CJS handler throws — returns HandlerFailure', () => {
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
add: (_ctx) => { throw new Error('handler blew up'); },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = hub.dispatch({ family: 'phase', subcommand: 'add', args: ['desc'], cwd: '/', raw: false });
|
||||
});
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.HandlerFailure);
|
||||
assert.ok(result.message.includes('handler blew up'));
|
||||
});
|
||||
|
||||
test('HandlerFailure details.originalError carries the thrown error', () => {
|
||||
const originalError = new Error('boom');
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
cjsRegistry: {
|
||||
state: {
|
||||
load: (_ctx) => { throw originalError; },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'state', subcommand: 'load', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.HandlerFailure);
|
||||
assert.strictEqual(result.details.originalError, originalError);
|
||||
});
|
||||
|
||||
test('hub does not throw when SDK handler throws (sdk mode)', () => {
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => (_input) => { throw new Error('sdk internal error'); },
|
||||
});
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
});
|
||||
|
||||
// SDK execution throw maps to SdkDispatchFailed (not HandlerFailure)
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkDispatchFailed);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: SdkLoadFailed ─────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: SdkLoadFailed', () => {
|
||||
test('returns SdkLoadFailed when sdkLoader throws', () => {
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => { throw new Error('sdk/dist not found'); },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkLoadFailed);
|
||||
});
|
||||
|
||||
test('returns SdkLoadFailed when sdkLoader returns null', () => {
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => null,
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkLoadFailed);
|
||||
});
|
||||
|
||||
test('returns SdkLoadFailed when sdkLoader returns a non-function', () => {
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => 'not-a-function',
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkLoadFailed);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── errorKind: SdkDispatchFailed ─────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — errorKind: SdkDispatchFailed', () => {
|
||||
test('returns SdkDispatchFailed when SDK returns ok:false', () => {
|
||||
const sdkExecute = (_input) => ({
|
||||
ok: false,
|
||||
exitCode: 1,
|
||||
errorKind: 'native_failure',
|
||||
errorDetails: { message: 'phase not found' },
|
||||
});
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => sdkExecute,
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'complete', args: ['99'], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkDispatchFailed);
|
||||
assert.ok(result.message.includes('phase not found'));
|
||||
});
|
||||
|
||||
test('SdkDispatchFailed details.originalError is populated when SDK throws', () => {
|
||||
const sdkError = new Error('sdk crashed mid-dispatch');
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => (_input) => { throw sdkError; },
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkDispatchFailed);
|
||||
assert.strictEqual(result.details.originalError, sdkError);
|
||||
});
|
||||
|
||||
test('no transparent fallback: SDK crash does NOT retry via CJS', () => {
|
||||
// Provide a cjsRegistry — hub should NOT call it after SDK failure.
|
||||
const cjsCalls = [];
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => (_input) => { throw new Error('sdk dead'); },
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
add: (_ctx) => { cjsCalls.push(true); return { ok: true, data: 'cjs-result' }; },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.equal(cjsCalls.length, 0, 'CJS handler must not be called when mode is sdk');
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkDispatchFailed);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── mode is fixed at construction ────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — mode fixed at construction', () => {
|
||||
test('sdk-mode hub never calls cjsRegistry even when sdkLoader later fails', () => {
|
||||
const cjsCalls = [];
|
||||
// Start with a working sdkLoader
|
||||
let sdkShouldWork = true;
|
||||
const hub = createHub({
|
||||
mode: 'sdk',
|
||||
sdkLoader: () => {
|
||||
if (!sdkShouldWork) throw new Error('sdk unavailable');
|
||||
return (_input) => ({ ok: true, data: 'sdk-data', exitCode: 0 });
|
||||
},
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
add: (_ctx) => { cjsCalls.push(true); return { ok: true, data: 'cjs-data' }; },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// First dispatch: SDK works
|
||||
const first = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
assert.ok(first.ok);
|
||||
assert.equal(first.data, 'sdk-data');
|
||||
assert.equal(cjsCalls.length, 0);
|
||||
|
||||
// SDK breaks between calls — mode is still 'sdk', no fallback to cjs
|
||||
sdkShouldWork = false;
|
||||
const second = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
assert.ok(!second.ok);
|
||||
assert.equal(second.errorKind, ERROR_KINDS.SdkLoadFailed);
|
||||
assert.equal(cjsCalls.length, 0, 'CJS handler must never be called from an sdk-mode hub');
|
||||
});
|
||||
|
||||
test('cjs-mode hub never calls sdkLoader', () => {
|
||||
const sdkCalls = [];
|
||||
const hub = createHub({
|
||||
mode: 'cjs',
|
||||
sdkLoader: () => { sdkCalls.push(true); return () => ({ ok: true, data: 'sdk' }); },
|
||||
cjsRegistry: {
|
||||
phase: {
|
||||
add: (_ctx) => ({ ok: true, data: 'cjs-ok' }),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
|
||||
assert.ok(result.ok);
|
||||
assert.equal(result.data, 'cjs-ok');
|
||||
assert.equal(sdkCalls.length, 0, 'sdkLoader must never be called from a cjs-mode hub');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── hub never throws ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('CommandRoutingHub — hub never throws', () => {
|
||||
test('hub does not throw even when cjsRegistry is completely absent in cjs mode', () => {
|
||||
const hub = createHub({ mode: 'cjs' });
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
});
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.UnknownCommand);
|
||||
});
|
||||
|
||||
test('hub does not throw when sdkLoader is absent in sdk mode', () => {
|
||||
const hub = createHub({ mode: 'sdk' });
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
});
|
||||
|
||||
assert.ok(!result.ok);
|
||||
assert.equal(result.errorKind, ERROR_KINDS.SdkLoadFailed);
|
||||
});
|
||||
|
||||
test('hub does not throw when dispatch receives malformed request', () => {
|
||||
const hub = createHub({ mode: 'cjs', cjsRegistry: {} });
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
// Missing family — would normally throw on string ops
|
||||
result = hub.dispatch({ family: undefined, subcommand: 'add', args: [], cwd: '/', raw: false });
|
||||
});
|
||||
|
||||
// Result is an error, not a thrown exception
|
||||
assert.ok(!result.ok);
|
||||
});
|
||||
});
|
||||
420
tests/phase-command-router.test.cjs
Normal file
420
tests/phase-command-router.test.cjs
Normal file
@@ -0,0 +1,420 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Behavioral tests for the phase-command-router adapter (#3788).
|
||||
*
|
||||
* Shape:
|
||||
* 1. Adapter translation — CLI args → hub dispatch shape
|
||||
* 2. Result translation — hub result → stdout / error callback
|
||||
* 3. Unsupported subcommands — SDK-only commands produce the documented error
|
||||
* 4. Unknown subcommand — unmapped subcommands produce a well-formed error
|
||||
* 5. Integration — real hub + real CJS phase handler invocation
|
||||
*
|
||||
* Testing rules in force (CONTRIBUTING.md § Testing Standards):
|
||||
* - No readFileSync of source files.
|
||||
* - No mocking of the hub itself — tests use the real CommandRoutingHub
|
||||
* with stub cjsRegistry / sdkLoader entries.
|
||||
* - All assertions on return values and captured call arguments.
|
||||
*/
|
||||
|
||||
const { describe, test, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const { routePhaseCommand } = require('../get-shit-done/bin/lib/phase-command-router.cjs');
|
||||
|
||||
// Force CJS path throughout: set GSD_WORKSTREAM so tryLoadSdk() is bypassed.
|
||||
// This makes unit-level assertions deterministic regardless of SDK build state.
|
||||
let _prevWorkstream;
|
||||
before(() => {
|
||||
_prevWorkstream = process.env.GSD_WORKSTREAM;
|
||||
process.env.GSD_WORKSTREAM = 'test-unit';
|
||||
});
|
||||
after(() => {
|
||||
if (_prevWorkstream === undefined) delete process.env.GSD_WORKSTREAM;
|
||||
else process.env.GSD_WORKSTREAM = _prevWorkstream;
|
||||
});
|
||||
|
||||
// ─── Helper: build a minimal phase stub ──────────────────────────────────────
|
||||
|
||||
function makePhase(overrides = {}) {
|
||||
return {
|
||||
cmdPhaseMvpMode: () => {},
|
||||
cmdPhaseNextDecimal: () => {},
|
||||
cmdPhaseAdd: () => {},
|
||||
cmdPhaseAddBatch: () => {},
|
||||
cmdPhaseInsert: () => {},
|
||||
cmdPhaseRemove: () => {},
|
||||
cmdPhaseComplete: () => {},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// ─── 1. Adapter translation — CLI args → handler calls ───────────────────────
|
||||
|
||||
describe('phase-command-router — CLI arg translation (CJS path)', () => {
|
||||
test('routes phase mvp-mode: passes cwd, args.slice(2), raw to handler', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseMvpMode: (cwd, slicedArgs, raw) => calls.push({ cwd, slicedArgs, raw }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'mvp-mode', '--enable'], cwd: '/proj', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls.length, 1);
|
||||
assert.equal(calls[0].cwd, '/proj');
|
||||
assert.deepEqual(calls[0].slicedArgs, ['--enable']);
|
||||
assert.equal(calls[0].raw, false);
|
||||
});
|
||||
|
||||
test('routes phase next-decimal: passes cwd, args[2], raw to handler', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseNextDecimal: (cwd, label, raw) => calls.push({ cwd, label, raw }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'next-decimal', '5'], cwd: '/p', raw: true, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].label, '5');
|
||||
assert.equal(calls[0].raw, true);
|
||||
});
|
||||
|
||||
test('routes phase add: strips --raw, passes joined description and no customId', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseAdd: (cwd, desc, raw, customId) => calls.push({ cwd, desc, raw, customId }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'add', 'My', 'phase', '--raw'], cwd: '/p', raw: true, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].desc, 'My phase');
|
||||
assert.equal(calls[0].customId, null);
|
||||
});
|
||||
|
||||
test('routes phase add: captures --id value', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseAdd: (cwd, desc, raw, customId) => calls.push({ customId }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'add', '--id', '05', 'Desc'], cwd: '/p', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].customId, '05');
|
||||
});
|
||||
|
||||
test('routes phase add-batch: --descriptions JSON array parses correctly', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseAddBatch: (cwd, descriptions, raw) => calls.push({ descriptions }),
|
||||
});
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add-batch', '--descriptions', '["Phase A","Phase B"]'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { throw new Error(m); },
|
||||
});
|
||||
|
||||
assert.deepEqual(calls[0].descriptions, ['Phase A', 'Phase B']);
|
||||
});
|
||||
|
||||
test('routes phase add-batch: positional args used when --descriptions absent', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseAddBatch: (cwd, descriptions, raw) => calls.push({ descriptions }),
|
||||
});
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add-batch', 'A', 'B', '--raw'],
|
||||
cwd: '/p',
|
||||
raw: true,
|
||||
error: (m) => { throw new Error(m); },
|
||||
});
|
||||
|
||||
assert.deepEqual(calls[0].descriptions, ['A', 'B']);
|
||||
});
|
||||
|
||||
test('routes phase insert: passes cwd, phaseNum, trailing joined, raw', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseInsert: (cwd, phaseNum, desc, raw) => calls.push({ cwd, phaseNum, desc, raw }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'insert', '02', 'New', 'phase'], cwd: '/p', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].phaseNum, '02');
|
||||
assert.equal(calls[0].desc, 'New phase');
|
||||
});
|
||||
|
||||
test('routes phase remove: passes cwd, phaseNum, force:false, raw', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseRemove: (cwd, phaseNum, opts, raw) => calls.push({ cwd, phaseNum, opts, raw }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'remove', '03'], cwd: '/p', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].phaseNum, '03');
|
||||
assert.deepEqual(calls[0].opts, { force: false });
|
||||
});
|
||||
|
||||
test('routes phase remove: --force flag sets opts.force:true', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseRemove: (cwd, phaseNum, opts, raw) => calls.push({ opts }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'remove', '--force', '03'], cwd: '/p', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.deepEqual(calls[0].opts, { force: true });
|
||||
});
|
||||
|
||||
test('routes phase complete: passes cwd, args[2], raw', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseComplete: (cwd, phaseNum, raw) => calls.push({ cwd, phaseNum, raw }),
|
||||
});
|
||||
|
||||
routePhaseCommand({ phase, args: ['phase', 'complete', '04'], cwd: '/p', raw: false, error: (m) => { throw new Error(m); } });
|
||||
|
||||
assert.equal(calls[0].phaseNum, '04');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 2. Result translation — error callback on failure ───────────────────────
|
||||
|
||||
describe('phase-command-router — result translation (error path)', () => {
|
||||
test('phase add with --id missing value calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add', '--id'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null, 'error callback must be called');
|
||||
assert.ok(msg.includes('--id requires a value'));
|
||||
});
|
||||
|
||||
test('phase add with unknown flag calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add', '--unknown-flag'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('phase add does not support'));
|
||||
});
|
||||
|
||||
test('phase add-batch with non-JSON --descriptions calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add-batch', '--descriptions', 'not-json'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('JSON array'));
|
||||
});
|
||||
|
||||
test('phase insert with --dry-run calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'insert', '02', '--dry-run'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('does not support --dry-run'));
|
||||
});
|
||||
|
||||
test('phase remove with unsupported flag calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'remove', '--quiet', '03'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('phase remove does not support'));
|
||||
});
|
||||
|
||||
test('phase remove without a phase number calls error()', () => {
|
||||
let msg = null;
|
||||
const phase = makePhase();
|
||||
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'remove'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('exactly one phase number'));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 3. Unsupported (SDK-only) subcommands ────────────────────────────────────
|
||||
|
||||
describe('phase-command-router — SDK-only subcommands', () => {
|
||||
test('phase list-plans calls error() with SDK-only message', () => {
|
||||
let msg = null;
|
||||
routePhaseCommand({
|
||||
phase: makePhase(),
|
||||
args: ['phase', 'list-plans'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('SDK-only'), `expected "SDK-only" in: ${msg}`);
|
||||
assert.ok(msg.includes('list-plans'));
|
||||
});
|
||||
|
||||
test('phase list-artifacts calls error() with SDK-only message', () => {
|
||||
let msg = null;
|
||||
routePhaseCommand({
|
||||
phase: makePhase(),
|
||||
args: ['phase', 'list-artifacts'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('SDK-only'));
|
||||
assert.ok(msg.includes('list-artifacts'));
|
||||
});
|
||||
|
||||
test('phase scaffold calls error() with redirect message', () => {
|
||||
let msg = null;
|
||||
routePhaseCommand({
|
||||
phase: makePhase(),
|
||||
args: ['phase', 'scaffold'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('scaffold'));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 4. Unknown subcommand ────────────────────────────────────────────────────
|
||||
|
||||
describe('phase-command-router — unknown subcommand', () => {
|
||||
test('unknown phase subcommand calls error() listing available ones', () => {
|
||||
let msg = null;
|
||||
routePhaseCommand({
|
||||
phase: makePhase(),
|
||||
args: ['phase', 'frobnicate'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg !== null);
|
||||
assert.ok(msg.includes('Unknown phase subcommand'));
|
||||
assert.ok(msg.includes('Available:'), `expected "Available:" in: ${msg}`);
|
||||
});
|
||||
|
||||
test('unknown subcommand message lists canonical manifest commands like add and complete', () => {
|
||||
// mvp-mode is NOT in the available list (not in PHASE_SUBCOMMANDS manifest),
|
||||
// matching the pre-#3788 behaviour of routeCjsCommandFamily.
|
||||
// The list does include the manifest-backed commands: add, complete, etc.
|
||||
let msg = null;
|
||||
routePhaseCommand({
|
||||
phase: makePhase(),
|
||||
args: ['phase', 'bogus'],
|
||||
cwd: '/p',
|
||||
raw: false,
|
||||
error: (m) => { msg = m; },
|
||||
});
|
||||
|
||||
assert.ok(msg.includes('add'), `expected add in available list: ${msg}`);
|
||||
assert.ok(msg.includes('complete'), `expected complete in available list: ${msg}`);
|
||||
assert.ok(!msg.includes('list-plans'), `list-plans (SDK-only) must not appear in available list: ${msg}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 5. Integration — real hub + real CJS phase handler ──────────────────────
|
||||
//
|
||||
// This test exercises the full dispatch chain end-to-end:
|
||||
// routePhaseCommand → createHub (cjs mode) → dispatch → cjsRegistry handler
|
||||
// → phase stub → assert the call was received.
|
||||
//
|
||||
// No mocking of the hub. The phase stub is a real dependency collaborator.
|
||||
|
||||
describe('phase-command-router — integration: real hub + CJS phase handler', () => {
|
||||
test('dispatches phase complete through real hub to real CJS handler chain', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseComplete: (cwd, phaseNum, raw) => calls.push({ cwd, phaseNum, raw }),
|
||||
});
|
||||
|
||||
// No error should occur; handler should be called exactly once.
|
||||
let errorMsg = null;
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'complete', '07'],
|
||||
cwd: '/integration',
|
||||
raw: false,
|
||||
error: (m) => { errorMsg = m; },
|
||||
});
|
||||
|
||||
assert.equal(errorMsg, null, `unexpected error: ${errorMsg}`);
|
||||
assert.equal(calls.length, 1);
|
||||
assert.equal(calls[0].cwd, '/integration');
|
||||
assert.equal(calls[0].phaseNum, '07');
|
||||
assert.equal(calls[0].raw, false);
|
||||
});
|
||||
|
||||
test('dispatches phase add-batch through real hub with --descriptions', () => {
|
||||
const calls = [];
|
||||
const phase = makePhase({
|
||||
cmdPhaseAddBatch: (cwd, descriptions, raw) => calls.push({ descriptions }),
|
||||
});
|
||||
|
||||
let errorMsg = null;
|
||||
routePhaseCommand({
|
||||
phase,
|
||||
args: ['phase', 'add-batch', '--descriptions', '["Alpha","Beta"]'],
|
||||
cwd: '/integration',
|
||||
raw: false,
|
||||
error: (m) => { errorMsg = m; },
|
||||
});
|
||||
|
||||
assert.equal(errorMsg, null, `unexpected error: ${errorMsg}`);
|
||||
assert.deepEqual(calls[0].descriptions, ['Alpha', 'Beta']);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user