wheels-bot embeds a stale SHA in review idempotency markers, causing review re-fires

[bpamiri]

Summary

Reviewer A/B comments carry an idempotency marker — <!-- wheels-bot:review-a:<pr>:<SHA> --> (and review-b). The embedded <SHA> can lag the commit the review actually ran against (commit_id). When that happens, the skip-check gate doesn't recognize the current head as already-reviewed, so Reviewer A re-fires on superseded commits and Reviewer B can emit contradictory verdicts.

Evidence (from #2847, merged 70291faff)

Across several rapid pushes, Reviewer B self-diagnosed this twice:

• "review 4's idempotency marker contained the wrong SHA (2474385d3 instead of 315b3c0a)"

• "the marker says 315b3c0a but the review was submitted against head e476c511f (confirmed via the GitHub commit_id field)."

The result was a 5-round review thrash, including one Reviewer B converged-approve and one Reviewer B "not aligned" pointing at different SHAs.

Impact

• Redundant Reviewer A/B runs on a PR that's already been reviewed → wasted Anthropic API spend.
• Confusing, self-contradictory review history; harder for a maintainer to know the real verdict.
• The skip-check idempotency can fail to suppress duplicate runs.

Where to look

• Marker emission: .claude/commands/review-pr.md and .claude/commands/review-the-review.md (where the <!-- wheels-bot:review-*:... --> marker string is written).
• SHA source: how .github/workflows/bot-review-a.yml / bot-review-b.yml resolve and pass the head SHA into the prompt.

Acceptance criteria

• The marker SHA always equals the commit the review ran against (checked-out head / review commit_id).
• Capture the head SHA once (at checkout) and thread it consistently into the marker rather than re-deriving it later.
• On a PR with several quick successive pushes, each review's marker SHA matches its commit_id, and the skip-check suppresses duplicate runs for an already-reviewed head.

Surfaced during the #2847 review cycle.

[wheels-bot]

Wheels Bot — Triage

Classified as bug.

Suspected layer

cli (bot infrastructure / CI) — none of the standard framework layers apply; this is a defect in the wheels-bot review pipeline itself. Canonical reference is .ai/wheels/wheels-bot.md, not a per-layer doc.

Fix sketch

The marker SHA is emitted by the skill prompts (.claude/commands/review-pr.md lines 23–24, 118–119; .claude/commands/review-the-review.md around the round-marker logic) but the head SHA is resolved by the workflows (.github/workflows/bot-review-a.yml steps.pr.outputs.sha; bot-review-b.yml line 50 github.event.pull_request.head.sha). Between checkout and review submission a new push can change pull_request.head.sha, leaving the prompt's marker pointing at the wrong commit while the submitted review's commit_id reflects the newer one. Fix shape: capture the head SHA exactly once at the workflow's checkout step, export it as a single env var (e.g. REVIEW_HEAD_SHA), thread that same value into the prompt's marker emission, and have a post-submit verification step compare it against the actual review commit_id and either correct the marker or fail loudly. Skip-check (marker-pattern in both workflows) then keys off the same captured value, so duplicate runs on the same head are suppressed correctly.

Confidence: medium

Report is clear and well-evidenced with concrete PR #2847 examples, and the target files are unambiguous; downgraded from high because the fix spans workflow YAML + two skill prompts with a real design choice (capture-once-and-thread vs. read-back-from-commit_id-and-reconcile) that propose-fix should resolve before patching.

<!-- wheels-bot:triage:2848 -->
<!-- wheels-bot:triage-class:bug -->
<!-- wheels-bot:triage-confidence:medium -->

[wheels-bot]

Wheels Bot — Fix on hold for human review

The proposed fix touches .github/workflows/bot-review-{a,b}.yml and .claude/commands/review-{pr,the-review}.md. Both surfaces are restricted from this bot run:

• .github/workflows/*.yml — the bot's GitHub App installation token does not carry the workflows permission, so git push is rejected with refusing to allow a GitHub App to create or update workflow .github/workflows/bot-review-a.yml without workflows permission.
• .claude/commands/ — writes to this directory are gated by a separate permission scope in the harness that auto-rejected the edit calls.

Since the entire bug surface lives behind those two gates, the bot can't author a draft PR for this. I've kept the design + locally-committed changes here so a human reviewer can land them quickly:

Design

Capture the head SHA exactly once at the workflow level and thread it as a single REVIEW_HEAD_SHA env var. The prompt then reads \$REVIEW_HEAD_SHA as the authoritative marker SHA instead of re-deriving it from gh pr view --json headRefOid (which races with new pushes).

Surface	Source of REVIEW_HEAD_SHA
bot-review-a.yml Run Reviewer A step	\${{ steps.pr.outputs.sha }}
bot-review-b.yml Run Reviewer B step	\${{ github.event.review.commit_id }} (the commit A attached its review to, not the PR's drifting head)

Reviewer B's checkout ref: and skip-check marker-pattern also switch to github.event.review.commit_id so the gate keys off the same captured SHA.

Staged changes (workflow + spec + changelog)

```diff
diff --git a/.github/workflows/bot-review-a.yml b/.github/workflows/bot-review-a.yml
@@ -111,6 +111,16 @@ jobs:
- name: Run Reviewer A
if: github.event_name == 'issue_comment' || steps.gate.outputs.skip == 'false'

•    env:
  

•      REVIEW_HEAD_SHA: \${{ steps.pr.outputs.sha }}
     uses: anthropics/claude-code-action@v1
  

diff --git a/.github/workflows/bot-review-b.yml b/.github/workflows/bot-review-b.yml
@@ steps:
- name: Checkout PR head
uses: actions/checkout@v6
with:

•      ref: \${{ github.event.pull_request.head.sha }}
  

•      ref: \${{ github.event.review.commit_id }}
  

@@ - - name: Skip check
with:
target-type: pr
target-number: ${{ github.event.pull_request.number }}

•      marker-pattern: 'wheels-bot:review-b:\${{ github.event.pull_request.number }}:\${{ github.event.pull_request.head.sha }}:'
  

•      marker-pattern: 'wheels-bot:review-b:\${{ github.event.pull_request.number }}:\${{ github.event.review.commit_id }}:'
  

@@ - - name: Run Reviewer B
if: steps.gate.outputs.skip == 'false'

•    env:
  

•      REVIEW_HEAD_SHA: \${{ github.event.review.commit_id }}
     uses: anthropics/claude-code-action@v1
  

```

Prompt-side updates still needed

Have the model session emit the marker SHA from `$REVIEW_HEAD_SHA` rather than from `gh pr view --json headRefOid`. Specifically in `.claude/commands/review-pr.md` step 1 (idempotency) and step 4 (marker emission line), and in `.claude/commands/review-the-review.md` step 1 (round counting) and step 6 (marker format). The corresponding paragraphs should explicitly forbid re-deriving the SHA from `gh pr view` because of the race documented in #2848.

Spec

A failing-then-passing structural spec is staged at `vendor/wheels/tests/specs/cli/BotReviewMarkerShaThreadingSpec.cfc` (157 lines). It reads each workflow YAML and asserts:

1. `bot-review-a.yml` "Run Reviewer A" step's env block contains `REVIEW_HEAD_SHA: ${{ steps.pr.outputs.sha }}`.
2. `bot-review-b.yml` "Run Reviewer B" step's env block contains `REVIEW_HEAD_SHA:` and the file references `github.event.review.commit_id`.
3. `bot-review-b.yml` Skip check `marker-pattern` does not reference `github.event.pull_request.head.sha`.

Asserted via `reFindNoCase` on the file contents, modeled on `OnErrorFallbackGuardSpec.cfc` and `ConfigRoutesStaleDocUrlSpec.cfc`. Runs via `bash tools/test-local.sh cli` once `wheels` CLI is on PATH.

A CHANGELOG `[Unreleased] → Fixed` entry is also staged.