diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index b68ed08c6b..4270d8a869 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -1190,12 +1190,16 @@ export class CodexExecutor extends BaseExecutor { return null; } const result = await getAccessToken("codex", credentials, log); - if (!result || result.error) { + if (!result) { + log?.warn?.("TOKEN_REFRESH", "Codex: token refresh failed — re-authentication required"); + return null; + } + if (result.error) { log?.warn?.( "TOKEN_REFRESH", - `Codex: token refresh failed${result?.error ? ` (${result.error})` : ""} — re-authentication required` + `Codex: token refresh failed (${result.error}) — re-authentication required` ); - return null; + return result; } return result; } diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 7146188bc2..249463352d 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -13,7 +13,7 @@ import { ensureStreamReadiness } from "../utils/streamReadiness.ts"; import { createStreamController, pipeWithDisconnect } from "../utils/streamHandler.ts"; import { createSseHeartbeatTransform, shapeForClientFormat } from "../utils/sseHeartbeat.ts"; import { addBufferToUsage, filterUsageForFormat, estimateUsage } from "../utils/usageTracking.ts"; -import { refreshWithRetry } from "../services/tokenRefresh.ts"; +import { refreshWithRetry, isUnrecoverableRefreshError } from "../services/tokenRefresh.ts"; import { createRequestLogger } from "../utils/requestLogger.ts"; import { getModelTargetFormat, PROVIDER_ID_TO_ALIAS } from "../config/providerModels.ts"; import { @@ -3471,6 +3471,9 @@ export async function handleChatCore({ } } else { log?.warn?.("TOKEN", `${provider.toUpperCase()} | refresh failed`); + if (isUnrecoverableRefreshError(newCredentials) && onCredentialsRefreshed) { + await onCredentialsRefreshed({ testStatus: "expired", isActive: false }); + } } } @@ -4134,7 +4137,10 @@ export async function handleChatCore({ try { const firstChoice = translatedResponse?.choices?.[0]; const msg = firstChoice?.message; - cacheReasoningFromAssistantMessage(msg, provider, model, { requestId: skillRequestId, messageIndex: 0 }); + cacheReasoningFromAssistantMessage(msg, provider, model, { + requestId: skillRequestId, + messageIndex: 0, + }); } catch { // Cache capture is non-critical — never block the response } @@ -4421,7 +4427,10 @@ export async function handleChatCore({ const body = streamResponseBody as Record; const choices = body.choices as { message?: Record }[] | undefined; const msg = choices?.[0]?.message; - cacheReasoningFromAssistantMessage(msg, provider, model, { requestId: skillRequestId, messageIndex: 0 }); + cacheReasoningFromAssistantMessage(msg, provider, model, { + requestId: skillRequestId, + messageIndex: 0, + }); } catch { // Cache capture is non-critical — never block the stream } diff --git a/open-sse/services/tokenRefresh.ts b/open-sse/services/tokenRefresh.ts index 33f0848756..7f5964fece 100755 --- a/open-sse/services/tokenRefresh.ts +++ b/open-sse/services/tokenRefresh.ts @@ -346,11 +346,20 @@ export async function refreshClaudeOAuthToken(refreshToken, log, proxyConfig: un ); if (!response.ok) { - const errorText = await response.text(); + let errorBody: { error?: string; error_description?: string } = {}; + try { + errorBody = await response.json(); + } catch { + const text = await response.text().catch(() => "unknown"); + errorBody = { error: text }; + } log?.error?.("TOKEN_REFRESH", "Failed to refresh Claude OAuth token", { status: response.status, - error: errorText, + error: errorBody, }); + if (errorBody.error === "invalid_grant" || errorBody.error === "invalid_request") { + return { error: errorBody.error, code: `http_${response.status}` }; + } return null; } @@ -1277,6 +1286,13 @@ export async function refreshWithRetry( try { const result = await withTimeout(refreshFn, REFRESH_TIMEOUT_MS); + if (isUnrecoverableRefreshError(result)) { + log?.warn?.( + "TOKEN_REFRESH", + `Unrecoverable refresh error for ${provider}: ${result.error} — skipping retries` + ); + return result; + } if (result) { recordSuccess(provider); return result; diff --git a/src/sse/handlers/chatHelpers.ts b/src/sse/handlers/chatHelpers.ts index ec4dc2774a..b9d34d1b8e 100644 --- a/src/sse/handlers/chatHelpers.ts +++ b/src/sse/handlers/chatHelpers.ts @@ -334,7 +334,8 @@ export async function executeChatWithBreaker({ // apiKey blob mid-request — forward it so the DB credential // doesn't go stale after Set-Cookie rotation. apiKey: newCreds.apiKey, - testStatus: "active", + testStatus: newCreds.testStatus ?? "active", + isActive: newCreds.isActive, }); }, onRequestSuccess: async () => { diff --git a/src/sse/services/tokenRefresh.ts b/src/sse/services/tokenRefresh.ts index 244eb70558..ad9b77e484 100755 --- a/src/sse/services/tokenRefresh.ts +++ b/src/sse/services/tokenRefresh.ts @@ -141,6 +141,9 @@ export async function updateProviderCredentials(connectionId: string, newCredent if (newCredentials.testStatus) { updates.testStatus = newCredentials.testStatus; } + if (newCredentials.isActive !== undefined) { + updates.isActive = newCredentials.isActive; + } const result = await updateProviderConnection(connectionId, updates); log.info("TOKEN_REFRESH", "Credentials updated in localDb", { diff --git a/tests/unit/executor-codex.test.ts b/tests/unit/executor-codex.test.ts index 092320f075..41e093c8a7 100644 --- a/tests/unit/executor-codex.test.ts +++ b/tests/unit/executor-codex.test.ts @@ -1219,6 +1219,24 @@ test("CodexExecutor.refreshCredentials refreshes OAuth tokens and returns null w } }); +test("CodexExecutor.refreshCredentials propagates unrecoverable error object instead of returning null", async () => { + const executor = new CodexExecutor(); + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => + new Response( + JSON.stringify({ error: "invalid_grant", error_description: "Refresh token expired" }), + { status: 400, headers: { "Content-Type": "application/json" } } + ); + + try { + const result = await executor.refreshCredentials({ refreshToken: "dead-token" }, null); + assert.ok(result !== null, "should return error object, not null"); + assert.equal((result as any).error, "unrecoverable_refresh_error"); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("CodexExecutor maps usage_limit_reached websocket failures without explicit status to 429", () => { const raw = JSON.stringify({ type: "response.failed", diff --git a/tests/unit/token-refresh-service.test.ts b/tests/unit/token-refresh-service.test.ts index 8dfb5e93fe..339e912056 100644 --- a/tests/unit/token-refresh-service.test.ts +++ b/tests/unit/token-refresh-service.test.ts @@ -1121,3 +1121,82 @@ test("getAccessToken per-connection mutex: mutex cleared after success, next cal } ); }); + +// ─── Unrecoverable error bail-out tests ────────────────────────────────────── + +test("refreshWithRetry bails immediately on unrecoverable error without retrying", async () => { + const provider = `bail-unrecoverable-${Date.now()}`; + const log = createLog(); + let callCount = 0; + + const result = await refreshWithRetry( + async () => { + callCount++; + return { error: "unrecoverable_refresh_error", code: "http_400" }; + }, + 3, + log, + provider + ); + + assert.equal(callCount, 1, "should only call refreshFn once (no retries)"); + assert.deepEqual(result, { error: "unrecoverable_refresh_error", code: "http_400" }); + const warnMessages = log.entries.filter((e) => e.level === "warn").map((e) => e.message); + assert.ok( + warnMessages.some((m) => String(m).includes("Unrecoverable")), + "should log an unrecoverable warning" + ); +}); + +test("refreshWithRetry bails immediately on invalid_grant error without retrying", async () => { + const provider = `bail-invalid-grant-${Date.now()}`; + const log = createLog(); + let callCount = 0; + + const result = await refreshWithRetry( + async () => { + callCount++; + return { error: "invalid_grant", code: "http_400" }; + }, + 3, + log, + provider + ); + + assert.equal(callCount, 1, "should only call refreshFn once (no retries)"); + assert.deepEqual(result, { error: "invalid_grant", code: "http_400" }); +}); + +test("refreshClaudeOAuthToken returns error object for invalid_grant (expired refresh token)", async () => { + const log = createLog(); + + await withMockedFetch( + async () => + new Response(JSON.stringify({ error: "invalid_grant", error_description: "Token expired" }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }), + async () => { + const result = await refreshClaudeOAuthToken("expired-token", log); + assert.ok(result && typeof result === "object", "should return error object, not null"); + assert.equal((result as any).error, "invalid_grant"); + assert.ok(isUnrecoverableRefreshError(result), "should be detected as unrecoverable"); + } + ); +}); + +test("refreshClaudeOAuthToken returns null for transient server errors (not unrecoverable)", async () => { + const log = createLog(); + + await withMockedFetch( + async () => + new Response(JSON.stringify({ error: "server_error" }), { + status: 503, + headers: { "Content-Type": "application/json" }, + }), + async () => { + const result = await refreshClaudeOAuthToken("some-token", log); + assert.equal(result, null, "transient server errors should return null (retryable)"); + } + ); +});