fix: recover from active runs stuck on approval (#249)

* fix: recover from active runs stuck on approval

recoverOrphanedConversationApproval() only resolved approvals from
terminated (failed/cancelled) or abandoned (completed/requires_approval)
runs. Active runs with status=running and stop_reason=requires_approval
were skipped, even though they block the entire conversation.

This happens when the CLI's HITL approval flow leaves a pending approval
that no client will resolve (e.g. agent triggered via ADE, user walked
away). Every subsequent message fails with success=false.

Now also handles running+requires_approval: rejects the approval and
cancels the stuck run so lettabot can proceed.

Fixes the recovery path for all three call sites: pre-send check,
CONFLICT catch, and error-result retry.

Written by Cameron ◯ Letta Code

"The best error message is the one that never shows up." -- Thomas Fuchs

* fix: use cancelRuns return value for accurate diagnostics

Check boolean return from cancelRuns() instead of always logging
success. Details string now shows "(cancel failed)" when it didn't
work.

Written by Cameron ◯ Letta Code

"Trust, but verify." -- Ronald Reagan

* fix: detect terminal errors and retry with recovery in bot.ts

Monkey patch until SDK issue letta-code-sdk#31 is resolved.

1. Detect terminal errors (success=false OR error field), not just
   empty-success. Both trigger orphan recovery + retry.

2. Blind retry on terminal error when no orphaned approvals found --
   client-side approval failures may leave no detectable record.

3. sendToAgent() throws on terminal error instead of returning empty
   string. Background callers get actionable errors.

Written by Cameron ◯ Letta Code

"A good patch is one you can remove." -- unknown

* test: add tests for approval recovery including stuck runs

7 tests covering recoverOrphanedConversationApproval():
- Empty conversation
- No unresolved approvals
- Recovery from failed run
- Recovery from stuck running+requires_approval (with cancel)
- Already-resolved approvals skipped
- Healthy running run not touched
- Cancel failure reported accurately

Written by Cameron ◯ Letta Code

"Code without tests is broken by design." -- Jacob Kaplan-Moss

* fix: gate retry on sentAnyMessage to prevent duplicate delivery

Codex review caught that finalizeMessage() clears the response buffer
on type changes, so hasResponse could be false even when output was
already sent. This caused duplicate retries with potential side effects.

Now checks !sentAnyMessage (authoritative delivery flag) in addition
to !hasResponse before retrying.

Written by Cameron ◯ Letta Code

"Idempotency: the art of doing nothing twice." -- unknown

src/core/bot.ts

@@ -694,17 +694,40 @@ export class LettaBot implements AgentSession {
           }
           
           if (streamMsg.type === 'result') {
-            console.log(`[Bot] Stream result: success=${streamMsg.success}, hasResponse=${response.trim().length > 0}`);
+            const resultText = typeof streamMsg.result === 'string' ? streamMsg.result : '';
+            const hasResponse = response.trim().length > 0;
+            const isTerminalError = streamMsg.success === false || !!streamMsg.error;
+            console.log(`[Bot] Stream result: success=${streamMsg.success}, hasResponse=${hasResponse}, resultLen=${resultText.length}`);
             console.log(`[Bot] Stream message counts:`, msgTypeCounts);
             if (streamMsg.error) {
-              console.error(`[Bot] Result error: ${streamMsg.error}`);
+              const detail = resultText.trim();
+              if (detail) {
+                console.error(`[Bot] Result error: ${streamMsg.error} (${detail.slice(0, 200)})`);
+              } else {
+                console.error(`[Bot] Result error: ${streamMsg.error}`);
+              }
             }
-            
+
-            // Empty result recovery
+            // Retry once when stream ends without any assistant text.
-            if (streamMsg.success && streamMsg.result === '' && !response.trim()) {
+            // This catches both empty-success and terminal-error runs.
-              console.error('[Bot] Warning: Agent returned empty result with no response.');
+            // TODO(letta-code-sdk#31): Remove once SDK handles HITL approvals in bypassPermissions mode.
+            // Only retry if we never sent anything to the user. hasResponse tracks
+            // the current buffer, but finalizeMessage() clears it on type changes.
+            // sentAnyMessage is the authoritative "did we deliver output" flag.
+            const nothingDelivered = !hasResponse && !sentAnyMessage;
+            const shouldRetryForEmptyResult = streamMsg.success && resultText === '' && nothingDelivered;
+            const shouldRetryForErrorResult = isTerminalError && nothingDelivered;
+            if (shouldRetryForEmptyResult || shouldRetryForErrorResult) {
+              if (shouldRetryForEmptyResult) {
+                console.error('[Bot] Warning: Agent returned empty result with no response.');
+              }
+              if (shouldRetryForErrorResult) {
+                console.error('[Bot] Warning: Agent returned terminal error result with no response.');
+              }
+
               if (!retried && this.store.agentId && this.store.conversationId) {
-                console.log('[Bot] Empty result - attempting orphaned approval recovery...');
+                const reason = shouldRetryForErrorResult ? 'error result' : 'empty result';
+                console.log(`[Bot] ${reason} - attempting orphaned approval recovery...`);
                 session.close();
                 session = null;
                 clearInterval(typingInterval);
@@ -717,8 +740,20 @@ export class LettaBot implements AgentSession {
                   return this.processMessage(msg, adapter, true);
                 }
                 console.warn(`[Bot] No orphaned approvals found: ${convResult.details}`);
+
+                // Some client-side approval failures do not surface as pending approvals.
+                // Retry once anyway in case the previous run terminated mid-tool cycle.
+                if (shouldRetryForErrorResult) {
+                  console.log('[Bot] Retrying once after terminal error (no orphaned approvals detected)...');
+                  return this.processMessage(msg, adapter, true);
+                }
               }
             }
+
+            if (isTerminalError && !hasResponse && !sentAnyMessage) {
+              const err = streamMsg.error || 'unknown error';
+              response = `(Agent run failed: ${err}. Try sending your message again.)`;
+            }
             
             break;
           }
@@ -834,7 +869,14 @@ export class LettaBot implements AgentSession {
           if (msg.type === 'assistant') {
             response += msg.content || '';
           }
-          if (msg.type === 'result') break;
+          if (msg.type === 'result') {
+            // TODO(letta-code-sdk#31): Remove once SDK handles HITL approvals in bypassPermissions mode.
+            if (msg.success === false || msg.error) {
+              const detail = typeof msg.result === 'string' ? msg.result.trim() : '';
+              throw new Error(detail ? `Agent run failed: ${msg.error || 'error'} (${detail})` : `Agent run failed: ${msg.error || 'error'}`);
+            }
+            break;
+          }
         }
         return response;
       } finally {

src/tools/letta-api.test.ts

@@ -0,0 +1,168 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+// Mock the Letta client before importing the module under test
+const mockConversationsMessagesList = vi.fn();
+const mockConversationsMessagesCreate = vi.fn();
+const mockRunsRetrieve = vi.fn();
+const mockAgentsMessagesCancel = vi.fn();
+
+vi.mock('@letta-ai/letta-client', () => {
+  return {
+    Letta: class MockLetta {
+      conversations = {
+        messages: {
+          list: mockConversationsMessagesList,
+          create: mockConversationsMessagesCreate,
+        },
+      };
+      runs = { retrieve: mockRunsRetrieve };
+      agents = { messages: { cancel: mockAgentsMessagesCancel } };
+    },
+  };
+});
+
+import { recoverOrphanedConversationApproval } from './letta-api.js';
+
+// Helper to create a mock async iterable from an array (Letta client returns paginated iterators)
+function mockPageIterator<T>(items: T[]) {
+  return {
+    [Symbol.asyncIterator]: async function* () {
+      for (const item of items) yield item;
+    },
+  };
+}
+
+describe('recoverOrphanedConversationApproval', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('returns false when no messages in conversation', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([]));
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(false);
+    expect(result.details).toBe('No messages in conversation');
+  });
+
+  it('returns false when no unresolved approval requests', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      { message_type: 'assistant_message', content: 'hello' },
+    ]));
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(false);
+    expect(result.details).toBe('No unresolved approval requests found');
+  });
+
+  it('recovers from failed run with unresolved approval', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-1', name: 'Bash' }],
+        run_id: 'run-1',
+        id: 'msg-1',
+      },
+    ]));
+    mockRunsRetrieve.mockResolvedValue({ status: 'failed', stop_reason: 'error' });
+    mockConversationsMessagesCreate.mockResolvedValue({});
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(true);
+    expect(result.details).toContain('Denied 1 approval(s) from failed run run-1');
+    expect(mockConversationsMessagesCreate).toHaveBeenCalledOnce();
+    // Should NOT cancel -- run is already terminated
+    expect(mockAgentsMessagesCancel).not.toHaveBeenCalled();
+  });
+
+  it('recovers from stuck running+requires_approval and cancels the run', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-2', name: 'Grep' }],
+        run_id: 'run-2',
+        id: 'msg-2',
+      },
+    ]));
+    mockRunsRetrieve.mockResolvedValue({ status: 'running', stop_reason: 'requires_approval' });
+    mockConversationsMessagesCreate.mockResolvedValue({});
+    mockAgentsMessagesCancel.mockResolvedValue(undefined);
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(true);
+    expect(result.details).toContain('(cancelled)');
+    // Should send denial
+    expect(mockConversationsMessagesCreate).toHaveBeenCalledOnce();
+    const createCall = mockConversationsMessagesCreate.mock.calls[0];
+    expect(createCall[0]).toBe('conv-1');
+    const approvals = createCall[1].messages[0].approvals;
+    expect(approvals[0].approve).toBe(false);
+    expect(approvals[0].tool_call_id).toBe('tc-2');
+    // Should cancel the stuck run
+    expect(mockAgentsMessagesCancel).toHaveBeenCalledOnce();
+  });
+
+  it('skips already-resolved approvals', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-3', name: 'Read' }],
+        run_id: 'run-3',
+        id: 'msg-3',
+      },
+      {
+        message_type: 'approval_response_message',
+        approvals: [{ tool_call_id: 'tc-3' }],
+      },
+    ]));
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(false);
+    expect(result.details).toBe('No unresolved approval requests found');
+    expect(mockRunsRetrieve).not.toHaveBeenCalled();
+  });
+
+  it('does not recover from healthy running run', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-4', name: 'Bash' }],
+        run_id: 'run-4',
+        id: 'msg-4',
+      },
+    ]));
+    // Running but NOT stuck on approval -- normal in-progress run
+    mockRunsRetrieve.mockResolvedValue({ status: 'running', stop_reason: null });
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(false);
+    expect(result.details).toContain('not orphaned');
+    expect(mockConversationsMessagesCreate).not.toHaveBeenCalled();
+  });
+
+  it('reports cancel failure accurately', async () => {
+    mockConversationsMessagesList.mockReturnValue(mockPageIterator([
+      {
+        message_type: 'approval_request_message',
+        tool_calls: [{ tool_call_id: 'tc-5', name: 'Grep' }],
+        run_id: 'run-5',
+        id: 'msg-5',
+      },
+    ]));
+    mockRunsRetrieve.mockResolvedValue({ status: 'running', stop_reason: 'requires_approval' });
+    mockConversationsMessagesCreate.mockResolvedValue({});
+    // Cancel fails
+    mockAgentsMessagesCancel.mockRejectedValue(new Error('cancel failed'));
+
+    const result = await recoverOrphanedConversationApproval('agent-1', 'conv-1');
+
+    expect(result.recovered).toBe(true);
+    expect(result.details).toContain('(cancel failed)');
+  });
+});

