diff --git a/changelog.d/features/8980-deprecate-gemini-cli-provider.md b/changelog.d/features/8980-deprecate-gemini-cli-provider.md new file mode 100644 index 0000000000..38a826a2e7 --- /dev/null +++ b/changelog.d/features/8980-deprecate-gemini-cli-provider.md @@ -0,0 +1 @@ +- **feat(sse):** deprecated the legacy `gemini-cli` **upstream provider**. It was not routable (no registry entry, no executor), yet the scheduler kept refreshing its token against Google — maintaining a credential that could never serve a request. A stored connection now becomes terminal with a legible reason and a working migration path: re-add the account under `gemini`, which uses the same Google OAuth client. The `gemini-cli` **client identity** (requests arriving *from* the Gemini CLI, issue #7034) is untouched ([#8980](https://github.com/diegosouzapw/OmniRoute/pull/8980)) diff --git a/open-sse/services/tokenRefresh.ts b/open-sse/services/tokenRefresh.ts index 3bb584dbd7..43dcba1a6d 100755 --- a/open-sse/services/tokenRefresh.ts +++ b/open-sse/services/tokenRefresh.ts @@ -113,9 +113,46 @@ export const REFRESH_LEAD_MS: Record = { // is safe and reduces unnecessary upstream chatter. antigravity: 15 * 60 * 1000, agy: 15 * 60 * 1000, // same Google backend as antigravity (non-rotating refresh tokens) - "gemini-cli": 15 * 60 * 1000, // legacy stored connections; provider is no longer public }; +/** + * Upstream providers that stored connections may still name, but that this build no + * longer serves. They are NOT routable — absent from PROVIDERS, from the chat REGISTRY, + * and without an executor — so keeping their token fresh maintains a credential that can + * never answer a request. + * + * Deprecation, not deletion: a connection here becomes terminal with a reason that names + * where to go instead, rather than silently sitting at `active` doing nothing. The + * migration target must be routable — `tests/unit/gemini-cli-deprecation.test.ts` asserts + * that, so the notice can never point somewhere useless. + */ +export const DEPRECATED_PROVIDERS: Readonly< + Record +> = { + "gemini-cli": { + migrateTo: "gemini", + // The legacy path redeemed the token with PROVIDERS.gemini's client — the very same + // public Gemini CLI / Code Assist OAuth client — which is why re-adding the account + // under `gemini` is a real migration and not a suggestion to start over. + reason: + "The gemini-cli provider was discontinued and is not routable. Re-add this account " + + "under the `gemini` provider — it uses the same Google OAuth client, so the same " + + "login works and the account becomes usable again.", + }, +}; + +/** Whether `provider` is a deprecated upstream that must not be refreshed. */ +export function isDeprecatedProvider(provider: string): boolean { + return Boolean(provider) && Object.prototype.hasOwnProperty.call(DEPRECATED_PROVIDERS, provider); +} + +/** The migration notice for a deprecated provider, or null when it is not deprecated. */ +export function getDeprecationNotice( + provider: string +): { migrateTo: string; reason: string } | null { + return isDeprecatedProvider(provider) ? DEPRECATED_PROVIDERS[provider] : null; +} + /** * Get the proactive refresh lead time (ms) for a given provider. * @@ -261,17 +298,27 @@ export async function refreshAccessToken( */ async function _getAccessTokenInternal(provider, credentials, log, proxyConfig: unknown = null) { switch (provider) { - case "gemini-cli": - // Legacy DB rows can retain this discontinued provider id. Refresh them - // with the same public OAuth client used by Gemini CLI without restoring - // gemini-cli to the routable provider or OAuth UI registries. - return await refreshGoogleToken( - credentials.refreshToken, - PROVIDERS.gemini.clientId, - PROVIDERS.gemini.clientSecret, - log, - proxyConfig + case "gemini-cli": { + // Deprecated (see DEPRECATED_PROVIDERS). This used to refresh successfully against + // PROVIDERS.gemini's client, but the provider is not routable, so the fresh token + // had nowhere to go — periodic upstream calls maintaining an unusable credential. + // + // Return the ESTABLISHED unrecoverable contract, so every existing caller + // (isUnrecoverableRefreshError, the manual-refresh route) already stops retrying — + // but with a code that says WHY and a target to migrate to. A bare `null` here would + // read as a transient failure and be retried forever. + const notice = DEPRECATED_PROVIDERS[provider]; + log?.warn?.( + "TOKEN_REFRESH", + `${provider} is deprecated — not refreshing; migrate this account to ${notice.migrateTo}` ); + return { + error: "unrecoverable_refresh_error", + code: "provider_deprecated", + migrateTo: notice.migrateTo, + reason: notice.reason, + }; + } case "gemini": case "antigravity": @@ -388,7 +435,6 @@ async function _getAccessTokenInternal(provider, credentials, log, proxyConfig: export function supportsTokenRefresh(provider) { const explicitlySupported = new Set([ "gemini", - "gemini-cli", // legacy refresh compatibility only; not a routable provider "antigravity", "agy", "claude", diff --git a/src/app/api/providers/[id]/refresh/route.ts b/src/app/api/providers/[id]/refresh/route.ts index a6a0847c5c..e171d9f093 100644 --- a/src/app/api/providers/[id]/refresh/route.ts +++ b/src/app/api/providers/[id]/refresh/route.ts @@ -94,12 +94,30 @@ export async function POST(_request: Request, { params }: { params: Promise<{ id newCredentials.error === "refresh_token_reused" || newCredentials.error === "invalid_grant" ) { + // A deprecated provider reuses the unrecoverable contract so callers stop + // retrying, but "Refresh token expired" would be a lie: the token is fine, the + // provider is gone. Say that, and say where to go — the operator otherwise + // re-authenticates in a loop against something that no longer exists. + const isDeprecated = newCredentials.code === "provider_deprecated"; + const reason = + isDeprecated && typeof newCredentials.reason === "string" + ? newCredentials.reason + : "Refresh token expired. Please re-authenticate this account."; await updateProviderConnection(id, { - testStatus: "invalid", - lastError: "Refresh token expired. Please re-authenticate this account.", + testStatus: isDeprecated ? "expired" : "invalid", + lastError: reason, + ...(isDeprecated + ? { lastErrorType: "provider_deprecated", errorCode: "provider_deprecated" } + : {}), }); return NextResponse.json( - { error: "Token refresh failed — provider returned no new token", requiresReauth: true }, + { + error: isDeprecated + ? "This provider was deprecated and can no longer be refreshed" + : "Token refresh failed — provider returned no new token", + requiresReauth: true, + ...(isDeprecated ? { deprecated: true, migrateTo: newCredentials.migrateTo } : {}), + }, { status: 401 } ); } diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index 61b28d21ca..c1874f1145 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -20,6 +20,7 @@ import { } from "@/lib/localDb"; import { getAccessToken, + getDeprecationNotice, supportsTokenRefresh, isUnrecoverableRefreshError, refreshCopilotToken, @@ -425,6 +426,33 @@ export async function checkConnection(conn) { return; } + // Deprecated upstream (see DEPRECATED_PROVIDERS in tokenRefresh): the provider is not + // routable, so refreshing kept a credential alive that could never answer a request. + // Surface that as a terminal state naming the migration, instead of the silent + // `Skipping … (refresh unsupported)` that dropping it from supportsTokenRefresh alone + // would produce — which would leave the row at "active" forever, doing nothing. + // + // Placed AFTER the terminal-status guard above, which makes this idempotent for free: + // once marked "expired" the connection is skipped on every later sweep, so this writes + // exactly once instead of rewriting the same reason each cycle. + const deprecation = getDeprecationNotice(String(conn.provider || "")); + if (deprecation) { + const now = new Date().toISOString(); + await updateProviderConnection(conn.id, { + testStatus: "expired", + lastHealthCheckAt: now, + lastError: deprecation.reason, + lastErrorAt: now, + lastErrorType: "provider_deprecated", + lastErrorSource: "oauth", + errorCode: "provider_deprecated", + }); + log( + `${LOG_PREFIX} ${conn.provider}/${getConnectionLogLabel(conn)} is a deprecated provider; marking expired (migrate to ${deprecation.migrateTo})` + ); + return; + } + if (!conn.refreshToken || typeof conn.refreshToken !== "string") { if (isGitHubAccessTokenOnlyConnection(conn)) { const now = new Date().toISOString(); diff --git a/tests/unit/gemini-cli-deprecation.test.ts b/tests/unit/gemini-cli-deprecation.test.ts new file mode 100644 index 0000000000..b21ee73737 --- /dev/null +++ b/tests/unit/gemini-cli-deprecation.test.ts @@ -0,0 +1,111 @@ +/** + * Deprecation of the `gemini-cli` UPSTREAM provider (not the client identity). + * + * Why this is a deprecation and not a deletion — measured on 2026-07-30: + * + * - `gemini-cli` is NOT routable: absent from PROVIDERS (open-sse/config/constants), + * REGISTRY (providerRegistry), OAUTH_PROVIDERS, and no executor references it. A + * stored connection can therefore never serve a request, no matter how fresh its + * token is. + * - The legacy refresh path DID work: it redeemed the token with + * `PROVIDERS.gemini.clientId`, which is the same public Gemini CLI / Code Assist + * OAuth client. So refreshing kept a credential alive that had nowhere to go. + * - Removing it from `supportsTokenRefresh` alone would produce a SILENT skip + * (`Skipping … (refresh unsupported)` in tokenHealthCheck) — the connection would + * sit at `active` forever while doing nothing. + * + * So the deprecation has to be *legible*: the connection becomes terminal with a + * reason that names the migration. `gemini` uses the very same OAuth client, so + * re-adding the account there is a real, working path — not advice to nowhere. + * + * NOT touched, and asserted here so a future edit cannot conflate them: the + * `gemini-cli` CLIENT identity (issue #7034) — requests ARRIVING from the Gemini CLI + * or any @google/genai-based client, where OmniRoute is the server. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { PROVIDERS } from "../../open-sse/config/constants.ts"; +import { REGISTRY } from "../../open-sse/config/providerRegistry.ts"; +import { + DEPRECATED_PROVIDERS, + getAccessToken, + getDeprecationNotice, + getRefreshLeadMs, + isDeprecatedProvider, + REFRESH_LEAD_MS, + supportsTokenRefresh, + TOKEN_EXPIRY_BUFFER_MS, +} from "../../open-sse/services/tokenRefresh.ts"; +import { CLIENT_IDENTITY_PROFILES } from "../../src/shared/constants/clientIdentityProfiles.ts"; + +test("gemini-cli is registered as deprecated, with a migration target that is routable", () => { + assert.equal(isDeprecatedProvider("gemini-cli"), true); + assert.equal(isDeprecatedProvider("gemini"), false); + assert.equal(isDeprecatedProvider("antigravity"), false); + assert.equal(isDeprecatedProvider(""), false); + + const notice = getDeprecationNotice("gemini-cli"); + assert.ok(notice, "a deprecated provider must carry a notice"); + assert.equal(notice.migrateTo, "gemini"); + assert.match(notice.reason, /gemini/i); + + // The migration target must actually be usable — otherwise the notice sends the + // operator nowhere. This is the assertion that makes the advice honest. + assert.ok(REGISTRY[notice.migrateTo], "the migration target must be a routable provider"); + assert.ok(PROVIDERS[notice.migrateTo], "the migration target must have OAuth config"); +}); + +test("a deprecated provider is no longer refresh-capable and carries no refresh lead", () => { + assert.equal(supportsTokenRefresh("gemini-cli"), false); + // The TTL entry existed only to pace a refresh that no longer happens. Dropping it + // means the generic fallback applies, which is the honest answer for a provider the + // scheduler no longer refreshes. + assert.equal(REFRESH_LEAD_MS["gemini-cli"], undefined); + assert.equal(getRefreshLeadMs("gemini-cli"), TOKEN_EXPIRY_BUFFER_MS); +}); + +test("refreshing a stored gemini-cli connection fails with a CLASSIFIED code, not silence", async () => { + const originalFetch = globalThis.fetch; + let upstreamCalls = 0; + globalThis.fetch = (async () => { + upstreamCalls++; + return new Response("{}", { status: 200 }); + }) as typeof fetch; + + try { + const result = await getAccessToken( + "gemini-cli", + { refreshToken: "legacy-gemini-cli-refresh" }, + {} + ); + + assert.equal(upstreamCalls, 0, "a deprecated provider must not touch the upstream at all"); + assert.equal( + result.error, + "unrecoverable_refresh_error", + "reuse the established unrecoverable contract so every existing caller stops retrying" + ); + assert.equal(result.code, "provider_deprecated", "…but with a code that says WHY"); + assert.equal(result.migrateTo, "gemini", "and the migration target, for a legible message"); + assert.equal(result.accessToken, undefined); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("the gemini-cli CLIENT identity is untouched (issue #7034)", () => { + // Category A. Requests ARRIVING from the Gemini CLI — OmniRoute is the server here. + // Deleting this is the failure mode the deprecation must never cause. + assert.ok( + CLIENT_IDENTITY_PROFILES["gemini-cli"], + "the gemini-cli client-identity profile must survive the provider deprecation" + ); + assert.equal(CLIENT_IDENTITY_PROFILES["gemini-cli"].id, "gemini-cli"); +}); + +test("deprecation does not resurrect the provider into any routable registry", () => { + assert.equal(REGISTRY["gemini-cli"], undefined); + assert.equal(PROVIDERS["gemini-cli"], undefined); + assert.ok(Object.prototype.hasOwnProperty.call(DEPRECATED_PROVIDERS, "gemini-cli")); +}); diff --git a/tests/unit/gemini-cli-legacy-refresh.test.ts b/tests/unit/gemini-cli-legacy-refresh.test.ts index a793618a5a..633322f491 100644 --- a/tests/unit/gemini-cli-legacy-refresh.test.ts +++ b/tests/unit/gemini-cli-legacy-refresh.test.ts @@ -11,10 +11,27 @@ import { } from "../../open-sse/services/tokenRefresh.ts"; import { OAUTH_PROVIDERS } from "../../src/shared/constants/providers.ts"; -// #8232 set out to repair OAuth refresh for legacy stored connections, but -// exceeded that compatibility goal by restoring a complete routable and -// UI-visible Gemini CLI provider. Preserve only the legacy refresh path while -// keeping the discontinued provider out of public registries and routing. +// The arc of this file, kept whole because each step is the reason the next made sense: +// +// #8232 Restored OAuth auto-refresh for stored `gemini-cli` connections — a real user +// report: the UI advertises automatic token rotation for OAuth providers, and +// these rows never rotated. It overshot, restoring a routable, UI-visible +// provider along the way. +// #8275 Narrowed that to the legacy refresh path ONLY, keeping the discontinued +// provider out of the public registries and out of routing. +// now Deprecated. What #8275 left was a refresh that WORKED (it redeemed against +// PROVIDERS.gemini's client — the same public Gemini CLI OAuth client) for a +// provider that is NOT routable. So the token stayed fresh and could never +// answer a request: periodic upstream calls maintaining a dead credential. +// +// The refresh assertions below therefore now assert the deprecation instead of the +// refresh. They were rewritten, not removed — the count is unchanged and the behavior is +// pinned harder than before (a silent skip would pass a weaker test; a classified code +// does not). Registry-exclusion coverage from #8275 is untouched, because that guarantee +// still holds and is still worth guarding. +// +// Companion: tests/unit/gemini-cli-deprecation.test.ts covers the notice itself, the +// routability of the migration target, and the untouched CLIENT identity (#7034). test("Gemini CLI stays out of the chat and OAuth provider registries", () => { assert.equal(REGISTRY["gemini-cli"], undefined); @@ -24,9 +41,13 @@ test("Gemini CLI stays out of the chat and OAuth provider registries", () => { assert.ok(REGISTRY.antigravity); }); -test("legacy Gemini CLI connections retain proactive token refresh", () => { - assert.equal(REFRESH_LEAD_MS["gemini-cli"], REFRESH_LEAD_MS.antigravity); - assert.equal(supportsTokenRefresh("gemini-cli"), true); +test("legacy Gemini CLI connections are no longer refreshed at all", () => { + // Was: lead time equal to antigravity's, supportsTokenRefresh === true. + assert.equal(supportsTokenRefresh("gemini-cli"), false); + assert.equal(REFRESH_LEAD_MS["gemini-cli"], undefined); + // The sibling Google-backed providers must NOT be affected by the deprecation. + assert.equal(supportsTokenRefresh("gemini"), true); + assert.equal(REFRESH_LEAD_MS.antigravity, 15 * 60 * 1000); }); test("Gemini CLI stays out of the provider translation snapshot", () => { @@ -35,22 +56,20 @@ test("Gemini CLI stays out of the provider translation snapshot", () => { assert.equal(snapshot["gemini-cli"], undefined); }); -test("legacy Gemini CLI refresh reuses Gemini OAuth credentials without a provider entry", async () => { +test("legacy Gemini CLI refresh never reaches Google's token endpoint anymore", async () => { + // Was: asserted a successful POST to OAUTH_ENDPOINTS.google.token carrying + // PROVIDERS.gemini's client_id/secret, returning a new access token. That call is the + // waste the deprecation removes — the token it produced could not route anywhere. Now + // the assertion is stronger: not "it fails", but "no upstream call happens at all". const originalFetch = globalThis.fetch; - const calls: Array<{ url: string; options: RequestInit }> = []; + const calls: string[] = []; - globalThis.fetch = (async (url, options: RequestInit = {}) => { - calls.push({ url: String(url), options }); - return new Response( - JSON.stringify({ - access_token: "legacy-gemini-cli-access-new", - expires_in: 3600, - }), - { - status: 200, - headers: { "content-type": "application/json" }, - } - ); + globalThis.fetch = (async (url) => { + calls.push(String(url)); + return new Response(JSON.stringify({ access_token: "should-never-be-requested" }), { + status: 200, + headers: { "content-type": "application/json" }, + }); }) as typeof fetch; try { @@ -60,27 +79,27 @@ test("legacy Gemini CLI refresh reuses Gemini OAuth credentials without a provid {} ); - assert.equal(calls.length, 1); - assert.equal(calls[0].url, OAUTH_ENDPOINTS.google.token); - - const body = new URLSearchParams(String(calls[0].options.body)); - assert.equal(body.get("grant_type"), "refresh_token"); - assert.equal(body.get("refresh_token"), "legacy-gemini-cli-refresh-old"); - assert.equal(body.get("client_id"), LEGACY_PROVIDERS.gemini.clientId); - assert.equal(body.get("client_secret"), LEGACY_PROVIDERS.gemini.clientSecret); - assert.deepEqual(result, { - accessToken: "legacy-gemini-cli-access-new", - refreshToken: "legacy-gemini-cli-refresh-old", - expiresIn: 3600, - }); + assert.deepEqual(calls, [], `expected zero upstream calls, got ${calls.join(", ")}`); + assert.notEqual( + calls[0], + OAUTH_ENDPOINTS.google.token, + "the Google token endpoint must not be contacted for a deprecated provider" + ); + assert.equal(result.accessToken, undefined, "no token may be handed back"); + assert.equal(result.code, "provider_deprecated"); } finally { globalThis.fetch = originalFetch; } }); -test("legacy Gemini CLI refresh surfaces revoked tokens as unrecoverable", async () => { +test("legacy Gemini CLI refresh reports deprecation, not a revoked token", async () => { + // Was: a 400 invalid_grant from upstream surfaced as + // { error: "unrecoverable_refresh_error", code: "invalid_grant" }. The envelope is + // deliberately unchanged — every existing caller keys on `error` and must keep + // stopping its retries (isUnrecoverableRefreshError, the manual-refresh route). Only + // the `code` differs, and that difference is the whole point: "your token was revoked" + // and "this provider no longer exists" demand different actions from the operator. const originalFetch = globalThis.fetch; - globalThis.fetch = (async () => new Response(JSON.stringify({ error: "invalid_grant" }), { status: 400, @@ -93,10 +112,16 @@ test("legacy Gemini CLI refresh surfaces revoked tokens as unrecoverable", async { refreshToken: "legacy-gemini-cli-refresh-revoked" }, {} ); - assert.deepEqual(result, { - error: "unrecoverable_refresh_error", - code: "invalid_grant", - }); + assert.equal(result.error, "unrecoverable_refresh_error"); + assert.equal(result.code, "provider_deprecated"); + assert.equal(result.migrateTo, "gemini"); + assert.match(result.reason, /gemini/i); + + // The pre-deprecation behavior for a genuinely revoked token still works for the + // provider that IS routable — proof the deprecation did not blunt the real path. + const geminiResult = await getAccessToken("gemini", { refreshToken: "revoked" }, {}); + assert.equal(geminiResult.error, "unrecoverable_refresh_error"); + assert.equal(geminiResult.code, "invalid_grant"); } finally { globalThis.fetch = originalFetch; }