fix(oauth): set tokenExpiresAt at creation + surface no-refresh-token as expired (#5326) (#5347)

Integrated into release/v3.8.41 — Antigravity Token Expired badge: set tokenExpiresAt at creation + surface no-refresh-token as expired (#5326). Tests green (2 + 3).
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-29 11:21:57 -03:00
committed by GitHub
parent 98c9f5b136
commit cd9a9abbf5
6 changed files with 235 additions and 38 deletions

View File

@@ -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 `<tool>`-contract system message + `<tool>{…}</tool>` 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

View File

@@ -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();

View File

@@ -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<string, any>,
expiresAt: string | null
) {
return {
provider,
authType: "oauth" as const,
...tokenData,
expiresAt,
tokenExpiresAt: expiresAt,
testStatus: "active" as const,
};
}
async function syncToCloudIfEnabled(): Promise<void> {
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();

View File

@@ -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") {

View File

@@ -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);
});

View File

@@ -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");
});