src/tools/letta-api.ts

@@ -624,9 +624,13 @@ export async function recoverOrphanedConversationApproval(
         const stopReason = run.stop_reason;
         const isTerminated = status === 'failed' || status === 'cancelled';
         const isAbandonedApproval = status === 'completed' && stopReason === 'requires_approval';
+        // Active runs stuck on approval block the entire conversation.
+        // No client is going to approve them -- reject and cancel so
+        // lettabot can proceed.
+        const isStuckApproval = status === 'running' && stopReason === 'requires_approval';
         
-        if (isTerminated || isAbandonedApproval) {
+        if (isTerminated || isAbandonedApproval || isStuckApproval) {
-          console.log(`[Letta API] Found ${approvals.length} orphaned approval(s) from ${status}/${stopReason} run ${runId}`);
+          console.log(`[Letta API] Found ${approvals.length} blocking approval(s) from ${status}/${stopReason} run ${runId}`);
           
           // Send denial for each unresolved tool call
           const approvalResponses = approvals.map(a => ({
@@ -644,8 +648,20 @@ export async function recoverOrphanedConversationApproval(
             streaming: false,
           });
           
+          // Cancel active stuck runs after rejecting their approvals
+          let cancelled = false;
+          if (isStuckApproval) {
+            cancelled = await cancelRuns(agentId, [runId]);
+            if (cancelled) {
+              console.log(`[Letta API] Cancelled stuck run ${runId}`);
+            } else {
+              console.warn(`[Letta API] Failed to cancel stuck run ${runId}`);
+            }
+          }
+          
           recoveredCount += approvals.length;
-          details.push(`Denied ${approvals.length} approval(s) from ${status} run ${runId}`);
+          const suffix = isStuckApproval ? (cancelled ? ' (cancelled)' : ' (cancel failed)') : '';
+          details.push(`Denied ${approvals.length} approval(s) from ${status} run ${runId}${suffix}`);
         } else {
           details.push(`Run ${runId} is ${status}/${stopReason} - not orphaned`);
         }