feat(mcp): let agents change thread workspaces#8680

🟠 High mcp/WorktreeMcpService.ts:982

checkout can let transitions from different threads concurrently mutate the same project root or worktree, leaving the earlier thread's durable branch binding inconsistent with the branch Git leaves checked out and allowing two threads to share one mutable workspace. The per-thread workspaceTransitionsInFlight guard does not prevent this, and each dispatch validates only that thread's prior binding, so both optimistic updates can commit. Add workspace-level serialization or recheck workspace ownership after the Git mutation and before dispatch.

Fixed in 98151e0c2e. Checkout now acquires an in-process guard for the canonical target workspace in addition to the per-thread guard, then rechecks Git inventory, status, and thread ownership while holding it. Added a two-caller race test targeting the same worktree.

🟠 High mcp/WorktreeMcpService.ts:1129

performCheckout can leave Git inconsistent with the durable thread binding when another actor changes branch or worktreePath after the initial projection read. It switches the checkout before the optimistic dispatch at threadManagement.dispatch detects the stale binding, then rolls Git back to targetBefore while the concurrent metadata update remains committed; add a binding recheck immediately before mutating Git, as performHandoff does.

✅ Resolved in 98151e0c2ee5b55e702164a7ae31136b96d351d9

Fixed in 98151e0c2e. Immediately before any Git mutation or binding move, checkout reloads the calling thread and rejects if branch, worktree path, or archive state changed. The metadata command still carries expectedBranch/expectedWorktreePath as the durable compare-and-set backstop. Added a regression test that changes the binding during preparation and verifies Git is untouched.

Addressed in 8aafc41636ec84adab80a84e289c7cefb0757f1b with a durable backstop rather than relying on another adjacent read. Workspace-binding metadata updates now carry optional expectedArchived: false alongside the branch/worktree CAS, and the production V2 decider checks all three inside its serialized command decision. A real V2 regression archives first, then dispatches the previously prepared workspace update and proves the binding is rejected and unchanged. Rollback also refuses to act once caller lifecycle or ownership differs.

The CAS and ownership-aware rollback improve recovery, but do not close the race: the recheck at 1176 is followed by resolveCommit calls before mutation. A concurrent actor can bind the target in that window with its observed branch, after which this checkout changes Git; rollback then correctly declines as unsafe, but that actor’s durable binding can disagree with Git. The binding/ownership validation still needs to be adjacent to mutation, or all binding writers need a shared lock.

Fixed in 9335400d20. Ref/commit resolution now completes before a final adjacent gate re-reads the caller lifecycle, physical bindings across projects, and target ref/dirty state immediately before createRef/switchRef. The deterministic regression changes the caller binding during commit resolution and verifies checkout_in_progress with no Git mutation or metadata dispatch.

🟠 High mcp/WorktreeMcpService.ts:776

checkout fails after performHandoff has durably created and bound the new worktree if this readWorkspaceStatus is interrupted or fails, so the caller receives no successful result even though the session is detached and the thread now points at the new worktree. Use the branch returned by performHandoff (or otherwise make this post-commit read best-effort) so status-read failure cannot turn a completed transition into a failed checkout.

✅ Resolved in 98151e0c2ee5b55e702164a7ae31136b96d351d9

Fixed in 98151e0c2e. The new-worktree checkout result now uses the completed handoff receipt branch and no longer performs a post-commit status read that could misreport a successful durable handoff as failed. Added a regression test where the next status read would fail.

🟢 Low user/source-control.md:85

A failed t3_worktree_handoff can remove the newly created target worktree, so this absolute statement misleads callers troubleshooting a failed handoff. The rollback calls gitWorkflow.removeWorktree when the pre-commit recheck or durable binding update fails; please qualify the documentation to distinguish that cleanup from normal worktree controls.

Fixed in d1e40d08db. Both user and internal docs now distinguish existing-worktree controls from rollback cleanup: a failed new-worktree transition may remove only the checkout it just created when the durable binding did not commit.

🟢 Low orchestration-v2/orchestrator-mcp-server.md:337

The documentation says moving to a shared project root is allowed when other threads are idle, but t3_thread_checkout rejects the root mutation whenever any other thread is bound there, including idle threads. Update the allowance to state that the move is rejected whenever another thread is bound to the root.

No longer relevant as of 1ed8ca43207f300ed83e1f991be080eb0b9f3e54

This is already accurate in d1e40d08db. For a project-root move without a branch switch, shouldMutateCheckout is false: the workspace_shared root guard applies only when shouldMutateCheckout is true, while the preceding workspace_in_use guard rejects the move only when another root-bound thread has an active run. An idle root owner therefore remains allowed exactly as documented.

🟠 High mcp/WorktreeMcpService.ts:200

loadActiveWorkspaceBindings excludes archived threads, so an active thread can claim or switch an archived thread's worktreePath; after thread.unarchive, both threads retain usable bindings to the same mutable checkout. Include archived threads in this ownership recheck so unarchiving cannot create a shared worktree.

Fixed in 5ef4d4c3499e4b6114850f44717f98ebc9d60a93. Canonical ownership discovery now combines active and archived shell snapshots before repository/worktree identity resolution, so an archived binding reserves the same physical checkout and blocks reuse. Added a regression proving an archived owner produces workspace_shared without Git mutation or binding dispatch.

Detached remote checkout skips rollback

High Severity

Checking out a remote ref can succeed as a detached HEAD. switchRef then returns refName of null, which performCheckout reports as partial_failure with rollback: "not_possible" without restoring the previous branch. Git is left detached while the durable binding is unchanged. This is the documented real-Git outcome when origin/feature is requested and a same-named untracked local branch exists.

^{Reviewed by Cursor Bugbot for commit 5ef4d4c3499e4b6114850f44717f98ebc9d60a93. Configure here.}

Remote default checkout targets project root

High Severity

Auto workspace resolution sends an attached thread that requests the remote default ref (for example origin/main) to projectWorktreeRoot and then mutates that checkout. Remote refs always have a null worktreePath, and origin's HEAD is marked isDefault, so the local-default heuristic misfires. A non-default remote such as origin/feature correctly stays on the current worktree. The result is an unexpected branch switch or detach on the shared project root instead of checking out the requested remote ref in the thread's current workspace.

^{Reviewed by Cursor Bugbot for commit 8aafc41636ec84adab80a84e289c7cefb0757f1b. Configure here.}

New worktree skips workspace serialization

Medium Severity

new_worktree calls performHandoff without taking the per-physical-workspace guard, and handoff never claims the created path. After createWorktree succeeds and before the binding commits, another thread can list the new checkout, pass ownership checks, and bind to it. Both threads then share one mutable worktree, which the new guards were meant to prevent.

^{Reviewed by Cursor Bugbot for commit 8aafc41636ec84adab80a84e289c7cefb0757f1b. Configure here.}

🟡 Medium mcp/WorktreeMcpService.ts:1433

When target.create successfully creates the branch but switchRef fails, the checkout returns operation_failed while leaving the newly created local branch behind. rollbackOwnedCheckout only restores checkout state and never calls deleteLocalBranch, so retries with the same branch are rejected as already existing. Clean up the created branch on this failure path when it is safe to do so.

Intentional conservative behavior in 05f8e6450d, matching the parent review direction. A failed switch does not establish safe ownership of the created ref: it may have become attached or advanced elsewhere, so deleting it to make the original create request retryable risks data loss. The result preserves the branch and reports the failure; a retry can select/reuse the retained branch after inspecting current state. The focused test verifies no delete is attempted.

🟠 High mcp/WorktreeMcpService.ts:489

Rollback can delete commits belonging to another actor. After removeWorktree succeeds, deleteLocalBranch runs git branch -D on worktree.worktree.refName without confirming that the branch still points to the commit created by this handoff, so a concurrent checkout or branch update can cause unrelated commits to be deleted. Verify the branch tip is unchanged before deleting it, or use a compare-and-delete operation.

Fixed in 05f8e6450d. Failed handoff cleanup still removes the newly created worktree path, but no longer force-deletes the branch by name; the branch is retained because concurrent activity means this operation cannot prove the ref is still unattached and unchanged. Focused handoff race and dispatch-failure tests assert removal plus branch retention.

Unresolved same-repo owners skipped

Medium Severity

loadActiveWorkspaceBindings drops a thread when inventory belongs to the target repository but currentWorktreeRoot is null. That is an unresolved same-repo owner, yet checkout continues as if nobody holds the checkout instead of failing closed.

^{Reviewed by Cursor Bugbot for commit 05f8e6450db277e831f923c4c278f790716a9090. Configure here.}

List omits archived checkout owners

Medium Severity

listWorktrees builds bindings only from listProjectThreads, which reads snapshot.threads and ignores archivedThreads. Checkout now treats an archived binding as occupying the physical worktree, so list can report available with no owners for a checkout that t3_thread_checkout will reject as workspace_shared.

^{Reviewed by Cursor Bugbot for commit 05f8e6450db277e831f923c4c278f790716a9090. Configure here.}

