From 371beb4166bb5d27ec95396993cc307d50deb3c9 Mon Sep 17 00:00:00 2001 From: Terry Crapts Date: Thu, 29 Oct 2026 23:42:01 +0100 Subject: [PATCH] fix(server): T3 write tools keep working after switching a thread's provider and back Sending a message with a different provider left the thread attached to the shared Codex session. Switching back skipped MCP credential setup, so Codex reused the other provider's credential and every write tool failed with parent_not_active. Cross-provider sends now detach the left provider's live sessions, like queued starts and provider.switch already do. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 41 ++++-- .../ProviderSwitch.integration.test.ts | 130 ++++++++++++++++-- 2 files changed, 146 insertions(+), 25 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index c464a8743f3c2..f8d1223b31b68 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -7157,30 +7157,47 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }); } - if (canResumeAcrossInstances && activeProviderThread.providerSessionId !== null) { - const previousProviderSessionId = activeProviderThread.providerSessionId; + const detachReason = canResumeAcrossInstances + ? "Provider account changed; continuing the native thread." + : "Provider or model selection changed."; + // Shared sessions keep a left thread attached, and with it the other provider's MCP credential. + const sessionsToDetach = !isProviderSwitch + ? [] + : canResumeAcrossInstances + ? activeProviderThread.providerSessionId === null + ? [] + : [ + { + id: activeProviderThread.providerSessionId, + driver: activeProviderThread.driver, + providerInstanceId: activeProviderThread.providerInstanceId, + }, + ] + : projection.providerSessions.filter( + (session) => + session.providerInstanceId !== modelSelection.instanceId && + session.status !== "stopped" && + session.status !== "error", + ); + for (const session of sessionsToDetach) { yield* emitEvent({ type: "provider-session.detached", threadId: command.threadId, - driver: activeProviderThread.driver, - providerInstanceId: activeProviderThread.providerInstanceId, + driver: session.driver, + providerInstanceId: session.providerInstanceId, occurredAt: now, - payload: { - providerSessionId: previousProviderSessionId, - detachedAt: now, - reason: "Provider account changed; continuing the native thread.", - }, + payload: { providerSessionId: session.id, detachedAt: now, reason: detachReason }, }); yield* Ref.update(effects, (existing) => [ ...existing, { - id: `effect:${command.commandId}:provider-session.detach:${previousProviderSessionId}`, + id: `effect:${command.commandId}:provider-session.detach:${session.id}`, commandId: command.commandId, threadId: command.threadId, request: { type: "provider-session.detach", - providerSessionId: previousProviderSessionId, - detail: "Provider account changed; continuing the native thread.", + providerSessionId: session.id, + detail: detachReason, }, } satisfies PendingOrchestrationEffectV2, ]); diff --git a/apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts b/apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts index bb21d8ca9929a..9cf12116dbfa0 100644 --- a/apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts +++ b/apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts @@ -747,19 +747,8 @@ describe("orchestration v2 provider switching", () => { ); yield* wait(3); } - if (replaceNative) { - const target = (yield* orchestrator.getThreadProjection(threadId)).providerThreads.find( - (thread) => thread.providerInstanceId === CLAUDE_MODEL_SELECTION.instanceId, - )!; - yield* orchestrator.dispatch({ - type: "provider-session.detach", - commandId: CommandId.make("detach-for-native-replacement"), - threadId, - providerSessionId: target.providerSessionId!, - }); - yield* worker.drain(); - yield* Ref.set(failResumeOnce, true); - } + // Returning to Codex detached Claude, so this turn resumes its native thread. + if (replaceNative) yield* Ref.set(failResumeOnce, true); if (scenario.includes("retry")) yield* Ref.set(failStartOnce, true); if (scenario.includes("target-background") || scenario.includes("target-subagent")) yield* Ref.set(pendingBackgroundWork, true); @@ -2520,6 +2509,121 @@ describe("orchestration v2 provider switching", () => { ), ); + // A shared Codex session that stays attached keeps the other provider's MCP credential. + it.live("detaches the left provider's session when a sent message switches providers", () => + Effect.scoped( + Effect.gen(function* () { + const cwd = yield* checkpointWorkspace("direct-provider-switch-detach"); + const capturedTurns = yield* Ref.make>([]); + const layerRegistry = ProviderAdapterRegistry.layerFromAdapters([ + makeTestAdapter({ + instanceId: CODEX_MODEL_SELECTION.instanceId, + driver: CODEX_DRIVER, + capabilities: CodexProviderCapabilitiesV2, + modelSelection: CODEX_MODEL_SELECTION, + responseByRunOrdinal: {}, + capturedTurns, + }), + makeTestAdapter({ + instanceId: CLAUDE_MODEL_SELECTION.instanceId, + driver: CLAUDE_DRIVER, + capabilities: ClaudeProviderCapabilitiesV2, + modelSelection: CLAUDE_MODEL_SELECTION, + responseByRunOrdinal: {}, + capturedTurns, + }), + ]); + const layerDatabase = SqlitePersistence.layerMemory; + const layerOutboxProvided = EffectOutbox.layer.pipe(Layer.provide(layerDatabase)); + yield* Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const worker = yield* EffectWorker.OrchestrationEffectWorkerV2; + const effectOutbox = yield* EffectOutbox.EffectOutboxV2; + const commandId = (ordinal: number) => + CommandId.make(`direct-provider-switch:${ordinal}`); + const run = Effect.fn("run")(function* (ordinal: number, selection: ModelSelection) { + yield* orchestrator.dispatch({ + type: "message.dispatch", + commandId: commandId(ordinal), + threadId, + messageId: MessageId.make(`direct-provider-switch:${ordinal}`), + createdBy: "user", + creationSource: "web", + text: `Request ${ordinal}`, + attachments: [], + modelSelection: selection, + dispatchMode: { type: "start_immediately" }, + }); + yield* orchestrator.streamStoredEvents.pipe( + Stream.filter( + ({ event }) => + event.type === "run.updated" && + event.payload.ordinal === ordinal && + event.payload.status === "completed", + ), + Stream.runHead, + ); + yield* worker.drain(); + }); + const sessionIdFor = Effect.fn("sessionIdFor")(function* (instanceId: string) { + const projection = yield* orchestrator.getThreadProjection(threadId); + return projection.providerSessions.find( + (session) => session.providerInstanceId === instanceId, + )?.id; + }); + const detachedSessionIds = Effect.fn("detachedSessionIds")(function* (ordinal: number) { + return (yield* effectOutbox.listByCommandId(commandId(ordinal))).flatMap((effect) => + effect.request.type === "provider-session.detach" + ? [effect.request.providerSessionId] + : [], + ); + }); + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make("direct-provider-switch:create"), + threadId, + projectId, + createdBy: "user", + creationSource: "web", + title: "Direct provider switch", + modelSelection: CODEX_MODEL_SELECTION, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + yield* run(1, CODEX_MODEL_SELECTION); + const codexSessionId = yield* sessionIdFor(CODEX_MODEL_SELECTION.instanceId); + yield* run(2, CLAUDE_MODEL_SELECTION); + const claudeSessionId = yield* sessionIdFor(CLAUDE_MODEL_SELECTION.instanceId); + yield* run(3, CODEX_MODEL_SELECTION); + + assert.isDefined(codexSessionId); + assert.deepEqual(yield* detachedSessionIds(2), [codexSessionId]); + assert.deepEqual(yield* detachedSessionIds(3), [claudeSessionId]); + }).pipe( + Effect.provide( + Layer.merge( + ProviderReplayHarness.layerWithRegistry( + { + name: "direct-provider-switch-detach", + runtimePolicyOverride: { + cwd, + approvalPolicy: "never", + sandboxPolicy: { type: "readOnly" }, + }, + }, + layerRegistry, + { databaseLayer: layerDatabase }, + ), + layerOutboxProvided, + ), + ), + ); + }), + ), + ); + // regression: #21314 a queued PR update must follow the provider that took over the thread it.live.each([ ["automatic", "Claude"],