Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 53 additions & 8 deletions apps/daemon/src/run-failure-classification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserve explicit exhaustion phrases such as quota exhausted and quota exceeded as hard quota signals. This new conjunction only recognizes quota when one of wallet/balance/credit/billing/funds/payment/plan also appears, so classify('AGENT_EXECUTION_FAILED', 'quota exhausted') now falls through to the generic retryable execution-failure path; with RATE_LIMITED, the same text becomes retryable rate_limit_429. That changes a genuine terminal quota failure into a retry candidate, contrary to the existing hard_quota suppression contract. The repository already contains the exact upstream phrase quota exhausted in apps/daemon/tests/byok-tools.test.ts, and quota exceeded is also a common provider form. Add tightly scoped alternatives such as quota (?:exceeded|exhausted|depleted|reached) (without restoring bare quota) and include these terse forms in the fixture matrix alongside the empty-output fallback.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment on lines 192 to 193

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the corroborating term in the same message as quota. collectFailureText joins the status error and up to 24 event fragments with newlines, but these two independent regexes search that whole aggregate. Therefore the exact empty-output fallback supplies quota, while an unrelated event such as Failed to load plan supplies plan; together they still return hard_quota, and run-retry-policy suppresses the retry this PR intends to restore. Match corroborators within the same line or a tightly bounded phrase around quota (or classify each collected fragment separately), and add a regression fixture combining EMPTY_OUTPUT_FALLBACK with an unrelated event containing plan or payment that must remain empty_output.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

function isHardQuotaText(parts: string[]): boolean {
return parts.some(isHardQuotaFragment);
}

// A transient, retryable rate limit (distinct from a hard quota). vela/upstream
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
119 changes: 119 additions & 0 deletions apps/daemon/tests/run-failure-classification.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading