diff --git a/src/core/bot.ts b/src/core/bot.ts index fb2e320..daf2d91 100644 --- a/src/core/bot.ts +++ b/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 - if (streamMsg.success && streamMsg.result === '' && !response.trim()) { - console.error('[Bot] Warning: Agent returned empty result with no response.'); + + // Retry once when stream ends without any assistant text. + // This catches both empty-success and terminal-error runs. + // 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 { diff --git a/src/tools/letta-api.test.ts b/src/tools/letta-api.test.ts new file mode 100644 index 0000000..fa79f5f --- /dev/null +++ b/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(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)'); + }); +}); diff --git a/src/tools/letta-api.ts b/src/tools/letta-api.ts index 735d759..0f93363 100644 --- a/src/tools/letta-api.ts +++ b/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) { - console.log(`[Letta API] Found ${approvals.length} orphaned approval(s) from ${status}/${stopReason} run ${runId}`); + if (isTerminated || isAbandonedApproval || isStuckApproval) { + 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`); }