diff --git a/CHANGELOG.md b/CHANGELOG.md index 23ff034ba5..6bcad617b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ _In development — bullets added per PR; finalized at release._ ### 🔧 Bug Fixes -- **chatgpt-web:** wire tool/function calling into the `chatgpt-web` provider. It was the only web-session executor that never read `body.tools` — both response builders hardcoded `finish_reason:"stop"` and emitted only content, so tool calls were silently dropped (the model answered in prose). It now uses the shared `webTools` prompt-emulation shim (a ``-contract system message + `{…}` response parsing) exactly like its 9 sibling executors (qwen-web, perplexity-web, …) — it was simply omitted from the #3259 rollout. Tool mode buffers and emits `tool_calls` + `finish_reason:"tool_calls"` (gated off the image-gen path); plain chat is unchanged. Regression guard: `tests/unit/chatgpt-web-tools-5240.test.ts`. ([#5240](https://github.com/diegosouzapw/OmniRoute/issues/5240) — thanks @Rougler) +- **oauth/dashboard:** fix the persistent/false Antigravity "Token Expired" badge (continuation of #3679/#3850). Two causes: **(1)** new OAuth connections never set `tokenExpiresAt` (only `expiresAt`), so the dashboard badge — which prefers `tokenExpiresAt || expiresAt` — fell back to the original grant clock and could flash a false "Token Expired" until the first background refresh. Creation now mirrors `expiresAt` into `tokenExpiresAt` across all 5 OAuth create paths (a shared `buildOAuthConnectionCreatePayload`), consistent with every refresh path which already writes both. **(2)** when a refresh-capable connection has no usable refresh token, the health-check sweep silently skipped it, leaving `testStatus="active"` forever while the cosmetic badge showed expired; it now surfaces a terminal `testStatus="expired"` ("needs re-auth"), tightly gated so it never clobbers non-refresh providers or already-terminal/cooldown states. Regression guards: `tests/unit/oauth-connection-tokenexpiresat-5326.test.ts`, `tests/unit/token-health-no-refresh-token-expired-5326.test.ts`. ([#5326](https://github.com/diegosouzapw/OmniRoute/issues/5326)) ### 🔧 Bug Fixes diff --git a/src/app/api/oauth/[provider]/[action]/route.ts b/src/app/api/oauth/[provider]/[action]/route.ts index 5b1d5fb179..4bf2a1ba24 100755 --- a/src/app/api/oauth/[provider]/[action]/route.ts +++ b/src/app/api/oauth/[provider]/[action]/route.ts @@ -9,7 +9,10 @@ import { pollForToken, resolveBrowserOAuthRedirectUri, } from "@/lib/oauth/providers"; -import { persistOAuthConnection } from "@/lib/oauth/connectionPersistence"; +import { + persistOAuthConnection, + buildOAuthConnectionCreatePayload, +} from "@/lib/oauth/connectionPersistence"; import { createDeviceFlowTicket, getDeviceFlowTicketStatus } from "@/lib/oauth/deviceFlowTickets"; import { createProviderConnection, @@ -497,13 +500,9 @@ export async function POST( } } if (!connection) { - connection = await createProviderConnection({ - provider, - authType: "oauth", - ...tokenData, - expiresAt, - testStatus: "active", - }); + connection = await createProviderConnection( + buildOAuthConnectionCreatePayload(provider, tokenData, expiresAt) + ); } // Auto sync to Cloud if enabled @@ -589,13 +588,9 @@ export async function POST( } } if (!connection) { - connection = await createProviderConnection({ - provider, - authType: "oauth", - ...result.tokens, - expiresAt, - testStatus: "active", - }); + connection = await createProviderConnection( + buildOAuthConnectionCreatePayload(provider, result.tokens, expiresAt) + ); } // Auto sync to Cloud if enabled @@ -722,13 +717,9 @@ export async function POST( } } if (!connection) { - connection = await createProviderConnection({ - provider, - authType: "oauth", - ...tokenData, - expiresAt, - testStatus: "active", - }); + connection = await createProviderConnection( + buildOAuthConnectionCreatePayload(provider, tokenData, expiresAt) + ); } await syncToCloudIfEnabled(); @@ -796,13 +787,9 @@ export async function POST( } } if (!connection) { - connection = await createProviderConnection({ - provider, - authType: "oauth", - ...tokenData, - expiresAt, - testStatus: "active", - }); + connection = await createProviderConnection( + buildOAuthConnectionCreatePayload(provider, tokenData, expiresAt) + ); } await syncToCloudIfEnabled(); diff --git a/src/lib/oauth/connectionPersistence.ts b/src/lib/oauth/connectionPersistence.ts index 4bbffec13f..5ecc4e6b90 100644 --- a/src/lib/oauth/connectionPersistence.ts +++ b/src/lib/oauth/connectionPersistence.ts @@ -27,6 +27,34 @@ function safeEqual(a: string | null | undefined, b: string | null | undefined): return timingSafeEqual(ba, bb); } +/** + * Build the create payload for a brand-new OAuth connection. + * + * #5326: mirror the freshly computed `expiresAt` into `tokenExpiresAt` at creation + * time. The dashboard token-health badge prefers `tokenExpiresAt` over `expiresAt` + * (ConnectionRow.tsx: `connection.tokenExpiresAt || connection.expiresAt`). If + * `tokenExpiresAt` stays null on a freshly created connection, the badge falls back + * to the original grant clock and can flash a false amber/"Token Expired" until the + * first background refresh writes both fields together. All refresh paths already + * persist `expiresAt` and `tokenExpiresAt` in lockstep + * (tokenHealthCheck onPersist, tokenRefresh.updateProviderCredentials); this makes + * creation consistent with them. + */ +export function buildOAuthConnectionCreatePayload( + provider: string, + tokenData: Record, + expiresAt: string | null +) { + return { + provider, + authType: "oauth" as const, + ...tokenData, + expiresAt, + tokenExpiresAt: expiresAt, + testStatus: "active" as const, + }; +} + async function syncToCloudIfEnabled(): Promise { try { const cloudEnabled = await isCloudEnabled(); @@ -76,13 +104,9 @@ export async function persistOAuthConnection( } } if (!connection) { - connection = await createProviderConnection({ - provider, - authType: "oauth", - ...tokenData, - expiresAt, - testStatus: "active", - }); + connection = await createProviderConnection( + buildOAuthConnectionCreatePayload(provider, tokenData, expiresAt) + ); } await syncToCloudIfEnabled(); diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index 80316fb682..5760e41a75 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -341,7 +341,36 @@ export async function checkConnection(conn) { const intervalMin = conn.healthCheckInterval ?? DEFAULT_HEALTH_CHECK_INTERVAL_MIN; if (intervalMin <= 0) return; if (!conn.isActive) return; - if (!conn.refreshToken || typeof conn.refreshToken !== "string") return; + if (!conn.refreshToken || typeof conn.refreshToken !== "string") { + // #5326: a refresh-CAPABLE provider (e.g. antigravity/gemini) with no usable + // refresh token can never self-heal via the sweep — it genuinely needs re-auth. + // Silently skipping here left the row at testStatus="active" while the dashboard + // badge (which derives expiry from tokenExpiresAt||expiresAt) showed a confusing + // cosmetic "Token Expired". Surface reality as a terminal "expired" status instead. + // Guard tightly so we do NOT clobber: + // - providers that simply don't use refresh tokens (supportsTokenRefresh=false) + // - connections already in a terminal/specific state (expired/banned/credits_exhausted) + // - transient cooldown state (unavailable) owned by the request path + const refreshCapableNeedsReauth = + supportsTokenRefresh(conn.provider) && + (!conn.testStatus || conn.testStatus === "active"); + if (refreshCapableNeedsReauth) { + const now = new Date().toISOString(); + await updateProviderConnection(conn.id, { + testStatus: "expired", + lastHealthCheckAt: now, + lastError: "No refresh token available — re-authenticate this account.", + lastErrorAt: now, + lastErrorType: "no_refresh_token", + lastErrorSource: "oauth", + errorCode: "no_refresh_token", + }); + log( + `${LOG_PREFIX} ${conn.provider}/${getConnectionLogLabel(conn)} has no refresh token; marking expired (needs re-auth)` + ); + } + return; + } // Retry expired connections with exponential backoff up to EXPIRED_RETRY_MAX times. if (conn.testStatus === "expired") { diff --git a/tests/unit/oauth-connection-tokenexpiresat-5326.test.ts b/tests/unit/oauth-connection-tokenexpiresat-5326.test.ts new file mode 100644 index 0000000000..206183ee85 --- /dev/null +++ b/tests/unit/oauth-connection-tokenexpiresat-5326.test.ts @@ -0,0 +1,43 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { buildOAuthConnectionCreatePayload } from "../../src/lib/oauth/connectionPersistence.ts"; + +// Regression for #5326: a freshly created OAuth connection (e.g. antigravity) used +// to persist only `expiresAt`, leaving `tokenExpiresAt` null. The dashboard token +// badge prefers `tokenExpiresAt` and falls back to the original grant clock when it +// is null, flashing a false "Token Expired" until the first background refresh. +// The create payload must mirror the computed expiry into BOTH fields. +test("buildOAuthConnectionCreatePayload mirrors expiresAt into tokenExpiresAt (#5326)", () => { + const expiresAt = new Date(Date.now() + 3600 * 1000).toISOString(); + const tokenData = { + accessToken: "at-123", + refreshToken: "rt-123", + email: "user@example.com", + expiresIn: 3600, + }; + + const payload = buildOAuthConnectionCreatePayload("antigravity", tokenData, expiresAt); + + assert.equal(payload.provider, "antigravity"); + assert.equal(payload.authType, "oauth"); + assert.equal(payload.testStatus, "active"); + assert.equal(payload.expiresAt, expiresAt); + // The fix: tokenExpiresAt is set (was null/undefined before) and equals expiresAt. + assert.equal(payload.tokenExpiresAt, expiresAt); + assert.equal(payload.tokenExpiresAt, payload.expiresAt); + // tokenData fields are still carried through. + assert.equal(payload.accessToken, "at-123"); + assert.equal(payload.refreshToken, "rt-123"); +}); + +test("buildOAuthConnectionCreatePayload keeps tokenExpiresAt null when expiry is unknown", () => { + const payload = buildOAuthConnectionCreatePayload( + "antigravity", + { accessToken: "at-456" }, + null + ); + + assert.equal(payload.expiresAt, null); + assert.equal(payload.tokenExpiresAt, null); +}); diff --git a/tests/unit/token-health-no-refresh-token-expired-5326.test.ts b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts new file mode 100644 index 0000000000..062a8d77ce --- /dev/null +++ b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts @@ -0,0 +1,114 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +process.env.NODE_ENV = "test"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-hc-no-refresh-5326-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const tokenHealthCheck = await import("../../src/lib/tokenHealthCheck.ts"); + +async function resetStorage() { + core.resetDbInstance(); + for (let attempt = 0; attempt < 10; attempt++) { + try { + if (fs.existsSync(TEST_DATA_DIR)) { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } + break; + } catch (error: any) { + if ((error?.code === "EBUSY" || error?.code === "EPERM") && attempt < 9) { + await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1))); + } else { + throw error; + } + } + } + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +// Regression for #5326: a refresh-CAPABLE provider (antigravity) with NO refresh +// token used to be silently skipped by the sweep (`if (!conn.refreshToken) return`), +// leaving the row at testStatus="active" while the dashboard badge showed a +// confusing cosmetic "Token Expired". The sweep must surface reality as a terminal +// "expired" status so the row reflects that it genuinely needs re-auth. +test("checkConnection marks refresh-capable provider with no refresh token as expired (#5326)", async () => { + await resetStorage(); + + const connection = await providersDb.createProviderConnection({ + provider: "antigravity", + authType: "oauth", + name: "Antigravity No-Refresh Account", + email: "antigravity-no-refresh@example.com", + accessToken: "access-token-only", + refreshToken: null, + testStatus: "active", + isActive: true, + }); + + await tokenHealthCheck.checkConnection(connection); + + const updated = await providersDb.getProviderConnectionById((connection as any).id); + assert.equal(updated?.testStatus, "expired"); + assert.equal(updated?.errorCode, "no_refresh_token"); + assert.ok(updated?.lastHealthCheckAt); +}); + +// A connection WITH a refresh token must NOT be force-expired by this branch. Use a +// far-future known expiry so the sweep returns before attempting any network refresh, +// isolating the behavior of the no-refresh-token branch. +test("checkConnection leaves a connection WITH a refresh token untouched (#5326)", async () => { + await resetStorage(); + + const connection = await providersDb.createProviderConnection({ + provider: "antigravity", + authType: "oauth", + name: "Antigravity Healthy Account", + email: "antigravity-healthy@example.com", + accessToken: "access-token", + refreshToken: "refresh-token-present", + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(), + tokenExpiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(), + testStatus: "active", + isActive: true, + }); + + await tokenHealthCheck.checkConnection(connection); + + const updated = await providersDb.getProviderConnectionById((connection as any).id); + assert.equal(updated?.testStatus, "active"); + assert.notEqual(updated?.errorCode, "no_refresh_token"); +}); + +// A provider that does NOT support token refresh must be left untouched even with no +// refresh token (it never had one to lose, and is not "expired" — just non-refreshing). +test("checkConnection leaves a non-refresh provider with no refresh token untouched (#5326)", async () => { + await resetStorage(); + + const connection = await providersDb.createProviderConnection({ + provider: "custom-no-refresh-support-5326", // not in supportsTokenRefresh + no tokenUrl/refreshUrl + authType: "oauth", + name: "Non-refresh Provider Account", + email: "non-refresh@example.com", + accessToken: "access-token-only", + refreshToken: null, + testStatus: "active", + isActive: true, + }); + + await tokenHealthCheck.checkConnection(connection); + + const updated = await providersDb.getProviderConnectionById((connection as any).id); + assert.equal(updated?.testStatus, "active"); + assert.notEqual(updated?.errorCode, "no_refresh_token"); +});