diff --git a/.changeset/curious-tunas-fly.md b/.changeset/curious-tunas-fly.md
new file mode 100644
index 000000000..d7a3c01c1
--- /dev/null
+++ b/.changeset/curious-tunas-fly.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3437
+---
+code-review: every phase diff-base derivation now uses the same anchored, POSIX-portable phase-mention grep. Fixes wrong review scope from /gsd:code-review when a phase has no SUMMARY artifacts: the reviewer diff_base and the fallow --changed-since base no longer resolve to old unrelated commits whose messages merely contain the phase digits, and the anchored search now actually matches on macOS (the previous \b word boundary is not POSIX ERE and silently matched nothing there).
diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md
index 91a263eec..a93fd3d80 100644
--- a/gsd-core/workflows/code-review.md
+++ b/gsd-core/workflows/code-review.md
@@ -237,12 +237,14 @@ against the diff and warn about (then add) any changed files the SUMMARY extract
surface — so a partial SUMMARY result can no longer silently mask the rest of the phase.
```bash
# Compute diff base from phase commits — fail closed if no reliable base found.
-# #2989: anchor the grep to the phase-mention convention ("Phase N" / "phase N"
-# with a word boundary) so a bare digit substring doesn't match version strings,
-# dates, issue refs, or other phases' numbers. With --extended-regexp, \b is
-# a word boundary. When no commit genuinely references the phase, this yields
-# empty and the fail-closed warning below actually fires.
-PHASE_COMMITS=$(git log --oneline --all --grep="[Pp]hase ${PADDED_PHASE}\b" --extended-regexp --format="%H" 2>/dev/null)
+# #2989: anchor the grep to the phase-mention convention ("Phase N" / "phase N")
+# so a bare digit substring doesn't match version strings, dates, issue refs,
+# or other phases' numbers. When no commit genuinely references the phase, this
+# yields empty and the fail-closed warning below actually fires.
+# #3191: the trailing boundary is the POSIX class ([^[:alnum:]_]|$), NOT \b —
+# \b is not a POSIX ERE token, so under --extended-regexp it silently matches
+# nothing on macOS regex(3), making this fallback dead on Apple platforms.
+PHASE_COMMITS=$(git log --oneline --all --grep="[Pp]hase ${PADDED_PHASE}([^[:alnum:]_]|$)" --extended-regexp --format="%H" 2>/dev/null)
DIFF_BASE=""
if [ -n "$PHASE_COMMITS" ]; then
DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)^
@@ -423,11 +425,19 @@ Compute the review output path:
REVIEW_PATH="${PHASE_DIR}/${PADDED_PHASE}-REVIEW.md"
```
-Compute DIFF_BASE for agent context (in case agent needs it):
+Compute DIFF_BASE for agent context (in case agent needs it). #3191: this must be
+the SAME anchored, POSIX-portable derivation the Tier-3 scope step uses — the
+reviewer agent consumes `diff_base` exactly when `files:` is empty, i.e. the same
+fail-closed scenario Tier 3 protects, so a divergent unanchored recomputation here
+re-arms the mis-scoping one tier down:
```bash
-PHASE_COMMITS=$(git log --oneline --all --grep="${PADDED_PHASE}" --format="%H" 2>/dev/null)
+PHASE_COMMITS=$(git log --oneline --all --grep="[Pp]hase ${PADDED_PHASE}([^[:alnum:]_]|$)" --extended-regexp --format="%H" 2>/dev/null)
if [ -n "$PHASE_COMMITS" ]; then
DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)^
+ # Verify the parent commit exists (first commit in repo has no parent)
+ if ! git rev-parse "${DIFF_BASE}" >/dev/null 2>&1; then
+ DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)
+ fi
else
DIFF_BASE=""
fi
diff --git a/gsd-core/workflows/code-review/steps/structural-pre-pass.md b/gsd-core/workflows/code-review/steps/structural-pre-pass.md
index e4c9dbede..57a0286c4 100644
--- a/gsd-core/workflows/code-review/steps/structural-pre-pass.md
+++ b/gsd-core/workflows/code-review/steps/structural-pre-pass.md
@@ -26,10 +26,14 @@ FALLOW_STDERR_TMP=$(mktemp)
# Phase scope uses fallow's native changed-files scoping (--changed-since ).
# Derive the phase base commit; if none is found, fall back to repo scope (fallow
-# auto-detects the base branch).
+# auto-detects the base branch). #3191: the grep is the SAME anchored,
+# POSIX-portable phase-mention derivation the workflow's Tier-3 scope step uses
+# ("Phase N" followed by a non-alphanumeric or end-of-line) — a bare digit
+# substring matches version strings, dates, and other phases, and the oldest
+# such false match would silently widen --changed-since far past the phase.
FALLOW_SCOPE_ARGS=()
if [ \"$FALLOW_SCOPE\" = \"phase\" ]; then
- FALLOW_PHASE_COMMITS=$(git log --oneline --all --grep=\"${PADDED_PHASE}\" --format=\"%H\" 2>/dev/null)
+ FALLOW_PHASE_COMMITS=$(git log --oneline --all --grep=\"[Pp]hase ${PADDED_PHASE}([^[:alnum:]_]|$)\" --extended-regexp --format=\"%H\" 2>/dev/null)
if [ -n \"$FALLOW_PHASE_COMMITS\" ]; then
FALLOW_BASE=$(echo \"$FALLOW_PHASE_COMMITS\" | tail -1)^
FALLOW_SCOPE_ARGS=(--changed-since \"$FALLOW_BASE\")
diff --git a/tests/code-review-pipeline-regression.test.cjs b/tests/code-review-pipeline-regression.test.cjs
index 2c864c5f2..6146af7d9 100644
--- a/tests/code-review-pipeline-regression.test.cjs
+++ b/tests/code-review-pipeline-regression.test.cjs
@@ -26,12 +26,13 @@ const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { runHook } = require('./helpers/process-seam.cjs');
-const { toLegacyResult } = require('./helpers/git-fixture.cjs');
-const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
-const { createTempDir, cleanup, readFileNormalized } = require('./helpers.cjs');
+const { toLegacyResult, gitOrThrow } = require('./helpers/git-fixture.cjs');
+const { PROBE_TIMEOUT_MS, GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
+const { createTempDir, createTempGitProject, cleanup, readFileNormalized } = require('./helpers.cjs');
const ROOT = path.resolve(__dirname, '..');
const WORKFLOW_PATH = path.join(ROOT, 'gsd-core', 'workflows', 'code-review.md');
+const PRE_PASS_STEP_PATH = path.join(ROOT, 'gsd-core', 'workflows', 'code-review', 'steps', 'structural-pre-pass.md');
const FIXER_PATH = path.join(ROOT, 'agents', 'gsd-code-fixer.md');
const REVIEWER_PATH = path.join(ROOT, 'agents', 'gsd-code-reviewer.md');
@@ -727,3 +728,271 @@ describe('Bug 4 (#2352) — compute_file_scope tilde-path expansion', () => {
cleanup(tmpHome);
});
});
+
+// ---------------------------------------------------------------------------
+// Bug 5 (#3191) — EVERY diff-base derivation must use the same anchored,
+// portable phase-mention grep.
+//
+// The workflow computes "the phase's base commit" in three independent bash
+// invocations (each is its own shell): the Tier-3 file-scope fallback
+// (compute_file_scope), the agent-context DIFF_BASE (spawn_reviewer), and the
+// fallow pre-pass's --changed-since base (structural-pre-pass.md). #2989
+// anchored only the Tier-3 copy — and did so with `\b`, which is not a POSIX
+// ERE token, so on macOS (regex(3)) that grep matches NOTHING and Tier 3
+// always fails closed. The other two sites kept the original unanchored
+// `--grep="${PADDED_PHASE}"`, whose oldest substring match is routinely a
+// version-string/date commit from months before the phase existed.
+//
+// Behavioral style follows Bug 4: extract the SHIPPED bash from the workflow
+// .md files by content anchor and execute it via a real bash subprocess
+// against a git fixture — so the assertion binds the deployed text, not a
+// JS reimplementation. Running the real `git log` (not a regex shim) is what
+// makes the macOS `\b` hole visible: the fixture's real "Phase 06" commit
+// MUST be matched by the shipped pattern on every platform (#3191 AC2).
+// ---------------------------------------------------------------------------
+describe('Bug 5 (#3191) — anchored, portable phase-mention grep at all three diff-base sites', () => {
+ const SKIP_WIN32 = { skip: process.platform === 'win32' };
+
+ // The ```bash fence containing `marker`, located after `fromIdx`.
+ function fenceContaining(src, marker, fromIdx = 0) {
+ const markerIdx = src.indexOf(marker, fromIdx);
+ assert.ok(markerIdx !== -1, `expected to find "${marker}" in workflow source`);
+ const fenceStart = src.lastIndexOf('```bash', markerIdx);
+ assert.ok(fenceStart !== -1, `no \`\`\`bash fence before "${marker}"`);
+ const bodyStart = src.indexOf('\n', fenceStart) + 1;
+ const fenceEnd = src.indexOf('\n```', bodyStart);
+ assert.ok(fenceEnd !== -1, `unterminated \`\`\`bash fence containing "${marker}"`);
+ return src.slice(bodyStart, fenceEnd);
+ }
+
+ // The Tier-3 derivation prefix: fence start up to the REVIEW_FILES branch.
+ function extractTier3Derivation() {
+ const src = readFileNormalized(WORKFLOW_PATH);
+ const fence = fenceContaining(src, '# Compute diff base from phase commits');
+ const cut = fence.indexOf('if [ ${#REVIEW_FILES[@]} -eq 0 ]');
+ assert.ok(cut !== -1, 'Tier-3 fence must contain the REVIEW_FILES empty-scope branch');
+ return fence.slice(0, cut);
+ }
+
+ // spawn_reviewer's whole DIFF_BASE fence.
+ function extractSpawnReviewerDerivation() {
+ const src = readFileNormalized(WORKFLOW_PATH);
+ const spawnIdx = src.indexOf('');
+ assert.ok(spawnIdx !== -1, 'code-review.md must have a spawn_reviewer step');
+ return fenceContaining(src, 'PHASE_COMMITS=$(git log', spawnIdx);
+ }
+
+ // The fallow phase-scope derivation, from the step fragment. The fragment
+ // carries markdown-escaped quotes (\") in this fence — an authoring
+ // artifact that survived #2994 fragmentization verbatim; the runtime agent
+ // normalizes them when transcribing, so the test does the same before
+ // executing. Sliced from FALLOW_SCOPE_ARGS=() (skipping the gsd-tools
+ // runtime resolver line above it, which exits 1 on machines without an
+ // installed gsd-tools and is orthogonal to the base-derivation under test)
+ // to just before the gsd_run invocation (which needs the real binary).
+ function extractFallowDerivation() {
+ const src = readFileNormalized(PRE_PASS_STEP_PATH);
+ const fence = fenceContaining(src, 'FALLOW_PHASE_COMMITS=$(git log');
+ const scopeStart = fence.indexOf('FALLOW_SCOPE_ARGS=()');
+ assert.ok(scopeStart !== -1, 'fallow fence must define FALLOW_SCOPE_ARGS=()');
+ const cut = fence.indexOf('gsd_run run-with-timeout');
+ assert.ok(cut !== -1, 'fallow fence must contain the gsd_run run-with-timeout call');
+ assert.ok(scopeStart < cut, 'FALLOW_SCOPE_ARGS must precede the gsd_run invocation');
+ return fence.slice(scopeStart, cut).replace(/\\"/g, '"');
+ }
+
+ // Execute a derivation snippet with PADDED_PHASE (and the fallow scope gate)
+ // set, echoing the values it computes between sentinels so multi-line
+ // PHASE_COMMITS parse cleanly.
+ function runDerivation(repo, snippet, phase) {
+ const script = [
+ `PADDED_PHASE=${phase}`,
+ 'FALLOW_SCOPE=phase',
+ snippet,
+ 'echo "===PHASE_COMMITS==="',
+ 'printf \'%s\\n\' "$PHASE_COMMITS"',
+ 'echo "===DIFF_BASE==="',
+ 'printf \'%s\\n\' "$DIFF_BASE"',
+ 'echo "===FALLOW_BASE==="',
+ 'printf \'%s\\n\' "$FALLOW_BASE"',
+ 'echo "===END==="',
+ ].join('\n');
+ return toLegacyResult(
+ runHook('-c', [script, 'bash'], {
+ interpreter: 'bash',
+ cwd: repo,
+ timeoutMs: PROBE_TIMEOUT_MS,
+ })
+ );
+ }
+
+ function parseSentinel(stdout, name) {
+ const m = stdout.match(new RegExp(`===${name}===\\n([\\s\\S]*?)\\n===`));
+ if (!m) return null;
+ return m[1].split('\n').map((l) => l.trim()).filter((l) => l.length > 0);
+ }
+
+ // Fixture: five commits whose messages exercise every false-match class
+ // from the issue — version string + date, bare digits in a (NN) scope,
+ // another phase whose number is a digit-superset — plus the phase's real
+ // first commit and an unrelated HEAD.
+ function buildFixture(prefix, phaseCommitMessage) {
+ const repo = createTempGitProject(prefix);
+ const commits = [
+ ['c1.txt', 'chore: bump to v2.06.0 on 2026-01-05'],
+ ['c2.txt', 'docs(06-01): unrelated sub-phase work'],
+ ['c3.txt', phaseCommitMessage],
+ ['c4.txt', 'chore: Phase 60 cleanup'],
+ ['c5.txt', 'docs: touch README'],
+ ];
+ const hashes = {};
+ for (const [file, message] of commits) {
+ fs.writeFileSync(path.join(repo, file), `${message}\n`);
+ gitOrThrow(['add', file], { cwd: repo, timeoutMs: GIT_TIMEOUT_MS });
+ gitOrThrow(['commit', '-m', message], { cwd: repo, timeoutMs: GIT_TIMEOUT_MS });
+ hashes[file] = gitOrThrow(['rev-parse', 'HEAD'], { cwd: repo, timeoutMs: GIT_TIMEOUT_MS }).trim();
+ }
+ return { repo, hashes };
+ }
+
+ test(
+ 'T1 + T4: Tier-3 derivation matches ONLY the real phase-mention commit — including on macOS (#3191 \\b portability)',
+ SKIP_WIN32,
+ () => {
+ const { repo, hashes } = buildFixture('gsd-3191-tier3-', 'feat: Phase 06 kickoff — scanner core');
+ try {
+ const result = runDerivation(repo, extractTier3Derivation(), '06');
+ assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`);
+ const phaseCommits = parseSentinel(result.stdout, 'PHASE_COMMITS');
+ const diffBase = parseSentinel(result.stdout, 'DIFF_BASE');
+ // AC: the phase's real commits are a small minority of digit-containing
+ // commits; the derivation must resolve to an ancestor near the phase's
+ // actual first commit (c3^) — never the older v2.06.0/docs(06-01) hits.
+ assert.deepStrictEqual(
+ phaseCommits,
+ [hashes['c3.txt']],
+ `Tier-3 grep must match only the real "Phase 06" commit; got: ${JSON.stringify(phaseCommits)}`
+ );
+ assert.deepStrictEqual(
+ diffBase,
+ [`${hashes['c3.txt']}^`],
+ 'Tier-3 DIFF_BASE must be the phase first-commit parent'
+ );
+ } finally {
+ cleanup(repo);
+ }
+ }
+ );
+
+ test(
+ 'T2: spawn_reviewer DIFF_BASE derivation uses the same anchored grep (not the bare digit)',
+ SKIP_WIN32,
+ () => {
+ const { repo, hashes } = buildFixture('gsd-3191-spawn-', 'feat: Phase 06 kickoff — scanner core');
+ try {
+ const result = runDerivation(repo, extractSpawnReviewerDerivation(), '06');
+ assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`);
+ const phaseCommits = parseSentinel(result.stdout, 'PHASE_COMMITS');
+ const diffBase = parseSentinel(result.stdout, 'DIFF_BASE');
+ // Pre-fix this matches c1 and c2 as well and tail -1 picks c1 — the
+ // oldest unrelated match — feeding a bogus diff_base to the reviewer
+ // agent exactly when files: is empty (the fail-closed scenario).
+ assert.deepStrictEqual(
+ phaseCommits,
+ [hashes['c3.txt']],
+ `spawn_reviewer grep must match only the real "Phase 06" commit; got: ${JSON.stringify(phaseCommits)}`
+ );
+ assert.deepStrictEqual(
+ diffBase,
+ [`${hashes['c3.txt']}^`],
+ 'spawn_reviewer DIFF_BASE must be the phase first-commit parent'
+ );
+ } finally {
+ cleanup(repo);
+ }
+ }
+ );
+
+ test(
+ 'T3: fallow phase scope derives --changed-since from the anchored grep, never an old substring match',
+ SKIP_WIN32,
+ () => {
+ const { repo, hashes } = buildFixture('gsd-3191-fallow-', 'feat: Phase 06 kickoff — scanner core');
+ try {
+ const result = runDerivation(repo, extractFallowDerivation(), '06');
+ assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`);
+ const fallowBase = parseSentinel(result.stdout, 'FALLOW_BASE');
+ // Pre-fix the unanchored grep's oldest match is the v2.06.0 commit, so
+ // FALLOW_SCOPE_ARGS resolves to --changed-since
+ // and widens the structural pre-pass far beyond the phase.
+ assert.deepStrictEqual(
+ fallowBase,
+ [`${hashes['c3.txt']}^`],
+ `FALLOW_BASE must be the phase first-commit parent, got: ${JSON.stringify(fallowBase)}`
+ );
+ } finally {
+ cleanup(repo);
+ }
+ }
+ );
+
+ test(
+ 'T5: with no genuine phase-mention commit, every derivation yields NO base (fail-closed preserved)',
+ SKIP_WIN32,
+ () => {
+ const { repo } = buildFixture('gsd-3191-closed-', 'feat: scanner core'); // no "Phase 06" anywhere
+ try {
+ for (const [label, snippet] of [
+ ['tier3', extractTier3Derivation()],
+ ['spawn_reviewer', extractSpawnReviewerDerivation()],
+ ['fallow', extractFallowDerivation()],
+ ]) {
+ const result = runDerivation(repo, snippet, '06');
+ assert.equal(result.status, 0, `${label} exited ${result.status}; stderr=${result.stderr}`);
+ const phaseCommits = parseSentinel(result.stdout, 'PHASE_COMMITS');
+ const diffBase = parseSentinel(result.stdout, 'DIFF_BASE');
+ const fallowBase = parseSentinel(result.stdout, 'FALLOW_BASE');
+ assert.deepStrictEqual(phaseCommits, [], `${label}: no substring-only matches may survive`);
+ assert.deepStrictEqual(diffBase, [], `${label}: DIFF_BASE must stay empty (no bogus base)`);
+ assert.deepStrictEqual(fallowBase, [], `${label}: FALLOW_BASE must stay unset`);
+ }
+ } finally {
+ cleanup(repo);
+ }
+ }
+ );
+
+ // T6 docs-parity anti-revert: every `git log --grep` derivation in both
+ // files must be anchored with the POSIX-portable boundary and must not use
+ // `\b` under --extended-regexp (which silently no-ops on macOS regex(3)).
+ test('T6 docs-parity: all git-log grep derivations are anchored and free of the non-POSIX \\b', () => {
+ const sources = [
+ readFileNormalized(WORKFLOW_PATH),
+ readFileNormalized(PRE_PASS_STEP_PATH).replace(/\\"/g, '"'),
+ ];
+ const grepLines = [];
+ for (const src of sources) {
+ for (const m of src.matchAll(/^\s*[A-Z_]+=\$\(git log[^\n]*--grep=[^\n]*$/gm)) {
+ grepLines.push(m[0]);
+ }
+ }
+ assert.ok(
+ grepLines.length >= 3,
+ `expected at least 3 git-log grep derivation sites (Tier 3, spawn_reviewer, fallow); found ${grepLines.length}`
+ );
+ for (const line of grepLines) {
+ assert.ok(
+ line.includes('--grep="[Pp]hase ${PADDED_PHASE}([^[:alnum:]_]|$)"'),
+ `grep derivation must be anchored to the phase-mention convention with a POSIX boundary:\n${line}`
+ );
+ assert.ok(
+ line.includes('--extended-regexp'),
+ `grep derivation must pass --extended-regexp:\n${line}`
+ );
+ assert.ok(
+ !line.includes('\\b'),
+ `grep derivation must not use \\b under --extended-regexp — it is not POSIX ERE and silently matches nothing on macOS (#3191):\n${line}`
+ );
+ }
+ });
+});
diff --git a/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json b/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json
deleted file mode 100644
index 3452f97c2..000000000
--- a/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json
+++ /dev/null
@@ -1,6 +0,0 @@
-{
- "version": 1,
- "paths": {
- "code-review.md": "#2989: the diff-base fallback's git log --grep was changed from an unanchored bare phase number (matching version strings, dates, issue refs) to an anchored '[Pp]hase N\\b' with --extended-regexp, plus a 5-line comment explaining the anchor. Makes the fail-closed branch reachable when no commit genuinely references the phase."
- }
-}
diff --git a/tests/emitted-drift-acks/3191-unanchored-grep-sites.json b/tests/emitted-drift-acks/3191-unanchored-grep-sites.json
new file mode 100644
index 000000000..c6b6ffb0f
--- /dev/null
+++ b/tests/emitted-drift-acks/3191-unanchored-grep-sites.json
@@ -0,0 +1,6 @@
+{
+ "version": 1,
+ "paths": {
+ "code-review.md": "#3191: +711 bytes. The two remaining unanchored `git log --grep=\"${PADDED_PHASE}\"` diff-base derivations (spawn_reviewer; Tier-3 was anchored in #2989) now use the anchored '[Pp]hase N([^[:alnum:]_]|$)' with --extended-regexp, and the #2989 site's trailing \\b — not a POSIX ERE token, silently matching nothing on macOS regex(3) — is replaced by the POSIX class; spawn_reviewer also gains Tier-3's parent-exists guard. The fallow structural-pre-pass.md fragment (not size-measured; hash ripple explained by its own committed change) carries the same anchoring. Supersedes the spent 2989-code-review-anchored-diff-base.json fragment."
+ }
+}