[orchestrator-v2] fix(orchestrator): Surface Claude background wake turns as continuation runs#3752
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ordinal: projection.messages.length + 1, | ||
| }); | ||
| const commandId = CommandId.make(`provider-continuation:${messageId}`); | ||
| yield* threads.dispatch({ |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderContinuationService.ts:42
dispatchContinuation builds the message.dispatch without passing request.providerThreadId or request.driver, so the orchestrator resolves the target provider thread from the thread's current active/model state. If the app thread has switched providers or has no active provider thread when the wake arrives, the continuation run starts on the wrong (or empty) native thread and the buffered wake messages are drained from an unrelated buffer — the wake turn silently vanishes. Consider propagating providerThreadId and driver into the dispatch payload so the run targets the originating native thread.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderContinuationService.ts around line 42:
`dispatchContinuation` builds the `message.dispatch` without passing `request.providerThreadId` or `request.driver`, so the orchestrator resolves the target provider thread from the thread's current active/model state. If the app thread has switched providers or has no active provider thread when the wake arrives, the continuation run starts on the wrong (or empty) native thread and the buffered wake messages are drained from an unrelated buffer — the wake turn silently vanishes. Consider propagating `providerThreadId` and `driver` into the dispatch payload so the run targets the originating native thread.
mwolson
force-pushed
the
fix/claude-wake-turns
branch
from
7174931 to
5ebd8b9
Compare
| updated.set(key, { ...latestEntry, pinnedSinceMs }); | ||
| return updated; | ||
| }); | ||
| yield* scheduleIdleReleaseInternal(input.providerSessionId); |
There was a problem hiding this comment.
🟠 High orchestration-v2/ProviderSessionManager.ts:588
The pin-deferral path in releaseIfStillIdle deadlocks: when hasPendingBackgroundWork is true and the pin hasn't expired, it calls scheduleIdleReleaseInternal, which calls cancelIdleFiber(entry.idleFiber). That idleFiber is the fiber currently executing releaseIfStillIdle, so Fiber.interrupt interrupts the current fiber and then awaits it — a self-interrupt/self-await that never resolves. The replacement idle timer is never scheduled, so a session with pending background work stays pinned forever after the first idle timeout, bypassing maxIdlePinMs and leaking the provider process until shutdown.
The re-arm should fork the new timer first and update entry.idleFiber before cancelling the old fiber, or the cancellation should skip the current fiber.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 588:
The pin-deferral path in `releaseIfStillIdle` deadlocks: when `hasPendingBackgroundWork` is true and the pin hasn't expired, it calls `scheduleIdleReleaseInternal`, which calls `cancelIdleFiber(entry.idleFiber)`. That `idleFiber` is the fiber currently executing `releaseIfStillIdle`, so `Fiber.interrupt` interrupts the current fiber and then awaits it — a self-interrupt/self-await that never resolves. The replacement idle timer is never scheduled, so a session with pending background work stays pinned forever after the first idle timeout, bypassing `maxIdlePinMs` and leaking the provider process until shutdown.
The re-arm should fork the new timer first and update `entry.idleFiber` before cancelling the old fiber, or the cancellation should skip the current fiber.
…on runs When a background task (run_in_background Bash) settles, the Claude CLI streams a whole new turn over the still-open SDK query. handleSdkMessage dropped every message of that turn (activeTurn null), and the 30 minute idle release killed the CLI child before pending tasks could even wake. Buffer null-activeTurn messages per native thread, request one internal continuation run per wake (message.dispatch with createdBy agent / creationSource provider via a new ProviderContinuationRequests queue and worker), and drain the buffer into the continuation turn without sending a prompt to the CLI. Pin idle provider sessions while background work is pending via a new optional hasPendingBackgroundWork runtime capability, capped by maxIdlePinMs (default 4h).
mwolson
force-pushed
the
fix/claude-wake-turns
branch
from
5ebd8b9 to
6054588
Compare
There was a problem hiding this comment.
🟡 Medium
makeDefaultClaudeAdapterV2 reads ProviderContinuationRequests from the context, but since it is a Context.Reference with a no-op default (offer: () => Effect.void), any caller that provides ClaudeAdapterV2.layer without also supplying the live continuation queue captures the no-op. In that configuration Claude background wake turns are silently dropped, defeating the continuation behavior this PR introduces. Consider adding ProviderContinuationRequests to layer's environment type so the live queue must be provided, or falling back to the live implementation inside the adapter when the default is detected.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts around line 3848:
`makeDefaultClaudeAdapterV2` reads `ProviderContinuationRequests` from the context, but since it is a `Context.Reference` with a no-op default (`offer: () => Effect.void`), any caller that provides `ClaudeAdapterV2.layer` without also supplying the live continuation queue captures the no-op. In that configuration Claude background wake turns are silently dropped, defeating the continuation behavior this PR introduces. Consider adding `ProviderContinuationRequests` to `layer`'s environment type so the live queue must be provided, or falling back to the live implementation inside the adapter when the default is detected.
There was a problem hiding this comment.
🟡 Medium
When hasPendingBackgroundWork returns true and the pin cap has not expired, releaseIfStillIdle calls scheduleIdleReleaseInternal, which sleeps for the full idleTimeoutMs before checking again. With idleTimeoutMs=30m and maxIdlePinMs=5m, the session can stay pinned up to ~60 minutes instead of the intended ~35-minute ceiling — the maxIdlePinMs cap is only evaluated once per idle interval rather than as a hard deadline. Consider scheduling the re-check with min(idleTimeoutMs, maxIdlePinMs - elapsed) so the pin cap acts as a true upper bound on deferral time.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 630:
When `hasPendingBackgroundWork` returns true and the pin cap has not expired, `releaseIfStillIdle` calls `scheduleIdleReleaseInternal`, which sleeps for the full `idleTimeoutMs` before checking again. With `idleTimeoutMs=30m` and `maxIdlePinMs=5m`, the session can stay pinned up to ~60 minutes instead of the intended ~35-minute ceiling — the `maxIdlePinMs` cap is only evaluated once per idle interval rather than as a hard deadline. Consider scheduling the re-check with `min(idleTimeoutMs, maxIdlePinMs - elapsed)` so the pin cap acts as a true upper bound on deferral time.
| const hasPendingWork = | ||
| entry.runtime.hasPendingBackgroundWork === undefined | ||
| ? false | ||
| : yield* entry.runtime.hasPendingBackgroundWork.pipe( |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderSessionManager.ts:568
releaseIfStillIdle swallows all failures from entry.runtime.hasPendingBackgroundWork with Effect.catchCause(() => Effect.succeed(false)), so any adapter defect or unexpected cancellation — not just the benign idle-fiber interrupt race — causes the manager to treat pending background work as absent and proceed to releaseEntry, closing the session and killing the very background task this feature is meant to preserve. Consider narrowing the catch to only the interrupt-race cause (or distinguishing Interrupt from other failures) so genuine probe failures do not trigger premature session release.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 568:
`releaseIfStillIdle` swallows all failures from `entry.runtime.hasPendingBackgroundWork` with `Effect.catchCause(() => Effect.succeed(false))`, so any adapter defect or unexpected cancellation — not just the benign idle-fiber interrupt race — causes the manager to treat pending background work as absent and proceed to `releaseEntry`, closing the session and killing the very background task this feature is meant to preserve. Consider narrowing the catch to only the interrupt-race cause (or distinguishing `Interrupt` from other failures) so genuine probe failures do not trigger premature session release.
1 participant
Summary
Background tasks started inside a Claude turn (
run_in_backgroundBash)currently vanish from t3code when they settle: the CLI's post-settle wake
turn is silently dropped, so the displayed thread diverges from the real
native transcript, and the 30 minute idle release kills the CLI (and the
pending task) before it can even wake. This PR ingests wake turns as
first-class continuation runs and keeps the provider session pinned while
background work is pending.
Stacked on #3750 (
fix/claude-session-resume); one commit onfix/claude-wake-turnsabove it.Problem and Fix
ClaudeAdapterV2.handleSdkMessageearly-returns whenactiveTurnis null, so every message of the wake turn is dropped.activeTurnmessages now buffer per native thread when they are wake evidence (assistant/user/result, or a task notification for a tracked pending task, with or without a summary). A pending-task notification or result offers one continuation request (deduped per wake) to a newProviderContinuationRequestsqueue; a worker (ProviderContinuationService) dispatches an internalmessage.dispatch(createdBy: "agent",creationSource: "provider",queue_after_active), so the wake flows through the normal run pipeline, starting immediately on an idle thread and queueing behind any active run. The continuation turn sends no prompt to the CLI and instead drains the buffered wake messages into itself (results replayed last; a result finalizes the turn).ProviderSessionManageridle-releases the provider session after 30 minutes. Closing the SDK query kills the CLI child, and any pending background task dies with it (observed: a task notification flushed to the transcript 13ms afterquery.close, with no response).hasPendingBackgroundWorkruntime capability (pendinglocal_bashtasks, non-empty wake buffer, or outstanding continuation request).releaseIfStillIdledefers theidle_timeoutrelease while it reports true, re-arming the timer, with cumulative deferral capped by a newmaxIdlePinMsoption (default 4h) so a hung task cannot pin a session forever. Other release reasons are unchanged.Design notes:
hasPendingBackgroundWorkyields to the adapter between the idle check and the release, so
releaseEntrynow takesonlyIfIdleGenerationand revalidatesbusyCount/idleGeneration inside its atomic entry removal, making a release
of a session that turned busy during the check structurally impossible
(the existing interrupt-on-activity path already covered every
deterministic interleaving; this closes the preemption-timing residue).
continuation queue behind the user run (
queue_after_active; Claude setssupportsQueuedMessages); the user turn leaves the wake buffer intact andthe continuation drains it afterwards with correct attribution. A
continuation turn that finds an empty buffer settles immediately as a
completed run with no items.
local_bashtasks no longer render as subagent nodes; theirlifetime is tracked from
task_startedto terminaltask_notificationin a session-scoped registry (the durable cross-turn version of the
previous per-turn
ignoredTaskIds).creationSource: "provider"just produces a continuation turn thatattaches to an empty buffer and settles instantly.
continuation sink are optional with no-op defaults.
Validation
vp check: passvp run typecheck: pass (full monorepo)vp test apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts:19/19, including 5 new wake-turn tests (buffer + single deduped
continuation request with correct routing and detail; drain into a
continuation turn with no CLI prompt and the wake result surfacing as
assistant output; racing user turn leaving the buffer for the queued
continuation; null-summary notification still clearing the pending-task
registry; spurious continuation settling immediately)
all Medium findings addressed (null-summary notifications, queued-run
race, user-drain attribution), remainder documented below
vp test apps/server/src/orchestration-v2/ProviderSessionManager.test.ts:18/18, including 3 new tests (idle release deferred while background work
is pending, pinned session released once
maxIdlePinMsexpires, and aturn starting during the pending-work check never losing the session)
ClaudeReplayFixtures.integration.test.tsandOrchestratorReplayFixtures.integration.test.ts(claude fixtures): passKnown limitations
the server and the wake is lost (out of scope; nothing the orchestrator
can do server-side).
active turn, exactly as today (the over-settle family tracked around
[orchestrator-v2] fix(orchestrator): Harden Grok v2 settlement and steer message visibility #3578); this PR only changes the previously-dropped null-
activeTurnpath.
today and that wake is lost (same outcome as before, minus up to 4 hours).
dispatch failure) is logged but not retried; the wake buffer then sits
until a later turn drains it or the pin cap releases the session.
hasPendingBackgroundWorkchecks the pending-task registry and wakebuffer but not the outstanding-continuation flag; the dispatch window is
covered by the run's own busy pin.
dedicated unit tests; its behavior is covered indirectly by the adapter
tests plus the trivial dispatch body.