From df535e57bfa23763f0aa26372fb27b6a72a8c611 Mon Sep 17 00:00:00 2001 From: Cole Medin Date: Sat, 11 Apr 2026 10:29:36 -0500 Subject: [PATCH 1/3] fix: surface auth errors instead of silently dropping them (#1076) When Claude OAuth refresh token is expired, the SDK yields a result chunk with is_error=true and no session_id. Both handleStreamMode and handleBatchMode guarded the result branch with `&& msg.sessionId`, silently dropping the error. Users saw no response at all. Changes: - Remove sessionId guard from result branches in orchestrator-agent.ts - Add isError early-exit that sends error message to user - Add 4 OAuth patterns to AUTH_PATTERNS in claude.ts and codex.ts - Add OAuth refresh-token handler to error-formatter.ts - Add tests for new error-formatter branches Fixes #1076 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/orchestrator/orchestrator-agent.ts | 26 ++++++++-- .../core/src/utils/error-formatter.test.ts | 51 +++++++++++++++++++ packages/core/src/utils/error-formatter.ts | 10 ++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index f43ffe0454..251e493af4 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -954,8 +954,17 @@ async function handleStreamMode( if (!commandDetected && platform.sendStructuredEvent) { await platform.sendStructuredEvent(conversationId, msg); } - } else if (msg.type === 'result' && msg.sessionId) { - newSessionId = msg.sessionId; + } else if (msg.type === 'result') { + if (msg.sessionId) { + newSessionId = msg.sessionId; + } + if (msg.isError) { + await platform.sendMessage( + conversationId, + '⚠️ AI error. Check your credentials or use /reset.' + ); + return; + } if (!commandDetected && platform.sendStructuredEvent) { await platform.sendStructuredEvent(conversationId, msg); } @@ -1066,8 +1075,17 @@ async function handleBatchMode( allChunks.push({ type: 'tool', content: toolMessage }); getLog().debug({ toolName: msg.toolName }, 'tool_call'); } - } else if (msg.type === 'result' && msg.sessionId) { - newSessionId = msg.sessionId; + } else if (msg.type === 'result') { + if (msg.sessionId) { + newSessionId = msg.sessionId; + } + if (msg.isError) { + await platform.sendMessage( + conversationId, + '⚠️ AI error. Check your credentials or use /reset.' + ); + return; + } } if (!commandDetected && allChunks.length > MAX_BATCH_TOTAL_CHUNKS) { diff --git a/packages/core/src/utils/error-formatter.test.ts b/packages/core/src/utils/error-formatter.test.ts index 0e3bfe01c8..2fbdeaf6cf 100644 --- a/packages/core/src/utils/error-formatter.test.ts +++ b/packages/core/src/utils/error-formatter.test.ts @@ -19,6 +19,40 @@ describe('classifyAndFormatError', () => { }); }); + describe('OAuth refresh-token errors', () => { + test('detects "refresh token" in message', () => { + const result = classifyAndFormatError(new Error('Your refresh token was already used')); + expect(result).toBe( + '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + ); + }); + + test('detects "could not be refreshed" in message', () => { + const result = classifyAndFormatError(new Error('Your access token could not be refreshed')); + expect(result).toBe( + '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + ); + }); + + test('detects "log out and sign in" in message', () => { + const result = classifyAndFormatError(new Error('Please log out and sign in again')); + expect(result).toBe( + '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + ); + }); + + test('handles full Claude OAuth error message', () => { + const result = classifyAndFormatError( + new Error( + 'Claude Code auth error: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.' + ) + ); + expect(result).toBe( + '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + ); + }); + }); + describe('authentication errors', () => { test('detects "API key" in message', () => { const result = classifyAndFormatError(new Error('Invalid API key provided')); @@ -30,6 +64,13 @@ describe('classifyAndFormatError', () => { expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); }); + test('detects "auth error" prefix in message', () => { + const result = classifyAndFormatError( + new Error('Claude Code auth error: something went wrong') + ); + expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + }); + test('detects "401" in message', () => { const result = classifyAndFormatError(new Error('HTTP 401 Unauthorized')); expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); @@ -232,6 +273,16 @@ describe('classifyAndFormatError', () => { expect(result).toBe('⚠️ AI rate limit reached. Please wait a moment and try again.'); }); + test('OAuth check takes precedence over general auth check', () => { + // Contains both "refresh token" and "auth error" — OAuth branch fires first + const result = classifyAndFormatError( + new Error('Claude Code auth error: refresh token expired') + ); + expect(result).toBe( + '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + ); + }); + test('auth check takes precedence over short-message fallback', () => { const result = classifyAndFormatError(new Error('API key')); expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); diff --git a/packages/core/src/utils/error-formatter.ts b/packages/core/src/utils/error-formatter.ts index 86e51f8a41..2da74b2ff0 100644 --- a/packages/core/src/utils/error-formatter.ts +++ b/packages/core/src/utils/error-formatter.ts @@ -19,10 +19,20 @@ export function classifyAndFormatError(error: Error): string { return '⚠️ AI rate limit reached. Please wait a moment and try again.'; } + // AI/SDK errors - OAuth credential refresh (e.g. "claude /login" token expired) + if ( + message.includes('refresh token') || + message.includes('could not be refreshed') || + message.includes('log out and sign in') + ) { + return '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.'; + } + // AI/SDK errors - authentication if ( message.includes('API key') || message.includes('authentication') || + message.includes('auth error') || message.includes('401') ) { return '⚠️ AI service authentication error. Please check configuration.'; From e30c6b66c06e7e0e2dcf2ad4cf85bd5a27b38e31 Mon Sep 17 00:00:00 2001 From: Cole Medin Date: Sat, 11 Apr 2026 10:40:52 -0500 Subject: [PATCH 2/3] fix: add structured logging to isError path and remove overly broad auth pattern - Add getLog().warn({ conversationId, errorSubtype }, 'ai_result_error') in both handleStreamMode and handleBatchMode isError branches so auth failures are visible server-side instead of silently swallowed - Remove 'access token' from AUTH_PATTERNS in claude.ts and codex.ts; the real OAuth refresh error is already covered by 'refresh token' and 'could not be refreshed', eliminating false-positive auth classification risk Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/orchestrator/orchestrator-agent.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index 251e493af4..f09bb58801 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -959,6 +959,7 @@ async function handleStreamMode( newSessionId = msg.sessionId; } if (msg.isError) { + getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); await platform.sendMessage( conversationId, '⚠️ AI error. Check your credentials or use /reset.' @@ -1080,6 +1081,7 @@ async function handleBatchMode( newSessionId = msg.sessionId; } if (msg.isError) { + getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); await platform.sendMessage( conversationId, '⚠️ AI error. Check your credentials or use /reset.' From 9c4d0fdc5ac448e94395da7970b7f3af87579f9c Mon Sep 17 00:00:00 2001 From: Cole Medin Date: Thu, 16 Apr 2026 09:28:39 -0500 Subject: [PATCH 3/3] fix: route isError results through classifyAndFormatError with provider-specific messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The isError path in stream/batch mode used a hardcoded generic message, bypassing the classifyAndFormatError infrastructure. Now constructs a synthetic Error from errorSubtype and routes through the formatter. Error formatter updated with provider-specific auth detection: - Claude: OAuth token refresh, sign-in expired → guidance to run /login - Codex: 401 retry exhaustion → guidance to run codex login - General: tightened patterns (removed broad 'auth error' substring match) Also persists session ID before early-returning on isError. Co-Authored-By: Claude Opus 4.6 --- .../src/orchestrator/orchestrator-agent.ts | 18 ++-- .../core/src/utils/error-formatter.test.ts | 100 ++++++++++++------ packages/core/src/utils/error-formatter.ts | 33 ++++-- 3 files changed, 104 insertions(+), 47 deletions(-) diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index f09bb58801..d5eb9397b3 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -960,10 +960,11 @@ async function handleStreamMode( } if (msg.isError) { getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); - await platform.sendMessage( - conversationId, - '⚠️ AI error. Check your credentials or use /reset.' - ); + const syntheticError = new Error(msg.errorSubtype ?? 'AI result error'); + await platform.sendMessage(conversationId, classifyAndFormatError(syntheticError)); + if (newSessionId) { + await tryPersistSessionId(session.id, newSessionId); + } return; } if (!commandDetected && platform.sendStructuredEvent) { @@ -1082,10 +1083,11 @@ async function handleBatchMode( } if (msg.isError) { getLog().warn({ conversationId, errorSubtype: msg.errorSubtype }, 'ai_result_error'); - await platform.sendMessage( - conversationId, - '⚠️ AI error. Check your credentials or use /reset.' - ); + const syntheticError = new Error(msg.errorSubtype ?? 'AI result error'); + await platform.sendMessage(conversationId, classifyAndFormatError(syntheticError)); + if (newSessionId) { + await tryPersistSessionId(session.id, newSessionId); + } return; } } diff --git a/packages/core/src/utils/error-formatter.test.ts b/packages/core/src/utils/error-formatter.test.ts index 2fbdeaf6cf..c9c82c867b 100644 --- a/packages/core/src/utils/error-formatter.test.ts +++ b/packages/core/src/utils/error-formatter.test.ts @@ -19,66 +19,97 @@ describe('classifyAndFormatError', () => { }); }); - describe('OAuth refresh-token errors', () => { + describe('Claude OAuth refresh-token errors', () => { test('detects "refresh token" in message', () => { const result = classifyAndFormatError(new Error('Your refresh token was already used')); - expect(result).toBe( - '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' - ); + expect(result).toContain('Claude authentication expired'); + expect(result).toContain('/login'); }); test('detects "could not be refreshed" in message', () => { const result = classifyAndFormatError(new Error('Your access token could not be refreshed')); - expect(result).toBe( - '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' - ); + expect(result).toContain('Claude authentication expired'); }); test('detects "log out and sign in" in message', () => { const result = classifyAndFormatError(new Error('Please log out and sign in again')); - expect(result).toBe( - '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + expect(result).toContain('Claude authentication expired'); + }); + + test('detects "OAuth token has expired" in message', () => { + const result = classifyAndFormatError( + new Error('API Error: 401 OAuth token has expired. Please run /login') ); + expect(result).toContain('Claude authentication expired'); + expect(result).toContain('claude logout && claude login'); }); - test('handles full Claude OAuth error message', () => { + test('detects "sign-in has expired" in message', () => { + const result = classifyAndFormatError( + new Error('Unable to start session: sign-in has expired') + ); + expect(result).toContain('Claude authentication expired'); + }); + + test('handles full Claude OAuth error with refresh token race condition', () => { const result = classifyAndFormatError( new Error( 'Claude Code auth error: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again.' ) ); - expect(result).toBe( - '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' + expect(result).toContain('Claude authentication expired'); + }); + }); + + describe('Claude general auth errors', () => { + test('detects "Claude Code auth error:" prefix for non-OAuth errors', () => { + const result = classifyAndFormatError(new Error('Claude Code auth error: 403 forbidden')); + expect(result).toContain('Claude authentication error'); + expect(result).toContain('/login'); + }); + }); + + describe('Codex auth errors', () => { + test('detects Codex 401 retry exhaustion', () => { + const result = classifyAndFormatError( + new Error('Codex query failed: exceeded retry limit, last status: 401 Unauthorized') ); + expect(result).toContain('Codex authentication error'); + expect(result).toContain('codex login'); + }); + + test('detects Codex query failed with Unauthorized', () => { + const result = classifyAndFormatError(new Error('Codex query failed: Unauthorized')); + expect(result).toContain('Codex authentication error'); + expect(result).toContain('codex login'); }); }); - describe('authentication errors', () => { + describe('general authentication errors', () => { test('detects "API key" in message', () => { const result = classifyAndFormatError(new Error('Invalid API key provided')); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + expect(result).toContain('authentication error'); }); - test('detects "authentication" in message', () => { - const result = classifyAndFormatError(new Error('authentication failed')); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + test('detects "authentication_error" in message', () => { + const result = classifyAndFormatError(new Error('authentication_error: invalid')); + expect(result).toContain('authentication error'); }); - test('detects "auth error" prefix in message', () => { - const result = classifyAndFormatError( - new Error('Claude Code auth error: something went wrong') - ); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + test('detects "authentication error" in message', () => { + const result = classifyAndFormatError(new Error('authentication error')); + expect(result).toContain('authentication error'); }); test('detects "401" in message', () => { const result = classifyAndFormatError(new Error('HTTP 401 Unauthorized')); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + expect(result).toContain('authentication error'); }); - test('detects 401 as standalone in message', () => { - const result = classifyAndFormatError(new Error('Status: 401')); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + test('does not false-positive on generic messages containing "auth"', () => { + // "auth" alone should NOT match — only specific patterns + const result = classifyAndFormatError(new Error('author name missing')); + expect(result).not.toContain('authentication'); }); }); @@ -273,19 +304,24 @@ describe('classifyAndFormatError', () => { expect(result).toBe('⚠️ AI rate limit reached. Please wait a moment and try again.'); }); - test('OAuth check takes precedence over general auth check', () => { - // Contains both "refresh token" and "auth error" — OAuth branch fires first + test('Claude OAuth check takes precedence over general auth check', () => { + // Contains both "refresh token" and "Claude Code auth error:" — OAuth branch fires first const result = classifyAndFormatError( new Error('Claude Code auth error: refresh token expired') ); - expect(result).toBe( - '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.' - ); + expect(result).toContain('Claude authentication expired'); + }); + + test('Codex auth takes precedence over generic Codex error handler', () => { + // Contains "Codex query failed:" AND "401" — Codex auth branch fires first + const result = classifyAndFormatError(new Error('Codex query failed: 401 Unauthorized')); + expect(result).toContain('Codex authentication error'); + expect(result).toContain('codex login'); }); test('auth check takes precedence over short-message fallback', () => { const result = classifyAndFormatError(new Error('API key')); - expect(result).toBe('⚠️ AI service authentication error. Please check configuration.'); + expect(result).toContain('authentication error'); }); test('Codex check is applied before generic fallback', () => { diff --git a/packages/core/src/utils/error-formatter.ts b/packages/core/src/utils/error-formatter.ts index 2da74b2ff0..25658b5cd6 100644 --- a/packages/core/src/utils/error-formatter.ts +++ b/packages/core/src/utils/error-formatter.ts @@ -19,23 +19,42 @@ export function classifyAndFormatError(error: Error): string { return '⚠️ AI rate limit reached. Please wait a moment and try again.'; } - // AI/SDK errors - OAuth credential refresh (e.g. "claude /login" token expired) + // Claude-specific auth errors — OAuth token refresh failures + // These come from Claude Code subprocess stderr or SDK result subtypes. + // Recovery: `/login` in-session or `claude logout && claude login` in terminal. if ( message.includes('refresh token') || message.includes('could not be refreshed') || - message.includes('log out and sign in') + message.includes('log out and sign in') || + message.includes('OAuth token has expired') || + message.includes('sign-in has expired') ) { - return '⚠️ AI authentication error. Please re-login (e.g. `claude /login`) and try again.'; + return '⚠️ Claude authentication expired. Run `/login` inside Claude Code or `claude logout && claude login` in your terminal.'; } - // AI/SDK errors - authentication + // Claude-specific auth errors — general (subprocess crash with auth classification) + if (message.startsWith('Claude Code auth error:')) { + return '⚠️ Claude authentication error. Run `/login` inside Claude Code or check your API key configuration.'; + } + + // Codex-specific auth errors — 401 retry exhaustion + // Codex surfaces auth failures as "exceeded retry limit, last status: 401 Unauthorized" + // Recovery: `codex login` in terminal. + if ( + message.includes('Codex query failed:') && + (message.includes('401') || message.includes('Unauthorized')) + ) { + return '⚠️ Codex authentication error. Run `codex login` in your terminal to re-authenticate.'; + } + + // General AI/SDK authentication errors if ( message.includes('API key') || - message.includes('authentication') || - message.includes('auth error') || + message.includes('authentication_error') || + message.includes('authentication error') || message.includes('401') ) { - return '⚠️ AI service authentication error. Please check configuration.'; + return '⚠️ AI service authentication error. Please check your API key or credentials.'; } // Network errors - timeout