From 940dc455b0dba5690a12a576e7c9fe3eeca56a4d Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Mon, 22 Jun 2026 10:57:44 +0000 Subject: [PATCH] fix(llmist): classify OpenRouter provider errors instead of crashing the agent --- CLAUDE.md | 2 + src/backends/llmist/index.ts | 69 ++++++- src/backends/llmist/openrouterErrors.ts | 180 +++++++++++++++++++ tests/unit/backends/llmist.test.ts | 109 +++++++++++ tests/unit/backends/openrouterErrors.test.ts | 162 +++++++++++++++++ 5 files changed, 514 insertions(+), 8 deletions(-) create mode 100644 src/backends/llmist/openrouterErrors.ts create mode 100644 tests/unit/backends/openrouterErrors.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 2bcf5010..46414232 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -170,6 +170,8 @@ Auth: - **Codex subscription**: store `CODEX_AUTH_JSON` credential (contents of `~/.codex/auth.json` after `codex login`). CASCADE persists refreshed tokens back to the DB after each run. - **API-key providers**: store `OPENAI_API_KEY` / other keys as project credentials. +**OpenRouter provider error classification (MNG-1646).** When llmist's OpenRouter provider returns a configuration / billing failure (HTTP 402 "Insufficient credits", 401 unauthorized, 429 rate-limited, 503 model-unavailable), the `LlmistEngine` adapter catches the error before it hits the generic `agent_execution` Sentry path. The error is classified by `classifyOpenRouterError` in `src/backends/llmist/openrouterErrors.ts`, captured under the stable Sentry tag key `openrouter_provider_error` (values such as `openrouter_insufficient_credits` for filtering), and surfaced to the PM card as an actionable plain-English summary (`OpenRouter rejected the request because the account has insufficient credits. Top up the OpenRouter balance at https://openrouter.ai/credits or switch the project to a different model/engine before retrying.`) instead of a raw stack trace. Errors that don't match the OpenRouter shape are re-thrown so the shared pipeline keeps capturing them as generic `agent_execution` crashes. + ## Environment Required: diff --git a/src/backends/llmist/index.ts b/src/backends/llmist/index.ts index 36ba8dbb..1f6fb401 100644 --- a/src/backends/llmist/index.ts +++ b/src/backends/llmist/index.ts @@ -13,9 +13,16 @@ import { createAgentLogger } from '../../agents/utils/logging.js'; import { createTrackingContext } from '../../agents/utils/tracking.js'; import { CUSTOM_MODELS } from '../../config/customModels.js'; import { getSessionState } from '../../gadgets/sessionState.js'; +import { captureException } from '../../sentry.js'; import { createLLMCallLogger } from '../../utils/llmLogging.js'; import { LLMIST_ENGINE_DEFINITION } from '../catalog.js'; import type { AgentEngine, AgentEngineResult, AgentExecutionPlan } from '../types.js'; +import { + classifyOpenRouterError, + formatOpenRouterErrorMessage, + OPENROUTER_ERROR_SENTRY_TAG, + openRouterErrorSentryTagValue, +} from './openrouterErrors.js'; /** * LLMist engine adapter — executes agents using the llmist SDK. @@ -164,15 +171,61 @@ export class LlmistEngine implements AgentEngine { runId, }); - // Run the agent event loop (includes loop detection, session notices, etc.) + // Run the agent event loop (includes loop detection, session notices, etc.). + // Provider-side configuration errors (OpenRouter credit exhaustion, auth + // failures, model-unavailable, rate limits) surface as plain `Error` + // instances from the llmist SDK. We classify them here so they: + // 1. don't bubble up as `agent_execution` Sentry crashes (which would + // pollute the on-call dashboard with billing/config issues), and + // 2. produce an actionable PM-card message instead of a stack-trace. + // Any other failure is re-thrown so the shared execution pipeline at + // `src/agents/shared/executionPipeline.ts` records it as a generic + // agent_execution failure with full stack capture (unchanged behavior). const agent = builder.ask(taskPrompt); - const result = await runAgentLoop( - agent, - log, - trackingContext, - agentInput.interactive === true, - agentInput.autoAccept === true, - ); + let result: Awaited>; + try { + result = await runAgentLoop( + agent, + log, + trackingContext, + agentInput.interactive === true, + agentInput.autoAccept === true, + ); + } catch (err) { + const kind = classifyOpenRouterError(err); + if (!kind) throw err; + + const rawMessage = err instanceof Error ? err.message : String(err); + const friendly = formatOpenRouterErrorMessage(kind, rawMessage); + log.error('OpenRouter provider error', { + kind, + rawMessage, + model, + runId, + }); + captureException(err, { + tags: { + [OPENROUTER_ERROR_SENTRY_TAG]: openRouterErrorSentryTagValue(kind), + engine: this.definition.id, + agent: agentType, + }, + extra: { model, runId }, + level: 'warning', + }); + const cost = (() => { + try { + return agent.getTree?.()?.getTotalCost() ?? 0; + } catch { + return 0; + } + })(); + return { + success: false, + output: '', + error: friendly, + cost, + }; + } log.info('Agent completed', { iterations: result.iterations, diff --git a/src/backends/llmist/openrouterErrors.ts b/src/backends/llmist/openrouterErrors.ts new file mode 100644 index 00000000..9dd5d597 --- /dev/null +++ b/src/backends/llmist/openrouterErrors.ts @@ -0,0 +1,180 @@ +/** + * OpenRouter error classification for llmist-backed agent runs. + * + * Llmist's OpenRouter provider wraps upstream errors with friendly prefixes + * such as `OpenRouter: Insufficient credits...` (see `enhanceError` in the + * llmist SDK). These wrapped messages are the most reliable signal that the + * underlying failure was a provider-side configuration problem rather than a + * transient runtime issue — none of them are retryable, none are caused by + * CASCADE bugs, and none should be treated like ordinary agent execution + * crashes (Sentry tag `agent_execution`). + * + * This module exposes: + * + * - `OpenRouterErrorKind` — the actionable categories we care about. + * - `classifyOpenRouterError(err)` — returns the kind for any wrapped + * OpenRouter error, or `null` for everything else. + * - `formatOpenRouterErrorMessage(kind, raw)` — produces the operator-facing + * summary written to the run row / PM card. + * - `OPENROUTER_ERROR_SENTRY_TAG` / `openRouterErrorSentryTagValue(kind)` — + * stable Sentry tag values so credit-exhaustion failures are filterable + * and don't drown out real agent crashes. + * + * The classifier is intentionally string-based: llmist surfaces the error as a + * regular `Error` with no preserved status code or provider-specific class, + * and we never want to crash the worker pipeline because llmist changed the + * error class hierarchy. + */ + +export type OpenRouterErrorKind = + | 'insufficient_credits' + | 'rate_limit' + | 'unauthorized' + | 'model_unavailable' + | 'other'; + +/** Stable Sentry tag name. Used by `captureException({ tags: { source: ... } })`. */ +export const OPENROUTER_ERROR_SENTRY_TAG = 'openrouter_provider_error'; + +/** + * Map an `OpenRouterErrorKind` to the Sentry `source` tag value. + * + * Insufficient-credit failures get their own tag so operator dashboards can + * filter them out (they are an account / billing problem, not a CASCADE bug). + */ +export function openRouterErrorSentryTagValue(kind: OpenRouterErrorKind): string { + switch (kind) { + case 'insufficient_credits': + return 'openrouter_insufficient_credits'; + case 'rate_limit': + return 'openrouter_rate_limit'; + case 'unauthorized': + return 'openrouter_unauthorized'; + case 'model_unavailable': + return 'openrouter_model_unavailable'; + default: + return 'openrouter_provider_error'; + } +} + +function extractMessage(err: unknown): string | null { + if (err === null || err === undefined) return null; + if (typeof err === 'string') return err; + if (err instanceof Error) return err.message ?? null; + if (typeof err === 'object' && 'message' in err) { + const candidate = (err as { message?: unknown }).message; + if (typeof candidate === 'string') return candidate; + } + return null; +} + +/** + * Classify an error originating from llmist's OpenRouter provider. + * + * Returns `null` when the error isn't OpenRouter-flavored. The classifier + * recognizes both: + * + * - llmist's wrapped form: messages starting with `OpenRouter: ...` + * (produced by `enhanceError` in `node_modules/llmist`). + * - The unwrapped HTTP signals: explicit `402`, `Insufficient credits`, + * `429`, `rate limit`, `401`, `Unauthorized`, `503`, etc. Operators may + * extend the llmist SDK or swap models in ways that change the wrapping, + * so the classifier covers both shapes. + */ +export function classifyOpenRouterError(err: unknown): OpenRouterErrorKind | null { + const raw = extractMessage(err); + if (!raw) return null; + const message = raw.toLowerCase(); + + const hasOpenRouterPrefix = message.includes('openrouter:'); + const mentionsCredits = + message.includes('insufficient credits') || + message.includes('insufficient credit') || + message.includes('insufficient balance'); + const mentionsPayment = message.includes('402') || message.includes('payment required'); + + if (mentionsCredits || (hasOpenRouterPrefix && mentionsPayment)) { + return 'insufficient_credits'; + } + + // Plain HTTP 402 from any upstream that wasn't wrapped — still a credit / + // payment failure on the user's side. Treat the same way. + if (mentionsPayment) { + return 'insufficient_credits'; + } + + if (!hasOpenRouterPrefix) { + return null; + } + + if (message.includes('rate limit') || message.includes('429')) { + return 'rate_limit'; + } + if ( + message.includes('authentication failed') || + message.includes('unauthorized') || + message.includes('401') + ) { + return 'unauthorized'; + } + if ( + message.includes('temporarily unavailable') || + message.includes('model unavailable') || + message.includes('503') + ) { + return 'model_unavailable'; + } + + return 'other'; +} + +/** + * Build an operator-facing summary for a classified OpenRouter error. + * + * The summary is what shows up in `AgentResult.error`, which the PM lifecycle + * surfaces verbatim as `❌ Agent failed: ` on the work item. We keep + * the original llmist message in parentheses so engineers debugging from logs + * can still see the exact wording that came back from OpenRouter, but lead + * with a short, plain-English explanation + the actionable next step. + * + * Truncates excessively long raw messages so PM cards (especially Trello, + * which has a 16k comment cap) never break on multi-paragraph payloads. + */ +export function formatOpenRouterErrorMessage( + kind: OpenRouterErrorKind, + rawMessage: string | null | undefined, +): string { + const trimmed = rawMessage ? truncate(rawMessage.trim(), 600) : ''; + const detail = trimmed ? ` (details: ${trimmed})` : ''; + + switch (kind) { + case 'insufficient_credits': + return ( + `OpenRouter rejected the request because the account has insufficient credits. ` + + `Top up the OpenRouter balance at https://openrouter.ai/credits or switch the project to a ` + + `different model/engine before retrying.${detail}` + ); + case 'rate_limit': + return ( + `OpenRouter rate-limited the request. Reduce the project's request rate, upgrade the ` + + `OpenRouter plan, or switch to a different model before retrying.${detail}` + ); + case 'unauthorized': + return ( + `OpenRouter rejected the request as unauthorized. Verify the OPENROUTER_API_KEY project ` + + `credential is current and has access to the configured model.${detail}` + ); + case 'model_unavailable': + return ( + `OpenRouter reports the requested model is temporarily unavailable. Switch the project ` + + `to a different model or retry after the provider recovers.${detail}` + ); + default: + return `OpenRouter provider error: the request could not be completed.${detail}`; + } +} + +function truncate(value: string, maxLen: number): string { + if (value.length <= maxLen) return value; + return `${value.slice(0, maxLen - 1)}…`; +} diff --git a/tests/unit/backends/llmist.test.ts b/tests/unit/backends/llmist.test.ts index 7b6c0de6..9dd4d67c 100644 --- a/tests/unit/backends/llmist.test.ts +++ b/tests/unit/backends/llmist.test.ts @@ -94,13 +94,23 @@ vi.mock('../../../src/gadgets/sessionState.js', async (importOriginal) => { }; }); +vi.mock('../../../src/sentry.js', () => ({ + captureException: vi.fn(), + addBreadcrumb: vi.fn(), + setTag: vi.fn(), + flush: vi.fn(async () => {}), + sentryEnabled: false, +})); + import { runAgentLoop } from '../../../src/agents/utils/agentLoop.js'; import { LlmistEngine } from '../../../src/backends/llmist/index.js'; import type { AgentExecutionPlan } from '../../../src/backends/types.js'; import { getSessionState } from '../../../src/gadgets/sessionState.js'; +import { captureException } from '../../../src/sentry.js'; const mockRunAgentLoop = vi.mocked(runAgentLoop); const mockGetSessionState = vi.mocked(getSessionState); +const mockCaptureException = vi.mocked(captureException); function makeInput(agentType = 'implementation'): AgentExecutionPlan { return { @@ -429,3 +439,102 @@ describe('LlmistEngine.execute', () => { ); }); }); + +describe('LlmistEngine.execute — OpenRouter provider error handling', () => { + beforeEach(() => { + mockGetSessionState.mockReturnValue({ prUrl: null } as ReturnType); + mockCaptureException.mockClear(); + }); + + it('converts an OpenRouter "Insufficient credits" failure into a handled AgentEngineResult', async () => { + // Reproduces MNG-1646: llmist's OpenRouterProvider.executeStreamRequest re-throws + // HTTP 402 as `new Error("OpenRouter: Insufficient credits...")`. Without the + // classifier, the worker pipeline would tag this as a generic agent_execution + // crash and dump a stack trace onto the PM card. + mockRunAgentLoop.mockRejectedValueOnce( + new Error( + 'OpenRouter: Insufficient credits. Add funds at https://openrouter.ai/credits\n' + + 'Original error: 402 Payment Required', + ), + ); + + const engine = new LlmistEngine(); + const result = await engine.execute(makeInput()); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + expect(result.error).toContain('OpenRouter'); + expect(result.error).toContain('insufficient credits'); + // The actionable next-step link is included + expect(result.error).toContain('https://openrouter.ai/credits'); + // Cost is reported as 0 (no LLM calls completed) but the call itself doesn't throw + expect(result.cost).toBe(0); + expect(result.output).toBe(''); + expect(result.prUrl).toBeUndefined(); + }); + + it('captures Sentry with a credit-exhaustion tag (not the generic agent_execution tag)', async () => { + mockRunAgentLoop.mockRejectedValueOnce( + new Error('OpenRouter: Insufficient credits. Add funds at https://openrouter.ai/credits'), + ); + + const engine = new LlmistEngine(); + await engine.execute(makeInput('review')); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + const [capturedErr, opts] = mockCaptureException.mock.calls[0]; + expect(capturedErr).toBeInstanceOf(Error); + expect((capturedErr as Error).message).toContain('Insufficient credits'); + expect(opts?.tags).toMatchObject({ + openrouter_provider_error: 'openrouter_insufficient_credits', + engine: 'llmist', + agent: 'review', + }); + expect(opts?.level).toBe('warning'); + }); + + it('uses distinct Sentry tags for each OpenRouter error kind', async () => { + mockRunAgentLoop.mockRejectedValueOnce( + new Error( + 'OpenRouter: Authentication failed. Check that OPENROUTER_API_KEY is set correctly.\n' + + 'Original error: 401 Unauthorized', + ), + ); + + const engine = new LlmistEngine(); + const result = await engine.execute(makeInput()); + + expect(result.success).toBe(false); + expect(result.error).toContain('OPENROUTER_API_KEY'); + expect(mockCaptureException.mock.calls[0]?.[1]?.tags).toMatchObject({ + openrouter_provider_error: 'openrouter_unauthorized', + }); + }); + + it('re-throws non-OpenRouter errors so the shared pipeline can record them as agent crashes', async () => { + // Non-classified errors must propagate so executeAgentPipeline can tag them as + // `source: 'agent_execution'` and produce a stack-trace Sentry event. If this + // regressed, every llmist crash would silently become a handled "warning". + const cause = new Error('Network error: ECONNRESET while reading stream'); + mockRunAgentLoop.mockRejectedValueOnce(cause); + + const engine = new LlmistEngine(); + await expect(engine.execute(makeInput())).rejects.toBe(cause); + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it('handles a bare HTTP 402 from any upstream as insufficient_credits', async () => { + // If llmist's wrapping ever fails (or a future provider proxies the raw error), + // we still want to classify HTTP 402 as a billing problem. + mockRunAgentLoop.mockRejectedValueOnce(new Error('Request failed: 402 Payment Required')); + + const engine = new LlmistEngine(); + const result = await engine.execute(makeInput()); + + expect(result.success).toBe(false); + expect(result.error).toContain('insufficient credits'); + expect(mockCaptureException.mock.calls[0]?.[1]?.tags).toMatchObject({ + openrouter_provider_error: 'openrouter_insufficient_credits', + }); + }); +}); diff --git a/tests/unit/backends/openrouterErrors.test.ts b/tests/unit/backends/openrouterErrors.test.ts new file mode 100644 index 00000000..457df5f0 --- /dev/null +++ b/tests/unit/backends/openrouterErrors.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from 'vitest'; + +import { + classifyOpenRouterError, + formatOpenRouterErrorMessage, + OPENROUTER_ERROR_SENTRY_TAG, + openRouterErrorSentryTagValue, +} from '../../../src/backends/llmist/openrouterErrors.js'; + +describe('classifyOpenRouterError', () => { + it('returns "insufficient_credits" for the llmist-wrapped 402 message', () => { + // This is the exact message shape emitted by llmist's OpenRouterProvider.enhanceError + // for HTTP 402 / Insufficient credits (see node_modules/llmist/dist/index.js). + const err = new Error( + 'OpenRouter: Insufficient credits. Add funds at https://openrouter.ai/credits\n' + + 'Original error: 402 Payment Required', + ); + expect(classifyOpenRouterError(err)).toBe('insufficient_credits'); + }); + + it('returns "insufficient_credits" for a bare HTTP 402 from any upstream', () => { + // Captured when the wrapping fails or when llmist surfaces the raw HTTP error. + const err = new Error('Request failed with status code 402: Payment Required'); + expect(classifyOpenRouterError(err)).toBe('insufficient_credits'); + }); + + it('returns "insufficient_credits" for "Insufficient balance" wording', () => { + // Some providers use "balance" instead of "credits" — both mean the same thing. + const err = new Error('OpenRouter: Insufficient balance. Top up at openrouter.ai'); + expect(classifyOpenRouterError(err)).toBe('insufficient_credits'); + }); + + it('returns "rate_limit" for the llmist-wrapped 429 message', () => { + const err = new Error( + 'OpenRouter: Rate limit exceeded. Consider upgrading your plan or reducing request frequency.\n' + + 'Original error: 429 Too Many Requests', + ); + expect(classifyOpenRouterError(err)).toBe('rate_limit'); + }); + + it('returns "unauthorized" for the llmist-wrapped 401 message', () => { + const err = new Error( + 'OpenRouter: Authentication failed. Check that OPENROUTER_API_KEY is set correctly.\n' + + 'Original error: 401 Unauthorized', + ); + expect(classifyOpenRouterError(err)).toBe('unauthorized'); + }); + + it('returns "model_unavailable" for the llmist-wrapped 503 message', () => { + const err = new Error( + "OpenRouter: Model temporarily unavailable. Try a different model or use the 'models' fallback option for automatic retry.\n" + + 'Original error: 503 Service Unavailable', + ); + expect(classifyOpenRouterError(err)).toBe('model_unavailable'); + }); + + it('returns "other" for an OpenRouter-prefixed message that does not match a known kind', () => { + const err = new Error('OpenRouter: weird new failure mode the SDK does not categorize'); + expect(classifyOpenRouterError(err)).toBe('other'); + }); + + it('returns null for an error with no message', () => { + expect(classifyOpenRouterError(new Error())).toBeNull(); + }); + + it('returns null for non-OpenRouter errors that lack the prefix and a 402', () => { + expect(classifyOpenRouterError(new Error('Network error: ECONNRESET'))).toBeNull(); + expect( + classifyOpenRouterError(new Error('Agent terminated due to persistent loop')), + ).toBeNull(); + }); + + it('returns null for null / undefined inputs without crashing', () => { + expect(classifyOpenRouterError(null)).toBeNull(); + expect(classifyOpenRouterError(undefined)).toBeNull(); + }); + + it('accepts string errors and plain message-bearing objects', () => { + expect(classifyOpenRouterError('OpenRouter: Insufficient credits.')).toBe( + 'insufficient_credits', + ); + expect(classifyOpenRouterError({ message: 'OpenRouter: Rate limit exceeded.' })).toBe( + 'rate_limit', + ); + }); + + it('is case-insensitive on the prefix and the keyword', () => { + expect(classifyOpenRouterError(new Error('openrouter: INSUFFICIENT CREDITS.'))).toBe( + 'insufficient_credits', + ); + }); +}); + +describe('formatOpenRouterErrorMessage', () => { + it('produces an actionable summary for insufficient_credits including the openrouter.ai link', () => { + const raw = 'OpenRouter: Insufficient credits. Add funds at https://openrouter.ai/credits'; + const out = formatOpenRouterErrorMessage('insufficient_credits', raw); + expect(out).toContain('insufficient credits'); + expect(out).toContain('https://openrouter.ai/credits'); + expect(out).toContain('switch the project to a different model'); + // Original raw message is preserved inside parens for debuggability + expect(out).toContain('details:'); + }); + + it('preserves the original message for rate_limit', () => { + const raw = 'OpenRouter: Rate limit exceeded.'; + const out = formatOpenRouterErrorMessage('rate_limit', raw); + expect(out).toContain('rate-limited'); + expect(out).toContain('details:'); + expect(out).toContain(raw); + }); + + it('preserves the original message for unauthorized', () => { + const raw = 'OpenRouter: Authentication failed.'; + const out = formatOpenRouterErrorMessage('unauthorized', raw); + expect(out).toContain('unauthorized'); + expect(out).toContain('OPENROUTER_API_KEY'); + }); + + it('preserves the original message for model_unavailable', () => { + const raw = 'OpenRouter: Model temporarily unavailable.'; + const out = formatOpenRouterErrorMessage('model_unavailable', raw); + expect(out).toContain('temporarily unavailable'); + }); + + it('produces a generic summary for the "other" kind', () => { + const out = formatOpenRouterErrorMessage('other', 'OpenRouter: something else.'); + expect(out).toContain('OpenRouter provider error'); + }); + + it('omits the details suffix when the raw message is missing', () => { + expect(formatOpenRouterErrorMessage('insufficient_credits', null)).not.toContain('details:'); + expect(formatOpenRouterErrorMessage('insufficient_credits', '')).not.toContain('details:'); + }); + + it('truncates excessively long raw messages so PM-card comments stay within provider caps', () => { + // Trello has a 16k comment limit; we cap the embedded raw message at ~600 chars + // so the final summary never blows past it even when nested inside a card. + const long = `${'OpenRouter: '.repeat(200)}credits gone`; + const out = formatOpenRouterErrorMessage('insufficient_credits', long); + expect(out.length).toBeLessThan(1500); + expect(out).toContain('…'); + }); +}); + +describe('openRouterErrorSentryTagValue', () => { + it('maps each kind to a stable tag value distinct from the others', () => { + // The values are part of the operator-facing Sentry filter contract — if they + // change, every Sentry saved query needs to be updated. Pin them here. + expect(openRouterErrorSentryTagValue('insufficient_credits')).toBe( + 'openrouter_insufficient_credits', + ); + expect(openRouterErrorSentryTagValue('rate_limit')).toBe('openrouter_rate_limit'); + expect(openRouterErrorSentryTagValue('unauthorized')).toBe('openrouter_unauthorized'); + expect(openRouterErrorSentryTagValue('model_unavailable')).toBe('openrouter_model_unavailable'); + expect(openRouterErrorSentryTagValue('other')).toBe('openrouter_provider_error'); + }); + + it('exposes the stable Sentry tag key', () => { + expect(OPENROUTER_ERROR_SENTRY_TAG).toBe('openrouter_provider_error'); + }); +});