fix(listener): preserve allow comments in approval responses (#1442)

Co-authored-by: Letta Code <noreply@letta.com>
This commit is contained in:
Charles Packer
2026-03-18 16:41:25 -07:00
committed by GitHub
parent 63eccd037b
commit 3b99b7d3b9
8 changed files with 48 additions and 2 deletions

View File

@@ -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;
}

View File

@@ -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<string, unknown>;
};
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);

View File

@@ -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<string, unknown> | null;
/** TODO: Not implemented - dynamic permission rule updates */

View File

@@ -283,6 +283,7 @@ export interface StreamDeltaMessage extends RuntimeEnvelope {
export interface ApprovalResponseAllowDecision {
behavior: "allow";
message?: string;
updated_input?: Record<string, unknown> | null;
updated_permissions?: string[];
}

View File

@@ -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";

View File

@@ -483,6 +483,7 @@ export async function resolveRecoveredApprovalResponse(
toolArgs: JSON.stringify(decision.updated_input),
}
: entry.approval,
reason: decision.message,
});
} else {
decisions.push({

View File

@@ -213,6 +213,7 @@ async function resolveStaleApprovals(
toolArgs: JSON.stringify(response.updated_input),
}
: ac.approval,
reason: response.message,
});
} else {
decisions.push({

View File

@@ -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",