* test(#4663): add failing-first contract coverage for the blocked-uat canonicalization gate verify-work.md's complete_session step flips VERIFICATION.md to passed on 'zero issues' alone, so a session whose every UAT row is blocked (a session that observed nothing) canonicalizes the report. Pins the deployed contract the fix must satisfy: the flip runs the unflagged phase uat-passed predicate inside the human_needed branch, frontmatter.set sits inside a passed==true guard, a refusal message carries the blocker count and keeps human_needed, and an indeterminate pre-check fails closed. All four new assertions are RED until the workflow grows the guard. * fix(#4663): gate the canonicalization on the uat-passed predicate complete_session flipped VERIFICATION.md to passed whenever the session recorded zero issues and the status was human_needed — but blocked rows are not issues by this workflow's own rule, so a 0-passed / 0-issues / N-blocked session (one that observed nothing) rewrote the canonical report to passed. Every later reader (transition.md's preliminary check, resume paths, validate-phase, verification.status) then inherited the unearned pass while the phase-close predicate correctly refused it. The flip now runs the phase-close predicate in a new --uat-only form before canonicalizing: UAT rows evaluated (at least one pass, no pending/blocked/failed/unexplained-skip row), VERIFICATION-status blockers skipped — they must be, because the report still reads human_needed at pre-check time and that status is itself a blocking verification entry, so the full predicate could never pass there and the flip would deadlock (found by isolated review, probed). The --require-verification call stays the transition gate; the refusal branch reports the blocker count and keeps human_needed; an indeterminate pre-check fails closed. Emitted-Drift-Ack-Growth: verify-work.md — canonicalize block gains the uat-only pre-check and refusal branch (#4663) * test(#4663): align the canonicalize pre-check needles with the shipped line The workflow line carries a 2>/dev/null redirect the needles did not include, so both pre-check assertions fail against the committed fix (fixed-string grep verified). Reviewer-found; needle and message aligned. * fix(#4663): reword the canonicalize prose and refresh its size baseline The rationale paragraph mentioned the flagged transition-gate call by its flag, putting a --require-verification literal before the first phase uat-passed occurrence and breaking the existing ordering pin; the prose now describes it without the literal. verify-work.md's growth also drifted the committed compact-content baseline; regenerated via benchmark-compact-content.cjs --write (derived artifact, report-not-gate contract). Emitted-Drift-Ack-Growth: verify-work.md — canonicalize block gains the uat-only pre-check and refusal branch (#4663) * docs(#4663): backfill changeset PR number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/humble-hawks-munch.md
Normal file
5
.changeset/humble-hawks-munch.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4809
|
||||
---
|
||||
**verify-work no longer canonicalizes a vacuous pass** — a UAT session with zero logged issues but every row blocked (0 passed) flipped VERIFICATION.md to `passed`, leaving a phase whose central claim was never observed carrying a passed verification. The canonicalize flip now requires the same `phase uat-passed` predicate the phase-close uses (at least one pass, no blocked/pending/failed rows) and, when it refuses, says the verification stays human_needed with the blocker count. (#4663)
|
||||
@@ -639,7 +639,7 @@ If an active secure-phase step hook exists AND `SECURITY_FILE` exists: check fro
|
||||
|
||||
If no active secure-phase step hook exists OR (`SECURITY_FILE` exists AND `threats_open` is `0`):
|
||||
|
||||
If execution verification is waiting only on human UAT and this session recorded zero issues, canonicalize the report before the shared completion predicate:
|
||||
If execution verification is waiting only on human UAT and this session recorded zero issues, canonicalize the report before the shared completion predicate. (#4663) Zero issues is NOT pass evidence on its own — blocked rows are not issues by this workflow's own rule, so a session that observed nothing (0 passed / 0 issues / N blocked) must NOT flip the report. The flip runs the SAME UAT-row predicate the phase-close uses, in its `--uat-only` form: it skips the verification-status blockers (the report still reads `human_needed` at this point — the full predicate could never pass here), and `passed` means at least one UAT check passed with no row pending/blocked/failed or skipped without a reason. The flagged transition-gate call below stays the final say on canonical verification:
|
||||
|
||||
```bash
|
||||
PHASE_DIR=$(printf '%s' "$INIT" | jq -r '.phase_dir // empty')
|
||||
@@ -648,7 +648,15 @@ VERIFICATION_STATUS=$(gsd_run query verification.status "$PHASE_DIR" 2>/dev/null
|
||||
VERIFICATION_STATUS_VALUE=$(printf '%s' "$VERIFICATION_STATUS" | jq -r '.status // empty' 2>/dev/null || echo "")
|
||||
PHASE_VERIFICATION_STATUS="$VERIFICATION_STATUS_VALUE"
|
||||
if [ "$VERIFICATION_STATUS_VALUE" = "human_needed" ]; then
|
||||
gsd_run query frontmatter.set "$VERIFICATION_FILE" --field status --value passed
|
||||
UAT_PRECHECK=$(gsd_run phase uat-passed "{phase}" --uat-only 2>/dev/null)
|
||||
UAT_PRECHECK_PASSED=$(printf '%s' "$UAT_PRECHECK" | jq -r '.passed // false' 2>/dev/null || echo "false")
|
||||
if [ "$UAT_PRECHECK_PASSED" = "true" ]; then
|
||||
gsd_run query frontmatter.set "$VERIFICATION_FILE" --field status --value passed
|
||||
else
|
||||
UAT_BLOCKERS=$(printf '%s' "$UAT_PRECHECK" | jq -r '.blockers | length' 2>/dev/null)
|
||||
[ -n "$UAT_BLOCKERS" ] || UAT_BLOCKERS="?"
|
||||
echo "NOT canonicalizing: ${UAT_BLOCKERS} UAT row(s) blocked or not passing; verification stays human_needed. Resolve or pass them, then re-run /gsd:verify-work {phase}." >&2
|
||||
fi
|
||||
fi
|
||||
```
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@ interface PhaseHandlers {
|
||||
) => void;
|
||||
cmdPhaseRemove: (cwd: string, phaseNum: string, opts: { force: boolean }, raw: boolean) => void;
|
||||
cmdPhaseComplete: (cwd: string, phaseNum: string | undefined, raw: boolean) => void;
|
||||
cmdPhaseUatPassed: (cwd: string, phaseNum: string | undefined, raw: boolean, opts?: { policy?: { requireVerification?: boolean } }) => void;
|
||||
cmdPhaseUatPassed: (cwd: string, phaseNum: string | undefined, raw: boolean, opts?: { policy?: { requireVerification?: boolean; uatOnly?: boolean } }) => void;
|
||||
cmdPhaseListPlans: (cwd: string, phaseNum: string | undefined, raw: boolean) => void;
|
||||
}
|
||||
|
||||
@@ -222,10 +222,14 @@ function routePhaseCommand({ phase, args, cwd, raw, error }: RoutePhaseCommandOp
|
||||
},
|
||||
'uat-passed': (_ctx: Record<string, unknown>): { ok: true; data: null } => {
|
||||
let requireVerification = false;
|
||||
let uatOnly = false;
|
||||
const positional: string[] = [];
|
||||
for (const token of args.slice(2)) {
|
||||
if (token === '--require-verification') {
|
||||
requireVerification = true;
|
||||
} else if (token === '--uat-only') {
|
||||
// #4663: evaluate UAT rows only (verification-status blockers skipped).
|
||||
uatOnly = true;
|
||||
} else if (token === '--raw') {
|
||||
// --raw is handled by the outer CLI layer; accepted here silently
|
||||
} else if (token.startsWith('--')) {
|
||||
@@ -234,7 +238,13 @@ function routePhaseCommand({ phase, args, cwd, raw, error }: RoutePhaseCommandOp
|
||||
positional.push(token);
|
||||
}
|
||||
}
|
||||
phase.cmdPhaseUatPassed(cwd, positional[0], raw, { policy: { requireVerification } });
|
||||
if (requireVerification && uatOnly) {
|
||||
return makeInvalidArgs(
|
||||
'--uat-only',
|
||||
'--uat-only and --require-verification are mutually exclusive',
|
||||
) as never;
|
||||
}
|
||||
phase.cmdPhaseUatPassed(cwd, positional[0], raw, { policy: { requireVerification, uatOnly } });
|
||||
return { ok: true as const, data: null };
|
||||
},
|
||||
// #1437 — list plan files for a phase
|
||||
|
||||
@@ -4693,7 +4693,7 @@ function cmdPhaseUatPassed(
|
||||
cwd: string,
|
||||
phaseNum: string | undefined,
|
||||
raw: boolean,
|
||||
opts: { policy?: { requireVerification?: boolean } } = {},
|
||||
opts: { policy?: { requireVerification?: boolean; uatOnly?: boolean } } = {},
|
||||
): void {
|
||||
if (!phaseNum) {
|
||||
error('phase number required for phase uat-passed');
|
||||
|
||||
@@ -55,6 +55,14 @@ interface UatPassedReport {
|
||||
no_uat_artifacts: boolean;
|
||||
policy: {
|
||||
require_verification: boolean;
|
||||
/**
|
||||
* #4663: true when the report was produced in uat-only mode — UAT rows
|
||||
* evaluated, VERIFICATION-file blockers (and only those) skipped. The
|
||||
* verify-work canonicalize pre-check uses this form; the flip it gates is
|
||||
* what removes the human_needed verification status, so the full
|
||||
* predicate could never pass at pre-check time.
|
||||
*/
|
||||
uat_only: boolean;
|
||||
};
|
||||
/**
|
||||
* #3057 B3: true when the `requireVerification` policy check's own
|
||||
@@ -283,9 +291,15 @@ function parseUatResultItems(cleanContent: string): Array<{ test: number; name:
|
||||
*/
|
||||
function evaluateUatPassed(
|
||||
phaseFullDir: string,
|
||||
opts?: { policy?: { requireVerification?: boolean } },
|
||||
opts?: { policy?: { requireVerification?: boolean; uatOnly?: boolean } },
|
||||
): UatPassedReport {
|
||||
const requireVerification = opts?.policy?.requireVerification === true;
|
||||
// uatOnly (#4663) takes precedence: it evaluates the UAT rows ONLY, skipping
|
||||
// the VERIFICATION-file blockers entirely. The verify-work canonicalize
|
||||
// pre-check needs exactly that — it runs while the report still reads
|
||||
// `human_needed`, which is itself a blocking verification status, so the
|
||||
// full predicate could never pass there and the flip would deadlock.
|
||||
const uatOnly = opts?.policy?.uatOnly === true;
|
||||
const requireVerification = !uatOnly && opts?.policy?.requireVerification === true;
|
||||
|
||||
const blockers: string[] = [];
|
||||
const checks: UatCheckItem[] = [];
|
||||
@@ -309,7 +323,7 @@ function evaluateUatPassed(
|
||||
checks: [],
|
||||
blockers,
|
||||
no_uat_artifacts,
|
||||
policy: { require_verification: requireVerification },
|
||||
policy: { require_verification: requireVerification, uat_only: uatOnly },
|
||||
// readVerificationStatus was never reached on this early-return path.
|
||||
verification_stale_check_indeterminate: false,
|
||||
};
|
||||
@@ -392,7 +406,7 @@ function evaluateUatPassed(
|
||||
|
||||
// ── Process VERIFICATION files ─────────────────────────────────────────────
|
||||
let hasPassingVerification = false;
|
||||
for (const file of verFileNames) {
|
||||
for (const file of uatOnly ? [] : verFileNames) {
|
||||
verificationFiles.push(file);
|
||||
const verificationFilePath = path.join(phaseFullDir, file);
|
||||
let raw = '';
|
||||
@@ -451,6 +465,7 @@ function evaluateUatPassed(
|
||||
no_uat_artifacts,
|
||||
policy: {
|
||||
require_verification: requireVerification,
|
||||
uat_only: uatOnly,
|
||||
},
|
||||
verification_stale_check_indeterminate: verificationStaleCheckIndeterminate,
|
||||
};
|
||||
|
||||
@@ -33,14 +33,14 @@
|
||||
"reductionPct": 11.9
|
||||
},
|
||||
"verify-work": {
|
||||
"offTokens": 12664,
|
||||
"onTokens": 10691,
|
||||
"reductionPct": 15.58
|
||||
"offTokens": 12992,
|
||||
"onTokens": 11019,
|
||||
"reductionPct": 15.19
|
||||
}
|
||||
},
|
||||
"aggregate": {
|
||||
"offTokens": 108302,
|
||||
"onTokens": 91615,
|
||||
"reductionPct": 15.41
|
||||
"offTokens": 108630,
|
||||
"onTokens": 91943,
|
||||
"reductionPct": 15.36
|
||||
}
|
||||
}
|
||||
|
||||
@@ -599,6 +599,62 @@ describe('evaluateUatPassed — policy.requireVerification', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── evaluateUatPassed — policy.uatOnly (#4663) ───────────────────────────────
|
||||
//
|
||||
// The verify-work canonicalize pre-check runs WHILE the report still reads
|
||||
// `human_needed` — and `human_needed` is itself a blocking verification
|
||||
// status, so the full predicate can never pass at pre-check time and the
|
||||
// flip would deadlock. uatOnly evaluates the UAT rows ONLY: verification-file
|
||||
// blockers (and only those) are skipped, while every UAT-row blocker
|
||||
// (pending/blocked/failed/skip-without-reason) and the vacuous-pass guard
|
||||
// still apply.
|
||||
|
||||
describe('evaluateUatPassed — policy.uatOnly (#4663)', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = makeTmpDir();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmDir(tmpDir);
|
||||
});
|
||||
|
||||
test('uatOnly ignores the human_needed verification blocker the flip is about to remove', () => {
|
||||
writeFile(tmpDir, 'phase-UAT.md', makePassingUat(1));
|
||||
writeFile(tmpDir, 'phase-VERIFICATION.md', '---\nstatus: human_needed\n---\n\nWaiting on hardware.');
|
||||
const full = evaluateUatPassed(tmpDir, {});
|
||||
assert.strictEqual(full.passed, false,
|
||||
'the full predicate must refuse while verification is human_needed (the blocker the flip removes)');
|
||||
assert.ok(full.blockers.some(b => /human_needed/.test(b)),
|
||||
`expected a human_needed blocker, got: ${JSON.stringify(full.blockers)}`);
|
||||
const report = evaluateUatPassed(tmpDir, { policy: { uatOnly: true } });
|
||||
assert.strictEqual(report.passed, true, 'uat-only: all UAT rows pass → the pre-check may flip');
|
||||
assert.strictEqual(report.blockers.length, 0);
|
||||
assert.strictEqual(report.policy.uat_only, true);
|
||||
assert.strictEqual(report.policy.require_verification, false);
|
||||
});
|
||||
|
||||
test('uatOnly still refuses blocked UAT rows — the vacuous pass stays impossible', () => {
|
||||
writeFile(tmpDir, 'phase-UAT.md', [
|
||||
'---', 'status: human_needed', '---', '', '# UAT', '',
|
||||
'### 1. Test 1', 'expected: It works', 'result: blocked', '',
|
||||
].join('\n'));
|
||||
writeFile(tmpDir, 'phase-VERIFICATION.md', '---\nstatus: human_needed\n---\n\nWaiting on hardware.');
|
||||
const report = evaluateUatPassed(tmpDir, { policy: { uatOnly: true } });
|
||||
assert.strictEqual(report.passed, false, 'a blocked row is a non-pass result even in uat-only mode');
|
||||
assert.ok(report.blockers.length > 0);
|
||||
});
|
||||
|
||||
test('uatOnly and requireVerification are mutually exclusive — uatOnly wins', () => {
|
||||
writeFile(tmpDir, 'phase-UAT.md', makePassingUat(1));
|
||||
const report = evaluateUatPassed(tmpDir, { policy: { uatOnly: true, requireVerification: true } });
|
||||
assert.strictEqual(report.passed, true, 'uatOnly takes precedence; the verification policy is not evaluated');
|
||||
assert.strictEqual(report.policy.require_verification, false);
|
||||
assert.strictEqual(report.policy.uat_only, true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── evaluateUatPassed — #3057 B3: staleness-check indeterminate is surfaced ──
|
||||
//
|
||||
// readVerificationStatus's internal staleness check can fail (fs /
|
||||
|
||||
@@ -129,6 +129,58 @@ describe('verify-work.md — auto-transition after UAT passes with 0 issues', ()
|
||||
});
|
||||
});
|
||||
|
||||
// ── #4663 — the canonicalize flip requires the UAT predicate, not a vacuous zero ──
|
||||
// "zero issues" is not pass evidence: blocked rows are not issues by this same
|
||||
// workflow's rule, so a 0-passed / 0-issues / N-blocked session must NOT flip
|
||||
// VERIFICATION.md to `passed`. The flip now consumes the SAME predicate the
|
||||
// phase-close uses in its --uat-only form (the verification-status blocker
|
||||
// is exactly what the flip removes, so the full predicate could never pass
|
||||
// at pre-check time); the flagged call stays the later transition gate.
|
||||
describe('verify-work.md — canonicalize flip is gated by the UAT predicate (#4663)', () => {
|
||||
test('canonicalize flips to passed only when the uat-passed predicate reports passed (#4663)', () => {
|
||||
const content = fs.readFileSync(VERIFY_WORK, 'utf-8');
|
||||
const humanNeededIdx = content.indexOf('if [ "$VERIFICATION_STATUS_VALUE" = "human_needed" ]; then');
|
||||
const precheckIdx = content.indexOf('UAT_PRECHECK=$(gsd_run phase uat-passed "{phase}" --uat-only 2>/dev/null)');
|
||||
const flipGuardIdx = content.indexOf('if [ "$UAT_PRECHECK_PASSED" = "true" ]; then');
|
||||
const setPassedIdx = content.indexOf('gsd_run query frontmatter.set "$VERIFICATION_FILE" --field status --value passed');
|
||||
|
||||
assert.ok(precheckIdx !== -1, 'the canonicalize block must run the uat-passed predicate before flipping');
|
||||
assert.ok(humanNeededIdx !== -1 && precheckIdx > humanNeededIdx, 'the pre-check must sit inside the human_needed branch');
|
||||
assert.ok(content.includes(".passed // false"), 'the verdict must be extracted from the typed report with a false default');
|
||||
assert.ok(flipGuardIdx !== -1 && flipGuardIdx > precheckIdx, 'the flip must be guarded on the extracted passed verdict');
|
||||
assert.ok(setPassedIdx > flipGuardIdx, 'frontmatter.set must sit INSIDE the passed==true guard');
|
||||
});
|
||||
|
||||
test('the canonicalize pre-check runs uat-passed without --require-verification (#4663)', () => {
|
||||
const content = fs.readFileSync(VERIFY_WORK, 'utf-8');
|
||||
const precheckIdx = content.indexOf('UAT_PRECHECK=$(gsd_run phase uat-passed "{phase}" --uat-only 2>/dev/null)');
|
||||
const flaggedIdx = content.indexOf('PHASE_COMPLETE=$(gsd_run phase uat-passed "{phase}" --require-verification)');
|
||||
|
||||
assert.ok(precheckIdx !== -1, 'the --uat-only pre-check must exist');
|
||||
assert.ok(
|
||||
!content.slice(precheckIdx, precheckIdx + 120).includes('--require-verification'),
|
||||
'the pre-check is the unflagged predicate - requiring verification there would evaluate the very report being written'
|
||||
);
|
||||
assert.ok(flaggedIdx !== -1 && flaggedIdx > precheckIdx, 'the flagged predicate remains the later transition gate');
|
||||
});
|
||||
|
||||
test('refused canonicalization says the verification stays human_needed (#4663)', () => {
|
||||
const content = fs.readFileSync(VERIFY_WORK, 'utf-8');
|
||||
assert.match(content, /stays human_needed/, 'the refusal message must say verification stays human_needed');
|
||||
assert.match(content, /blockers \| length/, 'the refusal must carry the blocking-row count');
|
||||
});
|
||||
|
||||
test('an indeterminate pre-check must not flip the report (fail closed) (#4663)', () => {
|
||||
const content = fs.readFileSync(VERIFY_WORK, 'utf-8');
|
||||
// jq -r '.passed // false' with an `|| echo "false"` fallback: empty or
|
||||
// failed gsd_run output must yield no-flip, never a flip.
|
||||
assert.ok(
|
||||
content.includes(`jq -r '.passed // false' 2>/dev/null || echo "false"`),
|
||||
'the extraction must default to false on empty/failed output'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// Folded from tests/bug-3381-verify-work-workstream.test.cjs — consolidation epic #1969 (B4 #1973)
|
||||
|
||||
Reference in New Issue
Block a user