refactor(#1644): Hub extension — exitReason? field on InvalidArgs + adapter honestification (#1645)

Phase 1 of parent #1641. Implements the contract documented in the Phase 0 ADR-0174 §5 amendment (#1642 / #1643). src/command-routing-hub.cts * InvalidArgsResult interface gains optional exitReason?: string (carries an ERROR_REASON enum value, separate from reason which is the explanation text). * makeInvalidArgs(arg, reason, exitReason?) factory conditionally adds the field only when the third arg is truthy — preserves the strict- keys invariant tested at command-routing-hub.test.cjs:444. * _VARIANT_SCHEMA.InvalidArgs.allowed Set extended to include 'exitReason' so the runtime validator does not coerce well-formed extended Results to HandlerFailure. src/cjs-command-router-adapter.cts * Honestified the wrapper comment: the runtime check ('ok' in result) already passes any {ok:*} object through, so the historical {ok:true, data} return type was a lie for err Results. The lying cast is preserved because the Hub's export = syntax doesn't expose HubResult for import; the Hub's _validateErrResult runtime-validates the actual shape. * Result→error() translation branched: when InvalidArgs carries exitReason, the adapter calls error(result.reason, result.exitReason) so the JSON-error envelope (GSD_JSON_ERRORS=1) preserves the typed ERROR_REASON value. When exitReason is absent, error(msg) is called with exactly one arg — byte-identical with prior behavior. * RouteCjsCommandFamilyOptions.error and RouteHubCommandFamilyOptions .error callback types widened from (message) to (message, reason?) to match io.cts's actual error() signature. CONTEXT.md * Command Routing Hub predicate updated to document the new field, factory signature, and dispatcher translation contract. Tests (TDD red→green) * tests/command-routing-hub.test.cjs: 8 new tests covering 2-arg (strict-keys), 3-arg (key present), undefined, empty string, frozen result, hub.dispatch propagation, and validator acceptance. * tests/cjs-command-router-adapter.test.cjs: 2 new tests covering exitReason passed as second arg + byte-identical prior behavior when absent. Verification * npm run test:unit: 2448 tests, 0 fail (no regressions) * gsd-test-summary on docker: outcome=passed, 0 failures (RULESET.PR-FLOW.docker-before-push) Memtrace blast radius: LOW (get_impact makeInvalidArgs → 3 nodes; the optional field is non-breaking for the 1 existing caller routePhaseCommand).
2026-06-23 22:47:57 -04:00
parent bcc5a6d1ba
commit 6214039358
5 changed files with 219 additions and 10 deletions
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -65,7 +65,7 @@ Module owning command resolution, policy projection (`mutation`, `output_mode`),
 Module owning the `init.*` family of query handlers that compose atomic queries into the flat JSON bundles consumed by init workflows (`/gsd-execute-phase`, `/gsd-plan-phase`, `/gsd-verify-work`, `/gsd-new-project`, `/gsd-manager`, `/gsd-progress`, `/gsd-resume`, etc.). Source of truth: `gsd-core/bin/lib/init.cjs` — the basic handlers (plus `withProjectRoot` project-identity injection) and the 3 heavyweight handlers (`initNewProject`, `initProgress`, `initManager`). All handlers return `{ data: <flat JSON> }`. Test seams: `tests/init.test.cjs` and `tests/init-manager.test.cjs` (cover withProjectRoot precedence, progress/manager precedence regression #2674, workstream scoping regression #3196, and cross-milestone dependency regression #2267). (The SDK `handlers/init/*.ts` sources and the `init*.test.ts` seams were retired with the SDK package per ADR-0174.)

 ### Command Routing Hub
-Single dispatch seam (`gsd-core/bin/lib/command-routing-hub.cjs`) that centralizes CJS routing, the no-throw pure-result contract, typed error variants, and dispatch-event emission for all command family adapters. Interface: `createHub({ cjsRegistry, manifest, logger }) → hub`; `hub.dispatch({ family, subcommand, args, cwd, raw, parentTraceId? }) → Result` where `Result = { ok: true, data } | { ok: false, kind, ...typedPayload }` and `kind ∈ { UnknownCommand, InvalidArgs, HandlerRefusal, HandlerFailure }`. The Hub is single-runtime (no mode selection, no sdkLoader), never prints, never exits, never throws. Adapters call `createHub`, dispatch, then translate the pure Result to `output()`/`error()` calls. Source: `gsd-core/bin/lib/command-routing-hub.cjs`; ADR: `docs/adr/0174-retire-gsd-sdk-package-boundary.md`.
+Single dispatch seam (`gsd-core/bin/lib/command-routing-hub.cjs`) that centralizes CJS routing, the no-throw pure-result contract, typed error variants, and dispatch-event emission for all command family adapters. Interface: `createHub({ cjsRegistry, manifest, logger }) → hub`; `hub.dispatch({ family, subcommand, args, cwd, raw, parentTraceId? }) → Result` where `Result = { ok: true, data } | { ok: false, kind, ...typedPayload }` and `kind ∈ { UnknownCommand, InvalidArgs, HandlerRefusal, HandlerFailure }`. The `InvalidArgs` variant carries an optional `exitReason?: string` field (amendment #1642 / #1644 Phase 1) holding the `ERROR_REASON` enum value, separate from `reason` (the explanation text); the `makeInvalidArgs(arg, reason, exitReason?)` factory omits the field when the third arg is absent, undefined, or empty — preserving the strict-keys invariant tested at `tests/command-routing-hub.test.cjs:444`. The Hub is single-runtime (no mode selection, no sdkLoader), never prints, never exits, never throws. Adapters call `createHub`, dispatch, then translate the pure Result to `output()`/`error()` calls; when an `InvalidArgs` Result carries `exitReason`, the adapter passes it as the second arg to `error(message, exitReason)` so the JSON-error envelope (`GSD_JSON_ERRORS=1`) preserves the typed reason. Source: `gsd-core/bin/lib/command-routing-hub.cjs`; ADR: `docs/adr/0174-retire-gsd-sdk-package-boundary.md` (§5 amended #1642).

 ### Runtime Source Layout Module
 Single-runtime seam layout for this repository after SDK retirement. Runtime execution paths live under `gsd-core/bin/lib/` and are grouped by seam concern (dispatch, manifest, handlers, runtime, observability, installer). ADR-0174 preserves the seam vocabulary and defines the canonical long-term shape as a seam-aligned TypeScript `src/` tree (`src/dispatch/`, `src/handlers/`, `src/errors/`, `src/manifest/`, `src/config/`, `src/state/`, `src/workstream/`, `src/runtime/`, `src/cli/`, `src/observability/`) compiled to CJS.
--- a/src/cjs-command-router-adapter.cts
+++ b/src/cjs-command-router-adapter.cts
@@ -25,7 +25,10 @@ interface RouteCjsCommandFamilyOptions {
  defaultSubcommand?: string;
  unsupported?: Record<string, string>;
  unknownMessage: (subcommand: string, available: string[]) => string;
-  error: (message: string) => void;
+  // Amendment #1642 (#1644 Phase 1): widened to accept optional ERROR_REASON
+  // enum value as second arg. io.cts's error() already accepts (msg, reason?);
+  // the prior one-arg signature was narrower than the runtime contract.
+  error: (message: string, reason?: string) => void;
  cwd?: string;
  raw?: boolean;
 }
@@ -38,7 +41,7 @@ interface RouteHubCommandFamilyOptions {
  defaultSubcommand?: string;
  unsupported?: Record<string, string>;
  unknownMessage: (subcommand: string, available: string[]) => string;
-  error: (message: string) => void;
+  error: (message: string, reason?: string) => void;
  cwd?: string;
  raw?: boolean;
 }
@@ -100,6 +103,18 @@ function routeHubCommandFamily({
  const registryHandlers = Object.fromEntries(
    Object.entries(handlers).map(([name, handler]) => [
      name,
+      // Honestified via amendment #1642 (#1644 Phase 1): the runtime check
+      // `'ok' in result` already passes any `{ok:*}` object through, so the
+      // historical `{ok:true, data}` return type was a lie whenever the
+      // handler returned an err Result. The lying cast below is preserved
+      // because the Hub's `export =` syntax doesn't expose `HubResult` for
+      // import; the Hub's `_validateErrResult` runtime-validates the actual
+      // shape, so structural compatibility is sufficient. The wrapper's
+      // 0-arg signature is assignable to the Hub's `(ctx) => HubResult`
+      // Handler type via TypeScript parameter bivariance; the Hub's per-call
+      // ctx is intentionally ignored (host-router handlers don't use it;
+      // capability-router handlers in Phase 2 will return HubResults that
+      // already carry context).
      (): { ok: true; data: unknown } => {
        const result = handler();
        if (result && typeof result === 'object' && Object.prototype.hasOwnProperty.call(result, 'ok')) {
@@ -128,7 +143,21 @@ function routeHubCommandFamily({
    error(unknownMessage(subcommand ?? '', available));
    return;
  }
-  if (result.kind === ERROR_KINDS.InvalidArgs || result.kind === ERROR_KINDS.HandlerRefusal) {
+  if (result.kind === ERROR_KINDS.InvalidArgs) {
+    // Amendment #1642 (#1644): when the handler provided exitReason, pass it
+    // as the second arg to error() so the JSON-error envelope
+    // (GSD_JSON_ERRORS=1) preserves the typed ERROR_REASON value for
+    // downstream consumers. When exitReason is absent, call error(msg) with
+    // exactly one arg — byte-identical with prior behavior.
+    const invalidArgs = result as { reason: string; exitReason?: string };
+    if (invalidArgs.exitReason) {
+      error(invalidArgs.reason, invalidArgs.exitReason);
+    } else {
+      error(invalidArgs.reason);
+    }
+    return;
+  }
+  if (result.kind === ERROR_KINDS.HandlerRefusal) {
    error((result as { reason: string }).reason);
    return;
  }
--- a/src/command-routing-hub.cts
+++ b/src/command-routing-hub.cts
@@ -76,6 +76,14 @@ interface InvalidArgsResult {
  kind: 'InvalidArgs';
  arg: string;
  reason: string;
+  // Optional ERROR_REASON enum value (e.g. 'USAGE'), carried separately from
+  // `reason` (the human-readable explanation). Added by amendment #1642 so
+  // routers migrating from direct `error(msg, ERROR_REASON.USAGE)` calls to
+  // `makeInvalidArgs(...)` Results can preserve ERROR_REASON granularity
+  // through the Hub Result → `error(msg, exitReason)` translation. Omitted by
+  // the factory when the third arg is absent, undefined, or empty string —
+  // preserves the strict-keys invariant tested at command-routing-hub.test.cjs:444.
+  exitReason?: string;
 }

 interface HandlerRefusalResult {
@@ -117,8 +125,14 @@ function makeUnknownCommand(command: string): Readonly<UnknownCommandResult> {
  return Object.freeze({ ok: false as const, kind: ERROR_KINDS.UnknownCommand, command });
 }

-function makeInvalidArgs(arg: string, reason: string): Readonly<InvalidArgsResult> {
-  return Object.freeze({ ok: false as const, kind: ERROR_KINDS.InvalidArgs, arg, reason });
+function makeInvalidArgs(arg: string, reason: string, exitReason?: string): Readonly<InvalidArgsResult> {
+  const obj: InvalidArgsResult = { ok: false as const, kind: ERROR_KINDS.InvalidArgs, arg, reason };
+  // Conditionally add exitReason only when truthy — preserves strict-keys
+  // invariant (2-arg callers must continue to produce a 4-key frozen result).
+  if (exitReason) {
+    obj.exitReason = exitReason;
+  }
+  return Object.freeze(obj);
 }

 function makeHandlerRefusal(reason: string): Readonly<HandlerRefusalResult> {
@@ -163,10 +177,11 @@ const _VARIANT_SCHEMA: Record<string, { required: string[]; allowed: Set<string>
    required: ['command'],
    allowed: new Set(['ok', 'kind', 'command']),
  },
-  InvalidArgs: {
-    required: ['arg', 'reason'],
-    allowed: new Set(['ok', 'kind', 'arg', 'reason']),
-  },
+ InvalidArgs: {
+  required: ['arg', 'reason'],
+  // Amendment #1642: exitReason? is allowed but not required.
+  allowed: new Set(['ok', 'kind', 'arg', 'reason', 'exitReason']),
+ },
  HandlerRefusal: {
    required: ['reason'],
    allowed: new Set(['ok', 'kind', 'reason']),
--- a/tests/cjs-command-router-adapter.test.cjs
+++ b/tests/cjs-command-router-adapter.test.cjs
@@ -93,6 +93,58 @@ describe('cjs-command-router-adapter routeHubCommandFamily', () => {
    assert.equal(errorMessage, '--phase must be an integer');
  });

+  test('projects InvalidArgs exitReason as second error() arg when present (#1644)', () => {
+    let capturedMessage = null;
+    let capturedExitReason = null;
+    let callCount = 0;
+
+    routeHubCommandFamily({
+      family: 'unit',
+      args: ['unit', 'invalid'],
+      subcommands: ['invalid'],
+      handlers: {
+        invalid: () => makeInvalidArgs('--phase', '--phase must be an integer', 'USAGE'),
+      },
+      unknownMessage: () => 'should not be used',
+      error: (message, exitReason) => {
+        callCount += 1;
+        capturedMessage = message;
+        capturedExitReason = exitReason;
+      },
+      cwd: '/tmp/proj',
+      raw: false,
+    });
+
+    assert.equal(callCount, 1);
+    assert.equal(capturedMessage, '--phase must be an integer',
+      `error() message must be the InvalidArgs.reason; got: ${JSON.stringify(capturedMessage)}`);
+    assert.equal(capturedExitReason, 'USAGE',
+      `error() exitReason must be passed as second arg; got: ${JSON.stringify(capturedExitReason)}`);
+  });
+
+  test('omits second error() arg when InvalidArgs has no exitReason (byte-identical with prior behavior)', () => {
+    let capturedArgs = null;
+
+    routeHubCommandFamily({
+      family: 'unit',
+      args: ['unit', 'invalid'],
+      subcommands: ['invalid'],
+      handlers: {
+        invalid: () => makeInvalidArgs('--phase', '--phase must be an integer'),
+      },
+      unknownMessage: () => 'should not be used',
+      error: (...args) => {
+        capturedArgs = args;
+      },
+      cwd: '/tmp/proj',
+      raw: false,
+    });
+
+    assert.equal(capturedArgs.length, 1,
+      `error() must be called with EXACTLY one arg when exitReason absent (preserve byte-identical prior behavior); got ${capturedArgs.length} args`);
+    assert.equal(capturedArgs[0], '--phase must be an integer');
+  });
+
  test('projects thrown handler exceptions as HandlerFailure message', () => {
    let errorMessage = null;

--- a/tests/command-routing-hub.test.cjs
+++ b/tests/command-routing-hub.test.cjs
@@ -836,3 +836,116 @@ describe('CommandRoutingHub — Finding 4: makeHandlerFailure wraps non-Error ca
    assert.equal(result.cause, undefined);
  });
 });
+
+// ─── Amendment #1642: exitReason? field on InvalidArgs (Phase 1, #1644) ───────
+// The optional exitReason? field carries an ERROR_REASON enum value separately
+// from the existing `reason` explanation text. The factory conditionally adds
+// the field only when a truthy third arg is provided, preserving the strict-keys
+// invariant tested above (L444).
+
+describe('CommandRoutingHub — exitReason? field on InvalidArgs (#1644 / amendment #1642)', () => {
+  test('makeInvalidArgs(arg, reason) 2-arg form omits exitReason key (strict-keys invariant preserved)', () => {
+    const result = makeInvalidArgs('--phase', '--phase must be an integer');
+    const keys = Object.keys(result).sort();
+    assert.deepStrictEqual(keys, ['arg', 'kind', 'ok', 'reason'],
+      `2-arg form must NOT include exitReason key; got: ${JSON.stringify(keys)}`);
+    assert.equal(result.exitReason, undefined);
+  });
+
+  test('makeInvalidArgs(arg, reason, exitReason) 3-arg form includes exitReason key with the value', () => {
+    const result = makeInvalidArgs('--phase', '--phase must be an integer', 'USAGE');
+    const keys = Object.keys(result).sort();
+    assert.deepStrictEqual(keys, ['arg', 'exitReason', 'kind', 'ok', 'reason'],
+      `3-arg form must include exitReason key; got: ${JSON.stringify(keys)}`);
+    assert.equal(result.exitReason, 'USAGE');
+  });
+
+  test('makeInvalidArgs(arg, reason, undefined) treats undefined as absent (omits key)', () => {
+    const result = makeInvalidArgs('--phase', '--phase must be an integer', undefined);
+    const keys = Object.keys(result).sort();
+    assert.deepStrictEqual(keys, ['arg', 'kind', 'ok', 'reason'],
+      `undefined exitReason must be omitted; got: ${JSON.stringify(keys)}`);
+  });
+
+  test('makeInvalidArgs(arg, reason, "") treats empty string as absent (omits key)', () => {
+    const result = makeInvalidArgs('--phase', '--phase must be an integer', '');
+    const keys = Object.keys(result).sort();
+    assert.deepStrictEqual(keys, ['arg', 'kind', 'ok', 'reason'],
+      `empty-string exitReason must be omitted; got: ${JSON.stringify(keys)}`);
+  });
+
+  test('3-arg factory result is still frozen', () => {
+    const result = makeInvalidArgs('--phase', 'required', 'USAGE');
+    assert.ok(Object.isFrozen(result), '3-arg factory result must be frozen');
+  });
+
+  test('hub.dispatch propagates handler-returned InvalidArgs with exitReason unchanged', () => {
+    const hub = createHub({
+      cjsRegistry: {
+        unit: {
+          check: (_ctx) => ({
+            ok: false,
+            kind: ERROR_KINDS.InvalidArgs,
+            arg: '--flag',
+            reason: 'not supported',
+            exitReason: 'USAGE',
+          }),
+        },
+      },
+    });
+
+    const result = hub.dispatch({ family: 'unit', subcommand: 'check', args: [], cwd: '/', raw: false });
+
+    assert.ok(!result.ok);
+    assert.equal(result.kind, ERROR_KINDS.InvalidArgs);
+    assert.equal(result.arg, '--flag');
+    assert.equal(result.reason, 'not supported');
+    assert.equal(result.exitReason, 'USAGE',
+      `Hub must propagate exitReason from handler-returned InvalidArgs; got: ${JSON.stringify(result)}`);
+  });
+
+  test('hub.dispatch still accepts InvalidArgs WITHOUT exitReason (no contract regression)', () => {
+    const hub = createHub({
+      cjsRegistry: {
+        unit: {
+          check: (_ctx) => ({
+            ok: false,
+            kind: ERROR_KINDS.InvalidArgs,
+            arg: '--flag',
+            reason: 'not supported',
+          }),
+        },
+      },
+    });
+
+    const result = hub.dispatch({ family: 'unit', subcommand: 'check', args: [], cwd: '/', raw: false });
+
+    assert.ok(!result.ok);
+    assert.equal(result.kind, ERROR_KINDS.InvalidArgs);
+    assert.equal(result.exitReason, undefined,
+      `Hub must not synthesize exitReason when handler omits it; got: ${JSON.stringify(result)}`);
+  });
+
+  test('hub validator does NOT reject InvalidArgs with exitReason (well-formed extension)', () => {
+    // The runtime validator (_validateErrResult) coerces MALFORMED returns to HandlerFailure.
+    // A well-formed InvalidArgs with the new exitReason field must NOT be coerced.
+    const hub = createHub({
+      cjsRegistry: {
+        unit: {
+          check: (_ctx) => ({
+            ok: false,
+            kind: ERROR_KINDS.InvalidArgs,
+            arg: '--flag',
+            reason: 'required',
+            exitReason: 'USAGE',
+          }),
+        },
+      },
+    });
+
+    const result = hub.dispatch({ family: 'unit', subcommand: 'check', args: [], cwd: '/', raw: false });
+
+    assert.equal(result.kind, ERROR_KINDS.InvalidArgs,
+      `Extended InvalidArgs must not be coerced to HandlerFailure; got kind: ${result.kind}`);
+  });
+});