From 39aeaf404ad6d923f1bb7de4ec61549559d356ff Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 14 Oct 2026 15:53:52 -0700 Subject: [PATCH] fix(server): "Always allow this session" stops Codex asking for every command (#19635) After "Always allow this session", the Codex adapter answers later command (or file change) approvals in that thread for the rest of the session. A turn in another runtime mode, or the thread unloading, ends it. Takes over #19598. Co-authored-by: lovelycodemonkey Co-authored-by: Claude Opus 5.5 --- .../Adapters/CodexAdapterV2.test.ts | 211 ++++++++++++++++++ .../Adapters/CodexAdapterV2.ts | 78 +++++++ 2 files changed, 289 insertions(+) diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts index ae29f052617b928b5b34b34ffbef6e5fe40813ed..e7eb16c378cb78592620ed2b0e81e4462b2d8000 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts @@ -19,6 +19,7 @@ import { type OrchestrationV2ProviderTurn, type OrchestrationV2TurnItem, ProjectId, + type ProviderApprovalDecision, ProviderInstanceId, ProviderSessionId, ProviderThreadId, @@ -2786,6 +2787,216 @@ describe("CodexAdapterV2 post-settle continuation", () => { ), ); + it.effect("stops asking for commands the user allowed for the session", () => + Effect.scoped( + Effect.gen(function* () { + const nativeThreadId = "session-approval-thread"; + const turnIds = [ + "session-approval-turn-1", + "session-approval-turn-2", + "session-approval-turn-3", + ]; + const turnPolicies = { + "approval-required": { + approvalPolicy: "untrusted", + approvalsReviewer: "user", + sandboxPolicy: { type: "readOnly" }, + }, + "full-access": { + approvalPolicy: "never", + approvalsReviewer: "user", + sandboxPolicy: { type: "dangerFullAccess" }, + }, + } as const; + const turnModes = ["approval-required", "full-access", "approval-required"] as const; + const turnText = (index: number) => `Review round ${index + 1}.`; + const startTurn = (index: number): ReadonlyArray => [ + { + type: "expect_outbound", + label: `turn ${index + 1} turn/start`, + frame: { + id: 3 + index, + method: "turn/start", + params: { + threadId: nativeThreadId, + input: [{ type: "text", text: turnText(index) }], + cwd: "/workspace", + model: "gpt-5.4", + ...turnPolicies[turnModes[index]!], + summary: "detailed", + }, + }, + }, + { + type: "emit_inbound", + label: `turn ${index + 1} turn/start`, + frame: { + id: 3 + index, + result: { turn: makeCodexReplayTurn({ id: turnIds[index]!, status: "inProgress" }) }, + }, + }, + turnEvent("turn/started", turnIds[index]!), + ]; + const approval = (input: { + readonly id: number; + readonly turnId: string; + readonly kind: "commandExecution" | "fileChange"; + readonly itemId: string; + readonly decision: string; + }): ReadonlyArray => [ + { + type: "emit_inbound", + label: `${input.itemId} approval`, + frame: { + id: input.id, + method: `item/${input.kind}/requestApproval`, + params: { + threadId: nativeThreadId, + turnId: input.turnId, + itemId: input.itemId, + startedAtMs: 1782622441000, + ...(input.kind === "commandExecution" ? { command: `cat ${input.itemId}` } : {}), + }, + }, + }, + { + type: "expect_outbound", + label: `${input.itemId} decision`, + frame: { id: input.id, result: { decision: input.decision } }, + }, + ]; + const turnEvent = ( + method: "turn/started" | "turn/completed", + turnId: string, + ): CodexReplay.CodexAppServerReplayEntry => ({ + type: "emit_inbound", + label: `${turnId} ${method}`, + frame: { + method, + params: { + threadId: nativeThreadId, + turn: makeCodexReplayTurn({ + id: turnId, + status: method === "turn/started" ? "inProgress" : "completed", + }), + }, + }, + }); + const [patchRequest, patchDecision] = approval({ + id: 102, + turnId: turnIds[0]!, + kind: "fileChange", + itemId: "first-patch", + decision: "acceptForSession", + }); + const transcript = makeCodexReplayTranscript({ + scenario: "codex-session-approvals", + entries: [ + ...codexReplayPreamble({ + nativeThreadId, + nativeTurnId: turnIds[0]!, + prompt: turnText(0), + }).slice(0, -3), + ...startTurn(0), + ...approval({ + id: 100, + turnId: turnIds[0]!, + kind: "commandExecution", + itemId: "first-command", + decision: "acceptForSession", + }), + // Only commands were allowed, so a file change still asks. The user + // answers it after the turn ended and the mode changed. + patchRequest!, + // Answered without asking: Codex alone would ask for every new command. + ...approval({ + id: 101, + turnId: turnIds[0]!, + kind: "commandExecution", + itemId: "second-command", + decision: "acceptForSession", + }), + turnEvent("turn/completed", turnIds[0]!), + ...startTurn(1), + patchDecision!, + turnEvent("turn/completed", turnIds[1]!), + ...startTurn(2), + // Back in the first mode, neither allowance comes back. + ...approval({ + id: 103, + turnId: turnIds[2]!, + kind: "commandExecution", + itemId: "third-command", + decision: "accept", + }), + ...approval({ + id: 104, + turnId: turnIds[2]!, + kind: "fileChange", + itemId: "second-patch", + decision: "accept", + }), + turnEvent("turn/completed", turnIds[2]!), + ], + }); + const harness = yield* makeCodexReplayHarness(transcript); + const requests = () => [ + ...new Map( + harness.events.flatMap((event) => + event.type === "runtime_request.updated" + ? [[event.runtimeRequest.id, event.runtimeRequest] as const] + : [], + ), + ).values(), + ]; + const answer = ( + count: number, + kind: "command" | "file-change", + decision: ProviderApprovalDecision, + ) => + Effect.gen(function* () { + yield* awaitUntil(() => requests().length === count, `approval request ${count}`); + const request = requests()[count - 1]!; + assert.equal(request.kind, kind); + yield* harness.runtime.respondToRuntimeRequest({ requestId: request.id, decision }); + }); + + const runTurn = (index: number) => + Effect.gen(function* () { + yield* harness.runtime.startTurn({ + ...makeCodexTestTurnInput({ + threadId: harness.threadId, + providerThread: harness.providerThread, + now: yield* DateTime.now, + attemptId: RunAttemptId.make(`session-approvals-${index + 1}`), + text: turnText(index), + }), + runOrdinal: index + 1, + providerTurnOrdinal: index + 1, + runtimePolicy: { ...CODEX_TEST_RUNTIME_POLICY, runtimeMode: turnModes[index]! }, + }); + }); + + yield* runTurn(0); + yield* answer(1, "command", "acceptForSession"); + yield* awaitUntil(() => requests().length === 2, "file change request"); + yield* awaitUntil(() => harness.terminalEvents().length === 1, "turn 1"); + yield* runTurn(1); + yield* answer(2, "file-change", "acceptForSession"); + yield* awaitUntil(() => harness.terminalEvents().length === 2, "turn 2"); + yield* runTurn(2); + yield* answer(3, "command", "accept"); + yield* answer(4, "file-change", "accept"); + yield* awaitUntil(() => harness.terminalEvents().length === 3, "turn 3"); + assert.lengthOf(requests(), 4); + }).pipe( + Effect.provide( + Layer.mergeAll(IdAllocator.layer, McpProviderSessions.layer, NodeServices.layer), + ), + ), + ), + ); + it.effect("preserves T3 context on the wire and restores it after compaction", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts index 1973ef95e379cd7445f25bd926ee3ce1cd2343d7..aace0e6e008610db7eadee25ec4dbe2e28332de9 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts @@ -1880,6 +1880,45 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( const pendingRuntimeRequests = yield* Ref.make( new Map(), ); + /** + * Approval kinds the user chose "Always allow this session" for, by + * native thread. Codex's own acceptForSession only skips the exact + * command or patch it answered, so every new command would ask again. + * A turn in another runtime mode ends the allowance, and so does the + * thread unloading, like Codex's own session approvals. + */ + const sessionApprovals = new Map< + string, + { readonly runtimeMode: RuntimeMode; readonly kinds: Set } + >(); + /** + * The allowed kinds for an approval request's turn. Read it when the + * request arrives and add to that same set when the user answers, so a + * late answer from a turn whose mode has since ended is dropped. + */ + /** Every native turn registers here. A turn in another mode ends the allowance. */ + const endSessionApprovalsOnModeChange = (context: ActiveCodexTurnContext) => { + const nativeThreadId = context.providerThread.nativeThreadRef?.nativeId; + if ( + nativeThreadId != null && + sessionApprovals.get(nativeThreadId)?.runtimeMode !== + context.input.runtimePolicy.runtimeMode + ) { + sessionApprovals.delete(nativeThreadId); + } + }; + const sessionAllowance = ( + nativeThreadId: string, + context: ActiveCodexTurnContext, + ): Set => { + const runtimeMode = context.input.runtimePolicy.runtimeMode; + const allowed = sessionApprovals.get(nativeThreadId); + if (allowed?.runtimeMode === runtimeMode) return allowed.kinds; + if (allowed !== undefined) return new Set(); + const kinds = new Set(); + sessionApprovals.set(nativeThreadId, { runtimeMode, kinds }); + return kinds; + }; /** * Turn contexts retained past turn/completed while background command * items started in that turn are still running, so late item events @@ -2030,6 +2069,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( return next; }); beginTurnTokenUsage(context); + endSessionApprovalsOnModeChange(context); yield* Ref.update(activeTurns, (current) => { const updated = new Map(current); updated.set(input.nativeTurnId, context); @@ -2615,6 +2655,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( itemPositions: new Map(), }; beginTurnTokenUsage(activeContext); + endSessionApprovalsOnModeChange(activeContext); yield* Ref.update(activeTurns, (current) => { const updated = new Map(current); updated.set(turn.nativeTurnId, activeContext); @@ -5076,6 +5117,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( payload, ); } + const allowance = sessionAllowance(payload.threadId, context); + if (allowance.has("command")) { + return { + decision: "acceptForSession", + } satisfies CodexSchema.CommandExecutionRequestApprovalResponse; + } const nativeRequestId = payload.approvalId ?? payload.itemId; const artifacts = yield* buildApprovalRequestArtifacts({ @@ -5124,6 +5171,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( }), ), ); + if (resolved === "acceptForSession" || resolved === "acceptAlways") { + allowance.add("command"); + } return { decision: resolved === "acceptAlways" ? "acceptForSession" : resolved, } satisfies CodexSchema.CommandExecutionRequestApprovalResponse; @@ -5139,6 +5189,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( payload, ); } + const allowance = sessionAllowance(payload.threadId, context); + if (allowance.has("file-change")) { + return { + decision: "acceptForSession", + } satisfies CodexSchema.FileChangeRequestApprovalResponse; + } const artifacts = yield* buildApprovalRequestArtifacts({ context, @@ -5184,6 +5240,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( }), ), ); + if (resolved === "acceptForSession" || resolved === "acceptAlways") { + allowance.add("file-change"); + } return { decision: resolved === "acceptAlways" ? "acceptForSession" : resolved, } satisfies CodexSchema.FileChangeRequestApprovalResponse; @@ -5343,6 +5402,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( payload, ); } + const allowance = sessionAllowance(payload.conversationId, context); + if (allowance.has("command")) { + return { + decision: "approved_for_session", + } satisfies CodexSchema.ExecCommandApprovalResponse; + } const nativeRequestId = payload.approvalId ?? payload.callId; const artifacts = yield* buildApprovalRequestArtifacts({ @@ -5389,6 +5454,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( }), ), ); + if (resolved === "acceptForSession" || resolved === "acceptAlways") { + allowance.add("command"); + } return { decision: approvalDecisionToLegacyReviewDecision(resolved), } satisfies CodexSchema.ExecCommandApprovalResponse; @@ -5404,6 +5472,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( payload, ); } + const allowance = sessionAllowance(payload.conversationId, context); + if (allowance.has("file-change")) { + return { + decision: "approved_for_session", + } satisfies CodexSchema.ApplyPatchApprovalResponse; + } const artifacts = yield* buildApprovalRequestArtifacts({ context, @@ -5449,6 +5523,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( }), ), ); + if (resolved === "acceptForSession" || resolved === "acceptAlways") { + allowance.add("file-change"); + } return { decision: approvalDecisionToLegacyReviewDecision(resolved), } satisfies CodexSchema.ApplyPatchApprovalResponse; @@ -6706,6 +6783,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( unloadThread: (unloadInput) => Effect.gen(function* () { const nativeThreadId = yield* getNativeThreadId(unloadInput.providerThread); + sessionApprovals.delete(nativeThreadId); yield* client.request("thread/unsubscribe", { threadId: nativeThreadId }); }).pipe( Effect.mapError((cause) => -- 2.54.0