golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(hub): drop mode/sdkLoader/SdkDispatchFailed (#175) (#220)

Browse Source 
* feat(hub): drop mode/sdkLoader/SdkDispatchFailed (#175)

The Command Routing Hub no longer carries dual-runtime selection.
Removes `mode` and `sdkLoader` constructor parameters and the
`SdkDispatchFailed` and `SdkLoadFailed` errorKind values. The Hub
now routes exclusively through the CJS registry / handler resolution
path. ERROR_KINDS enum shrinks from 6 to 4 values.

phase-command-router.cjs updated to construct the Hub without the
removed params (removes tryLoadSdk, getExecuteForCjs, sdkLoader fn,
mode variable, and the post-dispatch SDK output branch). The 7
remaining family routers (init, phases, roadmap, state, validate,
verify, cjs-command-router-adapter) do not use createHub directly
and require no changes.

The CJS↔SDK bridge (bin/lib/cjs-sdk-bridge.cjs) is unchanged and
remains separately invokable; its removal is tracked in Phase 4 (#190).

ADR-0012 is no longer amended in this PR — the decision is captured
in ADR-0174 (which supersedes ADR-0012 entirely as part of the
SDK-retirement migration). Amending a superseded ADR would be
redundant noise.

Tests:
- Added assertions that Hub rejects/ignores `mode` and `sdkLoader`
- Removed obsolete mode-selection branching tests
- 57/57 local tests pass

Closes #175.
Part of #174 (ADR-0174).

* chore(changeset): add docs-exempt marker (#175)

P1.1's CommandRoutingHub work has no docs/ touchpoints — the
architectural decision is captured in ADR-0174 (merged via PR #198).
Per-phase ADR amendments would create noise; the SDK-retirement
migration's docs land in Phase 6 PRs (#193-#196) once the relevant
state is removed.

Adds the standard <!-- docs-exempt: <reason> --> marker inside the
changeset fragment so lint:docs accepts the PR without forcing a
docs/ touch that would be redundant.

Refs #175. Part of #174 (ADR-0174).
This commit is contained in:
Tom Boucher
2026-05-24 13:04:11 -04:00
committed by GitHub
parent 6fc46db49a
commit 21e3ce53c0
4 changed files with 156 additions and 398 deletions
Download Patch File Download Diff File Expand all files Collapse all files

387
tests/command-routing-hub.test.cjs
View File

@@ -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);
});
});