From 3b99b7d3b940ae1c2dbb44c5735b4b8228842509 Mon Sep 17 00:00:00 2001 From: Charles Packer Date: Wed, 18 Mar 2026 16:41:25 -0700 Subject: [PATCH] fix(listener): preserve allow comments in approval responses (#1442) Co-authored-by: Letta Code --- src/agent/approval-execution.ts | 1 + .../websocket/listen-client-protocol.test.ts | 33 +++++++++++++++++++ src/types/protocol.ts | 2 ++ src/types/protocol_v2.ts | 1 + src/websocket/listener/approval.ts | 4 ++- src/websocket/listener/recovery.ts | 1 + src/websocket/listener/send.ts | 1 + src/websocket/listener/turn-approval.ts | 7 +++- 8 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/agent/approval-execution.ts b/src/agent/approval-execution.ts index c53d4a7..a2ef2a7 100644 --- a/src/agent/approval-execution.ts +++ b/src/agent/approval-execution.ts @@ -170,6 +170,7 @@ export type ApprovalDecision = | { type: "approve"; approval: ApprovalRequest; + reason?: string; // If set, skip executeTool and use this result (for fancy UI tools) precomputedResult?: ToolExecutionResult; } diff --git a/src/tests/websocket/listen-client-protocol.test.ts b/src/tests/websocket/listen-client-protocol.test.ts index 0f76a2a..561494e 100644 --- a/src/tests/websocket/listen-client-protocol.test.ts +++ b/src/tests/websocket/listen-client-protocol.test.ts @@ -1066,6 +1066,7 @@ describe("listen-client capability-gated approval flow", () => { if ("decision" in response) { const canUseToolResponse = response.decision as { behavior: string; + message?: string; updated_input?: Record; }; expect(canUseToolResponse.behavior).toBe("allow"); @@ -1076,6 +1077,38 @@ describe("listen-client capability-gated approval flow", () => { } }); + test("approval_response with allow preserves optional comment", async () => { + const runtime = __listenClientTestUtils.createRuntime(); + const socket = new MockSocket(WebSocket.OPEN); + const requestId = "perm-allow-comment-test"; + + const pending = requestApprovalOverWS( + runtime, + socket as unknown as WebSocket, + requestId, + makeControlRequest(requestId), + ); + + resolvePendingApprovalResolver(runtime, { + request_id: requestId, + decision: { + behavior: "allow", + message: "Ship it", + }, + }); + + const response = await pending; + expect("decision" in response).toBe(true); + if ("decision" in response) { + const canUseToolResponse = response.decision as { + behavior: string; + message?: string; + }; + expect(canUseToolResponse.behavior).toBe("allow"); + expect(canUseToolResponse.message).toBe("Ship it"); + } + }); + test("approval_response with deny includes reason", async () => { const runtime = __listenClientTestUtils.createRuntime(); const socket = new MockSocket(WebSocket.OPEN); diff --git a/src/types/protocol.ts b/src/types/protocol.ts index 80ffe5d..e22af37 100644 --- a/src/types/protocol.ts +++ b/src/types/protocol.ts @@ -723,6 +723,8 @@ export type ControlResponseBody = // --- can_use_tool response payloads --- export interface CanUseToolResponseAllow { behavior: "allow"; + /** Optional user comment attached to an allow decision */ + message?: string; /** Modified tool input */ updatedInput?: Record | null; /** TODO: Not implemented - dynamic permission rule updates */ diff --git a/src/types/protocol_v2.ts b/src/types/protocol_v2.ts index 442855a..3950258 100644 --- a/src/types/protocol_v2.ts +++ b/src/types/protocol_v2.ts @@ -283,6 +283,7 @@ export interface StreamDeltaMessage extends RuntimeEnvelope { export interface ApprovalResponseAllowDecision { behavior: "allow"; + message?: string; updated_input?: Record | null; updated_permissions?: string[]; } diff --git a/src/websocket/listener/approval.ts b/src/websocket/listener/approval.ts index 628025d..5894389 100644 --- a/src/websocket/listener/approval.ts +++ b/src/websocket/listener/approval.ts @@ -94,6 +94,8 @@ export function isValidApprovalResponseBody( updated_permissions?: unknown; }; if (decision.behavior === "allow") { + const hasMessage = + decision.message === undefined || typeof decision.message === "string"; const hasUpdatedInput = decision.updated_input === undefined || decision.updated_input === null || @@ -104,7 +106,7 @@ export function isValidApprovalResponseBody( decision.updated_permissions.every( (entry) => typeof entry === "string", )); - return hasUpdatedInput && hasUpdatedPermissions; + return hasMessage && hasUpdatedInput && hasUpdatedPermissions; } if (decision.behavior === "deny") { return typeof decision.message === "string"; diff --git a/src/websocket/listener/recovery.ts b/src/websocket/listener/recovery.ts index a7582c6..9e37caa 100644 --- a/src/websocket/listener/recovery.ts +++ b/src/websocket/listener/recovery.ts @@ -483,6 +483,7 @@ export async function resolveRecoveredApprovalResponse( toolArgs: JSON.stringify(decision.updated_input), } : entry.approval, + reason: decision.message, }); } else { decisions.push({ diff --git a/src/websocket/listener/send.ts b/src/websocket/listener/send.ts index 2171470..2abe372 100644 --- a/src/websocket/listener/send.ts +++ b/src/websocket/listener/send.ts @@ -213,6 +213,7 @@ async function resolveStaleApprovals( toolArgs: JSON.stringify(response.updated_input), } : ac.approval, + reason: response.message, }); } else { decisions.push({ diff --git a/src/websocket/listener/turn-approval.ts b/src/websocket/listener/turn-approval.ts index f356f13..d557c67 100644 --- a/src/websocket/listener/turn-approval.ts +++ b/src/websocket/listener/turn-approval.ts @@ -50,6 +50,7 @@ type Decision = toolName: string; toolArgs: string; }; + reason?: string; } | { type: "deny"; @@ -230,7 +231,11 @@ export async function handleApprovalStop(params: { toolArgs: JSON.stringify(response.updated_input), } : ac.approval; - decisions.push({ type: "approve", approval: finalApproval }); + decisions.push({ + type: "approve", + approval: finalApproval, + reason: response.message, + }); } else { decisions.push({ type: "deny",