fix(server): stop the branch rename before cancelling worktree setup

[saphid]

Problem

When a thread launches into a new worktree, ThreadLaunchService renames the temporary t3/<hash> branch in a background fiber forked into the service-lifetime preparation scope. If worktree setup is then cancelled, cleanup removes the worktree and clears it from the thread, but the rename fiber keeps running. When it lands it dispatches thread.metadata.update with the removed worktreePath, so the thread points at a worktree that no longer exists. On main this showed up as an intermittent ThreadLaunchService.test.ts > cancels tracked setup before provider work is released failure (expected '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/repo-worktrees/feature' to equal null), and any added latency on the launch commit path makes it common.

A very small, focused fix for an obvious bug (CONTRIBUTING.md): one defect, one function, plus a regression test.

Change

Keep the rename fiber and interrupt it (which waits for it to stop) as the first step of the cleanup branch that removes the worktree, before the worktree is removed and forgotten. Main's current cleanup rule (#19102) is kept as is: the worktree is removed only when (cancelled && !runReleased) || !workspaceRecorded, so a run already released to the provider keeps both its worktree and its rename, and successful launches are untouched.

Verification

Head d45b8984cc (main dff750b1b8 merged; the conflict with #19102's runReleased cleanup was resolved by keeping main's condition and adding the interrupt inside it).

• vp test run apps/server/src/orchestration-v2/ThreadLaunchService.test.ts: 73 passed.
• Red check on the merged head: removing the interrupt line fails exactly the new test, stops an in-flight branch rename when tracked setup is cancelled (the cancelled setup's branch rename kept running: expected false to be true). The test holds branch-name generation open with Deferreds (no sleeps or polling), cancels, then releases it.
• vp run typecheck (apps/server) clean; vp lint on the two files: one pre-existing optional-chaining warning in the test file.
• Not checked: the race in a live app (it needs a cancel inside the rename window). No UI change.

Claude Opus 5.5 (Claude Code). Merge with main: Claude Opus 5.5 (Claude Code) via T3 Code; review GPT-6.1 Sol.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at 7082440

Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that interrupts an existing background rename only when cancelled worktree cleanup is removing the worktree, with normal launches unchanged. A deterministic regression test covers the race without introducing new production capability or configuration.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

No code changes detected at d45b898. Prior analysis still applies.

^{You can add or adjust custom eligibility rules. Learn more.}

Dismissing prior approval to re-evaluate 7082440

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: ba69fb87-7642-4e3f-9ae2-e368718cc01c

📥 Commits

Reviewing files that changed from the base of the PR and between e637841 and d45b898.

📒 Files selected for processing: 2

• apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
• apps/server/src/orchestration-v2/ThreadLaunchService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

ThreadLaunchService now runs temporary worktree branch renaming in the background during workspace preparation. Cleanup interrupts the tracked rename fiber before removing an abandoned worktree. A test checks cancellation while branch-name generation is blocked.

Changes: Branch Rename Lifecycle

Layer / File(s)	Summary
Background rename and cleanup apps/server/src/orchestration-v2/ThreadLaunchService.ts, apps/server/src/orchestration-v2/ThreadLaunchService.test.ts	ThreadLaunchService tracks and forks branch-name generation, renaming, and metadata updates in the preparation scope. Cleanup interrupts the rename fiber before worktree removal. The test checks that cancellation interrupts generation, prevents a rename call, and leaves the thread’s worktree path unset.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to d45b8

The change coordinates branch renaming with cancelled-worktree cleanup without an identified regression in successful launches. It is mergeable after normal checks.

Security architecture risk: ⚪ Minimal · up to d45b8

The fix stops an existing background operation before deleting its worktree. It improves cancellation consistency without adding an access path, expanding privileges, or changing public contracts. No material security risk was identified in the changed flow.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed operation remains scoped to the existing preparation's thread identity and claimed worktree. The diff does not broaden the inputs, paths or authority used for branch generation, Git rename or worktree removal.

Trust Boundaries and Controls

• observed — Branch generation still consumes message text, attachments and optional context through the existing generation service, then uses the existing Git and metadata operations. Metadata dispatch uses the existing per-thread lock; this PR adds cancellation coordination rather than a new authorization boundary.

Resilience and Maintainability Implications

• observed — The regression test blocks branch-name generation, cancels tracked setup, then releases generation. Its assertions require interruption, no rename attempt and null worktree metadata. This directly covers the stale-writer race during generation, but does not demonstrate every external-operation failure or crash-recovery state.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description explains the race, the fix, scope justification, focused verification, limitations, and agent usage. It omits the required “Scope and approval” heading, but includes the required small…
Linked Issues check		The description does not link a triaged issue, but it explains why this focused fix qualifies for the obvious-bug exemption.
Out of Scope Changes check		The changes are limited to interrupting the branch-rename fiber during cancelled worktree cleanup and adding a regression test.
Title check		The title clearly and concisely describes the primary fix: stopping the branch rename before cancelled worktree cleanup.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[AdEx-Partners-DE]

Independent confirmation from a downstream fork on Windows (Node 24.13.0).

• Our fork adds a little latency to the dispatch path, and cancels tracked setup before provider work is released failed on every run with expected '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/repo-worktrees/feature' to equal null. The fork's cause was the same one described here: the rename fiber in preparationScope records the removed worktree after the cancel cleanup.
• We applied the same change (keep the fiber, interrupt it in the cancelled cleanup before the worktree is forgotten). After that the test file passes.
• Using pingdotgg/t3code main at 5070a83 (its ThreadLaunchService.ts and test file), a test that blocks generateBranchName until the cancel and then checks that the rename was interrupted fails without the change (expected false to be true) and passes with it (63/63, twice).

So the fix here matches what we needed. We are carrying the same patch until this merges.

Posted by Claude (AI) on behalf of @AdEx-Partners-DE; no human reviewed this comment.