Repository navigation
Refresh session state after router form mutations so logout updates the nav - #776
Conversation
|
No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (4)
π WalkthroughWalkthroughRouter mutations now notify the app, allowing subsequent navigation-triggered session refreshes to bypass throttling. Authentication documentation and the smoke test are updated to describe and verify logout behavior. ChangesSession refresh synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RouterForm
participant routerEvents
participant App
participant SessionRefresh
RouterForm->>routerEvents: Dispatch mutation after form response
routerEvents->>App: Mark sessionMaybeStale
App->>SessionRefresh: Refresh on subsequent navigation without throttle
π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 warning)
β Passed checks (4 passed)
β¨ Finishing Touchesπ Generate docstrings
π§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
π Preview deployed: https://kody-pr-776.kody-a99.workers.dev Worker: Mocks:
|
Problem
Logging out left the top nav showing the logged-in state (username, Admin link, Log out button) on the
/loginpage.Root cause
Not the logout handler β the server destroys the cookie correctly. The gap is in the client router + shell session cache interaction:
packages/worker/client/client-router.tsx) intercepts every form POST (including the nav's/logoutform), follows the redirect, and SPA-navigates to/loginwithout a document reload.packages/worker/client/app.tsx) renders the nav from an in-memory session refreshed on navigation events β but navigation-triggered refreshes were throttled to 30s (PR Performance: package app loading, MCP execute isolate reuse, and hot-path cachingΒ #605). The comment claimed logout bypassed the throttle viasetSessionRefreshHandler, but nothing ever wired that up./sessionfetch (which hydration always performs) SPA-navigated with the refresh skipped, leaving stale logged-in nav.This was fundamental to the router, not specific to logout: the router had no concept of mutations, so listeners caching server-derived state could never know a form POST invalidated it.
Fix
mutationevent onrouterEventsafter every form POST it submits, exposed aslistenToRouterMutationsalongsidelistenToRouterNavigation./sessionregardless of the throttle. (The refresh must not start in the mutation listener itself: the redirect navigation re-renders the shell, which aborts in-flightqueueTaskwork and would drop the refresh β this is why the first-cut "just callqueueSessionRefresh()" approach failed in testing.)app.tsxand the client-session-refresh section ofdocs/contributing/architecture/authentication.md.Testing
main(stale nav reproduced) and pass with this fix.npm run validategreen: format, lint, typecheck, 861 unit tests, 15 Playwright E2E, MCP E2E.npm run devwith the seeded admin account:System recap β extends a primitive (medium risk)
Mode: recap Β· Base:
main@82af4acdΒ· Head:cursor/fix-stale-nav-after-logout-34e9Classification: extends β the browser app's client router gains a mutation event and the shell's session refresh contract changes; no primitives added.
Primitives touched
app-uimutationevents; shell session refresh bypasses its throttle after mutationsapp-sessions/logoutand/sessionhandlers unchanged; the client now revalidates against them correctlySystem map
Logout flows from the nav form through the client router's new mutation event into an unthrottled
/sessionrevalidation.Legend: green = composes (wiring only) Β· amber = extended by this PR Β· red = new primitive Β· gray = context (unchanged, included only when an edge crosses it).
Change flow
Summary by CodeRabbit
Bug Fixes
Tests