fix(auth): coordinate token refresh across pages - #3377
Conversation
|
@xgopilot /review |
Review completePosted a non-blocking review with 6 inline comments and a summary: pullrequestreview-4824626550. Highlights
All findings are non-blocking suggestions. |
There was a problem hiding this comment.
Code Review: coordinate token refresh across pages
The core approach is sound — serializing refresh via navigator.locks and re-reading shared credentials inside the lock (restoreUserState() before re-checking isAccessTokenValid()) is exactly the right pattern so a waiting page reuses a token another page just refreshed. Capturing refreshTokenBeforeRefresh before the async call and the double-check inside the lock are nicely done, and the test confirms a waiting page reuses another page's fresh token without calling refreshToken.
A few points worth considering before merge (all non-blocking):
Context (pre-existing, not introduced here): Persisting the full auth state — including the long-lived refreshToken — to localStorage, and adopting it via unvalidated Object.assign(userState, JSON.parse(stored)), already exists on dev. This PR does not newly introduce token persistence. What is new is that the storage listener now makes another tab's localStorage contents adopted live at runtime, which widens that unvalidated-adoption surface (see inline note).
Tests: The concurrency test stubs navigator.locks.request and never exercises two overlapping ensureAccessToken() calls contending for a real lock, so serialization / single-flight is asserted only indirectly. There is also no test for the watchEffect ↔ storage-listener interaction (the highest-risk new behavior). Consider adding a contention test (two overlapping calls, refreshToken called once) and a test that an incoming storage update does not trigger a redundant echo write.
See inline comments for the specific spots.
Closes #3376
Changes
Verification
pnpm exec vitest --run src/stores/user/signed-in.test.tspnpm run type-checkpnpm run lint -- src/stores/user/signed-in.ts src/stores/user/signed-in.test.ts