[habdelra]

Note

Stacked on #6942 (CS-14316). The base is that PR's branch, so this diff shows only the shared refresh schedule. Merge #6942 first; this PR will be retargeted to main when it lands.

The host keeps its realm-server session alive with RealmServerService.tokenRefresher, which waits until tokenRefreshPeriodSec before the session's exp and then mints a new one through _server-session. That wait was max(exp - now - period, 0) with no floor. If a freshly minted session arrives already inside the five-minute refresh period, the wait is zero and the refresher mints again at once. Every replacement lands in the same place, so the refresher loops. Each mint costs Synapse an openid/request_token and the realm server an openid/userinfo. Two things put a fresh session inside the period: a server-side session lifetime at or under five minutes, or a browser clock far enough ahead of the server's.

The per-realm refresher (RealmResource.tokenRefresher) already handles this case. This change moves that logic into one place, TokenRefreshSchedule in app/lib/token-refresh-schedule.ts, and both refreshers use it, so the two can't drift apart. The schedule:

• re-mints tokenRefreshPeriodSec before exp, going by exp alone;
• re-mints a session restored from storage at once when it is already inside the period, since a stale stored session is expected and replacing it is correct;
• when a fresh mint arrives inside the period, waits minTokenRefreshIntervalSec (30 s), doubling with each such mint in a row up to maxTokenRefreshBackoffSec (30 min), and resets on the first usable mint. That wait never runs past the session's exp while at least the 30 s floor of it remains, so a server whose sessions are shorter than the refresh period gets one mint per session and the session doesn't sit expired. A session that is already expired when it arrives, as happens with clock skew, still gets the full backoff. Each of these mints is reported with console.error, naming the realm or realm server and the session's expiry.

The schedule tells a fresh mint from a restored session by remembering the last token its owner minted. RealmServerService.logout and RealmResource.logout reset it, so a new account starts without the previous account's backoff. clearRealmSession, which re-authentication after a 401 also calls, deliberately leaves the backoff in place. The constants live in the new module, and services/realm re-exports them for existing importers.

RealmServerService gains a waitForTokenRefresh(ms) seam, matching the one RealmService already has, so its refresher can run against a virtual clock in tests.

Tests

tests/unit/services/realm-server-token-refresh-test.ts runs the realm-server refresher on a virtual clock, with a stub _server-session and a minimal Matrix client:

• a server that always returns a session inside the refresh period is asked twice in a minute, with waits of 30 s and then 60 s, and both mints are reported;
• with five-minute sessions, the wait doubles to 240 s and then stays at the session's lifetime;
• after logout() and a fresh login, the backoff starts again at 30 s;
• a stored session inside the period is replaced at once, and its usable replacement is scheduled from its exp, with nothing reported.

realm-token-refresh-test.ts gains the matching logout case for RealmResource, and the rest of that suite passes unchanged against the shared schedule. Each new test fails when the code it covers is removed: the minted() call, the expiry cap, or either reset(). The virtual clock stops after a fixed number of waits, so a refresher that loops fails on its counts instead of hanging the run.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T14:49:19.083728Z	0fe9c16	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[habdelra]

[Claude Code 🤖] This review checked the shared schedule's logic, how its backoff affects the realm-server service's token consumers, and whether the two refreshers match. I did not run the host test suite, so I haven't seen the new tests pass.

No blocking issues. The schedule moves the per-realm logic over intact. Two things to address: whether the realm-server session may sit expired under a long backoff, and a reset that only one of the two refreshers gets.

1. Decide whether the wait may run past exp (see the thread on refreshDelayMs in token-refresh-schedule.ts).
2. Reset the per-realm schedule in RealmResource.logout() (see the thread on refreshSchedule in realm.ts).
3. Add a test that pins the logout reset (see the thread on logout in realm-server.ts).
4. In the PR description, the paragraph starting "To check that the first test catches the bug, I removed…" tells the verification story. The description should state the current test plan instead. Not blocking.

[habdelra]

