fix(review): surface the head commit so a resumed session can't mistake a re-review for a repeat - #73
Conversation
…ke a re-review for a repeat A resumed session (resume_sessions: true, the default) carries every prior commit's conversation forward as one growing transcript, but nothing in the composed prompt states the current head commit explicitly. On a long-lived pull request the model has no mechanical way to check its own resumed memory against reality, and can conclude a new commit is the same one it already reviewed. Add the head commit SHA as its own fact line in the "This run" section for review mode, resolved from the same pull-request event context the gate step already uses, and always present regardless of resume_sessions. Also instruct the model, in the re-review section of the review prompt, to check that fact against what it actually remembers reviewing before treating a run as a repeat.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Small, well-scoped fix that does exactly what the description says: adds HEAD_SHA as a review-mode-only fact line, sourced from the same github.event.pull_request.head.sha context the Dependabot gate step already reads, and pairs it with an explicit instruction in prompts/review/base.md to check it before trusting a resumed session's own memory of "already reviewed this." Verified the mechanism actually works end-to-end: this very review's own "This run" facts include Head commit: 165711d75b1ffba39f85238b0015d1b935b5c95a, matching this PR's real head commit, and the new prompt paragraph is present in the instructions I'm following right now.
Checked for regressions: dependabot.yml's own direct-form review call is also pull_request-triggered, so HEAD_SHA resolves there too, not just through the reusable-workflow path. No prior reviews or inline comments exist on this PR, so there's nothing to reconcile from an earlier revision.
No Blockers or Should-fix findings. Two minor observations, neither worth blocking:
- [stack-default]
HEAD_SHAhas no override input analogous topr_number/inputs.pr_numberforENTITY_NUMBERinaction.yml, so a hypothetical futureworkflow_dispatch-driven review with an explicitpr_numberoverride would silently omit the "Head commit" line (theif [ -n "${HEAD_SHA:-}" ]guard degrades gracefully rather than erroring, so this isn't a bug today — review mode has no such trigger currently — just a gap worth remembering if one is ever added). - The PR description's own validation section is unusually thorough (typecheck/lint/format, commitlint, and a manual
actionlintdiff againstmainfor the composite-action steps) — genuinely useful for a reviewer, worth calling out as good practice rather than a finding.
Title and description are already complete and accurate; nothing to correct there. No unreadiness signal, so no draft conversion.
|
🎉 This PR is included in version 1.12.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
This run is triggered against the exact same head commit (165711d75b1ffba39f85238b0015d1b935b5c95a) already carrying an APPROVED review from this same identity, and the PR has since merged (8ddf0853427f461fff78b7dab9c293f8d0885b19). Per the "Head commit" fact this PR itself teaches review mode to check, and per the commit_id on the existing review, this is not a new revision to review — so no fresh review comments are being added, and nothing from the prior review needs dismissing or updating (resolving stale threads only applies to an older commit than the current head, which this isn't).
Independently re-verified the change against the diff and the PR description's own technical claims, and reached the same conclusion as the prior review:
HEAD_SHAinaction.yml's "Compose prompt" step is sourced fromgithub.event.pull_request.head.sha— the same contextexamples/claude-review.yml's own gate step already reads it from, as the description states. Confirmed viagrep -n "head.sha" action.ymlthat this is the only occurrence, so no duplicate or conflicting logic.- The new
- Head commit:fact line is correctly scoped to review mode only and unconditional onresume_sessions, matching the stated intent. - The new paragraph in
prompts/review/base.mdis correctly placed in the "Resolving stale threads and reviews on re-review" section, instructing exactly the check needed to fix the described bug. - No Blocker or Should-fix findings. Title and description are complete and accurate; no unreadiness signal, so no draft conversion.
Two minor observations, both already raised by the prior review and neither worth blocking:
- [stack-default] Nit:
HEAD_SHAhas no override analogous topr_number/ENTITY_NUMBER, so a hypothetical futureworkflow_dispatch-driven review with an explicitpr_numberoverride would silently omit the "Head commit" line (the[ -n "${HEAD_SHA:-}" ]guard degrades gracefully rather than erroring, and review mode has no such trigger today, so this is a gap worth remembering rather than a live bug). - The PR description's validation section (typecheck/lint/format, commitlint, a manual
actionlintdiff againstmain's synthetic wrapper) is unusually thorough for a 6-line change — worth calling out as good practice, not a finding.
Approving again, consistent with the prior review's own honest state (no Should-fix or Blocker findings) and the "always submit a decisive state" rule, rather than falling back to a non-committal comment merely because the verdict is unchanged.
|
🗜️ Headroom context compression
|
1 participant
What this fixes
On a long-lived pull request with many commits, review mode's session resumption
(
resume_sessions, on by default) carries every prior run's conversation forward as onegrowing transcript. Nothing in the composed prompt ever states the current head commit
explicitly, so a resumed session has no mechanical way to check its own memory against
reality. Confirmed twice independently on the same downstream repository, across two
different pull requests: a review got recorded against the PR's true current head commit
(via GitHub's own review metadata), but the review's own written reasoning claimed the
triggering commit was identical to the one it last reviewed, even re-flagging findings that
had already been fixed in the new commit.
Root cause
The "Compose prompt" step's "This run" facts block (
action.yml) never surfaces the currenthead commit SHA anywhere. Every other run-scoped fact the model needs (repository, mode,
entity number, base branch, severity mode, and so on) is stated explicitly so the model
doesn't have to infer it — the head commit was the one fact missing, and it's exactly the one
a resumed session most needs to check itself against.
The fix
HEAD_SHAto the "Compose prompt" step's env block, resolved fromgithub.event.pull_request.head.sha— the same context the "Decide whether this eventchanged anything worth reviewing" gate step already reads it from.
- Head commit: \`fact line in the "This run" section, scoped to review mode (this is PR/commit-scoped information, not relevant to triage or interactive modes), and unconditional onresume_sessions` — always present whether or notthe session was resumed.
section of the review prompt (
prompts/review/base.md): before treating a run as are-review of an already-seen commit, check the "Head commit" fact against what the model
actually reviewed last (e.g. the
commit_idon its most recent review), rather thanassuming a resumed session's transcript is still on the right commit.
Validation
npx turbo run typecheck lint format:check— all pass.npx commitlint --from HEAD~1 --to HEAD— the commit message passes.action.yml'sruns.stepsisn't directly checked byactionlint(it only understandsworkflow files); validated it anyway using the README's own documented technique — wrapping
the steps array in a synthetic
workflow_callreusable workflow and runningactionlintagainst that. Diffed the resulting warning set against the same synthetic wrapper built from
main: one additionalSC2016(style-level, "expressions don't expand in single quotes")note appears, from the new
printffact line following the exact sameprintf -- '- Label: `%s`\n' "$VAR"pattern every other fact line in this block alreadyuses and already triggers the identical note for — not a new class of issue.
Not done here
No CHANGELOG.md edit — it's generated by semantic-release from commit history, not hand-maintained.