fix(server): Stop waits for the turn to settle instead of polling #35948
main fromrefs/pull/35948/headmerged by juliusmarminge View on GitHubProblem. On Stop, ProviderTurnControlService.interrupt sent the interrupt, then polled the projection every 10 ms on the forced real clock, for up to 2 s, until the provider turn stopped running. The poll never checked the run attempt, so Stop could return before the provider's turn.terminal was ingested and RunExecutionService.finalizeRootRun committed run-attempt.updated. The thread.background-work.settle follow-up then repaired the run first. When a usage limit lands just as Stop arrives (TaskCancelUsageLimit), the repair wins and the run never becomes failed.
Fix. Stop reads the latest thread sequence, checks the projection, and follows EventSink.stream({ threadId, afterSequence }). It re-reads on provider-turn.updated / run-attempt.updated until both the turn and its attempt stop running. The wait is bounded with Effect.timeoutOption("2 seconds") on the normal Clock, with no polling and no forced wall clock. EventSink is wired into runtimeLayer.ts and the replay harness.
Two SteeringCompletion fakes needed to match real adapters:
- The
nextTurnSelectionHarnessfake now ends an interrupted turn withturn.terminal, as every real adapter does (audit below). Before, the attempt only settled in the follow-up, so Stop waited out its bound. startFirstTurnraced RunExecution's root-turn ownership check: the test rewrote the run topreparingbefore the check ran. On main that check's "started after Stop" cleanup interrupts the turn, which was harmless only because the old fake never terminated it.startFirstTurnnow takes the thread command lock once, so it continues only after the check. The replay harness exposesThreadCommandExecutorfor this.
Adapter audit. After T3 interrupts a running turn, does the adapter emit the turn.terminal that leads finalizeRootRun to settle the attempt?
| Adapter | Running turn | Already finished turn | Evidence |
|---|---|---|---|
| Codex | yes: turn/interrupt, then turn/completed (interrupted) → emitOrDeferRootTerminal | not active → returns (restart) or a protocol error; projection already settled, so Stop does not wait | CodexAdapterV2.ts ~6746, interruptTurn ~7847; fixtures turn_interrupt, turn_interrupt_mid_tool |
| Claude | yes: query.interrupt + close; query close emits terminal interrupted | no active turn → early return | ClaudeAdapterV2.ts ~9104, ~12687; fixtures turn_interrupt, turn_interrupt_mid_tool, turn_interrupt_restart, claude_background_task_interrupt |
| ACP generic (Grok, Kiro, Antigravity, ACP registry) | yes: finalizeTurn(context, "interrupted") on every branch (session close, hard restart, settled soft), or on prompt return after cancel; then awaits context.completed | no active turn → "already interrupted" early return | provider-acp/src/server/adapter.ts finalizeTurn ~7756 (emits at ~7903), interrupt ~9045/9102/9284; fixtures turn_interrupt (grok/kiro/registry), kiro_turn_interrupt_mid_tool, kiro_subagent_interrupt |
| Cursor | yes: finalizeTurn({ status: "interrupted" }) | no context → return | provider-cursor/src/server/adapter.ts ~2835, terminal ~2190; fixture turn_interrupt_mid_tool |
| OpenCode v1 | yes: session.abort, then the session idle/error path emits terminal | no turn → return | provider-opencode/src/server/adapter.ts ~3505, terminal ~2115; fixture turn_interrupt |
| OpenCode v2 | yes: finishTurn(state, { status: "interrupted" }) | not active → return | provider-opencode/src/server/v2/adapter.ts ~4911, terminal ~2276; fixture opencode2_interrupt |
| Pi | yes: abort → terminal | not active → return / protocol error | provider-pi/src/server/adapter.ts ~3084, terminal ~1816; fixture turn_interrupt_mid_tool |
All 10 *ReplayFixtures.integration.test.ts files pass with this change: 172 tests. They run on TestClock, so a Stop that never saw a terminal would have hung at the bound instead of passing.
Before/after
SteeringCompletion.integration.test.ts: with the Stop change alone, 13 tests hung to the 120 s test timeout. With the fixed fakes, all 60 pass in 45 s.ProjectionControlReads.test.ts: the new case fails (hangs) on main's polling code and passes with the fix.
Verification (server tests run sandboxed in a PID namespace)
- Replay fixtures: Codex/Claude/Cursor/Pi 83 ✓; AcpRegistry/Grok/Kiro/OpenCode/Fx/Muse 89 ✓
- SelectionRestart, ThreadStop, BackgroundWorkStop, ProjectionControlReads, ProviderTurnControlService: 69 ✓
- TaskCancelUsageLimit, ProviderSwitch, OrchestratorMcpService (+activity): 139 ✓
- DelegatedCompletionDelivery, Orchestrator.control-reads, RestartContinuation, ThreadFork.execution, runtimeLayer: 315 ✓
- EffectWorker, RunExecutionService, ProviderTurnStartService, ThreadSettlementService, ThreadManagementService: 199 ✓
- ProviderSessionManager, ProviderRuntimeRecoveryService, ProviderCommitBatch, LegacyV1Cutover: 106 ✓
vp run check:changed: format, lint, knip, typecheck, 7 changed test files (242 tests) ✓
Done by Claude Opus 5.5 in Claude Code.
🤖 Generated with Claude Code
github-actions[bot](GitHub profile, opens in a new tab) added labelsvouch:trusted
github-actions[bot](GitHub profile, opens in a new tab) added labelssize:M
Approvability
Verdict: Approved at
e97f2e5Macroscope's review found this PR approvable — Stop now waits on thread events for the provider turn and its attempt to settle instead of polling. The existing two-second bound remains, keeping this coordination change limited in scope.
Notes:
- All code in this push has already been reviewed. Approvability was decided on eligibility alone.
macroscopeapp[bot](GitHub profile, opens in a new tab) approved these changes
No actionable comments were generated in the recent review.
Walkthrough
-
ProviderTurnControlService now waits for provider-turn and run-attempt updates after interrupting a running turn. The wait ends when both are settled or after two seconds. Runtime layers provide EventSinkV2, and tests cover timeout and settlement paths.
Changes: Provider turn stop settlement
Layer / File(s) Summary Event-driven settlement
apps/server/src/orchestration-v2/ProviderTurnControlService.ts,apps/server/src/orchestration-v2/runtimeLayer.ts,apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.tsProviderTurnControlService waits for provider-turn and run-attempt updates to confirm settlement, with a two-second timeout. Runtime and replay layers provide EventSinkV2. The replay runtime also provides ThreadCommandExecutor. Settlement and interruption tests
apps/server/src/orchestration-v2/ProjectionControlReads.test.ts,apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts,apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.ts,apps/server/src/orchestration-v2/SteeringCompletion.integration.test.tsTests cover timeout and confirmed settlement. Integration tests synchronize with provider interrupts, emit terminal interruption events, and wait for the thread command lock. Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram participant ProviderTurnControlService participant ProviderSessionManager participant ProjectionStore participant EventSinkV2 ProviderTurnControlService->>ProviderSessionManager: interrupt running turn ProviderTurnControlService->>ProjectionStore: check turn and attempt state ProviderTurnControlService->>EventSinkV2: watch turn and attempt updates EventSinkV2-->>ProviderTurnControlService: deliver update events ProviderTurnControlService->>ProjectionStore: check for settlementPossibly related issues
- pingdotgg/t3code#30985 — The issue describes Stop completing after an acknowledged interrupt that does not end the turn. This PR changes Stop to wait for settlement and bounds that wait.
Suggested labels:
bugfixSuggested reviewers:
maria-rcks,yash-singh1,fireboltdude1357
Merge Risk:
e97f2-
Stop now waits for the provider turn and its run attempt to settle through events instead of polling, with a two-second fallback. No concrete defect was established in the changed code. The PR author says the real-app Claude delegated-child Stop race could not be run, but that is a normal validation gap and not an identified risk.
Pre-merge checks:
-
Check name Status Explanation Title check 
The title names the main change: Stop now waits for the turn to settle, replacing the 10 ms polling loop. It follows the conventional commit format and matches the objectives and the changed ProviderT… Description check The description follows the template's Problem, Change, Scope, and Verification sections. The Problem section explains the race and the failing run-attempt repair. The Change section explains the Even… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the main change: Stop now waits for the turn to settle, replacing the 10 ms polling loop. It follows the conventional commit format and matches the objectives and the changed ProviderTurnControlService.ts. It is specific enough for a teammate scanning history.
Full details: Description check
-
Explanation
The description follows the template's Problem, Change, Scope, and Verification sections. The Problem section explains the race and the failing run-attempt repair. The Change section explains the EventSink-based settle wait and the wiring changes. The Verification section lists focused test groups, observed counts, and an adapter audit. Scope is weak: it does not link an issue or say why no issue is needed, and it does not say whether a maintainer requested the PR. The objectives also say the delegated-child Stop race and the real-app behavior remain unverified, and the description does not state that limitation. These gaps are non-critical, so the description is mostly complete.
-
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
-
-
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: fix/turn-stop-await-settle-v2
- File: .coderabbit.config.ts
- Commit: e97f2e5
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Team
- Run ID:
72c925bc-9f28-4fd1-b413-bb08d81cc24d
-
Reviewing files that changed from the base of the PR and between 6b60a1d270521682826a3560c7791b284b3df885 and e97f2e5932c7592a7b9e1afe171ceba484aac027.
-
apps/server/src/orchestration-v2/BackgroundWorkStop.integration.test.tsapps/server/src/orchestration-v2/SteeringCompletion.integration.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
-
-
juliusmarminge added labelstakeover
- juliusmarmingecommented
The automated pipeline stopped on this pull request and will not merge it:
the fix is written and reviewed, but proving it needs something no worker has:
- Permanent blocker: Claude’s native usage-limit flow requires first-party Claude OAuth access, explicitly unavailable on every VM. Could not run the delegated-child Stop race on either commit.
- Preservation of the failed outcome, captured-attempt settlement, and the two-second fallback bound remain unverified in the real app. No captures were produced.
It stays open for a person to finish or close.
macroscopeapp[bot](GitHub profile, opens in a new tab) approved these changes
juliusmarminge added labelstakeover:in-progress
juliusmarminge merged this
15d1ef5juliusmarminge closed this
Sign in to join this conversation.
Thread transfer impact#
✅ Thread transfer remains within every enforced ceiling.
Other scenarios#
All 64 scenario measurements match
main.All 64 scenario measurements
short turn resumeWebSocket decodedshort turn resumeWebSocket messageslong turn running, cold openHTTP decodedlong turn running, cold openrowslong turn running, history pageHTTP decodedlong turn running, history pagerowslong turn finished, cold openHTTP decodedlong turn finished, cold openrowslong turn finished, socket cold openWebSocket decodedlong turn finished, socket cold openWebSocket messageslong turn finished, socket cold openrowslong turn finished, work pageHTTP decodedlong turn finished, work pagerowsidle thread resumeWebSocket decodedidle thread resumeWebSocket messagesfleet shell snapshotHTTP decodedfleet shell snapshotrowsfleet shell subscribeWebSocket decodedfleet shell subscribeWebSocket messagesfleet shell resume checkHTTP decodedfleet shell resume checkrowsstale shell reloadHTTP decodedstale shell reloadrowsstale shell resumeWebSocket decodedstale shell resumeWebSocket messagesstale todo list cold openHTTP decodedstale todo list cold openrowsstale todo list cold opennodesstale todo list cold opensubagentsstale todo list cold openplansstale todo list cold openproviderTurnsstale todo list cold opencheckpointsshort turn resumeWebSocket decodedshort turn resumeWebSocket messageslong turn running, cold openHTTP decodedlong turn running, cold openrowslong turn running, history pageHTTP decodedlong turn running, history pagerowslong turn finished, cold openHTTP decodedlong turn finished, cold openrowslong turn finished, socket cold openWebSocket decodedlong turn finished, socket cold openWebSocket messageslong turn finished, socket cold openrowslong turn finished, work pageHTTP decodedlong turn finished, work pagerowsidle thread resumeWebSocket decodedidle thread resumeWebSocket messagesfleet shell snapshotHTTP decodedfleet shell snapshotrowsfleet shell subscribeWebSocket decodedfleet shell subscribeWebSocket messagesfleet shell resume checkHTTP decodedfleet shell resume checkrowsstale shell reloadHTTP decodedstale shell reloadrowsstale shell resumeWebSocket decodedstale shell resumeWebSocket messagesstale todo list cold openHTTP decodedstale todo list cold openrowsstale todo list cold opennodesstale todo list cold opensubagentsstale todo list cold openplansstale todo list cold openproviderTurnsstale todo list cold opencheckpointsBaseline:
ee64555· PR result:e97f2e5· Source CI: successScenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.