Skip to content

fix(oauth): parse numeric-epoch expiresAt on the request-time refresh path - #16621

Merged
diegosouzapw merged 2 commits into
release/v3.8.52from
fix/port-pr-3997-oauth-numeric-epoch-expiry
Oct 15, 2026
Merged

diegosouzapw merged 2 commits into
release/v3.8.52from
fix/port-pr-3997-oauth-numeric-epoch-expiry

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Summary

  • The request-time OAuth refresh paths now parse a numeric-epoch expiresAt (seconds or ms, as a number or as the string a TEXT column returns) the same way the background token sweep already does.
  • Fixes three defects: the proactive on-request refresh never fired for numeric-string expiries, epoch-seconds numbers caused a refresh on every request, and a still-valid rotated DB token was discarded in favour of a needless OAuth refresh.
  • An unparseable or missing expiry never triggers a refresh. ISO values behave exactly as before.

Attribution

Thanks to @elielsousa-pathbit for the original implementation.

Changes

  • src/sse/services/tokenRefresh.ts — checkAndRefreshToken uses parseTokenExpiryMs() and skips the regular-token check when the result is 0.
  • open-sse/executors/base.ts — BaseExecutor.needsRefresh uses parseTokenExpiryMs(); <= 0 means no refresh.
  • open-sse/services/tokenRefresh.ts — the stale-token-from-DB check (_refreshWithFreshCredentials) uses parseTokenExpiryMs() for dbExpiresAt.
  • All three reuse the existing pure helper src/lib/tokenHealthCheckExpiry.ts::parseTokenExpiryMs, which has no imports, so no import cycle is added. No new helper, no contract change.
  • tests/unit/oauth-expiry-numeric-epoch-request-path.test.ts — 7 new regression tests.

Test plan

  • TDD: new test before the fix → ℹ tests 7 ℹ pass 2 ℹ fail 5. The two passing tests are the ISO/unknown-expiry controls. After the fix → ℹ tests 7 ℹ pass 7 ℹ fail 0.
  • All 137 existing unit test files that reference checkAndRefreshToken / needsRefresh / getAccessToken / parseTokenExpiryMs / executors/base.ts / services/tokenRefresh, run with the test:unit:ci preamble and an isolated DATA_DIR → ℹ tests 1456 ℹ pass 1456 ℹ fail 0 ℹ cancelled 0.
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> → exit 0.
  • npx prettier --check <changed files> → clean.
  • npm run typecheck:core → clean.
  • node scripts/check/check-pr-test-policy.mjs → Result: PASS.
  • GITHUB_BASE_REF=release/v3.8.52 npm run check:test-masking → OK — 0 modificado(s) … sem enfraquecimento.
  • npm run check:cycles → no cycle touches the changed files.

⚠️ base-red inherited: #15306 — unit/vitest/integration/package-artifact gates on release/v3.8.52

diegosouzapw and others added 2 commits October 14, 2026 19:31
… path

expires_at is a TEXT column, so a numeric epoch written by an external
import reads back as a string and `new Date("1789012345678")` is an
Invalid Date. The background sweep already handled this through
parseTokenExpiryMs, but the three request-time paths did not:

- checkAndRefreshToken: a numeric string gave NaN, so the proactive
  refresh never fired; an epoch-seconds number was read as ms (1970),
  so it refreshed on every request.
- BaseExecutor.needsRefresh: same defect.
- the stale-token-from-DB check in getAccessToken: NaN discarded a
  still-valid rotated DB token and issued a needless OAuth refresh.

All three now go through parseTokenExpiryMs; an unparseable or missing
expiry (0) never triggers a refresh, and ISO values behave as before.

Co-authored-by: Eliel Sousa <eliel@pathbit.co>
Inspired-by: decolua/9router#3997
@github-actions

Copy link
Copy Markdown
Contributor

CI Coverage Report

  • Coverage job: skipped
  • PR test policy: success

Coverage artifact was not available for this run.

@diegosouzapw
diegosouzapw merged commit c092c20 into release/v3.8.52 Oct 15, 2026
32 of 51 checks passed
diegosouzapw added a commit that referenced this pull request Oct 15, 2026
## Summary

- Re-measure the five files this owner merge-batch pushed over the frozen ceilings, on release/v3.8.52 at d50cd80. No slack.
- `open-sse/executors/base.ts` 1743→1745 (#16621), `open-sse/handlers/chatCore/streamingResponse.ts` 1308→1309 (#16705), `open-sse/handlers/chatCore.ts` 3938→3943 (#16720), `open-sse/translator/response/openai-responses.ts` 1547→1590 (#16645), `tests/unit/provider-validation-specialty.test.ts` 2912→2965 (#16625).
- `node scripts/check/check-file-size.mjs` exits 0 on this branch. README drift (#16132) and the inherited doc-source drift were not stamped.

⚠️ base-red inherited: #15306

Thanks @diegosouzapw.
@diegosouzapw
diegosouzapw deleted the fix/port-pr-3997-oauth-numeric-epoch-expiry branch October 18, 2026 23:50
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