From c970fa878791416dbc34a2a9834dea0951548c6c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 12 Sep 2026 20:34:58 +0200 Subject: [PATCH] fix(drive-local): report runs the acceptance argv, not just prints DoD (#271) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: `report` only called verifiedPackage() which validates the acceptance contract's integrity (checkScope + validateAcceptance argv-shape checks) then printed the DoD strings verbatim. An unattended tick could exit "REPORT ... DoD: X" even when the DoD was unmet — the argv was never executed. After: `report` runs the same runChecks(pkg) path as `verify` (which was already correct), then re-asserts the package + prints REPORT lines + PACKAGE_VERIFIED. Scope refusal is unchanged (was already enforced via checkScope in verifiedPackage). The DoD is now executable in report, not decorative. Tests: - new: report refuses when the acceptance argv fails against the working tree - new: report prints PACKAGE_VERIFIED only after the argv passes - existing "reporting after SDK suite effects" tests: updated so the in-scope simulation preserves the DoD-required state (writing "broken" back would correctly fail post-#271; existing tests exercised scope enforcement, not DoD, so the fixture data was tightened to isolate the invariant) - existing "allowed untracked change" test: now also fixes value.txt so the DoD is met alongside the untracked file addition 10/10 review tests pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- ops/local-work-package.mjs | 9 +++++++++ ops/local-work-review.test.mjs | 28 +++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/ops/local-work-package.mjs b/ops/local-work-package.mjs index 75eb62b78..d25f47e69 100644 --- a/ops/local-work-package.mjs +++ b/ops/local-work-package.mjs @@ -221,7 +221,15 @@ async function verifiedPackage() { } async function report() { + // Assert scope AND run the acceptance argv before printing anything. Without + // this, an unattended local drive tick could exit "REPORT ... DoD: X" while + // the DoD is unmet: verifiedPackage only validates the contract's integrity, + // it does not execute the checks that prove the actual state satisfies it. + // The scope refusal is already covered by verifiedPackage -> checkScope; + // runChecks makes the DoD assertion executable rather than decorative (#271). const pkg = await verifiedPackage(); + runChecks(pkg); + await verifiedPackage(); // The package pins the HEAD it was selected against. Reporting a diff from a // different commit would describe work this tick did not do. const head = git('rev-parse', 'HEAD'); @@ -233,6 +241,7 @@ async function report() { console.log(`REPORT ${pkg.title}`); console.log(stat || ' (no working-tree changes)'); for (const item of pkg.definitionOfDone) console.log(` DoD: ${item}`); + console.log(`PACKAGE_VERIFIED: ${pkg.verificationCommands.length} check(s)`); } try { diff --git a/ops/local-work-review.test.mjs b/ops/local-work-review.test.mjs index ce2993ae3..1344c97a7 100644 --- a/ops/local-work-review.test.mjs +++ b/ops/local-work-review.test.mjs @@ -10,6 +10,7 @@ for (const kind of ['unstaged', 'staged', 'untracked']) { pass(f.run('select')); const path = kind === 'untracked' ? 'src/new.txt' : 'src/value.txt'; f.put(path, 'fixed'); + if (kind === 'untracked') f.put('src/value.txt', 'fixed'); // meet DoD alongside the untracked file (#271: report runs acceptance) if (kind === 'staged') f.git('add', path); pass(f.run('scope')); const report = f.run('report'); @@ -43,6 +44,28 @@ test('selection skips untracked scope and accepts the next committed scope', t = assert.match(result.stdout, /SELECTED Fix value/); }); +test('report refuses when the acceptance check fails against the working tree (#271)', t => { + const f = fixture(t); + pass(f.run('select')); + // Deliberately leave src/value.txt as its fixture-baseline "broken" — the DoD + // is unmet. Before #271 the report path only validated the acceptance + // contract's integrity and printed DoD strings verbatim, so it exited 0 with + // PACKAGE_VERIFIED regardless. After #271 report runs the argv and refuses. + const result = f.run('report'); + fail(result, /AssertionError|Expected|broken/); + assert.doesNotMatch(result.stdout, /PACKAGE_VERIFIED/); +}); + +test('report prints PACKAGE_VERIFIED only after the acceptance check passes (#271)', t => { + const f = fixture(t); + pass(f.run('select')); + f.put('src/value.txt', 'fixed'); + const result = f.run('report'); + pass(result); + assert.match(result.stdout, /REPORT Fix value/); + assert.match(result.stdout, /PACKAGE_VERIFIED: 1 check/); +}); + for (const path of ['outside.txt', 'src/value.txt']) { test(`reporting after SDK suite effects enforces scope for ${path}`, t => { const f = fixture(t); @@ -50,7 +73,10 @@ for (const path of ['outside.txt', 'src/value.txt']) { f.put('src/value.txt', 'fixed'); pass(f.run('verify')); // Model a Git-visible effect produced by the SDK suite after package checks. - f.put(path, 'suite effect'); + // For the in-scope case the "suite effect" must preserve the DoD-required + // state (writing "broken" back would clobber value.txt and correctly fail + // report's re-executed acceptance check post-#271); "fixed" preserves it. + f.put(path, path === 'src/value.txt' ? 'fixed' : 'suite effect'); let previous = 'verify'; let result; while (previous !== 'report') {