From de08c8cd60ef9d96fcbccaab68395c7d9f990344 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 26 May 2026 12:25:52 -0400 Subject: [PATCH] feat(#301): remove retired sdk command-contract drift (#302) --- .changeset/lively-bears-sprint.md | 6 +++ get-shit-done/bin/gsd-tools.cjs | 38 +++++++------------ get-shit-done/bin/lib/command-aliases.cjs | 18 +-------- .../bin/lib/phase-command-router.cjs | 16 +++----- tests/phase-command-router.test.cjs | 21 +++++----- 5 files changed, 37 insertions(+), 62 deletions(-) create mode 100644 .changeset/lively-bears-sprint.md diff --git a/.changeset/lively-bears-sprint.md b/.changeset/lively-bears-sprint.md new file mode 100644 index 000000000..6203f32a2 --- /dev/null +++ b/.changeset/lively-bears-sprint.md @@ -0,0 +1,6 @@ +--- +type: Changed +pr: 301 +--- + +Remove retired SDK command-contract text from gsd-tools phase routing. Phase list-plans/list-artifacts are no longer surfaced as SDK-only aliases and now resolve via the standard unknown-subcommand path. diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index 48609e959..8c68e74a9 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -1,11 +1,7 @@ #!/usr/bin/env node /** - * @deprecated The supported programmatic surface is `gsd-sdk query` (SDK query registry) - * and the `@opengsd/gsd-sdk` package. This Node CLI remains the compatibility implementation - * for shell scripts and older workflows; prefer calling the SDK from agents and automation. - * - * GSD Tools — CLI utility for GSD workflow operations + * GSD Tools — CLI utility for GSD workflow operations. * * Replaces repetitive inline bash patterns across ~50 GSD command/workflow/agent files. * Centralizes: config parsing, model resolution, phase lookup, git commits, summary verification. @@ -206,17 +202,16 @@ const { parseNamedArgs, parseMultiwordArg } = require('./lib/command-arg-project // sequence; it always returns false so callers fall through to CJS. /** - * Attempt SDK dispatch for a non-family command. + * Retired bridge-era shim for non-family dispatch. * - * Returns true when the SDK was available and handled the command (success or - * typed error). Returns false when the SDK is unavailable, signalling the - * caller to fall through to the CJS handler. + * Always returns false so command handlers continue down the CJS path. + * Kept only to avoid churn while legacy call sites are being deleted. * * @param {object} opts - * @param {string} opts.registryCommand - canonical command name in the SDK registry - * @param {string[]} opts.registryArgs - args to pass to the SDK handler - * @param {string} opts.legacyCommand - original gsd-tools command name (for error messages) - * @param {string[]} opts.legacyArgs - original args (for error messages) + * @param {string} opts.registryCommand - legacy bridge placeholder + * @param {string[]} opts.registryArgs - legacy bridge placeholder + * @param {string} opts.legacyCommand - original gsd-tools command name + * @param {string[]} opts.legacyArgs - original args * @param {string} opts.cwd - project dir * @param {boolean} opts.raw - raw output mode * @param {Function} opts.error - error reporter @@ -334,7 +329,7 @@ async function main() { let command = args[0]; - // Accept `query` meta-prefix parity with `gsd-sdk query ...`. + // Accept `query` as a meta-prefix for canonical dotted/spaced commands. // Workflows may call `node gsd-tools.cjs query ` directly. if (command === 'query') { args.shift(); @@ -342,15 +337,11 @@ async function main() { } // #3243: accept dotted canonical form (e.g. `state.update`) as well as the - // spaced form (`state update`). Workflow files and stale SDK binaries pass - // the dotted canonical form directly; any caller that bypasses the SDK - // client-side split hit "Unknown command" before this shim. + // spaced form (`state update`). Some workflow callers pass the dotted + // canonical form directly; this normalization keeps both forms valid. // // Split on the FIRST dot only — `check.decision-coverage-plan` becomes // command='check', args=['check','decision-coverage-plan',...rest]. - // Parallel to dottedCommandToCjsArgv in sdk/src/query/query-fallback-bridge-adapter.ts; - // kept separate here to avoid SDK coupling (see TODO: extract to shared helper). - // // Guard: head and rest must both be non-empty (rejects leading-dot args like // ".hidden" and bare-dot "."). const originalCommand = command; // preserved for "Unknown command" suggestion @@ -396,11 +387,8 @@ async function main() { // and exit 0 — not error out with "Unknown flag". The previous shape // erred on agent-hallucinated flags, but it also blocked humans from // discovering the command surface via subcommand help requests routed - // here from the SDK CLI's query dispatcher (after the cli.ts fix that - // stops harvesting --help as a global flag). Rendering top-level usage - // on --help is strictly better UX than the old short-circuit, which - // printed the SDK-level usage that doesn't mention any of these - // subcommands. + // through this dispatcher. Rendering top-level usage on --help is strictly + // better UX than the old short-circuit that printed unrelated usage text. const HELP_FLAGS = new Set(['-h', '--help', '-?', '--h', '--usage']); if (args.some((a) => HELP_FLAGS.has(a))) { process.stdout.write(TOP_LEVEL_USAGE + '\n'); diff --git a/get-shit-done/bin/lib/command-aliases.cjs b/get-shit-done/bin/lib/command-aliases.cjs index fc5873ed5..c978440e3 100644 --- a/get-shit-done/bin/lib/command-aliases.cjs +++ b/get-shit-done/bin/lib/command-aliases.cjs @@ -373,22 +373,6 @@ const INIT_COMMAND_ALIASES = [ ]; const PHASE_COMMAND_ALIASES = [ - { - "canonical": "phase.list-plans", - "aliases": [ - "phase list-plans" - ], - "subcommand": "list-plans", - "mutation": false - }, - { - "canonical": "phase.list-artifacts", - "aliases": [ - "phase list-artifacts" - ], - "subcommand": "list-artifacts", - "mutation": false - }, { "canonical": "phase.uat-passed", "aliases": [ @@ -821,4 +805,4 @@ module.exports = { PHASES_SUBCOMMANDS, VALIDATE_SUBCOMMANDS, ROADMAP_SUBCOMMANDS, -}; \ No newline at end of file +}; diff --git a/get-shit-done/bin/lib/phase-command-router.cjs b/get-shit-done/bin/lib/phase-command-router.cjs index 8715a9ce5..f3babd563 100644 --- a/get-shit-done/bin/lib/phase-command-router.cjs +++ b/get-shit-done/bin/lib/phase-command-router.cjs @@ -9,9 +9,7 @@ const { createHub, ERROR_KINDS, makeInvalidArgs } = require('./command-routing-h * Manifest-backed phase subcommand router. * Keeps gsd-tools.cjs thin while preserving existing command semantics. * - * Unsupported in this router (error returned before dispatch): - * - list-plans. - * - list-artifacts. + * Unsupported in this router: * - scaffold: routed through top-level scaffold command. * * CJS-only subcommands: mvp-mode (dispatched directly, before hub). @@ -20,11 +18,9 @@ const { createHub, ERROR_KINDS, makeInvalidArgs } = require('./command-routing-h * and observable CLI behaviour are unchanged. */ function routePhaseCommand({ phase, args, cwd, raw, error }) { - // ── Unsupported subcommands ──────────────────────────────────────────────── - // Resolved before dispatch so the error message matches the pre-#3788 text. + // ── Unsupported subcommands ───────────────────────────────────────────────── + // Resolved before dispatch so the error message stays deterministic. const UNSUPPORTED = { - 'list-plans': 'phase list-plans is not supported in this router.', - 'list-artifacts': 'phase list-artifacts is not supported in this router.', scaffold: 'phase scaffold is routed through the top-level scaffold command.', }; @@ -146,9 +142,9 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { // ── Build manifest (available subcommands for UnknownCommand detection) ───── // `availableSubcommands` is what the error message shows. It excludes - // 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. + // 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 diff --git a/tests/phase-command-router.test.cjs b/tests/phase-command-router.test.cjs index c6e9326e4..1a3f20803 100644 --- a/tests/phase-command-router.test.cjs +++ b/tests/phase-command-router.test.cjs @@ -6,7 +6,7 @@ * Shape: * 1. Adapter translation — CLI args → hub dispatch shape * 2. Result translation — hub result → stdout / error callback - * 3. Unsupported subcommands — unsupported commands produce the documented error + * 3. Unsupported subcommands — scaffold produces the documented redirect * 4. Unknown subcommand — unmapped subcommands produce a well-formed error * 5. Integration — real hub + real CJS phase handler invocation * @@ -22,7 +22,8 @@ const assert = require('node:assert/strict'); const { routePhaseCommand } = require('../get-shit-done/bin/lib/phase-command-router.cjs'); -// Set GSD_WORKSTREAM for deterministic routing context in tests. +// 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; @@ -281,10 +282,10 @@ describe('phase-command-router — result translation (error path)', () => { }); }); -// ─── 3. Unsupported subcommands ─────────────────────────────────────────────── +// ─── 3. Unsupported subcommands ──────────────────────────────────────────────── describe('phase-command-router — unsupported subcommands', () => { - test('phase list-plans calls error() with unsupported message', () => { + test('phase list-plans resolves as unknown subcommand', () => { let msg = null; routePhaseCommand({ phase: makePhase(), @@ -295,11 +296,11 @@ describe('phase-command-router — unsupported subcommands', () => { }); assert.ok(msg !== null); - assert.ok(msg.includes('not supported'), `expected unsupported text in: ${msg}`); - assert.ok(msg.includes('list-plans')); + assert.ok(msg.includes('Unknown phase subcommand')); + assert.ok(msg.includes('Available:'), `expected "Available:" in: ${msg}`); }); - test('phase list-artifacts calls error() with unsupported message', () => { + test('phase list-artifacts resolves as unknown subcommand', () => { let msg = null; routePhaseCommand({ phase: makePhase(), @@ -310,8 +311,8 @@ describe('phase-command-router — unsupported subcommands', () => { }); assert.ok(msg !== null); - assert.ok(msg.includes('not supported')); - assert.ok(msg.includes('list-artifacts')); + assert.ok(msg.includes('Unknown phase subcommand')); + assert.ok(msg.includes('Available:'), `expected "Available:" in: ${msg}`); }); test('phase scaffold calls error() with redirect message', () => { @@ -362,7 +363,7 @@ describe('phase-command-router — unknown subcommand', () => { 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 (unsupported) must not appear in available list: ${msg}`); + assert.ok(!msg.includes('list-plans'), `list-plans must not appear in available list: ${msg}`); }); });