Skip to content

Commit 364efc9

Browse files
esokulluclaude
andcommitted
Classify planner failures on the shapes providers actually emit
The recovery card's whole job is deciding whether a failure is fixed by opening Providers or by retrying, and the first classifier got the two most common browser cases backwards: Chrome's "Failed to fetch" and Firefox's "NetworkError…" — what a stopped local provider looks like from inside the extension — both fell through to `provider`, while a bare scan for 5xx-shaped digits read "max_tokens 512" as a server error and led with a Retry that could only fail the same way. Status codes are now only trusted where they read as a status, textual auth rejections ("Unauthorized", "invalid_api_key") are recognized without a code, and a remaining 4xx stays `provider` because retrying an argument the provider rejected is not the fix. Also: carry `failureKind` through the planner gate wrapper, which was dropping it before any caller could read it; name the failing provider in the card, since "open Providers" is only actionable when the user knows which one to look at; and end the detail sentence before appending "No tools ran." Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 23b45ec commit 364efc9

7 files changed

Lines changed: 180 additions & 16 deletions

File tree

src/chrome/src/agent/agent.js

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,38 @@ function isBrowserNewTabUrl(url) {
127127
return BROWSER_NEW_TAB_URL_PREFIXES.some(prefix => value.startsWith(prefix));
128128
}
129129

