From fb820ee89aa04ead994d9466fdfac117a7d88822 Mon Sep 17 00:00:00 2001
From: Cameron <cameron@pfiffer.org>
Date: Thu, 26 Feb 2026 15:07:33 -0800
Subject: [PATCH] fix: deduplicate tool_call_ids in orphaned approval recovery
 (#414)

---
 src/tools/letta-api.test.ts | 72 +++++++++++++++++++++++++++++++++++++
 src/tools/letta-api.ts      | 24 +++++++++----
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/src/tools/letta-api.test.ts b/src/tools/letta-api.test.ts
index 85e2462..227cd67 100644
--- a/src/tools/letta-api.test.ts
+++ b/src/tools/letta-api.test.ts
@@ -170,6 +170,78 @@ describe('recoverOrphanedConversationApproval', () => {
     expect(mockConversationsMessagesCreate).not.toHaveBeenCalled();
   });
 
+  it('deduplicates identical tool_call_ids across multiple approval_request_messages', async () => {
+    // Simulate the server returning the same tool_call_id in multiple
+    // approval_request_messages (the root cause of #359).
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-dup', name: 'Bash' }],
+        run_id: 'run-dup',
+        id: 'msg-dup-1',
+      },
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-dup', name: 'Bash' }],
+        run_id: 'run-dup',
+        id: 'msg-dup-2',
+      },
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-dup', name: 'Bash' }],
+        run_id: 'run-dup',
+        id: 'msg-dup-3',
+      },
+    ]));
+    mockRunsRetrieve.mockResolvedValue({ status: 'failed', stop_reason: 'error' });
+    mockConversationsMessagesCreate.mockResolvedValue({});
+    mockRunsList.mockReturnValue(mockPageIterator([]));
+
+    const resultPromise = recoverOrphanedConversationApproval('agent-1', 'conv-1');
+    await vi.advanceTimersByTimeAsync(3000);
+    const result = await resultPromise;
+
+    expect(result.recovered).toBe(true);
+    // Should only send ONE denial despite three identical approval_request_messages
+    expect(mockConversationsMessagesCreate).toHaveBeenCalledOnce();
+    const approvals = mockConversationsMessagesCreate.mock.calls[0][1].messages[0].approvals;
+    expect(approvals).toHaveLength(1);
+    expect(approvals[0].tool_call_id).toBe('tc-dup');
+  });
+
+  it('continues recovery if batch denial API call fails', async () => {
+    // Two runs with approvals -- first batch fails, second should still succeed
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-fail', name: 'Bash' }],
+        run_id: 'run-fail',
+        id: 'msg-fail',
+      },
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-ok', name: 'Read' }],
+        run_id: 'run-ok',
+        id: 'msg-ok',
+      },
+    ]));
+    mockRunsRetrieve.mockResolvedValue({ status: 'failed', stop_reason: 'error' });
+    mockConversationsMessagesCreate
+      .mockRejectedValueOnce(new Error('400 BadRequestError'))
+      .mockResolvedValueOnce({});
+    mockRunsList.mockReturnValue(mockPageIterator([]));
+
+    const resultPromise = recoverOrphanedConversationApproval('agent-1', 'conv-1');
+    await vi.advanceTimersByTimeAsync(3000);
+    const result = await resultPromise;
+
+    // Second run still recovered despite first failing
+    expect(result.recovered).toBe(true);
+    expect(result.details).toContain('Failed to deny');
+    expect(result.details).toContain('Denied 1 approval(s) from failed run run-ok');
+    expect(mockConversationsMessagesCreate).toHaveBeenCalledTimes(2);
+  });
+
   it('reports cancel failure accurately', async () => {
     mockConversationsMessagesList.mockReturnValue(mockPageIterator([
       {
diff --git a/src/tools/letta-api.ts b/src/tools/letta-api.ts
index 9c3d798..0cd91a2 100644
--- a/src/tools/letta-api.ts
+++ b/src/tools/letta-api.ts
@@ -668,6 +668,7 @@ export async function recoverOrphanedConversationApproval(
       runId: string;
     }
     const unresolvedByRun = new Map<string, UnresolvedApproval[]>();
+    const seenToolCallIds = new Set<string>();
     
     for (const msg of messages) {
       if (msg.message_type !== 'approval_request_message') continue;
@@ -678,6 +679,9 @@ export async function recoverOrphanedConversationApproval(
       
       for (const tc of toolCalls) {
         if (!tc.tool_call_id || resolvedToolCalls.has(tc.tool_call_id)) continue;
+        // Skip duplicate tool_call_ids across multiple approval_request_messages
+        if (seenToolCallIds.has(tc.tool_call_id)) continue;
+        seenToolCallIds.add(tc.tool_call_id);
         
         const key = runId || 'unknown';
         if (!unresolvedByRun.has(key)) unresolvedByRun.set(key, []);
@@ -731,13 +735,19 @@ export async function recoverOrphanedConversationApproval(
             reason: `Auto-denied: originating run was ${status}/${stopReason}`,
           }));
           
-          await client.conversations.messages.create(conversationId, {
-            messages: [{
-              type: 'approval',
-              approvals: approvalResponses,
-            }],
-            streaming: false,
-          });
+          try {
+            await client.conversations.messages.create(conversationId, {
+              messages: [{
+                type: 'approval',
+                approvals: approvalResponses,
+              }],
+              streaming: false,
+            });
+          } catch (batchError) {
+            log.warn(`Failed to submit approval denial batch for run ${runId} (${approvals.length} tool call(s)):`, batchError);
+            details.push(`Failed to deny ${approvals.length} approval(s) from run ${runId}`);
+            continue;
+          }
           
           // The denial triggers a new agent run server-side. Wait for it to
           // settle before returning, otherwise the caller retries immediately