Skip to content

fix(healthcheck): per-provider proactive-refresh skip list (rescue short-TTL OAuth like Kimi) #3159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 1 commit into from
Jun 4, 2026
Merged

fix(healthcheck): per-provider proactive-refresh skip list (rescue short-TTL OAuth like Kimi) #3159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,12 @@ APP_LOG_TO_FILE=true
# Disable the OAuth token healthcheck loop during tests (default: true).
# OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK=true

# Exclude specific providers from the PROACTIVE token-refresh sweep (comma-separated,
# case-insensitive). Targeted alternative to OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK: keeps
# rotating-cascade providers (Codex/OpenAI share one Auth0 family) on the reactive 401
# path only, while short-TTL providers like Kimi-coding keep being refreshed proactively.
# OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS=codex,openai

# Silence healthcheck noise in Playwright stdout (default: true).
# OMNIROUTE_HIDE_HEALTHCHECK_LOGS=true

Expand Down
1 change: 1 addition & 0 deletions docs/reference/ENVIRONMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,7 @@ value below unset in production deployments.
| `OMNIROUTE_E2E_PASSWORD` | falls back to `INITIAL_PASSWORD` | `scripts/dev/run-next-playwright.mjs` | Admin password injected into the Playwright environment. |
| `OMNIROUTE_DISABLE_LOCAL_HEALTHCHECK` | `true` | `scripts/dev/run-next-playwright.mjs` | Disable the local healthcheck poll during Playwright runs. |
| `OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK` | `true` | `scripts/dev/run-next-playwright.mjs` | Disable the OAuth token healthcheck loop during tests. |
| `OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS` | _(unset)_ | `src/lib/tokenHealthCheck.ts` | Comma-separated providers excluded from the proactive token-refresh sweep (e.g. `codex,openai`). Targeted alternative to fully disabling the healthcheck — short-TTL providers keep refreshing while cascade providers stay reactive-only. |
| `OMNIROUTE_HIDE_HEALTHCHECK_LOGS` | `true` | `scripts/dev/run-next-playwright.mjs` | Silence healthcheck noise in Playwright stdout. |
| `OMNIROUTE_PLAYWRIGHT_SKIP_BUILD` | `0` | `scripts/dev/run-next-playwright.mjs` | Skip the Next.js production build before Playwright starts (CI optimization). |
| `OMNIROUTE_SKIP_UNINSTALL_HOOK` | `0` | `scripts/build/uninstall.mjs` | Skip the OmniRoute uninstall hook (used by CI to keep `node_modules` intact). |
Expand Down
26 changes: 26 additions & 0 deletions src/lib/tokenHealthCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,25 @@ function isHealthCheckDisabled(): boolean {
);
}

/**
* Providers excluded from the PROACTIVE refresh sweep, comma-separated and
* case-insensitive (e.g. "codex,openai"). A targeted alternative to the blunt
* OMNIROUTE_DISABLE_TOKEN_HEALTHCHECK switch: it lets an operator keep the
* rotating-token cascade providers (Codex/OpenAI share one Auth0 family) off the
* proactive sweep — leaving their refresh to the reactive, serialized 401 path —
* WITHOUT also starving short-TTL providers like Kimi-coding, whose tokens expire
* while idle when the whole sweep is disabled.
*/
function getHealthCheckSkipProviders(): Set<string> {
const raw = process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS || "";
return new Set(
raw
.split(",")
.map((s) => s.trim().toLowerCase())
.filter(Boolean)
);
}
Comment on lines +121 to +129

@gemini-code-assist gemini-code-assist Bot Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing process.env and parsing the comma-separated skip list on every single connection check is inefficient, especially in environments with hundreds of connections. Since checkConnection is called in a loop during sweep(), we can optimize this by caching the parsed Set and only re-parsing if the raw environment variable changes (which is useful for tests).

let cachedSkipProviders: Set<string> | null = null;
let lastRawSkipProviders: string | null = null;

function getHealthCheckSkipProviders(): Set<string> {
  const raw = process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS || "";
  if (cachedSkipProviders && raw === lastRawSkipProviders) {
    return cachedSkipProviders;
  }
  const parsed = new Set(
    raw
      .split(",")
      .map((s) => s.trim().toLowerCase())
      .filter(Boolean)
  );
  cachedSkipProviders = parsed;
  lastRawSkipProviders = raw;
  return parsed;
}


// ── Logging helper ───────────────────────────────────────────────────────────
let cachedHideLogs: boolean | null = null;
let cacheTimestamp = 0;
Expand Down Expand Up @@ -265,6 +284,13 @@ export async function checkConnection(conn) {
const latestConnection = (await getProviderConnectionById(conn.id)) || conn;
conn = latestConnection;

// Per-provider opt-out of proactive refresh (e.g. Codex/OpenAI cascade
// providers) — their token stays on the reactive, serialized 401 path while
// other providers keep being refreshed proactively.
if (getHealthCheckSkipProviders().has(String(conn.provider || "").toLowerCase())) {
return;
}

// Determine interval (0 = disabled)
const intervalMin = conn.healthCheckInterval ?? DEFAULT_HEALTH_CHECK_INTERVAL_MIN;
if (intervalMin <= 0) return;
Expand Down
71 changes: 71 additions & 0 deletions tests/unit/token-health-check.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,3 +398,74 @@ test("checkConnection skips interval refresh when token expiry is known and stil
}
);
});

