* 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 <trekkie@nomorestars.com>
This commit is contained in:
5
.changeset/quick-goats-rest.md
Normal file
5
.changeset/quick-goats-rest.md
Normal file
@@ -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)
|
||||
@@ -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,
|
||||
|
||||
@@ -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. ────────────────────
|
||||
|
||||
|
||||
Reference in New Issue
Block a user