fix(approval): normalize approved client tool results (#1441)

Co-authored-by: Letta Code <noreply@letta.com>

src/agent/approval-result-normalization.ts

@@ -4,6 +4,10 @@ import { INTERRUPTED_BY_USER } from "../constants";
 import type { ApprovalResult } from "./approval-execution";
 
 type OutgoingMessage = MessageCreate | ApprovalCreate;
+type ToolReturnContent = Extract<
+  ApprovalResult,
+  { type: "tool" }
+>["tool_return"];
 
 export type ApprovalNormalizationOptions = {
   /**
@@ -47,6 +51,26 @@ function normalizeToolReturnText(value: unknown): string {
   }
 }
 
+function isToolReturnContent(value: unknown): value is ToolReturnContent {
+  if (typeof value === "string") return true;
+  if (!Array.isArray(value)) return false;
+
+  return value.every(
+    (part) =>
+      !!part &&
+      typeof part === "object" &&
+      "type" in part &&
+      (((part as { type?: unknown }).type === "text" &&
+        "text" in part &&
+        typeof (part as { text?: unknown }).text === "string") ||
+        ((part as { type?: unknown }).type === "image" &&
+          "data" in part &&
+          typeof (part as { data?: unknown }).data === "string" &&
+          "mimeType" in part &&
+          typeof (part as { mimeType?: unknown }).mimeType === "string")),
+  );
+}
+
 export function normalizeApprovalResultsForPersistence(
   approvals: ApprovalResult[] | null | undefined,
   options: ApprovalNormalizationOptions = {},
@@ -56,6 +80,39 @@ export function normalizeApprovalResultsForPersistence(
   const interruptedSet = new Set(options.interruptedToolCallIds ?? []);
 
   return approvals.map((approval) => {
+    if (
+      approval &&
+      typeof approval === "object" &&
+      "type" in approval &&
+      approval.type === "approval" &&
+      "approve" in approval &&
+      approval.approve === true &&
+      "tool_return" in approval &&
+      isToolReturnContent(approval.tool_return)
+    ) {
+      return {
+        type: "tool",
+        tool_call_id:
+          "tool_call_id" in approval &&
+          typeof approval.tool_call_id === "string"
+            ? approval.tool_call_id
+            : "",
+        tool_return: approval.tool_return,
+        status:
+          "status" in approval && approval.status === "error"
+            ? "error"
+            : "success",
+        stdout:
+          "stdout" in approval && Array.isArray(approval.stdout)
+            ? approval.stdout
+            : undefined,
+        stderr:
+          "stderr" in approval && Array.isArray(approval.stderr)
+            ? approval.stderr
+            : undefined,
+      } satisfies ApprovalResult;
+    }
+
     if (
       !approval ||
       typeof approval !== "object" ||

src/cli/App.tsx

@@ -10332,10 +10332,12 @@ ${SYSTEM_REMINDER_CLOSE}
                 // (can't use state here as it won't be available until next render)
                 const recoveryApprovalResults = [
                   ...autoAllowedResults.map((ar) => ({
-                    type: "approval" as const,
+                    type: "tool" as const,
                     tool_call_id: ar.toolCallId,
-                    approve: true,
                     tool_return: ar.result.toolReturn,
+                    status: ar.result.status,
+                    stdout: ar.result.stdout,
+                    stderr: ar.result.stderr,
                   })),
                   ...autoDeniedResults,
                 ];

src/tests/agent/approval-result-normalization.test.ts

@@ -8,6 +8,26 @@ import {
 import { INTERRUPTED_BY_USER } from "../../constants";
 
 describe("normalizeApprovalResultsForPersistence", () => {
+  test("converts legacy approved approval payloads with tool_return into tool results", () => {
+    const approvals: ApprovalResult[] = [
+      {
+        type: "approval",
+        tool_call_id: "call-legacy",
+        approve: true,
+        tool_return: "legacy result",
+      } as unknown as ApprovalResult,
+    ];
+
+    const normalized = normalizeApprovalResultsForPersistence(approvals);
+
+    expect(normalized[0]).toMatchObject({
+      type: "tool",
+      tool_call_id: "call-legacy",
+      tool_return: "legacy result",
+      status: "success",
+    });
+  });
+
   test("forces status=error for structured interrupted tool_call_ids", () => {
     const approvals: ApprovalResult[] = [
       {
@@ -73,6 +93,31 @@ describe("normalizeApprovalResultsForPersistence", () => {
 });
 
 describe("normalizeOutgoingApprovalMessages", () => {
+  test("canonicalizes malformed approved approval payloads before sending", () => {
+    const approvalMessage: ApprovalCreate = {
+      type: "approval",
+      approvals: [
+        {
+          type: "approval",
+          tool_call_id: "call-legacy",
+          approve: true,
+          tool_return: "legacy result",
+        } as unknown as ApprovalResult,
+      ],
+    };
+
+    const messages = normalizeOutgoingApprovalMessages([approvalMessage]);
+    const normalizedApproval = messages[0] as ApprovalCreate;
+    const approvals = normalizedApproval.approvals ?? [];
+
+    expect(approvals[0]).toMatchObject({
+      type: "tool",
+      tool_call_id: "call-legacy",
+      tool_return: "legacy result",
+      status: "success",
+    });
+  });
+
   test("normalizes approvals and preserves non-approval messages", () => {
     const approvalMessage: ApprovalCreate = {
       type: "approval",