130+
// Only read a 3-digit number as a status code where it actually reads like
131+
// one — prefixed by a status word, leading the message, or parenthesized.
132+
// A bare scan would turn "max_tokens 512" into a 5xx and recommend a retry
133+
// that can only fail the same way.
134+
const PLANNER_FAILURE_STATUS_PATTERNS = [
135+
/\b(?:error|http|status|code|returned|response)\s*[:#-]?\s*(\d{3})\b/i,
136+
/^\s*\(?(\d{3})\)?\b/,
137+
/\((\d{3})\)/,
138+
];
139+
const PLANNER_AUTH_FAILURE_RE = /\b(?:unauthori[sz]ed|unauthenticated|forbidden|permission denied|invalid credentials|(?:invalid|incorrect|missing|expired)[\s_-]*api[\s_-]*key|api[\s_-]*key[\s_-]*(?:is\s*)?(?:missing|invalid|incorrect|expired|not\s*set|required)|authentication|(?:expired|invalid)[\s_-]*token|token[\s_-]*expired)\b/i;
140+
// "Failed to fetch" (Chrome) and "NetworkError…" (Firefox) are the shapes a
141+
// dead local provider or an offline machine actually produces, so they have
142+
// to land in the bucket that offers Retry first.
143+
const PLANNER_TRANSIENT_FAILURE_RE = /\b(?:timed?\s*out|timeout|network\w*|failed to fetch|fetch failed|load failed|connection (?:closed|reset|refused|error)|econn\w+|socket hang up|temporar(?:y|ily)|unavailable|overloaded|rate[\s_-]?limit\w*|too many requests|bad gateway|gateway timeout)\b/i;
144+
130145
function plannerRequestFailureKind(detail) {
131146
const text = String(detail || '');
132-
const statusMatch = text.match(/\b(?:error|http|status)\s*[:#-]?\s*(\d{3})\b/i)
133-
|| text.match(/\b(401|403|408|425|429|5\d\d)\b/);
134-
const status = Number(statusMatch?.[1] || 0);
135-
if (status === 401 || status === 403) return 'auth';
136-
if ([408, 425, 429].includes(status) || status >= 500) return 'transient';
137-
if (/\b(?:timed?\s*out|timeout|network|fetch failed|connection (?:closed|reset|refused)|econn\w+|temporar(?:y|ily)|unavailable)\b/i.test(text)) {
138-
return 'transient';
147+
let status = 0;
148+
for (const pattern of PLANNER_FAILURE_STATUS_PATTERNS) {
149+
const match = text.match(pattern);
150+
if (match) {
151+
status = Number(match[1]);
152+
break;
153+
}
139154
}
155+
if (status === 401 || status === 403) return 'auth';
156+
if ([408, 425, 429].includes(status) || (status >= 500 && status <= 599)) return 'transient';
157+
if (PLANNER_AUTH_FAILURE_RE.test(text)) return 'auth';
158+
// A remaining 4xx is a request the provider rejected on its merits; a retry
159+
// of the same request is not the fix.
160+
if (status >= 400 && status <= 499) return 'provider';
161+
if (PLANNER_TRANSIENT_FAILURE_RE.test(text)) return 'transient';
140162
return 'provider';
141163
}
142164

@@ -7233,6 +7255,9 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
72337255
reason: gate.reason,
72347256
requestKind: gate.requestKind,
72357257
requiresStateChange: gate.requiresStateChange,
7258+
// Carried so a caller can tell an auth failure from a transient one
7259+
// without re-parsing the message text.
7260+
...(gate.failureKind ? { failureKind: gate.failureKind } : {}),
72367261
};
72377262
}
72387263

@@ -7412,7 +7437,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
74127437
500,
74137438
{ collapseWhitespace: true },
74147439
);
7415-
const message = `Planner request failed before a valid response was available: ${detail} No tools ran.`;
7440+
const detailSentence = /[.!?]$/.test(detail) ? detail : `${detail}.`;
7441+
const message = `Planner request failed before a valid response was available: ${detailSentence} No tools ran.`;
74167442
const failureKind = plannerRequestFailureKind(detail);
74177443
onUpdate('warning', {
74187444
code: 'planner_request_failed',

src/chrome/src/ui/sidepanel.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5255,6 +5255,15 @@ function renderPlannerRequestFailure(assistantEl, data, retryPayload = null) {
52555255
const message = document.createElement('div');
52565256
message.className = 'planner-request-failure-message';
52575257
message.textContent = data.message || 'Planner request failed before a valid response was available.';
5258+
// Name the provider that failed — with several configured, "open Providers"
5259+
// is only actionable if the user knows which one to look at. The label is a
5260+
// proper noun, so it needs no translation.
5261+
if (data.provider) {
5262+
const providerName = document.createElement('div');
5263+
providerName.className = 'planner-request-failure-provider';
5264+
providerName.textContent = data.provider;
5265+
message.appendChild(providerName);
5266+
}
52585267

52595268
const actions = document.createElement('div');
52605269
actions.className = 'planner-request-failure-actions';

src/chrome/styles/sidepanel.css

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2877,6 +2877,13 @@ body {
28772877
color: var(--error-soft);
28782878
}
28792879

2880+
.planner-request-failure-provider {
2881+
margin-top: 4px;
2882+
font-size: 11px;
2883+
font-weight: 600;
2884+
opacity: 0.85;
2885+
}
2886+
28802887
.planner-request-failure-actions {
28812888
display: flex;
28822889
flex-wrap: wrap;

src/firefox/src/agent/agent.js

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,38 @@ function normalizeDoneOutcome(value) {
122122
return DONE_OUTCOMES.has(outcome) ? outcome : null;
123123
}
124124

125+
// Only read a 3-digit number as a status code where it actually reads like
126+
// one — prefixed by a status word, leading the message, or parenthesized.
127+
// A bare scan would turn "max_tokens 512" into a 5xx and recommend a retry
128+
// that can only fail the same way.
129+
const PLANNER_FAILURE_STATUS_PATTERNS = [
130+
/\b(?:error|http|status|code|returned|response)\s*[:#-]?\s*(\d{3})\b/i,
131+
/^\s*\(?(\d{3})\)?\b/,
132+
/\((\d{3})\)/,
133+
];
134+
const PLANNER_AUTH_FAILURE_RE = /\b(?:unauthori[sz]ed|unauthenticated|forbidden|permission denied|invalid credentials|(?:invalid|incorrect|missing|expired)[\s_-]*api[\s_-]*key|api[\s_-]*key[\s_-]*(?:is\s*)?(?:missing|invalid|incorrect|expired|not\s*set|required)|authentication|(?:expired|invalid)[\s_-]*token|token[\s_-]*expired)\b/i;
135+
// "Failed to fetch" (Chrome) and "NetworkError…" (Firefox) are the shapes a
136+
// dead local provider or an offline machine actually produces, so they have
137+
// to land in the bucket that offers Retry first.
138+
const PLANNER_TRANSIENT_FAILURE_RE = /\b(?:timed?\s*out|timeout|network\w*|failed to fetch|fetch failed|load failed|connection (?:closed|reset|refused|error)|econn\w+|socket hang up|temporar(?:y|ily)|unavailable|overloaded|rate[\s_-]?limit\w*|too many requests|bad gateway|gateway timeout)\b/i;
139+
125140
function plannerRequestFailureKind(detail) {
126141
const text = String(detail || '');
127-
const statusMatch = text.match(/\b(?:error|http|status)\s*[:#-]?\s*(\d{3})\b/i)
128-
|| text.match(/\b(401|403|408|425|429|5\d\d)\b/);
129-
const status = Number(statusMatch?.[1] || 0);
130-
if (status === 401 || status === 403) return 'auth';
131-
if ([408, 425, 429].includes(status) || status >= 500) return 'transient';
132-
if (/\b(?:timed?\s*out|timeout|network|fetch failed|connection (?:closed|reset|refused)|econn\w+|temporar(?:y|ily)|unavailable)\b/i.test(text)) {
133-
return 'transient';
142+
let status = 0;
143+
for (const pattern of PLANNER_FAILURE_STATUS_PATTERNS) {
144+
const match = text.match(pattern);
145+
if (match) {
146+
status = Number(match[1]);
147+
break;
148+
}
134149
}
150+
if (status === 401 || status === 403) return 'auth';
151+
if ([408, 425, 429].includes(status) || (status >= 500 && status <= 599)) return 'transient';
152+
if (PLANNER_AUTH_FAILURE_RE.test(text)) return 'auth';
153+
// A remaining 4xx is a request the provider rejected on its merits; a retry
154+
// of the same request is not the fix.
155+
if (status >= 400 && status <= 499) return 'provider';
156+
if (PLANNER_TRANSIENT_FAILURE_RE.test(text)) return 'transient';
135157
return 'provider';
136158
}
137159

@@ -6172,6 +6194,9 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
61726194
reason: gate.reason,
61736195
requestKind: gate.requestKind,
61746196
requiresStateChange: gate.requiresStateChange,
6197+
// Carried so a caller can tell an auth failure from a transient one
6198+
// without re-parsing the message text.
6199+
...(gate.failureKind ? { failureKind: gate.failureKind } : {}),
61756200
};
61766201
}
61776202

@@ -6350,7 +6375,8 @@ Rules: no prose intro, no conclusion, no "this screenshot shows...", no layout d
63506375
500,
63516376
{ collapseWhitespace: true },
63526377
);
6353-
const message = `Planner request failed before a valid response was available: ${detail} No tools ran.`;
6378+
const detailSentence = /[.!?]$/.test(detail) ? detail : `${detail}.`;
6379+
const message = `Planner request failed before a valid response was available: ${detailSentence} No tools ran.`;
63546380
const failureKind = plannerRequestFailureKind(detail);
63556381
onUpdate('warning', {
63566382
code: 'planner_request_failed',

src/firefox/src/ui/sidepanel.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5092,6 +5092,15 @@ function renderPlannerRequestFailure(assistantEl, data, retryPayload = null) {
50925092
const message = document.createElement('div');
50935093
message.className = 'planner-request-failure-message';
50945094
message.textContent = data.message || 'Planner request failed before a valid response was available.';
5095+
// Name the provider that failed — with several configured, "open Providers"
5096+
// is only actionable if the user knows which one to look at. The label is a
5097+
// proper noun, so it needs no translation.
5098+
if (data.provider) {
5099+
const providerName = document.createElement('div');
5100+
providerName.className = 'planner-request-failure-provider';
5101+
providerName.textContent = data.provider;
5102+
message.appendChild(providerName);
5103+
}
50955104

50965105
const actions = document.createElement('div');
50975106
actions.className = 'planner-request-failure-actions';

src/firefox/styles/sidepanel.css

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,6 +2761,13 @@ body {
27612761
color: var(--error-soft);
27622762
}
27632763

2764+
.planner-request-failure-provider {
2765+
margin-top: 4px;
2766+
font-size: 11px;
2767+
font-weight: 600;
2768+
opacity: 0.85;
2769+
}
2770+
27642771
.planner-request-failure-actions {
27652772
display: flex;
27662773
flex-wrap: wrap;

test/run.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49590,6 +49590,80 @@ test('planner request errors stop accurately instead of masquerading as JSON rep
4959049590
);
4959149591
assert.equal(transient.failureKind, 'transient', `${label}: 503 planner failure was not classified as transient`);
4959249592
assert.equal(warning?.failureKind, 'transient', `${label}: transient category was not exposed to the sidepanel`);
49593+
49594+
// The category only reorders two always-present buttons, but it is the
49595+
// whole point of the card: an auth failure must lead with Providers and
49596+
// a dropped connection must lead with Retry. Classification runs on raw
49597+
// provider prose, so pin the shapes real providers actually emit —
49598+
// including the browser's own fetch failures, which is what a stopped
49599+
// local server looks like from inside the extension.
49600+
const classifications = [
49601+
['Incorrect API key provided: sk-***. (status 401)', 'auth', 'parenthesized 401'],
49602+
['Unauthorized', 'auth', 'status-free auth rejection'],
49603+
['invalid_api_key: authentication failed', 'auth', 'textual auth rejection'],
49604+
['TypeError: Failed to fetch', 'transient', 'Chrome transport failure'],
49605+
['NetworkError when attempting to fetch resource.', 'transient', 'Firefox transport failure'],
49606+
['The request timed out after 60000 ms', 'transient', 'timeout'],
49607+
['ECONNREFUSED 127.0.0.1:11434', 'transient', 'unreachable local provider'],
49608+
['Rate limit reached. Please try again (429)', 'transient', 'rate limit'],
49609+
['Provider returned 400: max_tokens 512 is invalid', 'provider', 'rejected request carrying a 5xx-shaped number'],
49610+
['This model requires 8192 context, got 500 tokens', 'provider', 'prose carrying a 5xx-shaped number'],
49611+
['insufficient_quota: You exceeded your current quota', 'provider', 'billing failure'],
49612+
];
49613+
for (const [detail, expected, why] of classifications) {
49614+
agent._chatWithCostAllowance = async () => { throw new Error(detail); };
49615+
warning = null;
49616+
const classified = await agent._runPlannerGate(
49617+
tabId,
49618+
{ role: 'user', content: 'Classify this failure.' },
49619+
onUpdate,
49620+
null,
49621+
);
49622+
assert.equal(
49623+
classified.failureKind,
49624+
expected,
49625+
`${label}: ${why} ("${detail}") should recover as ${expected}`,
49626+
);
49627+
assert.equal(
49628+
warning?.failureKind,
49629+
expected,
49630+
`${label}: ${why} lost its category on the way to the sidepanel`,
49631+
);
49632+
assert.match(
49633+
classified.message || '',
49634+
/[.!?] No tools ran\.$/,
49635+
`${label}: ${why} produced a run-on failure message`,
49636+
);
49637+
}
49638+
}
49639+
});
49640+
});
49641+
49642+
test('planner failure category survives the planner gate wrapper', async () => {
49643+
await withPlannerBrowserGlobals(async () => {
49644+
for (const [label, AgentClass] of [['chrome', AgentCh], ['firefox', AgentFx]]) {
49645+
const tabId = label === 'chrome' ? 9157 : 9158;
49646+
const provider = { name: 'broken-provider', model: 'broken-model', config: {} };
49647+
const agent = new AgentClass({ getActive: () => provider });
49648+
agent._chatWithCostAllowance = async () => { throw new Error('401 invalid API key'); };
49649+
49650+
const messages = [];
49651+
const outcome = await agent._maybeRunPlannerGate(
49652+
tabId,
49653+
messages,
49654+
{ role: 'user', content: 'Perform this task.' },
49655+
() => {},
49656+
'act',
49657+
null,
49658+
null,
49659+
);
49660+
assert.equal(outcome.proceed, false, `${label}: planner failure unexpectedly continued into execution`);
49661+
assert.equal(outcome.reason, 'planner_error', `${label}: planner failure lost its planner status`);
49662+
assert.equal(
49663+
outcome.failureKind,
49664+
'auth',
49665+
`${label}: the gate wrapper dropped the failure category before any caller could read it`,
49666+
);
4959349667
}
4959449668
});
4959549669
});
@@ -49658,8 +49732,14 @@ test('planner request failures expose provider settings and retry actions in bot
4965849732
/function rebindPlannerRequestFailureControls\(\) \{[\s\S]*?planner-request-failure-provider-btn[\s\S]*?bindPlannerProviderSettingsButton/,
4965949733
`${label}: restored Providers buttons are not rebound`,
4966049734
);
49735+
assert.match(
49736+
panel,
49737+
/if \(data\.provider\) \{[\s\S]*?providerName\.className = 'planner-request-failure-provider';[\s\S]*?providerName\.textContent = data\.provider;/,
49738+
`${label}: the failing provider is never named, so "open Providers" does not say which one to fix`,
49739+
);
4966149740
assert.match(css, /\.planner-request-failure-actions \{/, `${label}: planner failure action row is not styled`);
4966249741
assert.match(css, /\.planner-request-failure-action\.primary \{/, `${label}: planner failure primary action is not styled`);
49742+
assert.match(css, /\.planner-request-failure-provider \{/, `${label}: planner failure provider label is not styled`);
4966349743
}
4966449744
});
4966549745

0 commit comments

Comments
 (0)