From 31229ca0dfcc501f08b649bc9bac4eac88b92113 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Fri, 7 Aug 2026 17:58:09 -0700 Subject: [PATCH] fix: warn on too-short sha= marker in Minimize PASS review comment step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Check review verdict" step already warns and logs when a skip-claude-review sha= marker is too short to trust (<7 chars) and proceeds with review. The "Minimize PASS review comment" step had no equivalent branch for the same too-short case — it fell through silently to the minimize logic with no log message at all, creating a behavioral asymmetry between the two steps that made them harder to reason about together when debugging. Adds the missing elif branch: a too-short MARKER_SHA now logs the same ::warning:: as the verdict step, then falls through to the minimize logic (matching the verdict step's "reject and proceed" behavior) rather than silently doing nothing. Verified with a standalone harness exercising all five marker states: no marker, unscoped marker, too-short scoped marker (now warns and falls through), scoped marker matching head (skips, unchanged), and scoped marker not matching head (falls through silently, unchanged — this is a distinct case from too-short and correctly has no warning, matching the verdict step's precedent of only warning on the too-short/invalid case). Committed with SKIP=zizmor per the documented escape hatch (dotfiles pre-commit/config.yaml, PR #155): zizmor lints the whole file and surfaces pre-existing findings (template-injection, known-vulnerable- actions, ref-version-mismatch, artipacked) unrelated to this diff. Confirmed identical finding count/type on main before this change via `zizmor .github/workflows/claude-blocking-review.yml` — this diff introduces zero new findings. Given this repo's wide fleet-wide blast radius and the explicit ask to keep fixes narrowly scoped/separable, those pre-existing findings are out of scope here and warrant their own dedicated triage rather than a drive-by fix or suppression bundled into this PR. Closes #92. --- .github/workflows/claude-blocking-review.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/claude-blocking-review.yml b/.github/workflows/claude-blocking-review.yml index 1e3a83c..a3f2014 100644 --- a/.github/workflows/claude-blocking-review.yml +++ b/.github/workflows/claude-blocking-review.yml @@ -675,7 +675,14 @@ jobs: if [ -z "$MARKER_SHA" ]; then echo "[skip-claude-review] unscoped override active — nothing to minimize." exit 0 - elif [ "${#MARKER_SHA}" -ge 7 ] && [ "${SHA#"$MARKER_SHA"}" != "$SHA" ]; then + elif [ "${#MARKER_SHA}" -lt 7 ]; then + # Mirrors the "Check review verdict" step's rejection of + # too-short sha= values (#92) — log the same warning here so + # an operator debugging why a PASS comment wasn't minimized + # sees the same too-short-marker explanation in both steps' + # logs, instead of this step silently falling through. + echo "::warning::[skip-claude-review sha=${MARKER_SHA}] marker rejected — sha= value must be at least 7 characters. Proceeding with minimize check." + elif [ "${SHA#"$MARKER_SHA"}" != "$SHA" ]; then echo "[skip-claude-review sha=${MARKER_SHA}] override active for current head — nothing to minimize." exit 0 fi