diff --git a/.changeset/sunny-dogs-frolic.md b/.changeset/sunny-dogs-frolic.md new file mode 100644 index 000000000..c6e5798e8 --- /dev/null +++ b/.changeset/sunny-dogs-frolic.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3311 +--- +**`gsd-tools --json-errors` covers every error path** — every "Unknown subcommand" and missing-required-arg error now emits a typed `ERROR_REASON` code (`sdk_unknown_command` or `usage`) instead of the fallback `unknown`. Tests can now lock these paths via `JSON.parse(stderr).reason` without grepping the human message (#3310, builds on #3255). diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index 2d61834d7..b805dd721 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -244,24 +244,42 @@ function parseMultiwordArg(args, flag) { async function main() { let args = process.argv.slice(2); + // --json-errors / GSD_JSON_ERRORS=1: when active, error() emits structured + // JSON ({ ok: false, reason: , message }) to stderr + // instead of "Error: ". Lets test suites assert on typed reason codes + // per CONTRIBUTING.md "Prohibited: Raw Text Matching" (#2974). + // + // Detect early — before any flag parsing that can fire error() — so even + // --cwd and workstream-resolution failures emit structured stderr (#3310). + // The argv splice must happen here too, otherwise the dispatcher below sees + // "--json-errors" as an unknown command. Default off — human operators keep + // their plain-text diagnostic. + const jsonErrorsIdx = args.indexOf('--json-errors'); + if (jsonErrorsIdx !== -1) { + core.setJsonErrorMode(true); + args.splice(jsonErrorsIdx, 1); + } else if (process.env.GSD_JSON_ERRORS === '1') { + core.setJsonErrorMode(true); + } + // Optional cwd override for sandboxed subagents running outside project root. let cwd = process.cwd(); const cwdEqArg = args.find(arg => arg.startsWith('--cwd=')); const cwdIdx = args.indexOf('--cwd'); if (cwdEqArg) { const value = cwdEqArg.slice('--cwd='.length).trim(); - if (!value) error('Missing value for --cwd'); + if (!value) error('Missing value for --cwd', ERROR_REASON.USAGE); args.splice(args.indexOf(cwdEqArg), 1); cwd = path.resolve(value); } else if (cwdIdx !== -1) { const value = args[cwdIdx + 1]; - if (!value || value.startsWith('--')) error('Missing value for --cwd'); + if (!value || value.startsWith('--')) error('Missing value for --cwd', ERROR_REASON.USAGE); args.splice(cwdIdx, 2); cwd = path.resolve(value); } if (!fs.existsSync(cwd) || !fs.statSync(cwd).isDirectory()) { - error(`Invalid --cwd: ${cwd}`); + error(`Invalid --cwd: ${cwd}`, ERROR_REASON.USAGE); } // Resolve worktree root: in a linked worktree, .planning/ lives in the main worktree. @@ -294,20 +312,6 @@ async function main() { const raw = rawIndex !== -1; if (rawIndex !== -1) args.splice(rawIndex, 1); - // --json-errors: when present, error() emits structured JSON to stderr - // ({ ok: false, reason: , message }) instead of plain - // "Error: ". Lets test suites assert on typed reason codes per the - // CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" rule - // (#2974). Default off — human operators see the original plain-text - // diagnostic. - const jsonErrorsIdx = args.indexOf('--json-errors'); - if (jsonErrorsIdx !== -1) { - core.setJsonErrorMode(true); - args.splice(jsonErrorsIdx, 1); - } else if (process.env.GSD_JSON_ERRORS === '1') { - core.setJsonErrorMode(true); - } - // --pick : extract a single field from JSON output (replaces jq dependency). // Supports dot-notation (e.g., --pick workflow.research) and bracket notation // for arrays (e.g., --pick directories[-1]). @@ -579,7 +583,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand wave: wave || '1', }, raw); } else { - error('Unknown template subcommand. Available: select, fill'); + error('Unknown template subcommand. Available: select, fill', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -597,7 +601,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } else if (subcommand === 'validate') { frontmatter.cmdFrontmatterValidate(cwd, file, parseNamedArgs(args, ['schema']).schema, raw); } else { - error('Unknown frontmatter subcommand. Available: get, set, merge, validate'); + error('Unknown frontmatter subcommand. Available: get, set, merge, validate', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -706,7 +710,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand if (subcommand === 'mark-complete') { milestone.cmdRequirementsMarkComplete(cwd, args.slice(2), raw); } else { - error('Unknown requirements subcommand. Available: mark-complete'); + error('Unknown requirements subcommand. Available: mark-complete', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -736,7 +740,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const archivePhases = args.includes('--archive-phases'); milestone.cmdMilestoneComplete(cwd, args[2], { name: milestoneName, archivePhases }, raw); } else { - error('Unknown milestone subcommand. Available: complete'); + error('Unknown milestone subcommand. Available: complete', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -788,7 +792,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const options = parseNamedArgs(args, ['file']); uat.cmdRenderCheckpoint(cwd, options, raw); } else { - error('Unknown uat subcommand. Available: render-checkpoint'); + error('Unknown uat subcommand. Available: render-checkpoint', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -806,7 +810,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } else if (subcommand === 'match-phase') { commands.cmdTodoMatchPhase(cwd, args[2], raw); } else { - error('Unknown todo subcommand. Available: complete, match-phase'); + error('Unknown todo subcommand. Available: complete, match-phase', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -882,7 +886,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const sessionsPath = pathIdx !== -1 ? args[pathIdx + 1] : null; const projectArg = args[1]; if (!projectArg || projectArg.startsWith('--')) { - error('Usage: gsd-tools extract-messages [--session ] [--limit N] [--path ]\nRun scan-sessions first to see available projects.'); + error('Usage: gsd-tools extract-messages [--session ] [--limit N] [--path ]\nRun scan-sessions first to see available projects.', ERROR_REASON.USAGE); } await profilePipeline.cmdExtractMessages(projectArg, { sessionId, limit }, raw, sessionsPath); break; @@ -906,7 +910,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand case 'write-profile': { const inputIdx = args.indexOf('--input'); const inputPath = inputIdx !== -1 ? args[inputIdx + 1] : null; - if (!inputPath) error('--input is required'); + if (!inputPath) error('--input is required', ERROR_REASON.USAGE); const outputIdx = args.indexOf('--output'); const outputPath = outputIdx !== -1 ? args[outputIdx + 1] : null; profileOutput.cmdWriteProfile(cwd, { input: inputPath, output: outputPath }, raw); @@ -972,7 +976,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } else if (subcommand === 'progress') { workstream.cmdWorkstreamProgress(cwd, raw); } else { - error('Unknown workstream subcommand. Available: create, list, status, complete, set, get, progress'); + error('Unknown workstream subcommand. Available: create, list, status, complete, set, get, progress', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -984,7 +988,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const subcommand = args[1]; if (subcommand === 'query') { const term = args[2]; - if (!term) error('Usage: gsd-tools intel query '); + if (!term) error('Usage: gsd-tools intel query ', ERROR_REASON.USAGE); const planningDir = path.join(cwd, '.planning'); core.output(intel.intelQuery(term, planningDir), raw); } else if (subcommand === 'status') { @@ -1006,14 +1010,14 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand core.output(intel.intelSnapshot(planningDir), raw); } else if (subcommand === 'patch-meta') { const filePath = args[2]; - if (!filePath) error('Usage: gsd-tools intel patch-meta '); + if (!filePath) error('Usage: gsd-tools intel patch-meta ', ERROR_REASON.USAGE); core.output(intel.intelPatchMeta(path.resolve(cwd, filePath)), raw); } else if (subcommand === 'validate') { const planningDir = path.join(cwd, '.planning'); core.output(intel.intelValidate(planningDir), raw); } else if (subcommand === 'extract-exports') { const filePath = args[2]; - if (!filePath) error('Usage: gsd-tools intel extract-exports '); + if (!filePath) error('Usage: gsd-tools intel extract-exports ', ERROR_REASON.USAGE); core.output(intel.intelExtractExports(path.resolve(cwd, filePath)), raw); } else if (subcommand === 'update') { const planningDir = path.join(cwd, '.planning'); @@ -1031,7 +1035,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const subcommand = args[1]; if (subcommand === 'query') { const term = args[2]; - if (!term) error('Usage: gsd-tools graphify query '); + if (!term) error('Usage: gsd-tools graphify query ', ERROR_REASON.USAGE); const budgetIdx = args.indexOf('--budget'); const budget = budgetIdx !== -1 ? parseInt(args[budgetIdx + 1], 10) : null; core.output(graphify.graphifyQuery(cwd, term, { budget }), raw); @@ -1046,7 +1050,7 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand core.output(graphify.graphifyBuild(cwd), raw); } } else { - error('Unknown graphify subcommand. Available: build, query, status, diff'); + error('Unknown graphify subcommand. Available: build, query, status, diff', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -1067,21 +1071,21 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } else if (subcommand === 'query') { const tagIdx = args.indexOf('--tag'); const tag = tagIdx !== -1 ? args[tagIdx + 1] : null; - if (!tag) error('Usage: gsd-tools learnings query --tag '); + if (!tag) error('Usage: gsd-tools learnings query --tag ', ERROR_REASON.USAGE); learnings.cmdLearningsQuery(tag, raw); } else if (subcommand === 'copy') { learnings.cmdLearningsCopy(cwd, raw); } else if (subcommand === 'prune') { const olderIdx = args.indexOf('--older-than'); const olderThan = olderIdx !== -1 ? args[olderIdx + 1] : null; - if (!olderThan) error('Usage: gsd-tools learnings prune --older-than '); + if (!olderThan) error('Usage: gsd-tools learnings prune --older-than ', ERROR_REASON.USAGE); learnings.cmdLearningsPrune(olderThan, raw); } else if (subcommand === 'delete') { const id = args[2]; - if (!id) error('Usage: gsd-tools learnings delete '); + if (!id) error('Usage: gsd-tools learnings delete ', ERROR_REASON.USAGE); learnings.cmdLearningsDelete(id, raw); } else { - error('Unknown learnings subcommand. Available: list, query, copy, prune, delete'); + error('Unknown learnings subcommand. Available: list, query, copy, prune, delete', ERROR_REASON.SDK_UNKNOWN_COMMAND); } break; } @@ -1101,11 +1105,11 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const configDirIdx = args.indexOf('--config-dir'); const configDir = configDirIdx !== -1 ? args[configDirIdx + 1] : null; if (!configDir) { - error('Usage: gsd-tools detect-custom-files --config-dir '); + error('Usage: gsd-tools detect-custom-files --config-dir ', ERROR_REASON.USAGE); } const resolvedConfigDir = path.resolve(configDir); if (!fs.existsSync(resolvedConfigDir)) { - error(`Config directory not found: ${resolvedConfigDir}`); + error(`Config directory not found: ${resolvedConfigDir}`, ERROR_REASON.USAGE); } const manifestPath = path.join(resolvedConfigDir, 'gsd-file-manifest.json'); diff --git a/tests/feat-3310-followup-typed-codes.test.cjs b/tests/feat-3310-followup-typed-codes.test.cjs new file mode 100644 index 000000000..5f91dba66 --- /dev/null +++ b/tests/feat-3310-followup-typed-codes.test.cjs @@ -0,0 +1,225 @@ +/** + * Follow-up tests for #3310: every remaining `error()` call at a subcommand + * boundary or usage check in `gsd-tools.cjs` carries a typed `ERROR_REASON`. + * + * #3304 wired four representative paths (unknown top-level command, unknown + * intel subcommand, missing --pick value, --version flag). The rest fell + * through to `ERROR_REASON.UNKNOWN`. This file locks the post-#3310 contract: + * + * - Every "Unknown subcommand" emits reason: "sdk_unknown_command". + * - Every "Usage: ..." / missing-required-arg path emits reason: "usage". + * + * All assertions parse stderr via JSON.parse — never `.includes()` — per the + * #2974 / CONTRIBUTING.md "Prohibited: Raw Text Matching" rule. + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +// Run gsd-tools with GSD_JSON_ERRORS=1 (env-var activation, exercises the +// path #3304 added alongside the --json-errors flag) and parse the +// structured stderr. Returns the parsed object; throws if stderr is not JSON. +function runJsonErrors(args, tmpDir, env = {}) { + const result = runGsdTools(args, tmpDir, { ...env, GSD_JSON_ERRORS: '1' }); + assert.strictEqual(result.success, false, + `Expected failure with GSD_JSON_ERRORS=1 for args: ${args.join(' ')}\n` + + `stdout: ${result.output}\nstderr: ${result.error}`); + let parsed; + try { + parsed = JSON.parse(result.error); + } catch (e) { + throw new Error( + `GSD_JSON_ERRORS=1 must emit valid JSON on stderr.\n` + + `Args: ${args.join(' ')}\nstderr: ${result.error}\nparse error: ${e.message}` + ); + } + return parsed; +} + +// Assert the typed-IR contract: object shape + reason. Keeps the per-test +// boilerplate minimal so each error-path test reads as a single fact. +function assertTypedError(parsed, expectedReason, label) { + assert.strictEqual(parsed.ok, false, + `${label}: error object must have ok: false`); + assert.strictEqual(parsed.reason, expectedReason, + `${label}: reason must be "${expectedReason}", got: ${parsed.reason}`); + assert.ok(typeof parsed.message === 'string' && parsed.message.length > 0, + `${label}: message must be a non-empty string`); +} + +describe('feat #3310: typed ERROR_REASON codes on remaining error paths', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ── Unknown subcommand → SDK_UNKNOWN_COMMAND ──────────────── + // Each of these used to fall through to reason: "unknown" before #3310. + + test('unknown template subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['template', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'template'); + }); + + test('unknown frontmatter subcommand → sdk_unknown_command', () => { + // frontmatter expects subcommand at args[1] and file at args[2]; pass a + // bogus subcommand with a placeholder file so we definitely reach the + // unknown-subcommand branch, not an earlier validation. + const parsed = runJsonErrors( + ['frontmatter', 'bogus-subcommand-xyzzy', 'placeholder.md'], + tmpDir + ); + assertTypedError(parsed, 'sdk_unknown_command', 'frontmatter'); + }); + + test('unknown requirements subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['requirements', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'requirements'); + }); + + test('unknown milestone subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['milestone', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'milestone'); + }); + + test('unknown uat subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['uat', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'uat'); + }); + + test('unknown todo subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['todo', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'todo'); + }); + + test('unknown workstream subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['workstream', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'workstream'); + }); + + test('unknown graphify subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['graphify', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'graphify'); + }); + + test('unknown learnings subcommand → sdk_unknown_command', () => { + const parsed = runJsonErrors(['learnings', 'bogus-subcommand-xyzzy'], tmpDir); + assertTypedError(parsed, 'sdk_unknown_command', 'learnings'); + }); + + // ── Missing required positional/flag values → USAGE ───────────────────── + // These previously emitted reason: "unknown" because the second argument + // to error() was absent. + + test('missing --cwd value → usage', () => { + // The --cwd flag is consumed before the command dispatcher; passing it + // bare with no following value triggers the usage error at L253/L258. + const parsed = runJsonErrors(['--cwd'], tmpDir); + assertTypedError(parsed, 'usage', '--cwd missing value'); + }); + + test('invalid --cwd directory → usage', () => { + // --cwd hits the existsSync / isDirectory check at L264. + const parsed = runJsonErrors( + ['--cwd', '/this/path/should/not/exist/anywhere/xyzzy', 'state', 'load'], + tmpDir + ); + assertTypedError(parsed, 'usage', 'invalid --cwd directory'); + }); + + test('intel query missing term → usage', () => { + const parsed = runJsonErrors(['intel', 'query'], tmpDir); + assertTypedError(parsed, 'usage', 'intel query missing term'); + }); + + test('intel patch-meta missing file path → usage', () => { + const parsed = runJsonErrors(['intel', 'patch-meta'], tmpDir); + assertTypedError(parsed, 'usage', 'intel patch-meta missing file'); + }); + + test('intel extract-exports missing file path → usage', () => { + const parsed = runJsonErrors(['intel', 'extract-exports'], tmpDir); + assertTypedError(parsed, 'usage', 'intel extract-exports missing file'); + }); + + test('graphify query missing term → usage', () => { + const parsed = runJsonErrors(['graphify', 'query'], tmpDir); + assertTypedError(parsed, 'usage', 'graphify query missing term'); + }); + + test('learnings query missing --tag → usage', () => { + const parsed = runJsonErrors(['learnings', 'query'], tmpDir); + assertTypedError(parsed, 'usage', 'learnings query missing --tag'); + }); + + test('learnings prune missing --older-than → usage', () => { + const parsed = runJsonErrors(['learnings', 'prune'], tmpDir); + assertTypedError(parsed, 'usage', 'learnings prune missing --older-than'); + }); + + test('learnings delete missing id → usage', () => { + const parsed = runJsonErrors(['learnings', 'delete'], tmpDir); + assertTypedError(parsed, 'usage', 'learnings delete missing id'); + }); + + test('extract-messages missing project arg → usage', () => { + // L877 — args[1] is undefined or starts with '--'. + const parsed = runJsonErrors(['extract-messages'], tmpDir); + assertTypedError(parsed, 'usage', 'extract-messages missing project'); + }); + + test('write-profile missing --input → usage', () => { + const parsed = runJsonErrors(['write-profile'], tmpDir); + assertTypedError(parsed, 'usage', 'write-profile missing --input'); + }); + + test('detect-custom-files missing --config-dir → usage', () => { + const parsed = runJsonErrors(['detect-custom-files'], tmpDir); + assertTypedError(parsed, 'usage', 'detect-custom-files missing --config-dir'); + }); + + test('detect-custom-files invalid --config-dir → usage', () => { + const parsed = runJsonErrors( + ['detect-custom-files', '--config-dir', '/nonexistent/path/xyzzy'], + tmpDir + ); + assertTypedError(parsed, 'usage', 'detect-custom-files invalid --config-dir'); + }); + + // ── Shape regression guard: every newly-typed path emits the canonical + // {ok, reason, message} object — no leakage of reason: "unknown". ──── + + test('every remaining typed path emits the canonical {ok, reason, message} shape', () => { + const probes = [ + ['template', 'bogus'], + ['frontmatter', 'bogus', 'placeholder.md'], + ['requirements', 'bogus'], + ['milestone', 'bogus'], + ['uat', 'bogus'], + ['todo', 'bogus'], + ['workstream', 'bogus'], + ['graphify', 'bogus'], + ['learnings', 'bogus'], + ['intel', 'query'], + ['extract-messages'], + ['write-profile'], + ['detect-custom-files'], + ]; + for (const args of probes) { + const parsed = runJsonErrors(args, tmpDir); + const keys = Object.keys(parsed).sort(); + assert.deepStrictEqual(keys, ['message', 'ok', 'reason'], + `args ${args.join(' ')}: keys must be exactly {ok,reason,message}, got ${keys.join(',')}`); + assert.notStrictEqual(parsed.reason, 'unknown', + `args ${args.join(' ')}: reason must be a typed code, not the fallback "unknown"`); + } + }); +});