diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index cd8c5064476b2..efcd5cf9adc9f 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -7,6 +7,7 @@ import { ProjectId, ProviderDriverKind, ProviderInstanceId, + RunAttemptId, RunId, ScheduledTaskId, SECRET_REQUEST_UNSAVED_NOTE, @@ -1195,3 +1196,91 @@ it("requestSecret returns the unsaved note for a declined, cancelled or timed-ou { status: "timed_out", note: SECRET_REQUEST_UNSAVED_NOTE.timed_out }, ]); }); + +it("requestSecret cancels its card when a steering restart supersedes the request", async () => { + const originalRoot = NodeId.make("node-mcp-secret-root-original"); + const steeredRoot = NodeId.make("node-mcp-secret-root-steered"); + const dispatched: Array<{ + readonly type: string; + readonly turnItemId?: TurnItemId; + readonly nodeId?: NodeId; + readonly secretStatus?: string; + }> = []; + let steered = false; + const projection = () => { + const card = dispatched.find( + (command) => command.type === "secret_request.record" && command.secretStatus === "pending", + ); + return { + thread: baseThread({ + threadId: parentThreadId, + title: "Parent", + instanceId: parentInstanceId, + model: "gpt-5.4", + }), + runs: [ + { + ...makeRun({ id: activeRunId, ordinal: 1, status: "running" }), + rootNodeId: steered ? steeredRoot : originalRoot, + activeAttemptId: RunAttemptId.make("attempt-mcp-secret-original"), + }, + ], + visibleTurnItems: [], + turnItems: + card === undefined + ? [] + : [{ type: "secret_request", id: card.turnItemId, secretStatus: "pending" }], + providerTurns: [], + runtimeRequests: [], + messages: [], + contextTransfers: [], + subagents: [], + updatedAt: now, + } as unknown as OrchestrationV2ThreadProjection; + }; + const layer = layerOrchestratorService.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: () => Effect.sync(projection), + getThreadShell: () => + Effect.sync(() => projection().thread as unknown as OrchestrationV2ThreadShell), + dispatch: (command) => + Effect.sync(() => { + dispatched.push(command as (typeof dispatched)[number]); + // The user steers right after the card opens, replacing the run's root. + if (command.type === "secret_request.record" && command.secretStatus === "pending") { + steered = true; + } + return undefined as never; + }), + } satisfies Partial), + ServerSettings.layerTest(), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + getProviders: Effect.succeed([]), + } satisfies Partial), + Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), + Layer.mock(RemoteDelegation.RemoteDelegation)({}), + Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({}), + NodeCrypto.layer, + ), + ), + ); + + await Effect.gen(function* () { + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + const result = yield* service.requestSecret(makeScope(), { + label: "Booking details", + reason: "To check the passenger in.", + timeoutMs: 60_000, + }); + // The wait ended the moment its attempt was superseded instead of blocking + // the agent's turn until the timeout, and the card closed with it. + expect(result).toEqual({ status: "cancelled", note: SECRET_REQUEST_UNSAVED_NOTE.cancelled }); + const records = dispatched.filter((command) => command.type === "secret_request.record"); + expect(records.map((command) => command.secretStatus)).toEqual(["pending", "cancelled"]); + expect(records.every((command) => command.nodeId === originalRoot)).toBe(true); + }).pipe(Effect.provide(layer), Effect.runPromise); +}); diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index d3160184f2a3f..15f3834cdf69b 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -2420,6 +2420,13 @@ const make = Effect.gen(function* () { yield* record("cancelled"); return "cancelled" as const; } + // A steering restart replaced the run's root, so this request belonged to + // the superseded attempt: its card can never be answered into that attempt, + // and the call must stop blocking the agent's turn. + if (current.rootNodeId !== nodeId) { + yield* record("cancelled"); + return "cancelled" as const; + } yield* Effect.sleep(Duration.millis(SECRET_REQUEST_POLL_INTERVAL_MS)); } }).pipe( diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 062049e71cc78..32eb19fa570de 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -10357,14 +10357,23 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ); const run = projection.runs.find((candidate) => candidate.id === command.runId); - if (run === undefined || run.rootNodeId !== command.nodeId) { + if (run === undefined) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, commandType: command.type, - cause: `Node ${command.nodeId} is not the root of run ${command.runId}.`, + cause: `Run ${command.runId} was not found.`, }); } const existing = projection.turnItems.find((item) => item.id === command.turnItemId); + // Only a new card needs its run's current root. A steer restarts the run + // under a new root, and the card asked before it must still close. + if (existing === undefined && run.rootNodeId !== command.nodeId) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: `Node ${command.nodeId} is not the root of run ${command.runId}.`, + }); + } if (existing !== undefined && existing.type !== "secret_request") { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -10379,6 +10388,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: `Secret request ${command.turnItemId} belongs to another run.`, }); } + const nodeId = existing?.nodeId ?? command.nodeId; const now = yield* DateTime.now; const unchanged = existing !== undefined && @@ -10390,9 +10400,17 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio id: command.turnItemId, threadId: command.threadId, runId: command.runId, - nodeId: command.nodeId, - providerThreadId: run.providerThreadId, - providerTurnId: providerTurnForRun(projection, run)?.id ?? null, + nodeId, + // An update keeps where the card was asked, even after a restart. + ...(existing === undefined + ? { + providerThreadId: run.providerThreadId, + providerTurnId: providerTurnForRun(projection, run)?.id ?? null, + } + : { + providerThreadId: existing.providerThreadId, + providerTurnId: existing.providerTurnId, + }), nativeItemRef: null, parentItemId: null, ordinal: existing?.ordinal ?? (yield* nextTurnItemOrdinal(projection)), @@ -10436,7 +10454,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio type: "turn-item.updated", threadId: command.threadId, runId: command.runId, - nodeId: command.nodeId, + nodeId, providerInstanceId: run.providerInstanceId, occurredAt: now, payload: turnItem, diff --git a/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts b/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts new file mode 100644 index 0000000000000..2860b279c836a --- /dev/null +++ b/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts @@ -0,0 +1,292 @@ +import { assert, it } from "@effect/vitest"; +import { + CommandId, + EventId, + MessageId, + NodeId, + ProjectId, + ProviderDriverKind, + ProviderInstanceId, + ProviderSessionId, + ProviderThreadId, + ProviderTurnId, + RunAttemptId, + RunId, + ThreadId, + TurnItemId, +} from "@t3tools/contracts"; +import * as DateTime from "effect/DateTime"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as SqlitePersistence from "../persistence/Sqlite.ts"; +import { CodexProviderCapabilitiesV2 } from "./Adapters/CodexAdapterV2.ts"; +import * as EffectOutbox from "./EffectOutbox.ts"; +import * as Orchestrator from "./Orchestrator.ts"; +import * as ProjectionStore from "./ProjectionStore.ts"; +import * as ProviderAdapterRegistry from "./ProviderAdapterRegistry.ts"; +import * as ProviderReplayHarness from "./testkit/ProviderReplayHarness.ts"; +import type * as ProviderAdapter from "@t3tools/provider-core/server/ProviderAdapter"; + +const instanceId = ProviderInstanceId.make("codex"); +const modelSelection = { instanceId, model: "gpt-5.1-codex" }; +const adapter = { + instanceId, + driver: ProviderDriverKind.make("codex"), + getCapabilities: () => Effect.succeed(CodexProviderCapabilitiesV2), + planSelectionTransition: () => Effect.succeed({ type: "apply_on_next_turn" as const }), + openSession: () => Effect.die("No provider process needed for record controls"), +} as ProviderAdapter.ProviderAdapterV2["Service"]; +const layerDatabase = SqlitePersistence.layerMemory; +const layerTest = Layer.mergeAll( + layerDatabase, + ProjectionStore.layer.pipe(Layer.provide(layerDatabase)), + EffectOutbox.layer.pipe(Layer.provide(layerDatabase)), + ProviderReplayHarness.layerWithRegistry( + { name: "secret-request-record" }, + ProviderAdapterRegistry.layerFromAdapters([adapter]), + { databaseLayer: layerDatabase, runEffectWorker: false }, + ), +); + +const projectId = ProjectId.make("project:secret-request-record"); +const threadId = ThreadId.make("thread:secret-request-record"); +const sessionId = ProviderSessionId.make("session:secret-request-record"); +const providerThreadId = ProviderThreadId.make("provider-thread:secret-request-record"); +const runId = RunId.make("run:secret-request-record"); +const firstAttemptId = RunAttemptId.make("attempt:secret-request-record:1"); +const steeredAttemptId = RunAttemptId.make("attempt:secret-request-record:2"); +const originalRoot = NodeId.make("node:secret-request-record:original"); +const steeredRoot = NodeId.make("node:secret-request-record:steered"); +const originalTurnId = ProviderTurnId.make("provider-turn:secret-request-record:original"); +const messageId = MessageId.make("message:secret-request-record"); +const cardId = TurnItemId.make("turn-item:secret-request:secret-request-record"); +const staleCardId = TurnItemId.make("turn-item:secret-request:secret-request-record:stale"); +const now = DateTime.makeUnsafe("2026-11-11T20:00:00.000Z"); + +/** + * A pending secret card must stay closable after a steering restart replaces the + * run's root node: the call that opened it only ever carries the card's own node, + * so rejecting that node wedges the card open forever. + */ +it.effect("closes a secret card recorded under an earlier root after a steering restart", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projections = yield* ProjectionStore.ProjectionStoreV2; + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make("create-secret-request-record"), + threadId, + projectId, + title: "Secret request record", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + createdBy: "user", + creationSource: "web", + }); + yield* projections.apply({ + id: EventId.make("secret-request-record:session"), + type: "provider-session.attached", + threadId, + occurredAt: now, + payload: { + id: sessionId, + driver: adapter.driver, + providerInstanceId: instanceId, + status: "ready", + cwd: "/repo", + model: modelSelection.model, + capabilities: CodexProviderCapabilitiesV2, + createdAt: now, + updatedAt: now, + lastError: null, + }, + }); + yield* projections.apply({ + id: EventId.make("secret-request-record:provider-thread"), + type: "provider-thread.updated", + threadId, + occurredAt: now, + payload: { + id: providerThreadId, + driver: adapter.driver, + providerInstanceId: instanceId, + providerSessionId: sessionId, + appThreadId: threadId, + ownerNodeId: null, + nativeThreadRef: { driver: adapter.driver, nativeId: "native-thread", strength: "strong" }, + nativeConversationHeadRef: null, + status: "idle", + firstRunOrdinal: 1, + lastRunOrdinal: 1, + handoffIds: [], + forkedFrom: null, + createdAt: now, + updatedAt: now, + }, + }); + yield* projections.apply({ + id: EventId.make("secret-request-record:run"), + type: "run.created", + threadId, + occurredAt: now, + payload: { + id: runId, + threadId, + ordinal: 1, + providerInstanceId: instanceId, + modelSelection, + providerThreadId, + userMessageId: messageId, + rootNodeId: originalRoot, + activeAttemptId: firstAttemptId, + status: "running", + requestedAt: now, + startedAt: now, + completedAt: null, + checkpointId: null, + contextHandoffId: null, + }, + }); + yield* projections.apply({ + id: EventId.make("secret-request-record:attempt-1"), + type: "run-attempt.created", + threadId, + occurredAt: now, + payload: { + id: firstAttemptId, + runId, + attemptOrdinal: 1, + rootNodeId: originalRoot, + providerInstanceId: instanceId, + providerThreadId, + providerTurnId: null, + reason: "initial", + status: "running", + startedAt: now, + completedAt: null, + }, + }); + + // The first attempt reached the provider; the card is asked on that turn. + yield* projections.apply({ + id: EventId.make("secret-request-record:turn"), + type: "provider-turn.updated", + threadId, + occurredAt: now, + payload: { + id: originalTurnId, + providerThreadId, + nodeId: originalRoot, + runAttemptId: firstAttemptId, + nativeTurnRef: null, + ordinal: 1, + status: "running", + startedAt: now, + completedAt: null, + }, + }); + + const record = (commandId: string, nodeId: NodeId, turnItemId: TurnItemId) => + orchestrator.dispatch({ + type: "secret_request.record", + commandId: CommandId.make(commandId), + threadId, + runId, + nodeId, + turnItemId, + label: "Booking details", + reason: "To check the passenger in.", + secretStatus: "pending" as const, + }); + const readCard = (id: TurnItemId) => + projections + .getThreadRecords(threadId, ["turnItems"], { + turnItemTypes: ["secret_request"], + messageRoles: [], + }) + .pipe( + Effect.map((records) => { + const item = records.turnItems.find((candidate) => candidate.id === id); + return item?.type === "secret_request" ? item : undefined; + }), + ); + + yield* record("secret-request-record:open", originalRoot, cardId); + const opened = yield* readCard(cardId); + assert.equal(opened?.secretStatus, "pending"); + assert.equal(opened?.providerTurnId, originalTurnId); + + // The user steers while the card waits: the run's root moves to a new attempt. + yield* projections.apply({ + id: EventId.make("secret-request-record:run-steered"), + type: "run.updated", + threadId, + occurredAt: now, + payload: { + id: runId, + threadId, + ordinal: 1, + providerInstanceId: instanceId, + modelSelection, + providerThreadId, + userMessageId: messageId, + rootNodeId: steeredRoot, + activeAttemptId: steeredAttemptId, + status: "starting", + requestedAt: now, + startedAt: now, + completedAt: null, + checkpointId: null, + contextHandoffId: null, + }, + }); + yield* projections.apply({ + id: EventId.make("secret-request-record:attempt-2"), + type: "run-attempt.created", + threadId, + occurredAt: now, + payload: { + id: steeredAttemptId, + runId, + attemptOrdinal: 2, + rootNodeId: steeredRoot, + providerInstanceId: instanceId, + providerThreadId, + providerTurnId: null, + reason: "steering_restart", + status: "pending", + startedAt: null, + completedAt: null, + }, + }); + + // The waiting call only carries the card's own node; closing must still work. + yield* orchestrator.dispatch({ + type: "secret_request.record", + commandId: CommandId.make("secret-request-record:close"), + threadId, + runId, + nodeId: originalRoot, + turnItemId: cardId, + label: "Booking details", + reason: "To check the passenger in.", + secretStatus: "cancelled" as const, + }); + const cancelled = yield* readCard(cardId); + assert.equal(cancelled?.secretStatus, "cancelled"); + assert.equal(cancelled?.status, "cancelled"); + // Closing through the card's own node keeps the identity it was asked with, + // even though the run's active attempt has no provider turn of its own. + assert.equal(cancelled?.nodeId, originalRoot); + assert.equal(cancelled?.providerTurnId, originalTurnId); + + // A new card may not be opened under the stale root. + const stale = yield* Effect.flip( + record("secret-request-record:stale-open", originalRoot, staleCardId), + ); + assert.match(String((stale as { cause?: unknown }).cause ?? ""), /is not the root of run/); + }).pipe(Effect.provide(layerTest)), +);