fix(#1279): node-test prover fail-closes on missing violationFixture + honest projection docs

Addresses the #1314 maintainer review (trek-e):

- Major 1 (fail-OPEN): defaultProveFailFirst's node-test branch only guarded
  `if (!fixture)`. A missing/typo'd/stale violationFixture made GSD_PROHIB_SUBJECT
  point at a missing file; an honest negative test threw ENOENT *inside its
  callback* (a failing test named distinctly from the file), which
  isNonVacuousNodeTestRed accepted as proof -> a green forged from a setup crash.
  Now requires fs.existsSync(path.resolve(cwd, fixture)) before spawning, symmetric
  with the lint-rule path's file-result guard. Regression test pins it (RED without
  the guard); a second test pins cwd-relative fixture resolution.

- Major 2 (misleading prose): the #1278 projection carries no violationFixture, so
  the deterministic-locate path always hard-gates (fail-closed) until a
  check_violation_fixture scalar is threaded through. verify-phase.md and
  prohibition-probe.md no longer read as if the projected path produces greens;
  the ADR-550 addendum records both items. Tracked as follow-up #1346.

- Documented residual: existence is necessary but not sufficient (a red caused by
  the env being set vs the subject's content); recorded as a constraint, in #1346.

- Nit: stale 'NOT attested fail-first' comment -> 'NOT machine-proven fail-first'.

64 tests pass; eslint + tsc clean; changeset valid.
This commit is contained in:
Dave
2026-06-16 13:05:20 -04:00
parent ee9ef7a8e4
commit 33b6ee6f0c
6 changed files with 91 additions and 9 deletions

View File

@@ -5,6 +5,6 @@ pr: 1314
**Test-tier prohibition fail-first is now MACHINE-PROVEN, not caller-attested** — the deferred literal `regression-must-fail-first` property of ADR-550 Decision 4 (the gap #1259 / PR #1273 left as a tracked follow-up) has landed. The `check prohibition-enforcement` producer (`src/prohibition-enforcement.cts`, compiled by `build:lib` to the gitignored `gsd-core/bin/lib/prohibition-enforcement.cjs`) no longer trusts the caller's `failFirst` attestation: before a clean, non-vacuous pass can dispose a `test`-tier prohibition green, the new `defaultProveFailFirst` prover independently RUNS the wired check against a KNOWN VIOLATION and confirms it goes RED. Attestation is gone from the green AND (`passed = proof.provenFailFirst === true && run.passed === true`); any other outcome — passes-on-violation, can't-prove, throws, times out, or no violation source — **hard-gates in BOTH interactive and autonomous modes** (ADR-550 D4 / D3). The evidence record gains a `failFirstProof` field recording HOW fail-first was proven (FF-07). A caller can no longer green a toothless check.
The violation is sourced from a new descriptor field, **`CheckDescriptor.violationFixture`** — an author-supplied path to a known-bad subject. For a `lint-rule` the prover lints that fixture and requires the rule id to appear in the JSON report (the rule must have teeth); for a `node-test` the prover spawns the negative test with the subject injected through the **`GSD_PROHIB_SUBJECT`** env convention and requires a NON-VACUOUS red — `# fail >= 1` AND a failing test named distinctly from the file (`isNonVacuousNodeTestRed`), so a violation fixture that merely CRASHES the test at load is not mistaken for the negative assertion firing red (symmetric with the clean-pass non-vacuity guard). The `lint-rule` path is fully shippable now and is dogfooded against the in-tree `local/no-source-grep` rule; the `node-test` path ships its mechanism (a fixture-bearing descriptor IS machine-proven) and is exercised by SYNTHETIC temp fixtures — there is no live in-tree `node --test` prohibition to dogfood. `CheckDescriptor.failFirst` is **DEMOTED, not removed** (FF-08): it is kept as a non-authoritative hint so the #1259 route-JSON shape and the `CheckDescriptor` type stay backward-compatible mid-migration, but no path greens on it alone. The green/fail-closed policy in `src/probe-core.cts` (`dispositionForProhibition`, reads only `evidence.length > 0`) is untouched; the evidence array shape is additive. This closes ADR-550's D5d follow-up — see the dated 2026-06-15 ADR-550 addendum. (#1279)
The violation is sourced from a new descriptor field, **`CheckDescriptor.violationFixture`** — an author-supplied path to a known-bad subject. For a `lint-rule` the prover lints that fixture and requires the rule id to appear in the JSON report (the rule must have teeth); for a `node-test` the prover spawns the negative test with the subject injected through the **`GSD_PROHIB_SUBJECT`** env convention and requires a NON-VACUOUS red — `# fail >= 1` AND a failing test named distinctly from the file (`isNonVacuousNodeTestRed`), so a violation fixture that merely CRASHES the test at load is not mistaken for the negative assertion firing red (symmetric with the clean-pass non-vacuity guard). The node-test prover also requires the `violationFixture` to EXIST (resolved against `cwd`) before spawning — a missing/typo'd path fail-CLOSES rather than letting an honest test's ENOENT crash forge a green (symmetric with the lint path's file-result guard). A documented residual — proving the red was *caused by* the violation rather than by the env merely being set — and threading a `check_violation_fixture` scalar through the #1278 projection so the deterministic path can green end-to-end are both tracked as follow-up **#1346**. The `lint-rule` path is fully shippable now and is dogfooded against the in-tree `local/no-source-grep` rule; the `node-test` path ships its mechanism (a fixture-bearing descriptor IS machine-proven) and is exercised by SYNTHETIC temp fixtures — there is no live in-tree `node --test` prohibition to dogfood. `CheckDescriptor.failFirst` is **DEMOTED, not removed** (FF-08): it is kept as a non-authoritative hint so the #1259 route-JSON shape and the `CheckDescriptor` type stay backward-compatible mid-migration, but no path greens on it alone. The green/fail-closed policy in `src/probe-core.cts` (`dispositionForProhibition`, reads only `evidence.length > 0`) is untouched; the evidence array shape is additive. This closes ADR-550's D5d follow-up — see the dated 2026-06-15 ADR-550 addendum. (#1279)
**PR-review flag — `GSD_PROHIB_SUBJECT` + `violationFixture` are PROPOSED, renamable conventions.** Both are net-new surface with ZERO live in-tree consumers (no in-tree node-test prohibition yet; node-test proof runs only on synthetic test fixtures, the real dogfood stays the lint-rule). They are forward-looking scaffolding, so a later rename — or replacing the env var with an argv — is a mechanical, zero-migration find/replace. Surfacing them here so the maintainer can **ratify, rename, or replace them at PR review** with no migration cost, exactly as #1278's ADR addendum was reviewed at PR time. The `failFirst` demotion is likewise open to weighing outright removal; the keep-as-hint rationale is recorded in the ADR addendum.

View File

@@ -113,6 +113,10 @@ This addendum ratifies three contract points:
Net effect on D4: the *guarantee* ("a `test`-tier prohibition is never a silent pass") was preserved at every step — fail-closed-now (#644), genuine-execution (#1259), and now **machine-proven fail-first (#1279)**. A `test`-tier prohibition reaches `green`/`passed` ONLY when the wired check both genuinely, non-vacuously passes AND is independently proven to fail on a violation; every miss/fail/un-provable hard-gates. The decision also lives in `src/prohibition-enforcement.cts` comments, `gsd-core/references/prohibition-probe.md`, `gsd-core/workflows/verify-phase.md`, and the #1279 changeset.
**Review corrections (#1314 maintainer review) — two soundness items, tracked:**
- **node-test fixture-existence guard (was fail-OPEN).** The node-test prover originally guarded only `if (!fixture)`. A missing/typo'd/stale `violationFixture` path made `GSD_PROHIB_SUBJECT` point at a non-existent file; an honest negative test then threw ENOENT *inside its callback* — a failing test named distinctly from the file — which `isNonVacuousNodeTestRed` accepted as proof, **forging a green from a setup crash** (asymmetric with the lint-rule path, which fail-CLOSES on `< 1` file result). Fixed by requiring `fs.existsSync(fixture)` before spawning (symmetric fail-closed). **Documented residual (#1346):** existence is necessary but not sufficient — a deceptive test that reds merely *because* `GSD_PROHIB_SUBJECT` is set (not because the subject's CONTENT violates) is still accepted; proving causation generically for an arbitrary author-supplied test is not possible, so it is recorded as a constraint, not implied-solved.
- **`violationFixture` has no projection source (#1278 ↔ #1279 do not yet compose).** `descriptorFromProjection` reconstructs only `{ kind, target, rule? }`; the projected `check_*` scalars carry **no** `violationFixture`. Since green now *requires* a fixture, a prohibition wired purely through the #1278 deterministic path **always hard-gates (fail-closed, safe)** until a `check_violation_fixture` scalar is threaded through — tracked as **#1346**. Until then green requires a hand-supplied `violationFixture`; `verify-phase.md` now states this explicitly rather than implying the projected path produces greens.
## Addendum (2026-06-15): optional `check` descriptor on the prohibition item — D3 shape extension (#1278)
This ratifies the **deterministic SOURCE** for the test-tier `CheckDescriptor` that #1259 (PR #1273) left caller/verifier-supplied. #1259 shipped the PRODUCER (`check prohibition-enforcement`) that *runs* a wired check given a `{kind, target, rule?}` descriptor, but the descriptor itself was invented by the verify-phase LLM each run (the "locate" half). #1278 makes that locate half **deterministic**: an optional `check` descriptor is authored at spec-phase on the resolved `test`-tier prohibition, projected by `projectProhibitions`, and read back by verify-phase — so a wired, passing test closes the gap with **zero manual authoring**. This extends the **Decision 3 prohibition-item shape** (it adds optional keys to that item), so it is ratified here rather than rewriting D3 in place.

View File

@@ -169,15 +169,20 @@ nested object would flatten/mangle the round-trip (ADR-550 2026-06-15 addendum;
rewrite" precedent). `projectProhibitions` emits these keys **only for a well-formed descriptor**
(valid `check_kind` + non-empty `check_target`; `check_rule` only on the lint-rule path), and
verify-phase reads them back via `descriptorFromProjection` into the `CheckDescriptor` handed to
`check prohibition-enforcement`. A wired, passing test then closes the gap with **zero manual
descriptor authoring**.
`check prohibition-enforcement`. This closes the **locate** half with **zero manual descriptor
authoring**. Note the projection carries **no `violationFixture`** (`descriptorFromProjection`
reconstructs only `{ kind, target, rule? }`); because #1279 machine-proves fail-first against a
fixture, a prohibition wired purely through this projected path **hard-gates (fail-closed)** until a
`check_violation_fixture` scalar is threaded through — tracked as **#1346**. Until then green needs a
hand-supplied `violationFixture`.
**Fail-closed + backward-compat.** A partial descriptor (`lint-rule` missing `check_rule`), an
unknown `check_kind`, or an **absent** descriptor on a test-tier prohibition falls through to the
producer's existing fail-closed locate — never a silent green. A prohibition with no descriptor
parses and disposes byte-identically to today. `failFirst` is **not** sourced from the
descriptor — it stays a verify-time caller attestation (machine-proven fail-first is tracked in
#1279; the `dispositionForProhibition` policy is unchanged).
descriptor and is now **demoted** (machine-proven fail-first DELIVERED in #1279 — no path greens on
attestation alone, FF-08); the `dispositionForProhibition` policy is unchanged. The field that gates
green and is **not yet projected** is `violationFixture` (follow-up #1346).
## Output schema

View File

@@ -70,7 +70,7 @@ Aggregate all must_haves across plans for phase-level verification.
**Prohibitions (`must_haves.prohibitions`, ADR-550 D3 — the must-NOT sibling block):** When a plan carries `must_haves.prohibitions`, extract each `{ statement, status, verification }` item and route it by `verification` tier in verdict assembly (ADR-550 D4, "B-with-guard", 2026-06-12 maintainer decision). These are NEGATIVE checks (the must-NOT must NOT have happened), distinct from positive `truths`:
- **judgment-tier → mode-dependent soft-gate.** Interactive verify defers each item to the end-of-phase human checkpoint (`human_verify_mode: end-of-phase`). Autonomous verify records a NON-AUTHORITATIVE LLM-judge verdict + a prominent `unverified-prohibition — human review recommended` flag (autonomous completion reads "complete with N flagged prohibitions"). NEVER a silent pass; NEVER a hard halt of an AFK run.
- **test-tier → ENFORCED via `check prohibition-enforcement` (green on pass, hard-gate on miss/fail).** Accept the `verification: test` value (the SPEC↔must_haves.prohibitions projection contract holds — no forced schema change later). For each test-tier item, the verifier builds `request.check` **DETERMINISTICALLY from the projected descriptor** — it does NOT invent `{ kind, target, rule }`. Read the flat scalar keys `check_kind` / `check_target` / `check_rule` off the `must_haves.prohibitions` item and reconstruct the `CheckDescriptor` via the `descriptorFromProjection` adapter in `prohibition-enforcement` (`descriptorFromProjection(projectedItem)` → `{ kind: check_kind, target: check_target, rule?: check_rule }`). Then attest `failFirst: true` in the request — `failFirst` is the ONE field NOT sourced from the projection; it stays a verify-time caller attestation (#1279 machine-proves it against a violation fixture). Invoke the producer (CLI surface unchanged):
- **test-tier → ENFORCED via `check prohibition-enforcement` (green on pass, hard-gate on miss/fail).** Accept the `verification: test` value (the SPEC↔must_haves.prohibitions projection contract holds — no forced schema change later). For each test-tier item, the verifier builds `request.check` **DETERMINISTICALLY from the projected descriptor** — it does NOT invent `{ kind, target, rule }`. Read the flat scalar keys `check_kind` / `check_target` / `check_rule` off the `must_haves.prohibitions` item and reconstruct the `CheckDescriptor` via the `descriptorFromProjection` adapter in `prohibition-enforcement` (`descriptorFromProjection(projectedItem)` → `{ kind: check_kind, target: check_target, rule?: check_rule }`). Supply a `violationFixture` (a path to a KNOWN-BAD subject) in the request — it is the field that gates **green**, and it is **NOT sourced from the projection today** (the projected `check_*` scalars carry no fixture; threading a `check_violation_fixture` scalar through is the tracked follow-up **#1346**). So with projection alone the item **hard-gates fail-closed** — green currently requires a hand-supplied `violationFixture`. Do NOT rely on `failFirst`: it is DEMOTED (#1279) and greens nothing on its own. Invoke the producer (CLI surface unchanged):
```bash
gsd_run check prohibition-enforcement <request.json>
@@ -80,7 +80,7 @@ Aggregate all must_haves across plans for phase-level verification.
- **`status: 'green'`, `flagged: false`** (a genuinely-passing wired negative test / lint rule, `located: true`, non-empty `evidence`) → the item is satisfiable → it can reach **passed**.
- **missing, non-attested, or genuinely-non-passing check** (`located: false` OR `status: 'unverified'`, `flagged: true`) → **hard-gate**: disposes flagged-unverified, NEVER green, routing to `gaps_found` in BOTH interactive and autonomous modes (a failing mechanical check blocks even AFK; ADR-550 D4 / D3). The deterministic fail-closed default backing every miss/fail is `dispositionForProhibition()` in probe-core (`status: 'unverified'`, `flagged: true` on empty `enforcementEvidence`).
> **Descriptor source — deterministic locate (#1278, DELIVERED).** The `check` descriptor's `{ kind, target, rule }` is now sourced **deterministically from the projected `check_kind` / `check_target` / `check_rule` scalars** on the `must_haves.prohibitions` item (authored at `/gsd:spec-phase`, projected by `projectProhibitions`, read back via the `descriptorFromProjection` adapter). So a wired passing test closes the gap with **zero manual descriptor authoring** — the verifier no longer invents the locate (removing the spoofable invent-at-verify-time surface; ADR-857 §147 exogenous grading). **Fail-closed is preserved:** an item with NO projected descriptor — or a PARTIAL one (e.g. a `lint-rule` missing `check_rule`) — makes `descriptorFromProjection` return `null` / an under-specified descriptor, which falls through to the producer's existing fail-closed LOCATE (`located: false`) → flagged-unverified, NEVER green, in BOTH modes. The only field still attested at verify time (not projected) is `failFirst`; machine-proving it against a violation fixture is the remaining **tracked follow-up: #1279**.
> **Descriptor source — deterministic locate (#1278, DELIVERED).** The `check` descriptor's `{ kind, target, rule }` is now sourced **deterministically from the projected `check_kind` / `check_target` / `check_rule` scalars** on the `must_haves.prohibitions` item (authored at `/gsd:spec-phase`, projected by `projectProhibitions`, read back via the `descriptorFromProjection` adapter). So the **locate** half closes with **zero manual descriptor authoring** — the verifier no longer invents `{ kind, target, rule }` (removing the spoofable invent-at-verify-time surface; ADR-857 §147 exogenous grading). **Fail-closed is preserved:** an item with NO projected descriptor — or a PARTIAL one (e.g. a `lint-rule` missing `check_rule`) — makes `descriptorFromProjection` return `null` / an under-specified descriptor, which falls through to the producer's existing fail-closed LOCATE (`located: false`) → flagged-unverified, NEVER green, in BOTH modes. **Important — the projection does NOT yet carry a `violationFixture`** (`descriptorFromProjection` reconstructs only `{ kind, target, rule? }`). Because #1279 now machine-proves fail-first against a `violationFixture`, a prohibition wired **purely through this projected path always hard-gates** (fail-closed, safe) until a `check_violation_fixture` scalar is threaded through — the **tracked follow-up #1346**. Until then a green requires the verifier to hand-supply `violationFixture`; `failFirst` is demoted and greens nothing (#1279, DELIVERED).
**Option B: Use Success Criteria from ROADMAP.md**

View File

@@ -539,7 +539,23 @@ function defaultProveFailFirst(check: CheckDescriptor, cwd: string, timeoutMs?:
}
if (check.kind === 'node-test') {
const fixture = check.violationFixture;
if (!fixture) return { provenFailFirst: false }; // no fixture -> hard-gate (NEVER attestation)
// Fail-CLOSED on a missing/typo'd/stale fixture path, SYMMETRIC with the lint-rule path's
// `eslintFileResultCount >= 1` guard. Without the existence check, a non-existent fixture makes
// `GSD_PROHIB_SUBJECT` point at a missing file; an honest negative test reading that subject
// throws ENOENT *inside its callback* — a failing test named DISTINCTLY from the file, which
// `isNonVacuousNodeTestRed` would accept as proof. That is fail-OPEN: a typo forges a green from
// a setup crash, not from the prohibition firing. Requiring the fixture to exist before spawning
// closes the realistic typo/stale-path case (#1279 review, Major 1).
//
// KNOWN RESIDUAL (documented, fail-open direction, tracked follow-up #1346): existence is
// necessary but not sufficient — a deliberately deceptive negative test that reds merely BECAUSE
// `GSD_PROHIB_SUBJECT` is set (rather than because the subject's CONTENT violates the must-NOT)
// is still accepted. Proving "the red was CAUSED BY the violation" cannot be done generically for
// an arbitrary author-supplied test, so it is recorded as a constraint, not silently implied-solved.
// Resolve the fixture against `cwd` (NOT the verify process's cwd): the spawned test reads
// `GSD_PROHIB_SUBJECT` and resolves a relative subject against `cwd`, so the existence check must
// use the SAME base or it could pass here yet ENOENT in the child (re-opening the fail-open hole).
if (!fixture || !fs.existsSync(path.resolve(cwd, fixture))) return { provenFailFirst: false };
let out = '';
try {
out = execFileSync(process.execPath, buildNodeTestArgs(check), {
@@ -632,7 +648,7 @@ export function runProhibitionEnforcement(
const passed = proof.provenFailFirst === true && run.passed === true;
if (!passed) {
// NOT attested fail-first OR did not genuinely pass -> fail-closed, located: true (an actual
// NOT machine-proven fail-first OR did not genuinely pass -> fail-closed, located: true (an actual
// located miss/fail). Hard-gate applies in BOTH modes; the disposition stays non-green / flagged.
const disposition = dispositionForProhibition(prohibition, { enforcementEvidence: [] });
return {

View File

@@ -847,6 +847,63 @@ describe('prohibition-enforcement defaultProveFailFirst REAL prover (#1279)', ()
'a node-test with no violationFixture cannot be proven -> hard-gate, never attestation');
});
test('node-test: an HONEST test + a non-existent violationFixture path is NOT proven (FF-05 fail-OPEN guard, #1314 Major 1)', (t) => {
const enforce = require(ENFORCEMENT_LIB);
const dir = createTempDir('prohib-ff-node-missingfix-');
t.after(() => cleanup(dir));
// REGRESSION (#1314 review, Major 1): a REAL, honest negative test (its target file EXISTS and
// loads cleanly) reads GSD_PROHIB_SUBJECT and fs.readFileSync's it. Point violationFixture at a
// MISSING path (the realistic author typo / stale / moved-fixture case). Before the fix the missing
// subject made the honest test throw ENOENT *inside its callback* — a failing test named distinctly
// from the file — which isNonVacuousNodeTestRed accepted as a genuine RED, FORGING provenFailFirst:true
// from a setup crash (fail-OPEN). The fs.existsSync(fixture) guard now fail-CLOSES this, symmetric
// with the lint-rule path. Note this is the SAME honest-test shape as the FF-03 red-direction test —
// only the fixture path is missing — so it is exactly the green-able producer minus a valid fixture.
const negTest = path.join(dir, 'neg.test.cjs');
fs.writeFileSync(negTest,
"const { test } = require('node:test');\n" +
"const assert = require('node:assert');\n" +
"const fs = require('node:fs');\n" +
"test('subject must not contain FORBIDDEN', () => {\n" +
" const subject = fs.readFileSync(process.env.GSD_PROHIB_SUBJECT, 'utf-8');\n" +
" assert.ok(!subject.includes('FORBIDDEN'), 'subject is clean');\n" +
"});\n");
const missingFixture = path.join(dir, 'does-not-exist-subject.txt'); // deliberately NOT written
const proof = enforce.defaultProveFailFirst(
{ kind: 'node-test', target: negTest, violationFixture: missingFixture },
dir,
);
assert.equal(proof.provenFailFirst, false,
'a missing/typo\'d violationFixture must NOT forge a green from the honest test\'s ENOENT crash (fail-closed, symmetric with lint-rule)');
});
test('node-test: a RELATIVE violationFixture is resolved against cwd (existence guard matches the child, #1314 Major 1)', (t) => {
const enforce = require(ENFORCEMENT_LIB);
const dir = createTempDir('prohib-ff-node-relfix-');
t.after(() => cleanup(dir));
// The fixture is named RELATIVELY; the prover runs with cwd=dir and sets GSD_PROHIB_SUBJECT to the
// raw relative name, which the child resolves against its cwd (=dir). The existence guard must use
// the SAME base (path.resolve(cwd, fixture)) — a bare existsSync against the verify process's cwd
// would not find it and would wrongly hard-gate a valid fixture. Proving TRUE here confirms the
// relative path is honored end-to-end and the guard is cwd-correct.
const negTest = path.join(dir, 'neg.test.cjs');
fs.writeFileSync(negTest,
"const { test } = require('node:test');\n" +
"const assert = require('node:assert');\n" +
"const fs = require('node:fs');\n" +
"test('subject must not contain FORBIDDEN', () => {\n" +
" const subject = fs.readFileSync(process.env.GSD_PROHIB_SUBJECT, 'utf-8');\n" +
" assert.ok(!subject.includes('FORBIDDEN'), 'subject is clean');\n" +
"});\n");
fs.writeFileSync(path.join(dir, 'bad-subject.txt'), 'this subject contains FORBIDDEN content\n');
const proof = enforce.defaultProveFailFirst(
{ kind: 'node-test', target: negTest, violationFixture: 'bad-subject.txt' }, // RELATIVE to cwd
dir,
);
assert.equal(proof.provenFailFirst, true,
'a relative violationFixture resolved against cwd is found, runs RED, and proves fail-first');
});
test('prover never throws: a non-existent fixture / unresolvable tooling -> provenFailFirst:false (FF-05/FF-06)', () => {
const enforce = require(ENFORCEMENT_LIB);
// Non-existent lint fixture path -> eslint lints nothing / errors -> fail-closed, no throw.