From d97576069915e90f6e8d489994664edfe49c5b5f Mon Sep 17 00:00:00 2001 From: Emre K <110906681+kocaemre@users.noreply.github.com> Date: Sat, 12 Sep 2026 04:41:22 +0200 Subject: [PATCH] fix: preserve resolved prompt permissions Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com> --- src/CodexAcpClient.ts | 25 ++++++-- src/CodexAcpServer.ts | 33 +++++++++- src/SessionFork.ts | 3 + src/SessionMetadata.ts | 5 +- .../CodexACPAgent/CodexAcpClient.test.ts | 62 +++++++++++++++++++ src/__tests__/acp-test-utils.ts | 3 + 6 files changed, 125 insertions(+), 6 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index f8fc9380..49ef016e 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -34,6 +34,8 @@ import {listedSessionTitle} from "./SessionTitle"; import type { AccountLoginCompletedNotification, AccountUpdatedNotification, + ApprovalsReviewer, + AskForApproval, GetAccountRateLimitsResponse, GetAccountResponse, ListMcpServerStatusParams, @@ -87,6 +89,9 @@ type ResumedThread = { materialized: boolean; /** The mode that the resume response reports. Null when the thread has no rollout yet. */ collaborationMode: ModeKind | null; + approvalPolicy?: AskForApproval; + approvalsReviewer?: ApprovalsReviewer; + sandbox?: SandboxPolicy; }; /** @@ -642,6 +647,9 @@ export class CodexAcpClient { itemsBackwardsCursor: response.itemsBackwardsCursor ?? null, materialized: true, collaborationMode: response.collaborationMode?.mode ?? null, + ...(response.approvalPolicy !== undefined ? {approvalPolicy: response.approvalPolicy} : {}), + ...(response.approvalsReviewer !== undefined ? {approvalsReviewer: response.approvalsReviewer} : {}), + ...(response.sandbox !== undefined ? {sandbox: response.sandbox} : {}), }; } catch (err) { if (isThreadActiveWriterError(err)) throw threadActiveWriterRequestError(params.threadId, err); @@ -696,6 +704,9 @@ export class CodexAcpClient { currentServiceTier: response.serviceTier as ServiceTier ?? null, additionalDirectories, skippedMcpServers: sessionConfig.skippedMcpServers, + ...(response.approvalPolicy !== undefined ? {approvalPolicy: response.approvalPolicy} : {}), + ...(response.approvalsReviewer !== undefined ? {approvalsReviewer: response.approvalsReviewer} : {}), + ...(response.sandbox !== undefined ? {sandboxPolicy: response.sandbox} : {}), } } @@ -759,6 +770,9 @@ export class CodexAcpClient { history, additionalDirectories, skippedMcpServers: sessionConfig.skippedMcpServers, + ...(response.approvalPolicy !== undefined ? {approvalPolicy: response.approvalPolicy} : {}), + ...(response.approvalsReviewer !== undefined ? {approvalsReviewer: response.approvalsReviewer} : {}), + ...(response.sandbox !== undefined ? {sandboxPolicy: response.sandbox} : {}), }; } @@ -815,6 +829,9 @@ export class CodexAcpClient { currentServiceTier: response.serviceTier as ServiceTier ?? null, additionalDirectories, skippedMcpServers: sessionConfig.skippedMcpServers, + ...(response.approvalPolicy !== undefined ? {approvalPolicy: response.approvalPolicy} : {}), + ...(response.approvalsReviewer !== undefined ? {approvalsReviewer: response.approvalsReviewer} : {}), + ...(response.sandbox !== undefined ? {sandboxPolicy: response.sandbox} : {}), }; } @@ -1151,7 +1168,7 @@ export class CodexAcpClient { async sendPrompt( request: acp.PromptRequest, - agentMode: AgentMode, + turnPermissionSettings: {approvalPolicy: AskForApproval, approvalsReviewer: ApprovalsReviewer, sandboxPolicy: SandboxPolicy}, modelId: ModelId, serviceTier: ServiceTier | null, disableSummary: boolean, @@ -1169,9 +1186,9 @@ export class CodexAcpClient { return await this.codexClient.runTurn({ threadId: request.sessionId, input: input, - approvalPolicy: agentMode.approvalPolicy, - approvalsReviewer: agentMode.approvalsReviewer, - sandboxPolicy: addAdditionalDirectoriesToSandboxPolicy(agentMode.sandboxPolicy, additionalDirectories), + approvalPolicy: turnPermissionSettings.approvalPolicy, + approvalsReviewer: turnPermissionSettings.approvalsReviewer, + sandboxPolicy: addAdditionalDirectoriesToSandboxPolicy(turnPermissionSettings.sandboxPolicy, additionalDirectories), summary: disableSummary ? "none" : "auto", effort: effort, model: modelId.model, diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index d3f90a8b..0a7c126d 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -40,9 +40,12 @@ import type {InputModality, ReasoningEffort, ServerNotification} from "./app-ser import type { Account, AccountUpdatedNotification, + ApprovalsReviewer, + AskForApproval, GetAccountResponse, Model, ReasoningEffortOption, + SandboxPolicy, Thread, ThreadGoal, ThreadItem, @@ -198,6 +201,9 @@ export interface SessionState extends SessionIndexTitleState { supportedReasoningEfforts: Array, supportedInputModalities: Array, agentMode: AgentMode, + resolvedApprovalPolicy: AskForApproval, + resolvedApprovalsReviewer: ApprovalsReviewer, + resolvedSandboxPolicy: SandboxPolicy, collaborationMode: ModeKind, currentTurnId: string | null; lastTokenUsage: TokenCount | null; @@ -913,6 +919,9 @@ export class CodexAcpServer { supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], agentMode: AgentMode.getInitialAgentMode(), + resolvedApprovalPolicy: sessionMetadata.approvalPolicy ?? AgentMode.Agent.approvalPolicy, + resolvedApprovalsReviewer: sessionMetadata.approvalsReviewer ?? AgentMode.Agent.approvalsReviewer, + resolvedSandboxPolicy: sessionMetadata.sandboxPolicy ?? AgentMode.Agent.sandboxPolicy, collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -2493,6 +2502,9 @@ export class CodexAcpServer { supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], agentMode: AgentMode.getInitialAgentMode(), + resolvedApprovalPolicy: sessionMetadata.approvalPolicy ?? AgentMode.Agent.approvalPolicy, + resolvedApprovalsReviewer: sessionMetadata.approvalsReviewer ?? AgentMode.Agent.approvalsReviewer, + resolvedSandboxPolicy: sessionMetadata.sandboxPolicy ?? AgentMode.Agent.sandboxPolicy, collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -3506,6 +3518,7 @@ export class CodexAcpServer { throw RequestError.invalidRequest("The current model does not support image input"); } const agentMode = sessionState.agentMode; + const turnPermissionSettings = this.createTurnPermissionSettings(sessionState, agentMode); const serviceTier = resolveFastServiceTier( sessionState.fastModeEnabled, sessionState.currentModelSupportsFast, @@ -3515,7 +3528,7 @@ export class CodexAcpServer { const sendPromptPromise = this.runOnAppServer(promptGeneration, promptClient, () => promptClient.sendPrompt( effectiveParams, - agentMode, + turnPermissionSettings, modelId, serviceTier, disableSummary, @@ -3893,6 +3906,24 @@ export class CodexAcpServer { } } + private createTurnPermissionSettings( + sessionState: SessionState, + agentMode: AgentMode, + ): {approvalPolicy: AskForApproval, approvalsReviewer: ApprovalsReviewer, sandboxPolicy: SandboxPolicy} { + if (agentMode.id !== AgentMode.DEFAULT_AGENT_MODE.id) { + return { + approvalPolicy: agentMode.approvalPolicy, + approvalsReviewer: agentMode.approvalsReviewer, + sandboxPolicy: agentMode.sandboxPolicy, + }; + } + return { + approvalPolicy: sessionState.resolvedApprovalPolicy, + approvalsReviewer: sessionState.resolvedApprovalsReviewer, + sandboxPolicy: sessionState.resolvedSandboxPolicy, + }; + } + private cancelledPromptResponse(sessionState: SessionState): acp.PromptResponse { return { stopReason: "cancelled", diff --git a/src/SessionFork.ts b/src/SessionFork.ts index 31124af1..155786d3 100644 --- a/src/SessionFork.ts +++ b/src/SessionFork.ts @@ -64,6 +64,9 @@ export async function forkSession( currentServiceTier: response.serviceTier as ServiceTier ?? null, additionalDirectories, skippedMcpServers: sessionConfig.skippedMcpServers, + approvalPolicy: response.approvalPolicy, + approvalsReviewer: response.approvalsReviewer, + sandboxPolicy: response.sandbox, }; } diff --git a/src/SessionMetadata.ts b/src/SessionMetadata.ts index 8e74a7ed..d046191f 100644 --- a/src/SessionMetadata.ts +++ b/src/SessionMetadata.ts @@ -1,6 +1,6 @@ import type {ModeKind} from "./app-server/ModeKind"; import type {ServiceTier} from "./app-server/ServiceTier"; -import type {Model, Thread, ThreadItem} from "./app-server/v2"; +import type {ApprovalsReviewer, AskForApproval, Model, SandboxPolicy, Thread, ThreadItem} from "./app-server/v2"; export type SessionMetadata = { sessionId: string, @@ -11,6 +11,9 @@ export type SessionMetadata = { currentServiceTier?: ServiceTier | null, additionalDirectories: string[], skippedMcpServers?: string[], + approvalPolicy?: AskForApproval, + approvalsReviewer?: ApprovalsReviewer, + sandboxPolicy?: SandboxPolicy, } export type SessionMetadataWithThread = SessionMetadata & { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 043cc54e..7017a2e4 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -11,6 +11,7 @@ import { createTestSessionState, type TestFixture, deferred, + setupPromptTestSession, } from "../acp-test-utils"; import type {ServerNotification} from "../../app-server"; import type {SessionState} from "../../CodexAcpServer"; @@ -1414,6 +1415,67 @@ describe('ACP server test', { timeout: 40_000 }, () => { .toBe(true); }); + it('preserves resolved default prompt permissions while adding ACP directories', async () => { + const {mockFixture, turnStartSpy} = setupPromptTestSession({ + sessionId: "session-id", + cwd: "/workspace", + additionalDirectories: ["/workspace/extra"], + agentMode: AgentMode.DEFAULT_AGENT_MODE, + resolvedApprovalPolicy: "on-request", + resolvedApprovalsReviewer: "user", + resolvedSandboxPolicy: { + type: "workspaceWrite", + writableRoots: ["/shared-cache"], + networkAccess: true, + excludeTmpdirEnvVar: false, + excludeSlashTmp: false, + }, + }); + + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "Hello"}], + }); + + expect(turnStartSpy.mock.calls[0]![0]).toMatchObject({ + approvalPolicy: "on-request", + approvalsReviewer: "user", + sandboxPolicy: { + type: "workspaceWrite", + writableRoots: ["/shared-cache", "/workspace/extra"], + networkAccess: true, + }, + }); + }); + + it('uses explicit non-default mode prompt permissions instead of resolved defaults', async () => { + const {mockFixture, turnStartSpy} = setupPromptTestSession({ + sessionId: "session-id", + cwd: "/workspace", + agentMode: AgentMode.AgentFullAccess, + resolvedApprovalPolicy: "on-request", + resolvedApprovalsReviewer: "user", + resolvedSandboxPolicy: { + type: "workspaceWrite", + writableRoots: ["/shared-cache"], + networkAccess: true, + excludeTmpdirEnvVar: false, + excludeSlashTmp: false, + }, + }); + + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "Hello"}], + }); + + expect(turnStartSpy.mock.calls[0]![0]).toMatchObject({ + approvalPolicy: "never", + approvalsReviewer: "user", + sandboxPolicy: {type: "dangerFullAccess"}, + }); + }); + it('applies ACP additional directories to turn skill discovery and sandbox policy', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpAgent = mockFixture.getCodexAcpAgent(); diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index df6bcb74..ad47ce54 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -429,6 +429,9 @@ export function createTestSessionState(overrides?: Partial): Sessi supportedReasoningEfforts: [], supportedInputModalities: ["text", "image"], agentMode: AgentMode.DEFAULT_AGENT_MODE, + resolvedApprovalPolicy: "on-request", + resolvedApprovalsReviewer: "auto_review", + resolvedSandboxPolicy: AgentMode.DEFAULT_AGENT_MODE.sandboxPolicy, collaborationMode: DEFAULT_COLLABORATION_MODE, fastModeEnabled: false, currentModelSupportsFast: false,