From 6b3850e89e0fac0fd9b58b536ad1159ffcde072d Mon Sep 17 00:00:00 2001 From: kagura-agent Date: Sat, 11 Apr 2026 20:22:09 +0800 Subject: [PATCH 1/2] fix(orchestrator): surface AI error results instead of silent drop (fixes #1076) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the Claude OAuth refresh token is expired, the result message has is_error=true but session_id=undefined. Both handleStreamMode and handleBatchMode required sessionId to be truthy to process result messages, causing errors to be silently dropped. Changes: - Split result handler condition to process sessionId and isError independently in both stream and batch modes - Send descriptive error message to user when AI errors produce no assistant messages (with auth-specific hint for authentication_error) - Include errorResult in debug log for observability Tests: 5 new tests covering stream/batch × auth/generic/no-subtype --- .../orchestrator/orchestrator-agent.test.ts | 159 +++++++++++++++++- .../src/orchestrator/orchestrator-agent.ts | 42 +++-- 2 files changed, 176 insertions(+), 25 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.test.ts b/packages/core/src/orchestrator/orchestrator-agent.test.ts index ab8165ca7e..946b3e0c61 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.test.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.test.ts @@ -37,10 +37,6 @@ const mockExecuteWorkflow = mock(() => Promise.resolve()); const mockHandleCommand = mock(() => Promise.resolve({ success: true, message: 'ok', workflow: undefined }) ); -const mockSendQuery = mock(async function* () { - yield { type: 'assistant', content: 'test response' }; - yield { type: 'result', sessionId: 'session-1' }; -}); const mockGetCodebaseEnvVars = mock(() => Promise.resolve({})); const mockLoadConfig = mock(() => Promise.resolve({ @@ -104,12 +100,14 @@ mock.module('@archon/workflows/executor', () => ({ executeWorkflow: mockExecuteWorkflow, })); +const mockSendQuery = mock(async function* () {}); +const mockGetAgentProvider = mock(() => ({ + sendQuery: mockSendQuery, + getType: mock(() => 'claude'), + getCapabilities: mock(() => ({})), +})); mock.module('@archon/providers', () => ({ - getAgentProvider: mock(() => ({ - sendQuery: mockSendQuery, - getType: mock(() => 'claude'), - getCapabilities: mock(() => ({})), - })), + getAgentProvider: mockGetAgentProvider, getProviderCapabilities: mock(() => ({ envInjection: true })), })); @@ -1566,3 +1564,146 @@ describe('handleMessage — workflow context injection', () => { await expect(handleMessage(platform, 'conv-1', 'Hello')).resolves.toBeUndefined(); }); }); + +// ─── AI error result handling (stream + batch) ────────────────────────────── + +describe('AI error result handling', () => { + beforeEach(() => { + mockSendQuery.mockReset(); + mockGetAgentProvider.mockClear(); + mockGetAgentProvider.mockImplementation(() => ({ + sendQuery: mockSendQuery, + getType: mock(() => 'claude'), + getCapabilities: mock(() => ({})), + })); + mockGetOrCreateConversation.mockReset(); + mockGetCodebase.mockReset(); + mockGetOrCreateConversation.mockImplementation(() => Promise.resolve(null)); + mockGetCodebase.mockImplementation(() => Promise.resolve(null)); + mockLogger.debug.mockClear(); + }); + + describe('stream mode', () => { + function makeStreamPlatform(): IPlatformAdapter { + return { + sendMessage: mock(() => Promise.resolve()), + ensureThread: mock((id: string) => Promise.resolve(id)), + getStreamingMode: mock(() => 'stream' as const), + getPlatformType: mock(() => 'web'), + start: mock(() => Promise.resolve()), + stop: mock(() => {}), + }; + } + + test('sends error message when result has isError=true and no assistant messages', async () => { + const conversation = makeConversation({ codebase_id: null }); + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); + mockGetPausedWorkflowRun.mockReturnValueOnce(Promise.resolve(null)); + + mockSendQuery.mockImplementation(async function* () { + yield { + type: 'result', + isError: true, + errorSubtype: 'authentication_error', + }; + }); + + const platform = makeStreamPlatform(); + await handleMessage(platform, 'conv-1', 'hello'); + + expect(platform.sendMessage).toHaveBeenCalledWith( + 'conv-1', + 'AI error (authentication_error). Check your Claude credentials or use /reset.' + ); + }); + + test('sends generic hint when errorSubtype is not authentication_error', async () => { + const conversation = makeConversation({ codebase_id: null }); + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); + mockGetPausedWorkflowRun.mockReturnValueOnce(Promise.resolve(null)); + + mockSendQuery.mockImplementation(async function* () { + yield { + type: 'result', + isError: true, + errorSubtype: 'rate_limit', + }; + }); + + const platform = makeStreamPlatform(); + await handleMessage(platform, 'conv-1', 'hello'); + + expect(platform.sendMessage).toHaveBeenCalledWith( + 'conv-1', + 'AI error (rate_limit). Check server logs for details.' + ); + }); + + test('sends error message without subtype when errorSubtype is undefined', async () => { + const conversation = makeConversation({ codebase_id: null }); + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); + mockGetPausedWorkflowRun.mockReturnValueOnce(Promise.resolve(null)); + + mockSendQuery.mockImplementation(async function* () { + yield { + type: 'result', + isError: true, + }; + }); + + const platform = makeStreamPlatform(); + await handleMessage(platform, 'conv-1', 'hello'); + + expect(platform.sendMessage).toHaveBeenCalledWith( + 'conv-1', + 'AI error. Check server logs for details.' + ); + }); + }); + + describe('batch mode', () => { + test('sends error message when result has isError=true and no assistant messages', async () => { + const conversation = makeConversation({ codebase_id: null }); + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); + mockGetPausedWorkflowRun.mockReturnValueOnce(Promise.resolve(null)); + + mockSendQuery.mockImplementation(async function* () { + yield { + type: 'result', + isError: true, + errorSubtype: 'authentication_error', + }; + }); + + const platform = makePlatform(); // batch mode by default + await handleMessage(platform, 'conv-1', 'hello'); + + expect(platform.sendMessage).toHaveBeenCalledWith( + 'conv-1', + 'AI error (authentication_error). Check your Claude credentials or use /reset.' + ); + }); + + test('sends generic hint for non-authentication errors', async () => { + const conversation = makeConversation({ codebase_id: null }); + mockGetOrCreateConversation.mockReturnValueOnce(Promise.resolve(conversation)); + mockGetPausedWorkflowRun.mockReturnValueOnce(Promise.resolve(null)); + + mockSendQuery.mockImplementation(async function* () { + yield { + type: 'result', + isError: true, + errorSubtype: 'internal_error', + }; + }); + + const platform = makePlatform(); + await handleMessage(platform, 'conv-1', 'hello'); + + expect(platform.sendMessage).toHaveBeenCalledWith( + 'conv-1', + 'AI error (internal_error). Check server logs for details.' + ); + }); + }); +}); diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index d5eb9397b3..2c8a19ea33 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -916,6 +916,7 @@ async function handleStreamMode( ): Promise { const allMessages: string[] = []; let newSessionId: string | undefined; + let errorResult: { subtype?: string } | undefined; let commandDetected = false; for await (const msg of aiClient.sendQuery( @@ -959,13 +960,7 @@ async function handleStreamMode( newSessionId = msg.sessionId; } if (msg.isError) { - getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); - const syntheticError = new Error(msg.errorSubtype ?? 'AI result error'); - await platform.sendMessage(conversationId, classifyAndFormatError(syntheticError)); - if (newSessionId) { - await tryPersistSessionId(session.id, newSessionId); - } - return; + errorResult = { subtype: msg.errorSubtype }; } if (!commandDetected && platform.sendStructuredEvent) { await platform.sendStructuredEvent(conversationId, msg); @@ -978,7 +973,17 @@ async function handleStreamMode( } if (allMessages.length === 0) { - getLog().debug({ conversationId }, 'no_ai_response'); + if (errorResult) { + const hint = + errorResult.subtype === 'authentication_error' + ? 'Check your Claude credentials or use /reset.' + : 'Check server logs for details.'; + await platform.sendMessage( + conversationId, + `AI error${errorResult.subtype ? ` (${errorResult.subtype})` : ''}. ${hint}` + ); + } + getLog().debug({ conversationId, errorResult }, 'no_ai_response'); return; } @@ -1046,6 +1051,7 @@ async function handleBatchMode( let assistantChunksTruncated = false; let totalChunksTruncated = false; let newSessionId: string | undefined; + let errorResult: { subtype?: string } | undefined; let commandDetected = false; for await (const msg of aiClient.sendQuery( @@ -1082,13 +1088,7 @@ async function handleBatchMode( newSessionId = msg.sessionId; } if (msg.isError) { - getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); - const syntheticError = new Error(msg.errorSubtype ?? 'AI result error'); - await platform.sendMessage(conversationId, classifyAndFormatError(syntheticError)); - if (newSessionId) { - await tryPersistSessionId(session.id, newSessionId); - } - return; + errorResult = { subtype: msg.errorSubtype }; } } @@ -1123,7 +1123,17 @@ async function handleBatchMode( const finalMessage = filterToolIndicators(assistantMessages); if (!finalMessage) { - getLog().debug({ conversationId }, 'no_ai_response'); + if (errorResult) { + const hint = + errorResult.subtype === 'authentication_error' + ? 'Check your Claude credentials or use /reset.' + : 'Check server logs for details.'; + await platform.sendMessage( + conversationId, + `AI error${errorResult.subtype ? ` (${errorResult.subtype})` : ''}. ${hint}` + ); + } + getLog().debug({ conversationId, errorResult }, 'no_ai_response'); return; } From 0476be62511194665667aa0f3f5cff56a9722fbc Mon Sep 17 00:00:00 2001 From: kagura-agent Date: Sat, 11 Apr 2026 20:29:45 +0800 Subject: [PATCH 2/2] refactor: use provider-specific label in auth error hint Address CodeRabbit review: replace hardcoded 'Claude credentials' with provider-aware label from aiClient.getType() so error messages are accurate for non-Claude assistants. --- packages/core/src/orchestrator/orchestrator-agent.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index 2c8a19ea33..f64acfdc8e 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -974,9 +974,10 @@ async function handleStreamMode( if (allMessages.length === 0) { if (errorResult) { + const providerLabel = aiClient.getType() === 'claude' ? 'Claude' : 'AI'; const hint = errorResult.subtype === 'authentication_error' - ? 'Check your Claude credentials or use /reset.' + ? `Check your ${providerLabel} credentials or use /reset.` : 'Check server logs for details.'; await platform.sendMessage( conversationId, @@ -1124,9 +1125,10 @@ async function handleBatchMode( if (!finalMessage) { if (errorResult) { + const providerLabel = aiClient.getType() === 'claude' ? 'Claude' : 'AI'; const hint = errorResult.subtype === 'authentication_error' - ? 'Check your Claude credentials or use /reset.' + ? `Check your ${providerLabel} credentials or use /reset.` : 'Check server logs for details.'; await platform.sendMessage( conversationId,