diff --git a/open-sse/executors/chatgpt-session.ts b/open-sse/executors/chatgpt-session.ts index d580bf3ce4..09a140bce1 100644 --- a/open-sse/executors/chatgpt-session.ts +++ b/open-sse/executors/chatgpt-session.ts @@ -313,13 +313,13 @@ export class ChatGptSessionExecutor extends BaseExecutor { void run(); const opened = await openChatGptSessionStream(events, meta); if (opened.kind === "error") { - // `opened.status`/`opened.code` are authoritative: the bridge classified the real - // adapter event, which carries its own `status`/`code` and its own `name` (a Playwright - // TimeoutError is classified by name, and its message alone would fall through to a - // breaker-tripping 502). `ChatGptSessionStreamOpen` has no `fallbackHint` field, so the - // message is re-examined for that one value and nothing else. - const hint = classifyChatGptSessionError(new Error(opened.message)).fallbackHint; - return wrapped(errorResponse(opened.status, opened.message, opened.code, hint), input.body); + // Every field of the verdict comes from the bridge's classification of the real adapter + // event — status, code and fallbackHint alike. Re-classifying the sanitized message here + // would discard the event's own `status`/`code`/`name`. + return wrapped( + errorResponse(opened.status, opened.message, opened.code, opened.fallbackHint), + input.body + ); } return wrapped( new Response(opened.stream, { status: 200, headers: SSE_HEADERS }), diff --git a/open-sse/executors/chatgpt-session/errors.ts b/open-sse/executors/chatgpt-session/errors.ts index 03a392a45a..5ecb9a5e4a 100644 --- a/open-sse/executors/chatgpt-session/errors.ts +++ b/open-sse/executors/chatgpt-session/errors.ts @@ -68,6 +68,22 @@ export function classifyChatGptSessionError(error: unknown): ChatGptSessionError return { status: 400, code: "browser_ui_timeout" }; } + // An explicit numeric status is the adapter's own authoritative verdict. The vendor documents + // it that way on `AdapterEvent.error.status` ("Authoritative upstream/proxy status when known; + // avoids message-based classification"), and trusting it first is what stops a 503 whose prose + // happens to mention signing in from being reported as an expired session — which would mark a + // healthy account's credentials dead and pull it out of rotation. Message matching still runs + // below, because failures thrown by this executor itself carry no status at all. + if (typeof like.status === "number" && like.status >= 400) { + const status = like.status; + return { + status, + code: like.code ?? "turn_failed", + // 503/429 must cool this one connection down rather than trip the whole-provider breaker. + ...(status === 503 || status === 429 ? { fallbackHint: "connection_cooldown" as const } : {}), + }; + } + if (BROWSER_UNAVAILABLE.test(like.message)) { return { status: 503, code: "browser_unavailable", fallbackHint: "connection_cooldown" }; } @@ -86,8 +102,5 @@ export function classifyChatGptSessionError(error: unknown): ChatGptSessionError if (UI_TIMEOUT.test(like.message)) { return { status: 400, code: "browser_ui_timeout" }; } - if (typeof like.status === "number" && like.status >= 400) { - return { status: like.status, code: like.code ?? "turn_failed" }; - } return { status: 502, code: "turn_failed" }; } diff --git a/tests/unit/chatgpt-session-errors.test.ts b/tests/unit/chatgpt-session-errors.test.ts index dc41269352..ac1b80ac8d 100644 --- a/tests/unit/chatgpt-session-errors.test.ts +++ b/tests/unit/chatgpt-session-errors.test.ts @@ -80,13 +80,72 @@ test("an explicit upstream status is used when no message pattern matches", () = assert.equal(result.status, 502); }); -// Message-pattern matching intentionally runs before the explicit-status branch, so a -// recognised message wins even when the error also carries an explicit upstream status. -test("a matching message wins over an explicit upstream status", () => { +// CONTRACT CHANGE (deliberate inversion of the previous expectation). This test used to assert +// that a recognised message beat an explicit upstream status; the precedence is now the other way +// round. The vendor documents `AdapterEvent.error.status` as "Authoritative upstream/proxy status +// when known; avoids message-based classification", and honouring that is what stops a 503 whose +// prose happens to mention signing in from being reported as an expired session — which would +// mark a HEALTHY account's credentials dead and pull it out of rotation. Message matching still +// runs, just as the fallback for failures that carry no status at all (everything this executor +// throws itself). +test("an explicit upstream status wins over a matching message", () => { const result = classifyChatGptSessionError({ message: "ChatGPT reported a usage limit", status: 500, }); + assert.equal(result.status, 500); + assert.equal(result.code, "turn_failed"); +}); + +test("a session-expired message cannot downgrade an explicit 503 to a 401", () => { + const result = classifyChatGptSessionError({ + message: "Please sign in to continue", + status: 503, + }); + assert.equal(result.status, 503); + assert.notEqual(result.code, "session_expired"); +}); + +test("an explicit 503 carries the connection cooldown hint", () => { + const result = classifyChatGptSessionError({ message: "upstream unavailable", status: 503 }); + assert.equal(result.status, 503); + assert.equal(result.fallbackHint, "connection_cooldown"); +}); + +test("an explicit 429 carries the connection cooldown hint", () => { + const result = classifyChatGptSessionError({ message: "slow down", status: 429 }); + assert.equal(result.status, 429); + assert.equal(result.fallbackHint, "connection_cooldown"); +}); + +test("an explicit 400 carries no cooldown hint", () => { + const result = classifyChatGptSessionError({ message: "bad request", status: 400 }); + assert.equal(result.status, 400); + assert.equal(result.fallbackHint, undefined); +}); + +test("an explicit status keeps the event's own code when it carries one", () => { + const result = classifyChatGptSessionError({ + message: "anything", + status: 503, + code: "upstream_unavailable", + }); + assert.equal(result.code, "upstream_unavailable"); +}); + +test("a TimeoutError still outranks an explicit upstream status", () => { + const timeout = new Error("locator.waitForSelector: Timeout 30000ms exceeded") as Error & { + status?: number; + }; + timeout.name = "TimeoutError"; + timeout.status = 503; + const result = classifyChatGptSessionError(timeout); + assert.equal(result.status, 400); + assert.equal(result.code, "browser_ui_timeout"); +}); + +test("message patterns still classify failures that carry no status", () => { + const result = classifyChatGptSessionError(new Error("ChatGPT reported a usage limit")); assert.equal(result.status, 429); assert.equal(result.code, "rate_limited"); });