fix: stop a merge follow-up from blocking on the worktree that spawned it - #4227
Merged
Conversation
…d it A PR's merge follow-up attaches its own worktree to the PR branch, and the CoS evaluation tick preps that task a second or two after the cleanup queues it — while that same cleanup is still tearing its own worktree down. `git worktree add` then fails with "is already used by worktree at …", the task is blocked as `worktree-failed`, and nothing is left to land the PR (the two calls ran 0.7s apart in the reported incident). Release the branch before anything is queued against it, on both producers in `cleanupAgentWorktree` — the PR path and the `if-missing` net that hands a failed run's orphaned PR to the same machinery. That closes the routine race; a worktree `removeWorktree` refuses to delete (uncommitted changes) can still hold the branch, so a branch-busy failure is now a TIMED pause (`worktree-busy`) revived by the existing cooldown sweeper rather than a permanent block, bounded at 5 waits before it gives up and takes the ordinary `worktree-failed` block. Two related fixes fall out: - `updateTask` stripped `existingBranch` on any non-paused terminal status, including a follow-up's own copy — which is that task's configuration, not a resume pointer. Re-running a blocked follow-up therefore cut a worktree fresh off main, so a fix-and-push landed on a branch the PR never heard of. Keyed on `resumedFromAgentId` now, the marker the resume mechanism always writes with it. - The orphaned-PR notifier and the voice proactive trigger stay quiet on a timed-cooldown block. Both dedupe once per PR/task, so announcing a pause that self-revives would have swallowed the card for a real block later.
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
A CoS merge follow-up (
sys-rl-*) went straight toblockedwith "Worktree creation failed — isolation was explicitly requested", orphaning the PR it existed to land. The task file showed the follow-up carryinguseWorktree: truebut noexistingBranch, which made it look like a serialization problem. It wasn't — the server log has the real sequence:cleanupAgentWorktreequeued the follow-up before tearing down its own worktree. Git allows a branch in exactly one worktree, so the follow-up'sgit worktree addlost the race against a teardown that was already in flight. (The missingexistingBranchwas a consequence:updateTaskstrips the pointer on a terminal status.)Fixes
1. Release the branch before queueing anything against it — on both producers in
cleanupAgentWorktree: the PR path, and theif-missingnet that hands a failed run's orphaned PR to the same follow-up machinery. The Copilot pre-request and thefindPullRequestForBranchlookup still run first, since those need the checkout.2. A branch-busy failure is now a timed pause, not a permanent block. The ordering fix closes the routine race, but a worktree
removeWorktreerefuses to delete (uncommitted changes) holds the branch until a human clears it. SoagentWorkspacePrepclassifies the failure via a newisBranchCheckedOutElsewhereErrorpredicate and blocks withworktree-busy+ acooldownUntil— a new member ofTIMED_COOLDOWN_BLOCKED_CATEGORIES, revived by the existing cooldown sweeper. Bounded at 5 waits (~10 min), after which it takes the ordinaryworktree-failedblock so the orphaned-PR card still reaches a human. The predicate is deliberately kept out ofisGitLockError, whose retry budget is sized in milliseconds for bookkeeping locks.3. A follow-up's own
existingBranchsurvives a terminal status.updateTaskstripped it as a spent resume pointer, but a follow-up sets that key as its configuration — it exists to land the PR on that branch. Re-running a blocked follow-up therefore cut a worktree fresh offmain: the merge still targets the right PR by url, but any fix-and-push lands on a branch the PR never heard of. Now keyed onresumedFromAgentId, the marker the resume mechanism always writes alongside it — so the store stays task-kind-agnostic rather than naming one task type.4. Two block reporters stay quiet on a self-reviving pause. The orphaned-PR notifier and the voice proactive trigger both dedupe once per PR/task, so announcing a cooldown would have swallowed the card for a real block the task lands on later.
Supporting cleanups:
PAUSED_BLOCKED_CATEGORIESis now composed fromTIMED_COOLDOWN_BLOCKED_CATEGORIES(the file's own anti-drift convention — every timed cooldown is a pause by construction),unblockExpiredOrphanCooldowns→unblockExpiredCooldownsnow that it sweeps the whole vocabulary, andworktreeBusyAttemptsresets inreviveBlockedTaskalongside the other spent budgets.Test plan
server/services/worktreeManager.test.js—isBranchCheckedOutElsewhereErroragainst both git wordings (is already used by worktree at/is already checked out at), and that it does not match the permanent'<path>' already exists/a branch named 'X' already existsfailures or leak intoisGitLockError.server/services/cleanupAgentWorktree.test.js—removeWorktreeruns beforeaddTaskon both producers (asserted viamock.invocationCallOrder), and the Copilot pre-request still runs before the teardown that takes its checkout away.server/services/agentWorkspacePrep.test.js— branch-busy pauses withworktree-busy+cooldownUntiland keepsexistingBranch; counts up from a string-typedworktreeBusyAttempts; gives up at the cap with the git error in the reason; a permanent failure still hard-blocks.server/services/cosTaskStore.test.js— a self-configured branch pointer survives a terminal block, one marked byresumedFromAgentIddoes not,worktree-busykeeps it, andreviveBlockedTaskclears the attempt counter.server/services/cosTaskGenerator.cooldown.test.js— the sweeper revivesworktree-busy, preserving both the branch pointer and the attempt count.server/services/orphanedPrNotifier.test.js— no card for a timed-cooldown block, and the check precedes the dedupe probe so the PR's one card isn't consumed.