fix(#586): make ship PHASE_VERIFICATION_INCOMPLETE actionable, drop dead pass status branch (#650)

* fix(#586): make ship PHASE_VERIFICATION_INCOMPLETE actionable, drop dead `pass` arm

The ship preflight gate blocked with PHASE_VERIFICATION_INCOMPLETE but named no
next step, and accepted a `pass` status the verifier never emits. Capture the
verification status and route per value (gaps_found / human_needed / missing),
mirroring execute-phase's status table; accept only `passed`.

Closes #586

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(#586): backfill changeset PR number 650

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#586): scope ship verification status to frontmatter only

Codex adversarial review of PR #650 flagged that the status gate grepped
`^status:` over the entire VERIFICATION.md, so a `status:` line in the report
body (a code block / copied artifact) concatenates into a non-matching value and
blocks a genuinely-passed phase with the wrong next action. Restrict extraction
to the leading YAML frontmatter block, first match only. Adds a behavioral
regression test that runs the gate's own bash pipeline against a passing report
whose body contains decoy `status:` lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(#586): drop manual PR ref from changeset body

The changelog renderer auto-appends `(#<pr>)` from the fragment's pr: field
(scripts/changeset/serialize.cjs, github-release-notes.cjs). The manual trailing
`(#586)` produced a double, mismatched ref (issue #586 + auto PR #650); remove it
to match the sibling-fragment convention.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(#586): make ship-586 bash-fence regex Windows-safe (CRLF)

The behavioral test extracted the gate's bash block with /```bash\n.../ — a
literal \n that fails to match Windows CRLF checkouts and trips the
windows-test-parity-guard (fenceRegexLiteralNewline). Use ```bash\r?\n and
normalize the captured block to LF before running it. Full unit suite: 0 fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(#586): run ship-586 bash-pipeline tests on POSIX only

On Windows CI the behavioral tests failed: git-bash is present (so the old
hasBash guard ran them) but receives a Windows-style tmpdir path it cannot glob,
so extraction returned empty. The extraction logic is platform-independent and
the gate's bash only runs in a POSIX workflow context, so skip the pipeline
execution on win32. POSIX (macOS/Linux) still runs and asserts it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-03 20:51:53 -04:00
committed by GitHub
parent d585871fe2
commit 0d97532a57
3 changed files with 125 additions and 3 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 650
---
**ship now tells you how to clear a blocked verification** — the `PHASE_VERIFICATION_INCOMPLETE` block names the exact next action per status (`gaps_found`, `human_needed`, or missing), and the dead `pass` status arm is gone.

View File

@@ -41,10 +41,15 @@ Verify the work is ready to ship:
1. **Verification passed?**
```bash
VERIFICATION=$(cat ${PHASE_DIR}/*-VERIFICATION.md 2>/dev/null)
VERIFICATION_FILE=$(ls ${PHASE_DIR}/*-VERIFICATION.md 2>/dev/null | head -1)
STATUS=$(sed -n '/^---$/,/^---$/p' "${VERIFICATION_FILE}" 2>/dev/null | grep -m1 "^status:" | cut -d: -f2 | tr -d ' ')
```
Check for `status: pass` or `status: passed`.
If no VERIFICATION.md or status is anything other than `pass` / `passed` (including `human_needed` / `gaps_found`): block with `PHASE_VERIFICATION_INCOMPLETE`; complete or formally re-run verification before shipping.
The verifier emits exactly `passed`, `gaps_found`, or `human_needed` (see the status table in `execute-phase.md`); only `passed` may ship. Route on `${STATUS}` — on any non-`passed` value, block with `PHASE_VERIFICATION_INCOMPLETE` and state the matching next action:
- `passed` → verification complete; continue to the next preflight check.
- `gaps_found` → run `/gsd:plan-phase ${PHASE_NUMBER} --gaps` to plan the fixes, then re-run `/gsd:execute-phase` before shipping.
- `human_needed` → complete the manual tests in `${PHASE_DIR}/*-UAT.md`, then re-run the verify step until status is `passed`.
- empty (no `*-VERIFICATION.md`) → the verify step never completed; re-run `/gsd:execute-phase`.
- any other value → unexpected status `${STATUS}`; re-run `/gsd:execute-phase` verification.
2. **Clean working tree?**
```bash

View File

@@ -0,0 +1,112 @@
'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), '');
});