diff --git a/apps/daemon/src/run-failure-classification.ts b/apps/daemon/src/run-failure-classification.ts index dd833aa7ebe..629ecdbb7e6 100644 --- a/apps/daemon/src/run-failure-classification.ts +++ b/apps/daemon/src/run-failure-classification.ts @@ -128,7 +128,7 @@ function inferFailureStageFromEvents( return fallback; } -function collectFailureText(input: RunFailureClassificationInput): string { +function collectFailureParts(input: RunFailureClassificationInput): string[] { const parts: string[] = []; const statusError = readString(input.status.error); if (statusError) parts.push(statusError); @@ -143,12 +143,56 @@ function collectFailureText(input: RunFailureClassificationInput): string { parts.push(...eventStderrText(rec.data)); } } - return parts.join('\n'); + return parts; } -function isHardQuotaText(text: string): boolean { - return /\b(session limit|usage limit|limit reached|quota|billing (?:hard )?limit|insufficient[ _-]?(?:quota|credit|credits|funds)|exceeded your current quota|out of credits|no payment method|requires more credits|can only afford)\b|DAILY_LIMIT_EXCEEDED|用户额度不足|额度不足|预扣费额度失败/i - .test(text); +function collectFailureText(input: RunFailureClassificationInput): string { + return collectFailureParts(input).join('\n'); +} + +// A bare `quota` alternative used to live in the alternation below, which made +// this matcher fire on our *own* generic empty-output fallback (server.ts: +// "...then try re-authenticating the agent, checking quota, or switching +// models."). The quota branch is evaluated before `isEmptyOutputText`, so every +// output-less run — for any reason — was reported as an exhausted quota, and +// run-retry-policy suppresses retries outright on `hard_quota`. That is the +// misclassification behind #6143: third-party API runs failed permanently while +// the provider still had quota. +// +// So `quota` on its own is not a signal; it needs a corroborating word. This +// mirrors the detector that already got it right — +// integrations/vela-errors.ts requires one of wallet/balance/credit/billing/funds +// alongside `quota`. Phrases that are unambiguous on their own (session limit, +// insufficient quota, exceeded your current quota…) keep matching directly. +// +// Evaluated per collected fragment, never over the joined text. `collectFailureText` +// concatenates up to 24 unrelated messages with '\n', so whole-text corroboration +// would let *any* fragment mentioning a plan or payment vouch for a bare `quota` +// sitting in a different one — which is precisely how the empty-output fallback +// would find its way back to `hard_quota`. Fragment scope also keeps the `\s+` +// in the adjacency patterns below from bridging two messages across the join. +function isHardQuotaFragment(text: string): boolean { + if (/\b(session limit|usage limit|limit reached|billing (?:hard )?limit|insufficient[ _-]?(?:quota|credit|credits|funds)|exceeded your current quota|out of credits|no payment method|requires more credits|can only afford)\b|DAILY_LIMIT_EXCEEDED|用户额度不足|额度不足|预扣费额度失败/i + .test(text)) { + return true; + } + // `quota` next to an exhaustion word is unambiguous on its own — no wallet or + // balance needed. `quota exhausted` is a real upstream payload in this repo + // (tests/byok-tools.test.ts: `status_msg: 'quota exhausted'`); requiring + // corroboration for it would turn a terminal failure into a retry candidate + // (and under RATE_LIMITED, a retryable `rate_limit_429`), which is the exact + // inverse of the misclassification this function is being fixed for. + if (/\bquota\s+(?:exceeded|exhausted|depleted|reached|used\s+up)\b/i.test(text) + || /\b(?:exceeded|exhausted|out\s+of|ran\s+out\s+of|no\s+remaining)\s+(?:\w+\s+){0,3}quota\b/i + .test(text)) { + return true; + } + return /\bquota\b/i.test(text) && + /\b(wallet|balance|credits?|billing|funds?|payment|plan)\b/i.test(text); +} + +function isHardQuotaText(parts: string[]): boolean { + return parts.some(isHardQuotaFragment); } // A transient, retryable rate limit (distinct from a hard quota). vela/upstream @@ -636,7 +680,8 @@ export function classifyRunFailure( } const errorCode = normalizeCode(input.errorCode ?? input.status.errorCode); - const text = collectFailureText(input); + const parts = collectFailureParts(input); + const text = parts.join('\n'); const retryableHint = latestRetryable(input.events); const amrFailure = classifyAmrAccountFailure(text); const byokOpenCodeProviderNotFound = isByokOpenCodeProviderNotFoundText( @@ -810,8 +855,8 @@ export function classifyRunFailure( ); } - if (errorCode === 'RATE_LIMITED' || serviceFailure === 'RATE_LIMITED' || isHardQuotaText(text) || isRateLimitText(text)) { - const hardQuota = isHardQuotaText(text); + const hardQuota = isHardQuotaText(parts); + if (errorCode === 'RATE_LIMITED' || serviceFailure === 'RATE_LIMITED' || hardQuota || isRateLimitText(text)) { const workspaceCredits = isWorkspaceCreditsText(text); const retryable = hardQuota ? false : (retryableHint ?? true); return classification( diff --git a/apps/daemon/tests/run-failure-classification.test.ts b/apps/daemon/tests/run-failure-classification.test.ts index 86a20bae2c6..8af837b2226 100644 --- a/apps/daemon/tests/run-failure-classification.test.ts +++ b/apps/daemon/tests/run-failure-classification.test.ts @@ -333,6 +333,125 @@ describe('classifyRunFailure', () => { }); }); + // Refs #6143. The daemon's own generic empty-output fallback (server.ts) ends + // with "...then try re-authenticating the agent, checking quota, or switching + // models." `isHardQuotaText` used to carry a bare `quota` alternative, so the + // daemon read its own message back and concluded the user was out of quota. + // + // That is not cosmetic: run-retry-policy suppresses retries outright on + // `hard_quota`, which is why the reporter saw every third-party-API run fail + // permanently while the provider itself had quota left. + const EMPTY_OUTPUT_FALLBACK = + 'Agent completed without producing any output. The model or provider may ' + + 'have returned an empty response. Check the agent logs for upstream ' + + 'errors, then try re-authenticating the agent, checking quota, or ' + + 'switching models.'; + + it('does not read its own empty-output fallback as a hard quota', () => { + expect(classify('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK)).toMatchObject({ + failure_category: 'empty_output', + failure_detail: 'empty_output', + }); + }); + + it('does not let an unrelated fragment corroborate the fallback back into a hard quota', () => { + // `collectFailureText` joins up to 24 unrelated messages with '\n'. If the + // corroboration rule is applied to that joined string, any *other* fragment + // mentioning a plan or a payment vouches for the bare `quota` sitting in the + // empty-output fallback — and the run is suppressed from retrying again. + // Corroboration has to hold within a single collected fragment. + for (const unrelated of [ + 'Switched to the fallback plan for this workspace.', + 'Your payment details were updated 3 days ago.', + 'Restored session from the previous billing period snapshot.', + ]) { + expect( + classify('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK, [ + errorEvent('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK), + errorEvent('AGENT_EXECUTION_FAILED', unrelated), + ]), + unrelated, + ).toMatchObject({ + failure_category: 'empty_output', + failure_detail: 'empty_output', + }); + } + + // Same boundary for the exhaustion collocations: `\s+` matches the '\n' the + // fragments are joined with, so a message ending in `quota` must not pair up + // with the next message starting in `exceeded`. + expect( + classify('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK, [ + errorEvent('AGENT_EXECUTION_FAILED', 'Reported usage against the quota'), + errorEvent('AGENT_EXECUTION_FAILED', 'exceeded the configured step budget'), + ]), + ).toMatchObject({ + failure_category: 'empty_output', + failure_detail: 'empty_output', + }); + + // …while a fragment that corroborates *itself* still classifies, even when + // it arrives alongside the fallback. + expect( + classify('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK, [ + errorEvent('AGENT_EXECUTION_FAILED', EMPTY_OUTPUT_FALLBACK), + errorEvent( + 'AGENT_EXECUTION_FAILED', + 'You exceeded your current quota, please check your plan and billing details.', + ), + ]), + ).toMatchObject({ + failure_category: 'rate_limit', + failure_detail: 'hard_quota', + retryable: false, + }); + }); + + it('still treats a real quota exhaustion as a hard quota', () => { + // The word `quota` alone must not be the trigger, but a corroborated quota + // message still has to classify — otherwise this fix trades one silent + // misclassification for another. + for (const text of [ + 'You exceeded your current quota, please check your plan and billing details.', + 'Insufficient quota for the current billing period.', + ]) { + expect(classify('AGENT_EXECUTION_FAILED', text), text).toMatchObject({ + failure_category: 'rate_limit', + failure_detail: 'hard_quota', + retryable: false, + }); + } + // Explicit exhaustion collocations must survive the corroboration rule. + // `quota exhausted` is a real upstream payload in this repo + // (tests/byok-tools.test.ts: `status_msg: 'quota exhausted'`), and dropping + // it would turn a terminal quota failure into a retry candidate — the exact + // inverse of the bug this PR fixes. + for (const text of [ + 'quota exhausted', + 'quota exceeded', + 'Quota depleted for this account.', + 'You have exceeded your monthly quota.', + 'HTTP 429: out of quota', + ]) { + expect(classify('AGENT_EXECUTION_FAILED', text), text).toMatchObject({ + failure_detail: 'hard_quota', + retryable: false, + }); + // With RATE_LIMITED the stakes are higher: without the phrase match this + // becomes a retryable rate_limit_429 and the suppression contract breaks. + expect(classify('RATE_LIMITED', text), `RATE_LIMITED ${text}`).toMatchObject({ + failure_detail: 'hard_quota', + retryable: false, + }); + } + // The Chinese vela pre-charge text has its own, more specific detail and + // must keep it — it is routed before the quota branch. + expect(classify('AGENT_EXECUTION_FAILED', '用户额度不足')).toMatchObject({ + failure_detail: 'amr_insufficient_balance', + retryable: false, + }); + }); + it('maps upstream failures to retry guidance', () => { expect(classify('UPSTREAM_UNAVAILABLE', 'HTTP 503 upstream unavailable')).toMatchObject({ failure_category: 'upstream_unavailable',