From 590edec7a7008f41f6db783541418b5b3e8eb297 Mon Sep 17 00:00:00 2001 From: Rezolv Date: Thu, 3 Sep 2026 17:46:50 -0400 Subject: [PATCH] fix(#3956): require positive evidence for verify artifacts/key-links pass (#4004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3956): require positive evidence for verify artifacts/key-links pass An all-string or path-less must_haves.artifacts / key_links block is item-by-item skipped, leaving zero checked results, yet the pass verdict was computed as `passed === results.length` (0 === 0), so all_passed / all_verified read true with status valid and exit 0: a silent false GREEN over zero acceptance evidence. Add a positive-evidence floor (results.length > 0) to both verdicts, mirroring the no-vacuous-pass rule at src/uat-predicate.cts. A well-formed block, the fully-empty-block error, the parser's string tolerance, and key-links pending (#1202) semantics are all unchanged. Governing: ADR-3473 section 8 / 37C (absence, emptiness and failure must not encode as success) and Decision 3 (failure is a value). * chore(#3956): add changeset for verify vacuous-pass fix * test(#3956): add mixed-block coverage and correct the key-links vacuous-pass comment Addresses review on #4004: - Correct the cmdVerifyKeyLinks positive-evidence-floor comment: only bare-string items are continue-skipped; a from:-less object is NOT skipped (it falls through to a verified:false hard failure), so it was never part of the vacuous-pass surface. The prior comment overclaimed symmetry with the artifacts side. - Add a mixed-block regression test per verb (one bare-string prose bullet + one well-formed entry): the string is skipped, results.length === 1 > 0, and the verdict follows the single real entry — pinning that the floor does not over-reject a partial block. - Tighten the changeset wording to match (all-bare-string, not "no path:/from: key"). --------- Co-authored-by: Tom Boucher --- .changeset/quick-goats-rest.md | 5 ++ src/verify.cts | 23 +++++++-- tests/verify.test.cjs | 89 ++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 .changeset/quick-goats-rest.md diff --git a/.changeset/quick-goats-rest.md b/.changeset/quick-goats-rest.md new file mode 100644 index 000000000..cefc8bbb5 --- /dev/null +++ b/.changeset/quick-goats-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4004 +--- +`gsd-tools verify artifacts` and `verify key-links` no longer report a phase as fully verified when its `must_haves` block was authored entirely as prose bullets. A block whose items are all bare strings (no checkable `path:`/`from:` entry) is now reported as `invalid` with `total: 0` instead of a silent all-passed GREEN over zero checks, so a phase with no verifiable acceptance evidence can no longer read green. A block that mixes a prose bullet with a real entry is unaffected — the string is skipped and the verdict follows the checkable entry. (#3956) diff --git a/src/verify.cts b/src/verify.cts index 8ca5b8aab..84ee1db18 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -1200,15 +1200,22 @@ function cmdVerifyArtifacts(cwd: string, planFilePath: string, raw: boolean): vo } const passed = results.filter((r) => r['passed']).length; + // Positive-evidence floor (#3956): a non-empty artifacts block whose items are + // all bare strings / path-less objects is item-by-item skipped, leaving results + // empty; `passed === results.length` would then be `0 === 0` → a vacuous GREEN + // over zero checks. Require at least one checked artifact, mirroring the + // no-vacuous-pass rule at src/uat-predicate.cts. The fully-empty block is still + // caught earlier by the `artifacts.length === 0` guard and returns its error. + const allPassed = results.length > 0 && passed === results.length; output( { - all_passed: passed === results.length, + all_passed: allPassed, passed, total: results.length, artifacts: results, }, raw, - passed === results.length ? 'valid' : 'invalid', + allPassed ? 'valid' : 'invalid', ); } @@ -1417,7 +1424,17 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi // A pending link (from: file promised by a same-or-later-wave plan) is not a // hard failure — it should not count against the all_verified gate (#1202). const hardFailed = results.filter((r) => !r['verified'] && !r['pending']).length; - const allVerified = hardFailed === 0; + // Positive-evidence floor (#3956): an all-bare-string key_links block skips + // every item (only `typeof link === 'string'` items are continue-skipped + // above), leaving results empty; `hardFailed === 0` would then be a vacuous + // GREEN over zero checks. Require at least one checked link. (A `from:`-less + // object is NOT skipped, unlike a path-less object on the artifacts side — it + // falls through to a `verified: false` result and hard-fails, so it was never + // part of the vacuous-pass surface; only the all-bare-string case is.) A + // pending link IS pushed to results (with pending: true), so an all-pending + // block still satisfies results.length > 0 and its #1202 non-hard-failing + // semantics are unchanged — the floor only rejects the zero-result case. + const allVerified = results.length > 0 && hardFailed === 0; output( { all_verified: allVerified, diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index e21e57561..c2d193ba3 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -1620,6 +1620,50 @@ describe('verify artifacts command', () => { `Expected "No must_haves.artifacts" in error: ${output.error}` ); }); + + // A non-empty artifacts block whose items are all bare strings (prose bullets + // with no `path:` key) is item-by-item skipped, leaving zero checked results. + // The verdict must not read GREEN over an empty result set — mirrors the + // positive-evidence floor at src/uat-predicate.cts (no vacuous pass). (#3956) + test('does not report a vacuous pass for an all-string artifacts block (#3956)', () => { + writePlanWithArtifacts(tmpDir, [ + '- login flow implemented', + '- user can reset password', + ]); + + const result = runGsdTools('verify artifacts .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.total, 0, `Expected zero checked artifacts: ${JSON.stringify(output)}`); + assert.strictEqual( + output.all_passed, + false, + `Expected all_passed false over a zero-check block: ${JSON.stringify(output)}` + ); + }); + + // A MIXED artifacts block (one bare-string prose bullet + one well-formed + // `path:` entry) must not be disturbed by the positive-evidence floor: the + // string is item-skipped, the real entry is checked, results.length === 1 > 0, + // and the verdict follows that single item — not a vacuous pass, not a false + // fail. Guards the floor against over-rejecting a partial block. (#3956) + test('mixed artifacts block: bare string is skipped, real entry drives a passing verdict (#3956)', () => { + writePlanWithArtifacts(tmpDir, [ + '- login flow implemented', + '- path: "src/app.js"', + ' min_lines: 2', + ' contains: "export"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'app.js'), 'const x = 1;\nexport default x;\n'); + + const result = runGsdTools('verify artifacts .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.total, 1, `Expected exactly the one checkable entry: ${JSON.stringify(output)}`); + assert.strictEqual(output.all_passed, true, `Expected all_passed true from the real entry: ${JSON.stringify(output)}`); + }); }); // ───────────────────────────────────────────────────────────────────────────── @@ -1851,6 +1895,51 @@ describe('verify key-links command', () => { ); }); + // A non-empty key_links block whose items are all bare strings (prose bullets + // with no `from:` key) is item-by-item skipped, leaving zero checked results. + // A pending link (a `from:` file promised by a same-or-later-wave plan) is a + // real parsed object that IS pushed to results, so this floor keys on the + // empty-result case only and does not disturb #1202 pending semantics. (#3956) + test('does not report a vacuous pass for an all-string key_links block (#3956)', () => { + writePlanWithKeyLinks(tmpDir, [ + '- source calls the reset endpoint', + '- token is persisted', + ]); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.total, 0, `Expected zero checked links: ${JSON.stringify(output)}`); + assert.strictEqual( + output.all_verified, + false, + `Expected all_verified false over a zero-check block: ${JSON.stringify(output)}` + ); + }); + + // A MIXED key_links block (one bare-string prose bullet + one well-formed + // `from:`/`to:` link) must not be disturbed by the positive-evidence floor: + // only the bare string is skipped, the real link is checked, results.length + // === 1 > 0, and the verdict follows that single link. (#3956) + test('mixed key_links block: bare string is skipped, real link drives a passing verdict (#3956)', () => { + writePlanWithKeyLinks(tmpDir, [ + '- source calls the reset endpoint', + '- from: "src/a.js"', + ' to: "src/b.js"', + ' pattern: "import.*b"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'a.js'), "import { x } from './b';\n"); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'exports.x = 1;\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.total, 1, `Expected exactly the one checkable link: ${JSON.stringify(output)}`); + assert.strictEqual(output.all_verified, true, `Expected all_verified true from the real link: ${JSON.stringify(output)}`); + }); + // ── #3493: path confinement — from:/to: are untrusted plan frontmatter and // must never be readable outside the project directory. ────────────────────