Repository navigation
feat: implement issue #1106 — [Phase 3] HEAD-SHA freshness safeguard so no tier reviews stale pre-fed context - #1164
Conversation
…so no tier reviews stale pre-fed context
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Code Review
This pull request introduces a HEAD-SHA freshness safeguard to prevent reviewing stale code when a force-push or new push occurs mid-run. It adds the assert_prefetch_context_fresh function in scripts/lib/pr-context-prefetch.sh, integrates it into scripts/review-one-pr.sh, and provides comprehensive unit tests in tests/test_prefetch_freshness.bats. Feedback was provided to replace a piped sed | head command with a single sed expression to avoid potential script crashes from SIGPIPE when set -o pipefail is active.
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-07-10T16:23:19Z. |
There was a problem hiding this comment.
Pull request overview
Implements the Phase 3 “HEAD-SHA freshness” safeguard for the PR review cascade so agentic tiers never consume stale pre-fetched context when a PR’s HEAD moves mid-run (e.g., force-push). This fits into the repo’s agentic review infrastructure by hardening the Story 2 prefetch optimization against staleness.
Changes:
- Adds
assert_prefetch_context_freshto validate the stamped prefetch SHA against a lightweightgh pr view --json headRefOidcheck and invalidate/skip on mismatch. - Wires the freshness check into
review-one-pr.shimmediately before tier consumption, exiting with sentinel100when HEAD moved. - Adds Bats coverage for flag-off, matched-SHA, moved-SHA invalidation, and
ghfailure degradation; registers the new test in the lint workflow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
scripts/review-one-pr.sh |
Calls the freshness guard before running agentic tiers; skips with exit-100 if HEAD moved. |
scripts/lib/pr-context-prefetch.sh |
Introduces the SHA freshness validation helper and stale-context invalidation behavior. |
tests/test_prefetch_freshness.bats |
Adds unit tests covering off/match/move/gh-failure behaviors for the safeguard. |
.github/workflows/lint.yml |
Includes the new Bats test in the CI lint test list. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1164 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: f63ad1894840996a3d8b335b3d0a14269cc696e8
Review mode: triage-approved (single reviewer)
Summary
Adds a HEAD-SHA freshness safeguard (assert_prefetch_context_fresh) that runs once before the agentic review tiers consume pre-fed context. It re-checks the PR's current head SHA via a single bounded 'gh pr view --json headRefOid' call against the SHA stamped on the pre-fed files; on mismatch it discards the stale context and exits with the established skip sentinel (100). No-op when PREFETCH_CONTEXT_ENABLED is off. Includes 6 bats tests wired into lint.yml.
Linked issue analysis
Closes #1106 ([Phase 3] HEAD-SHA freshness safeguard). All five acceptance criteria are substantively addressed: (1) cheap re-check before deep/audit/single tiers consume pre-fed context — placed after triage and before tier selection in review-one-pr.sh; (2) matched SHA uses cached context as-is with no extra fetch; (3) moved SHA discards stale files, unsets PR_CONTEXT_* exports, and takes the exit-100 skip sentinel for retry (idempotency marker prevents duplicate reviews); (4) bounded to a single lightweight headRefOid call and a byte-identical no-op when the flag is off (verified by tests asserting zero gh calls); (5) bats coverage for matched / moved / flag-off, plus nothing-prefed and gh-failure degradation cases beyond the AC minimum.
Findings
No blocking findings.
- Stamp-format compatibility verified: the reader matches exactly what prefetch_pr_context (main) writes — top-level .pr_head_sha in metadata JSON and the '# PR_HEAD_SHA: ' diff header fallback.
- Both prior bot review threads (gemini, Copilot — SIGPIPE risk from a 'sed | head' pipeline under pipefail) are resolved; the current code uses 'sed -n "/.../{s///p;q;}"' with no pipeline, eliminating the concern.
- Graceful degradation on gh failure (proceed on the SHA-bound stamped context rather than crash) is a reasonable trade-off, explicitly documented and test-covered.
- The lint.yml change is a one-line test registration in the existing bats step — no permission or action changes.
- Minor residual race (HEAD could move after the check but before tier consumption) is acknowledged in-code as belt-and-suspenders; acceptable given tiers run seconds apart in one process.
- Secret scan: run_secret_scanning MCP tool unavailable in this environment; gitleaks CI check passed. No credentials or sensitive patterns in the diff.
CI status
All required checks green at f63ad18: shellcheck, ShellCheck, bats, unit-tests, Lint, CodeQL (actions + python), gitleaks secret scan, SonarCloud quality gate, agent-shield, gh-aw-compile, validate-agent-profiles, holdout-guard, and all stub/permission guards SUCCESS. A few earlier dev-lead dispatch / review runs show CANCELLED (superseded by later successful runs); dependency-audit ecosystem jobs SKIPPED (not applicable).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.



Closes #1106
Implemented by dev-lead agent. Please review.