From 5541ea3fb404d8464829efb1894dda2db9345993 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 10 May 2026 17:37:55 -0400 Subject: [PATCH] fix(verifier): require direct probe execution --- .changeset/fix-3321-verifier-probes.md | 5 ++ agents/gsd-verifier.md | 42 ++++++++++++++ tests/bug-3321-verifier-runs-probes.test.cjs | 58 ++++++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 .changeset/fix-3321-verifier-probes.md create mode 100644 tests/bug-3321-verifier-runs-probes.test.cjs diff --git a/.changeset/fix-3321-verifier-probes.md b/.changeset/fix-3321-verifier-probes.md new file mode 100644 index 000000000..80c3a3c88 --- /dev/null +++ b/.changeset/fix-3321-verifier-probes.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3350 +--- +The verifier now runs declared probe scripts directly instead of accepting SUMMARY-reported probe PASS markers as evidence. diff --git a/agents/gsd-verifier.md b/agents/gsd-verifier.md index 2c85292b3..9f4acd93f 100644 --- a/agents/gsd-verifier.md +++ b/agents/gsd-verifier.md @@ -489,6 +489,43 @@ npm test -- --grep "$PHASE_TEST_PATTERN" 2>&1 | grep -q "passing" - Do not modify state (no writes, no mutations, no side effects) - If the project has no runnable entry points yet, skip with: "Step 7b: SKIPPED (no runnable entry points)" +## Step 7c: Probe Execution + +SUMMARY.md probe pass claims are not evidence. If a phase declares or implies probe-based verification, the verifier must run the probe in its own process and record the command result. + +**When to run:** For migration phases, CLI/tooling phases, or any phase whose PLAN/SUMMARY/verification criteria mention probes, PASS markers, stage markers, runnable checks, or `scripts/*/tests/probe-*.sh`. + +**Probe discovery:** + +```bash +# Conventional project probes +find scripts -path '*/tests/probe-*.sh' -type f 2>/dev/null | sort + +# Phase-declared probes +grep -R -n -E 'probe-[^[:space:]]+\.sh|scripts/.*/tests/probe-.*\.sh' "$PHASE_DIR"/*-PLAN.md "$PHASE_DIR"/*-SUMMARY.md 2>/dev/null +``` + +**Execution contract:** + +1. Build the `PROBES` list from explicit PLAN declarations first; include conventional `scripts/*/tests/probe-*.sh` when the phase is a migration/tooling phase or the success criteria mention probes. +2. For every documented probe path, if the file is missing or unreadable, mark `MISSING_PROBE` and set `status: gaps_found`. Do not require the executable bit because probes run through `bash "$probe"`. +3. Run each probe from the built `PROBES` list (declared + conventional) from the repository root: + +```bash +for probe in "${PROBES[@]}"; do + timeout 30s bash "$probe" +done +``` + +4. Exit code 0 is PASS. Any non-zero exit is FAILED and must include stdout/stderr evidence in VERIFICATION.md. +5. Do not substitute executor narration, SUMMARY.md PASS-marker counts, or a different dry-run driver command for the probe result. + +**Probe status:** + +| Probe | Command | Result | Status | +| ----- | ------- | ------ | ------ | +| `scripts/.../probe-name.sh` | `bash "$probe"` | exit code/output | PASS / FAILED / MISSING_PROBE | + ## Step 8: Identify Human Verification Needs **Always needs human:** Visual appearance, user flow completion, real-time behavior, external service integration, performance feel, error message clarity. @@ -724,6 +761,11 @@ Only include this section if deferred items exist (from Step 9b). | Behavior | Command | Result | Status | | -------- | ------- | ------ | ------ | +### Probe Execution + +| Probe | Command | Result | Status | +| ----- | ------- | ------ | ------ | + ### Requirements Coverage | Requirement | Source Plan | Description | Status | Evidence | diff --git a/tests/bug-3321-verifier-runs-probes.test.cjs b/tests/bug-3321-verifier-runs-probes.test.cjs new file mode 100644 index 000000000..8caf8be6b --- /dev/null +++ b/tests/bug-3321-verifier-runs-probes.test.cjs @@ -0,0 +1,58 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const REPO_ROOT = path.join(__dirname, '..'); +const VERIFIER_AGENT = path.join(REPO_ROOT, 'agents', 'gsd-verifier.md'); + +function verifierProbeContract(content) { + const sectionStart = content.indexOf('## Step 7c: Probe Execution'); + const sectionEnd = content.indexOf('## Step 8:', sectionStart); + assert.notEqual(sectionStart, -1, 'verifier must define Step 7c'); + assert.notEqual(sectionEnd, -1, 'verifier must close Step 7c before Step 8'); + + const section = content.slice(sectionStart, sectionEnd); + const codeBlocks = [...section.matchAll(/```bash\n([\s\S]*?)\n```/g)].map((match) => match[1]); + const executionSteps = [...section.matchAll(/^\d+\.\s+(.+)$/gm)].map((match) => match[1]); + return { + title: 'Step 7c: Probe Execution', + conventionalDiscoveryCommand: codeBlocks[0]?.split('\n').find((line) => line.startsWith('find scripts')) || null, + declaredDiscoveryCommand: codeBlocks[0]?.split('\n').find((line) => line.startsWith('grep -R')) || null, + executionCommand: codeBlocks[1] || '', + executionSteps, + statusRows: [...section.matchAll(/^\|\s*`([^`]+)`\s*\|\s*`([^`]+)`\s*\|[^|]+\|\s*([^|]+)\|$/gm)] + .map((match) => ({ probe: match[1], command: match[2], statuses: match[3].trim() })), + summaryClaimsRejected: section.includes('SUMMARY.md probe pass claims are not evidence'), + }; +} + +describe('bug #3321: gsd-verifier runs probes instead of trusting SUMMARY claims', () => { + test('verifier prompt requires direct probe discovery and execution', () => { + const content = fs.readFileSync(VERIFIER_AGENT, 'utf8'); + const contract = verifierProbeContract(content); + + assert.equal(contract.title, 'Step 7c: Probe Execution'); + assert.equal(contract.conventionalDiscoveryCommand, "find scripts -path '*/tests/probe-*.sh' -type f 2>/dev/null | sort"); + assert.equal( + contract.declaredDiscoveryCommand, + "grep -R -n -E 'probe-[^[:space:]]+\\.sh|scripts/.*/tests/probe-.*\\.sh' \"$PHASE_DIR\"/*-PLAN.md \"$PHASE_DIR\"/*-SUMMARY.md 2>/dev/null", + ); + assert.deepEqual(contract.executionSteps, [ + 'Build the `PROBES` list from explicit PLAN declarations first; include conventional `scripts/*/tests/probe-*.sh` when the phase is a migration/tooling phase or the success criteria mention probes.', + 'For every documented probe path, if the file is missing or unreadable, mark `MISSING_PROBE` and set `status: gaps_found`. Do not require the executable bit because probes run through `bash "$probe"`.', + 'Run each probe from the built `PROBES` list (declared + conventional) from the repository root:', + 'Exit code 0 is PASS. Any non-zero exit is FAILED and must include stdout/stderr evidence in VERIFICATION.md.', + 'Do not substitute executor narration, SUMMARY.md PASS-marker counts, or a different dry-run driver command for the probe result.', + ]); + assert.equal(contract.executionCommand, 'for probe in "${PROBES[@]}"; do\n timeout 30s bash "$probe"\ndone'); + assert.deepEqual(contract.statusRows, [{ + probe: 'scripts/.../probe-name.sh', + command: 'bash "$probe"', + statuses: 'PASS / FAILED / MISSING_PROBE', + }]); + assert.equal(contract.summaryClaimsRejected, true); + }); +});