Missing worktrees block all moves

Medium Severity

t3_thread_checkout and attached t3_worktree_handoff both require the thread's recorded workspace to still be a readable project worktree before any move. A workspace_missing binding therefore cannot return to the project root, reuse another listed worktree, or create a new one, even though the project repository itself is available.

^{Reviewed by Cursor Bugbot for commit 0a622c2b38dc25c215036226855d9392ff539540. Configure here.}

Confirmed and fixed in 03d0e2bce4. A recorded checkout now resolves through the canonical binding inventory as optional: only a confirmed non-repository/missing source is treated as stale, while unresolved Git failures still fail closed. Recovery targets are always selected from the healthy project inventory, so return-to-root and listed-worktree reuse never touch the missing checkout; new-worktree handoff resolves its base from the physical project root. Focused regressions cover all three recovery paths.

Project root skips branch checks

Low Severity

target.type='project_root' never applies the existence checks used for target.type='branch'. create=true on an existing local branch still counts as a mutation, so dirty and shared-root guards fire and createRef later fails. A missing branch without create also falls through to switchRef and surfaces as operation_failed.

^{Reviewed by Cursor Bugbot for commit 0a622c2b38dc25c215036226855d9392ff539540. Configure here.}

Confirmed and fixed in 03d0e2bce4. project_root requests with a branch now resolve the same local/remote ref maps and reject create=true for an existing local branch or create=false for a missing ref before dirty/shared/mutation checks. The focused regression asserts both typed invalid_request outcomes and that neither createRef nor switchRef runs.

Status failure reported as not a repo

Low Severity

When the recorded checkout inventory succeeds but the follow-up status read fails, that failure is turned into { isRepo: false }. A same-workspace checkout then rejects the target as not a Git repository instead of failing closed as an unresolved Git error.

^{Reviewed by Cursor Bugbot for commit 4453207fd2655b3debb17b3146e8bc45110f1958. Configure here.}

Fixed in 1d545798de018346d5a02146a79727b602844f6d. Once inventory confirms the recorded checkout exists, the status read stays in the typed error channel and returns before ref selection or mutation. The synthetic result is now limited to inventory-confirmed missing/non-repository paths; the regression asserts no switch, ref creation, or metadata dispatch.

🟡 Medium orchestration-v2/orchestrator-mcp-server.md:49

The documentation promises that credentials expire after a maximum lifetime, but McpSessionRegistry.touch runs for provider turns, so an active session can keep its credential indefinitely; the stated security bound is not enforced. Remove the maximum-lifetime claim and document only the liveness-based expiry.

Fixed in 76ccdb2bbb30da271f74fa152ba3c3df14409ba9. The docs now match McpSessionRegistry: resolve and provider-turn touch refresh lastAliveAt, pruning uses only livenessWindowMs, and provider-session release revokes eagerly. The conditional preview wording remains intact. Targeted formatter and diff checks pass.

Rollback ignores moved original branch

Medium Severity

rollbackOwnedCheckout attests the post-switch checkout, then switchRefs back to targetBefore.refName without checking that this ref still points at targetBeforeCommit. If that branch moved during the attempt, rollback lands on a different commit than the workspace started from, which breaks the rule that unattested Git state is left untouched.

^{Reviewed by Cursor Bugbot for commit 1d545798de018346d5a02146a79727b602844f6d. Configure here.}

Project-root binding not normalized

Medium Severity

A project_root target only writes worktreePath: null when the physical checkout changes. If the recorded path is a nested directory or alias of the project root, workspaceChanged is false and the durable binding keeps the old path. Later turns can keep using that nested cwd even though the caller asked to return to the main checkout.

^{Reviewed by Cursor Bugbot for commit 1d545798de018346d5a02146a79727b602844f6d. Configure here.}

🟢 Low user/source-control.md:84

The documentation incorrectly says every workspace-path move restarts the agent session, but a path-only checkout ends the current turn unless continuationPrompt is supplied; users must send another message to continue. Please document the conditional continuation behavior.

Fixed in 875fd5e4a1832f0ce7632c4b264cc98c0dbe5284. The user guide now says a workspace-path move ends the current turn; continuationPrompt queues a replacement turn, while an omitted continuation leaves the thread idle until its next message. The internal MCP documentation uses the same conditional wording.

🟠 High mcp/WorktreeMcpService.ts:1284

performCheckout executes the entire guarded transition inside Effect.uninterruptibleMask without calling restore, so Git, status, inventory, and dispatch operations ignore cancellation. A stalled or abandoned request therefore keeps the per-thread and per-workspace guard entries until completion, causing subsequent calls to fail with checkout_in_progress. Use restore around the transition after acquiring the guard and registering its finalizer, as handoff does.

✅ Resolved in 875fd5e4a1832f0ce7632c4b264cc98c0dbe5284

Fixed in 875fd5e4a1832f0ce7632c4b264cc98c0dbe5284 with a narrow cancellation boundary. Projection, inventory, owner, status, and ref-resolution preflight reads—including the final mutation-adjacent recheck—are interruptible. Once that recheck passes, Git mutation, durable binding/continuation, and ownership-proven rollback remain uninterruptible. The Deferred regression requests interruption synchronously before releasing the read gate, proves zero Git mutation/dispatch, and proves a later checkout succeeds after both guards are released. The focused WorktreeMcpService suite passes 117/117.

New worktrees can be double-bound

High Severity

performHandoff creates the worktree and then rechecks only the caller thread before binding. It never takes the physical checkout guard and never inspects other owners of the new path. A concurrent t3_thread_checkout of that branch or path can bind another thread first; the handoff still commits, so two threads share one mutable checkout.

^{Reviewed by Cursor Bugbot for commit 69afd25d7a604c150202fec55efd927f0c8eda83. Configure here.}

Fixed in 875fd5e4a1832f0ce7632c4b264cc98c0dbe5284. After creation, handoff resolves the actual physical checkout, acquires the same common-dir/worktree-root guard as checkout, and rereads global active/archived bindings before committing. The deterministic handoff-versus-checkout race proves the winning binding survives and the losing handoff retains rather than deletes the checkout. Every cleanup path now re-proves no competing owner, unchanged caller binding, exact physical identity, clean/ref state, and the pre-create expected HEAD; ambiguous or changed state is retained. The focused suite passes 117/117.

mcp/WorktreeMcpService.test.ts:2658

This block (through the does not attribute a nested independent repository... case) is t3_worktree_list coverage living inside the t3_thread_checkout suite, and three of the cases — includes archived thread bindings retained on a physical checkout, attributes a nested recorded cwd to its physical worktree root, and does not attribute a nested independent repository to the project worktree — are byte-identical duplicates of tests still present in the t3_worktree_list describe above. Looks like a merge/rebase artifact. Consider deleting the duplicates here and moving the remaining runList cases back into describe("t3_worktree_list") so the checkout suite only exercises runCheckout.

No longer relevant as of 1ed8ca43207f300ed83e1f991be080eb0b9f3e54

Fixed mechanically in 1ed8ca43207f300ed83e1f991be080eb0b9f3e54. I verified the archived-binding, nested-cwd attribution, and nested-independent-repository runList case bodies duplicated the existing t3_worktree_list coverage, then removed only the checkout-describe copies. The three unique pagination/bounded-identity runList cases were moved intact into the t3_worktree_list describe. No production code or assertions changed. WorktreeMcpService passes 114/114, canonical t3 typecheck exits 0, and targeted formatting/diff check pass.

🟡 Medium mcp/WorktreeMcpService.ts:1657

A target.create=true request with branch: "--unset-upstream" executes git branch --unset-upstream, changing the current branch’s upstream configuration instead of creating a branch; the subsequent checkout then fails. Validate requestedBranch as a Git ref name and ensure the driver terminates options before passing user input to git branch.

Rollback fails closed without partial details

Medium Severity

rollbackOwnedCheckout lets loadActiveWorkspaceBindings fail the effect instead of treating that unresolved-owner case as not_possible. After Git has already moved, callers then surface a generic operation_failed with no partial payload, so agents cannot tell that checkout changed, that the binding did not commit, or that rollback was skipped.

^{Reviewed by Cursor Bugbot for commit d653ad1c5d343d0257fd4b82a2acc03840db68b5. Configure here.}

🟡 Medium mcp/WorktreeMcpService.ts:1715

After a successful branch switch, checkout leaves VcsStatusBroadcaster's cached status unchanged, so subscribers and later getStatus calls can continue reporting the previous branch/PR status until another refresh occurs. The normal vcsSwitchRef path refreshes this cache; call vcsStatusBroadcaster.refreshStatus(targetWorkspacePath) after the successful checkout here as well.

Fixed in 48384161a148da3e5965a62a47ed586b2bd37f03. After a switched or newly-created checkout is durably bound, the service starts the existing broadcaster refresh as an ignored detached effect. Refresh failure therefore cannot relabel a committed checkout as failed. Focused upstack coverage passes 115/115.