From 2a77e50dafd5439d590ddc5ea44a9f47235c97a2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 5 Aug 2026 20:03:03 -0400 Subject: [PATCH] fix(#2989): anchor code-review diff-base grep to phase-mention convention (#3096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2989): anchor code-review diff-base grep to phase-mention convention The diff-base fallback in code-review.md used git log --grep with a bare phase number (unanchored substring), matching version strings, dates, issue refs, and other phases' numbers. tail -1 took the oldest match — routinely a commit from months or years before the phase existed. The fail-closed branch was dead code because a bare digit almost always matches something. Changed --grep to '[Pp]hase N\b' with --extended-regexp, anchoring to the phase-mention convention. When no commit genuinely references the phase, the derivation yields empty and the fail-closed warning fires (now reachable). All three consumers (Tier 3 file scope, fallow pre-pass, agent context) use the same corrected value. * chore(#2989): backfill changeset PR number 3096 --------- Co-authored-by: sim --- .changeset/proud-pumas-bark.md | 5 +++++ gsd-core/workflows/code-review.md | 9 +++++++-- .../2989-code-review-anchored-diff-base.json | 6 ++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 .changeset/proud-pumas-bark.md create mode 100644 tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json diff --git a/.changeset/proud-pumas-bark.md b/.changeset/proud-pumas-bark.md new file mode 100644 index 000000000..1cbbe9665 --- /dev/null +++ b/.changeset/proud-pumas-bark.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3096 +--- +**`/gsd-code-review` no longer picks a wrong diff base from unanchored commit-message grep** — the diff-base fallback searched all commit messages for the bare phase number as a substring, matching version strings, dates, and issue refs, then took the oldest match. The grep is now anchored to the phase-mention convention (`Phase N` with a word boundary), so the fail-closed branch is reachable when no commit genuinely references the phase. (#2989) diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md index b64264738..9adc0f639 100644 --- a/gsd-core/workflows/code-review.md +++ b/gsd-core/workflows/code-review.md @@ -236,8 +236,13 @@ Additionally, whenever a reliable diff base is available, cross-check the SUMMAR against the diff and warn about (then add) any changed files the SUMMARY extractor did not 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 -PHASE_COMMITS=$(git log --oneline --all --grep="${PADDED_PHASE}" --format="%H" 2>/dev/null) +# 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) DIFF_BASE="" if [ -n "$PHASE_COMMITS" ]; then DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)^ 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 new file mode 100644 index 000000000..3452f97c2 --- /dev/null +++ b/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json @@ -0,0 +1,6 @@ +{ + "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." + } +}