test("checkConnection skips providers listed in OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS (#kimi-15)", async () => {
await resetStorage();

const providerId = "custom-oauth-skip-list";
const refreshRequests: string[] = [];
const prevSkip = process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS;

await withHttpServer(
(req, res) => {
let body = "";
req.setEncoding("utf8");
req.on("data", (chunk) => {
body += chunk;
});
req.on("end", () => {
refreshRequests.push(body);
res.writeHead(200, { "Content-Type": "application/json" });
res.end(
JSON.stringify({
access_token: "should-not-be-fetched",
refresh_token: "should-not-be-fetched",
expires_in: 3600,
})
);
});
},
async (tokenServer) => {
await withPatchedProvider(
providerId,
{
tokenUrl: `${tokenServer.url}/token`,
clientId: "skip-client-id",
clientSecret: "skip-client-secret",
},
async () => {
const connection = await providersDb.createProviderConnection({
provider: providerId,
authType: "oauth",
name: "Skip-list Account",
email: "skip@example.com",
accessToken: "stale-access-token",
refreshToken: "refresh-token-skip",
isActive: true,
});

// The connection is due for refresh (no known expiry, never checked).
// With the provider listed, the proactive sweep must skip it entirely —
// NO refresh request is made.
process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = `foo, ${providerId} ,bar`;
await tokenHealthCheck.checkConnection(connection);
assert.equal(
refreshRequests.length,
0,
"listed provider must NOT trigger a proactive refresh"
);

// Control: with the provider no longer listed, the same due connection
// IS refreshed — proving the skip (not token freshness) gated it.
process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = "some-other-provider";
const stillStale = await providersDb.getProviderConnectionById((connection as any).id);
await tokenHealthCheck.checkConnection(stillStale);
assert.equal(refreshRequests.length, 1, "non-listed provider must refresh");
}
);
}
);

if (prevSkip === undefined) delete process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS;
else process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = prevSkip;
});
Comment on lines +402 to +471

@gemini-code-assist gemini-code-assist Bot Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If any assertion fails during the test execution (e.g., assert.equal(refreshRequests.length, 0)), the test will abort immediately, and the cleanup code at the end of the test will never run. This leaks the modified process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS to subsequent tests, causing potential test pollution. Wrap the test logic in a try...finally block to guarantee the environment variable is restored.

test("checkConnection skips providers listed in OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS (#kimi-15)", async () => {
  await resetStorage();

  const providerId = "custom-oauth-skip-list";
  const refreshRequests: string[] = [];
  const prevSkip = process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS;

  try {
    await withHttpServer(
      (req, res) => {
        let body = "";
        req.setEncoding("utf8");
        req.on("data", (chunk) => {
          body += chunk;
        });
        req.on("end", () => {
          refreshRequests.push(body);
          res.writeHead(200, { "Content-Type": "application/json" });
          res.end(
            JSON.stringify({
              access_token: "should-not-be-fetched",
              refresh_token: "should-not-be-fetched",
              expires_in: 3600,
            })
          );
        });
      },
      async (tokenServer) => {
        await withPatchedProvider(
          providerId,
          {
            tokenUrl: tokenServer.url + "/token",
            clientId: "skip-client-id",
            clientSecret: "skip-client-secret",
          },
          async () => {
            const connection = await providersDb.createProviderConnection({
              provider: providerId,
              authType: "oauth",
              name: "Skip-list Account",
              email: "skip@example.com",
              accessToken: "stale-access-token",
              refreshToken: "refresh-token-skip",
              isActive: true,
            });

            // The connection is due for refresh (no known expiry, never checked).
            // With the provider listed, the proactive sweep must skip it entirely —
            // NO refresh request is made.
            process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = "foo, " + providerId + " ,bar";
            await tokenHealthCheck.checkConnection(connection);
            assert.equal(
              refreshRequests.length,
              0,
              "listed provider must NOT trigger a proactive refresh"
            );

            // Control: with the provider no longer listed, the same due connection
            // IS refreshed — proving the skip (not token freshness) gated it.
            process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = "some-other-provider";
            const stillStale = await providersDb.getProviderConnectionById((connection as any).id);
            await tokenHealthCheck.checkConnection(stillStale);
            assert.equal(refreshRequests.length, 1, "non-listed provider must refresh");
          }
        );
      }
    );
  } finally {
    if (prevSkip === undefined) delete process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS;
    else process.env.OMNIROUTE_HEALTHCHECK_SKIP_PROVIDERS = prevSkip;
  }
});