From 5f48db878f81badf151c3264050daa202b1ed9f2 Mon Sep 17 00:00:00 2001 From: Ali Alwan <11404482+Ali-Al1@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:28:58 +0000 Subject: [PATCH 1/3] fix(server): a secret card closes after the steering restart that supersedes it A steering restart replaces the run's root node while request_secret is still pending. secret_request.record then rejected the card's own node as "not the root of run", so the card could never reach a terminal state, and the requestSecret wait loop polled forever, blocking the agent's turn and the restart that was trying to interrupt it. Record updates for an existing card now go through that card's own node (new cards still require the run's current root), and the wait loop returns cancelled as soon as the run's root changes. --- .../OrchestratorMcpService.activity.test.ts | 89 ++++++ apps/server/src/mcp/OrchestratorMcpService.ts | 7 + .../src/orchestration-v2/Orchestrator.ts | 12 +- .../SecretRequestRecord.test.ts | 266 ++++++++++++++++++ 4 files changed, 372 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/orchestration-v2/SecretRequestRecord.test.ts diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index dfaebe9228d52..c167015226fd5 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -6,6 +6,7 @@ import { ProjectId, ProviderDriverKind, ProviderInstanceId, + RunAttemptId, RunId, ScheduledTaskId, SECRET_REQUEST_UNSAVED_NOTE, @@ -849,3 +850,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 71427548f86e6..e14940b994724 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -2267,6 +2267,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 4393d3692a641..7c7a9664034c1 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -9495,14 +9495,22 @@ 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) { + const existing = projection.turnItems.find((item) => item.id === command.turnItemId); + // A steering restart replaces the run's root node, so a card recorded under an + // earlier root keeps closing through that card's own node; only a new card must + // still be rooted at the run's current root. + const referencesKnownNode = + run !== undefined && + (run.rootNodeId === command.nodeId || + (existing !== undefined && + (existing.nodeId === command.nodeId || existing.nodeId === null))); + if (!referencesKnownNode) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, commandType: command.type, cause: `Node ${command.nodeId} is not the root of run ${command.runId}.`, }); } - const existing = projection.turnItems.find((item) => item.id === command.turnItemId); if (existing !== undefined && existing.type !== "secret_request") { return yield* new OrchestratorDispatchError({ commandId: command.commandId, 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..e94c8922522a9 --- /dev/null +++ b/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts @@ -0,0 +1,266 @@ +import { assert, it } from "@effect/vitest"; +import { + CommandId, + EventId, + MessageId, + NodeId, + ProjectId, + ProviderDriverKind, + ProviderInstanceId, + ProviderSessionId, + ProviderThreadId, + 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 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, + }, + }); + + 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"); + + // 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"); + + // 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)), +); From 2bd4c8107216a475fcd942c9fbb048b4ce220b79 Mon Sep 17 00:00:00 2001 From: Ali Alwan <11404482+Ali-Al1@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:10:03 +0000 Subject: [PATCH 2/3] fix(server): a secret card update keeps the identity it was asked with Closing a card through its own node after a steering restart rebuilt the item from the run's current provider thread and active attempt, silently re-pointing a card that still belongs to the node that asked it. Keep the recorded nodeId, providerThreadId and providerTurnId on updates, matching what link_request.record already does. --- .../src/orchestration-v2/Orchestrator.ts | 17 +++++++++--- .../SecretRequestRecord.test.ts | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 7c7a9664034c1..35230069a65d7 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -9525,6 +9525,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 && @@ -9536,9 +9537,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)), @@ -9582,7 +9591,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 index e94c8922522a9..2860b279c836a 100644 --- a/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts +++ b/apps/server/src/orchestration-v2/SecretRequestRecord.test.ts @@ -9,6 +9,7 @@ import { ProviderInstanceId, ProviderSessionId, ProviderThreadId, + ProviderTurnId, RunAttemptId, RunId, ThreadId, @@ -56,6 +57,7 @@ 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"); @@ -168,6 +170,25 @@ it.effect("closes a secret card recorded under an earlier root after a steering }, }); + // 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", @@ -196,6 +217,7 @@ it.effect("closes a secret card recorded under an earlier root after a steering 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({ @@ -256,6 +278,10 @@ it.effect("closes a secret card recorded under an earlier root after a steering 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( From a594ef0f4680450baf019083dfa492e1051739b5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 14 Nov 2026 18:30:37 +0000 Subject: [PATCH 3/3] refactor(server): a secret card's root check matches the link card's Only a new secret card must be rooted at its run's current root, the same rule link_request.record uses; a missing run gets its own error. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 457f09d90d858..32eb19fa570de 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -10357,16 +10357,17 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ); const run = projection.runs.find((candidate) => candidate.id === command.runId); + if (run === undefined) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: `Run ${command.runId} was not found.`, + }); + } const existing = projection.turnItems.find((item) => item.id === command.turnItemId); - // A steering restart replaces the run's root node, so a card recorded under an - // earlier root keeps closing through that card's own node; only a new card must - // still be rooted at the run's current root. - const referencesKnownNode = - run !== undefined && - (run.rootNodeId === command.nodeId || - (existing !== undefined && - (existing.nodeId === command.nodeId || existing.nodeId === null))); - if (!referencesKnownNode) { + // 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,