From ff404a27236f1c2315fad7d197b8b92d86c45786 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 15 Jul 2026 15:35:58 +0300 Subject: [PATCH 1/5] fix(agent): honor Codex full access permissions Generated-By: PostHog Code Task-Id: 1f1fe07c-6384-4d1e-8ab9-bad3700caa2e --- .../codex-app-server-agent.test.ts | 8 +- .../codex-app-server/session-config.test.ts | 14 +++ .../codex-app-server/session-config.ts | 4 +- packages/core/src/sessions/sessionService.ts | 50 +++++---- .../sessions/sessionServiceHost.test.ts | 103 ++++++++++++++++-- .../src/services/agent/agent.test.ts | 85 ++++++++++++++- .../src/services/agent/agent.ts | 25 ++++- 7 files changed, 250 insertions(+), 39 deletions(-) diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts index f613c0f236..7781fb3234 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts @@ -864,7 +864,7 @@ describe("CodexAppServerAgent", () => { ).toBe("untrusted"); }); - it("falls back to auto for a non-codex initial permissionMode", async () => { + it("maps bypassPermissions to Codex full-access", async () => { const stub = makeStubRpc({ "thread/start": { thread: { id: "t" } }, "turn/start": { turn: { id: "turn_1" } }, @@ -874,7 +874,6 @@ describe("CodexAppServerAgent", () => { processOptions: { binaryPath: "/x/codex" }, rpcFactory: stub.factory, }); - // "bypassPermissions" is a Claude mode, not a codex mode → default "auto". await agent.newSession({ cwd: "/r", _meta: { permissionMode: "bypassPermissions" }, @@ -889,7 +888,10 @@ describe("CodexAppServerAgent", () => { const turnStart = stub.requests.find((r) => r.method === "turn/start"); expect( (turnStart?.params as { approvalPolicy?: string }).approvalPolicy, - ).toBe("on-request"); + ).toBe("never"); + expect( + (turnStart?.params as { sandboxPolicy?: unknown }).sandboxPolicy, + ).toEqual({ type: "dangerFullAccess" }); }); it("applies a read-only sandboxPolicy + approvalPolicy when the picker is Plan", async () => { diff --git a/packages/agent/src/adapters/codex-app-server/session-config.test.ts b/packages/agent/src/adapters/codex-app-server/session-config.test.ts index 9eed010c5a..ece51251f4 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.test.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.test.ts @@ -6,6 +6,7 @@ import { collaborationModeFor, DEFAULT_EFFORTS, modeApprovalPolicy, + resolveInitialMode, sandboxPolicyFor, } from "./session-config"; @@ -106,6 +107,19 @@ describe("collaborationModeFor", () => { }); }); +describe("resolveInitialMode", () => { + it.each([ + ["read-only", "read-only"], + ["auto", "auto"], + ["full-access", "full-access"], + ["bypassPermissions", "full-access"], + ["default", "auto"], + [undefined, "auto"], + ])("maps initial permission mode %s to %s", (mode, expected) => { + expect(resolveInitialMode(mode)).toBe(expected); + }); +}); + describe("buildConfigOptions", () => { const byCategory = ( opts: ReturnType, diff --git a/packages/agent/src/adapters/codex-app-server/session-config.ts b/packages/agent/src/adapters/codex-app-server/session-config.ts index f64931874e..76b8488d2b 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.ts @@ -122,9 +122,11 @@ export function collaborationModeFor( /** * Resolve the host's initial `_meta.permissionMode` to a codex mode. A recognized - * mode is honored; anything else (e.g. "bypassPermissions") falls back to default. + * mode is honored; Claude's bypass mode maps to Codex full access. Other unknown + * modes fall back to default. */ export function resolveInitialMode(permissionMode: string | undefined): string { + if (permissionMode === "bypassPermissions") return "full-access"; return permissionMode && CODEX_MODES.some((m) => m.id === permissionMode) ? permissionMode : DEFAULT_MODE; diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index 84137ab12f..7a2b0ac073 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -4029,7 +4029,7 @@ export class SessionService { this.d.store.updateSession(session.taskRunId, { configOptions: updatedOptions, }); - this.d.updatePersistedConfigOptionValue(session.taskRunId, configId, value); + this.d.setPersistedConfigOptions(session.taskRunId, updatedOptions); if ( !session.isCloud && @@ -4063,11 +4063,7 @@ export class SessionService { this.d.store.updateSession(session.taskRunId, { configOptions: rolledBackOptions, }); - this.d.updatePersistedConfigOptionValue( - session.taskRunId, - configId, - String(previousValue), - ); + this.d.setPersistedConfigOptions(session.taskRunId, rolledBackOptions); this.d.log.error("Failed to set session config option", { taskId, configId, @@ -4483,6 +4479,27 @@ export class SessionService { runState?: Record, ): () => void { const taskRunId = runId; + const persistedConfigOptions = this.d.getPersistedConfigOptions(taskRunId); + const buildInitialConfigOptions = ( + mode: string | undefined, + ): SessionConfigOption[] => { + const defaults = addMissingCloudRuntimeConfigOptions( + buildCloudDefaultConfigOptions(mode, adapter), + adapter, + initialModel, + initialReasoningEffort, + ); + if (!persistedConfigOptions?.length) return defaults; + + const defaultIds = new Set(defaults.map((option) => option.id)); + const completeOptions = [ + ...defaults, + ...persistedConfigOptions.filter( + (option) => !defaultIds.has(option.id), + ), + ]; + return mergeConfigOptions(completeOptions, persistedConfigOptions); + }; if (this.supersededRunIds.has(runId)) return () => {}; @@ -4512,12 +4529,7 @@ export class SessionService { if (shouldRefreshConfigOptions) { this.d.store.updateSession(existing.taskRunId, { adapter, - configOptions: addMissingCloudRuntimeConfigOptions( - buildCloudDefaultConfigOptions(currentMode, adapter), - adapter, - initialModel, - initialReasoningEffort, - ), + configOptions: buildInitialConfigOptions(currentMode), }); } else { const configOptions = addMissingCloudRuntimeConfigOptions( @@ -4613,12 +4625,7 @@ export class SessionService { session.status = "disconnected"; session.isCloud = true; session.adapter = adapter; - session.configOptions = addMissingCloudRuntimeConfigOptions( - buildCloudDefaultConfigOptions(initialMode, adapter), - adapter, - initialModel, - initialReasoningEffort, - ); + session.configOptions = buildInitialConfigOptions(initialMode); this.d.store.setSession(session); // Optimistic seeding for the initial task description is deferred // until `hydrateCloudTaskSessionFromLogs` confirms there's no prior @@ -4636,12 +4643,7 @@ export class SessionService { )?.currentValue; const currentMode = typeof existingMode === "string" ? existingMode : initialMode; - updates.configOptions = addMissingCloudRuntimeConfigOptions( - buildCloudDefaultConfigOptions(currentMode, adapter), - adapter, - initialModel, - initialReasoningEffort, - ); + updates.configOptions = buildInitialConfigOptions(currentMode); } else { const configOptions = addMissingCloudRuntimeConfigOptions( existing.configOptions, diff --git a/packages/ui/src/features/sessions/sessionServiceHost.test.ts b/packages/ui/src/features/sessions/sessionServiceHost.test.ts index c52491d785..98af485274 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.test.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.test.ts @@ -189,7 +189,9 @@ vi.mock("@features/sessions/stores/modelsStore", () => ({ })); const mockSessionConfigStore = vi.hoisted(() => ({ - getPersistedConfigOptions: vi.fn(() => undefined), + getPersistedConfigOptions: vi.fn< + (taskRunId: string) => SessionConfigOption[] | undefined + >(() => undefined), setPersistedConfigOptions: vi.fn(), removePersistedConfigOptions: vi.fn(), updatePersistedConfigOptionValue: vi.fn(), @@ -320,7 +322,8 @@ vi.mock("@posthog/shared", async (importOriginal) => ({ ...(await importOriginal()), getCloudUrlFromRegion: () => "https://api.anthropic.com", getConfigOptionByCategory: mockGetConfigOptionByCategory, - mergeConfigOptions: vi.fn((live: unknown[], _persisted: unknown[]) => live), + mergeConfigOptions: (await importOriginal()) + .mergeConfigOptions, flattenSelectOptions: vi.fn( (options: Array<{ options?: unknown[] }> | undefined) => { if (!options?.length) return []; @@ -979,6 +982,75 @@ describe("SessionService", () => { ); }); + it("keeps full-access after changing tasks and rebuilding the cloud session", async () => { + let persistedConfigOptions: SessionConfigOption[] | undefined; + mockSessionConfigStore.setPersistedConfigOptions.mockImplementation( + (_taskRunId, options) => { + persistedConfigOptions = options; + }, + ); + mockSessionConfigStore.getPersistedConfigOptions.mockImplementation( + () => persistedConfigOptions, + ); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue( + createMockSession({ + isCloud: true, + adapter: "codex", + cloudStatus: "in_progress", + configOptions: [ + { + id: "mode", + name: "Approval Preset", + type: "select", + category: "mode", + currentValue: "auto", + options: [], + }, + ], + }), + ); + mockTrpcCloudTask.sendCommand.mutate.mockResolvedValue({ success: true }); + + const initialService = getSessionService(); + await initialService.setSessionConfigOption( + "task-123", + "mode", + "full-access", + ); + expect(persistedConfigOptions).toEqual([ + expect.objectContaining({ + id: "mode", + currentValue: "full-access", + }), + ]); + + resetSessionService(); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(undefined); + mockSessionStoreSetters.setSession.mockClear(); + const restoredService = getSessionService(); + restoredService.watchCloudTask( + "task-123", + "run-123", + "https://api.example.com", + 123, + undefined, + undefined, + "auto", + "codex", + ); + + expect(mockSessionStoreSetters.setSession).toHaveBeenCalledWith( + expect.objectContaining({ + configOptions: expect.arrayContaining([ + expect.objectContaining({ + id: "mode", + currentValue: "full-access", + }), + ]), + }), + ); + }); + it("shows the selected cloud model and reasoning before preview config loads", () => { const service = getSessionService(); @@ -6549,8 +6621,13 @@ describe("SessionService", () => { }, ); expect( - mockSessionConfigStore.updatePersistedConfigOptionValue, - ).toHaveBeenCalledWith("run-123", "model", "claude-3-sonnet"); + mockSessionConfigStore.setPersistedConfigOptions, + ).toHaveBeenCalledWith("run-123", [ + expect.objectContaining({ + id: "model", + currentValue: "claude-3-sonnet", + }), + ]); expect(mockTrpcAgent.setConfigOption.mutate).toHaveBeenCalledWith({ sessionId: "run-123", configId: "model", @@ -6597,8 +6674,13 @@ describe("SessionService", () => { }, ); expect( - mockSessionConfigStore.updatePersistedConfigOptionValue, - ).toHaveBeenLastCalledWith("run-123", "mode", "default"); + mockSessionConfigStore.setPersistedConfigOptions, + ).toHaveBeenLastCalledWith("run-123", [ + expect.objectContaining({ + id: "mode", + currentValue: "default", + }), + ]); }); it("skips backend call when local session is idle-killed so reconnect restore handles it", async () => { @@ -6640,8 +6722,13 @@ describe("SessionService", () => { }, ); expect( - mockSessionConfigStore.updatePersistedConfigOptionValue, - ).toHaveBeenCalledWith("run-123", "mode", "acceptEdits"); + mockSessionConfigStore.setPersistedConfigOptions, + ).toHaveBeenCalledWith("run-123", [ + expect.objectContaining({ + id: "mode", + currentValue: "acceptEdits", + }), + ]); }); it("skips backend call when local session is reconnecting (disconnected status)", async () => { diff --git a/packages/workspace-server/src/services/agent/agent.test.ts b/packages/workspace-server/src/services/agent/agent.test.ts index 37374a1bc6..f918b98734 100644 --- a/packages/workspace-server/src/services/agent/agent.test.ts +++ b/packages/workspace-server/src/services/agent/agent.test.ts @@ -16,12 +16,42 @@ const mockNewSession = vi.hoisted(() => }), ); +const mockAcpClient = vi.hoisted(() => ({ + current: undefined as + | { + requestPermission: (params: { + options: Array<{ optionId: string; kind: string; name: string }>; + toolCall?: { toolCallId?: string; title?: string }; + }) => Promise; + } + | undefined, +})); + const mockClientSideConnection = vi.hoisted(() => - vi.fn().mockImplementation(function (this: Record) { + vi.fn().mockImplementation(function ( + this: Record, + clientFactory: (agent: unknown) => typeof mockAcpClient.current, + ) { + mockAcpClient.current = clientFactory({}); this.initialize = vi.fn().mockResolvedValue({}); this.newSession = mockNewSession; this.loadSession = vi.fn().mockResolvedValue({ configOptions: [] }); this.resumeSession = vi.fn().mockResolvedValue({ configOptions: [] }); + this.setSessionConfigOption = vi.fn( + async ({ value }: { value: string }) => ({ + configOptions: [ + { + id: "mode", + name: "Mode", + description: "Permission mode", + category: "mode", + type: "select", + currentValue: value, + options: [], + }, + ], + }), + ); }), ); @@ -100,7 +130,12 @@ vi.mock("node:fs", async (importOriginal) => { // --- Import after mocks --- import type { RegisteredFolder } from "../folders/schemas"; -import { AgentService, buildAutoApproveOutcome } from "./agent"; +import { + AgentService, + buildAutoApproveOutcome, + shouldAutoApprovePermissionRequest, +} from "./agent"; +import { AgentServiceEvent } from "./schemas"; // --- Test helpers --- @@ -377,6 +412,37 @@ describe("AgentService", () => { }); }); + describe("permission requests", () => { + it("auto-approves after switching a live Codex session to full access", async () => { + await service.startSession({ + ...baseSessionParams, + adapter: "codex", + permissionMode: "auto", + }); + + await service.setSessionConfigOption("run-1", "mode", "full-access"); + const response = await mockAcpClient.current?.requestPermission({ + toolCall: { + toolCallId: "tool-call-1", + title: "Run command", + }, + options: [ + { optionId: "reject", kind: "reject_once", name: "Reject" }, + { optionId: "allow", kind: "allow_once", name: "Allow" }, + ], + }); + + expect(response).toEqual({ + outcome: { outcome: "selected", optionId: "allow" }, + }); + expect(service.emit).not.toHaveBeenCalledWith( + AgentServiceEvent.PermissionRequest, + expect.anything(), + ); + expect(deps.sleepService.release).not.toHaveBeenCalledWith("run-1"); + }); + }); + describe("idle timeout", () => { function injectSession( svc: AgentService, @@ -715,3 +781,18 @@ describe("buildAutoApproveOutcome", () => { expect(buildAutoApproveOutcome([])).toEqual({ outcome: "cancelled" }); }); }); + +describe("shouldAutoApprovePermissionRequest", () => { + it.each([ + ["codex", "full-access", true], + ["codex", "bypassPermissions", true], + ["codex", "auto", false], + ["codex", "read-only", false], + ["claude", "bypassPermissions", false], + [undefined, "full-access", false], + ])("adapter %s in mode %s => %s", (adapter, permissionMode, expected) => { + expect(shouldAutoApprovePermissionRequest(adapter, permissionMode)).toBe( + expected, + ); + }); +}); diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index f79648cf89..02124799fa 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -349,6 +349,16 @@ export function buildAutoApproveOutcome( return { outcome: "selected", optionId }; } +export function shouldAutoApprovePermissionRequest( + adapter: string | undefined, + permissionMode: string | undefined, +): boolean { + return ( + adapter === "codex" && + (permissionMode === "full-access" || permissionMode === "bypassPermissions") + ); +} + interface PendingPermission { resolve: (response: RequestPermissionResponse) => void; reject: (error: Error) => void; @@ -1715,8 +1725,21 @@ For git operations while detached: optionCount: params.options.length, }); + const session = service.sessions.get(taskRunId); + if ( + shouldAutoApprovePermissionRequest( + session?.config.adapter, + session?.config.permissionMode, + ) + ) { + service.log.info("Auto-approving Codex full-access permission", { + taskRunId, + toolCallId, + }); + return { outcome: buildAutoApproveOutcome(params.options) }; + } + if (toolName && isMcpToolReadOnly(toolName)) { - const session = service.sessions.get(taskRunId); const approvalState = session?.mcpToolApprovals?.[toolName]; if (approvalState === "approved") { service.log.info("Auto-approving read-only MCP tool", { From 18216515ca80427b0d8af22468b94c3c572a22ea Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 15 Jul 2026 15:52:02 +0300 Subject: [PATCH 2/5] fix(agent): guard full access permission handling Generated-By: PostHog Code Task-Id: 1f1fe07c-6384-4d1e-8ab9-bad3700caa2e --- .../codex-app-server/session-config.ts | 16 +- packages/core/src/sessions/sessionService.ts | 28 ++-- .../features/sessions/sessionConfigStore.ts | 33 ---- ...onServiceHost.recovery.integration.test.ts | 1 - .../sessions/sessionServiceHost.test.ts | 149 ++++++++++++++---- .../features/sessions/sessionServiceHost.ts | 2 - .../src/services/agent/agent.test.ts | 73 +++++++-- .../src/services/agent/agent.ts | 16 +- 8 files changed, 224 insertions(+), 94 deletions(-) diff --git a/packages/agent/src/adapters/codex-app-server/session-config.ts b/packages/agent/src/adapters/codex-app-server/session-config.ts index 76b8488d2b..9000610068 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.ts @@ -1,5 +1,10 @@ import type { SessionConfigOption } from "@agentclientprotocol/sdk"; -import { CODEX_MODE_PRESETS, type CodexModePreset } from "@posthog/shared"; +import { + CODEX_MODE_PRESETS, + type CodexModePreset, + type ExecutionMode, + resolveCloudInitialPermissionMode, +} from "@posthog/shared"; import { type GatewayModel, isOpenAIModel } from "../../gateway-models"; import { getReasoningEffortOptions } from "./models"; @@ -126,10 +131,11 @@ export function collaborationModeFor( * modes fall back to default. */ export function resolveInitialMode(permissionMode: string | undefined): string { - if (permissionMode === "bypassPermissions") return "full-access"; - return permissionMode && CODEX_MODES.some((m) => m.id === permissionMode) - ? permissionMode - : DEFAULT_MODE; + if (!permissionMode) return DEFAULT_MODE; + return resolveCloudInitialPermissionMode( + "codex", + permissionMode as ExecutionMode, + ); } /** Codex's standard reasoning efforts; used when model/list doesn't expose them. */ diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index 7a2b0ac073..7271facee4 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -324,7 +324,6 @@ export interface SessionServiceDeps { options: SessionConfigOption[], ) => void; removePersistedConfigOptions: (taskRunId: string) => void; - updatePersistedConfigOptionValue: (...args: any[]) => any; adapterStore: { getAdapter(taskRunId: string): Adapter | undefined; setAdapter(taskRunId: string, adapter: Adapter): void; @@ -4054,16 +4053,25 @@ export class SessionService { }); } } catch (error) { - // Rollback on error - const rolledBackOptions = configOptions.map((opt) => - opt.id === configId - ? ({ ...opt, currentValue: previousValue } as SessionConfigOption) - : opt, + const latestConfigOptions = + this.d.store.getSessionByTaskId(taskId)?.configOptions ?? []; + const latestOption = latestConfigOptions.find( + (option) => option.id === configId, ); - this.d.store.updateSession(session.taskRunId, { - configOptions: rolledBackOptions, - }); - this.d.setPersistedConfigOptions(session.taskRunId, rolledBackOptions); + if (latestOption?.currentValue === value) { + const rolledBackOptions = latestConfigOptions.map((option) => + option.id === configId + ? ({ + ...option, + currentValue: previousValue, + } as SessionConfigOption) + : option, + ); + this.d.store.updateSession(session.taskRunId, { + configOptions: rolledBackOptions, + }); + this.d.setPersistedConfigOptions(session.taskRunId, rolledBackOptions); + } this.d.log.error("Failed to set session config option", { taskId, configId, diff --git a/packages/ui/src/features/sessions/sessionConfigStore.ts b/packages/ui/src/features/sessions/sessionConfigStore.ts index f88f0ad425..45bc88d887 100644 --- a/packages/ui/src/features/sessions/sessionConfigStore.ts +++ b/packages/ui/src/features/sessions/sessionConfigStore.ts @@ -15,12 +15,6 @@ interface SessionConfigActions { getConfigOptions: (taskRunId: string) => SessionConfigOption[] | undefined; /** Remove config options for a task run */ removeConfigOptions: (taskRunId: string) => void; - /** Update a single config option value */ - updateConfigOptionValue: ( - taskRunId: string, - configId: string, - value: string, - ) => void; } type SessionConfigStore = SessionConfigState & SessionConfigActions; @@ -42,22 +36,6 @@ export const useSessionConfigStore = create()( const { [taskRunId]: _removed, ...rest } = state.configsByRunId; return { configsByRunId: rest }; }), - - updateConfigOptionValue: (taskRunId, configId, value) => - set((state) => { - const existing = state.configsByRunId[taskRunId]; - if (!existing) return state; - - const updated = existing.map((opt) => - opt.id === configId - ? ({ ...opt, currentValue: value } as SessionConfigOption) - : opt, - ); - - return { - configsByRunId: { ...state.configsByRunId, [taskRunId]: updated }, - }; - }), }), { name: "session-config-storage", @@ -86,14 +64,3 @@ export function setPersistedConfigOptions( export function removePersistedConfigOptions(taskRunId: string): void { useSessionConfigStore.getState().removeConfigOptions(taskRunId); } - -/** Non-hook accessor for updating a single config option value */ -export function updatePersistedConfigOptionValue( - taskRunId: string, - configId: string, - value: string, -): void { - useSessionConfigStore - .getState() - .updateConfigOptionValue(taskRunId, configId, value); -} diff --git a/packages/ui/src/features/sessions/sessionServiceHost.recovery.integration.test.ts b/packages/ui/src/features/sessions/sessionServiceHost.recovery.integration.test.ts index 226a38a295..422203c6b1 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.recovery.integration.test.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.recovery.integration.test.ts @@ -127,7 +127,6 @@ const mockSessionConfigStore = vi.hoisted(() => ({ getPersistedConfigOptions: vi.fn(() => undefined), setPersistedConfigOptions: vi.fn(), removePersistedConfigOptions: vi.fn(), - updatePersistedConfigOptionValue: vi.fn(), })); vi.mock( diff --git a/packages/ui/src/features/sessions/sessionServiceHost.test.ts b/packages/ui/src/features/sessions/sessionServiceHost.test.ts index 98af485274..0cfaf79522 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.test.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.test.ts @@ -194,7 +194,6 @@ const mockSessionConfigStore = vi.hoisted(() => ({ >(() => undefined), setPersistedConfigOptions: vi.fn(), removePersistedConfigOptions: vi.fn(), - updatePersistedConfigOptionValue: vi.fn(), })); vi.mock( @@ -1006,6 +1005,14 @@ describe("SessionService", () => { currentValue: "auto", options: [], }, + { + id: "model", + name: "Model", + type: "select", + category: "model", + currentValue: "gpt-5.5", + options: [], + }, ], }), ); @@ -1022,6 +1029,10 @@ describe("SessionService", () => { id: "mode", currentValue: "full-access", }), + expect.objectContaining({ + id: "model", + currentValue: "gpt-5.5", + }), ]); resetSessionService(); @@ -1046,9 +1057,19 @@ describe("SessionService", () => { id: "mode", currentValue: "full-access", }), + expect.objectContaining({ + id: "model", + currentValue: "gpt-5.5", + }), ]), }), ); + + mockSessionConfigStore.getPersistedConfigOptions.mockReset(); + mockSessionConfigStore.getPersistedConfigOptions.mockReturnValue( + undefined, + ); + mockSessionConfigStore.setPersistedConfigOptions.mockReset(); }); it("shows the selected cloud model and reasoning before preview config loads", () => { @@ -6637,19 +6658,25 @@ describe("SessionService", () => { it("rolls back on API failure", async () => { const service = getSessionService(); - mockSessionStoreSetters.getSessionByTaskId.mockReturnValue( - createMockSession({ - configOptions: [ - { - id: "mode", - name: "Mode", - type: "select", - category: "mode", - currentValue: "default", - options: [], - }, - ], - }), + let currentSession = createMockSession({ + configOptions: [ + { + id: "mode", + name: "Mode", + type: "select", + category: "mode", + currentValue: "default", + options: [], + }, + ], + }); + mockSessionStoreSetters.getSessionByTaskId.mockImplementation( + () => currentSession, + ); + mockSessionStoreSetters.updateSession.mockImplementation( + (_taskRunId, updates) => { + currentSession = { ...currentSession, ...updates }; + }, ); mockTrpcAgent.setConfigOption.mutate.mockRejectedValue( new Error("Failed"), @@ -6657,22 +6684,84 @@ describe("SessionService", () => { await service.setSessionConfigOption("task-123", "mode", "acceptEdits"); - // Should rollback - expect(mockSessionStoreSetters.updateSession).toHaveBeenLastCalledWith( - "run-123", - { - configOptions: [ - { - id: "mode", - name: "Mode", - type: "select", - category: "mode", - currentValue: "default", - options: [], - }, - ], + expect(currentSession.configOptions).toEqual([ + expect.objectContaining({ + id: "mode", + currentValue: "default", + }), + ]); + expect( + mockSessionConfigStore.setPersistedConfigOptions, + ).toHaveBeenLastCalledWith("run-123", [ + expect.objectContaining({ + id: "mode", + currentValue: "default", + }), + ]); + }); + + it("preserves a newer successful config change during rollback", async () => { + const service = getSessionService(); + let currentSession = createMockSession({ + configOptions: [ + { + id: "mode", + name: "Mode", + type: "select", + category: "mode", + currentValue: "default", + options: [], + }, + { + id: "model", + name: "Model", + type: "select", + category: "model", + currentValue: "claude-3-opus", + options: [], + }, + ], + }); + mockSessionStoreSetters.getSessionByTaskId.mockImplementation( + () => currentSession, + ); + mockSessionStoreSetters.updateSession.mockImplementation( + (_taskRunId, updates) => { + currentSession = { ...currentSession, ...updates }; }, ); + + let rejectModeChange: (error: Error) => void = () => undefined; + const pendingModeChange = new Promise((_resolve, reject) => { + rejectModeChange = reject; + }); + mockTrpcAgent.setConfigOption.mutate.mockImplementation(({ configId }) => + configId === "mode" ? pendingModeChange : Promise.resolve({}), + ); + + const modeChange = service.setSessionConfigOption( + "task-123", + "mode", + "acceptEdits", + ); + await service.setSessionConfigOption( + "task-123", + "model", + "claude-3-sonnet", + ); + rejectModeChange(new Error("Mode change failed")); + await modeChange; + + expect(currentSession.configOptions).toEqual([ + expect.objectContaining({ + id: "mode", + currentValue: "default", + }), + expect.objectContaining({ + id: "model", + currentValue: "claude-3-sonnet", + }), + ]); expect( mockSessionConfigStore.setPersistedConfigOptions, ).toHaveBeenLastCalledWith("run-123", [ @@ -6680,6 +6769,10 @@ describe("SessionService", () => { id: "mode", currentValue: "default", }), + expect.objectContaining({ + id: "model", + currentValue: "claude-3-sonnet", + }), ]); }); diff --git a/packages/ui/src/features/sessions/sessionServiceHost.ts b/packages/ui/src/features/sessions/sessionServiceHost.ts index 06971d2579..44bfc7a9f7 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.ts @@ -29,7 +29,6 @@ import { getPersistedConfigOptions, removePersistedConfigOptions, setPersistedConfigOptions, - updatePersistedConfigOptionValue, } from "@posthog/ui/features/sessions/sessionConfigStore"; import { sessionStoreSetters } from "@posthog/ui/features/sessions/sessionStore"; import { @@ -105,7 +104,6 @@ function buildSessionServiceDeps(): SessionServiceDeps { getPersistedConfigOptions(taskRunId) ?? undefined, setPersistedConfigOptions, removePersistedConfigOptions, - updatePersistedConfigOptionValue, adapterStore: { getAdapter: (taskRunId) => useSessionAdapterStore.getState().getAdapter(taskRunId), diff --git a/packages/workspace-server/src/services/agent/agent.test.ts b/packages/workspace-server/src/services/agent/agent.test.ts index f918b98734..7d479b71df 100644 --- a/packages/workspace-server/src/services/agent/agent.test.ts +++ b/packages/workspace-server/src/services/agent/agent.test.ts @@ -21,7 +21,11 @@ const mockAcpClient = vi.hoisted(() => ({ | { requestPermission: (params: { options: Array<{ optionId: string; kind: string; name: string }>; - toolCall?: { toolCallId?: string; title?: string }; + toolCall?: { + toolCallId?: string; + title?: string; + _meta?: { codeToolKind?: string }; + }; }) => Promise; } | undefined, @@ -421,7 +425,7 @@ describe("AgentService", () => { }); await service.setSessionConfigOption("run-1", "mode", "full-access"); - const response = await mockAcpClient.current?.requestPermission({ + const responsePromise = mockAcpClient.current?.requestPermission({ toolCall: { toolCallId: "tool-call-1", title: "Run command", @@ -432,6 +436,8 @@ describe("AgentService", () => { ], }); + expect(service.getDebugSnapshot().pendingPermissions).toEqual([]); + const response = await responsePromise; expect(response).toEqual({ outcome: { outcome: "selected", optionId: "allow" }, }); @@ -441,6 +447,39 @@ describe("AgentService", () => { ); expect(deps.sleepService.release).not.toHaveBeenCalledWith("run-1"); }); + + it("still prompts for structured user questions in full access", async () => { + await service.startSession({ + ...baseSessionParams, + adapter: "codex", + permissionMode: "full-access", + }); + + const responsePromise = mockAcpClient.current?.requestPermission({ + toolCall: { + toolCallId: "question-1", + title: "Which one?", + _meta: { codeToolKind: "question" }, + }, + options: [ + { optionId: "option_0", kind: "allow_once", name: "A" }, + { optionId: "option_1", kind: "allow_once", name: "B" }, + ], + }); + + expect(service.getDebugSnapshot().pendingPermissions).toEqual([ + { taskRunId: "run-1", toolCallId: "question-1" }, + ]); + expect(service.emit).toHaveBeenCalledWith( + AgentServiceEvent.PermissionRequest, + expect.objectContaining({ taskRunId: "run-1" }), + ); + + service.cancelPermission("run-1", "question-1"); + await expect(responsePromise).resolves.toEqual({ + outcome: { outcome: "cancelled" }, + }); + }); }); describe("idle timeout", () => { @@ -784,15 +823,23 @@ describe("buildAutoApproveOutcome", () => { describe("shouldAutoApprovePermissionRequest", () => { it.each([ - ["codex", "full-access", true], - ["codex", "bypassPermissions", true], - ["codex", "auto", false], - ["codex", "read-only", false], - ["claude", "bypassPermissions", false], - [undefined, "full-access", false], - ])("adapter %s in mode %s => %s", (adapter, permissionMode, expected) => { - expect(shouldAutoApprovePermissionRequest(adapter, permissionMode)).toBe( - expected, - ); - }); + ["codex", "full-access", undefined, true], + ["codex", "bypassPermissions", undefined, true], + ["codex", "full-access", "question", false], + ["codex", "auto", undefined, false], + ["codex", "read-only", undefined, false], + ["claude", "bypassPermissions", undefined, false], + [undefined, "full-access", undefined, false], + ])( + "adapter %s in mode %s for %s => %s", + (adapter, permissionMode, codeToolKind, expected) => { + expect( + shouldAutoApprovePermissionRequest( + adapter, + permissionMode, + codeToolKind, + ), + ).toBe(expected); + }, + ); }); diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index 02124799fa..dfb440287c 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -67,7 +67,9 @@ import { import { type AcpMessage, type Adapter, + type ExecutionMode, isAuthError, + resolveCloudInitialPermissionMode, serializeError, TypedEventEmitter, } from "@posthog/shared"; @@ -352,10 +354,16 @@ export function buildAutoApproveOutcome( export function shouldAutoApprovePermissionRequest( adapter: string | undefined, permissionMode: string | undefined, + codeToolKind?: string, ): boolean { + if (adapter !== "codex" || !permissionMode || codeToolKind === "question") { + return false; + } return ( - adapter === "codex" && - (permissionMode === "full-access" || permissionMode === "bypassPermissions") + resolveCloudInitialPermissionMode( + "codex", + permissionMode as ExecutionMode, + ) === "full-access" ); } @@ -1716,6 +1724,9 @@ For git operations while detached: (params.toolCall?.rawInput as { toolName?: string } | undefined) ?.toolName || ""; const toolCallId = params.toolCall?.toolCallId || ""; + const codeToolKind = ( + params.toolCall?._meta as { codeToolKind?: string } | undefined + )?.codeToolKind; service.log.info("requestPermission called", { taskRunId, @@ -1730,6 +1741,7 @@ For git operations while detached: shouldAutoApprovePermissionRequest( session?.config.adapter, session?.config.permissionMode, + codeToolKind, ) ) { service.log.info("Auto-approving Codex full-access permission", { From 33783a08ca3a9a49255dac334372938bbd3479a8 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 15 Jul 2026 15:57:13 +0300 Subject: [PATCH 3/5] fix(agent): address permission review feedback Generated-By: PostHog Code Task-Id: 1f1fe07c-6384-4d1e-8ab9-bad3700caa2e --- .../codex-app-server/session-config.test.ts | 16 ++++ .../codex-app-server/session-config.ts | 2 +- packages/core/src/sessions/sessionService.ts | 19 ++++- .../sessions/sessionServiceHost.test.ts | 81 +++++++++++++++++++ 4 files changed, 114 insertions(+), 4 deletions(-) diff --git a/packages/agent/src/adapters/codex-app-server/session-config.test.ts b/packages/agent/src/adapters/codex-app-server/session-config.test.ts index ece51251f4..79e4dbad5c 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.test.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.test.ts @@ -7,6 +7,7 @@ import { DEFAULT_EFFORTS, modeApprovalPolicy, resolveInitialMode, + SessionConfigState, sandboxPolicyFor, } from "./session-config"; @@ -120,6 +121,21 @@ describe("resolveInitialMode", () => { }); }); +describe("SessionConfigState", () => { + it("canonicalizes bypassPermissions during a live mode update", () => { + const config = new SessionConfigState("gpt-5.5"); + + config.setOption("mode", "bypassPermissions"); + + expect(config.mode).toBe("full-access"); + expect(config.approvalPolicy()).toBe("never"); + expect(config.sandboxPolicy()).toEqual({ type: "dangerFullAccess" }); + expect( + config.options.find((option) => option.category === "mode")?.currentValue, + ).toBe("full-access"); + }); +}); + describe("buildConfigOptions", () => { const byCategory = ( opts: ReturnType, diff --git a/packages/agent/src/adapters/codex-app-server/session-config.ts b/packages/agent/src/adapters/codex-app-server/session-config.ts index 9000610068..6028543275 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.ts @@ -268,7 +268,7 @@ export class SessionConfigState { if (configId === "model") this._model = value; else if (configId === "effort") this._effort = value; else if (configId === "mode") { - this._mode = value; + this._mode = resolveInitialMode(value); modeChanged = true; } } diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index 7271facee4..8c29049924 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -4488,8 +4488,10 @@ export class SessionService { ): () => void { const taskRunId = runId; const persistedConfigOptions = this.d.getPersistedConfigOptions(taskRunId); + const persistedAdapter = this.d.adapterStore.getAdapter(taskRunId); const buildInitialConfigOptions = ( mode: string | undefined, + configAdapter: Adapter | undefined = persistedAdapter, ): SessionConfigOption[] => { const defaults = addMissingCloudRuntimeConfigOptions( buildCloudDefaultConfigOptions(mode, adapter), @@ -4498,6 +4500,7 @@ export class SessionService { initialReasoningEffort, ); if (!persistedConfigOptions?.length) return defaults; + if (configAdapter && configAdapter !== adapter) return defaults; const defaultIds = new Set(defaults.map((option) => option.id)); const completeOptions = [ @@ -4510,6 +4513,7 @@ export class SessionService { }; if (this.supersededRunIds.has(runId)) return () => {}; + this.d.adapterStore.setAdapter(taskRunId, adapter); const existingWatcher = this.cloudTaskWatchers.get(taskId); @@ -4537,7 +4541,10 @@ export class SessionService { if (shouldRefreshConfigOptions) { this.d.store.updateSession(existing.taskRunId, { adapter, - configOptions: buildInitialConfigOptions(currentMode), + configOptions: buildInitialConfigOptions( + currentMode, + existing.adapter, + ), }); } else { const configOptions = addMissingCloudRuntimeConfigOptions( @@ -4633,7 +4640,10 @@ export class SessionService { session.status = "disconnected"; session.isCloud = true; session.adapter = adapter; - session.configOptions = buildInitialConfigOptions(initialMode); + session.configOptions = buildInitialConfigOptions( + initialMode, + existing?.taskRunId === taskRunId ? existing.adapter : persistedAdapter, + ); this.d.store.setSession(session); // Optimistic seeding for the initial task description is deferred // until `hydrateCloudTaskSessionFromLogs` confirms there's no prior @@ -4651,7 +4661,10 @@ export class SessionService { )?.currentValue; const currentMode = typeof existingMode === "string" ? existingMode : initialMode; - updates.configOptions = buildInitialConfigOptions(currentMode); + updates.configOptions = buildInitialConfigOptions( + currentMode, + existing.adapter, + ); } else { const configOptions = addMissingCloudRuntimeConfigOptions( existing.configOptions, diff --git a/packages/ui/src/features/sessions/sessionServiceHost.test.ts b/packages/ui/src/features/sessions/sessionServiceHost.test.ts index 0cfaf79522..c17203b38b 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.test.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.test.ts @@ -432,6 +432,8 @@ describe("SessionService", () => { mockGetConfigOptionByCategory.mockReturnValue(undefined); mockBuildAuthenticatedClient.mockReturnValue(mockAuthenticatedClient); mockAuthenticatedClient.getTaskRunSessionLogs.mockResolvedValue([]); + mockSessionConfigStore.getPersistedConfigOptions.mockReturnValue(undefined); + mockAdapterFns.getAdapter.mockReturnValue(undefined); mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(undefined); mockSessionStoreSetters.getSessions.mockReturnValue({}); mockAuth.fetchAuthState.mockResolvedValue({ @@ -1072,6 +1074,85 @@ describe("SessionService", () => { mockSessionConfigStore.setPersistedConfigOptions.mockReset(); }); + it("drops persisted options when the cloud adapter changes", () => { + const service = getSessionService(); + mockAdapterFns.getAdapter.mockReturnValue("claude"); + mockSessionConfigStore.getPersistedConfigOptions.mockReturnValue([ + { + id: "mode", + name: "Mode", + type: "select", + category: "mode", + currentValue: "acceptEdits", + options: [], + }, + { + id: "model", + name: "Model", + type: "select", + category: "model", + currentValue: "claude-opus-4-8", + options: [], + }, + { + id: "effort", + name: "Effort", + type: "select", + category: "thought_level", + currentValue: "high", + options: [], + }, + ]); + + service.watchCloudTask( + "task-adapter-change", + "run-adapter-change", + "https://api.example.com", + 123, + undefined, + undefined, + "auto", + "codex", + "gpt-5.5", + undefined, + undefined, + undefined, + "max", + ); + + expect(mockSessionStoreSetters.setSession).toHaveBeenCalledWith( + expect.objectContaining({ + adapter: "codex", + configOptions: expect.arrayContaining([ + expect.objectContaining({ + category: "mode", + currentValue: "auto", + }), + expect.objectContaining({ + category: "model", + currentValue: "gpt-5.5", + }), + expect.objectContaining({ + category: "thought_level", + currentValue: "max", + }), + ]), + }), + ); + const session = mockSessionStoreSetters.setSession.mock.calls.at(-1)?.[0]; + expect(session?.configOptions).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ currentValue: "acceptEdits" }), + expect.objectContaining({ currentValue: "claude-opus-4-8" }), + expect.objectContaining({ id: "effort" }), + ]), + ); + expect(mockAdapterFns.setAdapter).toHaveBeenCalledWith( + "run-adapter-change", + "codex", + ); + }); + it("shows the selected cloud model and reasoning before preview config loads", () => { const service = getSessionService(); From 8136c9cfcb7e2f1760db63e45add6c9cafbeefcb Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 15 Jul 2026 16:21:38 +0300 Subject: [PATCH 4/5] refactor(agent): rename codex mode resolver Generated-By: PostHog Code Task-Id: 1f1fe07c-6384-4d1e-8ab9-bad3700caa2e --- .../codex-app-server/session-config.test.ts | 8 ++++---- .../codex-app-server/session-config.ts | 19 ++++++++----------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/agent/src/adapters/codex-app-server/session-config.test.ts b/packages/agent/src/adapters/codex-app-server/session-config.test.ts index 79e4dbad5c..d476904096 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.test.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.test.ts @@ -6,7 +6,7 @@ import { collaborationModeFor, DEFAULT_EFFORTS, modeApprovalPolicy, - resolveInitialMode, + resolveCodexMode, SessionConfigState, sandboxPolicyFor, } from "./session-config"; @@ -108,7 +108,7 @@ describe("collaborationModeFor", () => { }); }); -describe("resolveInitialMode", () => { +describe("resolveCodexMode", () => { it.each([ ["read-only", "read-only"], ["auto", "auto"], @@ -116,8 +116,8 @@ describe("resolveInitialMode", () => { ["bypassPermissions", "full-access"], ["default", "auto"], [undefined, "auto"], - ])("maps initial permission mode %s to %s", (mode, expected) => { - expect(resolveInitialMode(mode)).toBe(expected); + ])("maps host mode %s to codex mode %s", (mode, expected) => { + expect(resolveCodexMode(mode)).toBe(expected); }); }); diff --git a/packages/agent/src/adapters/codex-app-server/session-config.ts b/packages/agent/src/adapters/codex-app-server/session-config.ts index 6028543275..6070ca7e27 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.ts @@ -126,16 +126,13 @@ export function collaborationModeFor( } /** - * Resolve the host's initial `_meta.permissionMode` to a codex mode. A recognized - * mode is honored; Claude's bypass mode maps to Codex full access. Other unknown - * modes fall back to default. + * Resolve a host permission mode or live picker value to a codex mode. A + * recognized mode is honored; Claude's bypass mode maps to Codex full access. + * Other unknown modes fall back to default. */ -export function resolveInitialMode(permissionMode: string | undefined): string { - if (!permissionMode) return DEFAULT_MODE; - return resolveCloudInitialPermissionMode( - "codex", - permissionMode as ExecutionMode, - ); +export function resolveCodexMode(mode: string | undefined): string { + if (!mode) return DEFAULT_MODE; + return resolveCloudInitialPermissionMode("codex", mode as ExecutionMode); } /** Codex's standard reasoning efforts; used when model/list doesn't expose them. */ @@ -254,7 +251,7 @@ export class SessionConfigState { /** Apply the host's initial approval mode (from `_meta.permissionMode`). */ setInitialMode(permissionMode: string | undefined): void { - this._mode = resolveInitialMode(permissionMode); + this._mode = resolveCodexMode(permissionMode); this.rebuild(); } @@ -268,7 +265,7 @@ export class SessionConfigState { if (configId === "model") this._model = value; else if (configId === "effort") this._effort = value; else if (configId === "mode") { - this._mode = resolveInitialMode(value); + this._mode = resolveCodexMode(value); modeChanged = true; } } From 5e8d18b65674790c27ae2b9a1b25d11098d2aa53 Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 15 Jul 2026 16:50:46 +0300 Subject: [PATCH 5/5] test(core): provide adapter store in notification harness Generated-By: PostHog Code Task-Id: 3fe0371b-73b0-401d-9aa4-2bd5474b686f --- .../core/src/sessions/cloudTaskUpdateNotifications.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core/src/sessions/cloudTaskUpdateNotifications.test.ts b/packages/core/src/sessions/cloudTaskUpdateNotifications.test.ts index 3bbc6d0e9c..4fd5c1bc70 100644 --- a/packages/core/src/sessions/cloudTaskUpdateNotifications.test.ts +++ b/packages/core/src/sessions/cloudTaskUpdateNotifications.test.ts @@ -143,6 +143,11 @@ function createHarness() { taskViewedApi: { markActivity }, getPersistedConfigOptions: () => undefined, setPersistedConfigOptions: vi.fn(), + adapterStore: { + getAdapter: () => undefined, + setAdapter: vi.fn(), + removeAdapter: vi.fn(), + }, trpc: { agent: { onSessionIdleKilled: {