| author | |
| committer | |
| log | 39aeaf404ad6d923f1bb7de4ec61549559d356ff |
| tree | 3f7626dd3960b9f8ebf69c006506e799d2ae3bfd |
| parent | 52ffe0cae6e5e03c95037152ef4548f40daf5c2e |
| signature | Signed by PGP key B5690EEEBB952194 |
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 <ghassemimahyad@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>2 files changed, 289 insertions(+), 0 deletions(-)
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts+211| ... | ... | @@ -19,6 +19,7 @@ import { |
| 19 | 19 | type OrchestrationV2ProviderTurn, |
| 20 | 20 | type OrchestrationV2TurnItem, |
| 21 | 21 | ProjectId, |
| 22 | type ProviderApprovalDecision, | |
| 22 | 23 | ProviderInstanceId, |
| 23 | 24 | ProviderSessionId, |
| 24 | 25 | ProviderThreadId, |
| ... | ... | @@ -2786,6 +2787,216 @@ describe("CodexAdapterV2 post-settle continuation", () => { |
| 2786 | 2787 | ), |
| 2787 | 2788 | ); |
| 2788 | 2789 | |
| 2790 | it.effect("stops asking for commands the user allowed for the session", () => | |
| 2791 | Effect.scoped( | |
| 2792 | Effect.gen(function* () { | |
| 2793 | const nativeThreadId = "session-approval-thread"; | |
| 2794 | const turnIds = [ | |
| 2795 | "session-approval-turn-1", | |
| 2796 | "session-approval-turn-2", | |
| 2797 | "session-approval-turn-3", | |
| 2798 | ]; | |
| 2799 | const turnPolicies = { | |
| 2800 | "approval-required": { | |
| 2801 | approvalPolicy: "untrusted", | |
| 2802 | approvalsReviewer: "user", | |
| 2803 | sandboxPolicy: { type: "readOnly" }, | |
| 2804 | }, | |
| 2805 | "full-access": { | |
| 2806 | approvalPolicy: "never", | |
| 2807 | approvalsReviewer: "user", | |
| 2808 | sandboxPolicy: { type: "dangerFullAccess" }, | |
| 2809 | }, | |
| 2810 | } as const; | |
| 2811 | const turnModes = ["approval-required", "full-access", "approval-required"] as const; | |
| 2812 | const turnText = (index: number) => `Review round ${index + 1}.`; | |
| 2813 | const startTurn = (index: number): ReadonlyArray<CodexReplay.CodexAppServerReplayEntry> => [ | |
| 2814 | { | |
| 2815 | type: "expect_outbound", | |
| 2816 | label: `turn ${index + 1} turn/start`, | |
| 2817 | frame: { | |
| 2818 | id: 3 + index, | |
| 2819 | method: "turn/start", | |
| 2820 | params: { | |
| 2821 | threadId: nativeThreadId, | |
| 2822 | input: [{ type: "text", text: turnText(index) }], | |
| 2823 | cwd: "/workspace", | |
| 2824 | model: "gpt-5.4", | |
| 2825 | ...turnPolicies[turnModes[index]!], | |
| 2826 | summary: "detailed", | |
| 2827 | }, | |
| 2828 | }, | |
| 2829 | }, | |
| 2830 | { | |
| 2831 | type: "emit_inbound", | |
| 2832 | label: `turn ${index + 1} turn/start`, | |
| 2833 | frame: { | |
| 2834 | id: 3 + index, | |
| 2835 | result: { turn: makeCodexReplayTurn({ id: turnIds[index]!, status: "inProgress" }) }, | |
| 2836 | }, | |
| 2837 | }, | |
| 2838 | turnEvent("turn/started", turnIds[index]!), | |
| 2839 | ]; | |
| 2840 | const approval = (input: { | |
| 2841 | readonly id: number; | |
| 2842 | readonly turnId: string; | |
| 2843 | readonly kind: "commandExecution" | "fileChange"; | |
| 2844 | readonly itemId: string; | |
| 2845 | readonly decision: string; | |
| 2846 | }): ReadonlyArray<CodexReplay.CodexAppServerReplayEntry> => [ | |
| 2847 | { | |
| 2848 | type: "emit_inbound", | |
| 2849 | label: `${input.itemId} approval`, | |
| 2850 | frame: { | |
| 2851 | id: input.id, | |
| 2852 | method: `item/${input.kind}/requestApproval`, | |
| 2853 | params: { | |
| 2854 | threadId: nativeThreadId, | |
| 2855 | turnId: input.turnId, | |
| 2856 | itemId: input.itemId, | |
| 2857 | startedAtMs: 1782622441000, | |
| 2858 | ...(input.kind === "commandExecution" ? { command: `cat ${input.itemId}` } : {}), | |
| 2859 | }, | |
| 2860 | }, | |
| 2861 | }, | |
| 2862 | { | |
| 2863 | type: "expect_outbound", | |
| 2864 | label: `${input.itemId} decision`, | |
| 2865 | frame: { id: input.id, result: { decision: input.decision } }, | |
| 2866 | }, | |
| 2867 | ]; | |
| 2868 | const turnEvent = ( | |
| 2869 | method: "turn/started" | "turn/completed", | |
| 2870 | turnId: string, | |
| 2871 | ): CodexReplay.CodexAppServerReplayEntry => ({ | |
| 2872 | type: "emit_inbound", | |
| 2873 | label: `${turnId} ${method}`, | |
| 2874 | frame: { | |
| 2875 | method, | |
| 2876 | params: { | |
| 2877 | threadId: nativeThreadId, | |
| 2878 | turn: makeCodexReplayTurn({ | |
| 2879 | id: turnId, | |
| 2880 | status: method === "turn/started" ? "inProgress" : "completed", | |
| 2881 | }), | |
| 2882 | }, | |
| 2883 | }, | |
| 2884 | }); | |
| 2885 | const [patchRequest, patchDecision] = approval({ | |
| 2886 | id: 102, | |
| 2887 | turnId: turnIds[0]!, | |
| 2888 | kind: "fileChange", | |
| 2889 | itemId: "first-patch", | |
| 2890 | decision: "acceptForSession", | |
| 2891 | }); | |
| 2892 | const transcript = makeCodexReplayTranscript({ | |
| 2893 | scenario: "codex-session-approvals", | |
| 2894 | entries: [ | |
| 2895 | ...codexReplayPreamble({ | |
| 2896 | nativeThreadId, | |
| 2897 | nativeTurnId: turnIds[0]!, | |
| 2898 | prompt: turnText(0), | |
| 2899 | }).slice(0, -3), | |
| 2900 | ...startTurn(0), | |
| 2901 | ...approval({ | |
| 2902 | id: 100, | |
| 2903 | turnId: turnIds[0]!, | |
| 2904 | kind: "commandExecution", | |
| 2905 | itemId: "first-command", | |
| 2906 | decision: "acceptForSession", | |
| 2907 | }), | |
| 2908 | // Only commands were allowed, so a file change still asks. The user | |
| 2909 | // answers it after the turn ended and the mode changed. | |
| 2910 | patchRequest!, | |
| 2911 | // Answered without asking: Codex alone would ask for every new command. | |
| 2912 | ...approval({ | |
| 2913 | id: 101, | |
| 2914 | turnId: turnIds[0]!, | |
| 2915 | kind: "commandExecution", | |
| 2916 | itemId: "second-command", | |
| 2917 | decision: "acceptForSession", | |
| 2918 | }), | |
| 2919 | turnEvent("turn/completed", turnIds[0]!), | |
| 2920 | ...startTurn(1), | |
| 2921 | patchDecision!, | |
| 2922 | turnEvent("turn/completed", turnIds[1]!), | |
| 2923 | ...startTurn(2), | |
| 2924 | // Back in the first mode, neither allowance comes back. | |
| 2925 | ...approval({ | |
| 2926 | id: 103, | |
| 2927 | turnId: turnIds[2]!, | |
| 2928 | kind: "commandExecution", | |
| 2929 | itemId: "third-command", | |
| 2930 | decision: "accept", | |
| 2931 | }), | |
| 2932 | ...approval({ | |
| 2933 | id: 104, | |
| 2934 | turnId: turnIds[2]!, | |
| 2935 | kind: "fileChange", | |
| 2936 | itemId: "second-patch", | |
| 2937 | decision: "accept", | |
| 2938 | }), | |
| 2939 | turnEvent("turn/completed", turnIds[2]!), | |
| 2940 | ], | |
| 2941 | }); | |
| 2942 | const harness = yield* makeCodexReplayHarness(transcript); | |
| 2943 | const requests = () => [ | |
| 2944 | ...new Map( | |
| 2945 | harness.events.flatMap((event) => | |
| 2946 | event.type === "runtime_request.updated" | |
| 2947 | ? [[event.runtimeRequest.id, event.runtimeRequest] as const] | |
| 2948 | : [], | |
| 2949 | ), | |
| 2950 | ).values(), | |
| 2951 | ]; | |
| 2952 | const answer = ( | |
| 2953 | count: number, | |
| 2954 | kind: "command" | "file-change", | |
| 2955 | decision: ProviderApprovalDecision, | |
| 2956 | ) => | |
| 2957 | Effect.gen(function* () { | |
| 2958 | yield* awaitUntil(() => requests().length === count, `approval request ${count}`); | |
| 2959 | const request = requests()[count - 1]!; | |
| 2960 | assert.equal(request.kind, kind); | |
| 2961 | yield* harness.runtime.respondToRuntimeRequest({ requestId: request.id, decision }); | |
| 2962 | }); | |
| 2963 | ||
| 2964 | const runTurn = (index: number) => | |
| 2965 | Effect.gen(function* () { | |
| 2966 | yield* harness.runtime.startTurn({ | |
| 2967 | ...makeCodexTestTurnInput({ | |
| 2968 | threadId: harness.threadId, | |
| 2969 | providerThread: harness.providerThread, | |
| 2970 | now: yield* DateTime.now, | |
| 2971 | attemptId: RunAttemptId.make(`session-approvals-${index + 1}`), | |
| 2972 | text: turnText(index), | |
| 2973 | }), | |
| 2974 | runOrdinal: index + 1, | |
| 2975 | providerTurnOrdinal: index + 1, | |
| 2976 | runtimePolicy: { ...CODEX_TEST_RUNTIME_POLICY, runtimeMode: turnModes[index]! }, | |
| 2977 | }); | |
| 2978 | }); | |
| 2979 | ||
| 2980 | yield* runTurn(0); | |
| 2981 | yield* answer(1, "command", "acceptForSession"); | |
| 2982 | yield* awaitUntil(() => requests().length === 2, "file change request"); | |
| 2983 | yield* awaitUntil(() => harness.terminalEvents().length === 1, "turn 1"); | |
| 2984 | yield* runTurn(1); | |
| 2985 | yield* answer(2, "file-change", "acceptForSession"); | |
| 2986 | yield* awaitUntil(() => harness.terminalEvents().length === 2, "turn 2"); | |
| 2987 | yield* runTurn(2); | |
| 2988 | yield* answer(3, "command", "accept"); | |
| 2989 | yield* answer(4, "file-change", "accept"); | |
| 2990 | yield* awaitUntil(() => harness.terminalEvents().length === 3, "turn 3"); | |
| 2991 | assert.lengthOf(requests(), 4); | |
| 2992 | }).pipe( | |
| 2993 | Effect.provide( | |
| 2994 | Layer.mergeAll(IdAllocator.layer, McpProviderSessions.layer, NodeServices.layer), | |
| 2995 | ), | |
| 2996 | ), | |
| 2997 | ), | |
| 2998 | ); | |
| 2999 | ||
| 2789 | 3000 | it.effect("preserves T3 context on the wire and restores it after compaction", () => |
| 2790 | 3001 | Effect.scoped( |
| 2791 | 3002 | Effect.gen(function* () { |
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts+78| ... | ... | @@ -1880,6 +1880,45 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 1880 | 1880 | const pendingRuntimeRequests = yield* Ref.make( |
| 1881 | 1881 | new Map<string, PendingCodexRuntimeRequest>(), |
| 1882 | 1882 | ); |
| 1883 | /** | |
| 1884 | * Approval kinds the user chose "Always allow this session" for, by | |
| 1885 | * native thread. Codex's own acceptForSession only skips the exact | |
| 1886 | * command or patch it answered, so every new command would ask again. | |
| 1887 | * A turn in another runtime mode ends the allowance, and so does the | |
| 1888 | * thread unloading, like Codex's own session approvals. | |
| 1889 | */ | |
| 1890 | const sessionApprovals = new Map< | |
| 1891 | string, | |
| 1892 | { readonly runtimeMode: RuntimeMode; readonly kinds: Set<ProviderRequestKind> } | |
| 1893 | >(); | |
| 1894 | /** | |
| 1895 | * The allowed kinds for an approval request's turn. Read it when the | |
| 1896 | * request arrives and add to that same set when the user answers, so a | |
| 1897 | * late answer from a turn whose mode has since ended is dropped. | |
| 1898 | */ | |
| 1899 | /** Every native turn registers here. A turn in another mode ends the allowance. */ | |
| 1900 | const endSessionApprovalsOnModeChange = (context: ActiveCodexTurnContext) => { | |
| 1901 | const nativeThreadId = context.providerThread.nativeThreadRef?.nativeId; | |
| 1902 | if ( | |
| 1903 | nativeThreadId != null && | |
| 1904 | sessionApprovals.get(nativeThreadId)?.runtimeMode !== | |
| 1905 | context.input.runtimePolicy.runtimeMode | |
| 1906 | ) { | |
| 1907 | sessionApprovals.delete(nativeThreadId); | |
| 1908 | } | |
| 1909 | }; | |
| 1910 | const sessionAllowance = ( | |
| 1911 | nativeThreadId: string, | |
| 1912 | context: ActiveCodexTurnContext, | |
| 1913 | ): Set<ProviderRequestKind> => { | |
| 1914 | const runtimeMode = context.input.runtimePolicy.runtimeMode; | |
| 1915 | const allowed = sessionApprovals.get(nativeThreadId); | |
| 1916 | if (allowed?.runtimeMode === runtimeMode) return allowed.kinds; | |
| 1917 | if (allowed !== undefined) return new Set(); | |
| 1918 | const kinds = new Set<ProviderRequestKind>(); | |
| 1919 | sessionApprovals.set(nativeThreadId, { runtimeMode, kinds }); | |
| 1920 | return kinds; | |
| 1921 | }; | |
| 1883 | 1922 | /** |
| 1884 | 1923 | * Turn contexts retained past turn/completed while background command |
| 1885 | 1924 | * items started in that turn are still running, so late item events |
| ... | ... | @@ -2030,6 +2069,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 2030 | 2069 | return next; |
| 2031 | 2070 | }); |
| 2032 | 2071 | beginTurnTokenUsage(context); |
| 2072 | endSessionApprovalsOnModeChange(context); | |
| 2033 | 2073 | yield* Ref.update(activeTurns, (current) => { |
| 2034 | 2074 | const updated = new Map(current); |
| 2035 | 2075 | updated.set(input.nativeTurnId, context); |
| ... | ... | @@ -2615,6 +2655,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 2615 | 2655 | itemPositions: new Map(), |
| 2616 | 2656 | }; |
| 2617 | 2657 | beginTurnTokenUsage(activeContext); |
| 2658 | endSessionApprovalsOnModeChange(activeContext); | |
| 2618 | 2659 | yield* Ref.update(activeTurns, (current) => { |
| 2619 | 2660 | const updated = new Map(current); |
| 2620 | 2661 | updated.set(turn.nativeTurnId, activeContext); |
| ... | ... | @@ -5076,6 +5117,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5076 | 5117 | payload, |
| 5077 | 5118 | ); |
| 5078 | 5119 | } |
| 5120 | const allowance = sessionAllowance(payload.threadId, context); | |
| 5121 | if (allowance.has("command")) { | |
| 5122 | return { | |
| 5123 | decision: "acceptForSession", | |
| 5124 | } satisfies CodexSchema.CommandExecutionRequestApprovalResponse; | |
| 5125 | } | |
| 5079 | 5126 | |
| 5080 | 5127 | const nativeRequestId = payload.approvalId ?? payload.itemId; |
| 5081 | 5128 | const artifacts = yield* buildApprovalRequestArtifacts({ |
| ... | ... | @@ -5124,6 +5171,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5124 | 5171 | }), |
| 5125 | 5172 | ), |
| 5126 | 5173 | ); |
| 5174 | if (resolved === "acceptForSession" || resolved === "acceptAlways") { | |
| 5175 | allowance.add("command"); | |
| 5176 | } | |
| 5127 | 5177 | return { |
| 5128 | 5178 | decision: resolved === "acceptAlways" ? "acceptForSession" : resolved, |
| 5129 | 5179 | } satisfies CodexSchema.CommandExecutionRequestApprovalResponse; |
| ... | ... | @@ -5139,6 +5189,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5139 | 5189 | payload, |
| 5140 | 5190 | ); |
| 5141 | 5191 | } |
| 5192 | const allowance = sessionAllowance(payload.threadId, context); | |
| 5193 | if (allowance.has("file-change")) { | |
| 5194 | return { | |
| 5195 | decision: "acceptForSession", | |
| 5196 | } satisfies CodexSchema.FileChangeRequestApprovalResponse; | |
| 5197 | } | |
| 5142 | 5198 | |
| 5143 | 5199 | const artifacts = yield* buildApprovalRequestArtifacts({ |
| 5144 | 5200 | context, |
| ... | ... | @@ -5184,6 +5240,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5184 | 5240 | }), |
| 5185 | 5241 | ), |
| 5186 | 5242 | ); |
| 5243 | if (resolved === "acceptForSession" || resolved === "acceptAlways") { | |
| 5244 | allowance.add("file-change"); | |
| 5245 | } | |
| 5187 | 5246 | return { |
| 5188 | 5247 | decision: resolved === "acceptAlways" ? "acceptForSession" : resolved, |
| 5189 | 5248 | } satisfies CodexSchema.FileChangeRequestApprovalResponse; |
| ... | ... | @@ -5343,6 +5402,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5343 | 5402 | payload, |
| 5344 | 5403 | ); |
| 5345 | 5404 | } |
| 5405 | const allowance = sessionAllowance(payload.conversationId, context); | |
| 5406 | if (allowance.has("command")) { | |
| 5407 | return { | |
| 5408 | decision: "approved_for_session", | |
| 5409 | } satisfies CodexSchema.ExecCommandApprovalResponse; | |
| 5410 | } | |
| 5346 | 5411 | |
| 5347 | 5412 | const nativeRequestId = payload.approvalId ?? payload.callId; |
| 5348 | 5413 | const artifacts = yield* buildApprovalRequestArtifacts({ |
| ... | ... | @@ -5389,6 +5454,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5389 | 5454 | }), |
| 5390 | 5455 | ), |
| 5391 | 5456 | ); |
| 5457 | if (resolved === "acceptForSession" || resolved === "acceptAlways") { | |
| 5458 | allowance.add("command"); | |
| 5459 | } | |
| 5392 | 5460 | return { |
| 5393 | 5461 | decision: approvalDecisionToLegacyReviewDecision(resolved), |
| 5394 | 5462 | } satisfies CodexSchema.ExecCommandApprovalResponse; |
| ... | ... | @@ -5404,6 +5472,12 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5404 | 5472 | payload, |
| 5405 | 5473 | ); |
| 5406 | 5474 | } |
| 5475 | const allowance = sessionAllowance(payload.conversationId, context); | |
| 5476 | if (allowance.has("file-change")) { | |
| 5477 | return { | |
| 5478 | decision: "approved_for_session", | |
| 5479 | } satisfies CodexSchema.ApplyPatchApprovalResponse; | |
| 5480 | } | |
| 5407 | 5481 | |
| 5408 | 5482 | const artifacts = yield* buildApprovalRequestArtifacts({ |
| 5409 | 5483 | context, |
| ... | ... | @@ -5449,6 +5523,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 5449 | 5523 | }), |
| 5450 | 5524 | ), |
| 5451 | 5525 | ); |
| 5526 | if (resolved === "acceptForSession" || resolved === "acceptAlways") { | |
| 5527 | allowance.add("file-change"); | |
| 5528 | } | |
| 5452 | 5529 | return { |
| 5453 | 5530 | decision: approvalDecisionToLegacyReviewDecision(resolved), |
| 5454 | 5531 | } satisfies CodexSchema.ApplyPatchApprovalResponse; |
| ... | ... | @@ -6706,6 +6783,7 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( |
| 6706 | 6783 | unloadThread: (unloadInput) => |
| 6707 | 6784 | Effect.gen(function* () { |
| 6708 | 6785 | const nativeThreadId = yield* getNativeThreadId(unloadInput.providerThread); |
| 6786 | sessionApprovals.delete(nativeThreadId); | |
| 6709 | 6787 | yield* client.request("thread/unsubscribe", { threadId: nativeThreadId }); |
| 6710 | 6788 | }).pipe( |
| 6711 | 6789 | Effect.mapError((cause) => |