* fix(#3019): query --help reaches handler instead of short-circuiting to top-level usage The query argv parser in sdk/src/cli.ts harvested -h/--help as a global flag and main() short-circuited dispatch when args.help was true. Net effect: every `gsd-sdk query <anything> --help` printed top-level USAGE instead of contextual subcommand help. There was no path for users to discover what arguments a query subcommand accepts — they had to trigger "required" errors by trial and error. Two-layer fix: 1. sdk/src/cli.ts (parseCliArgsQueryPermissive) - Push -h / --help onto queryArgv instead of consuming them silently, so the registered handler / gsd-tools.cjs fallback gets to interpret the flag and render contextual help. - Only honor the global help flag when there is NO real subcommand to dispatch to (i.e. queryArgv contains only help flags). Preserves `gsd-sdk query --help` → top-level USAGE while letting `gsd-sdk query phase add --help` reach the handler. 2. get-shit-done/bin/gsd-tools.cjs - Render top-level usage on --help / -h / -? / --usage instead of erroring with "Unknown flag". The discovery hint in the usage text points users at the working method (run without args → error names required arguments) and references #3019 for tracking subcommand- level help printers. - --version remains rejected (no discovery use-case). #1818 anti-hallucination invariant preserved: the destructive command NEVER executes when --help is present. The new shape returns success:true + usage on stdout instead of the old success:false + error on stderr — both satisfy "destructive command did not run", and the new shape also restores discoverability. Tests: - sdk/src/cli.test.ts: 4 new vitest cases covering #3019 — query argv parser keeps --help with subcommand, parses -h short flag, preserves bare `query --help` top-level behavior, preserves --help position when intermixed with other query flags. - tests/bug-3019-help-passthrough.test.cjs: 5 node:test cases on the fallback — bare gsd-tools (no args) errors with usage; --help renders usage on stdout exit 0; -h same; subcommand --help renders usage; usage hint mentions discovery method (without prose substring matching — parses into typed sections). - tests/bug-1818-unknown-flags.test.cjs: rewritten to assert the new invariant ("destructive command did not run" + "usage was rendered") instead of the old shape ("--help is rejected with non-zero exit"). Each destructive test seeds a sentinel artifact (phase dir, slug output) and asserts it survives. Verification: - 47/47 vitest pass on sdk/src/cli.test.ts - 5/5 pass on tests/bug-3019-help-passthrough.test.cjs - 8/8 pass on tests/bug-1818-unknown-flags.test.cjs (rewritten) - 6763/6763 pass on full node:test suite - lint-no-source-grep clean (0 violations) Closes #3019 * fix(#3019): SDK fallback forwards plain-text help, broader usage list (CR) CodeRabbit on PR #3026 (4 findings — 1 Major outside-diff, 2 inline, 1 nitpick): 1. **Major outside-diff** — sdk/src/cli.ts:442-454. The fallback path that delegates to gsd-tools.cjs called parseCliQueryJsonOutput (JSON.parse) on stdout. Now that gsd-tools renders plain-text usage on --help, JSON.parse threw "Unexpected token 'U'". Wrapped the parse in try/catch — on parse failure, forward the plain stdout verbatim so subcommand help reaches the user. Regression test: tests/bug-3019-help-passthrough.test.cjs spawns the built SDK and asserts `gsd-sdk query phase --help` exits 0, stdout contains the gsd-tools usage, and stderr does NOT contain a JSON-parse error. 2. .changeset/help-passthrough.md:3 — `pr: TBD` → `pr: 3026`. 3. gsd-tools.cjs:346 (TOP_LEVEL_USAGE): - Removed self-referencing `#3019` link (immediately stale after this PR merges). - Expanded Commands list from 17 → all 47 dispatcher cases: agent-skills, audit-open, audit-uat, check-commit, commit, … phase, phases, roadmap, milestone, validate, progress, intel, graphify, learnings, etc. — the bulk of the surface that was previously unreachable via --help discovery. 4. Nitpick: `isUsageOutput` was duplicated in bug-1818 and bug-3019-help-passthrough tests. Moved to tests/helpers.cjs with structural-comment, removed both duplicates. Verification: 47/47 vitest pass, 14/14 regression tests pass, 6764/6764 full suite, lint clean. * test(#3019): use t.skip() instead of bare return when SDK not built (CR) CodeRabbit follow-up on PR #3026: The integration test guarded against missing sdk/dist/cli.js with a bare `return;` — node:test counts that as a passing test (0 assertions exercised, 0 failures). On a CI checkout that hasn't run the SDK build, the #3026 regression test silently green-lit and no signal ever surfaced that the integration check was skipped. Switched to `t.skip(...)` via the test context parameter so the omission shows up in the test report. The unit-level fix (sdk/src/cli.ts) is still covered by vitest, so the skip only affects the end-to-end spawn-built-SDK check. Verification: 6/6 pass when SDK is built; 5 pass + 1 skip when not.
This commit is contained in:
Tom Boucher
5
.changeset/help-passthrough.md
Normal file
5
.changeset/help-passthrough.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3026
|
||||
---
|
||||
**`gsd-sdk query <subcommand> --help` now reaches the handler instead of returning top-level usage.** The query argv parser harvested `--help` as a global flag and `main()` short-circuited dispatch — there was no path to discover what arguments a query subcommand accepts. The parser now leaves `--help` in `queryArgv` so the handler/fallback can render contextual help. The `gsd-tools.cjs` fallback now renders top-level usage on `--help` (instead of erroring), preserving #1818's anti-hallucination invariant by NOT executing the destructive command. See #3019.
|
||||
@@ -341,17 +341,50 @@ async function main() {
|
||||
|
||||
const command = args[0];
|
||||
|
||||
// Top-level usage string — emitted by `gsd-tools` (no args) and by
|
||||
// `gsd-tools --help` / any `--help` request below.
|
||||
// CR feedback: the command list must enumerate every top-level command
|
||||
// supported by the dispatcher so `--help` is actually useful for
|
||||
// discovery; previously it was a partial subset that didn't include
|
||||
// phase / roadmap / milestone / progress / etc.
|
||||
const TOP_LEVEL_USAGE = 'Usage: gsd-tools <command> [args] [--raw] [--pick <field>] [--cwd <path>] [--ws <name>]\n' +
|
||||
'Commands: agent-skills, audit-open, audit-uat, check-commit, commit, commit-to-subrepo, ' +
|
||||
'config-ensure-section, config-get, config-new-project, config-path, config-set, ' +
|
||||
'current-timestamp, detect-custom-files, docs-init, extract-messages, find-phase, ' +
|
||||
'from-gsd2, frontmatter, gap-analysis, generate-claude-md, generate-claude-profile, ' +
|
||||
'generate-dev-preferences, generate-slug, graphify, history-digest, init, intel, ' +
|
||||
'learnings, list-todos, milestone, phase, phase-plan-index, phases, profile-questionnaire, ' +
|
||||
'profile-sample, progress, requirements, resolve-model, roadmap, scaffold, state, ' +
|
||||
'template, validate, verify, verify-path-exists, verify-summary, workstream\n\n' +
|
||||
'For command-specific argument requirements, invoke the command without args ' +
|
||||
'(e.g. `gsd-tools phase add`) — the resulting error lists what is required.';
|
||||
|
||||
if (!command) {
|
||||
error('Usage: gsd-tools <command> [args] [--raw] [--pick <field>] [--cwd <path>] [--ws <name>]\nCommands: state, resolve-model, find-phase, commit, verify-summary, verify, frontmatter, template, generate-slug, current-timestamp, list-todos, verify-path-exists, config-ensure-section, config-new-project, init, workstream, docs-init');
|
||||
error(TOP_LEVEL_USAGE);
|
||||
}
|
||||
|
||||
// Reject flags that are never valid for any gsd-tools command. AI agents
|
||||
// sometimes hallucinate --help or --version on tool invocations; silently
|
||||
// ignoring them can cause destructive operations to proceed unchecked.
|
||||
const NEVER_VALID_FLAGS = new Set(['-h', '--help', '-?', '--h', '--version', '-v', '--usage']);
|
||||
// #3019: a `--help` / `-h` flag in argv must render the top-level usage
|
||||
// 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.
|
||||
const HELP_FLAGS = new Set(['-h', '--help', '-?', '--h', '--usage']);
|
||||
if (args.some((a) => HELP_FLAGS.has(a))) {
|
||||
process.stdout.write(TOP_LEVEL_USAGE + '\n');
|
||||
return;
|
||||
}
|
||||
|
||||
// Reject version flags. AI agents sometimes hallucinate --version on tool
|
||||
// invocations; silently ignoring it can cause destructive operations to
|
||||
// proceed unchecked. (Help flags are handled above.)
|
||||
const NEVER_VALID_FLAGS = new Set(['--version', '-v']);
|
||||
for (const arg of args) {
|
||||
if (NEVER_VALID_FLAGS.has(arg)) {
|
||||
error(`Unknown flag: ${arg}\ngsd-tools does not accept help or version flags. Run "gsd-tools" with no arguments for usage.`);
|
||||
error(`Unknown flag: ${arg}\ngsd-tools does not accept version flags. Run "gsd-tools" with no arguments for usage.`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -126,6 +126,49 @@ describe('parseCliArgs', () => {
|
||||
expect(result.queryArgv).toEqual(['audit-open', '--json']);
|
||||
});
|
||||
|
||||
// ─── #3019: --help inside `query <subcommand>` reaches the handler ────
|
||||
|
||||
it('forwards --help to queryArgv when a subcommand precedes it (#3019)', () => {
|
||||
// gsd-sdk query phase add --help
|
||||
// Previously: --help was harvested as global, queryArgv = ['phase', 'add'],
|
||||
// help: true → main() short-circuits to top-level USAGE, never dispatching.
|
||||
// Now: --help travels with the rest of queryArgv so the registry handler
|
||||
// (or the gsd-tools.cjs fallback) can render contextual subcommand help.
|
||||
const result = parseCliArgs(['query', 'phase', 'add', '--help']);
|
||||
expect(result.command).toBe('query');
|
||||
expect(result.queryArgv).toEqual(['phase', 'add', '--help']);
|
||||
// The global help flag must NOT short-circuit dispatch when there is a
|
||||
// subcommand to dispatch to.
|
||||
expect(result.help).toBe(false);
|
||||
});
|
||||
|
||||
it('forwards -h to queryArgv when a subcommand precedes it (#3019)', () => {
|
||||
const result = parseCliArgs(['query', 'init', '-h']);
|
||||
expect(result.queryArgv).toEqual(['init', '-h']);
|
||||
expect(result.help).toBe(false);
|
||||
});
|
||||
|
||||
it('treats bare `query --help` as a top-level help request (no subcommand to dispatch to)', () => {
|
||||
// gsd-sdk query --help
|
||||
// No subcommand follows, so the only useful response is the top-level
|
||||
// USAGE. Preserve existing behavior: help: true.
|
||||
const result = parseCliArgs(['query', '--help']);
|
||||
expect(result.command).toBe('query');
|
||||
expect(result.help).toBe(true);
|
||||
// queryArgv may be empty or carry just the lone --help; either is fine
|
||||
// because main() short-circuits on help when there is no subcommand.
|
||||
expect((result.queryArgv ?? []).filter((x) => x !== '--help' && x !== '-h')).toEqual([]);
|
||||
});
|
||||
|
||||
it('preserves --help position when intermixed with other query flags (#3019)', () => {
|
||||
// gsd-sdk query phase --help --pick name
|
||||
// The handler/fallback should see --help in argv so it can render help
|
||||
// even when other flags are present.
|
||||
const result = parseCliArgs(['query', 'phase', '--help', '--pick', 'name']);
|
||||
expect(result.queryArgv).toEqual(['phase', '--help', '--pick', 'name']);
|
||||
expect(result.help).toBe(false);
|
||||
});
|
||||
|
||||
// ─── Init command parsing ──────────────────────────────────────────────
|
||||
|
||||
it('parses init with @file input', () => {
|
||||
|
||||
@@ -85,8 +85,17 @@ function parseCliArgsQueryPermissive(argv: string[]): ParsedCliArgs {
|
||||
i += 2;
|
||||
continue;
|
||||
}
|
||||
// #3019: do NOT consume -h / --help here unconditionally. Pushing the
|
||||
// flag onto queryArgv lets the registered handler (or the gsd-tools.cjs
|
||||
// fallback) render contextual subcommand help. We still set the global
|
||||
// `help` flag when the flag appears, but only short-circuit dispatch in
|
||||
// main() when there is no real subcommand to dispatch to (i.e. the only
|
||||
// tokens in queryArgv are the help flags themselves). That preserves
|
||||
// `gsd-sdk query --help` → top-level USAGE while letting
|
||||
// `gsd-sdk query phase add --help` reach the handler.
|
||||
if (a === '-h' || a === '--help') {
|
||||
help = true;
|
||||
queryArgv.push(a);
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
@@ -99,6 +108,14 @@ function parseCliArgsQueryPermissive(argv: string[]): ParsedCliArgs {
|
||||
i += 1;
|
||||
}
|
||||
|
||||
// If the user typed a real subcommand (anything other than help flags
|
||||
// alone in queryArgv), do NOT short-circuit to top-level USAGE on help.
|
||||
// The handler/fallback will render contextual help.
|
||||
const nonHelpTokens = queryArgv.filter((t) => t !== '-h' && t !== '--help');
|
||||
if (help && nonHelpTokens.length > 0) {
|
||||
help = false;
|
||||
}
|
||||
|
||||
return {
|
||||
command: 'query',
|
||||
prompt: undefined,
|
||||
@@ -430,7 +447,22 @@ export async function main(argv: string[] = process.argv.slice(2)): Promise<void
|
||||
args.ws,
|
||||
);
|
||||
if (stderr.trim()) console.error(stderr.trimEnd());
|
||||
let output: unknown = await parseCliQueryJsonOutput(stdout, args.projectDir);
|
||||
// #3026 CR (Major outside-diff): the gsd-tools.cjs fallback now
|
||||
// emits plain-text usage on --help / -h with exit 0, instead of
|
||||
// a JSON object. Wrap the JSON parse in a try/catch and forward
|
||||
// non-JSON stdout verbatim so subcommand help reaches the user.
|
||||
// (Previously this path JSON.parsed the help text and threw
|
||||
// "Unexpected token 'U'" — exitCode=1 — a regression introduced
|
||||
// alongside the --help passthrough fix.)
|
||||
let output: unknown;
|
||||
try {
|
||||
output = await parseCliQueryJsonOutput(stdout, args.projectDir);
|
||||
} catch {
|
||||
if (stdout.trim()) {
|
||||
process.stdout.write(stdout.endsWith('\n') ? stdout : stdout + '\n');
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (pickField) {
|
||||
output = extractField(output, pickField);
|
||||
}
|
||||
|
||||
@@ -1,19 +1,35 @@
|
||||
/**
|
||||
* Regression test for bug #1818
|
||||
* Regression test for bug #1818, updated for #3019.
|
||||
*
|
||||
* gsd-tools must reject unknown/invalid flags (--help, -h, etc.) with a
|
||||
* non-zero exit and an error message instead of silently ignoring them and
|
||||
* proceeding with the command — which can cause destructive operations to run
|
||||
* when an AI agent hallucinates a flag like --help.
|
||||
* Original #1818 invariant: gsd-tools must NOT silently ignore --help/-h
|
||||
* and proceed with a destructive command — that turned AI-agent
|
||||
* hallucinations into accidental data loss (e.g. `phases clear --help`
|
||||
* deleting phase dirs because the flag was dropped).
|
||||
*
|
||||
* #3019 update: the same destructive-protection invariant still holds,
|
||||
* but the response shape changed. Previously --help → non-zero error
|
||||
* exit. Now --help → render top-level usage and exit 0 WITHOUT running
|
||||
* the command. Both shapes satisfy the original invariant ("the
|
||||
* destructive command did not execute"); the new shape also restores
|
||||
* subcommand discoverability for `gsd-sdk query <subcommand> --help`.
|
||||
*
|
||||
* The tests therefore assert two things:
|
||||
* 1. The destructive command did NOT run (anti-hallucination invariant).
|
||||
* 2. The output contains the top-level usage (#3019 discoverability).
|
||||
*
|
||||
* --version remains rejected — it's never a valid gsd-tools flag and has
|
||||
* no discovery use-case.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { runGsdTools, createTempProject, cleanup, isUsageOutput } = require('./helpers.cjs');
|
||||
|
||||
describe('unknown flag guard (bug #1818)', () => {
|
||||
describe('unknown flag guard (bug #1818, updated for #3019)', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -24,51 +40,68 @@ describe('unknown flag guard (bug #1818)', () => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
// ── --help flag ────────────────────────────────────────────────────────────
|
||||
// ── --help renders usage and does NOT run the destructive command ────────
|
||||
|
||||
test('phases clear --help renders usage and does NOT clear phase dirs', () => {
|
||||
// Create a sentinel phase dir so we can assert it survives.
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', 'phase-99');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phaseDir, 'PLAN.md'), 'sentinel');
|
||||
|
||||
test('phases clear --help is rejected with non-zero exit', () => {
|
||||
const result = runGsdTools(['phases', 'clear', '--help'], tmpDir);
|
||||
assert.strictEqual(result.success, false, 'should fail, not run destructive clear');
|
||||
assert.match(result.error, /--help/);
|
||||
assert.strictEqual(result.success, true, 'help renders, no error exit');
|
||||
assert.ok(isUsageOutput(result.output), `expected top-level usage, got: ${result.output}`);
|
||||
// Anti-hallucination invariant: the destructive command did NOT run.
|
||||
assert.ok(fs.existsSync(phaseDir), 'phase dir must survive — clear must not have executed');
|
||||
assert.ok(fs.existsSync(path.join(phaseDir, 'PLAN.md')));
|
||||
});
|
||||
|
||||
test('generate-slug hello --help is rejected', () => {
|
||||
// Non-destructive baseline: generate-slug hello succeeds without --help
|
||||
test('generate-slug hello --help renders usage and does NOT emit a slug', () => {
|
||||
const ok = runGsdTools(['generate-slug', 'hello'], tmpDir);
|
||||
assert.strictEqual(ok.success, true, 'control: generate-slug without --help must succeed');
|
||||
assert.strictEqual(ok.success, true, 'control: generate-slug works without --help');
|
||||
// The control output is just the slug; the help output is the usage.
|
||||
const slugOut = ok.output;
|
||||
assert.ok(slugOut && !isUsageOutput(slugOut), `control should not be usage: ${slugOut}`);
|
||||
|
||||
const result = runGsdTools(['generate-slug', 'hello', '--help'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /--help/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output), 'help renders top-level usage');
|
||||
assert.notEqual(result.output, slugOut, 'help output must differ from the slug — generate-slug must not have run');
|
||||
});
|
||||
|
||||
test('phase complete --help is rejected', () => {
|
||||
test('phase complete --help renders usage and does NOT mark a phase complete', () => {
|
||||
const result = runGsdTools(['phase', 'complete', '--help'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /--help/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
// success:true + isUsageOutput is sufficient: if the destructive path
|
||||
// had executed it would have emitted a phase-resolution error to stderr
|
||||
// (success:false), not the usage to stdout (success:true).
|
||||
});
|
||||
|
||||
test('state load --help is rejected', () => {
|
||||
test('state load --help renders usage', () => {
|
||||
const result = runGsdTools(['state', 'load', '--help'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /--help/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
});
|
||||
|
||||
// ── -h shorthand ──────────────────────────────────────────────────────────
|
||||
// ── -h shorthand: same shape ─────────────────────────────────────────────
|
||||
|
||||
test('phases clear -h is rejected', () => {
|
||||
test('phases clear -h renders usage and does NOT clear phase dirs', () => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', 'phase-42');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
const result = runGsdTools(['phases', 'clear', '-h'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /-h/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
assert.ok(fs.existsSync(phaseDir), 'phase dir must survive');
|
||||
});
|
||||
|
||||
test('generate-slug hello -h is rejected', () => {
|
||||
test('generate-slug hello -h renders usage', () => {
|
||||
const result = runGsdTools(['generate-slug', 'hello', '-h'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /-h/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
});
|
||||
|
||||
// ── other common hallucinated flags ───────────────────────────────────────
|
||||
// ── --version is still rejected — no discovery use-case ──────────────────
|
||||
|
||||
test('generate-slug hello --version is rejected', () => {
|
||||
const result = runGsdTools(['generate-slug', 'hello', '--version'], tmpDir);
|
||||
@@ -76,9 +109,11 @@ describe('unknown flag guard (bug #1818)', () => {
|
||||
assert.match(result.error, /--version/);
|
||||
});
|
||||
|
||||
test('current-timestamp --help is rejected', () => {
|
||||
// ── current-timestamp --help: same as the others ─────────────────────────
|
||||
|
||||
test('current-timestamp --help renders usage', () => {
|
||||
const result = runGsdTools(['current-timestamp', '--help'], tmpDir);
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.match(result.error, /--help/);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
});
|
||||
});
|
||||
|
||||
120
tests/bug-3019-help-passthrough.test.cjs
Normal file
120
tests/bug-3019-help-passthrough.test.cjs
Normal file
@@ -0,0 +1,120 @@
|
||||
/**
|
||||
* Regression test for bug #3019.
|
||||
*
|
||||
* `gsd-sdk query <subcommand> --help` returned the top-level SDK USAGE
|
||||
* instead of contextual help for the subcommand. The query argv parser
|
||||
* harvested --help as a global flag and main() short-circuited dispatch
|
||||
* before the registry handler / gsd-tools.cjs fallback could render
|
||||
* useful help.
|
||||
*
|
||||
* Two-layer fix:
|
||||
* 1. sdk/src/cli.ts — leave --help in queryArgv so it travels to the
|
||||
* handler/fallback. Only honor the global help flag when there is
|
||||
* no subcommand to dispatch to.
|
||||
* 2. get-shit-done/bin/gsd-tools.cjs — render the top-level usage on
|
||||
* --help instead of erroring. Anti-hallucination invariant from
|
||||
* #1818 is preserved (the destructive command never executes).
|
||||
*
|
||||
* Tests the integration: invoke gsd-tools.cjs the same way the SDK
|
||||
* dispatcher does and assert structured-IR (success flag + usage shape)
|
||||
* rather than raw substring matches.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { runGsdTools, isUsageOutput } = require('./helpers.cjs');
|
||||
|
||||
// #3026 CR (Major outside-diff): the SDK fallback wraps gsd-tools.cjs.
|
||||
// When gsd-tools emits plain-text help (exit 0), the SDK previously
|
||||
// JSON.parsed stdout and threw "Unexpected token 'U'". Verify the fix
|
||||
// by invoking the built SDK end-to-end and asserting:
|
||||
// - exit 0
|
||||
// - stdout contains the gsd-tools usage
|
||||
// - stderr does NOT contain a JSON parse error
|
||||
const path = require('node:path');
|
||||
const { spawnSync } = require('node:child_process');
|
||||
const SDK_CLI = path.join(__dirname, '..', 'sdk', 'dist', 'cli.js');
|
||||
const fs = require('node:fs');
|
||||
|
||||
describe('bug #3026 (CR Major outside-diff): SDK forwards plain-text help from gsd-tools fallback', () => {
|
||||
test('gsd-sdk query phase --help (fallback path) returns usage, not a JSON parse error', (t) => {
|
||||
if (!fs.existsSync(SDK_CLI)) {
|
||||
// CR feedback (#3026): a bare `return` here silent-passes the test
|
||||
// when sdk/dist/cli.js is absent (CI checkouts that haven't run
|
||||
// `npm run build`), giving no signal that the integration check
|
||||
// was skipped. Use t.skip() so the omission is visible in the
|
||||
// test report. The unit-level fix is covered by vitest on
|
||||
// sdk/src/cli.ts; this integration test only runs when the
|
||||
// built SDK is on disk.
|
||||
t.skip('sdk/dist/cli.js not built — run `npm run build` in sdk/ to enable this integration test');
|
||||
return;
|
||||
}
|
||||
// `query phase --help` (no further subcommand) is NOT in the native
|
||||
// registry, so it routes through the gsd-tools.cjs fallback. That is
|
||||
// the path that JSON.parsed the help text and threw before this fix.
|
||||
const result = spawnSync(process.execPath, [SDK_CLI, 'query', 'phase', '--help'], {
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
timeout: 10000,
|
||||
});
|
||||
// The fallback gsd-tools.cjs emits exit 0 with usage on stdout.
|
||||
assert.strictEqual(result.status, 0,
|
||||
`must exit 0 — got ${result.status}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`);
|
||||
// Negative: must NOT see the JSON parse error that was the regression.
|
||||
assert.ok(!/Unexpected token|not valid JSON/i.test(result.stderr),
|
||||
`must NOT JSON.parse the help text (stderr): ${result.stderr}`);
|
||||
// Positive: the usage should reach the user via stdout.
|
||||
assert.ok(/Usage:\s*gsd-tools/.test(result.stdout) && /Commands:/.test(result.stdout),
|
||||
`usage must reach stdout: ${result.stdout}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bug #3019: gsd-tools renders usage on --help instead of erroring', () => {
|
||||
test('bare gsd-tools (no args) renders usage', () => {
|
||||
const result = runGsdTools([]);
|
||||
// No args path: error() helper emits to stderr and exits non-zero,
|
||||
// but the message body is the usage.
|
||||
assert.strictEqual(result.success, false);
|
||||
assert.ok(/Usage:\s*gsd-tools/.test(result.error));
|
||||
assert.ok(/Commands:/.test(result.error));
|
||||
});
|
||||
|
||||
test('gsd-tools --help renders usage on stdout, exits 0', () => {
|
||||
const result = runGsdTools(['--help']);
|
||||
assert.strictEqual(result.success, true, '--help should not be an error');
|
||||
assert.ok(isUsageOutput(result.output), `expected usage on stdout, got: ${result.output}`);
|
||||
});
|
||||
|
||||
test('gsd-tools -h renders usage on stdout, exits 0', () => {
|
||||
const result = runGsdTools(['-h']);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
});
|
||||
|
||||
test('gsd-tools <subcommand> --help renders usage (does not run subcommand)', () => {
|
||||
// The classic #3019 surface: the user types a subcommand expecting
|
||||
// contextual help. We render the top-level usage — strictly better
|
||||
// than the previous unhelpful "Unknown flag --help" error.
|
||||
const result = runGsdTools(['phase', 'add', '--help']);
|
||||
assert.strictEqual(result.success, true);
|
||||
assert.ok(isUsageOutput(result.output));
|
||||
});
|
||||
|
||||
test('usage hint mentions how to discover argument requirements', () => {
|
||||
// The usage now points users at the discovery method that actually works
|
||||
// (run without args → error message names required arguments). Asserting
|
||||
// on the parsed shape of the usage rather than substring-matching prose:
|
||||
const result = runGsdTools(['--help']);
|
||||
assert.strictEqual(result.success, true);
|
||||
// Structural check: split into sections.
|
||||
const lines = result.output.split('\n');
|
||||
const hasUsageLine = lines.some((l) => l.startsWith('Usage:'));
|
||||
const hasCommandsLine = lines.some((l) => l.startsWith('Commands:'));
|
||||
const hasDiscoveryHint = lines.some((l) => /argument requirements|without args|invoke the command/i.test(l));
|
||||
assert.ok(hasUsageLine, 'first section: Usage');
|
||||
assert.ok(hasCommandsLine, 'second section: Commands');
|
||||
assert.ok(hasDiscoveryHint, 'third section: how to discover per-command args');
|
||||
});
|
||||
});
|
||||
@@ -163,4 +163,11 @@ function parseFrontmatter(content) {
|
||||
return fields;
|
||||
}
|
||||
|
||||
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, TOOLS_PATH };
|
||||
// #3026 CR: shared `--help` output check used by bug-1818 + bug-3019 tests.
|
||||
// Render-on-help shape is `Usage: gsd-tools …\nCommands: …` — both lines
|
||||
// must be present; structural test, not prose substring matching.
|
||||
function isUsageOutput(text) {
|
||||
return /Usage:\s*gsd-tools/.test(text) && /Commands:/.test(text);
|
||||
}
|
||||
|
||||
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, isUsageOutput, TOOLS_PATH };
|
||||
|
||||
Reference in New Issue
Block a user