Fix commitAndPush hanging on non-fast-forward push against large/rewritten histories - #26
Open
timrogers wants to merge 3 commits into
Open
Fix commitAndPush hanging on non-fast-forward push against large/rewritten histories#26timrogers wants to merge 3 commits into
timrogers wants to merge 3 commits into
Conversation
Replaces the fetch+rebase retry in commitAndPush's push fallback with a fetch + squash-diff + reapply strategy. Problem: this SDK clones repositories shallowly (--depth 2). When a push is rejected because the remote branch is ahead, the previous code ran `git fetch` + `git rebase origin/<branch>`. On a large monorepo, once the remote has advanced (or been rewritten via squash-merge/force-push/branch protection workflows) enough that the local shallow history and the fetched remote tip share no common ancestor, git rebase falls back to replaying every local commit individually via patch-id search and a full three-way merge per commit, invoking hooks each time. This is slow and scales with the number of local commits and the size of the diffs. In production this showed up as multi-minute-to-tens-of-minutes silent hangs during agent session finalization on github/github (a very large monorepo), which then got killed by an unrelated outer job timeout with a confusing 'connection refused' error instead of a clear failure. Fix: instead of rebasing, compute the diff between our local tip and the remote tip we last knew about (tracked via the branch's upstream ref), fetch just the new remote tip (still shallow, matching clone depth), reset onto it, and reapply that diff as a single new commit (using `git apply -3` so content-level conflicts are still detected and reported, just without walking commit history to find them). This is O(size of the diff) rather than O(local commit count x remote history since diverging), and works correctly even with fully disjoint local/remote histories. Verified locally against reproductions of: a normal remote advance, a full history rewrite with completely disjoint ancestry (this took 40+ real seconds with the old rebase code against ~200 trivial commits and would be much worse with a real diff/hook set on a large repo; the new code stays under a second), a genuine content conflict (fails cleanly with the working tree restored rather than silently applying partial changes), and a binary file change. Also adds a bounded execFileSync timeout on the git subprocesses in this path so a stalled network call still fails fast instead of blocking indefinitely if something upstream of git itself stalls (e.g. a flaky proxy).
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
新建议题s introduced by this change (3)
| Severity | Finding |
|---|---|
 |
src/git.ts — git() uses execFileSync's default 1 MiB output buffer, but this new call captures the entire… |
 |
