* enhance(#3912): gsd-tools declares outcomes, pinned at v1 ADR-3889 §4. Phase 6 already moved error()'s terminator onto the seam, so what remained was the declaration — and the pin that makes it invisible today. The census corrected two documented figures before any code changed. ERROR_REASON has exactly 25 members (the ADR and epic were right; an earlier note of mine claiming 23 was wrong and is corrected). And output({error}) is **64 sites across 9 files, not the 60 ADR-2980 ratified** — the module shape holds but the total drifted +4: frontmatter 7 not 6, phase 4 not 2, roadmap 3 not 2. That matters because this phase's criterion demands the pin be asserted over the enumerated population rather than sampled; asserting over a stale 60 would leave four sites unpinned while claiming full coverage, which is the shape of failure this epic exists to remove. The issue does not state the fact that shapes the design: output() never touches the exit code. Confirmed by reading it — it writes fd 1 and returns. So a declared outcome for those 64 sites had nowhere to be READ. The mapping was never the work; wiring somewhere for the declaration to land was. The seam already existed twice over. cli-exit.cts holds two globalThis-Symbol cells, each because the module is emitted to three locations and a module-level `let` would let instances disagree, and runMain already maps a code returned by main(). A third cell inherits that solution. output() records DEGRADED for any {error} payload — key-order agnostic, which is exactly why the "42 sites" figure undercounts — and runMain projects the cell only when main() returns nothing, so an explicit return still wins. error() maps its reason through a table over the closed 25-member enum, leaving all 278 call sites untouched; 226 of them pass no reason at all. The version gate lives in error(), NOT in projectOutcome: registered names are version-invariant there, so mapping a reason straight through would make USAGE project to 64 under v1 and break the pin on its first line. projectOutcome is left exactly as Phase 2 shipped it, DEGRADED's 0/80 asymmetry included. Proven rather than asserted. v1 is byte-identical across three real CLI paths — config-get plain, config-get --json-errors, and an output({error}) path — matching exit code and exact bytes against the pre-change build. Under GSD_EXIT_CONTRACT=v2 the same commands now exit 66 (CONFIG_KEY_NOT_FOUND -> NO_INPUT) and 80 (DEGRADED), both looked up through the registry. An anti-vacuity test pins that v1 and v2 genuinely differ for at least one reason, because without it a mapping where everything projects to 1 under both versions would satisfy every other assertion and the declaration would be theatre. A1 iterates all 25 enum members and A3 asserts over the measured 64-site population, so a 26th reason or a 65th site fails until it is given a mapping — the drift guard this phase needs, given ADR-2980's own count had drifted +4 unnoticed. Verification runs on the remote runner. Refs #3912 * fix(#3912): the outcome cell must never lower an exit code The remote run caught a fail-open that this phase introduced, in the phase whose entire purpose is removing fail-opens. `state validate --strict` on a missing STATE.md exited **0** where it must exit 1. Mechanism: `runMain` projected the pending outcome whenever `main()` returned void, and under v1 DEGRADED projects to 0 — so a `process.exitCode` already set non-zero by the command was clobbered down to success. Confirmed live against a fixture, before and after. This refutes a review conclusion recorded earlier in this phase, that the cell was "fail-closed and can never mask a failure as success". It could, and did. Recording that plainly so the assumption is not repeated: the cell's danger was never only that it might add a failure — it was that projecting it unconditionally overwrites whatever decision came before. Projection is now guarded: it may set a code only when none is set, and an already-non-zero exit code always wins. The full precedence — explicit `main()` return, then an existing non-zero exitCode, then the declared outcome — is written at the projection site. A regression test drives a void return with a pre-set non-zero code and a pending DEGRADED, and fails against the pre-fix build. The second failure was my test encoding the wrong contract, not a code defect. It asserted `output({found:false, error: undefined})` records DEGRADED because the KEY is present. `JSON.stringify` drops undefined, so the payload the user receives is `{"found":false}` — carrying no error at all, and calling that degraded would hand back exit 80 under v2 for output that reads as clean. The discriminator is a serializable error VALUE, not key presence. The test now pins `{error: undefined}` as explicitly NOT degraded, and the design doc's wording is tightened to match. Verification runs on the remote runner. Refs #3912 * docs(#3912): the versioned exit contract, and a flag defect the docs found Diataxis pass for Phase 8, plus a real fix that only surfaced because writing the how-to meant running its own examples. The docs. ADR-2980's "Revisit if" clause asked for exactly the versioned projection this phase provides, so it gets an amendment naming #3912 / ADR-3889 section 4 as that boundary: v1 stays 0 byte-for-byte, v2 projects DEGRADED to 80. The amendment also records the count drift rather than restating a stale figure — the ADR ratified 60 output({error}) sites in 9 modules; the AST-measured population is 64 across the same 9 (frontmatter 7 not 6, phase 4 not 2, roadmap 3 not 2). The pin is asserted over the enumerated 64. json-errors.md gains the outcome-declaration reference, including the precedence order a review pass got wrong and the suite refuted: an explicit main() return, then an already-set non-zero process.exitCode, then the declared outcome. Projection may only ever set a code, never lower one. A how-to is owed here and is written, not skipped. Under v1 nothing changes, so the audience is an operator opting into v2 and needing to know what the codes mean for a CI gate — a migration, which is how-to shaped. It covers turning v2 on, the code table, why 80 is "ran and reported a condition" rather than a crash, and how to split a gate that treats any non-zero as fatal. No tutorial: there is no new entry point to learn, and under the default contract a reader would be walked through observing nothing. The defect. Running the how-to's own Step 1 example returned $ gsd-tools --exit-contract=v2 state validate --strict Error: Unknown command: --exit-contract=v2 (exit 64) while the same flag trailing the subcommand worked and exited 80. The flag half-worked, by argv position. resolveContractVersion scans argv non-destructively, so the token survived into the dispatcher, which treats argv[2] as the command name. --json-errors had already solved precisely this at gsd-tools.cjs:4455, under a comment naming the hazard verbatim: "The argv splice must happen here too, otherwise the dispatcher below sees --json-errors as an unknown command." The later flag never got the same treatment. Fixed rather than documented around: the version is resolved first — which memoizes the cell and makes an invalid value throw early — and then every --exit-contract= token is spliced out of the dispatcher's argv copy. --exit-contract is now listed in TOP_LEVEL_USAGE, where it never was. The regression test pins leading position, trailing position, agreement between the two, and a loud failure on v3 rather than a silent fall back to v1. Neither review engine would have caught this: the defect is invisible in the diff, because the diff does not touch argv handling. It surfaced only from running the documentation's own example. Writing a how-to is an execution pass. Verification runs on the remote runner. Refs #3912 * fix(#3912): the flag splice has to run before the run-with-timeout return An isolated review of the previous commit found that the fix did not deliver what it claimed, and that two of its own tests were weak. All three findings reproduced by execution before any change was made. The fix was placed below a return. main() intercepts `run-with-timeout` at gsd-tools.cjs:4436 and returns from there — above both the --json-errors block and the --exit-contract splice added in the previous commit. So the flag still died in leading position for that one command: $ gsd-tools --exit-contract=v2 run-with-timeout 5 -- node -e "..." Error: Unknown command: run-with-timeout (exit 64, child never ran) The previous commit message and the test's describe-block both claimed position-independence unconditionally. That was an overclaim, not a gap left open, and it is the part worth naming: the fix was verified by hand on the commands I happened to think of, and `run-with-timeout` returns before the code I was verifying. Both global-flag blocks now run above the interception, with a comment naming it so a later edit cannot slide them back down. Moving --json-errors up fixes the identical pre-existing bug for that flag, verified failing beforehand (exit 1, sdk_unknown_command). Fixing the sibling is deliberate: same defect, same block, and a known-broken twin next to a fixed one is not a resting state. Two tests were not pulling their weight. The invalid-value test was vacuous — it passed against the pre-fix build, because `--exit-contract=v3` already exited 1 there and already printed the resolve error lazily through error() -> getContractVersion. Both its assertions held before the fix, so it pinned nothing. The real discriminator is that the pre-fix build emits BOTH "Unknown command: --exit-contract=v3" and the resolve error, while the fixed build emits only the latter; the test now asserts that absence. The leading-position and leading==trailing tests asserted proxies — "not 64", "no Unknown command", "the two agree" — none of which pin a value, and all of which would survive both positions being identically broken. With a .planning directory and no STATE.md, state-snapshot exits exactly 80 under v2 and 0 under v1 in both positions. Those numbers are pinned now. The multi-token case the descending splice loop exists for is covered too, and run-with-timeout has regression tests for both flags. The lesson is narrower than "test more". Hand-verifying the production behavior does not verify that the test would have caught its absence. The pre-fix binary has to be run against the test's own assertions. Investigated and deliberately not changed: splicing before --cwd parsing degrades one diagnostic from "Missing value for --cwd" to "Invalid --cwd: <path>", but that is pre-existing — verified on the pre-fix build via --json-errors, which already did it. This change joins the pattern rather than creating it, and both forms exit 64 on malformed input either way. Verification runs on the remote runner. Refs #3912 * chore(#3912): backfill changeset pr numbers to 3983 * test(#3912): pin the reason-table invariant as set equality, not a count A graph-backed review flagged the unchecked lookup in expectedErrorCode3912. Investigated by execution: the drift guard DOES hold — for an unmapped reason under v2 the production error() yields 1 while the table yields undefined, so the assertion fails. Not a correctness defect, and deliberately NOT made tolerant, since a tolerant lookup would destroy the guard. Two real problems remained. The guard asserted the wrong invariant: it counted the TABLE's keys at 25 rather than checking they match the ENUM's values, so a renamed member keeps the count at 25 and slips past, and a 26th member leaves the table at 25 and slips past too. Both were then caught only indirectly, by an undefined mismatch producing 'must exit undefined'. It is now a sorted set equality, so the failure names the specific missing or extra reason. And the comment above it described a '?? FAIL' fallback that does not exist anywhere in the function. It now states what the code actually does, verified by running it rather than by reading it. Refs #3912 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -1120,6 +1120,123 @@ describe('#3906: ambient GSD_EXIT_CONTRACT/--exit-contract wiring (acceptance cr
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3912 regression: `--exit-contract=<v>` in LEADING argv position ──────
|
||||
//
|
||||
// resolveContractVersion() scans argv non-destructively (findExitContractFlag,
|
||||
// gsd-core/bin/lib/cli-exit.cjs). Nothing previously spliced the flag out of
|
||||
// the `gsd-tools` CLI dispatcher's own argv before it fell through to command
|
||||
// dispatch — the `--json-errors` block did this splice for itself, but
|
||||
// `--exit-contract=<v>` never got the same treatment. The dispatcher treats
|
||||
// argv[0] as the command name, so:
|
||||
// `gsd-tools --exit-contract=v2 state validate --strict` (leading) died
|
||||
// with "Unknown command: --exit-contract=v2" (exit 64) — the flag was never
|
||||
// consumed and squatted on the command-name slot.
|
||||
// `gsd-tools state-snapshot --exit-contract=v2` (trailing) worked, because
|
||||
// the flag landed after the command name and never collided with dispatch.
|
||||
describe('#3912: gsd-tools dispatcher splices --exit-contract=<v> regardless of argv position', () => {
|
||||
const GSD_TOOLS_BIN = path.resolve(__dirname, '../gsd-core/bin/gsd-tools.cjs');
|
||||
|
||||
function run(args, options = {}) {
|
||||
return toLegacyResult(runNode([GSD_TOOLS_BIN, ...args], { timeoutMs: PROBE_TIMEOUT_MS, ...options }));
|
||||
}
|
||||
|
||||
// Fixture: a temp dir with a `.planning/` directory but no `STATE.md`, so
|
||||
// `state-snapshot` takes the "STATE.md not found" branch — which the
|
||||
// outcome cell resolves to exit 80 under v2 and exit 0 under v1. Pinning
|
||||
// these exact numbers (rather than "not 64" / "leading == trailing")
|
||||
// catches the case where both positions dispatch but land on the SAME
|
||||
// wrong contract.
|
||||
let fixtureDir;
|
||||
afterEach(() => {
|
||||
if (fixtureDir) {
|
||||
cleanup(fixtureDir);
|
||||
fixtureDir = undefined;
|
||||
}
|
||||
});
|
||||
function makeFixture() {
|
||||
fixtureDir = createTempDir();
|
||||
fs.mkdirSync(path.join(fixtureDir, '.planning'), { recursive: true });
|
||||
return fixtureDir;
|
||||
}
|
||||
|
||||
test('leading position dispatches (no "Unknown command", not exit 64)', () => {
|
||||
const r = run(['--exit-contract=v2', 'state', 'validate', '--strict']);
|
||||
assert.notEqual(r.status, 64, `must not fall into the unknown-command path; stderr: ${r.stderr}`);
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`leading --exit-contract=v2 must not be treated as the command name; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('trailing position still dispatches (no regression)', () => {
|
||||
const r = run(['state-snapshot', '--exit-contract=v2']);
|
||||
assert.notEqual(r.status, 64, `must not regress into the unknown-command path; stderr: ${r.stderr}`);
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`trailing --exit-contract=v2 must keep dispatching; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('leading and trailing position agree on the SAME pinned exit code, per contract version', () => {
|
||||
const dir = makeFixture();
|
||||
const leadingV2 = run(['--exit-contract=v2', 'state-snapshot', `--cwd=${dir}`]);
|
||||
const trailingV2 = run(['state-snapshot', `--cwd=${dir}`, '--exit-contract=v2']);
|
||||
const leadingV1 = run(['--exit-contract=v1', 'state-snapshot', `--cwd=${dir}`]);
|
||||
const trailingV1 = run(['state-snapshot', `--cwd=${dir}`, '--exit-contract=v1']);
|
||||
assert.equal(leadingV2.status, 80, `leading v2 must exit 80; stderr: ${leadingV2.stderr}`);
|
||||
assert.equal(trailingV2.status, 80, `trailing v2 must exit 80; stderr: ${trailingV2.stderr}`);
|
||||
assert.equal(leadingV1.status, 0, `leading v1 must exit 0; stderr: ${leadingV1.stderr}`);
|
||||
assert.equal(trailingV1.status, 0, `trailing v1 must exit 0; stderr: ${trailingV1.stderr}`);
|
||||
});
|
||||
|
||||
test('an invalid leading value (v3) fails loudly rather than silently defaulting to v1', () => {
|
||||
const r = run(['--exit-contract=v3', 'state-snapshot']);
|
||||
assert.notEqual(r.status, 80, 'an invalid contract version must not silently resolve to a valid v2 exit code');
|
||||
assert.ok(
|
||||
/unrecognized exit-contract version/.test(r.stderr),
|
||||
`expected the resolveContractVersion rejection message on stderr; got: ${r.stderr.slice(0, 300)}`,
|
||||
);
|
||||
// Distinguishes this from the PRE-FIX build, where the leading flag was
|
||||
// never spliced: pre-fix stderr contains BOTH "Unknown command:
|
||||
// --exit-contract=v3" (from the dispatcher rejecting the leading token)
|
||||
// AND the resolve error (raised lazily via error() -> getContractVersion
|
||||
// later in the same run). Post-fix, the flag is spliced before dispatch,
|
||||
// so only the resolve error appears.
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`leading --exit-contract=v3 must be spliced before dispatch, not treated as the command name; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('multiple --exit-contract= tokens: first match wins, no leftover token reaches the dispatcher', () => {
|
||||
const dir = makeFixture();
|
||||
const r = run(['--exit-contract=v2', '--exit-contract=v1', 'state-snapshot', `--cwd=${dir}`]);
|
||||
assert.equal(r.status, 80, `first-match (v2) must win; stderr: ${r.stderr}`);
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`every --exit-contract= token must be spliced, not just the first; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('leading --exit-contract=v2 does not break run-with-timeout dispatch (#3912 P1 regression)', () => {
|
||||
const r = run(['--exit-contract=v2', 'run-with-timeout', '5', '--', 'node', '-e', 'process.exit(0)']);
|
||||
assert.equal(r.status, 0, `child must run and exit 0, not die with Unknown command; stderr: ${r.stderr}`);
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`leading --exit-contract=v2 must not intercept run-with-timeout's own dispatch; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('leading --json-errors does not break run-with-timeout dispatch (#3912 P1 regression)', () => {
|
||||
const r = run(['--json-errors', 'run-with-timeout', '5', '--', 'node', '-e', 'process.exit(0)']);
|
||||
assert.equal(r.status, 0, `child must run and exit 0, not die with Unknown command; stderr: ${r.stderr}`);
|
||||
assert.ok(
|
||||
!/Unknown command/.test(r.stderr),
|
||||
`leading --json-errors must not intercept run-with-timeout's own dispatch; stderr: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3906: terminateNow', () => {
|
||||
function spawnTerminateNow(lines) {
|
||||
return toLegacyResult(runNode(['-e', lines.join('\n')], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
@@ -1761,3 +1878,175 @@ describe('#3911: the --check guards for the new hooks/lib artifacts can actually
|
||||
assert.equal(restored.status, 0, `--check must pass again once the artifact is restored; stderr: ${restored.stderr}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3912 (ADR-3889 §4, P8): the pending-outcome cell ──────────────────────
|
||||
//
|
||||
// output()'s payload-carried-error detection lives in io.cts and is tested
|
||||
// there; these tests exercise the cell + runMain projection mechanics that
|
||||
// live in cli-exit.cts itself: precedence (test matrix row C5) and cross-copy
|
||||
// parity (row C6).
|
||||
|
||||
describe('#3912: pending-outcome cell (runMain precedence + parity)', () => {
|
||||
// E1: for every registered outcome name and version, projectOutcome is a
|
||||
// non-negative integer, and is 0 ONLY for PASS, or for DEGRADED under v1.
|
||||
// The existing "every projection... is a non-negative integer" test above
|
||||
// does not pin the ZERO-ONLY-FOR half of this; this test is what makes an
|
||||
// accidental future zero for some OTHER outcome fail loudly.
|
||||
test('E1 (fast-check): 0 is produced only by PASS, or by DEGRADED under v1', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.constantFrom('PASS', 'FAIL', ...REGISTERED_NAMES),
|
||||
fc.constantFrom(...VERSIONS),
|
||||
(outcome, version) => {
|
||||
const result = projectOutcome(outcome, version);
|
||||
assert.equal(Number.isInteger(result), true);
|
||||
assert.ok(result >= 0);
|
||||
if (result === 0) {
|
||||
const isPass = outcome === 'PASS';
|
||||
const isDegradedV1 = outcome === 'DEGRADED' && version === 'v1';
|
||||
assert.ok(
|
||||
isPass || isDegradedV1,
|
||||
`outcome=${outcome} version=${version} projected to 0 but is neither PASS nor DEGRADED/v1`,
|
||||
);
|
||||
} else {
|
||||
assert.notEqual(outcome, 'PASS', 'PASS must always project to 0');
|
||||
}
|
||||
},
|
||||
),
|
||||
{ seed: 3912, numRuns: 300 },
|
||||
);
|
||||
});
|
||||
|
||||
// C5: an explicit main() return beats a recorded DEGRADED cell. Without
|
||||
// this, a caller that both returns an explicit code AND had a stale
|
||||
// DEGRADED left in the cell (e.g. from an earlier output({error}) call in
|
||||
// the same process) would get the CELL's projection instead of its own
|
||||
// explicit decision — exactly backwards from "the cell is a fallback for
|
||||
// the absence of a decision".
|
||||
test('C5: an explicit main() return wins over a pending DEGRADED cell', () => {
|
||||
const script = [
|
||||
`const c = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`c.setPendingOutcome('DEGRADED');`,
|
||||
`c.runMain(() => 'PASS');`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n');
|
||||
// Under v2, DEGRADED alone would project to 80; PASS must win with 0.
|
||||
const r = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS,
|
||||
env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.equal(r.status, 0, `explicit PASS return must win over the pending DEGRADED cell; stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('C5 (numeric arm): an explicit numeric return also wins over a pending DEGRADED cell', () => {
|
||||
const script = [
|
||||
`const c = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`c.setPendingOutcome('DEGRADED');`,
|
||||
`c.runMain(() => 42);`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n');
|
||||
const r = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS,
|
||||
env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.equal(r.status, 42, `explicit numeric return must win over the pending DEGRADED cell; stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('a void return with NO pending outcome set leaves exit code untouched (v1 unchanged)', () => {
|
||||
const script = [
|
||||
`const c = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`c.runMain(() => undefined);`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n');
|
||||
const r = toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
assert.equal(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('a void return WITH a pending DEGRADED cell projects it under v1 (0) and v2 (80)', () => {
|
||||
const script = [
|
||||
`const c = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`c.setPendingOutcome('DEGRADED');`,
|
||||
`c.runMain(() => undefined);`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n');
|
||||
const v1 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v1' },
|
||||
}));
|
||||
assert.equal(v1.status, 0, `stderr: ${v1.stderr}`);
|
||||
const v2 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.equal(v2.status, 80, `stderr: ${v2.stderr}`);
|
||||
});
|
||||
|
||||
// Regression (fail-open, found live in `state validate --strict` on a
|
||||
// missing STATE.md): a void-returning main() that ALREADY set
|
||||
// process.exitCode to a non-zero value itself (e.g. `emit()` setting 1
|
||||
// directly on the STATE.md-not-found early return) must have that value
|
||||
// survive a pending DEGRADED cell — the cell must never LOWER an exit
|
||||
// code main() itself already raised. Before the fix, this arm
|
||||
// unconditionally overwrote process.exitCode with the cell's projection,
|
||||
// clobbering an already-set 1 down to DEGRADED's v1 projection (0) — a
|
||||
// real declared failure silently turned into success.
|
||||
test('a void return with an ALREADY-SET non-zero exitCode beats a pending DEGRADED cell (regression)', () => {
|
||||
const script = [
|
||||
`const c = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`c.setPendingOutcome('DEGRADED');`,
|
||||
`c.runMain(() => { process.exitCode = 1; });`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n');
|
||||
const v1 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v1' },
|
||||
}));
|
||||
assert.equal(v1.status, 1, `an already-set non-zero exitCode must not be clobbered down to DEGRADED's v1 projection (0); stderr: ${v1.stderr}`);
|
||||
const v2 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.equal(v2.status, 1, `an already-set non-zero exitCode must not be clobbered by DEGRADED's v2 projection (80) either; stderr: ${v2.stderr}`);
|
||||
});
|
||||
|
||||
// C6: extends the existing three-copy parity coverage (json-error-mode cell,
|
||||
// tested above at "both copies of the exit module share one json-error-mode
|
||||
// cell") to the pending-outcome cell, across all THREE emitted copies —
|
||||
// built (gsd-core/bin/lib), scripts/lib, and hooks/lib — not just two.
|
||||
test('C6: all three emitted copies of cli-exit share ONE pending-outcome cell', () => {
|
||||
const r = toLegacyResult(runNode(['-e', [
|
||||
`const built = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});`,
|
||||
`const scripts = require(${JSON.stringify(SCRIPTS_CLI_EXIT_PATH)});`,
|
||||
`const hooks = require(${JSON.stringify(HOOKS_CLI_EXIT_PATH)});`,
|
||||
`if (built === scripts || built === hooks || scripts === hooks) throw new Error('expected three distinct module instances');`,
|
||||
`built.setPendingOutcome('DEGRADED');`,
|
||||
`process.stdout.write(JSON.stringify({`,
|
||||
` viaBuilt: built.getPendingOutcome(),`,
|
||||
` viaScripts: scripts.getPendingOutcome(),`,
|
||||
` viaHooks: hooks.getPendingOutcome(),`,
|
||||
`}));`,
|
||||
].join('\n')], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
assert.deepStrictEqual(
|
||||
JSON.parse(r.stdout),
|
||||
{ viaBuilt: 'DEGRADED', viaScripts: 'DEGRADED', viaHooks: 'DEGRADED' },
|
||||
'all three copies must read one shared cell — three independent module-level flags would diverge here',
|
||||
);
|
||||
});
|
||||
|
||||
test('C6: all three copies also agree on the PROJECTED exit code via runMain', () => {
|
||||
for (const version of VERSIONS) {
|
||||
for (const modulePath of [BUILT_CLI_EXIT_PATH, SCRIPTS_CLI_EXIT_PATH, HOOKS_CLI_EXIT_PATH]) {
|
||||
const r = toLegacyResult(runNode(['-e', [
|
||||
`const c = require(${JSON.stringify(modulePath)});`,
|
||||
`c.setPendingOutcome('DEGRADED');`,
|
||||
`c.runMain(() => undefined);`,
|
||||
`setImmediate(() => {});`,
|
||||
].join('\n')], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: version },
|
||||
}));
|
||||
const expected = version === 'v1' ? 0 : 80;
|
||||
assert.equal(
|
||||
r.status, expected,
|
||||
`${modulePath} under ${version} expected ${expected}, got ${r.status}; stderr: ${r.stderr}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,10 +18,15 @@ const os = require('node:os');
|
||||
const fs = require('node:fs');
|
||||
|
||||
const io = require('../gsd-core/bin/lib/io.cjs');
|
||||
const { ExitError } = require('../gsd-core/bin/lib/cli-exit.cjs');
|
||||
const {
|
||||
ExitError, resolveContractVersion, setPendingOutcome, getPendingOutcome, runMain,
|
||||
} = require('../gsd-core/bin/lib/cli-exit.cjs');
|
||||
const { EXIT_CODES } = require('../gsd-core/bin/lib/exit-code-registry.cjs');
|
||||
const { runNode } = require('./helpers/process-seam.cjs');
|
||||
const { toLegacyResult } = require('./helpers/git-fixture.cjs');
|
||||
const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
const ts = require('typescript');
|
||||
|
||||
function runScript(script) {
|
||||
return toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
@@ -563,3 +568,525 @@ describe('bug #1891: @file: resolution in gsd-tools.cjs', () => {
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// #3912 (ADR-3889 §4, epic #3889 Phase 8) — gsd-tools declares outcomes,
|
||||
// pinned at v1. See .gsd/phase/enhance-3912-gsd-tools-outcomes/40-design.md
|
||||
// and 50-test-matrix.md.
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
const CLI_EXIT_PATH_3912 = path.resolve(__dirname, '../gsd-core/bin/lib/cli-exit.cjs');
|
||||
const IO_PATH_3912 = path.resolve(__dirname, '../gsd-core/bin/lib/io.cjs');
|
||||
const REGISTERED_NAMES_3912 = EXIT_CODES.map((e) => e.name);
|
||||
const CODE_FOR_3912 = new Map(EXIT_CODES.map((e) => [e.name, e.code]));
|
||||
const VERSIONS_3912 = ['v1', 'v2'];
|
||||
|
||||
/**
|
||||
* Expected reason -> outcome mapping, mirroring src/io.cts's own
|
||||
* REASON_TO_OUTCOME table. Kept as an independent, explicit table here
|
||||
* (rather than importing the internal function) so the test is a real
|
||||
* behavioral check against error()'s observable exit code, not a tautology
|
||||
* that re-imports the thing it is meant to verify.
|
||||
*/
|
||||
const EXPECTED_REASON_OUTCOME_3912 = {
|
||||
config_key_not_found: 'NO_INPUT',
|
||||
config_no_file: 'UNAVAILABLE',
|
||||
config_parse_failed: 'UNAVAILABLE',
|
||||
config_invalid_key: 'USAGE',
|
||||
sdk_fail_fast: 'INTERNAL',
|
||||
sdk_unknown_command: 'USAGE',
|
||||
sdk_missing_arg: 'USAGE',
|
||||
phase_not_found: 'UNAVAILABLE',
|
||||
phase_verification_incomplete: 'UNAVAILABLE',
|
||||
phase_plan_coverage_incomplete: 'UNAVAILABLE',
|
||||
summary_no_planning: 'NO_INPUT',
|
||||
workstream_mode_none_active: 'NO_INPUT',
|
||||
workstream_mode_marker_unresolved: 'UNAVAILABLE',
|
||||
graphify_no_graph: 'UNAVAILABLE',
|
||||
graphify_invalid_query: 'USAGE',
|
||||
estimate_phases_unreadable: 'UNAVAILABLE',
|
||||
hooks_opt_out: 'FAIL',
|
||||
commit_docs_guard_not_a_repo: 'UNAVAILABLE',
|
||||
commit_docs_guard_foreign_hook: 'UNAVAILABLE',
|
||||
commit_docs_guard_hooks_path_set: 'UNAVAILABLE',
|
||||
security_scan_failed: 'INTERNAL',
|
||||
pick_field_absent: 'UNAVAILABLE',
|
||||
pick_output_not_json: 'UNAVAILABLE',
|
||||
usage: 'USAGE',
|
||||
unknown: 'FAIL',
|
||||
};
|
||||
|
||||
/** Expected exit code for `error(msg, reason)` under a given contract version. */
|
||||
function expectedErrorCode3912(reasonValue, version) {
|
||||
if (version === 'v1') return 1;
|
||||
const outcome = EXPECTED_REASON_OUTCOME_3912[reasonValue];
|
||||
if (outcome === 'FAIL') return 1;
|
||||
return CODE_FOR_3912.get(outcome);
|
||||
}
|
||||
|
||||
describe('#3912 A1/B1: error() declares from ERROR_REASON, exhaustive over the 25-member enum', () => {
|
||||
afterEach(() => {
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} }); // restore v1 default
|
||||
});
|
||||
|
||||
// A1 — the acceptance criterion: EVERY member of ERROR_REASON, iterated
|
||||
// from the enum itself (not a hand-picked subset), exits 1 under v1. A
|
||||
// 26th member added to the enum without a table entry still exits 1
|
||||
// under v1 (v1 never consults the table at all); under v2, the table
|
||||
// lookup for that member yields `undefined`, `CODE_FOR_3912.get(undefined)`
|
||||
// yields `undefined`, and `err.code === expected` fails against the real
|
||||
// code (1) for that reason. With the set-equality assertion below in
|
||||
// place, that drift is caught first, with a message naming the specific
|
||||
// missing/extra reason instead of a confusing "must exit undefined".
|
||||
assert.deepEqual(
|
||||
Object.keys(EXPECTED_REASON_OUTCOME_3912).sort(),
|
||||
Object.values(io.ERROR_REASON).slice().sort(),
|
||||
'this table must cover exactly the ERROR_REASON enum values, no more, no less',
|
||||
);
|
||||
for (const [key, reasonValue] of Object.entries(io.ERROR_REASON)) {
|
||||
test(`v1: ERROR_REASON.${key} (${reasonValue}) exits 1`, () => {
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} }); // v1
|
||||
assert.throws(
|
||||
() => io.error('msg', reasonValue),
|
||||
(err) => err instanceof ExitError && err.code === 1,
|
||||
`ERROR_REASON.${key} must exit 1 under v1`,
|
||||
);
|
||||
});
|
||||
|
||||
test(`v2: ERROR_REASON.${key} (${reasonValue}) projects to its mapped outcome's registered code`, () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
const expected = expectedErrorCode3912(reasonValue, 'v2');
|
||||
assert.throws(
|
||||
() => io.error('msg', reasonValue),
|
||||
(err) => err instanceof ExitError && err.code === expected,
|
||||
`ERROR_REASON.${key} under v2 must exit ${expected}`,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
// A2 — the 226-site default: no reason argument at all -> UNKNOWN -> exit 1.
|
||||
test('A2: error() with no reason argument exits 1 under v1 (defaults to UNKNOWN)', () => {
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} });
|
||||
assert.throws(
|
||||
() => io.error('no reason given'),
|
||||
(err) => err instanceof ExitError && err.code === 1,
|
||||
);
|
||||
});
|
||||
|
||||
test('A2: error() with no reason argument stays FAIL (exit 1) under v2 too — UNKNOWN is not a specific outcome', () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
assert.throws(
|
||||
() => io.error('no reason given'),
|
||||
(err) => err instanceof ExitError && err.code === 1,
|
||||
);
|
||||
});
|
||||
|
||||
// B2 — spot-check the specific mappings the design calls out by name.
|
||||
test('B2: SDK_MISSING_ARG / SDK_UNKNOWN_COMMAND / USAGE all reach USAGE (64) under v2', () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
for (const key of ['SDK_MISSING_ARG', 'SDK_UNKNOWN_COMMAND', 'USAGE']) {
|
||||
assert.throws(
|
||||
() => io.error('msg', io.ERROR_REASON[key]),
|
||||
(err) => err instanceof ExitError && err.code === 64,
|
||||
`${key} must project to 64 under v2`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// B5 — the anti-vacuity test. Without this, a mapping where every reason
|
||||
// projects to 1 under both versions would satisfy every row above.
|
||||
test('B5 (anti-vacuity): v1 and v2 differ for at least one reason', () => {
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} });
|
||||
let v1Code;
|
||||
try { io.error('msg', io.ERROR_REASON.SDK_MISSING_ARG); } catch (e) { v1Code = e.code; }
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
let v2Code;
|
||||
try { io.error('msg', io.ERROR_REASON.SDK_MISSING_ARG); } catch (e) { v2Code = e.code; }
|
||||
assert.equal(v1Code, 1);
|
||||
assert.equal(v2Code, 64);
|
||||
assert.notEqual(v1Code, v2Code, 'v1 and v2 must differ for at least one reason, or the declaration is decorative');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3912 A3-A5: output({error}) records DEGRADED — shape-exhaustive plus a real census', () => {
|
||||
// A3/A4 — shape exhaustive: both key orders, and varying the error value's
|
||||
// own type/truthiness (irrelevant to detection — presence of the key is
|
||||
// what counts).
|
||||
// The discriminator is a SERIALIZABLE error value, not mere key presence:
|
||||
// every row here embeds its payload via `JSON.stringify(payload)` to build
|
||||
// the child script's literal, and `JSON.stringify` drops any key whose
|
||||
// value is `undefined` — so an `{ error: undefined }` row would silently
|
||||
// arrive at `output()` with NO `error` key at all, making the row pass for
|
||||
// the wrong reason (or, as originally written, fail outright: see the
|
||||
// dedicated `{error: undefined}` case below, which constructs the object
|
||||
// as source text instead so the key survives).
|
||||
const shapes = [
|
||||
['error first', { error: 'boom', found: false }],
|
||||
['error last (A4)', { found: false, error: 'boom' }],
|
||||
['error in the middle', { a: 1, error: 'boom', b: 2 }],
|
||||
['error value is falsy (0)', { found: false, error: 0 }],
|
||||
['error value is null', { found: false, error: null }],
|
||||
['error value is an object', { error: { code: 'X' }, found: false }],
|
||||
];
|
||||
for (const [label, payload] of shapes) {
|
||||
test(`A3/A4 (${label}): exits 0 under v1 and is recorded as DEGRADED`, () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
io.output(${JSON.stringify(payload)}, false);
|
||||
process.stdout.write('|PENDING=' + c.getPendingOutcome());
|
||||
`;
|
||||
const result = toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
assert.strictEqual(result.status, 0, `stderr: ${result.stderr}`);
|
||||
assert.ok(result.stdout.includes('|PENDING=DEGRADED'), `expected DEGRADED recorded; got: ${result.stdout}`);
|
||||
});
|
||||
}
|
||||
|
||||
// A3/A4 pin — `{ error: undefined }` is NOT degraded. The object literal is
|
||||
// written as SOURCE TEXT here (not round-tripped through
|
||||
// `JSON.stringify(payload)`), so the `error` key genuinely reaches
|
||||
// `output()` with an `undefined` value. `JSON.stringify` (the serializer
|
||||
// `output()` itself uses to build the payload the user actually receives)
|
||||
// drops a key whose value is `undefined`, so the wire payload is
|
||||
// `{"found":false}` — no error at all. Recording DEGRADED here would be a
|
||||
// false verdict: exit 80 under v2 for output the user sees as clean. The
|
||||
// discriminator is a SERIALIZABLE error value, not key presence.
|
||||
test('A3/A4 pin: {error: undefined} is NOT recorded as DEGRADED (JSON.stringify drops it)', () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
io.output({ found: false, error: undefined }, false);
|
||||
process.stdout.write('|PENDING=' + c.getPendingOutcome());
|
||||
`;
|
||||
const result = toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
assert.strictEqual(result.status, 0, `stderr: ${result.stderr}`);
|
||||
assert.ok(
|
||||
!result.stdout.includes('|PENDING=DEGRADED'),
|
||||
`{error: undefined} carries no serializable error and must not be degraded; got: ${result.stdout}`,
|
||||
);
|
||||
});
|
||||
|
||||
// A5 — negative space: no `error` key at all must NOT be recorded as degraded.
|
||||
test('A5: output() with no error key exits 0 and records NOTHING', () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
io.output({ ok: true, value: 1 }, false);
|
||||
process.stdout.write('|PENDING=' + c.getPendingOutcome());
|
||||
`;
|
||||
const result = toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS }));
|
||||
assert.strictEqual(result.status, 0, `stderr: ${result.stderr}`);
|
||||
assert.ok(!result.stdout.includes('|PENDING=DEGRADED'), `must not record DEGRADED; got: ${result.stdout}`);
|
||||
});
|
||||
|
||||
// A3 census — AST-based, not a text grep (local/no-source-grep bans
|
||||
// readFileSync().includes()/.match()/etc on source; this parses an AST
|
||||
// instead and never calls a string-search method on the source text).
|
||||
// Counts every call to an identifier literally named `output` (the name
|
||||
// every call site in this tree destructures it to — `const { output } =
|
||||
// ioMod`) whose first argument is an object literal carrying an `error`
|
||||
// property. This is the SHAPE the design measured, over the real tree,
|
||||
// not a hand-picked subset — and it independently reproduces the design
|
||||
// doc's per-file breakdown (frontmatter 7, phase 4, roadmap 3, state 25,
|
||||
// verify 8, workstream 7, commands 5, template 3, gsd2-import 2 = 64),
|
||||
// which is itself the corrected count over ADR-2980's stale 60.
|
||||
test('A3 census: exactly 64 output({error}) call sites exist in src/, across the 9 modules the design measured', () => {
|
||||
const SRC_ROOT = path.resolve(__dirname, '../src');
|
||||
|
||||
function listCtsFiles(dir) {
|
||||
const out = [];
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) out.push(...listCtsFiles(full));
|
||||
else if (entry.name.endsWith('.cts') && !entry.name.endsWith('.d.cts')) out.push(full);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function hasErrorProp(objLit) {
|
||||
return objLit.properties.some((p) => {
|
||||
if (ts.isPropertyAssignment(p) || ts.isShorthandPropertyAssignment(p)) {
|
||||
const name = p.name;
|
||||
if (ts.isIdentifier(name)) return name.text === 'error';
|
||||
if (ts.isStringLiteral(name)) return name.text === 'error';
|
||||
}
|
||||
return false;
|
||||
});
|
||||
}
|
||||
|
||||
const perFile = {};
|
||||
let total = 0;
|
||||
for (const file of listCtsFiles(SRC_ROOT)) {
|
||||
const text = fs.readFileSync(file, 'utf8');
|
||||
const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
|
||||
let count = 0;
|
||||
(function visit(node) {
|
||||
if (ts.isCallExpression(node)) {
|
||||
let calleeName = null;
|
||||
if (ts.isIdentifier(node.expression)) calleeName = node.expression.text;
|
||||
else if (ts.isPropertyAccessExpression(node.expression) && ts.isIdentifier(node.expression.name)) {
|
||||
calleeName = node.expression.name.text;
|
||||
}
|
||||
if (calleeName === 'output' && node.arguments.length > 0 && ts.isObjectLiteralExpression(node.arguments[0])) {
|
||||
if (hasErrorProp(node.arguments[0])) count += 1;
|
||||
}
|
||||
}
|
||||
ts.forEachChild(node, visit);
|
||||
})(sf);
|
||||
if (count > 0) perFile[path.basename(file)] = count;
|
||||
total += count;
|
||||
}
|
||||
|
||||
assert.deepStrictEqual(
|
||||
perFile,
|
||||
{
|
||||
'commands.cts': 5, 'frontmatter.cts': 7, 'gsd2-import.cts': 2, 'phase.cts': 4,
|
||||
'roadmap.cts': 3, 'state.cts': 25, 'template.cts': 3, 'verify.cts': 8, 'workstream.cts': 7,
|
||||
},
|
||||
`per-file output({error}) census drifted: ${JSON.stringify(perFile)}`,
|
||||
);
|
||||
assert.strictEqual(total, 64, `enumerated output({error}) population drifted from the measured 64: got ${total}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3912 C5/D: output({error}) DEGRADED reaches process.exitCode only through runMain', () => {
|
||||
afterEach(() => {
|
||||
setPendingOutcome(undefined);
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} });
|
||||
});
|
||||
|
||||
test('a real gsd-tools-shaped main() that calls output({error}) and returns nothing exits 0 under v1, 80 under v2', () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
c.runMain(() => {
|
||||
io.output({ found: false, error: 'not found' }, false);
|
||||
return undefined;
|
||||
});
|
||||
setImmediate(() => {});
|
||||
`;
|
||||
const v1 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v1' },
|
||||
}));
|
||||
assert.strictEqual(v1.status, 0, `stderr: ${v1.stderr}`);
|
||||
const v2 = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.strictEqual(v2.status, 80, `stderr: ${v2.stderr}`);
|
||||
});
|
||||
|
||||
test('an explicit main() return still wins over a DEGRADED output({error}) call in the same main()', () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
c.runMain(() => {
|
||||
io.output({ found: false, error: 'not found' }, false);
|
||||
return 0;
|
||||
});
|
||||
setImmediate(() => {});
|
||||
`;
|
||||
const r = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.strictEqual(r.status, 0, `an explicit 0 return must win over the DEGRADED cell; stderr: ${r.stderr}`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('review fix: pending-outcome cell lifetime (last-write-wins, cleared on consumption)', () => {
|
||||
// These drive runMain/output IN-PROCESS (not via a subprocess), which is
|
||||
// exactly the gap that let the leak through: every other #3912 test above
|
||||
// spawns a fresh process per case, so a cell that is never cleared was
|
||||
// unobservable. runMain mutates process.exitCode as a side effect, so each
|
||||
// test saves/restores it to avoid corrupting the real node:test run's own
|
||||
// exit code.
|
||||
function waitForRunMain() {
|
||||
// runMain resolves its outcome via a Promise.resolve().then().then()
|
||||
// chain (microtasks); a macrotask tick guarantees both have drained.
|
||||
return new Promise((resolve) => { setImmediate(resolve); });
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
// Harmless test hygiene now that production also clears the cell on
|
||||
// every runMain call and on every clean output() — this scrub is a
|
||||
// belt-and-suspenders reset between test cases, not the mechanism that
|
||||
// prevents the leak (that mechanism now lives in cli-exit.cts/io.cts).
|
||||
setPendingOutcome(undefined);
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} });
|
||||
});
|
||||
|
||||
test('leak regression: output({error}) then a second void-returning runMain must NOT inherit stale DEGRADED', async () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
const savedExitCode = process.exitCode;
|
||||
try {
|
||||
// First invocation declares DEGRADED via a payload-carried error and
|
||||
// returns nothing — runMain projects it to 80 under v2.
|
||||
runMain(() => {
|
||||
io.output({ found: false, error: 'not found' }, false);
|
||||
return undefined;
|
||||
});
|
||||
await waitForRunMain();
|
||||
assert.strictEqual(process.exitCode, 80, 'first runMain should have projected the DEGRADED cell to 80');
|
||||
|
||||
// Second, unrelated invocation in the SAME process declares nothing
|
||||
// and returns nothing. Before the fix, the cell was never cleared by
|
||||
// runMain, so this would inherit the first call's stale DEGRADED and
|
||||
// also exit 80 — the exact bug the reviewers found.
|
||||
process.exitCode = undefined;
|
||||
runMain(() => undefined);
|
||||
await waitForRunMain();
|
||||
assert.strictEqual(
|
||||
process.exitCode,
|
||||
undefined,
|
||||
'a later void-returning runMain must not inherit a prior invocation\'s stale DEGRADED declaration',
|
||||
);
|
||||
} finally {
|
||||
process.exitCode = savedExitCode;
|
||||
}
|
||||
});
|
||||
|
||||
test('last-write-wins: output({error}) then output({ok:true}) in the SAME invocation is not degraded', async () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
const savedExitCode = process.exitCode;
|
||||
try {
|
||||
runMain(() => {
|
||||
io.output({ found: false, error: 'not found' }, false);
|
||||
io.output({ ok: true }, false);
|
||||
return undefined;
|
||||
});
|
||||
await waitForRunMain();
|
||||
assert.strictEqual(
|
||||
process.exitCode,
|
||||
undefined,
|
||||
'a later clean output() must undo an earlier degraded one — exit code must stay untouched, not 80',
|
||||
);
|
||||
} finally {
|
||||
process.exitCode = savedExitCode;
|
||||
}
|
||||
});
|
||||
|
||||
test('consumption clears: after runMain consumes a pending outcome, the cell reads unset', async () => {
|
||||
resolveContractVersion({ argv: ['node', 'x', '--exit-contract=v2'], env: {} });
|
||||
const savedExitCode = process.exitCode;
|
||||
try {
|
||||
runMain(() => {
|
||||
io.output({ error: 'x' }, false);
|
||||
return undefined;
|
||||
});
|
||||
await waitForRunMain();
|
||||
assert.strictEqual(process.exitCode, 80);
|
||||
assert.strictEqual(getPendingOutcome(), undefined, 'the cell must be cleared once runMain has consumed it');
|
||||
} finally {
|
||||
process.exitCode = savedExitCode;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3912 A6: error() stderr bytes are unchanged by this phase', () => {
|
||||
afterEach(() => {
|
||||
resolveContractVersion({ argv: ['node', 'x'], env: {} });
|
||||
});
|
||||
|
||||
test('plain mode: "Error: <msg>" bytes are identical regardless of reason or contract version', () => {
|
||||
for (const version of VERSIONS_3912) {
|
||||
resolveContractVersion({ argv: ['node', 'x', `--exit-contract=${version}`], env: {} });
|
||||
let caught;
|
||||
const chunks = [];
|
||||
const origWrite = fs.writeSync;
|
||||
fs.writeSync = (fd, buf, offset, length) => {
|
||||
if (fd !== 2) return origWrite(fd, buf, offset, length);
|
||||
const slice = Buffer.isBuffer(buf) ? buf.subarray(offset, offset + length) : Buffer.from(String(buf));
|
||||
chunks.push(slice.toString('utf8'));
|
||||
return slice.length;
|
||||
};
|
||||
try {
|
||||
io.setJsonErrorMode(false);
|
||||
try { io.error('boundary case', io.ERROR_REASON.SDK_MISSING_ARG); } catch (e) { caught = e; }
|
||||
} finally {
|
||||
fs.writeSync = origWrite;
|
||||
}
|
||||
assert.ok(caught instanceof ExitError);
|
||||
assert.strictEqual(chunks.join(''), 'Error: boundary case\n', `version=${version}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('json mode: the stderr envelope is identical regardless of contract version (only the thrown exit code differs)', () => {
|
||||
for (const version of VERSIONS_3912) {
|
||||
resolveContractVersion({ argv: ['node', 'x', `--exit-contract=${version}`], env: {} });
|
||||
const chunks = [];
|
||||
const origWrite = fs.writeSync;
|
||||
fs.writeSync = (fd, buf, offset, length) => {
|
||||
if (fd !== 2) return origWrite(fd, buf, offset, length);
|
||||
const slice = Buffer.isBuffer(buf) ? buf.subarray(offset, offset + length) : Buffer.from(String(buf));
|
||||
chunks.push(slice.toString('utf8'));
|
||||
return slice.length;
|
||||
};
|
||||
let caught;
|
||||
try {
|
||||
io.setJsonErrorMode(true);
|
||||
try { io.error('boundary case', io.ERROR_REASON.SDK_MISSING_ARG); } catch (e) { caught = e; }
|
||||
} finally {
|
||||
fs.writeSync = origWrite;
|
||||
io.setJsonErrorMode(false);
|
||||
}
|
||||
assert.ok(caught instanceof ExitError);
|
||||
assert.deepStrictEqual(
|
||||
JSON.parse(chunks.join('').trim()),
|
||||
{ ok: false, reason: 'sdk_missing_arg', message: 'boundary case' },
|
||||
`version=${version}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3912 B1/B4/E1: projectOutcome-backed checks over the real registry', () => {
|
||||
test('B1: every registered outcome name is reachable through error() via SOME reason, and matches the registry', () => {
|
||||
// Sanity check that CODE_FOR_3912 (derived straight from the shipped
|
||||
// registry) matches the pinned table cli-exit.test.cjs already asserts.
|
||||
assert.strictEqual(CODE_FOR_3912.get('USAGE'), 64);
|
||||
assert.strictEqual(CODE_FOR_3912.get('NO_INPUT'), 66);
|
||||
assert.strictEqual(CODE_FOR_3912.get('UNAVAILABLE'), 69);
|
||||
assert.strictEqual(CODE_FOR_3912.get('INTERNAL'), 70);
|
||||
assert.strictEqual(CODE_FOR_3912.get('DEGRADED'), 80);
|
||||
});
|
||||
|
||||
test('B4: any output({error}) site under v2 exits 80 (DEGRADED)', () => {
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH_3912)});
|
||||
const c = require(${JSON.stringify(CLI_EXIT_PATH_3912)});
|
||||
c.runMain(() => { io.output({ error: 'x' }, false); return undefined; });
|
||||
setImmediate(() => {});
|
||||
`;
|
||||
const r = toLegacyResult(runNode(['-e', script], {
|
||||
timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_EXIT_CONTRACT: 'v2' },
|
||||
}));
|
||||
assert.strictEqual(r.status, 80, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
// E1 lives primarily in tests/cli-exit.test.cjs (projectOutcome is defined
|
||||
// there); this is the io-side control confirming REGISTERED_NAMES_3912
|
||||
// used by the reason-mapping table above matches the live registry.
|
||||
test('registered names used by the reason-mapping table are exactly the live registry names', () => {
|
||||
for (const outcome of Object.values(EXPECTED_REASON_OUTCOME_3912)) {
|
||||
if (outcome === 'FAIL') continue;
|
||||
assert.ok(
|
||||
REGISTERED_NAMES_3912.includes(outcome),
|
||||
`mapped outcome ${outcome} must be a registered exit-code name`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('fast-check: E1 sanity — every mapped outcome/version pair used by error() yields a non-negative integer', () => {
|
||||
const outcomes = [...new Set(Object.values(EXPECTED_REASON_OUTCOME_3912))];
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.constantFrom(...outcomes),
|
||||
fc.constantFrom(...VERSIONS_3912),
|
||||
(outcome, version) => {
|
||||
const code = outcome === 'FAIL' ? 1 : (version === 'v1' ? 1 : CODE_FOR_3912.get(outcome));
|
||||
assert.ok(Number.isInteger(code) && code >= 0);
|
||||
},
|
||||
),
|
||||
{ seed: 39120, numRuns: 100 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user