From 523e0d7cfe9127b1d6bc38a5a5735ec9dea65c5c Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Wed, 3 Jun 2026 13:04:49 +0200 Subject: [PATCH] Do not send reason as content on permission request According to the specification, the permissionRequest contains a `ToolCallUpdate`, which - again according to the spec - "Replace[s] the content collection." https://agentclientprotocol.com/protocol/v1/schema#param-content-13 This means that if the agent sends a tool call with a diff for file edits and then the permission request, a typical client would replace the the diff with just the reason text. I'd say that either ACP should add a new field `reason` to `RequestPermissionRequest`, or we use the existing `_meta` field to include the reason for clients that want to handle it specially. This PR moves the reason into _meta. --- src/CodexApprovalHandler.ts | 20 ++-------------- .../CodexACPAgent/approval-events.test.ts | 6 ++--- .../data/approval-command-allow-once.json | 22 +++++++++-------- .../data/approval-command-with-rawInput.json | 24 +++++++++++-------- .../data/approval-file-change.json | 24 ++++++++++--------- 5 files changed, 44 insertions(+), 52 deletions(-) diff --git a/src/CodexApprovalHandler.ts b/src/CodexApprovalHandler.ts index 0eddf3ac..d3379284 100644 --- a/src/CodexApprovalHandler.ts +++ b/src/CodexApprovalHandler.ts @@ -7,7 +7,6 @@ import type { FileChangeRequestApprovalParams, FileChangeRequestApprovalResponse } from "./app-server/v2"; -import type {ToolCallContent} from "@agentclientprotocol/sdk/dist/schema/types.gen"; import {logger} from "./Logger"; import {stripShellPrefix} from "./CodexEventHandler"; import {ApprovalOptionId} from "./ApprovalOptionId"; @@ -62,47 +61,32 @@ export class CodexApprovalHandler implements ApprovalHandler { sessionId: string, params: CommandExecutionRequestApprovalParams ): acp.RequestPermissionRequest { - const reasonContent = this.createContentFromReason(params.reason ?? null); return { sessionId, toolCall: { toolCallId: params.itemId, kind: "execute", status: "pending", - content: reasonContent ? [reasonContent] : null, rawInput: params.command ? { command: stripShellPrefix(params.command), cwd: params.cwd } : null, }, options: APPROVAL_OPTIONS, + _meta: { codex: { params } } }; } - private createContentFromReason(reason: string | null): ToolCallContent | null { - if (reason === null || reason === "") { - return null; - } - return { - type: "content", - content: { - type: "text", - text: reason - } - } - } - private buildFileChangePermissionRequest( sessionId: string, params: FileChangeRequestApprovalParams ): acp.RequestPermissionRequest { - const reasonContent = this.createContentFromReason(params.reason ?? null); return { sessionId, toolCall: { toolCallId: params.itemId, kind: "edit", status: "pending", - content: reasonContent ? [reasonContent] : null, }, options: APPROVAL_OPTIONS, + _meta: { codex: { params } } }; } diff --git a/src/__tests__/CodexACPAgent/approval-events.test.ts b/src/__tests__/CodexACPAgent/approval-events.test.ts index 7ee43746..47f6e33c 100644 --- a/src/__tests__/CodexACPAgent/approval-events.test.ts +++ b/src/__tests__/CodexACPAgent/approval-events.test.ts @@ -143,7 +143,7 @@ describe('Approval Events', () => { params ); - await expect(fixture.getAcpConnectionDump(['_meta'])).toMatchFileSnapshot( + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( 'data/approval-command-allow-once.json' ); @@ -172,7 +172,7 @@ describe('Approval Events', () => { params ); - await expect(fixture.getAcpConnectionDump(['_meta'])).toMatchFileSnapshot( + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( 'data/approval-command-with-rawInput.json' ); @@ -314,7 +314,7 @@ describe('Approval Events', () => { params ); - await expect(fixture.getAcpConnectionDump(['_meta'])).toMatchFileSnapshot( + await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( 'data/approval-file-change.json' ); diff --git a/src/__tests__/CodexACPAgent/data/approval-command-allow-once.json b/src/__tests__/CodexACPAgent/data/approval-command-allow-once.json index 453ed87b..e894337c 100644 --- a/src/__tests__/CodexACPAgent/data/approval-command-allow-once.json +++ b/src/__tests__/CodexACPAgent/data/approval-command-allow-once.json @@ -7,15 +7,6 @@ "toolCallId": "item-snapshot", "kind": "execute", "status": "pending", - "content": [ - { - "type": "content", - "content": { - "type": "text", - "text": "Running npm install" - } - } - ], "rawInput": null }, "options": [ @@ -34,7 +25,18 @@ "name": "Reject", "kind": "reject_once" } - ] + ], + "_meta": { + "codex": { + "params": { + "threadId": "test-session-id", + "turnId": "turn-1", + "itemId": "item-snapshot", + "reason": "Running npm install", + "proposedExecpolicyAmendment": null + } + } + } } ] } \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/approval-command-with-rawInput.json b/src/__tests__/CodexACPAgent/data/approval-command-with-rawInput.json index 5be9602b..1fe49383 100644 --- a/src/__tests__/CodexACPAgent/data/approval-command-with-rawInput.json +++ b/src/__tests__/CodexACPAgent/data/approval-command-with-rawInput.json @@ -7,15 +7,6 @@ "toolCallId": "item-with-command", "kind": "execute", "status": "pending", - "content": [ - { - "type": "content", - "content": { - "type": "text", - "text": "Installing dependencies" - } - } - ], "rawInput": { "command": "npm install", "cwd": "/home/user/project" @@ -37,7 +28,20 @@ "name": "Reject", "kind": "reject_once" } - ] + ], + "_meta": { + "codex": { + "params": { + "threadId": "test-session-id", + "turnId": "turn-1", + "itemId": "item-with-command", + "reason": "Installing dependencies", + "command": "npm install", + "cwd": "/home/user/project", + "proposedExecpolicyAmendment": null + } + } + } } ] } \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/approval-file-change.json b/src/__tests__/CodexACPAgent/data/approval-file-change.json index b4c351f1..cd35c654 100644 --- a/src/__tests__/CodexACPAgent/data/approval-file-change.json +++ b/src/__tests__/CodexACPAgent/data/approval-file-change.json @@ -6,16 +6,7 @@ "toolCall": { "toolCallId": "file-change-snapshot", "kind": "edit", - "status": "pending", - "content": [ - { - "type": "content", - "content": { - "type": "text", - "text": "Modifying config file" - } - } - ] + "status": "pending" }, "options": [ { @@ -33,7 +24,18 @@ "name": "Reject", "kind": "reject_once" } - ] + ], + "_meta": { + "codex": { + "params": { + "threadId": "test-session-id", + "turnId": "turn-1", + "itemId": "file-change-snapshot", + "reason": "Modifying config file", + "grantRoot": null + } + } + } } ] } \ No newline at end of file