Repository navigation
fix(desktop): prevent sidebar localStorage state loss and token-swap remounts - #6089
Conversation
All six localStorage-backed collections now hydrate at construction and never GC (startSync + gcTime: 0), making the idle/cleaned-up states unrepresentable: an insert into a not-yet-hydrated collection rewrote the whole storage key from empty memory, erasing every persisted row. Reads are per-row tolerant so one malformed entry no longer blanks the entire store via the library's hydration catch-all. The quota guard, read heal, and lifecycle options fold into one hardenLocalCollection wrapper. Also removes the manufactured signed-out window in AuthProvider token rotation: the token now swaps atomically instead of null -> signOut -> set, which let concurrent get-session reads observe "no session" and unmount the entire authenticated tree.
📝 WalkthroughWalkthroughThe change updates authenticated token replacement and hardens localStorage collection handling. Local collection hydration now removes invalid rows and isolates healing failures. Collection lifecycle tests cover synchronization behavior and corrupt stored entries. ChangesAuthentication token refresh
Local collection hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant localStorage
participant hardenLocalCollection
participant withReadHeal
participant LocalCollection
participant QuotaNotification
localStorage->>hardenLocalCollection: Load persisted rows
hardenLocalCollection->>withReadHeal: Validate and heal rows
withReadHeal-->>hardenLocalCollection: Return valid rows
hardenLocalCollection->>LocalCollection: Start synchronization
LocalCollection->>localStorage: Persist updates
hardenLocalCollection->>QuotaNotification: Report quota failure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx">
<violation number="1" location="apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx:82">
P2: The atomic swap now sets a fresh authToken while immediately nulling the JWT (`setJwt(null)`) and then awaiting `refetchSession()` to re-mint a new JWT via the 401-refresh path. This new null-the-JWT-then-refetch sequence has two unguarded edges: (1) if two `onTokenChanged` events arrive in quick succession, the overlapping async `refetchSession()` calls are not serialized/versioned, so a stale refresh result can apply to the newer swap — the same stale-refresh-overwrites-newer-state race that a generation guard is meant to prevent; and (2) if `refetchSession()` rejects, the state is left authenticated (`setAuthToken(data.token)` succeeded) but with `jwt === null` and no restore path, i.e. a session credential was permanently cleared. Consider versioning the swap (a generation counter captured around the swap, and only applying the refetch result if the generation is still current), and ensure the failure path restores a usable JWT or treats the null-JWT state as signed-out rather than leaving an inconsistent authenticated-without-credential state.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Sidebar as Sidebar UI
participant Coll as OrgCollections
participant Harden as hardenLocalCollection
participant WithRead as withReadHeal
participant Quota as withQuotaGuard
participant LS as localStorage
participant Auth as AuthProvider
participant Token as electronTrpc.auth.onTokenChanged
participant Session as refetchSession
Note over Sidebar,LS: Collection Initialization (constructor time)
Coll->>Harden: createOrgCollections() for orgId
Harden->>WithRead: wrap with startSync:true, gcTime:0
WithRead->>Quota: wrap with reclaim & notify
Quota->>LS: sync start → hydrate from storageKey
LS-->>Quota: raw JSON string
Quota-->>WithRead: parse via healingParser
WithRead->>WithRead: iterate entries per-row
alt Entry is envelope with versionKey & data
WithRead->>WithRead: apply heal(entry.data)
alt heal throws
WithRead->>WithRead: drop entry, console.warn
else heal succeeds
WithRead->>WithRead: keep entry with healed data
end
else Entry is non-envelope (malformed)
WithRead->>WithRead: drop entry, console.warn
end
WithRead-->>LS: hydrated collection in memory
Note over Coll,LS: Collection now ready – idle state unreachable
Note over Sidebar,Session: Runtime Sidebar Mutation (e.g. insert-on-enter)
Sidebar->>Coll: insert({...}) / update(...)
Coll->>Coll: read .state (non-reactive, guaranteed hydrated)
Coll->>LS: persist entire store
alt quota exceeded
LS-->>Coll: QuotaExceededError
Coll->>Quota: reclaimTerminalStateForQuota()
Quota-->>LS: retry write
alt retry fails
Quota->>Quota: notifyQuotaExhausted(storageKey)
Quota-->>Coll: drop write silently
end
else write succeeds
LS-->>Coll: OK
end
Coll-->>Sidebar: mutation result
Note over Auth,Session: Token Swap (no sign-out gap)
Token->>Auth: onTokenChanged({ token, expiresAt })
Auth->>Auth: setAuthToken(data.token)
Auth->>Auth: setJwt(null) (stale token cleared)
Auth->>Session: refetchSession()
alt session fetch succeeds
Session-->>Auth: new session
Auth-->>Token: ready
else session fetch fails
Auth->>Auth: stale JWT re-mints on next 401
Auth-->>Token: fallback state
end
Note over Auth,Session: No signed-out window → sidebar stays mounted
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // concurrent get-session read "no session" and unmount the whole | ||
| // authenticated tree. The stale JWT re-mints on the next 401. | ||
| setAuthToken(data.token); | ||
| setJwt(null); |
There was a problem hiding this comment.
P2: The atomic swap now sets a fresh authToken while immediately nulling the JWT (setJwt(null)) and then awaiting refetchSession() to re-mint a new JWT via the 401-refresh path. This new null-the-JWT-then-refetch sequence has two unguarded edges: (1) if two onTokenChanged events arrive in quick succession, the overlapping async refetchSession() calls are not serialized/versioned, so a stale refresh result can apply to the newer swap — the same stale-refresh-overwrites-newer-state race that a generation guard is meant to prevent; and (2) if refetchSession() rejects, the state is left authenticated (setAuthToken(data.token) succeeded) but with jwt === null and no restore path, i.e. a session credential was permanently cleared. Consider versioning the swap (a generation counter captured around the swap, and only applying the refetch result if the generation is still current), and ensure the failure path restores a usable JWT or treats the null-JWT state as signed-out rather than leaving an inconsistent authenticated-without-credential state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx, line 82:
<comment>The atomic swap now sets a fresh authToken while immediately nulling the JWT (`setJwt(null)`) and then awaiting `refetchSession()` to re-mint a new JWT via the 401-refresh path. This new null-the-JWT-then-refetch sequence has two unguarded edges: (1) if two `onTokenChanged` events arrive in quick succession, the overlapping async `refetchSession()` calls are not serialized/versioned, so a stale refresh result can apply to the newer swap — the same stale-refresh-overwrites-newer-state race that a generation guard is meant to prevent; and (2) if `refetchSession()` rejects, the state is left authenticated (`setAuthToken(data.token)` succeeded) but with `jwt === null` and no restore path, i.e. a session credential was permanently cleared. Consider versioning the swap (a generation counter captured around the swap, and only applying the refetch result if the generation is still current), and ensure the failure path restores a usable JWT or treats the null-JWT state as signed-out rather than leaving an inconsistent authenticated-without-credential state.</comment>
<file context>
@@ -75,9 +75,11 @@ export function AuthProvider({ children }: { children: ReactNode }) {
+ // concurrent get-session read "no session" and unmount the whole
+ // authenticated tree. The stale JWT re-mints on the next 401.
setAuthToken(data.token);
+ setJwt(null);
try {
await refetchSession();
</file context>
There was a problem hiding this comment.
Assessed as non-blocking, leaving as-is:
(1) Overlapping onTokenChanged events: better-auth's session atom aborts an in-flight get-session when a new fetch starts, and every request reads the bearer via getAuthToken() at send time — so the last swap's refetch always resolves against the newest token. Rotation events are also rare (OAuth deep-link saves only; the subscription does not fire on boot or refresh).
(2) jwt === null after a failed refetch is the designed lazy state, not a stranded one: Electric's 401 handler (refreshJwtAfterUnauthorized) and the 50-minute interval in this same provider both re-mint it, and ensureFreshJwt fetches on demand. The bearer token — the actual session credential — is never cleared on this path. The pre-fix code left the stale JWT in place, which was strictly worse.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
Why / Context
Investigation into the dashboard sidebar "randomly losing state" found two independent failure classes in
@tanstack/db0.6.14's localStorage collections (verified against the shipped library source):.state/.get()on a never-synced collection silently return empty, andinsert()doesn't auto-start sync foridlecollections. Every mutation re-serializes the entire store from memory, so an insert before hydration rewrotev2-workspace-local-state-<orgId>(etc.) from near-empty memory — permanently erasing all placement rows, pins, sections, and pane layouts for the org. The default 5-minute GC reintroduces the same hazard after subscribers unmount.Separately,
AuthProvider.onTokenChangeddidsetAuthToken(null)→ awaitedsignOut()→setAuthToken(new). Anyget-sessionfired in that gap (window focus, network reconnect) read "no session", which redirected to/sign-inand unmounted the whole dashboard — dropping all ephemeral UI state until the follow-up refetch remounted it.How It Works
hardenLocalCollection(collections.ts) folds the existing quota guard and read heal together withstartSync: true+gcTime: 0, applied uniformly to all six localStorage collections. Collections hydrate synchronously at construction and are never GC'd, making theidle/cleaned-upstates unrepresentable — the non-reactive.statereads in the sidebar mutation helpers become unconditionally correct with no per-call-site readiness gates.withReadHealnow parses per-row: non-envelope entries and heal throws drop that row (with a console warning) instead of escaping to the library's whole-store catch-all.healdefaults to identity so every collection gets the tolerance, not just the two with custom heals.onTokenChangedswaps the token atomically and refetches the session; the stale-identity JWT is cleared and re-mints via the existing 401-refresh path.Manual QA / CDP Verification
Driven end-to-end over CDP against this worktree's dev app (renderer on port 7965, signed-in session + active org verified first):
v2-workspace-local-state-<org>(21 entries on disk), froze the dying page's writes, hard-reloaded ignoring cache. The renderer logged[withReadHeal] Dropping malformed stored entry "s:__corrupt__"at hydration, the sidebar rendered all 3 projects and every workspace row, and the on-disk store diffed byte-identical to the pre-test backup (20/20 rows: pane layouts, tabOrder, hidden flags, viewedFiles all intact). Screenshot captured.@tanstack/dbbuild in-page (createCollection+localStorageCollectionOptionsover a store with one corrupt + one valid entry) hydrates zero rows — the whole-store blanking is real in the shipped runtime, not just under bun test.localCollectionLifecycle.test.tsagainst the real library) and the token-rotation swap (requires a live OAuth deep-link; covered by code-path analysis in the description).Testing
bun test src/renderer/routes/_authenticated/providers/CollectionsProvider/— 48 pass, including:localCollectionLifecycle.test.ts: insert on a freshstartSync: truecollection preserves existing storage rows; companion test pins the library's wipe behavior without it (if that ever changes, the hardening can be revisited)withReadHealcases: corrupt entry dropped per-row while sibling rows hydrate; heal throw drops only its own rowbun run lint(exit 0)bun run typecheck(desktop)Design Decisions
.insert()call site (there are several ungated ones, e.g. the ensure-on-visit effect inv2-workspace/layout.tsx), the collection lifecycle is fixed so the dangerous states cannot occur. Net complexity goes down — one wrapper instead of three at each call site.gcTime: 0: these stores are a few KB of localStorage; keeping them live forever is cheaper than the correctness risk of re-hydration windows.Known Limitations / Follow-ups
signOut()round trip means a previous user's server-side session isn't revoked on deep-link account switch; identity is governed by the bearer token, so this is cosmetic.activeOrganizationId = nullwhen a membership read transiently returns empty (packages/auth/src/lib/resolve-session-organization-state.ts) — deserves its own PR; and the library's cross-tab storage-event sync is inert for these collections (the storage wrapper breaks itsstorageAreaidentity check) plus leaks its storage listener across GC cycles — upstream issues, latent while the app has a single window.Risks / Rollout
startSync: truemoves hydration intocreateOrgCollections(render path). Synchronous localStorage reads of small stores; no measurable startup cost expected.Summary by cubic
Prevents sidebar state loss and mid-session unmounts by hardening all localStorage-backed sidebar collections and making auth token swaps atomic. Persisted rows are now safe, and token rotations no longer trigger sign-out flashes.
localStoragecollections now hydrate on construct and never GC (startSync: true,gcTime: 0). Per-row tolerant parsing viawithReadHealdrops bad entries instead of blanking stores; applied via ahardenLocalCollectionwrapper that also keeps the quota guard. This prevents insert-on-idle wipes and corrupt-entry blanking in@tanstack/react-db.AuthProvidernow swaps tokens atomically and clears the stale JWT before refetch, removing the brief signed-out window that redirected to/sign-inand unmounted the sidebar.Written for commit 24af1b1. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests