fix(server): recovering a pruned worktree no longer wedges every thread - #81
Merged
Merged
Conversation
A turn start holds the worktree's workspace lease, a one-permit semaphore that is not reentrant. When the thread's managed worktree had been pruned, the recovery path re-created it and ran the setup script from inside that lease. Running the script opens a terminal, which takes the same lease, so the turn start waited on itself forever. The reactor worker is serial, so every later turn start on any thread queued behind it and the threads sat on "Working" with no provider process. The setup script now runs in a forked fiber, so it takes the lease once the turn start releases it. A setup failure is logged instead of failing the turn. The recovery test missed this because its setup runner mock never took the lease. It does now, and it reproduced the hang before the fix. Model: Claude Fable 5.1 via Claude Code.
github-actions
Bot
added
vouch:trusted
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
1 participant
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
Sending a message to a thread whose managed worktree had been pruned left that thread stuck on "Working" forever, and then every other thread's next turn got stuck behind it. Only a server restart cleared it.
A turn start holds the worktree's workspace lease, a one-permit semaphore that is not reentrant. With the worktree missing, the recovery path re-created it and ran the setup script from inside that lease. Running the script opens a terminal, and
terminalManager.opentakes the same lease, so the turn start waited on itself. The reactor worker is serial, so all later turn starts queued behind the hung one and no provider process was ever spawned.The lease came from upstream's storage cleanup change and the recovery path is fork-local. Each is fine alone.
Fix
The recovery setup script runs in a forked fiber, so it takes the lease once the turn start releases it. A setup failure is logged instead of failing the turn.
The recovery test missed this because its setup runner mock never took the lease. The mock now takes it like the real terminal manager does. It reproduced the hang before the fix and passes after.
Verifying
vp test run src/orchestration/Layers/ProviderCommandReactor.test.tsinapps/serverpasses. In a running app: send a message to a thread whose worktree was pruned. It should start a provider session and a setup terminal instead of hanging.Model: Claude Fable 5.1 via Claude Code.