* refactor(#651): consolidate verification-status routing into one queryable seam The passed/gaps_found/human_needed verification status was re-encoded as bare strings across three prose surfaces (gsd-verifier emits, execute-phase routes, ship gates), each independently deciding the per-status next action with no parity coupling — the DEFECT.GENERATIVE-FIX class. Give the enum one home: src/verification.cts (-> bin/lib/verification.cjs) exposing `gsd_run query verification.status <phaseDir>` returning a typed {status, next_action, next_command}. ship.md and execute-phase.md now consume the query instead of re-deriving the routing in prose; gsd-verifier.md points at the shared vocabulary as the single emitter (values unchanged). Also fixes the latent broad-grep status misread (DEFECT.FRONTMATTER-SCALAR- BROAD-GREP): execute-phase.md read `grep "^status:"` over the whole report, so a body `status:` line could misroute a valid phase. Extraction is now frontmatter-scoped in one place. A parity test fails if a verifier status gains no route. Lands the two CONTEXT.md DEFECT entries captured on the issue. Closes #651 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#651): set changeset pr to 755 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -1,112 +0,0 @@
|
||||
'use strict';
|
||||
// allow-test-rule: runtime-contract-is-the-product
|
||||
// The ship.md verification gate is LLM-executed prose; its routing message text
|
||||
// IS the user-facing product surface (RULESET.TESTS.no-source-grep.exemption:
|
||||
// "reserved for tests where the file content IS the product surface ... agent .md").
|
||||
// The behavioral tests below additionally EXECUTE the gate's own bash extraction
|
||||
// pipeline (parsed out of ship.md) against fixture reports, so the extraction
|
||||
// contract is verified, not just asserted as text.
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
const helpers = require('./helpers.cjs');
|
||||
|
||||
const SHIP_MD = path.join(__dirname, '..', 'gsd-core', 'workflows', 'ship.md');
|
||||
const ship = fs.readFileSync(SHIP_MD, 'utf8');
|
||||
|
||||
const start = ship.indexOf('**Verification passed?**');
|
||||
const end = ship.indexOf('**Clean working tree?**');
|
||||
assert.ok(start !== -1 && end !== -1 && end > start, 'could not locate the verification gate block');
|
||||
const gate = ship.slice(start, end);
|
||||
|
||||
// ---- content assertions (the routing message IS the product surface) ----
|
||||
|
||||
test('gate captures the status value with a single first-match grep', () => {
|
||||
assert.match(gate, /grep -m1 "\^status:"/, 'gate must extract status via grep -m1 "^status:"');
|
||||
});
|
||||
|
||||
test('gate scopes status extraction to the YAML frontmatter only', () => {
|
||||
assert.match(gate, /sed -n '\/\^---\$\/,\/\^---\$\/p'/, 'gate must restrict extraction to the frontmatter block');
|
||||
});
|
||||
|
||||
test('gate routes gaps_found to /gsd:plan-phase --gaps', () => {
|
||||
assert.match(gate, /gaps_found/);
|
||||
assert.match(gate, /\/gsd:plan-phase[^\n]*--gaps/);
|
||||
});
|
||||
|
||||
test('gate routes human_needed to the UAT manual-test step', () => {
|
||||
assert.match(gate, /human_needed/);
|
||||
assert.match(gate, /UAT\.md/);
|
||||
});
|
||||
|
||||
test('gate routes a missing VERIFICATION.md to re-running execute-phase', () => {
|
||||
assert.match(gate, /\/gsd:execute-phase/);
|
||||
});
|
||||
|
||||
test('gate still blocks with PHASE_VERIFICATION_INCOMPLETE', () => {
|
||||
assert.match(gate, /PHASE_VERIFICATION_INCOMPLETE/);
|
||||
});
|
||||
|
||||
test('the dead `pass` status arm is gone — only `passed` is accepted', () => {
|
||||
assert.doesNotMatch(gate, /status:\s*pass(?!ed)/i, 'no bare `status: pass` arm may remain');
|
||||
assert.doesNotMatch(gate, /`pass`\s*\/\s*`passed`/, 'the `pass` / `passed` either-arm must be removed');
|
||||
assert.match(gate, /passed/);
|
||||
});
|
||||
|
||||
// ---- behavioral tests: run the gate's OWN bash pipeline against fixtures ----
|
||||
|
||||
const bashBlock = (() => {
|
||||
// `\r?\n` (not a literal `\n`) so the fence matches on Windows CRLF checkouts;
|
||||
// normalize the captured block to LF before handing it to bash.
|
||||
const m = gate.match(/```bash\r?\n([\s\S]*?)```/);
|
||||
assert.ok(m, 'gate must contain a bash block');
|
||||
return m[1].replace(/\r\n/g, '\n');
|
||||
})();
|
||||
|
||||
const hasBash = (() => {
|
||||
try { execFileSync('bash', ['-c', 'true'], { stdio: 'ignore' }); return true; }
|
||||
catch { return false; }
|
||||
})();
|
||||
|
||||
// The extraction logic is platform-independent; the bash *pipeline* is executed
|
||||
// only where the gate's shell actually runs (POSIX). On Windows, git-bash exists
|
||||
// but receives Windows-style tmpdir paths it cannot glob, so skip execution there.
|
||||
const skipBashPipeline = (process.platform === 'win32' || !hasBash) && 'bash pipeline runs on POSIX only';
|
||||
|
||||
function runGateExtraction(verificationContents) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ship586-'));
|
||||
try {
|
||||
if (verificationContents !== null) {
|
||||
fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), verificationContents);
|
||||
}
|
||||
const script = `PHASE_DIR='${dir}'\n${bashBlock}\nprintf '%s' "$STATUS"`;
|
||||
return execFileSync('bash', ['-c', script], { encoding: 'utf8' });
|
||||
} finally {
|
||||
helpers.cleanup(dir);
|
||||
}
|
||||
}
|
||||
|
||||
const FM = (status) =>
|
||||
`---\nphase: 01-demo\nverified: 2026-01-01T00:00:00Z\nstatus: ${status}\nscore: 3/3 must-haves verified\n---\n\n# Verification\n`;
|
||||
|
||||
test('extraction yields passed for a passing frontmatter', { skip: skipBashPipeline }, () => {
|
||||
assert.strictEqual(runGateExtraction(FM('passed')), 'passed');
|
||||
});
|
||||
|
||||
test('extraction yields gaps_found / human_needed verbatim', { skip: skipBashPipeline }, () => {
|
||||
assert.strictEqual(runGateExtraction(FM('gaps_found')), 'gaps_found');
|
||||
assert.strictEqual(runGateExtraction(FM('human_needed')), 'human_needed');
|
||||
});
|
||||
|
||||
test('REGRESSION: a body `status:` line does not corrupt a passing report (Codex PR #650 finding)', { skip: skipBashPipeline }, () => {
|
||||
const withBodyStatus = FM('passed') +
|
||||
'\n## Example\n\n```yaml\nstatus: gaps_found\n```\n\nstatus: human_needed\n';
|
||||
assert.strictEqual(runGateExtraction(withBodyStatus), 'passed');
|
||||
});
|
||||
|
||||
test('extraction yields empty when no VERIFICATION.md exists', { skip: skipBashPipeline }, () => {
|
||||
assert.strictEqual(runGateExtraction(null), '');
|
||||
});
|
||||
331
tests/verification-status.test.cjs
Normal file
331
tests/verification-status.test.cjs
Normal file
@@ -0,0 +1,331 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Tests for verification-status module (issue #651).
|
||||
*
|
||||
* Covers:
|
||||
* 1. status: passed → routing
|
||||
* 2. status: gaps_found with phase token extraction
|
||||
* 3. status: human_needed → routing
|
||||
* 4. No *-VERIFICATION.md → 'missing'
|
||||
* 5. Frontmatter status present but unknown value → 'unknown'
|
||||
* 6. BROAD-GREP REGRESSION: body `status:` lines ignored, frontmatter wins
|
||||
* 7. PARITY: VERIFIER_STATUSES covered by routing table; gsd-verifier.md emitted statuses covered
|
||||
* 8. CRLF line endings in frontmatter
|
||||
* 9. Body-only file (no frontmatter block) → missing
|
||||
* 10. Nonexistent phase directory → missing
|
||||
* 11. Multiple *-VERIFICATION.md files → first by sort
|
||||
* 12. ship.md PHASE_VERIFICATION_INCOMPLETE sentinel (contract anchor for #651 consolidation)
|
||||
*
|
||||
* PORTABILITY: pure JS — no shell-outs, no bash fences.
|
||||
* Cross-platform (passes on Windows). Ref: DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE.
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const os = require('node:os');
|
||||
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
|
||||
const {
|
||||
VERIFIER_STATUSES,
|
||||
VERIFICATION_ROUTING_TABLE,
|
||||
readVerificationStatus,
|
||||
} = require('../gsd-core/bin/lib/verification.cjs');
|
||||
|
||||
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Create a temporary phase directory under os.tmpdir().
|
||||
* Returns the absolute path; caller must clean up.
|
||||
*/
|
||||
function mkPhaseDir(suffix) {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), `gsd-651-${suffix}-`));
|
||||
}
|
||||
|
||||
/**
|
||||
* Write a *-VERIFICATION.md file with the given frontmatter status and
|
||||
* optional body content.
|
||||
*
|
||||
* @param {string} dir - Phase directory path
|
||||
* @param {string} filename - e.g. '01-review-VERIFICATION.md'
|
||||
* @param {string} status - Frontmatter status value
|
||||
* @param {string} [body] - Content after the closing `---`
|
||||
*/
|
||||
function writeVerificationMd(dir, filename, status, body = '') {
|
||||
const frontmatter = `---\nstatus: ${status}\n---\n`;
|
||||
fs.writeFileSync(path.join(dir, filename), frontmatter + body);
|
||||
}
|
||||
|
||||
// ─── Tests ────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('verification-status', () => {
|
||||
|
||||
// ── Case 1: passed ────────────────────────────────────────────────────────
|
||||
test('status: passed → next_command is empty, status is passed', () => {
|
||||
const dir = mkPhaseDir('passed');
|
||||
try {
|
||||
writeVerificationMd(dir, '01-foo-VERIFICATION.md', 'passed');
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(result.status, 'passed', 'status must be passed');
|
||||
assert.equal(result.next_command, '', 'next_command must be empty for passed');
|
||||
assert.ok(result.next_action.length > 0, 'next_action must be non-empty');
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 2: gaps_found with phase token extraction ────────────────────────
|
||||
test('status: gaps_found in "03-foo" dir → next_command includes phase token 03', () => {
|
||||
// Phase dir basename starts with "03" — extractPhaseToken('03-foo') → '03'
|
||||
const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-651-parent-'));
|
||||
const phaseDir = path.join(baseDir, '03-foo');
|
||||
fs.mkdirSync(phaseDir);
|
||||
try {
|
||||
writeVerificationMd(phaseDir, '03-foo-VERIFICATION.md', 'gaps_found');
|
||||
const result = readVerificationStatus(phaseDir);
|
||||
assert.equal(result.status, 'gaps_found', 'status must be gaps_found');
|
||||
assert.ok(
|
||||
result.next_command.includes('03'),
|
||||
`next_command should include phase token '03'; got: ${result.next_command}`,
|
||||
);
|
||||
assert.ok(
|
||||
result.next_command.includes('--gaps'),
|
||||
`next_command should include --gaps; got: ${result.next_command}`,
|
||||
);
|
||||
assert.equal(result.next_command, '/gsd:plan-phase 03 --gaps');
|
||||
} finally {
|
||||
cleanup(baseDir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 3: human_needed ──────────────────────────────────────────────────
|
||||
test('status: human_needed → status human_needed, next_command is empty', () => {
|
||||
const dir = mkPhaseDir('human-needed');
|
||||
try {
|
||||
writeVerificationMd(dir, '01-hn-VERIFICATION.md', 'human_needed');
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(result.status, 'human_needed');
|
||||
assert.equal(result.next_command, '');
|
||||
assert.ok(result.next_action.length > 0);
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 4: no *-VERIFICATION.md → missing ────────────────────────────────
|
||||
test('no *-VERIFICATION.md file → status missing, next_command execute-phase', () => {
|
||||
const dir = mkPhaseDir('missing');
|
||||
try {
|
||||
// write a non-matching file to confirm it is ignored
|
||||
fs.writeFileSync(path.join(dir, 'README.md'), '# phase');
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(result.status, 'missing');
|
||||
assert.equal(result.next_command, '/gsd:execute-phase');
|
||||
assert.ok(result.next_action.includes('verify step never completed'));
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 5: unknown frontmatter status value ──────────────────────────────
|
||||
test("frontmatter status 'bogus' → status unknown, next_command execute-phase", () => {
|
||||
const dir = mkPhaseDir('unknown');
|
||||
try {
|
||||
writeVerificationMd(dir, '01-u-VERIFICATION.md', 'bogus');
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(result.status, 'unknown');
|
||||
assert.equal(result.next_command, '/gsd:execute-phase');
|
||||
assert.ok(
|
||||
result.next_action.includes('bogus'),
|
||||
`next_action should mention the raw value; got: ${result.next_action}`,
|
||||
);
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 6: BROAD-GREP REGRESSION (critical) ──────────────────────────────
|
||||
//
|
||||
// Frontmatter: `status: passed`
|
||||
// Body: a fenced code block containing `status: gaps_found` AND `status: human_needed`
|
||||
// Result MUST be 'passed' — proving body lines are NOT matched.
|
||||
// This is the exact failure mode that issue #586 / PR #650 hit.
|
||||
//
|
||||
test('BROAD-GREP REGRESSION: body status lines ignored, frontmatter status wins', () => {
|
||||
const dir = mkPhaseDir('broad-grep');
|
||||
try {
|
||||
const bodyWithEmbeddedStatuses = [
|
||||
'',
|
||||
'## Section',
|
||||
'',
|
||||
'Some prose about the results.',
|
||||
'',
|
||||
'```yaml',
|
||||
'status: gaps_found',
|
||||
'gaps:',
|
||||
' - fix the thing',
|
||||
'```',
|
||||
'',
|
||||
'Another block:',
|
||||
'',
|
||||
'```',
|
||||
'status: human_needed',
|
||||
'```',
|
||||
'',
|
||||
'End of document.',
|
||||
].join('\n');
|
||||
|
||||
writeVerificationMd(dir, '01-bg-VERIFICATION.md', 'passed', bodyWithEmbeddedStatuses);
|
||||
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(
|
||||
result.status,
|
||||
'passed',
|
||||
`Expected status 'passed' (frontmatter wins); got '${result.status}'. ` +
|
||||
'Body status: lines must NOT be matched.',
|
||||
);
|
||||
assert.equal(result.next_command, '', 'next_command must be empty for passed');
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Case 7: PARITY ASSERTION ──────────────────────────────────────────────
|
||||
//
|
||||
// (a) Every value in VERIFIER_STATUSES has a corresponding key in VERIFICATION_ROUTING_TABLE.
|
||||
// (b) Parse agents/gsd-verifier.md for emitted statuses via /→ \*\*status:\s*([a-z_]+)\*\*/g,
|
||||
// collect the set, and assert every emitted status is a routing key.
|
||||
//
|
||||
test('PARITY: VERIFIER_STATUSES covered by routing table', () => {
|
||||
for (const s of VERIFIER_STATUSES) {
|
||||
assert.ok(
|
||||
s in VERIFICATION_ROUTING_TABLE,
|
||||
`VERIFIER_STATUS '${s}' has no entry in VERIFICATION_ROUTING_TABLE`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('PARITY: gsd-verifier.md emitted statuses all have routing table entries', () => {
|
||||
const verifierPath = path.join(__dirname, '..', 'agents', 'gsd-verifier.md');
|
||||
const content = fs.readFileSync(verifierPath, 'utf-8');
|
||||
|
||||
const emittedStatuses = new Set();
|
||||
|
||||
// Source (a): decision-tree arrow lines — `→ **status: <value>**`
|
||||
// These are the per-branch emission points in Step 9 (the decision tree).
|
||||
const reArrow = /→ \*\*status:\s*([a-z_]+)\*\*/g;
|
||||
let m;
|
||||
while ((m = reArrow.exec(content)) !== null) {
|
||||
emittedStatuses.add(m[1]);
|
||||
}
|
||||
|
||||
// Source (b): output-template line — `status: A | B | C` (pipe-delimited list
|
||||
// of permitted values inside the frontmatter template block in the <output> section).
|
||||
// Anchored to lines that start with `status:` and contain `|` to avoid false
|
||||
// matches on prose sentences that happen to mention "status:".
|
||||
const reTemplate = /^status:\s+([a-z_]+(?:\s*\|\s*[a-z_]+)+)\s*$/gm;
|
||||
while ((m = reTemplate.exec(content)) !== null) {
|
||||
for (const token of m[1].split('|')) {
|
||||
const t = token.trim();
|
||||
if (t) emittedStatuses.add(t);
|
||||
}
|
||||
}
|
||||
|
||||
assert.ok(
|
||||
emittedStatuses.size > 0,
|
||||
'No emitted statuses found in gsd-verifier.md — regex or file path may be wrong. ' +
|
||||
'Checked: (a) → **status: X** arrow lines, (b) status: A | B | C template lines.',
|
||||
);
|
||||
|
||||
for (const s of emittedStatuses) {
|
||||
assert.ok(
|
||||
s in VERIFICATION_ROUTING_TABLE,
|
||||
`gsd-verifier.md emits status '${s}' but VERIFICATION_ROUTING_TABLE has no entry for it. ` +
|
||||
'Add a route or remove/rename the status in gsd-verifier.md.',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Edge cases ────────────────────────────────────────────────────────────
|
||||
|
||||
// CRLF line endings in frontmatter
|
||||
test('CRLF line endings in frontmatter → correct status parsed', () => {
|
||||
const dir = mkPhaseDir('crlf');
|
||||
try {
|
||||
// Construct a file with CRLF line endings throughout
|
||||
const content = '---\r\nstatus: passed\r\nphase: 01-demo\r\n---\r\n\r\n# Body\r\n';
|
||||
fs.writeFileSync(path.join(dir, '01-crlf-VERIFICATION.md'), content);
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(result.status, 'passed', 'CRLF frontmatter must parse to passed');
|
||||
assert.equal(result.next_command, '');
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// File with NO frontmatter block — body-only `status:` line must NOT be matched
|
||||
test('body-only file with no frontmatter block (status: in body) → missing', () => {
|
||||
const dir = mkPhaseDir('no-fm');
|
||||
try {
|
||||
// No opening `---` — this is a plain markdown file with a status: line in the body
|
||||
const content = '# Phase Verification\n\nstatus: passed\n\nSome notes.\n';
|
||||
fs.writeFileSync(path.join(dir, '01-nofm-VERIFICATION.md'), content);
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(
|
||||
result.status,
|
||||
'missing',
|
||||
"A body-only status: line must NOT be read — result should be 'missing'",
|
||||
);
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// Missing / nonexistent phase directory → missing
|
||||
test('nonexistent phase directory → missing', () => {
|
||||
const nonexistent = path.join(os.tmpdir(), 'gsd-651-nonexistent-' + Date.now());
|
||||
const result = readVerificationStatus(nonexistent);
|
||||
assert.equal(result.status, 'missing', 'unreadable/nonexistent dir must return missing');
|
||||
assert.equal(result.next_command, '/gsd:execute-phase');
|
||||
});
|
||||
|
||||
// Multiple *-VERIFICATION.md files → deterministic pick (first by sort)
|
||||
test('multiple *-VERIFICATION.md files in dir → first by sort order wins', () => {
|
||||
const dir = mkPhaseDir('multi');
|
||||
try {
|
||||
// Write two files: alphabetically "01-a" comes before "02-b"
|
||||
// "01-a" has passed; "02-b" has gaps_found — first by sort must win
|
||||
const fm = (status) => `---\nstatus: ${status}\n---\n`;
|
||||
fs.writeFileSync(path.join(dir, '01-a-VERIFICATION.md'), fm('passed'));
|
||||
fs.writeFileSync(path.join(dir, '02-b-VERIFICATION.md'), fm('gaps_found'));
|
||||
const result = readVerificationStatus(dir);
|
||||
assert.equal(
|
||||
result.status,
|
||||
'passed',
|
||||
'When multiple *-VERIFICATION.md files exist, the first by lexicographic sort must be used',
|
||||
);
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Task 2 (B1): ship.md gate sentinel contract anchor ────────────────────
|
||||
//
|
||||
// The deleted tests/ship-586-verification-routing.test.cjs was the only
|
||||
// thing asserting that ship.md emits the PHASE_VERIFICATION_INCOMPLETE block
|
||||
// sentinel (its user-visible gate error key). This test re-anchors that contract.
|
||||
//
|
||||
test('ship.md still emits the PHASE_VERIFICATION_INCOMPLETE gate sentinel (contract anchor for #651 consolidation)', () => {
|
||||
const shipMdPath = path.join(__dirname, '..', 'gsd-core', 'workflows', 'ship.md');
|
||||
const content = fs.readFileSync(shipMdPath, 'utf-8');
|
||||
assert.ok(
|
||||
content.includes('PHASE_VERIFICATION_INCOMPLETE'),
|
||||
'ship.md must contain the literal PHASE_VERIFICATION_INCOMPLETE gate sentinel. ' +
|
||||
'If you renamed or removed it, update the verification routing and this contract test.',
|
||||
);
|
||||
});
|
||||
|
||||
});
|
||||
Reference in New Issue
Block a user