Repository navigation
Conversation
|
Warning Review limit reachedNext included review available in 56 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (12)
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 |
…e not rejected as stale
vitali87
force-pushed
the
fix/prune-stale-remote-refs
branch
from
August 30, 2026 19:24
e2495a9 to
72fb0aa
Compare
A push/PR-creation failure leaves the plan with branches recorded and no
PRs. execute then refused to run ('use clean first') and clean deletes the
plan file entirely, so recovering from a transient network failure meant
regenerating the whole plan - for the LLM backend, paying for planning
again and possibly getting a different split. Branch creation is
idempotent (add_worktree recreates an existing branch), so execute now
retries when the saved state has branches but no PRs, and only refuses
when PRs already exist.
git diff and git show were run with text=True, whose universal-newline translation strips the CR from CRLF files, and the worktree write used the platform newline. A one-line change to a CRLF file therefore became a whole-file line-ending rewrite in every sub-PR. Capture git output as bytes and decode it, and write reconstructed files with newline="".
git quotes and octal-escapes non-ASCII paths by default
(core.quotePath). unidiff then fails on new/deleted files ('Target
without source') and, for modified files, records the literal quoted
string as the path so materialization asks git for a path that does
not exist. Run git diff with core.quotePath=false.
…of an SDK traceback
…ansient push errors A push that failed with a transient server error left some groups with PRs and the rest without, and execute then refused to run at all, so the only way on was clean (closing good PRs) and a full re-run. push_branch now retries known transient server errors, and execute keeps the groups that already have PRs (their branches untouched) and creates only the rest.
Conflicts resolved: - cli.py: the rollback on a failed branch keeps groups whose PRs already exist (this branch) under main's note that failed groups clean up their own branch; a PR-creation failure still prints a clean message and exits (this branch), and stacks are linked once, after the plan is saved (main). - git_ops/branches.py: push retry on transient errors kept; the unused checkout/commit helpers main removed stay removed. - logs.py: PUSH_RETRY kept alongside main's CP_SAT_NOT_OPTIMAL. - tests: both sides' imports and test classes kept. Tests that feed execute a mocked diff now patch validate_coverage, which main's execute runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
… diff main's execute validates the plan against the parsed diff; these tests mock parse_diff, so the check would reject the plan before the retry logic runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
…sh that landed - execute: when some layers already have PRs, a stacked resume now also keeps the recorded branches of those layers' ancestors. Rebuilding an ancestor gave it a new commit and force-pushed it, so the kept child's open PR (which targets it) showed the ancestor's changes again. The kept ancestors still get their PRs opened. - push_branch: after a transient error (hung-up remote, timeout) the push may have landed; the retry is then rejected as a stale lease. If the remote branch already holds the local head, the push counts as done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
…x/execute-resume-partial-prs The base branch's history was rewritten when #174 landed, so its commits reappear here. Every conflict is this branch's version of execute's resume logic and its tests, which already build on the base's change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
#146 (the base of this branch) is already in main, so the merge keeps main's code and adds only #168's change: an Anthropic token-count failure is an LLMError, and a rejected key names ANTHROPIC_API_KEY. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
Since execute lays out any plan with dependency edges along its DAG, stacked or not, an unstacked child also sits on its parent's branch; rebuilding the parent on resume would rewrite the base under the child's open PR. Keep recorded ancestors whenever the plan has dependencies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
fix: resume execute past groups whose PRs already exist, and retry transient push errors
fix: report an Anthropic token-count failure as an LLM error, naming a rejected key
Also stub prune_remote_tracking_refs in the CLI network guards so tests that push through _push_and_create_prs never run `git remote prune origin` against the real checkout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
pr-split analysis
This PR has 637 LOC and could be split into 6 smaller PRs:
Run |
Owner
Author
|
This change is already in Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split branch names are reused across runs (
pr-split/<ns>/<group>). Afterpr-split merge(gh deletes the head branch),clean, or GitHub auto-deleting head branches, the remote branch is gone butrefs/remotes/origin/pr-split/…still points at the old head — so everygit push --force-with-leaseof the next split was rejected with "stale info",PRCreationErrorfired, and the plan was saved with branches but no PRs. Re-splitting the same dev branch was impossible without a manualgit fetch --prune._push_and_create_prsnow runsgit remote prune originonce before the push pool (warning-only if origin is unreachable). It drops only refs whose remote branch is gone — deliberately notfetch --prune, which would refresh live refs and silently defeat the lease against a reviewer's fixup push to a reused branch (the review caught that in the first version).delete_branch(remote=True)dropsrefs/remotes/origin/<branch>after the remote delete, covering the "already gone on origin" case.Stacked on #156 (stack #152 = #55→#151→#156→this).
Test plan
TestStaleRemoteTrackingRefs: remote branch deleted behind our back → push rejected as stale → prune → push succeeds; a third-party fixup push is still rejected after prune (lease intact); remote delete drops the tracking ref; already-deleted remote still drops itTestPushPrunesFirst(prune before any push); mockeddelete_branchtests stub the helper