Skip to content

fix(oauth): parse numeric epoch expiresAt so imported connections still refresh - #3997

Open
elielsousa-pathbit wants to merge 1 commit into
decolua:masterfrom
elielsousa-pathbit:fix/numeric-epoch-expires-at
Open

elielsousa-pathbit wants to merge 1 commit into
decolua:masterfrom
elielsousa-pathbit:fix/numeric-epoch-expires-at

Conversation

@elielsousa-pathbit

Copy link
Copy Markdown

Problem

parseTimeMs() in open-sse/services/oauthCredentialManager.js accepts a number and anything Date can parse, but a numeric epoch in string form falls through to the Date branch, where it is an Invalid Date:

new Date("1789012345678").getTime()  // NaN  -> parseTimeMs returns null
new Date("1789012345").getTime()     // NaN  -> parseTimeMs returns null

A null expiry silently disables both proactive refresh paths:

Call site Effect of null
shouldRefreshCredentials() (oauthCredentialManager.js:48-51) expiresAtMs !== null is false → on-request refresh in checkAndRefreshToken() never fires
selectConnectionsNeedingRefresh() (src/sse/services/backgroundTokenRefresh.js:59-60) if (expiresAtMs === null) continue; → the background sweep skips the connection entirely

The connection then keeps an expired access token and 401s until the user re-authenticates by hand — the same user-visible symptom as #2546 ("session dies 40-45 min after login"), reached through a different input shape.

Where a numeric epoch comes from

The bulk-import routes persist the user-supplied value verbatim, with no normalization:

  • src/app/api/oauth/grok-cli/bulk-import/route.js:77 — let expiresAt = raw.expires_at || raw.expiresAt || null;
  • src/app/api/oauth/codex/bulk-import/route.js:97 — only fills expiresAt when it is absent; an existing item.expiresAt passes through raw.
  • src/app/api/oauth/kiro/import-cli-proxy/route.js:20 — expiresAt: tokenData.expiresAt

Compare src/lib/oauth/kiroExternalIdp.js:56-62 (resolveExpiresAt), which already normalizes any parseable expires_at to ISO. The repo has the right convention; the parser just doesn't accept the shape those routes can store.

The same applies to any external tool that syncs credentials into providerConnections.data and writes the epoch as text.

Fix

Minimal and contained to the parser plus one caller:

  1. parseTimeMs() converts numeric strings with the same seconds/ms heuristic it already applies to numbers (< 1e12 → seconds), then falls back to Date for ISO and other date strings. Non-numeric garbage and empty values still return null.
  2. parseTimeMs() is exported, and normalizeExpiresAt() in src/sse/services/tokenRefresh.js now reuses it — so an epoch that is already stored self-heals to ISO on the next refresh instead of being dropped.

No behavior change for values that already worked (ISO strings, numbers).

Validation

Failing-then-passing test, plus a full-suite regression diff against master in the same checkout.

# new test, on master (before the fix)
$ npx vitest run unit/credential-expiry-numeric-epoch.test.js
  Tests  4 failed | 2 passed (6)

# same test, with the fix
$ npx vitest run unit/credential-expiry-numeric-epoch.test.js
  Tests  6 passed (6)

# related suites
$ npx vitest run unit/background-token-refresh.test.js unit/grok-cli-expiresat-2546.test.js \
    unit/codex-refresh-token.test.js unit/token-refresh-dispatch.test.js \
    unit/token-refresh-generic.test.js unit/auth-status.test.js
  Test Files  6 passed (6)
  Tests      34 passed (34)

$ npx eslint open-sse/services/oauthCredentialManager.js src/sse/services/tokenRefresh.js \
    tests/unit/credential-expiry-numeric-epoch.test.js
  (clean)

Full tests/ suite, run on master and on this branch in the same checkout:

passing failing
master 2214 114
this branch 2220 114

0 regressions — no test that passes on master fails here. +6 is exactly the new file. The 114 failures are identical on both sides and pre-existing (CLAUDE.md documents the suite as not all-green on a plain checkout).

Note: tests/__baseline__/verify-no-regression.mjs could not be used as the gate — it keys on f.name.split("/app/")[1], so outside the /app container every path becomes undefined and all failures read as regressions. The master-vs-branch diff above is the equivalent check.

