fix(server): keep turn interrupts responsive - #6531
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| ) { | ||
| return yield* worker.enqueue(event); | ||
| return yield* event.type === "thread.turn-interrupt-requested" | ||
| ? interruptWorker.enqueue(event) |
There was a problem hiding this comment.
🟠 High Layers/ProviderCommandReactor.ts:1418
A thread.turn-interrupt-requested can be processed before the same thread’s queued thread.turn-start-requested finishes, so providerService.interruptTurn runs before the provider session or turn exists; its failure is only logged, and the later turn starts without ever being interrupted. Coordinate the two lanes per thread while allowing interrupts for unrelated threads to bypass blocked work.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 1418:
A `thread.turn-interrupt-requested` can be processed before the same thread’s queued `thread.turn-start-requested` finishes, so `providerService.interruptTurn` runs before the provider session or turn exists; its failure is only logged, and the later turn starts without ever being interrupted. Coordinate the two lanes per thread while allowing interrupts for unrelated threads to bypass blocked work.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Two High-severity findings identify race conditions where interrupts can process before turns start or where stale aborts can cancel subsequent turns. The changes also introduce new concurrency patterns in provider session management that warrant careful review. You can customize Macroscope's approvability policy. Learn more. |
9276f63 to
c9d0bcb
Compare
| context.activeTurnId = undefined; | ||
| context.activeAgent = undefined; | ||
| context.activeVariant = undefined; | ||
| yield* updateProviderSession(context, { status: "ready" }, { clearActiveTurnId: true }); |
There was a problem hiding this comment.
🟠 High Layers/OpenCodeAdapter.ts:1595
interruptTurn marks the session ready and clears activeTurnId before session.abort settles, so a new sendTurn can start on the same session while the old abort is pending; that abort can cancel the new turn. The later MessageAbortedError then reads the newer context.activeTurnId, clears it, and emits turn.aborted for the wrong turn. Prevent new prompts until the abort settles without delaying the local lifecycle update, or correlate the abort and error with the interrupted turn before mutating session state.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OpenCodeAdapter.ts around line 1595:
`interruptTurn` marks the session `ready` and clears `activeTurnId` before `session.abort` settles, so a new `sendTurn` can start on the same session while the old abort is pending; that abort can cancel the new turn. The later `MessageAbortedError` then reads the newer `context.activeTurnId`, clears it, and emits `turn.aborted` for the wrong turn. Prevent new prompts until the abort settles without delaying the local lifecycle update, or correlate the abort and error with the interrupted turn before mutating session state.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c9d0bcb. Configure here.
| Effect.mapError(toRequestError), | ||
| Effect.ignore({ log: true }), | ||
| Effect.forkIn(context.sessionScope), | ||
| ); |
There was a problem hiding this comment.
Stale abort can cancel later turns
High Severity
interruptTurn now returns after a local turn.aborted and forks session.abort without binding it to that turn. A follow-up send can start while the abort is still in flight, so the remote abort or a later MessageAbortedError can cancel or settle the new turn instead of the stopped one.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c9d0bcb. Configure here.
| : event.type === "turn.completed" || event.type === "session.exited" | ||
| : event.type === "turn.completed" || | ||
| event.type === "turn.aborted" || | ||
| event.type === "session.exited" |
There was a problem hiding this comment.
Failed prompts can look like interrupts
Medium Severity
Ingestion now maps every turn.aborted to session ready with lastError cleared. OpenCode already emits turn.aborted when a fresh promptAsync fails, so that failure can overwrite recoverTurnStartFailure and hide the error in orchestration session state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c9d0bcb. Configure here.
Summary
Validation
Note
Medium Risk
Changes orchestration turn lifecycle and OpenCode interrupt ordering; behavior is covered by new reactor, ingestion, and adapter tests but affects user-visible Stop semantics.
Overview
Stop no longer waits behind long-running provider work on the general command reactor queue:
thread.turn-interrupt-requestedevents go to a dedicated interrupt worker, so a blocked session start on another thread cannot delay interrupt handling.For OpenCode,
interruptTurnnow publishesturn.abortedand marks the session ready locally before callingsession.abort. The remote abort runs best-effort in the background (errors ignored), so hung provider retries cannot block the UI transition.MessageAbortedErrorfromsession.erroris treated as expected user cancellation (ready session,turn.aborted) instead of a provider error.Runtime ingestion applies
turn.abortedlike a turn end: session returns to ready, active turn is cleared, and assistant messages are finalized the same path as completion.Reviewed by Cursor Bugbot for commit c9d0bcb. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix turn interrupts to process on a dedicated worker independently of blocked provider operations
thread.turn-interrupt-requestedevents to a new dedicatedinterruptWorkerinProviderCommandReactor, preventing interrupts from queuing behind long-running start/stop/recovery tasks.interruptTurninOpenCodeAdapterto immediately settle the local session and emitturn.abortedbefore calling the remote abort, so the interrupt is responsive regardless of remote failures.session.errorwithMessageAbortedErroras a successful interrupt: sets session status toready, clearslastError, and emitsturn.abortedinstead of transitioning to an error state.turn.abortedinProviderRuntimeIngestionby clearingactiveTurnId, setting thread session toready, and finalizing any assistant messages for the turn.interruptTurnno longer receive abort errors.Macroscope summarized c9d0bcb.