feat(#301): remove retired sdk command-contract drift (#302)

This commit is contained in:
Tom Boucher
2026-05-26 12:25:52 -04:00
committed by GitHub
parent aec61416d8
commit de08c8cd60
5 changed files with 37 additions and 62 deletions

View File

@@ -0,0 +1,6 @@
---
type: Changed
pr: 301
---
<!-- docs-exempt: command-surface cleanup with no new commands/flags or workflow contract changes -->
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.

View File

@@ -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 <command>` 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');

View File

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

View File

@@ -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

View File

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