Files changed

  • open-sse/services/oauthCredentialManager.js — parseTimeMs() accepts numeric strings; exported.
  • src/sse/services/tokenRefresh.js — normalizeExpiresAt() routes through parseTimeMs().
  • tests/unit/credential-expiry-numeric-epoch.test.js — new regression test (6 cases).

…ll refresh

parseTimeMs() accepted numbers and date-like strings, but a numeric epoch in
string form ("1789012345678") went down the Date branch, where it is an
Invalid Date, and returned null.

A null expiry silently disables both proactive refresh paths:
  - shouldRefreshCredentials() returns false, so the on-request refresh in
    checkAndRefreshToken() never fires;
  - selectConnectionsNeedingRefresh() skips the connection, so the background
    sweep never fires either.

The connection then keeps an expired access token and 401s until the user
re-authenticates by hand - the same symptom as decolua#2546, reached through a
different input shape.

Numeric epochs reach the DB unnormalized through the bulk-import routes, which
persist the user-supplied value verbatim:
  src/app/api/oauth/grok-cli/bulk-import/route.js:77  (raw.expires_at)
  src/app/api/oauth/codex/bulk-import/route.js:97     (item.expiresAt)
compare src/lib/oauth/kiroExternalIdp.js:57, which already normalizes to ISO.

parseTimeMs() now converts numeric strings with the same seconds/ms heuristic
it already applies to numbers, and is exported so normalizeExpiresAt() in
src/sse/services/tokenRefresh.js reuses it - a numeric epoch that is already
stored now self-heals to ISO on the next refresh.
bloodf pushed a commit to bloodf/durindoor that referenced this pull request Sep 17, 2026
…ric epoch string

`parseTimeMs` handled numbers and date-like strings, but `new Date("1789012345678")`
is an Invalid Date, so a numeric epoch stored as text resolved to no expiry at
all. With a null expiry `shouldRefreshCredentials` returns false, the proactive
refresh never fires, and the connection keeps serving an expired access token
until someone re-authorizes it by hand.

The shape is reachable: both OAuth bulk-import routes persist the user-supplied
value verbatim and only synthesize an ISO string when the field is absent —
src/app/api/oauth/grok-cli/bulk-import/route.js:79 and
src/app/api/oauth/codex/bulk-import/route.js:110.

Numeric text is now converted numerically before the Date fallback, and
normalizeExpiresAt routes through the same parser so an affected connection
self-heals to ISO form on its next refresh.

Upstream: decolua/9router#3997
bloodf pushed a commit to bloodf/durindoor that referenced this pull request Sep 17, 2026
…ric epoch string

`parseTimeMs` handled numbers and date-like strings, but `new Date("1789012345678")`
is an Invalid Date, so a numeric epoch stored as text resolved to no expiry at
all. With a null expiry `shouldRefreshCredentials` returns false, the proactive
refresh never fires, and the connection keeps serving an expired access token
until someone re-authorizes it by hand.

The shape is reachable: both OAuth bulk-import routes persist the user-supplied
value verbatim and only synthesize an ISO string when the field is absent —
src/app/api/oauth/grok-cli/bulk-import/route.js:79 and
src/app/api/oauth/codex/bulk-import/route.js:110.

Numeric text is now converted numerically before the Date fallback, and
normalizeExpiresAt routes through the same parser so an affected connection
self-heals to ISO form on its next refresh.

Upstream: decolua/9router#3997
bloodf added a commit to bloodf/durindoor that referenced this pull request Sep 17, 2026
…ric epoch string (#882)

`parseTimeMs` handled numbers and date-like strings, but `new Date("1789012345678")`
is an Invalid Date, so a numeric epoch stored as text resolved to no expiry at
all. With a null expiry `shouldRefreshCredentials` returns false, the proactive
refresh never fires, and the connection keeps serving an expired access token
until someone re-authorizes it by hand.

The shape is reachable: both OAuth bulk-import routes persist the user-supplied
value verbatim and only synthesize an ISO string when the field is absent —
src/app/api/oauth/grok-cli/bulk-import/route.js:79 and
src/app/api/oauth/codex/bulk-import/route.js:110.

Numeric text is now converted numerically before the Date fallback, and
normalizeExpiresAt routes through the same parser so an affected connection
self-heals to ISO form on its next refresh.

Upstream: decolua/9router#3997

Co-authored-by: durindoor-bot <bot@durindoor.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant