CL-6644: replace per-hop wake/mail bounds with one turn-level dispatch deadline - #321
Merged
Conversation
Covers the structural fix the ticket's last comment mandates: a never-settling dispatchTurn internals must post an undelivered notice within the injected budget and release the workbench's turn claim rather than wedging it for the claim TTL, while a dispatch that settles inside the budget (however slowly) must never be killed.
…er per-hop bound Three rounds of per-hop timeouts (#312's wake bound, #314's bypassed- wake bound, #316's mail-delivery bound) each closed one stalling hop and a fourth kept appearing -- most recently a direct send to an already-live run that needed neither #312's nor #316's bound and still hung silently with no notice posted. dispatchTurnBatch now wraps each recipient's dispatchTurn call in one wall-clock deadline (DEFAULT_TURN_DISPATCH_TIMEOUT_MS, 120s, injectable via SendWorkbenchMessageDeps.turnDispatchTimeoutMs) so no agent turn may hang past its budget regardless of which internal hop stalls. A timeout rejects with a message naming the turn's run address and its elapsed budget, which flows into dispatchTurnBatch's existing catch (#313): a reportError refId is logged and an undelivered notice is posted. The workbench's turn claim releases in createWorkbenchTurnQueue's existing finally block the moment dispatchTurnBatch's per-recipient Promise.all settles, so a timed-out turn never wedges the room for the claim TTL. dispatchTurn's own promise only ever covers "the mail was handed to the agent's mailbox" (see turn-queue.ts's note) -- the agent's actual streaming reply is produced and posted onto the timeline later, off this call stack, through chat-orchestrator.ts's independent sidecar- event subscription. The deadline therefore can never cut off a reply in progress: nothing it awaits is the reply itself. Lifted the withTimeout helper #316 wrote inline in platform-adapter.ts into a shared with-timeout.ts so the wake bound, the mail-delivery bound, and this new turn-level deadline share one implementation. Per-hop bounds (#312/#314/#316) stay in place: they still produce a sharper cause when they fire first, and the turn-level deadline is only the backstop for whatever they don't yet cover. Not fixed here, and not this PR's job: why a direct send to a live, registered run stalls at all -- that root cause continues under CL-6648. This deadline makes the stall loud instead of silent; the reportError refId will name it.
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
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.
Closes the structural ask in CL-6644's last comment: three rounds of per-hop timeouts (#312's wake bound, #314's bypassed-wake bound, #316's mail-delivery bound) each closed one stalling hop and a fourth kept appearing -- most recently a direct send to an already-live run that needed neither #312's nor #316's bound and still hung silently with zero log lines and no notice posted.
What changed
dispatchTurnBatch(packages/chat/src/workbench-service.ts) now wraps each recipient'sdispatchTurncall in one wall-clock deadline:DEFAULT_TURN_DISPATCH_TIMEOUT_MS(120s), injectable viaSendWorkbenchMessageDeps.turnDispatchTimeoutMs/CreateChatRoutesDeps.turnDispatchTimeoutMs, following the existingdepsinjection pattern. No agent turn may hang past this budget regardless of which internal hop stalls.On expiry the rejection names the turn's run address and its elapsed budget (
turnDispatchTimeoutMessage), and flows intodispatchTurnBatch's existing catch (#313): areportErrorrefId is logged and the existing undelivered notice is posted.Per-hop bounds (#312/#314/#316) are left in place -- they still produce a sharper, more specific cause when they fire first. This deadline is only the backstop for whatever a per-hop bound doesn't yet cover.
Design points
dispatchTurn's own promise only ever covers "the mail was handed to the agent's mailbox" (agentTurns.startTurn+platform.sendMail) -- see the existing note inturn-queue.ts. The agent's actual streaming reply is produced and posted onto the timeline later, off this call stack entirely, throughchat-orchestrator.ts's independent sidecaragent.eventsubscription. So the deadline structurally cannot cut off a reply in progress: nothing it awaits is the reply itself. No "reset on first token" logic was needed because there is no token stream on this call path to begin with. Verified by reading the vendoredsendUserMessageimplementation (vendor/intx/hub-sessions/src/session-service.ts) -- it hands mail to the sidecar and returns; it never waits on inference.createWorkbenchTurnQueue'srun(turn-queue.ts) already releases the claim in afinallyarounddispatch(batch), anddispatchTurnBatchalready catches every per-recipient failure internally so its ownPromise.allalways settles. A timed-out turn's rejection is caught inside the per-recipient loop, so the claim releases immediately -- never waiting out the claim TTL backstop.withTimeouthelper Bound post-deploy mail delivery so a stalled ack fails loud (CL-6644) #316 wrote inline inplatform-adapter.tsintopackages/chat/src/with-timeout.ts, used by the wake bound, the mail-delivery bound, and this new turn-level deadline.Tests
packages/chat/test/turn-dispatch-deadline.test.ts:dispatchTurnthat never settles posts an undelivered notice (with refId) well inside the injected budgetNot in scope
Live root-cause of the third hang variant (a direct send to a live-registered run stalling) continues under CL-6648 -- this deadline makes that stall loud instead of silent, and the
reportErrorrefId will name it for whoever picks that up next.Verification
WORKBENCH_CHECK_SINCE=origin/main bun run typecheck-- passWORKBENCH_CHECK_SINCE=origin/main bun run test-- pass (656 pass / 0 fail in@corbits/chat)bun run lint-- passNot run: live repro against the real stack (timeboxed, per the ticket's own note that this session's job is the structural close).