fix(orchestrator): defer queued provider handoff until delivery - #12226
Conversation
github-actions
Bot
added
vouch:trusted
Thread transfer impact
This comment will update automatically after the next completed run. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR substantially changes production queued-run orchestration, including cross-provider context handoff, provider-session detachment, projection semantics, and startup failure recovery. Although the new integration coverage is broad and the data changes are additive, the runtime state transitions and downstream effects warrant human review. Notes:
You can add or adjust custom eligibility rules. Learn more. |
715f5d1 to
ff8e024
Compare
github-actions
Bot
added
size:XL
| }, | ||
| ] | ||
| : []; | ||
| const sessionsToDetach = projection.providerSessions.filter( |
There was a problem hiding this comment.
This new queued-switch session-detachment path has no focused test that verifies the emitted provider-session.detached event and matching provider-session.detach outbox request. The existing switch tests only assert releaseProviderSessionIds at the planning layer, so a wiring regression here would be undetected. Add a queued provider-switch integration test that asserts live sessions selected by the plan are detached and stopped/error sessions are excluded.
No diff: the fix belongs in a test file.
Posted via Macroscope — Effect Service Conventions
This comment has been minimized.
This comment has been minimized.
| ], | ||
| ); | ||
| }); | ||
| }).pipe(Effect.catch((cause) => failQueuedRunStart(threadId, cause))); |
There was a problem hiding this comment.
This new queued-start recovery behavior has no focused test: existing queue tests cover successful promotion and cancellation, but none force startup failure or verify the attempt, node, error turn item, and run are terminalized (including the context_handoff_unsupported mapping). Add an integration test using a failing test layer that exercises this catch path and asserts those persisted events.
No diff: the fix belongs in a test file.
Posted via Macroscope — Effect Service Conventions
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Effect Service Conventions found one blocking issue: the new queued-start failure recovery path lacks focused integration coverage for its terminal state updates and failure-code mapping. See the inline review comment for the required test coverage. Posted via Macroscope — Effect Service Conventions |
| completedAt: now, | ||
| updatedAt: now, | ||
| }; | ||
| const handoffTurnItem: OrchestrationV2TurnItem | null = |
There was a problem hiding this comment.
This newly added queued-handoff timeline behavior has no focused test. Existing queue-promotion tests do not assert the persisted handoff turn item, its context/summary fields, or its ordinal * 100 - 1 placement. Add a queued provider-switch integration test using the existing test layers that verifies this item is emitted and projected correctly.
No diff: the fix belongs in a test file.
Posted via Macroscope — Effect Service Conventions
|
Effect Service Conventions found one blocking issue: the new queued-handoff timeline behavior lacks focused integration coverage for the emitted and projected handoff turn item. See the inline review comment for the required test coverage. Posted via Macroscope — Effect Service Conventions |
This comment has been minimized.
This comment has been minimized.
1 participant
Problem
Queuing a message for a different provider changed the thread provider and detached the running session before the queued message reached delivery. Earlier queued turns could be interrupted or lose context.
Fix
Keep each queued message's selected provider on its run. Change the thread provider and prepare its handoff only when that run reaches the front. Build context from the runs that actually completed, including a steered replacement turn. Preserve native continuation for compatible account changes. If the queued provider cannot start, fail that run visibly and advance the queue. Web and mobile sends pass the selected model through dispatch without a premature metadata update; mobile preserves queue mode after outbox reload.
Evidence
The provider-switch integration suite has 12 passing tests. It holds an active Codex turn, queues two Codex turns and one Claude turn, then verifies all three Codex responses reach Claude only at delivery. Another scenario steers the active turn to Grok and verifies the later Codex and Claude handoffs include the turns that finished. Focused cases cover native account resumption, imported v1 history after the first provider fails, queued-start rejection advancing to the next message, and the live session's detach event and outbox request at delivery. The mobile outbox suite has 45 passing tests, including a queue-mode storage round trip. Server, web, and mobile typechecks and changed-file lint passed. Required CI checks, including Check and all server test shards, passed on the latest head.
Relationship to #12255
Both PRs remove premature web/mobile model updates and record model selection at dispatch or queued delivery. This PR owns cross-provider queued handoffs. #12255 has separate imported-thread, handoff display, and automatic-message changes. Its overlapping dispatch and client hunks should be deduplicated when the branches are reconciled; applying both copies would duplicate selection events.
Stacked on #2829.
Model: GPT-6. Harness: Codex API.