fix(core): surface auth errors instead of silently dropping them (#1089)

* 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) <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* fix: route isError results through classifyAndFormatError with provider-specific messages

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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cole Medin 2026-04-16 09:36:40 -05:00 committed by GitHub
parent 818854474f
commit 7721259bdc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 157 additions and 19 deletions

View file

@ -954,8 +954,19 @@ 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) {
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;
}
if (!commandDetected && platform.sendStructuredEvent) {
await platform.sendStructuredEvent(conversationId, msg);
}
@ -1066,8 +1077,19 @@ 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) {
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;
}
}
if (!commandDetected && allChunks.length > MAX_BATCH_TOTAL_CHUNKS) {

View file

@ -19,25 +19,97 @@ describe('classifyAndFormatError', () => {
});
});
describe('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.');
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).toContain('Claude authentication expired');
expect(result).toContain('/login');
});
test('detects "authentication" in message', () => {
const result = classifyAndFormatError(new Error('authentication failed'));
expect(result).toBe('⚠️ AI service authentication error. Please check configuration.');
test('detects "could not be refreshed" in message', () => {
const result = classifyAndFormatError(new Error('Your access token could not be refreshed'));
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).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('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).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('general authentication errors', () => {
test('detects "API key" in message', () => {
const result = classifyAndFormatError(new Error('Invalid API key provided'));
expect(result).toContain('authentication error');
});
test('detects "authentication_error" in message', () => {
const result = classifyAndFormatError(new Error('authentication_error: invalid'));
expect(result).toContain('authentication error');
});
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');
});
});
@ -232,9 +304,24 @@ describe('classifyAndFormatError', () => {
expect(result).toBe('⚠️ AI rate limit reached. Please wait a moment and try again.');
});
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).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', () => {

View file

@ -19,13 +19,42 @@ export function classifyAndFormatError(error: Error): string {
return '⚠️ AI rate limit reached. Please wait a moment and try again.';
}
// AI/SDK errors - authentication
// 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('OAuth token has expired') ||
message.includes('sign-in has expired')
) {
return '⚠️ Claude authentication expired. Run `/login` inside Claude Code or `claude logout && claude login` in your terminal.';
}
// 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('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