You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(server): Grok hand-off turns settle after the worktree changes - #35390
AcpAdapterV2.interruptTurn takes runtimeTransitionPermit and runs its transition in a fiber forked with Effect.forkIn(sessionScope, ...), releasing the permit via Effect.ensuring inside that fiber. After a worktree hand-off, ProviderSessionManager.detach has already closed the session scope; Effect's forkIn then returns an unstarted, already-interrupted fiber, so the ensuring/release never runs. The first interrupt exits Interrupted and the permit leaks; the ingestion-failure cleanup's second interruptTurn blocks forever on the permit, so the failed terminal is never written and the run stays running with later messages queued. The fix forks the transition with Effect.forkDetach (same startImmediately/uninterruptible options) when sessionScope.state is already Closed, so the transition actually runs and releases the permit; with an open scope it still forks into the session scope as before. No regex or parser added. The live Grok repro was not rerun; the fix is shown by the unit test.
Commit fd8e386 (origin/main). Isolated dev server: T3CODE_DEV_SEED=0 vp run dev in a fresh worktree. Enabled the grok provider instance in the worktree's settings.json, added a fresh one-commit git repo with t3 project add, ran t3 thread start --provider grok --model grok-4.6 --wait (turn 1 completed, reply 'ready'), then t3 thread send <thread> "Call the t3_worktree_handoff MCP tool now... Do not pass a continuationPrompt...". Server trace: mcp.tool.t3_worktree_handoff Success (589 ms); AcpAdapterV2.interruptTurn exit Interrupted ('All fibers interrupted without error', 16 ms) inside ProviderSessionManager.detach (Success); then WARN 'orchestration V2 provider event ingestion failed' with ProviderAdapterEventStreamError -> 'Workspace changed.'. The second AcpAdapterV2.interruptTurn span (from the ingestion-failure cleanup) never ended, and 'failed to interrupt turn after ingestion failure' was never logged. t3 thread show reported the thread running with the hand-off worktree bound; a follow-up t3 thread send returned Queued. Still running more than a minute later. Deviations: model grok-4.6 (grok-4.7-build-fast isn't on this machine's model list), and a local, uncommitted change skipping AcpSessionRuntime's 'did not advertise configured authentication method' check, because this machine's Grok 1.0.77 with XAI_API_KEY advertises only grok.com auth (it does accept xai.api_key). Failing test: AcpAdapterV2.test.ts 'settles interrupts that arrive after the session scope has closed' fails on main with 'an interrupt nobody cancelled must not exit as interrupted'. It passes with a scratch change that forks the transition detached instead of into the closed sessionScope; that change was reverted and not committed.
Evidence
Being captured: before and after in the real app. This section is filled in before the pull request can merge.
Tests
vp test run src/orchestration-v2/Adapters/AcpAdapterV2.test.ts -t "after the session scope has closed" (apps/server): passes with the fix, fails without it ('an interrupt nobody cancelled must not exit as interrupted'). vp test run src/orchestration-v2/Adapters/AcpAdapterV2.test.ts: 105/105 passed. vp run check:changed: format, lint, knip, graphql, typecheck (14 packages), tests (2 files) all pass on the second run; the first run failed only on format, fixed with vp fmt.
Not in this change
Other Effect.forkIn(sessionScope) call sites in packages/provider-acp/src/server/adapter.ts have the same closed-scope behaviour but do not hold the transition permit; I left them unchanged.
The ingestion-failure cleanup in RunExecutionService.ts calls interruptTurn without the session manager's 15 s timeout wrapper; this fix does not change that.
AI-assisted; verified with the tests and tools listed above.
Reproduces #35274: interruptTurn forks its transition into a closed
session scope, the fiber never starts, the transition permit leaks, and
the next interrupt waits forever.
AI-assisted; verified with the tests and tools listed above.
AcpAdapterV2.interruptTurn forked its transition into the session scope.
After a worktree hand-off detaches the session, that scope is closed, so
the fork never started, the transition permit leaked, and the ingestion
cleanup's interrupt waited on it forever, leaving the run running. Fork
detached when the session scope is already closed.
Fixes#35274
AI-assisted; verified with the tests and tools listed above.
Macroscope's review found this PR approvable — The change lets interrupt cleanup finish after a session scope closes, preventing a leaked permit from leaving hand-off turns stuck. Normal open-session behavior and existing teardown safeguards remain unchanged.
No code changes detected at 973fe5f. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
interruptTurn now starts its transition fiber independently when the session scope is closed. A regression test covers normal and restart-requesting interrupts after scope closure.
Changes: ACP interrupt handling
Layer / File(s)
Summary
Transition fiber lifecycle and regression coverage packages/provider-acp/src/server/adapter.ts, apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts
When the session scope is closed, interruptTurn uses forkDetach; otherwise, it uses forkIn. Both paths start immediately and run uninterruptibly. The test closes the scope during a hanging prompt, then checks interrupts with and without a runtime-restart request.
The change makes interrupts after a closed session scope settle, which should fix hand-off turns left running. No concrete merge-blocking risk was found; the live Grok reproduction was not rerun, so a quick manual check is reasonable.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the fix (Grok hand-off turns settling after the worktree changes), which matches the changeset: interruptTurn now forks the transition detached when the session scope is closed, so the…
Description check
The description follows the template well. It gives a problem statement with the mechanism and a change section explaining the forkDetach switch for closed scopes. It names the issue (#35274) in Scope…
Linked Issues check
Directly linked issue #35274 requires that a Grok t3_worktree_handoff turn settles and that the next message runs in the new worktree. The reported hang follows from a leaked runtimeTransitionPermit. …
Out of Scope Changes check
The whole-PR diff against the merge base contains two code changes. (1) packages/provider-acp/src/server/adapter.ts: a 7-line change in AcpAdapterV2.interruptTurn that selects forkDetach or forkIn by …
Full details: Title check
Explanation
The title names the fix (Grok hand-off turns settling after the worktree changes), which matches the changeset: interruptTurn now forks the transition detached when the session scope is closed, so the permit is released. The title is specific and follows the conventional commit form. It does not name the mechanism (closed scope, forkDetach), but it still identifies the main change.
Full details: Description check
Explanation
The description follows the template well. It gives a problem statement with the mechanism and a change section explaining the forkDetach switch for closed scopes. It names the issue (#35274) in Scope and includes a reproduction on main. Verification lists focused tests, the full file result, and the check run. The Evidence section is a placeholder that says it is still being captured and must be filled before merge. The Scope section does not explain why a fix without an issue is needed, but it links the issue. The template's commit-title guidance is followed. The Evidence placeholder is the main gap.
Full details: Linked Issues check
Explanation
Directly linked issue #35274 requires that a Grok t3_worktree_handoff turn settles and that the next message runs in the new worktree. The reported hang follows from a leaked runtimeTransitionPermit. When the session scope is closed, Effect.forkIn(sessionScope, ...) returns a fiber that never starts, so its Effect.ensuring release never runs. The second interruptTurn then waits on the permit. The whole-PR diff (merge base to head) changes only packages/provider-acp/src/server/adapter.ts, in AcpAdapterV2.interruptTurn. When sessionScope.state._tag is Closed, the transition forks with Effect.forkDetach using the same startImmediately and uninterruptible options. When the scope is open, it still forks with Effect.forkIn(sessionScope, ...). This addresses the root cause described in #35274. The regression test 'settles interrupts that arrive after the session scope has closed' (apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts) covers the closed-scope path. The PR reports that the test fails without the fix and passes with it. The live Grok reproduction was not rerun, so the end-to-end thread state after hand-off is not observed in this review. The Codex, Claude, and Muse rows in the issue are contextual only.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff against the merge base contains two code changes. (1) packages/provider-acp/src/server/adapter.ts: a 7-line change in AcpAdapterV2.interruptTurn that selects forkDetach or forkIn by sessionScope state. This is the fix for #35274. (2) apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts: a regression test for interrupts after scope close, which supports that fix. Both changes connect directly to the linked issue. The PR states that other Effect.forkIn(sessionScope) call sites and the ingestion-failure timeout wrapper are deliberately unchanged, and the whole-PR adapter diff does not touch them. The Evidence section is a placeholder and does not affect code scope. The previous-head diff shows many unrelated hunks, but those reflect the earlier reviewed head and are not part of the whole-PR scope; they are not counted here.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.
The automated pipeline stopped on this pull request and will not merge it:
the capture step could not show the problem on the base commit, so there is nothing to prove fixed.
On head: enabled Grok, added a fresh project, started Handoff proof with 'Reply ready.' and observed completion. Sent 'Call the t3_worktree_handoff MCP tool now to move this thread to a new worktree on a new branch named handoff-proof. Do not pass a continuationPrompt. Do not run other tools.' The worktree was bound, but the turn failed with 'The provider event stream closed unexpectedly' and retained a Waiting handoff row. Sent 'In the new worktree, reply follow-up complete.'; it failed to open the provider session, while the stale Waiting row remained. On base: the initial 'Reply ready.' turn and retries failed to open the provider session because the configured authentication method was not advertised, preventing the handoff reproduction. Reviewed both recordings and the head screenshot before uploading. Stopped both servers and removed both worktrees and scratch files.
🤖 Claude Opus 5.5 responding on behalf of leonhoulier
I ran the live Grok repro this PR still needs. The hang from #35274 no longer reproduces on current main, so the app can't show a before and after for this fix.
Setup: macOS, Grok CLI 1.0.77, grok-4.7-build-fast, signed in normally (the auth check that blocked the capture step didn't fire here). Two isolated dev servers ran side by side from fresh homes: main at 2dd6cddbc3, and this PR's head 973fe5f7fc merged onto that commit, which merged cleanly. Each thread learned a code word, then called t3_worktree_handoff with no continuationPrompt. A follow-up asked for pwd, the branch and the code word. These are the same steps that hung 3 of 3 times when I filed #35274.
Hand-off turn
Next message
main at 65c0195b46 / e9e934bb3d (11-12)
stays running, later messages queued (3 of 3)
never runs
main at 2dd6cddbc3
completed (3 of 3)
runs in the worktree, code word kept
this PR on 2dd6cddbc3
completed (2 of 2)
same
Two other runs don't count. On main, my own edit to that checkout restarted the dev server mid-run. On this PR, Grok called the tool by the wrong name, so no hand-off happened.
Why it no longer hangs, as far as I can tell: the traces on both servers contain no AcpAdapterV2.interruptTurn span during the hand-off. That covers both the one inside detach that the PR description shows and the cleanup one. Other AcpAdapterV2 spans, such as startTurn, are recorded, so the missing span isn't a tracing gap. Without the first interrupt, the permit can't leak. #36360's new branch would also skip the second interrupt. Your base trace at fd8e386f7f still shows the detach interrupt; I haven't pinned down which change since then stopped it.
The adapter bug itself is still on main. With only this PR's new test applied to 2dd6cddbc3, settles interrupts that arrive after the session scope has closed fails ("an interrupt nobody cancelled must not exit as interrupted"). With the fix, AcpAdapterV2.test.ts passes 123 of 123. So this now reads as hardening for any other path that interrupts an ACP session after its scope closes, rather than the fix for #35274. Fixes #35274 may no longer fit; your call.
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
size:XS0-9 changed lines (additions + deletions).takeover:blockedTakeover blocked; reason in the PR bodyvouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
2 participants
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.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
added
the
takeover:in-progress
AcpAdapterV2.interruptTurn takes runtimeTransitionPermit and runs its transition in a fiber forked with Effect.forkIn(sessionScope, ...), releasing the permit via Effect.ensuring inside that fiber. After a worktree hand-off, ProviderSessionManager.detach has already closed the session scope; Effect's forkIn then returns an unstarted, already-interrupted fiber, so the ensuring/release never runs. The first interrupt exits Interrupted and the permit leaks; the ingestion-failure cleanup's second interruptTurn blocks forever on the permit, so the failed terminal is never written and the run stays running with later messages queued. The fix forks the transition with Effect.forkDetach (same startImmediately/uninterruptible options) when sessionScope.state is already Closed, so the transition actually runs and releases the permit; with an open scope it still forks into the session scope as before. No regex or parser added. The live Grok repro was not rerun; the fix is shown by the unit test.
Fixes #35274
Reproduction on main
Commit fd8e386 (origin/main). Isolated dev server:
T3CODE_DEV_SEED=0 vp run devin a fresh worktree. Enabled the grok provider instance in the worktree's settings.json, added a fresh one-commit git repo witht3 project add, rant3 thread start --provider grok --model grok-4.6 --wait(turn 1 completed, reply 'ready'), thent3 thread send <thread> "Call the t3_worktree_handoff MCP tool now... Do not pass a continuationPrompt...". Server trace: mcp.tool.t3_worktree_handoff Success (589 ms); AcpAdapterV2.interruptTurn exit Interrupted ('All fibers interrupted without error', 16 ms) inside ProviderSessionManager.detach (Success); then WARN 'orchestration V2 provider event ingestion failed' with ProviderAdapterEventStreamError -> 'Workspace changed.'. The second AcpAdapterV2.interruptTurn span (from the ingestion-failure cleanup) never ended, and 'failed to interrupt turn after ingestion failure' was never logged.t3 thread showreported the threadrunningwith the hand-off worktree bound; a follow-upt3 thread sendreturnedQueued.Still running more than a minute later. Deviations: model grok-4.6 (grok-4.7-build-fast isn't on this machine's model list), and a local, uncommitted change skipping AcpSessionRuntime's 'did not advertise configured authentication method' check, because this machine's Grok 1.0.77 with XAI_API_KEY advertises onlygrok.comauth (it does acceptxai.api_key). Failing test: AcpAdapterV2.test.ts 'settles interrupts that arrive after the session scope has closed' fails on main with 'an interrupt nobody cancelled must not exit as interrupted'. It passes with a scratch change that forks the transition detached instead of into the closed sessionScope; that change was reverted and not committed.Evidence
Being captured: before and after in the real app. This section is filled in before the pull request can merge.
Tests
vp test run src/orchestration-v2/Adapters/AcpAdapterV2.test.ts -t "after the session scope has closed" (apps/server): passes with the fix, fails without it ('an interrupt nobody cancelled must not exit as interrupted'). vp test run src/orchestration-v2/Adapters/AcpAdapterV2.test.ts: 105/105 passed. vp run check:changed: format, lint, knip, graphql, typecheck (14 packages), tests (2 files) all pass on the second run; the first run failed only on format, fixed with vp fmt.
Not in this change
AI-assisted; verified with the tests and tools listed above.