From b533f71857ab96d1ccec3286294d669c60b0d0fc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 21 May 2026 23:32:10 -0400 Subject: [PATCH] chore: introduce CommandRoutingHub and migrate phase-command-router (PoC) (#3828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * fix(docs): rename ADR to sequential convention 0012 (#3788) Co-Authored-By: Claude Sonnet 4.6 * 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 * 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 * 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 * 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 * 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 --- .changeset/mellow-tigers-gather.md | 5 + CONTEXT.md | 3 + docs/ARCHITECTURE.md | 4 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- docs/adr/0012-command-routing-hub.md | 51 ++ docs/adr/README.md | 1 + get-shit-done/bin/lib/command-routing-hub.cjs | 239 ++++++++ .../bin/lib/phase-command-router.cjs | 325 ++++++----- scripts/lint-test-file-count.allowlist.json | 2 +- tests/command-routing-hub.test.cjs | 548 ++++++++++++++++++ tests/phase-command-router.test.cjs | 420 ++++++++++++++ 12 files changed, 1457 insertions(+), 145 deletions(-) create mode 100644 .changeset/mellow-tigers-gather.md create mode 100644 docs/adr/0012-command-routing-hub.md create mode 100644 get-shit-done/bin/lib/command-routing-hub.cjs create mode 100644 tests/command-routing-hub.test.cjs create mode 100644 tests/phase-command-router.test.cjs diff --git a/.changeset/mellow-tigers-gather.md b/.changeset/mellow-tigers-gather.md new file mode 100644 index 000000000..f7ccdf6d0 --- /dev/null +++ b/.changeset/mellow-tigers-gather.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3828 +--- +Consolidate command dispatch behind CommandRoutingHub (issue #3788) diff --git a/CONTEXT.md b/CONTEXT.md index 8931d6ae5..b742da329 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -64,6 +64,9 @@ SDK Module owning the `init.*` family of query handlers that compose atomic SDK ### CJS Command Router Adapter Module Compatibility Adapter Module for `gsd-tools.cjs` command families. Uses generated command metadata plus small argument shapers to route to CJS handlers, rather than calling SDK Command Topology directly. Preserves CJS compatibility startup while reducing hand-written router drift. Per-family migration to call the **Sync Runtime Bridge Module**'s `executeForCjs` in-process — eliminating the remaining parallel CJS handler implementations — is the active work of #3524 Phase 5; the primitive itself ships in #3555, with each canonical command family (`state.*`, `verify.*`, `init.*`, `phase.*`, `phases.*`, `validate.*`, `roadmap.*`, `frontmatter.*`, `config.*`) routing through `executeForCjs` in its own follow-up enhancement. +### Command Routing Hub +Single dispatch seam (`get-shit-done/bin/lib/command-routing-hub.cjs`) that centralizes mode selection (sdk vs cjs), the no-throw pure-result contract, and the closed 6-value `errorKind` enum for all CJS command family router adapters. Interface: `createHub({ mode, sdkLoader, cjsRegistry, manifest }) → hub`; `hub.dispatch({ family, subcommand, args, cwd, raw }) → Result` where `Result = { ok: true, data } | { ok: false, errorKind, message, details? }` and `errorKind ∈ { UnknownCommand, InvalidArgs, HandlerRefusal, HandlerFailure, SdkLoadFailed, SdkDispatchFailed }`. Mode is fixed at construction; hub never prints, never exits, never throws. No transparent fallback: SDK crash → `SdkDispatchFailed`, not CJS retry. Adapters call `createHub`, dispatch, then translate the pure Result to `output()`/`error()` calls. Proof-of-concept migration: `phase-command-router.cjs` (#3788). ADR: `docs/adr/0012-command-routing-hub.md`. + ### Query Pre-Project Config Policy Module Module policy that defines query-time behavior when `.planning/config.json` is absent: use built-in defaults for parity-sensitive query Interfaces, and emit parity-aligned empty model ids for pre-project model resolution surfaces. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0ff012a27..873a0dd9b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -277,6 +277,10 @@ Programmatic SDK callers (`GSDTools`) route through one seam that owns query dis This keeps callers thin adapters and centralizes transport decisions for SDK publishability. +### Command Routing Hub (`get-shit-done/bin/lib/command-routing-hub.cjs`) + +CJS command family routers migrate to dispatch through `CommandRoutingHub` incrementally. `phase-command-router.cjs` is the first migration (issue #3788); remaining routers (`phases-command-router.cjs`, `roadmap-command-router.cjs`, etc.) continue using `routeCjsCommandFamily` until migrated in follow-up issues. The hub owns three cross-cutting concerns that each router previously duplicated: (1) mode selection (`sdk` when `tryLoadSdk()` succeeds and no `GSD_WORKSTREAM` is active, `cjs` otherwise), set once at construction; (2) a no-throw pure-result contract (`hub.dispatch()` catches all exceptions and returns `{ ok: false, errorKind, message, details }` instead of propagating); and (3) a closed six-value `errorKind` enum exported as the frozen `ERROR_KINDS` object. Router adapters remain thin CLI translators — they build the hub, call `dispatch`, then map the Result to `output()`/`error()` calls. No transparent SDK→CJS fallback: an SDK-mode hub that encounters a load or dispatch failure returns `SdkLoadFailed` or `SdkDispatchFailed` without retrying via CJS. See `docs/adr/0012-command-routing-hub.md`. + ### CLI Tools (`get-shit-done/bin/`) Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `get-shit-done/bin/lib/` (see [`docs/INVENTORY.md`](INVENTORY.md#cli-modules-33-shipped) for the authoritative roster): diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 234a36aca..201a7facf 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -268,6 +268,7 @@ "clusters.cjs", "code-review-flags.cjs", "command-aliases.generated.cjs", + "command-routing-hub.cjs", "commands.cjs", "config-schema.cjs", "config.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 03aa29584..8811e1e45 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -361,7 +361,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (73 shipped) +## CLI Modules (74 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -376,6 +376,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `clusters.cjs` | Skill cluster definitions for the runtime surface module (ADR-0011 Phase 2) | | `code-review-flags.cjs` | Typed flag parser for `/gsd:code-review`; exports `parseCodeReviewFlags(argv)` (→ `{ fix, all, auto, depth, files }`) and `resolveCodeReviewWorkflow(flags)` (→ `'code-review.md' \| 'code-review-fix.md'`); canonical dispatch seam for `--fix`/`--all`/`--auto` routing | | `command-aliases.generated.cjs` | Generated CJS alias/subcommand metadata for manifest-backed family routers | +| `command-routing-hub.cjs` | Pure-result dispatch hub that centralizes mode decision (SDK vs CJS), error taxonomy, and no-throw contract for all command-family routers (#3788) | | `commands.cjs` | Misc CLI commands (slug, timestamp, todos, scaffolding, stats) | | `config-schema.cjs` | Single source of truth for `VALID_CONFIG_KEYS` and dynamic key patterns; imported by both the validator and the config-schema-docs parity test | | `config.cjs` | `config.json` read/write, section initialization; imports validator from `config-schema.cjs` | diff --git a/docs/adr/0012-command-routing-hub.md b/docs/adr/0012-command-routing-hub.md new file mode 100644 index 000000000..bf284714a --- /dev/null +++ b/docs/adr/0012-command-routing-hub.md @@ -0,0 +1,51 @@ +# CommandRoutingHub as single dispatch seam for CJS command families + +- **Status:** Accepted +- **Date:** 2026-05-20 + +## Context + +Seven `*-command-router.cjs` files (`phase`, `phases`, `roadmap`, `state`, `verify`, `validate`, `init`) each duplicate the same three-part dispatch pattern: (1) check `GSD_WORKSTREAM` + `tryLoadSdk()` to decide whether to use the SDK or CJS handler, (2) invoke the selected path, (3) map errors to the `error()` callback. The duplicated mode-selection logic means a policy change (e.g., adding a new fallback condition) must be applied in eight places. Tests for these routers are mock-heavy — they stub `tryLoadSdk`, stub `getExecuteForCjs`, and assert on internal call shapes rather than observable dispatch outcomes. The SDK-vs-CJS fallback decision is smeared across every router, making it impossible to reason about or test the policy in isolation. + +## Decision + +Introduce `CommandRoutingHub` (`get-shit-done/bin/lib/command-routing-hub.cjs`) as the single dispatch seam for all CJS command family routers. The hub contract: + +``` +createHub({ mode: 'sdk' | 'cjs', sdkLoader, cjsRegistry, manifest }) -> hub +hub.dispatch({ family, subcommand, args, cwd, raw }) -> Result + +Result = { ok: true, data } + | { ok: false, errorKind, message, details? } +``` + +Load-bearing design properties: + +- **Pure result**: the hub never prints to stdout/stderr, never calls `process.exit`, and never throws. All internal throws are caught and converted to `{ ok: false, errorKind: 'HandlerFailure' }`. +- **Mode fixed at construction**: `mode` is set once when `createHub` is called; it is never re-evaluated per dispatch call. Each adapter (caller) computes mode based on its own env/sdk-load context before constructing the hub. +- **No transparent fallback**: an SDK-mode hub that encounters an SDK crash or load failure returns `{ ok: false, errorKind: 'SdkDispatchFailed' }` or `'SdkLoadFailed'` respectively. It does not silently retry via the CJS registry. +- **Closed `errorKind` enum**: the six error kinds (`UnknownCommand`, `InvalidArgs`, `HandlerRefusal`, `HandlerFailure`, `SdkLoadFailed`, `SdkDispatchFailed`) are exported as a frozen `ERROR_KINDS` object. Callers switch on `ERROR_KINDS` values, not bare string literals. Adding a new error kind requires amending this ADR. + +The router adapter's responsibilities shrink to: determine mode from env, build stubs/registry, construct hub, dispatch, translate the pure Result to `output()`/`error()` calls. Each adapter remains a thin CLI-facing translation layer. + +`phase-command-router.cjs` is migrated as the proof-of-concept for this PR. Remaining routers migrate in follow-up issues. + +## Consequences + +- **Positive**: policy (mode decision, no-throw contract, error taxonomy) is concentrated in one module rather than duplicated across eight. Testing the policy requires only the hub unit tests; adapter tests verify translation correctness (args → dispatch, Result → output/error). +- **Positive**: future routers can be onboarded by wiring `cjsRegistry` entries rather than hand-replicating the SDK/CJS conditional block. +- **Constraint**: adding a new `errorKind` value requires updating `ERROR_KINDS` in `command-routing-hub.cjs` AND amending this ADR. The closed enum is the drift-prevention property; the amendment requirement makes scope of impact explicit. +- **Constraint**: each adapter must compute mode before hub construction (no lazy re-evaluation). This is intentional — mode ambiguity at dispatch time is a prior source of subtle test flakiness. + +## Known limitation: SDK-incomplete subcommands + +The hub's mode is fixed at construction (`'sdk'` or `'cjs'`). This works cleanly only when every subcommand in a family has an implementation in the active mode. Today some phase subcommands have divergent CJS and SDK implementations. `phase.mvp-mode` is present in the SDK catalog (`command-static-catalog-domain.ts`) but its CJS-native implementation (`phase.cmdPhaseMvpMode`) differs in ROADMAP scan behaviour and error reason codes from the SDK query layer. Routing `mvp-mode` through the SDK hub would silently change observable CLI behaviour (exit codes, JSON error shape). + +The proof-of-concept adapter (`phase-command-router.cjs`) handles this with an early-return bypass: `mvp-mode` is intercepted before the dispatch call so it never reaches the hub. This preserves observable behavior but introduces a hub-level abstraction leak — the adapter now carries per-subcommand routing policy that the hub was meant to own. + +Future direction (deferred): the hub should consult `manifest` to detect per-subcommand SDK coverage and route to CJS automatically for subcommands not present in the SDK manifest. That refinement stays inside the global-mode decision — the mode still applies to the family as a whole — and avoids the per-command policy ladder that was explicitly rejected during design. This work is tracked alongside SDK-CJS migration #3524 closure. + +## References + +- Extends ADR-0001 (Dispatch Policy Module) — the hub implements the no-throw + structured-result contract ADR-0001 established for the SDK query layer, applying it to the CJS adapter layer. +- Issue: [#3788](https://github.com/gsd-build/get-shit-done/issues/3788) diff --git a/docs/adr/README.md b/docs/adr/README.md index 1ef8c2be7..67d605ad4 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -44,6 +44,7 @@ See **[CONTRIBUTING.md — "Proposing an ADR or PRD"](../../CONTRIBUTING.md#prop | [0011-skill-surface-budget-module.md](0011-skill-surface-budget-module.md) | Skill Surface Budget Module owns install-time profile staging and runtime surface control | Accepted | | [0011-review-default-reviewers.md](0011-review-default-reviewers.md) | Review default-reviewers selection policy for /gsd:review | Accepted | | [0011-review-default-reviewers-prd.md](0011-review-default-reviewers-prd.md) | PRD for review.default_reviewers feature (#3464) | Reference | +| [0012-command-routing-hub.md](0012-command-routing-hub.md) | CommandRoutingHub as single dispatch seam for CJS command families | Accepted | | [3524-cjs-sdk-hard-seam.md](3524-cjs-sdk-hard-seam.md) | CJS↔SDK hard seam — single canonical owner per responsibility (#3524) | Proposed | | [3660-runtime-artifact-layout-module.md](3660-runtime-artifact-layout-module.md) | Runtime Artifact Layout Module owns per-runtime artifact placement | Proposed | diff --git a/get-shit-done/bin/lib/command-routing-hub.cjs b/get-shit-done/bin/lib/command-routing-hub.cjs new file mode 100644 index 000000000..0cdab02c3 --- /dev/null +++ b/get-shit-done/bin/lib/command-routing-hub.cjs @@ -0,0 +1,239 @@ +'use strict'; + +/** + * Command Routing Hub — issue #3788. + * + * A pure-result dispatch hub that centralizes the mode decision (SDK vs CJS), + * the error taxonomy, and the no-throw contract that all command-family routers + * currently duplicate independently. + * + * Design: + * createHub({ mode, sdkLoader, 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 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. + */ + +/** + * Closed errorKind enum. Export as a frozen object so callers can switch on + * ERROR_KINDS.UnknownCommand etc. without relying on bare string literals. + * + * @readonly + */ +const ERROR_KINDS = Object.freeze({ + /** The requested family/subcommand combination is not present in the manifest. */ + UnknownCommand: 'UnknownCommand', + /** The handler rejected the supplied arguments before executing. */ + InvalidArgs: 'InvalidArgs', + /** A CJS handler returned an explicit refusal (e.g. unsupported subcommand). */ + 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', +}); + +/** + * @typedef {{ ok: true, data: unknown }} OkResult + * @typedef {{ ok: false, errorKind: string, message: string, details?: unknown }} ErrResult + * @typedef {OkResult | ErrResult} HubResult + */ + +/** + * @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'. + * @property {Record} [manifest] - Map of family -> known subcommands. + * Used for UnknownCommand detection regardless of mode. + */ + +/** + * Construct a CommandRoutingHub. + * + * @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; + const _cjsRegistry = cjsRegistry; + const _manifest = manifest; + + /** + * Dispatch a command through the hub. + * + * @param {{ family: string, subcommand: string, args?: unknown[], cwd?: string, raw?: boolean }} req + * @returns {HubResult} + */ + function dispatch(req) { + try { + return _dispatch(req); + } catch (err) { + return { + ok: false, + errorKind: ERROR_KINDS.HandlerFailure, + message: err instanceof Error ? err.message : String(err), + details: { originalError: err }, + }; + } + } + + function _dispatch(req) { + const { family, subcommand, args = [], cwd, raw } = req; + + // ── manifest check (applies to both modes) ────────────────────────────── + if (_manifest) { + const knownSubcommands = _manifest[family]; + if (!knownSubcommands) { + return { + ok: false, + errorKind: ERROR_KINDS.UnknownCommand, + message: `Unknown command family: ${family}`, + }; + } + if (subcommand && !knownSubcommands.includes(subcommand)) { + return { + ok: false, + errorKind: ERROR_KINDS.UnknownCommand, + message: `Unknown subcommand: ${family} ${subcommand}`, + }; + } + } + + 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 { + ok: false, + errorKind: ERROR_KINDS.UnknownCommand, + message: `No CJS registry provided for family: ${family}`, + }; + } + + const familyHandlers = _cjsRegistry[family]; + if (!familyHandlers) { + return { + ok: false, + errorKind: ERROR_KINDS.UnknownCommand, + message: `Unknown command family: ${family}`, + }; + } + + const handler = subcommand ? familyHandlers[subcommand] : familyHandlers['']; + if (typeof handler !== 'function') { + return { + ok: false, + errorKind: ERROR_KINDS.UnknownCommand, + message: `Unknown subcommand: ${family} ${subcommand}`, + }; + } + + // Invoke the handler. It must return a HubResult or throw. + // If it throws, the outer try/catch in dispatch() catches it. + const result = handler({ family, subcommand, args, cwd, raw }); + + // If the handler returned a well-formed HubResult, pass it through. + if (result && typeof result === 'object' && 'ok' in result) { + return result; + } + + // If the handler returned nothing (undefined), treat as success with no data. + if (result === undefined || result === null) { + return { ok: true, data: null }; + } + + // Any other return value is treated as the data payload. + return { ok: true, data: result }; + } + + return { dispatch }; +} + +module.exports = { + createHub, + ERROR_KINDS, +}; diff --git a/get-shit-done/bin/lib/phase-command-router.cjs b/get-shit-done/bin/lib/phase-command-router.cjs index 1330cf4bd..ac972f523 100644 --- a/get-shit-done/bin/lib/phase-command-router.cjs +++ b/get-shit-done/bin/lib/phase-command-router.cjs @@ -1,12 +1,14 @@ 'use strict'; const { PHASE_SUBCOMMANDS } = require('./command-aliases.generated.cjs'); -const { routeCjsCommandFamily } = require('./cjs-command-router-adapter.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) ────────────────────────────────────────── +const { createHub, ERROR_KINDS } = require('./command-routing-hub.cjs'); + /** * Manifest-backed phase subcommand router. * Keeps gsd-tools.cjs thin while preserving existing command semantics. @@ -22,164 +24,201 @@ const { tryLoadSdk, getExecuteForCjs } = require('./cjs-sdk-bridge.cjs'); * - scaffold: routed through top-level scaffold command. * * CJS-only subcommands: none. + * + * #3788: dispatch is now 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(); - function sdkHandler(registryCommand, registryArgs, legacyArgs, cjsFallback) { - if (!sdkAvailable) return cjsFallback; - return () => { - // #3631: under --raw, request mode:'raw' so the bridge runs the SDK's - // raw projection (formatQueryRawOutput) and returns the scalar string - // CJS callers used to print. We then bypass output()'s JSON-stringify - // path by passing rawValue (the third positional). With mode:'json', - // output() emits the JSON IR as before. - const result = getExecuteForCjs()({ - registryCommand, - registryArgs, - legacyCommand: 'phase', - legacyArgs, - mode: raw ? 'raw' : 'json', - projectDir: cwd, - }); - if (!result.ok) { - error(result.errorDetails && result.errorDetails.message - ? result.errorDetails.message - : `phase ${registryCommand} failed (${result.errorKind})`); - return; - } - if (raw) { - output(null, true, typeof result.data === 'string' ? result.data : String(result.data ?? '')); - } else { - output(result.data); - } - }; + // ── Unsupported / SDK-only subcommands ───────────────────────────────────── + // Resolved before dispatch so the error message matches the pre-#3788 text. + const UNSUPPORTED = { + 'list-plans': 'phase list-plans is SDK-only. Use: gsd-sdk query phase.list-plans ...', + 'list-artifacts': 'phase list-artifacts is SDK-only. Use: gsd-sdk query phase.list-artifacts ...', + scaffold: 'phase scaffold is routed through the top-level scaffold command.', + }; + + const subcommand = args[1]; + + if (subcommand && UNSUPPORTED[subcommand]) { + error(UNSUPPORTED[subcommand]); + return; } - routeCjsCommandFamily({ - args, - subcommands: PHASE_SUBCOMMANDS, - unsupported: { - 'list-plans': 'phase list-plans is SDK-only. Use: gsd-sdk query phase.list-plans ...', - 'list-artifacts': 'phase list-artifacts is SDK-only. Use: gsd-sdk query phase.list-artifacts ...', - scaffold: 'phase scaffold is routed through the top-level scaffold command.', - }, - error, - unknownMessage: (_subcommand, available) => `Unknown phase subcommand. Available: ${available.join(', ')}`, - handlers: { - 'mvp-mode': () => phase.cmdPhaseMvpMode(cwd, args.slice(2), raw), - 'next-decimal': sdkHandler( - 'phase.next-decimal', - args.slice(2), - args.slice(1), - () => phase.cmdPhaseNextDecimal(cwd, args[2], raw), - ), - add: sdkHandler( - 'phase.add', - args.slice(2), - args.slice(1), - () => { - let customId = null; - const descArgs = []; - for (let i = 2; i < args.length; i++) { - const token = args[i]; - if (token === '--raw') { - continue; - } - if (token === '--id') { - const id = args[i + 1]; - if (!id || id.startsWith('--')) { - error('--id requires a value'); - return; - } - customId = id; - i++; - } else if (token.startsWith('--')) { - error(`phase add does not support ${token}`); - return; - } else { - descArgs.push(token); - } + // ── No subcommand → reject early with helpful error ──────────────────────── + // Pre-#3788 code resolved unknown subcommands via routeCjsCommandFamily which + // fell through to error() when no handler matched (including undefined). + // Post-#3788 the hub's manifest check is skipped for falsy subcommand, so we + // must guard here to preserve the deterministic "Available: ..." error message. + if (!subcommand) { + const available = PHASE_SUBCOMMANDS.filter(s => !UNSUPPORTED[s]).join(', '); + error(`Unknown phase subcommand. Available: ${available}`); + return; + } + + // ── CJS-only subcommands (always bypass SDK path) ────────────────────────── + // `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). + if (subcommand === 'mvp-mode') { + phase.cmdPhaseMvpMode(cwd, args.slice(2), raw); + return; + } + + // ── Build the CJS registry ────────────────────────────────────────────────── + // Each handler receives a ctx object from the hub and must return a HubResult. + const cjsRegistry = { + phase: { + 'next-decimal': (_ctx) => { + phase.cmdPhaseNextDecimal(cwd, args[2], raw); + return { ok: true, data: null }; + }, + add: (_ctx) => { + let customId = null; + const descArgs = []; + for (let i = 2; i < args.length; i++) { + const token = args[i]; + if (token === '--raw') { + continue; } - phase.cmdPhaseAdd(cwd, descArgs.join(' '), raw, customId); - }, - ), - 'add-batch': sdkHandler( - 'phase.add-batch', - args.slice(2), - args.slice(1), - () => { - const descFlagIdx = args.indexOf('--descriptions'); - let descriptions; - if (descFlagIdx !== -1) { - const rawDescriptions = args[descFlagIdx + 1]; - if (!rawDescriptions || rawDescriptions.startsWith('--')) { - error('--descriptions must be a JSON array'); - return; - } - try { - descriptions = JSON.parse(rawDescriptions); - } catch { - error('--descriptions must be a JSON array'); - return; - } - if (!Array.isArray(descriptions)) { - error('--descriptions must be a JSON array'); - return; + if (token === '--id') { + const id = args[i + 1]; + if (!id || id.startsWith('--')) { + return { ok: false, errorKind: 'InvalidArgs', message: '--id requires a value' }; } + customId = id; + i++; + } else if (token.startsWith('--')) { + return { ok: false, errorKind: 'InvalidArgs', message: `phase add does not support ${token}` }; } else { - descriptions = args.slice(2).filter(a => a !== '--raw'); + descArgs.push(token); } - phase.cmdPhaseAddBatch(cwd, descriptions, raw); - }, - ), - insert: sdkHandler( - 'phase.insert', - args.slice(2), - args.slice(1), - () => { - if (args.includes('--dry-run')) { - error('phase insert does not support --dry-run'); - return; + } + phase.cmdPhaseAdd(cwd, descArgs.join(' '), raw, customId); + return { ok: true, data: null }; + }, + 'add-batch': (_ctx) => { + const descFlagIdx = args.indexOf('--descriptions'); + let descriptions; + if (descFlagIdx !== -1) { + const rawDescriptions = args[descFlagIdx + 1]; + if (!rawDescriptions || rawDescriptions.startsWith('--')) { + return { ok: false, errorKind: 'InvalidArgs', message: '--descriptions must be a JSON array' }; } - phase.cmdPhaseInsert(cwd, args[2], args.slice(3).join(' '), raw); - }, - ), - remove: sdkHandler( - 'phase.remove', - args.slice(2), - args.slice(1), - () => { - const removeArgs = args.slice(2).filter(token => token !== '--raw'); - let forceFlag = false; - const positional = []; - for (const token of removeArgs) { - if (token === '--force') { - forceFlag = true; - continue; - } - if (token.startsWith('--')) { - error(`phase remove does not support ${token}`); - return; - } - positional.push(token); + try { + descriptions = JSON.parse(rawDescriptions); + } catch { + return { ok: false, errorKind: 'InvalidArgs', message: '--descriptions must be a JSON array' }; } - if (positional.length !== 1) { - error('phase remove accepts exactly one phase number'); - return; + if (!Array.isArray(descriptions)) { + return { ok: false, errorKind: 'InvalidArgs', message: '--descriptions must be a JSON array' }; } - phase.cmdPhaseRemove(cwd, positional[0], { force: forceFlag }, raw); - }, - ), - complete: sdkHandler( - 'phase.complete', - args.slice(2), - args.slice(1), - () => phase.cmdPhaseComplete(cwd, args[2], raw), - ), + } else { + descriptions = args.slice(2).filter(a => a !== '--raw'); + } + phase.cmdPhaseAddBatch(cwd, descriptions, raw); + return { ok: true, data: null }; + }, + insert: (_ctx) => { + if (args.includes('--dry-run')) { + return { ok: false, errorKind: 'InvalidArgs', message: 'phase insert does not support --dry-run' }; + } + phase.cmdPhaseInsert(cwd, args[2], args.slice(3).join(' '), raw); + return { ok: true, data: null }; + }, + remove: (_ctx) => { + const removeArgs = args.slice(2).filter(token => token !== '--raw'); + let forceFlag = false; + const positional = []; + for (const token of removeArgs) { + if (token === '--force') { + forceFlag = true; + continue; + } + if (token.startsWith('--')) { + return { ok: false, errorKind: 'InvalidArgs', message: `phase remove does not support ${token}` }; + } + positional.push(token); + } + if (positional.length !== 1) { + return { ok: false, errorKind: 'InvalidArgs', message: 'phase remove accepts exactly one phase number' }; + } + phase.cmdPhaseRemove(cwd, positional[0], { force: forceFlag }, raw); + return { ok: true, data: null }; + }, + complete: (_ctx) => { + phase.cmdPhaseComplete(cwd, args[2], raw); + return { ok: true, data: null }; + }, }, + }; + + // ── 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 + // 'mvp-mode' because it was absent from PHASE_SUBCOMMANDS in the original + // and was not shown in the "Available:" list there either. + // + // `manifestSubcommands` is the full routing set for the hub — it includes + // 'mvp-mode' (which the original code routed via a handler even without a + // manifest entry) so the hub's UnknownCommand check passes for it. + const availableSubcommands = PHASE_SUBCOMMANDS.filter(s => !UNSUPPORTED[s]); + 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, }); + + // ── Dispatch ──────────────────────────────────────────────────────────────── + const result = hub.dispatch({ + family: 'phase', + subcommand, + args: args.slice(2), + cwd, + raw, + }); + + // ── Translate result → CLI output / error (adapter responsibility) ────────── + 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 + 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/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 65b98a7e2..3f9e05b55 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -1,7 +1,7 @@ { "_doc": "Baseline of modules currently exceeding the 2-test-file limit. Each entry locks in TODAY's count as the ceiling. Reductions are ratcheted automatically — when a cluster drops to ≤ 2, remove its entry. New entries require justification in PR description.", "modules": { - "phase": { "current": 4, "issue": "3740" }, + "phase": { "current": 5, "issue": "3788" }, "worktree": { "current": 13, "issue": "TBD" }, "milestone": { "current": 10, "issue": "TBD" }, "roadmap": { "current": 9, "issue": "TBD" }, diff --git a/tests/command-routing-hub.test.cjs b/tests/command-routing-hub.test.cjs new file mode 100644 index 000000000..790577d03 --- /dev/null +++ b/tests/command-routing-hub.test.cjs @@ -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); + }); +}); diff --git a/tests/phase-command-router.test.cjs b/tests/phase-command-router.test.cjs new file mode 100644 index 000000000..c7f9b060d --- /dev/null +++ b/tests/phase-command-router.test.cjs @@ -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']); + }); +});