diff --git a/changelog.d/fixes/cline-refresh-token-classification.md b/changelog.d/fixes/cline-refresh-token-classification.md new file mode 100644 index 0000000000..5641775afd --- /dev/null +++ b/changelog.d/fixes/cline-refresh-token-classification.md @@ -0,0 +1 @@ +- **fix(oauth):** classify embedded `invalid_grant` in Cline token refresh error bodies so permanently consumed refresh tokens trigger re-authentication instead of indefinite transient retry loops, and add `cline` to `ROTATION_LOCK_GROUP` to serialize concurrent sibling refreshes ([#13466](https://github.com/diegosouzapw/OmniRoute/pull/13466)) diff --git a/open-sse/services/refreshSerializer.ts b/open-sse/services/refreshSerializer.ts index 55f37c62ce..4375dd0d03 100644 --- a/open-sse/services/refreshSerializer.ts +++ b/open-sse/services/refreshSerializer.ts @@ -27,6 +27,12 @@ const ROTATION_LOCK_GROUP: Record = { "gitlab-duo": "gitlab-duo", kiro: "kiro", "kimi-coding": "kimi-coding", + // Cline rotates on every refresh — `refreshClineToken` reads a new + // `refreshToken` out of the response body, and a measured refresh moved a + // connection's stored token to a different value. It was missing here while + // already listed in tokenHealthCheck's ROTATING_REFRESH_PROVIDERS, so sibling + // connections could refresh concurrently and present superseded tokens. + cline: "cline", }; // Protective settle gap (ms) between two consecutive sibling refreshes when the diff --git a/open-sse/services/tokenRefresh/shared.ts b/open-sse/services/tokenRefresh/shared.ts index 808a349050..2ab0003231 100644 --- a/open-sse/services/tokenRefresh/shared.ts +++ b/open-sse/services/tokenRefresh/shared.ts @@ -41,6 +41,17 @@ const UNRECOVERABLE_OAUTH_ERROR_CODES = new Set([ "access_denied", ]); +/** + * Matches a known unrecoverable code EMBEDDED in a human-readable message, on + * word boundaries (`_` counts as a word char, so `xinvalid_grant` never hits). + * Alternation is safe against overlap because every candidate is delimited on + * both sides. Built once — the code set is a module constant. + */ +const EMBEDDED_OAUTH_ERROR_CODE_RE = new RegExp( + `(?"` field pair, so it used to return `null` + * and a permanently dead token was classified as a TRANSIENT failure — the + * unrecoverable branch in `tokenHealthCheck` (its `credentialsChangedSinceSweep` + * race guard, the "please re-authenticate" message, and the dead-token clear for + * rotating providers) never ran, so the token was retried forever and every + * request routed to that connection 401'd with no actionable signal. + * * Returns the matched code (only if it is in UNRECOVERABLE_OAUTH_ERROR_CODES) - * or null. Never matches loosely — a known code is accepted only when it is a - * bare code string or the value of an `"error"`/`"error_code"` field, so a 502 - * HTML page or a `server_error` body never becomes a false positive. + * or null. Matching stays conservative: a known code is accepted only as a bare + * code string, as the value of an `"error"`/`"error_code"` field, or as a + * word-delimited token inside such a value — so a `server_error` body or a 502 + * HTML page still classifies as null. The deliberate trade-off is that a + * message which merely MENTIONS a dead-token code is treated as unrecoverable; + * that fails safe, because the alternative is an unbounded retry loop that + * burns a rotating provider's refresh tokens. */ export function extractOAuthErrorCode(raw: unknown, depth = 0): string | null { if (raw == null || depth > 6) return null; @@ -76,6 +101,10 @@ export function extractOAuthErrorCode(raw: unknown, depth = 0): string | null { // field inside otherwise-unparsed text. Scoped to avoid false positives. const m = s.match(/"error(?:_code)?"\s*:\s*"([a-z_]+)"/i); if (m && UNRECOVERABLE_OAUTH_ERROR_CODES.has(m[1])) return m[1]; + // Last resort: the code carried inside a message rather than returned bare + // (Cline: "failed to refresh token: invalid_grant"). + const embedded = s.match(EMBEDDED_OAUTH_ERROR_CODE_RE); + if (embedded) return embedded[1].toLowerCase(); return null; } diff --git a/tests/unit/oauth-refresh-error-resilience.test.ts b/tests/unit/oauth-refresh-error-resilience.test.ts index aa3125ab79..155fa01139 100644 --- a/tests/unit/oauth-refresh-error-resilience.test.ts +++ b/tests/unit/oauth-refresh-error-resilience.test.ts @@ -62,7 +62,7 @@ test("extractOAuthErrorCode: bare string code 'invalid_grant'", () => { assert.equal(extractOAuthErrorCode("invalid_grant"), "invalid_grant"); }); -test("extractOAuthErrorCode: JSON string body '{\"error\":\"invalid_grant\"}'", () => { +test('extractOAuthErrorCode: JSON string body \'{"error":"invalid_grant"}\'', () => { assert.equal(extractOAuthErrorCode('{"error": "invalid_grant"}'), "invalid_grant"); }); @@ -92,11 +92,49 @@ test("extractOAuthErrorCode: transient errors are NOT misclassified (no false po assert.equal(extractOAuthErrorCode(undefined), null); }); +// ── code EMBEDDED in a message, not returned bare ─────────────────────────── +// Cline's refresh endpoint answers a dead refresh_token with +// 400 {"data":"","error":"failed to refresh token: invalid_grant","success":false} +// The code is the tail of a sentence, so the exact-match Set lookup and the +// `"error":""` field scan both miss it. It classified as null → TRANSIENT +// → the connection was retried forever instead of prompting a re-auth. + +test("extractOAuthErrorCode: code embedded in a message (the Cline production body)", () => { + assert.equal( + extractOAuthErrorCode({ error: "failed to refresh token: invalid_grant" }), + "invalid_grant" + ); + // Same body as the raw text the catch branch forwards. + assert.equal( + extractOAuthErrorCode( + '{"data":"","error":"failed to refresh token: invalid_grant","success":false}' + ), + "invalid_grant" + ); +}); + +test("extractOAuthErrorCode: embedded match is word-delimited (no substring false positives)", () => { + // `_` is a word character, so a code glued to other identifier chars must NOT match. + assert.equal(extractOAuthErrorCode({ error: "xinvalid_grant" }), null); + assert.equal(extractOAuthErrorCode({ error: "invalid_grantx" }), null); + assert.equal(extractOAuthErrorCode({ error: "my_invalid_grant_flag" }), null); + // A transient message that happens to contain no known code stays transient. + assert.equal(extractOAuthErrorCode({ error: "upstream timed out after 30s" }), null); + // Punctuation and whitespace ARE valid delimiters. + assert.equal(extractOAuthErrorCode({ error: "token rejected (invalid_grant)" }), "invalid_grant"); +}); + // ── refreshClaudeOAuthToken: every shape → unrecoverable sentinel ──────────── const SENTINEL_SHAPES: Array<{ name: string; body: string; ct?: string }> = [ - { name: "canonical object", body: '{"error": "invalid_grant", "error_description": "Refresh token not found or invalid"}' }, - { name: "double-encoded JSON string", body: JSON.stringify('{"error": "invalid_grant", "error_description": "x"}') }, + { + name: "canonical object", + body: '{"error": "invalid_grant", "error_description": "Refresh token not found or invalid"}', + }, + { + name: "double-encoded JSON string", + body: JSON.stringify('{"error": "invalid_grant", "error_description": "x"}'), + }, { name: "bare string code", body: '"invalid_grant"' }, { name: "nested error.code", body: '{"error": {"code": "invalid_grant", "message": "x"}}' }, { name: "json served as text/plain", body: '{"error": "invalid_grant"}', ct: "text/plain" }, @@ -105,7 +143,8 @@ const SENTINEL_SHAPES: Array<{ name: string; body: string; ct?: string }> = [ for (const shape of SENTINEL_SHAPES) { test(`refreshClaudeOAuthToken → unrecoverable sentinel for shape: ${shape.name}`, async () => { await withMockedFetch( - (async () => rawResponse(shape.body, 400, shape.ct ?? "application/json")) as unknown as typeof fetch, + (async () => + rawResponse(shape.body, 400, shape.ct ?? "application/json")) as unknown as typeof fetch, async () => { const result = await refreshClaudeOAuthToken("dead-refresh-token"); assert.ok( @@ -123,14 +162,19 @@ test("refreshClaudeOAuthToken: transient 500 server_error stays null (NOT unreco (async () => rawResponse('{"error": "server_error"}', 500)) as unknown as typeof fetch, async () => { const result = await refreshClaudeOAuthToken("token"); - assert.equal(result, null, "transient errors must remain recoverable (null), not deactivate the account"); + assert.equal( + result, + null, + "transient errors must remain recoverable (null), not deactivate the account" + ); } ); }); test("refreshClaudeOAuthToken: 502 HTML gateway error stays null", async () => { await withMockedFetch( - (async () => rawResponse("502 Bad Gateway", 502, "text/html")) as unknown as typeof fetch, + (async () => + rawResponse("502 Bad Gateway", 502, "text/html")) as unknown as typeof fetch, async () => { const result = await refreshClaudeOAuthToken("token"); assert.equal(result, null); @@ -178,3 +222,23 @@ for (const r of FRAGILE_REFRESHERS) { ); }); } + +// Cline's REAL production 400 body — the code arrives inside a sentence, so +// before the embedded-code scan this returned null and the HealthCheck treated a +// permanently consumed refresh_token as a retryable blip. +test("refreshClineToken: the verbatim production body yields an unrecoverable sentinel", async () => { + await withMockedFetch( + (async () => + rawResponse( + '{"data":"","error":"failed to refresh token: invalid_grant","success":false}\n', + 400 + )) as unknown as typeof fetch, + async () => { + const result = await refreshClineToken("consumed-token"); + assert.ok( + isUnrecoverableRefreshError(result), + `expected unrecoverable sentinel, got ${JSON.stringify(result)}` + ); + } + ); +});