[Claude Code 🤖] Once the doubling wait gets longer than the session's remaining lifetime, the realm-server service holds an expired session for most of each cycle, and its callers get 401s where they used to get a fresh session. Take a five-minute server lifetime. Every fresh mint lands at the refresh point with about 300 s before exp. The waits go 30, 60, 120, 240 s, and the session is still valid through those. The next wait is 480 s, so the session is expired for its last 180 s. At the 30-minute cap it is expired for about 25 of every 30 minutes.

During that window RealmServerService.getToken checks only that a token exists, so authedFetch sends the expired token. That covers billing, _github-pr, the info/types/counts fetches and check-correctness. authedFetch has no 401 → re-mint path like the one per-realm fetches get through RealmService's reauthentication. Only the federated endpoints recover, through reauthenticateRealmServer.

Can the wait be capped at the time left before exp whenever that time is at least minTokenRefreshIntervalSec? A server with short-lived sessions would then cost one mint per session lifetime and the session would stay valid. When clock skew makes exp look already past, the backoff would still grow. Don't clamp to max(timeToExp, floor): that pins the skew case at 30 s forever. If the expired windows are the intended price, a sentence here saying so would stop the next reader from "fixing" it.

Class: a regression only in the short-lifetime case, because the loop kept the session valid, at the cost this change removes. This is a decision for you, not blocking.

[habdelra]

[Claude Code 🤖] Fixed in 9675dae. After an unusable mint, the doubling wait is now capped at the session's remaining lifetime whenever at least minTokenRefreshIntervalSec of it is left. A server whose sessions are shorter than the refresh period therefore gets one mint per session, and the session doesn't sit expired. A session that is already expired, or about to expire, when it arrives (the clock-skew case) still backs off from 30 s doubling to the cap, so a bad clock can't drive a mint every 30 s. New test: the wait after unusable mints never runs past the session's expiry (waits 30/60/120/240 s, then about 300 s each).

[habdelra]

[Claude Code 🤖] RealmServerService.logout resets its schedule, but nothing resets this one, so the two refreshers already handle an account switch differently. RealmResources outlive RealmService.logout: it calls realm.logout() on each one and keeps them in the map. So a run of unusable mints under one account carries over to the next, and that account's first short-lived session can wait up to maxTokenRefreshBackoffSec instead of 30 s.

Fix: call this.refreshSchedule.reset() in RealmResource.logout(). Don't put it in clearRealmSession(). RealmService's 401 reauthentication calls that method, and resetting there would restart the backoff on every rejected session.

Class: pre-existing, since the fields had no reset before they moved here. Not blocking, but it's a one-liner now that reset() exists.

[habdelra]

[Claude Code 🤖] Fixed in 9675dae. RealmResource.logout() now calls refreshSchedule.reset(). As you suggested, the reset is in logout and not in clearRealmSession, so re-authentication after a 401 keeps its backoff. There's a code comment saying so. Covered by Unit | Service | realm | token refresh: logging out starts the next session without the previous one's backoff.

[habdelra]

[Claude Code 🤖] Removing this line wouldn't fail any test. A third case in realm-server-token-refresh-test.ts would pin it: two unusable mints, then logout(), then setClient + login(), and expect the next wait to be minTokenRefreshIntervalSec * 1000, not 120 s. Not blocking.

[habdelra]

[Claude Code 🤖] Added in 9675dae: logging out starts the next session without the previous one's backoff. Two unusable mints (30 s, then 60 s), then logout() and a fresh login; the next wait is back to 30 s. Removing the reset() call fails it, and the same is true of the per-realm twin.

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ±0      1 suites  ±0   2h 18m 14s ⏱️ + 18m 38s
5 932 tests ±0  5 921 ✅ ±0  11 💤 ±0  0 ❌ ±0 
5 947 runs  ±0  5 936 ✅ ±0  11 💤 ±0  0 ❌ ±0 

Results for commit 37504f3. ± Comparison against earlier commit 0b4075f.

Realm Server Test Results

    1 files  ±0    364 suites  ±0   2h 7m 4s ⏱️ + 6m 18s
6 040 tests ±0  6 040 ✅ ±0  0 💤 ±0  0 ❌ ±0 
6 103 runs  ±0  6 103 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 37504f3. ± Comparison against earlier commit 0b4075f.