From afec28191bee69230eb274e550cc24dc99c06f9e Mon Sep 17 00:00:00 2001 From: Anton Timmermans Date: Tue, 6 Oct 2026 15:42:32 +0200 Subject: [PATCH] fix(server): driver-only delegation re-checks a stale provider before refusing it #16219 re-probes a provider before refusing a delegation, but only when the target names an instance or inherits the parent's. A target that names just a driver (`{ driverKind }`) still read the cached snapshot, so one startup probe timeout kept refusing it until a client came to the foreground. Re-probe each enabled, installed instance of that driver once, then resolve again. Co-Authored-By: Claude Opus 5.5 --- .../src/mcp/OrchestratorMcpService.test.ts | 106 ++++++++++++++++++ apps/server/src/mcp/OrchestratorMcpService.ts | 30 +++-- 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.test.ts b/apps/server/src/mcp/OrchestratorMcpService.test.ts index 1b68f710d6df..23b13e0e9786 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.test.ts @@ -1710,6 +1710,112 @@ describe("OrchestratorMcpService provider resolution", () => { }), ); + it.effect("re-probes a driver-only target's instances once before refusing it", () => + Effect.gen(function* () { + const claudeInstanceId = ProviderInstanceId.make("claudeAgent"); + const claudeDriver = ProviderDriverKind.make("claudeAgent"); + const task = { + id: taskId, + threadId: parentThreadId, + runId: parentRunId, + parentNodeId, + origin: "app_owned", + createdBy: "agent", + driver: claudeDriver, + providerInstanceId: claudeInstanceId, + providerThreadId: null, + childThreadId, + nativeTaskRef: null, + prompt: "Review the diff.", + title: null, + model: "claude-opus-5-5", + status: "running", + result: null, + startedAt: null, + completedAt: null, + }; + const healthy = providerSnapshot({ + instanceId: claudeInstanceId, + driver: claudeDriver, + model: "claude-opus-5-5", + }); + const timedOut: ServerProvider = { + ...healthy, + status: "error", + message: + "Claude Agent CLI is installed but failed to run. Timed out while running command.", + }; + const codex = providerSnapshot({ + instanceId: codexInstanceId, + driver: ProviderDriverKind.make("codex"), + model: "gpt-5.4", + }); + // The cache still holds a startup probe timeout; a re-probe reports `probeSucceeds`. + let probeSucceeds = false; + const probed = yield* Ref.make>([]); + const dispatched = yield* Ref.make(0); + const layerDependencies = Layer.mergeAll( + NodeServices.layer, + Layer.mock(ThreadManagementService.ThreadManagementService)({ + hasReceipt: () => Effect.succeed(false), + isProviderInstanceEnabledForProject: () => Effect.succeed(true), + assertProviderInstanceEnabled: () => Effect.void, + getThreadRecords: (threadId) => + Effect.succeed( + threadId === parentThreadId ? parentProjection([task]) : childProjection, + ), + dispatch: () => + Ref.update(dispatched, (count) => count + 1).pipe( + Effect.as({ + sequence: 1, + storedEvents: [ + { + sequence: 1, + commandId: null, + event: { type: "subagent.updated", payload: task }, + }, + ], + } as never), + ), + }), + ServerSettings.layerTest(), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + getProviders: Effect.succeed([codex, timedOut]), + refreshInstance: (instanceId) => + Ref.update(probed, (ids) => [...ids, instanceId]).pipe( + Effect.as([codex, probeSucceeds ? healthy : timedOut]), + ), + }), + adapterRegistryLayer([codexInstanceId, claudeInstanceId]), + Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), + ); + + yield* Effect.gen(function* () { + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + const delegate = (clientRequestId: string) => + service.delegateTask(scope, { + task: "Review the diff.", + target: { driverKind: claudeDriver }, + mode: "async", + clientRequestId, + }); + + const error = yield* delegate("delegate-driver-recheck-1").pipe(Effect.flip); + assert.equal(error.code, "provider_unavailable"); + assert.deepStrictEqual(yield* Ref.get(probed), [claudeInstanceId]); + assert.equal(yield* Ref.get(dispatched), 0); + + probeSucceeds = true; + const result = yield* delegate("delegate-driver-recheck-2"); + assert.equal(result.providerInstanceId, claudeInstanceId); + assert.deepStrictEqual(yield* Ref.get(probed), [claudeInstanceId, claudeInstanceId]); + assert.equal(yield* Ref.get(dispatched), 1); + }).pipe(Effect.provide(OrchestratorMcpService.layer.pipe(Layer.provide(layerDependencies)))); + }), + ); + it.effect( "inherits an available parent instance for driver-only targets and otherwise selects a healthy peer", () => diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index f9bb61fa810e..5afe6a419f7c 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -1135,7 +1135,8 @@ const make = Effect.gen(function* () { /** * Provider snapshots only re-probe while a client is in the foreground, so an * unattended agent can see a provider as unavailable after it was fixed. - * Re-probe the requested instance once before refusing it. + * Re-probe the requested instance (or, for a driver-only target, each enabled + * and installed instance of that driver) once before refusing it. */ const assertTargetEnabledForProject = ( parent: Pick, @@ -1151,20 +1152,31 @@ const make = Effect.gen(function* () { .pipe(Effect.mapError(threadManagementFailure)); const resolveTargetRechecking = (input: Parameters[0]) => { - const instanceId = + const requestedDriver = input.target?.driverKind; + const requestedInstanceId = input.target?.providerInstanceId ?? - (input.target?.driverKind === undefined - ? input.parent.thread.modelSelection.instanceId - : undefined); + (requestedDriver === undefined ? input.parent.thread.modelSelection.instanceId : undefined); + const instanceIds = + requestedInstanceId === undefined + ? input.providers + .filter( + (provider) => + provider.driver === requestedDriver && provider.enabled && provider.installed, + ) + .map((provider) => provider.instanceId) + : [requestedInstanceId]; const resolved = resolveTarget(input); - if (instanceId === undefined) return resolved; + if (instanceIds.length === 0) return resolved; return resolved.pipe( Effect.catchIf( (error) => error.code === "provider_unavailable", () => - providerRegistry - .refreshInstance(instanceId) - .pipe(Effect.flatMap((providers) => resolveTarget({ ...input, providers }))), + // Each refresh returns the whole registry, so the last one carries every re-probe. + Effect.forEach(instanceIds, providerRegistry.refreshInstance).pipe( + Effect.flatMap((snapshots) => + resolveTarget({ ...input, providers: snapshots.at(-1) ?? input.providers }), + ), + ), ), ); };