diff --git a/.changeset/lucky-birds-bark.md b/.changeset/lucky-birds-bark.md new file mode 100644 index 000000000..5bc78315f --- /dev/null +++ b/.changeset/lucky-birds-bark.md @@ -0,0 +1,8 @@ +--- +type: Removed +pr: 175 +--- +**`CommandRoutingHub` no longer carries dual-runtime selection** — the `mode`, `sdkLoader`, and `SdkDispatchFailed` errorKind are removed. The Hub routes exclusively through CJS handlers. No change to the observable `dispatch()` contract. + + + diff --git a/get-shit-done/bin/lib/command-routing-hub.cjs b/get-shit-done/bin/lib/command-routing-hub.cjs index 0cdab02c3..ddd0951f5 100644 --- a/get-shit-done/bin/lib/command-routing-hub.cjs +++ b/get-shit-done/bin/lib/command-routing-hub.cjs @@ -1,27 +1,24 @@ 'use strict'; /** - * Command Routing Hub — issue #3788. + * Command Routing Hub — issue #3788, simplified in #175. * - * A pure-result dispatch hub that centralizes the mode decision (SDK vs CJS), + * A pure-result dispatch hub that centralizes CJS routing, * the error taxonomy, and the no-throw contract that all command-family routers * currently duplicate independently. * * Design: - * createHub({ mode, sdkLoader, cjsRegistry, manifest }) -> hub + * createHub({ cjsRegistry, manifest }) -> hub * hub.dispatch({ family, subcommand, args, cwd, raw }) -> Result * * Result = { ok: true, data } * | { ok: false, errorKind, message, details? } * * Invariants: - * - `mode` is fixed at construction; never re-evaluated per dispatch. + * - Hub always routes through CJS handlers. There is no SDK path (#175). * - Hub never prints to stdout/stderr, never calls process.exit. * - Hub never throws — all internal throws are caught and converted to * { ok: false, errorKind: 'HandlerFailure', message, details }. - * - No transparent fallback: an SDK-mode hub that encounters an SDK crash - * returns { ok: false, errorKind: 'SdkDispatchFailed' }; it does NOT - * silently retry via the CJS registry. * - The errorKind taxonomy is closed. Callers switch on ERROR_KINDS values. */ @@ -29,6 +26,8 @@ * Closed errorKind enum. Export as a frozen object so callers can switch on * ERROR_KINDS.UnknownCommand etc. without relying on bare string literals. * + * #175: SdkLoadFailed and SdkDispatchFailed removed — Hub is CJS-only. + * * @readonly */ const ERROR_KINDS = Object.freeze({ @@ -40,10 +39,6 @@ const ERROR_KINDS = Object.freeze({ HandlerRefusal: 'HandlerRefusal', /** A handler threw an unexpected exception. */ HandlerFailure: 'HandlerFailure', - /** The sdkLoader function threw or returned a falsy value at dispatch time. */ - SdkLoadFailed: 'SdkLoadFailed', - /** The SDK was loaded but threw or returned ok:false during execution. */ - SdkDispatchFailed: 'SdkDispatchFailed', }); /** @@ -54,13 +49,10 @@ const ERROR_KINDS = Object.freeze({ /** * @typedef {object} HubOptions - * @property {'sdk' | 'cjs'} mode - Dispatch mode, fixed at construction. - * @property {() => unknown} [sdkLoader] - Callable that returns the SDK execute - * function (or throws). Only used when mode === 'sdk'. * @property {Record HubResult>>} [cjsRegistry] - - * Nested map of family -> subcommand -> handler. Only used when mode === 'cjs'. + * Nested map of family -> subcommand -> handler. * @property {Record} [manifest] - Map of family -> known subcommands. - * Used for UnknownCommand detection regardless of mode. + * Used for UnknownCommand detection. */ /** @@ -69,14 +61,7 @@ const ERROR_KINDS = Object.freeze({ * @param {HubOptions} options * @returns {{ dispatch: (req: object) => HubResult }} */ -function createHub({ mode, sdkLoader, cjsRegistry, manifest }) { - if (mode !== 'sdk' && mode !== 'cjs') { - throw new TypeError(`CommandRoutingHub: mode must be 'sdk' or 'cjs', got ${JSON.stringify(mode)}`); - } - - // Validate mode once at construction; never re-check per dispatch. - const _mode = mode; - const _sdkLoader = sdkLoader; +function createHub({ cjsRegistry, manifest } = {}) { const _cjsRegistry = cjsRegistry; const _manifest = manifest; @@ -102,7 +87,7 @@ function createHub({ mode, sdkLoader, cjsRegistry, manifest }) { function _dispatch(req) { const { family, subcommand, args = [], cwd, raw } = req; - // ── manifest check (applies to both modes) ────────────────────────────── + // ── manifest check ──────────────────────────────────────────────────────── if (_manifest) { const knownSubcommands = _manifest[family]; if (!knownSubcommands) { @@ -121,70 +106,9 @@ function createHub({ mode, sdkLoader, cjsRegistry, manifest }) { } } - if (_mode === 'sdk') { - return _dispatchSdk({ family, subcommand, args, cwd, raw }); - } - return _dispatchCjs({ family, subcommand, args, cwd, raw }); } - function _dispatchSdk({ family, subcommand, args, cwd, raw }) { - // Load the SDK execute function (or return SdkLoadFailed). - let executeForCjs; - try { - executeForCjs = _sdkLoader ? _sdkLoader() : null; - } catch (err) { - return { - ok: false, - errorKind: ERROR_KINDS.SdkLoadFailed, - message: `SDK load failed: ${err instanceof Error ? err.message : String(err)}`, - details: { originalError: err }, - }; - } - - if (!executeForCjs || typeof executeForCjs !== 'function') { - return { - ok: false, - errorKind: ERROR_KINDS.SdkLoadFailed, - message: 'SDK loader did not return a callable execute function', - }; - } - - // Call the SDK. No transparent fallback — any error becomes SdkDispatchFailed. - let result; - try { - result = executeForCjs({ - registryCommand: subcommand ? `${family}.${subcommand}` : family, - registryArgs: args, - legacyCommand: family, - legacyArgs: [subcommand, ...args].filter(Boolean), - mode: raw ? 'raw' : 'json', - projectDir: cwd, - }); - } catch (err) { - return { - ok: false, - errorKind: ERROR_KINDS.SdkDispatchFailed, - message: err instanceof Error ? err.message : String(err), - details: { originalError: err }, - }; - } - - if (!result || !result.ok) { - const message = (result && result.errorDetails && result.errorDetails.message) - ? result.errorDetails.message - : `${family}${subcommand ? ' ' + subcommand : ''} failed (${result && result.errorKind})`; - return { - ok: false, - errorKind: ERROR_KINDS.SdkDispatchFailed, - message, - details: result || {}, - }; - } - - return { ok: true, data: result.data }; - } - function _dispatchCjs({ family, subcommand, args, cwd, raw }) { if (!_cjsRegistry) { return { diff --git a/get-shit-done/bin/lib/phase-command-router.cjs b/get-shit-done/bin/lib/phase-command-router.cjs index ac972f523..b520634dd 100644 --- a/get-shit-done/bin/lib/phase-command-router.cjs +++ b/get-shit-done/bin/lib/phase-command-router.cjs @@ -1,37 +1,28 @@ 'use strict'; const { PHASE_SUBCOMMANDS } = require('./command-aliases.generated.cjs'); -const { output } = require('./core.cjs'); -// ─── SDK bridge (Phase 6) — shared loader via cjs-sdk-bridge.cjs ────────────── -const { tryLoadSdk, getExecuteForCjs } = require('./cjs-sdk-bridge.cjs'); - -// ─── CommandRoutingHub (issue #3788) ────────────────────────────────────────── +// ─── CommandRoutingHub (issue #3788, simplified in #175) ────────────────────── const { createHub, ERROR_KINDS } = require('./command-routing-hub.cjs'); /** * Manifest-backed phase subcommand router. * Keeps gsd-tools.cjs thin while preserving existing command semantics. * - * Phase 6: all CJS-handled phase subcommands are dispatched via executeForCjs - * when the SDK is available. CJS fallback retained when: - * - GSD_WORKSTREAM is active (workstream-scoped requests fall through to CJS). - * - SDK is unavailable (build not present). + * #175: Hub is CJS-only. The SDK bridge is still separately invokable via + * bin/lib/cjs-sdk-bridge.cjs, but the Hub no longer routes to it. * - * SDK-only (unsupported in CJS router): + * SDK-only (unsupported in CJS router — error returned before dispatch): * - list-plans: SDK-only. * - list-artifacts: SDK-only. * - scaffold: routed through top-level scaffold command. * - * CJS-only subcommands: none. + * CJS-only subcommands: mvp-mode (dispatched directly, before hub). * - * #3788: dispatch is now mediated by CommandRoutingHub. The public entry point + * #3788: dispatch is mediated by CommandRoutingHub. The public entry point * and observable CLI behaviour are unchanged. */ function routePhaseCommand({ phase, args, cwd, raw, error }) { - const activeWorkstream = process.env.GSD_WORKSTREAM; - const sdkAvailable = !activeWorkstream && tryLoadSdk(); - // ── Unsupported / SDK-only subcommands ───────────────────────────────────── // Resolved before dispatch so the error message matches the pre-#3788 text. const UNSUPPORTED = { @@ -58,12 +49,11 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { return; } - // ── CJS-only subcommands (always bypass SDK path) ────────────────────────── + // ── CJS-only subcommands (dispatched directly, before hub) ───────────────── // `mvp-mode` has a CJS-native implementation in phase.cmdPhaseMvpMode that // differs from the SDK query layer (different ROADMAP scan + error codes). - // Dispatch it early so the SDK hub path is never reached for this subcommand, - // preserving the pre-migration observable behaviour (correct exit code, - // correct JSON error reason code, correct ROADMAP scan). + // Dispatch it early to preserve pre-migration observable behaviour (correct + // exit code, correct JSON error reason code, correct ROADMAP scan). if (subcommand === 'mvp-mode') { phase.cmdPhaseMvpMode(cwd, args.slice(2), raw); return; @@ -157,15 +147,6 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { }, }; - // ── Build the SDK loader ──────────────────────────────────────────────────── - function sdkLoader() { - const execute = getExecuteForCjs(); - if (!execute) return null; - // Wrap executeForCjs to match the hub's sdkLoader contract: - // hub calls sdkLoader() -> returns the execute function itself. - return execute; - } - // ── Build manifest (available subcommands for UnknownCommand detection) ───── // `availableSubcommands` is what the error message shows. It excludes // SDK-only unsupported commands (already handled above) but does NOT include @@ -179,14 +160,9 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { const manifestSubcommands = ['mvp-mode', ...availableSubcommands]; const manifest = { phase: manifestSubcommands }; - // ── Construct hub (mode fixed at call time based on env + SDK availability) ─ - const mode = sdkAvailable ? 'sdk' : 'cjs'; - const hub = createHub({ - mode, - sdkLoader: mode === 'sdk' ? sdkLoader : undefined, - cjsRegistry: mode === 'cjs' ? cjsRegistry : undefined, - manifest, - }); + // ── Construct hub ────────────────────────────────────────────────────────── + // #175: Hub is CJS-only — no mode param, no sdkLoader. + const hub = createHub({ cjsRegistry, manifest }); // ── Dispatch ──────────────────────────────────────────────────────────────── const result = hub.dispatch({ @@ -198,27 +174,18 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { }); // ── Translate result → CLI output / error (adapter responsibility) ────────── + // CJS handlers call output() themselves (inside phase.cmdPhase*()). + // No further output call is needed here. if (!result.ok) { if (result.errorKind === ERROR_KINDS.UnknownCommand) { const available = availableSubcommands.join(', '); error(`Unknown phase subcommand. Available: ${available}`); return; } - // InvalidArgs, HandlerRefusal, HandlerFailure, SdkLoadFailed, SdkDispatchFailed + // InvalidArgs, HandlerRefusal, HandlerFailure error(result.message); return; } - - // SDK path: the hub wraps executeForCjs; data projection is the adapter's job. - // CJS handlers call output() themselves (inside phase.cmdPhase*()), so no - // further output call is needed for cjs mode. - if (mode === 'sdk') { - if (raw) { - output(null, true, typeof result.data === 'string' ? result.data : String(result.data ?? '')); - } else { - output(result.data); - } - } } module.exports = { diff --git a/tests/command-routing-hub.test.cjs b/tests/command-routing-hub.test.cjs index 790577d03..94acffb56 100644 --- a/tests/command-routing-hub.test.cjs +++ b/tests/command-routing-hub.test.cjs @@ -1,12 +1,14 @@ 'use strict'; /** - * Behavioral contract tests for the CommandRoutingHub (issue #3788). + * Behavioral contract tests for the CommandRoutingHub (issue #3788, #175). + * + * #175: mode/sdkLoader/SdkDispatchFailed dropped. Hub always routes CJS. * * 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. + * 2. Stub 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. @@ -19,15 +21,13 @@ 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. +// #175: SdkDispatchFailed and SdkLoadFailed are removed from the closed enum. +// The set shrinks from 6 to 4 values. const EXPECTED_ERROR_KINDS = Object.freeze(new Set([ 'UnknownCommand', 'InvalidArgs', 'HandlerRefusal', 'HandlerFailure', - 'SdkLoadFailed', - 'SdkDispatchFailed', ])); describe('CommandRoutingHub — ERROR_KINDS taxonomy', () => { @@ -35,11 +35,21 @@ describe('CommandRoutingHub — ERROR_KINDS taxonomy', () => { assert.ok(Object.isFrozen(ERROR_KINDS), 'ERROR_KINDS must be frozen'); }); - test('ERROR_KINDS contains exactly the 6 documented values', () => { + test('ERROR_KINDS contains exactly the 4 documented values (SdkDispatchFailed and SdkLoadFailed removed)', () => { const actual = new Set(Object.values(ERROR_KINDS)); assert.deepStrictEqual(actual, EXPECTED_ERROR_KINDS); }); + test('ERROR_KINDS does NOT contain SdkDispatchFailed', () => { + assert.ok(!Object.values(ERROR_KINDS).includes('SdkDispatchFailed'), + 'SdkDispatchFailed must not be in ERROR_KINDS after #175'); + }); + + test('ERROR_KINDS does NOT contain SdkLoadFailed', () => { + assert.ok(!Object.values(ERROR_KINDS).includes('SdkLoadFailed'), + 'SdkLoadFailed must not be in ERROR_KINDS after #175'); + }); + 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}'`); @@ -48,84 +58,83 @@ describe('CommandRoutingHub — ERROR_KINDS taxonomy', () => { }); // ─── createHub validation ────────────────────────────────────────────────────── +// #175: mode param is removed. Hub is constructed without mode. 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 }); + test('constructs successfully without any mode parameter', () => { + // Hub no longer requires mode — no throw when mode is absent + const hub = createHub({ cjsRegistry: {} }); 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 }); + test('mode parameter is ignored — passing mode: sdk does not route to SDK', () => { + // Even if a legacy caller passes mode:'sdk', the hub must use CJS dispatch. + const cjsCalls = []; const hub = createHub({ mode: 'sdk', - sdkLoader: () => sdkExecute, - manifest: { phase: ['add', 'remove', 'complete'] }, + cjsRegistry: { + phase: { + add: (_ctx) => { cjsCalls.push(true); return { ok: true, data: 'cjs-dispatched' }; }, + }, + }, }); - const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: ['My phase'], cwd: '/tmp/proj', raw: false }); + const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false }); - assert.ok(result.ok); - assert.deepEqual(result.data, { phases: ['01'] }); + // Must route through CJS, not SDK + assert.ok(result.ok, `Expected ok:true but got: ${JSON.stringify(result)}`); + assert.equal(result.data, 'cjs-dispatched', 'Hub must dispatch through CJS regardless of mode parameter'); + assert.equal(cjsCalls.length, 1, 'CJS handler must be called exactly once'); }); - 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', () => { + test('mode parameter is ignored — passing mode: cjs also routes through CJS', () => { + const cjsCalls = []; const hub = createHub({ mode: 'cjs', + cjsRegistry: { + state: { + load: (_ctx) => { cjsCalls.push(true); return { ok: true, data: 'state-loaded' }; }, + }, + }, + }); + + const result = hub.dispatch({ family: 'state', subcommand: 'load', args: [], cwd: '/', raw: false }); + + assert.ok(result.ok); + assert.equal(result.data, 'state-loaded'); + assert.equal(cjsCalls.length, 1); + }); + + test('sdkLoader parameter is inert — passing sdkLoader does not cause SDK dispatch', () => { + // sdkLoader is removed; passing it must not cause the Hub to call it + const sdkCalls = []; + const hub = createHub({ + sdkLoader: () => { sdkCalls.push(true); return () => ({ ok: true, data: 'sdk-data' }); }, + cjsRegistry: { + phase: { + add: (_ctx) => ({ ok: true, data: 'cjs-data' }), + }, + }, + }); + + const result = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false }); + + assert.equal(sdkCalls.length, 0, 'sdkLoader must never be called — it is removed in #175'); + assert.ok(result.ok); + assert.equal(result.data, 'cjs-data'); + }); + + test('constructs successfully with only cjsRegistry', () => { + const hub = createHub({ cjsRegistry: { phase: { add: () => ({ ok: true, data: null }) } } }); + assert.ok(typeof hub.dispatch === 'function'); + }); +}); + +// ─── Happy path — always CJS ────────────────────────────────────────────────── + +describe('CommandRoutingHub — happy path, CJS dispatch', () => { + test('dispatch returns { ok: true, data } from CJS handler result', () => { + const hub = createHub({ cjsRegistry: { phase: { complete: (_ctx) => ({ ok: true, data: { completed: true } }), @@ -143,7 +152,6 @@ describe('CommandRoutingHub — happy path, mode: cjs', () => { 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 }; }, @@ -163,7 +171,6 @@ describe('CommandRoutingHub — happy path, mode: cjs', () => { test('handler returning undefined is treated as ok:true with data:null', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { state: { load: (_ctx) => undefined, @@ -179,7 +186,6 @@ describe('CommandRoutingHub — happy path, mode: cjs', () => { test('handler returning a plain value wraps it as data payload', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { verify: { check: (_ctx) => 'all-good', @@ -199,7 +205,6 @@ describe('CommandRoutingHub — happy path, mode: cjs', () => { describe('CommandRoutingHub — errorKind: UnknownCommand', () => { test('unknown family in manifest returns UnknownCommand', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: {}, manifest: { phase: ['add'] }, }); @@ -212,7 +217,6 @@ describe('CommandRoutingHub — errorKind: UnknownCommand', () => { test('unknown subcommand in manifest returns UnknownCommand', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: {}, manifest: { phase: ['add'] }, }); @@ -225,7 +229,6 @@ describe('CommandRoutingHub — errorKind: UnknownCommand', () => { test('missing family in cjsRegistry returns UnknownCommand (no manifest)', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { state: { load: () => ({ ok: true, data: null }) } }, }); @@ -237,7 +240,6 @@ describe('CommandRoutingHub — errorKind: UnknownCommand', () => { test('missing subcommand in cjsRegistry returns UnknownCommand', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { phase: { add: () => ({ ok: true, data: null }) } }, }); @@ -253,7 +255,6 @@ describe('CommandRoutingHub — errorKind: UnknownCommand', () => { describe('CommandRoutingHub — errorKind: InvalidArgs', () => { test('handler returning InvalidArgs result propagates it', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { phase: { insert: (_ctx) => ({ @@ -278,7 +279,6 @@ describe('CommandRoutingHub — errorKind: InvalidArgs', () => { describe('CommandRoutingHub — errorKind: HandlerRefusal', () => { test('handler returning HandlerRefusal result propagates it', () => { const hub = createHub({ - mode: 'cjs', cjsRegistry: { phase: { 'list-plans': (_ctx) => ({ @@ -302,7 +302,6 @@ describe('CommandRoutingHub — errorKind: HandlerRefusal', () => { 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'); }, @@ -323,7 +322,6 @@ describe('CommandRoutingHub — errorKind: HandlerFailure', () => { test('HandlerFailure details.originalError carries the thrown error', () => { const originalError = new Error('boom'); const hub = createHub({ - mode: 'cjs', cjsRegistry: { state: { load: (_ctx) => { throw originalError; }, @@ -337,180 +335,13 @@ describe('CommandRoutingHub — errorKind: HandlerFailure', () => { 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' }); + test('hub does not throw even when cjsRegistry is completely absent', () => { + const hub = createHub({}); let result; assert.doesNotThrow(() => { @@ -521,20 +352,8 @@ describe('CommandRoutingHub — hub never throws', () => { 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: {} }); + const hub = createHub({ cjsRegistry: {} }); let result; assert.doesNotThrow(() => { @@ -546,3 +365,43 @@ describe('CommandRoutingHub — hub never throws', () => { assert.ok(!result.ok); }); }); + +// ─── No SDK path — single-dispatch invariant ────────────────────────────────── +// #175: Hub is always CJS. There is no SDK path to fall through to. + +describe('CommandRoutingHub — single CJS dispatch invariant (#175)', () => { + test('two dispatches through the same hub produce consistent CJS results', () => { + const calls = []; + const hub = createHub({ + cjsRegistry: { + phase: { + add: (_ctx) => { calls.push('add'); return { ok: true, data: 'added' }; }, + complete: (_ctx) => { calls.push('complete'); return { ok: true, data: 'done' }; }, + }, + }, + }); + + const r1 = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false }); + const r2 = hub.dispatch({ family: 'phase', subcommand: 'complete', args: [], cwd: '/', raw: false }); + + assert.ok(r1.ok); + assert.equal(r1.data, 'added'); + assert.ok(r2.ok); + assert.equal(r2.data, 'done'); + assert.deepEqual(calls, ['add', 'complete']); + }); + + test('manifest check still applies in CJS-only hub', () => { + const hub = createHub({ + cjsRegistry: { phase: { add: () => ({ ok: true, data: null }) } }, + manifest: { phase: ['add'] }, + }); + + const known = hub.dispatch({ family: 'phase', subcommand: 'add', args: [], cwd: '/', raw: false }); + const unknown = hub.dispatch({ family: 'phase', subcommand: 'nonexistent', args: [], cwd: '/', raw: false }); + + assert.ok(known.ok); + assert.ok(!unknown.ok); + assert.equal(unknown.errorKind, ERROR_KINDS.UnknownCommand); + }); +});