src/git.ts — In the no-upstream case handled above, this fetch may not create origin/<branch>. Shallow clones… |
|
src/git.ts — The fallback does not actually bound every subprocess: its rev-parse, diff, reset, log, and… |
What changed in this PR
Replaces slow non-fast-forward rebasing with bounded, squash-diff reconciliation.
Changes:
- Fetches the latest remote tip and reapplies local changes as one commit.
- Adds subprocess timeouts and conflict cleanup.
- Preserves binary changes and commit messaging.
| File | Description |
|---|---|
src/git.ts |
Implements the new push-reconciliation fallback. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // single diff, rather than replaying each commit. We only need the net | ||
| // content change to end up on the remote - the commit boundaries in | ||
| // between are an implementation detail of how the agent got there. | ||
| const diff = git(["diff", "--binary", previousBase, localTip], repoLocation); |
Comment on lines
+328
to
+329
| gitWithTimeout(["fetch", "--depth", "2", "--no-tags", "origin", branch], repoLocation); | ||
| const newRemoteTip = git(["rev-parse", `origin/${branch}`], repoLocation).trim(); |
Comment on lines
+249
to
+253
| * Timeout (ms) applied to every git subprocess spawned by the non-fast-forward | ||
| * fallback below. Repos are cloned shallowly (see `cloneRepo`'s `--depth 2`), | ||
| * so any of these operations should complete in seconds; a large value here | ||
| * only exists to fail fast instead of hanging silently until the caller's own | ||
| * (often much longer) timeout kills the process out from under us. |
Fix three issues identified in Copilot review: 1. (HIGH) Shallow clone fetch may not create remote-tracking ref for new branches: - Changed fetch refspec from implicit to explicit: +refs/heads/<branch>:refs/remotes/origin/<branch> - Ensures remote-tracking ref is created even when remote.origin.fetch only targets the default branch - Supports rewritten histories with + prefix 2. (HIGH) Large diffs exceed execFileSync 1 MiB buffer: - Generate diff to temp file instead of capturing directly - Pass diff file path to git apply instead of stdin - Avoids ENOBUFS errors on large monorepo changes - Clean up temp file in finally block 3. (MEDIUM) Not all subprocesses use timeout helper: - Wrapped all git calls in pushWithRebaseFallback with gitWithTimeout - Includes rev-parse, log, commit operations that can run hooks - Ensures consistent 60-second timeout prevents indefinite hangs - Maintains best-effort cleanup timeout on reset operations All changes preserve the bounded O(diff size) reapply strategy and content-level conflict detection while addressing operational concerns for large repositories. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 57c3ac2b-e726-4ad3-bf7d-278c6da551db
timrogers
commented
Aug 29, 2026
timrogers
left a comment
Author
There was a problem hiding this comment.
Thanks for the detailed review! I've addressed all three issues in the new commit:
Issue 1 (HIGH): Shallow clone fetch for new branches
- Changed fetch refspec from implicit
origin <branch>to explicit+refs/heads/<branch>:refs/remotes/origin/<branch> - This ensures the remote-tracking ref is created even for newly-created branches where single-branch fetch only targets the default branch
- The
+prefix properly handles rewritten histories
Issue 2 (HIGH): Buffer overflow on large diffs
- Switched from capturing diff to stdin to writing it to a temp file
- Pass file path to
git applyinstead of using stdin/input - Added proper cleanup in finally block
- Set
maxBuffer: undefinedon the git diff itself to allow unbounded output
Issue 3 (MEDIUM): Incomplete timeout coverage
- Wrapped ALL git subprocess calls in
pushWithRebaseFallbackwithgitWithTimeout - This includes
rev-parse,log, andcommitwhich can run hooks - Added timeout to cleanup
reset --hardin error handler - Ensures consistent 60-second fail-fast guarantee throughout the path
The fix maintains the core O(diff size) reapply strategy while addressing production concerns for large monorepos.
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
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 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.
Problem
commitAndPush's push fallback (pushWithRebaseFallback) handles a rejected push (remote branch ahead) by runninggit fetch+git rebase origin/<branch>.This SDK clones repositories shallowly (
--depth 2, seecloneRepo). When the remote has advanced far enough - or been rewritten (squash-merge, force-push, branch protection auto-merge workflows) - that the local shallow history and the freshly fetched remote tip share no common ancestor,git rebasecan't find a merge-base and falls back to replaying every local commit individually (patch-id search + full three-way merge per commit, running hooks each time).On a large monorepo this is slow and scales with the number of local commits being replayed. In production (github/sweagentd -> github/github), this manifested as CCA (Copilot Cloud Agent) push-retry finalization silently hanging for 10+ minutes with no further log output, eventually getting killed by an unrelated outer job timeout and surfacing as a confusing "connection refused" error instead of a clear failure.
Reproduction
I reproduced this locally: a shallow clone whose local branch has 202 trivial one-line commits, rebased against a disjoint (force-rewritten) upstream history, took 40+ seconds just for the rebase step - with real diffs, hooks, and a genuinely large history (like github/github), this is consistent with the multi-minute+ hangs seen in production.
Fix
Replace the rebase-based reconciliation with a fetch + squash-diff + reapply strategy:
<branch>@{upstream}, i.e. the remote tip we last forked from).--depth 2).git diff --binary) - i.e., the net content change from all of our own local commits, regardless of how many there are.git apply -3 --index(three-way apply, so content-level conflicts are still detected and reported).This is O(size of our diff), not O(local commit count x remote history since diverging), and works correctly even when local and remote history are fully disjoint. Genuine content conflicts still fail loudly (via
git apply's exit code) with the working tree restored to a clean prior state, matching the safety guarantees of the old rebase-based approach - we just no longer pay the cost of walking commit history to detect them.Also adds a bounded
timeoutto the git subprocesses on this path, so a stalled network call fails fast instead of hanging indefinitely.Verified locally against:
--binarydiff)Context
Found via a live production incident investigation (sweagentd session), where a CCA job's final push-retry step hung until the job's own timeout killed it. Local reproduction of the shallow-clone + rebase-against-disjoint-history scenario confirmed the root cause before writing this fix.