* test(#3884): failing-first coverage for strict argv and absence-signalling --pick ADR-3473 §8.4 says failure is a value. Three families currently encode failure as success, and this commit pins each one RED before the fix lands. Measured on this tree, 2026-08-26: gsd-tools generate-slug "test" --pick nonexistent -> empty stdout, exit 0 (#3365) gsd-tools audit-open --pick nonexistent_field -> dumps the entire human-readable audit report, exit 0 gsd-tools generate-slug "Hello World" --raw --pick bogus -> prints "hello-world", another field's value, exit 0 gsd-tools query state.planned-phase 3 (positional, no --phase) -> exit 0; STATE.md's "Phase: 2 of 5 (Widget Support)" is overwritten to "Phase: null - READY TO EXECUTE" and the frontmatter gains a corrupted current_phase_name (#3358) tests/pick-flag.test.cjs:27 previously asserted the #3365 defect as the contract ("returns empty string for missing field", success === true). That assertion is replaced by the required behavior rather than deleted. The new parseNamedArgs block calls the spec-object signature that does not exist yet, so it fails today by construction. The 11 existing behavior-lock tests are left untouched here; they are corrected in the implementation commit. C1/C4 assert at the consumer's output - STATE.md's bytes - per ADR-3180 Decision 4(b). A unit assertion on the parser would have passed throughout this defect's life. Design: .gsd/phase/feat-3884-failure-is-a-value/40-design.md Test matrix: .gsd/phase/feat-3884-failure-is-a-value/50-test-matrix.md Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * enhance(#3884): failure is a value — strict argv, and --pick that signals absence Implements ADR-3473 §8.4. Absence, emptiness and failure stop being interchangeable ways to say "I could not answer". parseNamedArgs (src/command-arg-projection.cts) Takes a spec object with a REQUIRED `positionals: number | 'rest'` and returns the hub's Result shape instead of a bare Record. Declaring the positional arity is what makes #3358's call site unrepresentable rather than merely detectable: an unrecognized flag or a token past the declared boundary is now InvalidArgs, naming the offending token and listing the accepted flags. The legacy positional-array call shape throws a TypeError — an internal invariant violation per ADR-3473 Decision 2, so a stale hand-written .cjs call site fails loudly instead of destructuring undefined off a Result. parseNamedArgsOrExit projects a failure onto the caller's error(); it is a projection over the one parser, not a second parser. Measured before, against a STATE.md with a populated phase-2 block: query state.planned-phase 3 (positional, no --phase) -> exit 0; "Phase: 2 of 5 (Widget Support)" overwritten to "Phase: null - READY TO EXECUTE", frontmatter gains a corrupted current_phase_name After: exit 1, `unexpected positional argument "3"`, STATE.md byte-identical. The flag form is unchanged and still updates STATE.md. --pick <field> (gsd-core/bin/gsd-tools.cjs) extractField returns {found,value}, and the pick block no longer shares one catch between "output was not JSON" and "field was absent". An absent field exits 1 with pick_field_absent, naming the field and the keys that do exist; non-JSON output exits 1 with pick_output_not_json instead of dumping the command's entire output. A field that is PRESENT with value null, '', 0 or false still prints at exit 0 — that is an answer, not a failure, and it is what keeps `--pick count` printing 0 on a fresh project. Measured before: `audit-open --pick nonexistent_field` printed the whole human-readable audit report at exit 0, and `generate-slug X --raw --pick bogus` printed "hello-world" — a different field's value, confidently, at exit 0. ADR-3409 Decision 7 explicitly deferred this contract fix to #3473; this is it. The sub-issue's "returns 0 when the count is zero OR absent" wording is superseded by the ADR rule it implements: zero prints 0, absence exits non-zero. Defaulting absence to 0 would demote "could not answer" to "the answer is zero" — the hazard docs/how-to/resolve-unreachable-guard-findings.md already warns against. Guard ledger (ADR-3473 Decision 6) scripts/lint-unreachable-guard-drift.cjs Detector A is RETIRED. Its premise — that a `--pick ... || echo` arm can never fire — is now false, so the shape it forbade is the correct idiom and keeping it would forbid the fix. Detector B (glob-consuming cat/ls, a nullglob mechanism this change does not touch) is retained in full, as are the shared scanner, the escape-marker parser and the baseline. Net: -1 detector, 0 added. The file is not deleted. Call-site audit 45 prompt-layer --pick invocations, every one a plain X=$(...) assignment — none in an if test, && chain, or a pipeline whose status is consumed, and no shell block in workflows/commands/agents/references sets -e. Of the 13 (command, field) pairs the prompt layer reads, 10 are always present; the 3 sometimes-absent ones each sit behind a prior found/existence check. No ADR-3409-class "field the command never produces" remains. Design: .gsd/phase/feat-3884-failure-is-a-value/40-design.md Test matrix: .gsd/phase/feat-3884-failure-is-a-value/50-test-matrix.md Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3884): escape untrusted tokens in diagnostics, and cover five unpinned rows Two review findings, both fixed here rather than recorded as limits. 1. A newline in an untrusted token forged a second stderr line. Before, plain-text mode: $ gsd-tools query state.planned-phase $'foo\nError: forged second line' Error: unexpected positional argument "foo Error: forged second line" After: Error: unexpected positional argument "foo\nError: forged second line" --json-errors mode was never affected — io.error runs that payload through JSON.stringify. Plain-text mode writes 'Error: ' + message verbatim, and the three new InvalidArgs reasons plus the two new --pick diagnostics all interpolate a token that comes straight from argv. Fixed with ONE shared helper, formatDiagnosticToken (src/io.cts), applied at every interpolation site — not a copy per site. It is deliberately NOT applied inside error() itself: several callers in this tree emit intentional multi-line diagnostics, and escaping newlines there would mangle them. The available-top-level-keys list needed the same treatment for a reason the review did not anticipate: `frontmatter get <file>` reads an ARBITRARY user document and echoes that document's own keys into the diagnostic. Verified reachable — a frontmatter key containing a newline reaches the key list — so formatKeyForDiagnosticList is guarding a live path, not a hypothetical one. Ordinary keys still render plain and unquoted; a fix that merely dropped the key would also have passed a "one line" assertion, so the test pins the escaped key's presence too. 2. Five behavior-table rows were implemented but nothing pinned them: B7 a dotted path that dies partway B9 bracket syntax on a non-array B10 a negative array index, in and out of range B14 a JSON root that is not an object B17 an @file: payload over 50KB B17 is the load-bearing one. output() writes @file:<path> instead of inline JSON past 50000 characters, and --pick resolves that BEFORE parsing; with no test, a future reordering of those two steps turns every large result into a false pick_output_not_json. The fixture seeds 1200 phase directories and measures the payload at 62474 characters, asserting the spill actually happened rather than assuming it. Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3884): correct the strict-argv surface against a full verification run The first full run came back with 90 failures across 12 files, none in the new tests. They were the argv surface telling me what it actually is. Ten root causes; each classified before anything was changed. I over-implemented, and that is reverted. ADR-3473 §8.4 says parseNamedArgs rejects "unrecognized and positional tokens". It says nothing about a value flag whose value is missing. Making that an error was my design decision, not the rule, and it broke a deliberately recorded contract: `--prd` with no value resolving to null (tests/init.test.cjs emptyPrdValueIsFalsyAndTreatedAsAbsent, row B5; tests/section-manifest-init-facts.test.cjs "flag-shaped value"). The "requires a value" branch is deleted outright rather than kept behind an option — an unused strictness mode is speculative generality. Unknown-flag and unexpected-positional rejection, which is what §8.4 actually mandates, is unchanged. --wave needed a third flag kind the original design did not anticipate. `--wave N` is documented (commands/gsd/execute-phase.md:4,48) and the shipped workflow reconstructs and passes it (execute-phase.md:84), while #2932 records token-PRESENCE semantics: the CLI cares only that the flag appeared, and the value belongs to the workflow layer. That is neither a boolean flag nor a value flag, so `optionalValueFlags` now exists — presence-only in `data`, and the validation cursor consumes a following non-flag token so it is not reported as a stray positional. Every other declared boolean flag was checked against every argument-hint and prose usage in commands/, workflows/, agents/ and docs/; `--wave` is the only one of this shape. Five tests were pinning forms that never worked. tests/adr857-core-without-capabilities.test.cjs passed `init plan-phase --phase 01-stub`, but the documented form is positional (docs/CLI-TOOLS.md:776) and the handler reads args[2] — which for that form is the literal string "--phase". Measured on the pre-fix build against a real .planning/phases/01-stub/ directory: init plan-phase 01-stub -> phase_found=true init plan-phase --phase 01-stub -> phase_found=false The test asserted only exit 0 and key presence, so it had been green while proving nothing about phase resolution. Corrected to the documented form and strengthened to assert phase_found === true. Same class in state.test.cjs (`--plan-count`, a flag that does not exist; the real one is `--plans`), milestone-archive.test.cjs (`init new-milestone --json`, silently ignored), and concurrency-safety.test.cjs (a bare positional field name whose OR-assertion passed because a whole-document dump happens to contain the substring it looked for). Six handlers had no argv validation at all — the same #3358 shape this phase exists to close, found while fixing the rest: init verify-work / phase-op / review / todos / remove-workspace read args[2] with nothing checking the rest, and validate health read --repair/--backfill through a bare args.includes() scan that bypassed the parser entirely. All now go through the seam, so the flag has one owner. tests/init-debug.test.cjs rows C4/C5 asserted that an unrecognized flag must NOT fail. That is the behavior §8.4 removes, and Decision 8 says a caller's local expectation does not override §8, so they are inverted and renamed — a test still called "ignores an unrecognized flag" while asserting rejection would be its own defect. Row C6's point is its PWNED canary; that assertion is kept verbatim and only its exit-status expectation changed, because the hostile token is now rejected rather than absorbed. The blast-radius estimate in 40-design.md is corrected rather than quietly left wrong. get_impact reported MEDIUM / 8 symbols upstream, and that was accurate for what the graph can see — parseNamedArgs's callers. It cannot see that those callers' handlers accept argv shapes wider than the code reading args[2] suggests, which is where the real surface was. Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3884): withdraw the validate-health tightening, finish the A2/A3 revert Second full run: 46 failures, down from 90. Four causes, two of them mine. Reverted `validate health` entirely — it was scope creep, and it broke a real flag. ~30 of the 46 read `unknown flag "--json"; accepted: --repair, --backfill`. The previous commit routed `validate health` through the parser on the reasoning that a flag should have one owner. That was wrong twice over: §8.4 names parseNamedArgs and count queries, and `validate health` was never a parseNamedArgs call site — it read its flags, just not through the parser, so it had no silent-drop defect to fix. Tightening it omitted `--json`, which the health-diagnostic suites use heavily. The handler is now byte-for-behaviour back to its pre-branch form. `validate context` stays converted: it genuinely was a call site, and its `--json` is now declared rather than read by a second `args.includes` scan. The five handlers that had NO validation at all — init verify-work / phase-op / review / todos / remove-workspace — stay fixed. Those read args[2] with nothing checking the rest, which is the #3358 shape this phase owns. Finished the A2/A3 revert. Three tests still encoded the deleted "a value flag with a missing value is an error" rule, including one added by the previous commit for that rule. All three now assert the reverted null contract, and the ones whose titles said "rejected" are renamed — a test named for a contract it no longer asserts is its own defect. `--wave=` and `--wave --weird` are correctly rejected. Neither is documented in commands/gsd/execute-phase.md, gsd-core/workflows/execute-phase.md or docs/, and neither is emitted by the shipped prompt layer, so both are unrecognized tokens that §8.4 mandates rejecting. `doesNotConsumeFollowingFlagAsWaveValue` keeps the property it exists for — asserted directly now, at the parser, that `--wave` does not swallow a following flag as its value — and only its exit-status expectation changed. A contradiction inside this branch, surfaced by the audit and resolved the safe way. Two pre-existing #3573 tests call `state begin-phase '2'` and `state planned-phase '2'` with a bare positional, relying on the old permissive parser to ignore it. This branch's own #3358 regression test requires that exact argv to be REJECTED. The two are mutually exclusive. Widening the router to accept a bare positional — mirroring complete-phase — would have silently re-opened #3358, and was verified to do exactly that: with the widened router, `query state.planned-phase 3` returned exit 0 and wrote current_phase_name again. It is reverted. docs/CLI-TOOLS.md:116 and docs/COMMANDS.md:2192 document only the `--phase N` form for both verbs, so the two #3573 tests move to it. Their assertions were never about the call shape — only that total_phases survives the resync — and both still pass. complete-phase is untouched: its bare positional IS documented, and it keeps the dynamic boundary and the negative-space note that record why. The audit that produced this is in the PR body: for every handler whose declaration changed, the flags it reads anywhere in its body, the flags the shipped surface documents, and the shapes the suite passes, compared. The `--json` miss was a pattern, not an accident — declaring a handler's flags from its parseNamedArgs call alone misses whatever it reads elsewhere. Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#3884): backfill the changeset PR number Refs #3884 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
638 lines
29 KiB
JavaScript
638 lines
29 KiB
JavaScript
'use strict';
|
|
|
|
const { describe, test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const path = require('node:path');
|
|
const fs = require('node:fs');
|
|
|
|
const { ExitError, runMain } = require('../scripts/lib/cli-exit.cjs');
|
|
const { runNode } = require('./helpers/process-seam.cjs');
|
|
const { toLegacyResult } = require('./helpers/git-fixture.cjs');
|
|
const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
|
const { createTempDir, cleanup } = require('./helpers.cjs');
|
|
|
|
// Paths to the compiled product seam (src/cli-exit.cts → gsd-core/bin/lib/cli-exit.cjs)
|
|
// used for json-error mode regression tests which require io.cjs integration.
|
|
const BUILT_CLI_EXIT_PATH = path.resolve(__dirname, '../gsd-core/bin/lib/cli-exit.cjs');
|
|
const IO_PATH = path.resolve(__dirname, '../gsd-core/bin/lib/io.cjs');
|
|
const SCRIPTS_CLI_EXIT_PATH = path.resolve(__dirname, '../scripts/lib/cli-exit.cjs');
|
|
|
|
/** Settle the runMain promise chain before asserting. */
|
|
async function settle() {
|
|
await new Promise((r) => setImmediate(r));
|
|
}
|
|
|
|
describe('ExitError', () => {
|
|
test('default code is 1', () => {
|
|
const err = new ExitError();
|
|
assert.equal(err.code, 1);
|
|
});
|
|
|
|
test('name is ExitError', () => {
|
|
const err = new ExitError();
|
|
assert.equal(err.name, 'ExitError');
|
|
});
|
|
|
|
test('instanceof Error', () => {
|
|
assert.ok(new ExitError() instanceof Error);
|
|
});
|
|
|
|
test('hasUserMessage is false when no message passed', () => {
|
|
const err = new ExitError(1);
|
|
assert.equal(err.hasUserMessage, false);
|
|
});
|
|
|
|
test('hasUserMessage is true when message passed', () => {
|
|
const err = new ExitError(1, 'something went wrong');
|
|
assert.equal(err.hasUserMessage, true);
|
|
});
|
|
|
|
test('custom code is preserved', () => {
|
|
const err = new ExitError(42, 'boom');
|
|
assert.equal(err.code, 42);
|
|
});
|
|
|
|
test('message is set to user message when provided', () => {
|
|
const err = new ExitError(2, 'user msg');
|
|
assert.equal(err.message, 'user msg');
|
|
});
|
|
|
|
test('message is synthetic when no message provided', () => {
|
|
const err = new ExitError(3);
|
|
assert.equal(err.message, 'process exit 3');
|
|
});
|
|
});
|
|
|
|
describe('runMain', () => {
|
|
test('main returns a number sets process.exitCode', async () => {
|
|
const saved = process.exitCode;
|
|
try {
|
|
runMain(() => 42);
|
|
await settle();
|
|
assert.equal(process.exitCode, 42);
|
|
} finally {
|
|
process.exitCode = saved || 0;
|
|
}
|
|
});
|
|
|
|
test('main returns undefined leaves process.exitCode unchanged', async () => {
|
|
const saved = process.exitCode;
|
|
// Set a known value before calling
|
|
process.exitCode = 0;
|
|
try {
|
|
runMain(() => undefined);
|
|
await settle();
|
|
assert.equal(process.exitCode, 0);
|
|
} finally {
|
|
process.exitCode = saved || 0;
|
|
}
|
|
});
|
|
|
|
test('main throws ExitError sets process.exitCode to err.code', async () => {
|
|
const saved = process.exitCode;
|
|
try {
|
|
runMain(() => { throw new ExitError(2); });
|
|
await settle();
|
|
assert.equal(process.exitCode, 2);
|
|
} finally {
|
|
process.exitCode = saved || 0;
|
|
}
|
|
});
|
|
|
|
test('main rejects async ExitError(0) sets process.exitCode to 0', async () => {
|
|
const saved = process.exitCode;
|
|
try {
|
|
runMain(async () => { throw new ExitError(0); });
|
|
await settle();
|
|
assert.equal(process.exitCode, 0);
|
|
} finally {
|
|
process.exitCode = saved !== undefined ? saved : 0;
|
|
}
|
|
});
|
|
|
|
test('main throws generic Error sets process.exitCode to 1 and writes stderr', async () => {
|
|
const saved = process.exitCode;
|
|
const stderrChunks = [];
|
|
const origWrite = process.stderr.write.bind(process.stderr);
|
|
process.stderr.write = (chunk, ...args) => {
|
|
stderrChunks.push(typeof chunk === 'string' ? chunk : chunk.toString());
|
|
return origWrite(chunk, ...args);
|
|
};
|
|
try {
|
|
runMain(() => { throw new Error('kaboom'); });
|
|
await settle();
|
|
assert.equal(process.exitCode, 1);
|
|
const combined = stderrChunks.join('');
|
|
assert.ok(combined.includes('kaboom'), `expected "kaboom" in stderr: ${combined}`);
|
|
} finally {
|
|
process.stderr.write = origWrite;
|
|
process.exitCode = saved || 0;
|
|
}
|
|
});
|
|
|
|
test('ExitError with hasUserMessage and non-zero code writes to stderr', async () => {
|
|
const saved = process.exitCode;
|
|
const stderrChunks = [];
|
|
const origWrite = process.stderr.write.bind(process.stderr);
|
|
process.stderr.write = (chunk, ...args) => {
|
|
stderrChunks.push(typeof chunk === 'string' ? chunk : chunk.toString());
|
|
return origWrite(chunk, ...args);
|
|
};
|
|
try {
|
|
runMain(() => { throw new ExitError(1, 'user-visible error'); });
|
|
await settle();
|
|
assert.equal(process.exitCode, 1);
|
|
const combined = stderrChunks.join('');
|
|
assert.ok(combined.includes('user-visible error'), `expected message in stderr: ${combined}`);
|
|
} finally {
|
|
process.stderr.write = origWrite;
|
|
process.exitCode = saved || 0;
|
|
}
|
|
});
|
|
|
|
test('ExitError with hasUserMessage and code 0 does NOT write to stderr', async () => {
|
|
const saved = process.exitCode;
|
|
const stderrChunks = [];
|
|
const origWrite = process.stderr.write.bind(process.stderr);
|
|
process.stderr.write = (chunk, ...args) => {
|
|
stderrChunks.push(typeof chunk === 'string' ? chunk : chunk.toString());
|
|
return origWrite(chunk, ...args);
|
|
};
|
|
try {
|
|
runMain(() => { throw new ExitError(0, 'silent success'); });
|
|
await settle();
|
|
assert.equal(process.exitCode, 0);
|
|
const combined = stderrChunks.join('');
|
|
assert.equal(combined.includes('silent success'), false,
|
|
`did not expect message in stderr: ${combined}`);
|
|
} finally {
|
|
process.stderr.write = origWrite;
|
|
process.exitCode = saved !== undefined ? saved : 0;
|
|
}
|
|
});
|
|
});
|
|
|
|
// ─── Regressions ─────────────────────────────────────────────────────────────
|
|
|
|
/**
|
|
* bug #965 — runMain unexpected throw with --json-errors active emitted a raw
|
|
* stack trace instead of a structured { ok:false, reason, message } envelope.
|
|
* SDK consumers parsing structured errors would receive an unparseable string.
|
|
*
|
|
* Fix: src/cli-exit.cts non-ExitError catch branch now checks getJsonErrorMode()
|
|
* and emits the same structured envelope as error() when active.
|
|
*
|
|
* Tests run against the compiled product seam (gsd-core/bin/lib/cli-exit.cjs)
|
|
* via subprocess so that io.cjs module-level state is isolated per spawn.
|
|
*/
|
|
describe('regressions', () => {
|
|
/** Spawn a one-shot script that sets json-error mode and calls runMain with a throwing handler. */
|
|
function spawnJsonErrorRun({ jsonMode, errorType = 'TypeError', message = 'unexpected boom' } = {}) {
|
|
// ExitError lives in the same module as runMain; import it when the test
|
|
// wants to exercise the ExitError carve-out path. ExitError takes (code, message).
|
|
const isExitError = errorType === 'ExitError';
|
|
const destructure = isExitError ? '{ runMain, ExitError }' : '{ runMain }';
|
|
const throwExpr = isExitError
|
|
? `new ExitError(1, ${JSON.stringify(message)})`
|
|
: `new ${errorType}(${JSON.stringify(message)})`;
|
|
const script = `
|
|
const io = require(${JSON.stringify(IO_PATH)});
|
|
const ${destructure} = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
|
|
io.setJsonErrorMode(${jsonMode ? 'true' : 'false'});
|
|
runMain(() => { throw ${throwExpr}; });
|
|
setImmediate(() => {});
|
|
`;
|
|
return toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
|
}
|
|
|
|
describe('bug-965: unexpected throw in json-error mode emits structured envelope', () => {
|
|
test('stderr is a single parseable JSON object (not a raw stack trace)', () => {
|
|
const result = spawnJsonErrorRun({ jsonMode: true });
|
|
assert.strictEqual(result.status, 1,
|
|
`expected exit code 1, got ${result.status}; stderr: ${result.stderr}`);
|
|
const stderrTrimmed = result.stderr.trim();
|
|
assert.ok(stderrTrimmed.length > 0, 'expected non-empty stderr');
|
|
let parsed;
|
|
try {
|
|
parsed = JSON.parse(stderrTrimmed);
|
|
} catch (e) {
|
|
assert.fail(
|
|
`stderr is NOT valid JSON (raw stack trace leaked through):\n${stderrTrimmed}\nparse error: ${e.message}`
|
|
);
|
|
}
|
|
assert.strictEqual(parsed.ok, false, `expected ok:false, got: ${JSON.stringify(parsed)}`);
|
|
assert.strictEqual(parsed.reason, 'sdk_fail_fast',
|
|
`expected reason "sdk_fail_fast", got: ${parsed.reason}`);
|
|
assert.ok(
|
|
parsed.message && parsed.message.includes('unexpected boom'),
|
|
`expected message to include "unexpected boom", got: ${JSON.stringify(parsed.message)}`
|
|
);
|
|
});
|
|
|
|
test('stderr JSON works for RangeError as well as TypeError', () => {
|
|
const result = spawnJsonErrorRun({ jsonMode: true, errorType: 'RangeError', message: 'out of bounds' });
|
|
assert.strictEqual(result.status, 1);
|
|
const parsed = JSON.parse(result.stderr.trim());
|
|
assert.strictEqual(parsed.ok, false);
|
|
assert.strictEqual(parsed.reason, 'sdk_fail_fast');
|
|
assert.ok(parsed.message.includes('out of bounds'));
|
|
});
|
|
|
|
test('stdout is empty when unexpected throw emits structured error', () => {
|
|
const result = spawnJsonErrorRun({ jsonMode: true });
|
|
assert.strictEqual(result.stdout, '',
|
|
`expected empty stdout, got: ${result.stdout}`);
|
|
});
|
|
|
|
test('plain mode (json-error off) preserves raw stack trace on stderr', () => {
|
|
const result = spawnJsonErrorRun({ jsonMode: false });
|
|
assert.strictEqual(result.status, 1);
|
|
const stderrTrimmed = result.stderr.trim();
|
|
let parsed = null;
|
|
try { parsed = JSON.parse(stderrTrimmed); } catch { /* expected — not JSON */ }
|
|
assert.strictEqual(parsed, null,
|
|
`expected raw stack (non-JSON) on stderr in plain mode, but got valid JSON: ${stderrTrimmed.slice(0, 200)}`);
|
|
assert.ok(
|
|
stderrTrimmed.includes('unexpected boom'),
|
|
`expected "unexpected boom" in stderr, got: ${stderrTrimmed.slice(0, 200)}`
|
|
);
|
|
});
|
|
|
|
// #2979: characterization test pinning the two error paths under json-errors
|
|
// mode. The structured envelope covers non-ExitError failures; ExitError
|
|
// (usage errors) intentionally emits plain text with its own exit code.
|
|
// Both halves asserted together so the code cannot drift toward the doc's
|
|
// prior overstated claim that EVERY error emits JSON.
|
|
test('#2979: ExitError emits plain text (not JSON) even under --json-errors; non-ExitError emits the envelope', () => {
|
|
// ExitError path: plain text, own exit code, NOT a JSON object.
|
|
const exitResult = spawnJsonErrorRun({
|
|
jsonMode: true,
|
|
errorType: 'ExitError',
|
|
message: 'Usage: gsd-tools <command> [args]',
|
|
});
|
|
assert.strictEqual(exitResult.status, 1, 'ExitError exits with its code');
|
|
const exitStderr = exitResult.stderr.trim();
|
|
let exitParsed = null;
|
|
try { exitParsed = JSON.parse(exitStderr); } catch { /* expected — plain text */ }
|
|
assert.strictEqual(exitParsed, null,
|
|
`ExitError must emit plain text, not JSON; got: ${exitStderr.slice(0, 200)}`);
|
|
assert.ok(exitStderr.includes('Usage'),
|
|
`ExitError plain-text message must reach stderr; got: ${exitStderr.slice(0, 200)}`);
|
|
|
|
// Non-ExitError path: structured JSON envelope.
|
|
const envResult = spawnJsonErrorRun({ jsonMode: true });
|
|
assert.strictEqual(envResult.status, 1);
|
|
const envParsed = JSON.parse(envResult.stderr.trim());
|
|
assert.strictEqual(envParsed.ok, false);
|
|
assert.strictEqual(envParsed.reason, 'sdk_fail_fast');
|
|
assert.ok(envParsed.message, 'envelope must carry a message');
|
|
});
|
|
});
|
|
|
|
/**
|
|
* #3904 (epic #3889, ADR-3889 P0) — scripts/lib/cli-exit.cjs was a SECOND
|
|
* hand-written implementation of this seam, and it had no json-error arm at
|
|
* all: an unexpected throw printed a raw stack trace where the documented
|
|
* contract promises { ok:false, reason, message }. 64+ files under scripts/
|
|
* require that copy.
|
|
*
|
|
* Fix: scripts/lib/cli-exit.cjs is now GENERATED from src/cli-exit.cts's
|
|
* compiled output and byte-compared by scripts/gen-scripts-cli-exit.cjs
|
|
* --check, so the two cannot diverge again.
|
|
*
|
|
* These run against the SCRIPTS copy specifically — the sibling bug-965 block
|
|
* above deliberately targets the built copy, which is exactly how the drift
|
|
* stayed invisible.
|
|
*/
|
|
describe('bug-3904: the scripts copy is the same artifact as the built one', () => {
|
|
/** Build a one-shot driver script for whichever copy is under test. */
|
|
function driver(modulePath, { jsonMode, throwExpr }) {
|
|
return [
|
|
`const cliExit = require(${JSON.stringify(modulePath)});`,
|
|
`const { runMain, ExitError } = cliExit;`,
|
|
`void ExitError;`,
|
|
`cliExit.setJsonErrorMode(${jsonMode});`,
|
|
`runMain(() => { throw ${throwExpr}; });`,
|
|
`setImmediate(() => {});`,
|
|
].join('\n');
|
|
}
|
|
|
|
/**
|
|
* Drive the SCRIPTS copy. json-error mode is set through the scripts copy's
|
|
* own accessor, because a scripts/ consumer on an unbuilt clone has no
|
|
* io.cjs to reach for — that independence is part of what is under test.
|
|
*/
|
|
function spawnScriptsRun(opts) {
|
|
return toLegacyResult(
|
|
runNode(['-e', driver(SCRIPTS_CLI_EXIT_PATH, opts)], { timeoutMs: PROBE_TIMEOUT_MS }),
|
|
);
|
|
}
|
|
|
|
/** The same driver, pointed at the BUILT copy, for the parity row. */
|
|
function spawnBuiltRun(opts) {
|
|
return toLegacyResult(
|
|
runNode(['-e', driver(BUILT_CLI_EXIT_PATH, opts)], { timeoutMs: PROBE_TIMEOUT_MS }),
|
|
);
|
|
}
|
|
|
|
/** Run a snippet that prints JSON on stdout, and return the parsed value. */
|
|
function readJsonFromChild(lines) {
|
|
const r = toLegacyResult(runNode(['-e', lines.join('\n')], { timeoutMs: PROBE_TIMEOUT_MS }));
|
|
assert.strictEqual(r.status, 0, `child exited ${r.status}; stderr: ${r.stderr}`);
|
|
return JSON.parse(r.stdout);
|
|
}
|
|
|
|
/** Parse stderr as a single JSON object, failing with the raw text if it is not one. */
|
|
function parseEnvelope(result) {
|
|
const trimmed = result.stderr.trim();
|
|
try {
|
|
return JSON.parse(trimmed);
|
|
} catch (e) {
|
|
return assert.fail(
|
|
`stderr is NOT a single JSON object (raw stack leaked through):\n${trimmed}\nparse error: ${e.message}`,
|
|
);
|
|
}
|
|
}
|
|
|
|
// ── Matrix rows 1-3: the reported defect, at the consumer's output ────────
|
|
test('scripts copy emits the structured envelope on an unexpected throw under json mode', () => {
|
|
const result = spawnScriptsRun({ jsonMode: true, throwExpr: `new TypeError('unexpected boom')` });
|
|
assert.strictEqual(result.status, 1, `expected exit 1; stderr: ${result.stderr}`);
|
|
const parsed = parseEnvelope(result);
|
|
assert.strictEqual(parsed.ok, false);
|
|
assert.strictEqual(parsed.reason, 'sdk_fail_fast');
|
|
assert.ok(
|
|
String(parsed.message).includes('unexpected boom'),
|
|
`expected the thrown text in message, got: ${JSON.stringify(parsed.message)}`,
|
|
);
|
|
});
|
|
|
|
test('scripts copy envelope covers RangeError as well as TypeError', () => {
|
|
const result = spawnScriptsRun({ jsonMode: true, throwExpr: `new RangeError('out of bounds')` });
|
|
assert.strictEqual(result.status, 1);
|
|
const parsed = parseEnvelope(result);
|
|
assert.strictEqual(parsed.reason, 'sdk_fail_fast');
|
|
assert.ok(String(parsed.message).includes('out of bounds'));
|
|
});
|
|
|
|
test('scripts copy writes the envelope to stderr and leaves stdout empty', () => {
|
|
const result = spawnScriptsRun({ jsonMode: true, throwExpr: `new TypeError('boom')` });
|
|
assert.strictEqual(result.stdout, '', `expected empty stdout, got: ${result.stdout}`);
|
|
});
|
|
|
|
// ── Matrix rows 4-5: negative space — what must NOT become an envelope ────
|
|
test('scripts copy preserves the raw stack trace when json mode is off', () => {
|
|
const result = spawnScriptsRun({ jsonMode: false, throwExpr: `new TypeError('unexpected boom')` });
|
|
assert.strictEqual(result.status, 1);
|
|
const trimmed = result.stderr.trim();
|
|
let parsed = null;
|
|
try { parsed = JSON.parse(trimmed); } catch { /* expected — not JSON */ }
|
|
assert.strictEqual(parsed, null, `expected a raw stack in plain mode, got JSON: ${trimmed.slice(0, 200)}`);
|
|
assert.ok(trimmed.includes('unexpected boom'), `expected the thrown text; got: ${trimmed.slice(0, 200)}`);
|
|
});
|
|
|
|
test('scripts copy keeps ExitError plain-text under json mode', () => {
|
|
const result = spawnScriptsRun({
|
|
jsonMode: true,
|
|
throwExpr: `new ExitError(1, 'Usage: gsd-tools <command> [args]')`,
|
|
});
|
|
assert.strictEqual(result.status, 1, 'ExitError exits with its own code');
|
|
const trimmed = result.stderr.trim();
|
|
let parsed = null;
|
|
try { parsed = JSON.parse(trimmed); } catch { /* expected — plain text */ }
|
|
assert.strictEqual(parsed, null, `ExitError must stay plain text; got JSON: ${trimmed.slice(0, 200)}`);
|
|
assert.ok(trimmed.includes('Usage'), `plain-text message must reach stderr; got: ${trimmed.slice(0, 200)}`);
|
|
});
|
|
|
|
// ── Matrix rows 6-10: non-Error throws reach String(err) ─────────────────
|
|
for (const [label, throwExpr, expectedMessage] of [
|
|
['a thrown string', `'a bare string'`, 'a bare string'],
|
|
['a thrown null', `null`, 'null'],
|
|
['a thrown undefined', `undefined`, 'undefined'],
|
|
['an Error with an empty message', `new Error('')`, 'Error'],
|
|
]) {
|
|
test(`scripts copy envelope handles ${label}`, () => {
|
|
const result = spawnScriptsRun({ jsonMode: true, throwExpr });
|
|
assert.strictEqual(result.status, 1, `expected exit 1; stderr: ${result.stderr}`);
|
|
const parsed = parseEnvelope(result);
|
|
assert.strictEqual(parsed.ok, false);
|
|
assert.strictEqual(parsed.reason, 'sdk_fail_fast');
|
|
assert.ok(
|
|
String(parsed.message).includes(expectedMessage),
|
|
`expected ${JSON.stringify(expectedMessage)} in message, got ${JSON.stringify(parsed.message)}`,
|
|
);
|
|
});
|
|
}
|
|
|
|
test('scripts copy envelope stays parseable when the message contains quotes and newlines', () => {
|
|
// Proves JSON.stringify is doing the encoding rather than string concatenation:
|
|
// an unescaped quote or newline would split stderr into something JSON.parse rejects.
|
|
const hostile = 'he said "hi"\nthen \\left\ttab';
|
|
const result = spawnScriptsRun({ jsonMode: true, throwExpr: `new Error(${JSON.stringify(hostile)})` });
|
|
assert.strictEqual(result.status, 1);
|
|
const parsed = parseEnvelope(result);
|
|
assert.strictEqual(parsed.message, hostile, 'the message must round-trip byte-for-byte');
|
|
});
|
|
|
|
// ── Matrix row 11: the two copies are one artifact ───────────────────────
|
|
// Stack-trace bytes are NOT the contract here: on the json=false path, stderr
|
|
// is a raw stack trace, and the generated scripts/ copy carries an 11-line
|
|
// provenance banner that the built copy does not, so every frame line number
|
|
// is offset by exactly that banner length, and the two files necessarily sit
|
|
// at different absolute paths. The two copies share one compiled BODY —
|
|
// the banner is the only difference — so what actually must match is the
|
|
// VERDICT: same exit code, and (json mode) the same structured envelope, or
|
|
// (plain-text mode) the same unqualified error header line with no path or
|
|
// line number in it.
|
|
test('the built copy and the scripts copy produce identical verdicts for every throw class', () => {
|
|
const cases = [
|
|
{ jsonMode: true, throwExpr: `new TypeError('same boom')`, compare: 'json' },
|
|
{ jsonMode: false, throwExpr: `new TypeError('same boom')`, compare: 'firstLine' },
|
|
{ jsonMode: true, throwExpr: `new ExitError(3, 'same usage')`, compare: 'exact' },
|
|
];
|
|
for (const c of cases) {
|
|
const fromScripts = spawnScriptsRun(c);
|
|
const fromBuilt = spawnBuiltRun(c);
|
|
assert.strictEqual(
|
|
fromScripts.status, fromBuilt.status,
|
|
`exit status must match for ${c.throwExpr} (json=${c.jsonMode})`,
|
|
);
|
|
const label = `${c.throwExpr} (json=${c.jsonMode})`;
|
|
if (c.compare === 'json') {
|
|
// Structured output: parse both and compare the resulting objects.
|
|
assert.deepStrictEqual(
|
|
parseEnvelope(fromScripts), parseEnvelope(fromBuilt),
|
|
`parsed envelopes must match for ${label}`,
|
|
);
|
|
} else if (c.compare === 'firstLine') {
|
|
// Plain-text stack trace: only the header line (e.g. "TypeError: same
|
|
// boom") is path/line-number-free and therefore comparable; the frame
|
|
// lines below it are expected to diverge per the banner offset above.
|
|
for (const r of [fromScripts, fromBuilt]) {
|
|
assert.throws(() => JSON.parse(r.stderr.trim()), `stderr for ${label} must NOT be JSON`);
|
|
}
|
|
const firstLine = (s) => s.trim().split('\n')[0];
|
|
assert.strictEqual(
|
|
firstLine(fromScripts.stderr), firstLine(fromBuilt.stderr),
|
|
`stderr first line must match for ${label}`,
|
|
);
|
|
} else {
|
|
// ExitError: plain prose with no stack trace, so it is byte-identical.
|
|
assert.strictEqual(
|
|
fromScripts.stderr.trim(), fromBuilt.stderr.trim(),
|
|
`stderr must match for ${label}`,
|
|
);
|
|
}
|
|
}
|
|
});
|
|
|
|
// ── Matrix rows 13-15: ONE json-error-mode cell, not two ─────────────────
|
|
// This is the hazard the fix INTRODUCES and must therefore be tested rather
|
|
// than reasoned about: after generation there are two module instances of
|
|
// the same artifact, and a module-level `let` would give them two flags.
|
|
test('the mode set through io is visible through the scripts copy', () => {
|
|
assert.deepStrictEqual(
|
|
readJsonFromChild([
|
|
`const io = require(${JSON.stringify(IO_PATH)});`,
|
|
`const cliExit = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
|
`io.setJsonErrorMode(true);`,
|
|
`process.stdout.write(JSON.stringify({ viaCliExit: cliExit.getJsonErrorMode() }));`,
|
|
]),
|
|
{ viaCliExit: true },
|
|
);
|
|
});
|
|
|
|
test('the mode set through the scripts copy is visible through io', () => {
|
|
assert.deepStrictEqual(
|
|
readJsonFromChild([
|
|
`const io = require(${JSON.stringify(IO_PATH)});`,
|
|
`const cliExit = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
|
`cliExit.setJsonErrorMode(true);`,
|
|
`process.stdout.write(JSON.stringify({ viaIo: io.getJsonErrorMode() }));`,
|
|
]),
|
|
{ viaIo: true },
|
|
);
|
|
});
|
|
|
|
test('both copies of the exit module share one json-error-mode cell', () => {
|
|
assert.deepStrictEqual(
|
|
readJsonFromChild([
|
|
`const io = require(${JSON.stringify(IO_PATH)});`,
|
|
`const built = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
|
`const scripts = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
|
// Two distinct module instances of the same artifact.
|
|
`if (built === scripts) throw new Error('expected two distinct module instances');`,
|
|
`io.setJsonErrorMode(true);`,
|
|
`process.stdout.write(JSON.stringify({`,
|
|
` built: built.getJsonErrorMode(),`,
|
|
` scripts: scripts.getJsonErrorMode(),`,
|
|
` io: io.getJsonErrorMode(),`,
|
|
`}));`,
|
|
]),
|
|
{ built: true, scripts: true, io: true },
|
|
'all three views must read one cell — two module-level flags would diverge here',
|
|
);
|
|
});
|
|
|
|
// ── Matrix rows 16-17: coercion and default, preserved exactly ───────────
|
|
test('setJsonErrorMode keeps its truthiness coercion', () => {
|
|
const seen = readJsonFromChild([
|
|
`const c = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
|
`const seen = [];`,
|
|
`for (const v of [0, '', 'false', null, undefined, 1, 'x']) {`,
|
|
` c.setJsonErrorMode(v); seen.push(c.getJsonErrorMode());`,
|
|
`}`,
|
|
`process.stdout.write(JSON.stringify(seen));`,
|
|
]);
|
|
// `!!v` — note 'false' is a NON-EMPTY string and is therefore true.
|
|
assert.deepStrictEqual(seen, [false, false, true, false, false, true, true]);
|
|
});
|
|
|
|
test('json-error mode defaults to false when never set', () => {
|
|
assert.deepStrictEqual(
|
|
readJsonFromChild([
|
|
`const c = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
|
`const v = c.getJsonErrorMode();`,
|
|
`process.stdout.write(JSON.stringify({ v, type: typeof v }));`,
|
|
]),
|
|
{ v: false, type: 'boolean' },
|
|
'an unset cell must read as boolean false, never undefined',
|
|
);
|
|
});
|
|
|
|
// ── Matrix rows 18-21: io's export surface must not move (Hyrum) ─────────
|
|
test('io still exports both json-error-mode accessors and an unchanged ERROR_REASON', () => {
|
|
const seen = readJsonFromChild([
|
|
`const io = require(${JSON.stringify(IO_PATH)});`,
|
|
`process.stdout.write(JSON.stringify({`,
|
|
` setter: typeof io.setJsonErrorMode,`,
|
|
` getter: typeof io.getJsonErrorMode,`,
|
|
` failFast: io.ERROR_REASON.SDK_FAIL_FAST,`,
|
|
` frozen: Object.isFrozen(io.ERROR_REASON),`,
|
|
` reasonCount: Object.keys(io.ERROR_REASON).length,`,
|
|
` keys: Object.keys(io.ERROR_REASON).sort(),`,
|
|
`}));`,
|
|
]);
|
|
assert.strictEqual(seen.setter, 'function');
|
|
assert.strictEqual(seen.getter, 'function');
|
|
assert.strictEqual(seen.failFast, 'sdk_fail_fast', 'the literal must survive moving to cli-exit');
|
|
assert.strictEqual(seen.frozen, true);
|
|
// #3884 (ADR-3473 §8.4) legitimately added two new codes —
|
|
// PICK_FIELD_ABSENT and PICK_OUTPUT_NOT_JSON — for the `--pick`
|
|
// absence contract (see .gsd/phase/feat-3884-failure-is-a-value/40-design.md
|
|
// rows B6/B11). 23 -> 25 is an intentional, documented growth of the
|
|
// enum, not drift; bump the golden count rather than treat this as a
|
|
// Hyrum violation.
|
|
assert.strictEqual(seen.reasonCount, 25, 'ERROR_REASON must keep all 25 members (23 + #3884 PICK_FIELD_ABSENT/PICK_OUTPUT_NOT_JSON)');
|
|
assert.ok(
|
|
seen.keys.includes('SDK_FAIL_FAST'),
|
|
`ERROR_REASON must still include SDK_FAIL_FAST, got: ${JSON.stringify(seen.keys)}`,
|
|
);
|
|
});
|
|
|
|
// ── Matrix rows 22-23: the unbuilt-clone constraint ──────────────────────
|
|
test('the scripts copy loads with no gsd-core tree in scope at all', (t) => {
|
|
// The generated file is COMMITTED and 64+ scripts/ consumers require it,
|
|
// including scripts/check-env.cjs which runs before any build. It must
|
|
// therefore not reach into gsd-core/bin/lib/, which is gitignored tsc
|
|
// output absent on a fresh clone.
|
|
//
|
|
// Proven by copying the file into an isolated temp directory that has no
|
|
// gsd-core sibling and no node_modules — a require of the built tree is
|
|
// MODULE_NOT_FOUND there. Deliberately NOT done by renaming the real
|
|
// gsd-core/bin/lib: test files run in parallel, so mutating a shared
|
|
// production directory would break every sibling suite mid-run.
|
|
//
|
|
// This is the sole guard of the "depends on node: builtins only"
|
|
// constraint: it proves the property by real module resolution in an
|
|
// isolated directory, rather than by inspecting require() specifiers.
|
|
const dir = createTempDir('gsd-3904-standalone-');
|
|
t.after(() => cleanup(dir));
|
|
const copied = path.join(dir, 'cli-exit.cjs');
|
|
fs.copyFileSync(SCRIPTS_CLI_EXIT_PATH, copied);
|
|
|
|
const r = toLegacyResult(runNode(['-e', [
|
|
`const c = require(${JSON.stringify(copied)});`,
|
|
`c.setJsonErrorMode(true);`,
|
|
`c.runMain(() => { throw new TypeError('still works'); });`,
|
|
`setImmediate(() => {});`,
|
|
].join('\n')], { cwd: dir, timeoutMs: PROBE_TIMEOUT_MS }));
|
|
|
|
assert.ok(
|
|
!r.stderr.includes('MODULE_NOT_FOUND'),
|
|
`the scripts copy must not require anything outside node: builtins; got: ${r.stderr.slice(0, 400)}`,
|
|
);
|
|
assert.strictEqual(r.status, 1, `expected exit 1; stderr: ${r.stderr}`);
|
|
assert.strictEqual(JSON.parse(r.stderr.trim()).reason, 'sdk_fail_fast');
|
|
});
|
|
|
|
test('the build sentinel is still emitted', () => {
|
|
// gsd-core/bin/ensure-runtime-build.cjs keys isBuilt() on this exact filename.
|
|
assert.ok(
|
|
fs.statSync(BUILT_CLI_EXIT_PATH).isFile(),
|
|
'gsd-core/bin/lib/cli-exit.cjs must remain tsc output — it is the build sentinel',
|
|
);
|
|
});
|
|
});
|
|
});
|