Repository navigation
PR agent: work on and push against the expected head only - #2020
Conversation
pr-agent.yml already binds fix-ci to the failing run's head and skips a stale run at dispatch, but the routine runs later. On #2011 it was fired for 125f0f3's Windows failure; by the time it checked the branch out by name, a newer push (ac7c34f) had fixed that failure in the test. The routine stacked its own fix on top anyway with a plain push, and that fix changed the moved query's semantics (case-insensitive LIKE to a case-sensitive substr), which Codex then flagged as a regression. Make the routine confirm `git rev-parse HEAD` equals EXPECTED_HEAD after checkout (Common Setup, reconciliation step 2, fix-ci step 2), and push only with `--force-with-lease=refs/heads/$HEAD:$EXPECTED_HEAD`, so git rejects a push onto a head that moved even if the model skips or races the re-check. A rejected push is a silent no-op, never a rebase onto the new head. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 24 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
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 |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aa1fa18d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex flagged that the Absolute Rules at the bottom of the routine prompt told the routine to pull with rebase and push whenever the branch had "diverged unexpectedly". The new EXPECTED_HEAD lease push rejects exactly when the head moved while the routine was working — i.e. "the branch diverged" — so the labeled-absolute rebase instruction gave the routine a licensed path to rebase a stale fix onto the newer head and push it anyway. That recreates the #2011 regression this PR exists to prevent. Rewrite that rule to: - prohibit only unconditional force-push (`git push --force` / `-f`), - name the Common Setup `--force-with-lease` push as the only push used, - spell out that a rejected lease push is a silent stop — no rebase onto the new head, no retry, no comment, - carve out the base-divergence (merge-conflict) path as a merge of `origin/$BASE` into the PR head inside the reconciliation flow, not a rebase to recover from the lease. Extend `test_routine_works_on_and_pushes_against_the_expected_head_only` to pin these guards inside the Absolute Rules section itself, so a future edit to that section cannot silently reintroduce the contradiction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KUTQVZPcuFFxjT3ebkTQaQ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ebfe47996
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex flagged the Absolute Rule as contradicting the fix-main flow:
`fix-main` has no `EXPECTED_HEAD` because it opens a new
`claude/fix-main-*` branch from current `main`, so the
`--force-with-lease="refs/heads/$HEAD:$EXPECTED_HEAD"` push the rule
labels as the "only push used" has nothing to lease against. The rule
as written would either try to lease against an empty ref or refuse
the ordinary first push needed before `gh pr create`, breaking
automated recovery whenever `main` goes red.
Changes:
- Absolute Rules: scope the lease-is-only-push rule to every single-PR
task — the task kinds that carry `EXPECTED_HEAD` (`reconcile-pr`,
`reconcile-pr-auto`, `address-review`, `address-comment`,
`address-codex-review`, `fix-ci`) — and spell out that `fix-main` is
the one exempt task, with the reason (no `EXPECTED_HEAD`, brand-new
remote branch) and the pointer to its own live-state revalidation
(steps 1 and 7) as the real guard against publishing on a stale
incident.
- `fix-main` step 8: make the first push explicit
(`git push -u origin "claude/fix-main-$WORKFLOW_RUN"`) and name it as
the one place a plain `git push` is used, so the exemption is
visible from inside the task flow itself.
- `test_routine_works_on_and_pushes_against_the_expected_head_only`:
pin the scoping ("every task that carries `EXPECTED_HEAD`") and the
exemption ("`fix-main` is the one task this scoping exempts",
`git push -u origin "claude/fix-main-\$WORKFLOW_RUN"`, "explicitly
exempt") so a future edit cannot silently reintroduce the
contradiction.
Action needed after merge: the stored routine prompt isn't synced from
the repo. Re-paste `docs/pr-agent-routine-prompt.md` into the routine
at claude.ai/code/routines for the exemption to take effect.
Validation: `python -m pytest tests/test_pr_agent_workflow.py -v` — 30
passed. `ruff check` — clean.
[pr-agent-review-fix:2020]
<!-- pr-agent-generated -->
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015tgq6WBZh3tf2aqQjVw4bG
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
What changed
pr-agent.ymlalready bindsfix-cito the failing run's head and skips a stale run when it fires the routine (test_ci_fix_binds_to_failing_run_head_not_live_pr_head). But the routine runs afterwards, in the cloud, and checks the branch out by name, then pushes with a plaingit push. If someone pushes to the PR in between, the routine works on their newer commit and stacks a fix for a stale failure on top of it.That happened on #2011. The routine was fired for
125f0f389's Windows failure. By thenac7c34fcfhad already fixed that failure in the test. The routine still pushedebaf66065on top, and its "fix" changed the moved query's semantics (case-insensitiveLIKEto a case-sensitivesubstr). Codex flagged it as a regression for in-place imports on case-insensitive drives, and it was reverted.docs/pr-agent-routine-prompt.md:git rev-parse HEADequalsEXPECTED_HEAD, and push only with--force-with-lease="refs/heads/$HEAD:$EXPECTED_HEAD". The routine's commit hasEXPECTED_HEADas its parent, so that push is a plain fast-forward when nothing moved, and git rejects it when the head did move, even if the model skipped or raced the re-check. A rejected push is a silent no-op, never a rebase onto the new head.fix-ci: the same check after the checkout (step 2), with the reason (a newer push may already fix the failure another way), and the lease on the push (step 6).New
test_routine_works_on_and_pushes_against_the_expected_head_onlypins both guards in the shared rules, the reconciliation flow andfix-ci.Action needed after merge: the stored routine prompt isn't synced from the repo (see CLAUDE.md, PR Agent System). Paste the updated
docs/pr-agent-routine-prompt.mdinto the routine at claude.ai/code/routines for this to take effect.Test results
tests/test_pr_agent_workflow.py: 30 passed.🤖 Generated with Claude Code