fix(webui): stop realtime socket reconnect and refresh storm on dead session - #4178
Open
ajdevy wants to merge 3 commits into
Open
fix(webui): stop realtime socket reconnect and refresh storm on dead session#4178ajdevy wants to merge 3 commits into
ajdevy wants to merge 3 commits into
Conversation
…session iOfficeAI#4156 fixed reconnect scheduling on the WebUI bridge socket. The realtime stream in httpBridge.ts is a second, independent WebSocket with its own scheduler, and it still carried all three of those defects plus an unlatched refresh loop — reproducing iOfficeAI#4155's symptom on the socket iOfficeAI#4156 did not touch. - ensureWs() dialled on every wsSend() and every wsEmitter().on(), ignoring the pending wsReconnectTimer, so the reconnect rate tracked how busy the app was rather than the 1s->30s backoff. - The backoff reset in the `open` handler, so a backend that accepts the upgrade and drops it immediately (iOfficeAI#4155's `path=/ws status=101` once a second) kept the delay pinned at 1s forever. - handleWsAuthClose() set no stop flag on a failed refresh, so the next send re-dialled with the same dead cookie and fired another /api/auth/refresh. refreshSession()'s single-flight only collapses concurrent callers, so sequential 401s each spent a fresh POST on a rate-limited endpoint. - refreshSession() answered `false` for a dead refresh token, a 503 and a dropped connection alike, so browser.ts signed the user out over a blip. sessionRefresh.ts now reports why a refresh ended: 401/403 latches as `expired` (answered without a request until resetSessionRefresh()), while a network error / 429 / 5xx returns `unavailable` on a 1s->30s cooldown. refreshSession() keeps its boolean contract for existing callers. ensureWs() honours the latch and the pending timer; the backoff is credited on close, only for a connection that held >= 5s. The bridge socket now runs the same refresh-or-stop recovery on a 1008 close as on a realtime.error frame, retries only the refresh when the failure was inconclusive, and redirects to /login only on `expired`. Login clears the latch and resumes both sockets. Related to iOfficeAI#4155
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
iOfficeAI#4155 is a scenario that takes a day to reach, so it was never covered by a test. This adds one, with the clock fast-forwarded. Everything below the test is real: the shipped adapters run unmodified, over real TCP, through the real startStaticServer including its /api/* proxy and its /ws splice. Only aioncore is a stub, and only so its clock can be moved -- it mints the production lifetimes (24h access, 30d refresh/pairing) and judges them against a virtual clock, so core.advance(ACCESS_TTL_MS) is genuinely "the next day" for every token in the system at no wall-clock cost. The cookie jar expires nothing on purpose. A paired browser keeps holding its session cookie for 30 days while the JWT inside it dies after 24 hours, and keeps sending it; that mismatch is the bug, so the harness reproduces it rather than letting the browser drop the cookie. Each test pairs, jumps a day, then opens the WebUI once, and asserts one of the two outcomes the issue named as acceptable: a renewable session is renewed silently, a paired one gives up once and lands on the pairing screen. "Does not loop" is measured server-side in request counts. Driving the client for ~1.6s at T+24h against a paired browser, before this branch vs after: POST /api/auth/refresh 84 -> 1 /ws upgrades 43 -> 1 3 of the 6 tests fail against the pre-fix adapters. The web host is a parameter: AIONUI_WEBHOST_UNDER_TEST points the identical scenarios at a build carrying the real pairing layer, which is what the open-source static server cannot cover. Related to iOfficeAI#4155
ajdevy
force-pushed
the
fix/webui-realtime-session-storm
branch
from
August 25, 2026 10:59
bb74106 to
550a58e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
#4156 fixed reconnect scheduling on the WebUI bridge socket (
browser.ts). The WebUI has a second WebSocket — the realtime stream inhttpBridge.ts— with its own independent scheduler, and it still carries all three of the defects #4156 fixed, plus an unlatched refresh loop that arrived with the refresh path in #4175.Together they reproduce #4155's symptom on the socket #4156 didn't touch: point the WebUI at a backend that refuses its session and it dials
/wsand POSTs/api/auth/refreshcontinuously, forever.The defects
1.
ensureWs()bypasses the backoff (httpBridge.ts). It dials whenever the socket isn'tOPEN/CONNECTING, with a pendingwsReconnectTimersitting unused. It is called fromwsSend()and from everywsEmitter().on()subscription, so the reconnect rate tracks how busy the app is rather than the 1 s→30 s schedule. This is exactly theensureSocket()defect from #4156 — and it bites harder here, because #4155's failure mode is a full SWR revalidation on every reconnect, which re-subscribes every realtime consumer.2. The backoff resets on
open(httpBridge.ts).wsReconnectAttempt = 0ran in theopenhandler. A backend that completes the handshake and then drops the connection still firesopen, so the delay snapped back to 1 s on every attempt. That accept-then-drop shape is precisely what #4155's log shows —path=/ws status=101about once a second.3. A dead session never latches (
httpBridge.ts+sessionRefresh.ts).handleWsAuthClose()returned silently when the refresh failed, setting no stop flag, so the nextwsSend()re-dialled with the same dead cookie, earned another 1008, and fired another refresh POST.refreshSession()couldn't stop it either: its single-flight only collapses concurrent callers, so sequential 401s each spawn a fresh POST against a rate-limited endpoint. A paired browser that holds an access cookie but no refresh cookie hits this on every single 401.4. "Refresh failed" and "session is dead" were the same answer (
sessionRefresh.ts).refreshSession()returnedfalsefor a401on the refresh token, for a503, and for dropped Wi-Fi alike.browser.tsreads thatfalseas "sign the user out", so one transient blip kicks a perfectly refreshable session to/login— and any latch built on that boolean would strand it there.Changes
sessionRefresh.ts— teach the primitive why a refresh ended:refreshSessionOutcome(): Promise<'refreshed' | 'expired' | 'unavailable'>.refreshSession()keeps its exact boolean contract (=== 'refreshed'), so existing callers are untouched.401/403→expired, and it latches: later calls answer from the latch with no request at all.isSessionExpired()exposes it so reconnect loops can stop dialling.429/5xx→unavailablewith a 1 s→30 s cooldown, so an inconclusive failure is retried rather than treated as a logout.resetSessionRefresh()clears both; called on successful re-auth.httpBridge.ts— the realtime socket:ensureWs()returns early when the session has latched as expired or a reconnect is already queued.wsConnectedAt+WS_STABLE_CONNECTION_MS(5 s): the backoff is credited onclose, only for a connection that actually held — not onopen.handleWsAuthClose()branches on the outcome: reconnect onrefreshed, stay down onexpired(the latch makesensureWs()a no-op — no dial, no further POST), back off onunavailable.resumeRealtime()so a successful login resumes the stream immediately instead of waiting out a pending backoff.browser.ts— the bridge socket:unavailablerefresh now retries the refresh, on its own backoff — the socket stays down. Re-dialling with a cookie the backend just rejected only earns the same close, so fix(webui): stop bridge websocket reconnect storm on repeated failures #4156's "a terminal auth error stops reconnection" invariant holds unchanged.redirectToLogin()fires only onexpired.BASE_RECONNECT_DELAYwhere feat(auth): silently refresh WebUI session on 401 #4175 had re-introduced a literal500.Both sockets: a successful refresh now reconnects on the backoff rather than dialling immediately, and doesn't reset the delay. Otherwise a backend that keeps refusing even a freshly minted session becomes a new unthrottled
refresh → dial → close → refreshloop. The delay is credited back oncloseonce a connection has actually held, so a healthy recovery still returns to the floor.AuthContext.tsx— clear the previous session's latch on successful login, then resume both sockets.Related Issues
Continues the in-repo half of #4155. It does not close it: the primary defect there — a paired browser's Core session minted once with no renewal,
/auth/statusnot reflecting Core-session validity, and the proxy forwarding the browser's stale cookie — lives in the packaged web-host.packages/web-host/src/static-server.tsis still 285 lines with no auth, anddesktopPairing,createPairedSessionCookies,coreUserBridge,AuthSessionStore,aionui-webhost-sessionandscheduleSessionRenewalstill have zero hits in this repository.What this PR does do is make the client side safe to wire that flow onto. Once a paired browser holds a refresh cookie,
refreshSessionOutcome()returnsrefreshedand both sockets recover silently; until then it returnsexpiredonce and everything goes quiet, instead of looping.Type of Change
fix— Bug fix (non-breaking change which fixes an issue)feat— New feature (non-breaking change which adds functionality)perf— Performance improvementrefactor— Code restructuring (no behavior change)docs— Documentation updateAtomic PR Checklist (Rule 1)
<type>(<scope>): <subject>(English)One root cause — a session the backend refuses produces an unbounded reconnect + refresh loop — on one code path: realtime reconnect scheduling and the session-refresh primitive that drives it. Splitting leaves the storm in place. Guarding
ensureWs()alone still lets an accept-then-drop backend hammer at the 1 s floor; fixing only the backoff still letswsSend()skip the timer; and fixing both still POSTs/api/auth/refreshon every close, because without the outcome split there is nothing to latch on. Theunavailablebranch is not a separate feature either — it is what makes the latch safe to add at all.Local Checks (Rule 3)
bun run format— formatting passes (format:checkclean)bun run lint— 0 errors, no new warningsbunx tsc --noEmit— no type errorsbunx vitest run— 4965 passed, 5 skipped, 0 failed (520 files)bun run i18n:typesreports types up to date andnode scripts/check-i18n.jspasses; nolocales/or i18n-config changesRuntime Verification
Found from the #4155 reproduction: a phone paired to the desktop WebUI over LAN, against a backend rejecting the browser's expired session. Checks and tests were run on Ubuntu 24.04 / Node 22.14 / bun 1.4.
End to end: the 24-hour scenario, fast-forwarded
tests/integration/webuiSessionLifetime.test.ts(6 tests) reproduces the issue as reported rather than by proxy. Everything below the test is real — the shipped adapters run unmodified, over real TCP, through the realstartStaticServerincluding its/api/*proxy and its/wssplice. Only aioncore is a stub, and only so its clock can be moved: it mints the production lifetimes (24 h access, 30 d refresh/pairing) and judges them against a virtual clock, socore.advance(ACCESS_TTL_MS)is genuinely "the next day" for every token in the system and costs no wall-clock time.Each test pairs, jumps a day, and opens the WebUI once — the phone being picked up the next morning. The two outcomes the issue named as acceptable are the two asserted:
"Does not loop" is measured server-side in request counts, not inferred from client state. Driving the client for ~1.6 s at T+24 h, against a paired browser:
POST /api/auth/refresh/wsupgradesThat is ~52 refresh POSTs and ~27 upgrades per second against a rate-limited endpoint — #4155's "that full burst repeats every ~1 s", quantified. After the fix one socket's refresh latches the session and the other never gets as far as dialling.
The harness takes the web host as a parameter: set
AIONUI_WEBHOST_UNDER_TESTto a module exportingstartStaticServerto run the identical scenarios against a build that has the real pairing layer. The open-source static server is the default, with the pairing cookie shapes reproduced by hand.Unit coverage
New test file
tests/unit/adapter/realtimeSocketReconnect.dom.test.ts(8 tests) drives the realhttpBridgemodule in jsdom with fake timers and a scriptedWebSocketstub, plus 3 new tests inwebuiWebSocketReconnect.dom.test.ts, 1 inbrowserRealtimeError.test.tsand 5 insessionRefresh.dom.test.ts.Against the unpatched adapter, 8 of the 26 tests in the three socket test files fail — so they pin the regressions rather than just passing:
The 5 new
sessionRefreshtests cover APIs this PR adds, so they cannot be run against the unpatched module. The integration suite uses no new API and fails 3 of 6 unpatched.Notes on the existing tests that changed
No existing assertion was weakened, and one existing invariant deliberately shaped the design:
browserRealtimeError.test.ts— "does not redirect to login from close code 1008 without an auth error event" is unchanged and still passes. I had initially made a bare1008close on the bridge socket run the auth recovery; that test says 1008 is a generic policy violation and therealtime.errorframe is this socket's designated auth signal, so I dropped it. (The realtime socket keeps the1008handling feat(auth): silently refresh WebUI session on 401 #4175 gave it — that one has no such frame.)browserRealtimeError.test.ts— "…redirects to login when refresh fails" now stubsdocumentand a401refresh response. Its own comment said it was relying ondocumentbeing absent in the node env sorefreshSession()would short-circuit, i.e. it never exercised a refresh response at all. It now drives a genuine dead-refresh 401, and a sibling case asserts a503does not redirect. The redirect assertion itself is unchanged.sessionRefresh.dom.test.tsandwebuiWebSocketReconnect.dom.test.tsgained aresetSessionRefresh()inbeforeEach/afterEach. The latch and the cooldown are module-level state, so without it one test's verdict leaks into the next.Additional Context
Realtime socket behaviour for a connection that keeps failing:
/login