golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat(3310): wire remaining ERROR_REASON typed codes into gsd-tools (#3311)

Browse Source 
Closes #3310

Wires `ERROR_REASON.SDK_UNKNOWN_COMMAND` and `ERROR_REASON.USAGE` into the remaining untyped error paths in `gsd-tools.cjs` (template, frontmatter, requirements, milestone, uat, todo, workstream, graphify, learnings subcommand routers and `--cwd`/missing-required-arg paths). Tests assert via `JSON.parse(stderr).reason` rather than substring matching on prose. Closure regression guard locks the canonical `{ok, reason, message}` shape for every newly-typed path.

Also moves `--json-errors` activation up to the top of `main()` so `--cwd` validation paths emit JSON rather than plain text when `GSD_JSON_ERRORS=1` is set.
This commit is contained in:
Tom Boucher
2026-05-09 13:54:06 -04:00
committed by GitHub
parent eaf20d2cd3
commit 2e87c60afc
3 changed files with 271 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/sunny-dogs-frolic.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Changed
pr: 3311
---
**`gsd-tools --json-errors` covers every error path** — every "Unknown <subsystem> 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).

78
get-shit-done/bin/gsd-tools.cjs
View File

@@ -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: <ERROR_REASON code>, message }) to stderr
// instead of "Error: <text>". 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: <ERROR_REASON code>, message }) instead of plain
// "Error: <text>". 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 <name>: 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 <project> [--session <id>] [--limit N] [--path <dir>]\nRun scan-sessions first to see available projects.');
error('Usage: gsd-tools extract-messages <project> [--session <id>] [--limit N] [--path <dir>]\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 <analysis-json-path> is required');
if (!inputPath) error('--input <analysis-json-path> 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 <term>');
if (!term) error('Usage: gsd-tools intel query <term>', 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 <file-path>');
if (!filePath) error('Usage: gsd-tools intel patch-meta <file-path>', 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 <file-path>');
if (!filePath) error('Usage: gsd-tools intel extract-exports <file-path>', 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 <term>');
if (!term) error('Usage: gsd-tools graphify query <term>', 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 <tag>');
if (!tag) error('Usage: gsd-tools learnings query --tag <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 <duration>');
if (!olderThan) error('Usage: gsd-tools learnings prune --older-than <duration>', ERROR_REASON.USAGE);
learnings.cmdLearningsPrune(olderThan, raw);
} else if (subcommand === 'delete') {
const id = args[2];
if (!id) error('Usage: gsd-tools learnings delete <id>');
if (!id) error('Usage: gsd-tools learnings delete <id>', 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 <path>');
error('Usage: gsd-tools detect-custom-files --config-dir <path>', 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');

225
tests/feat-3310-followup-typed-codes.test.cjs Normal file
View File

@@ -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 <subsystem> 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 <subsystem> 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 <nonexistent-path> 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"`);
}
});
});