fix(client): persist resume token in localStorage so it survives browser restart (#123) #131

Merged
bosun merged 1 commit from i/123-localstorage-resume-token into main 2026-06-23 18:54:08 +02:00
Owner

What

Operator playtest (post round-22): on mobile, quitting the browser entirely and reopening failed to resume the in-flight versus match. Wave-0 probe traced the root cause to the resume token living in sessionStorage — which the browser clears when the origin's last tab closes. A page reload keeps sessionStorage, but a full browser quit→reopen does not, so the token was gone before the client could resume (within the server's 25s grace).

Operator-ratified fix (A): store the token in localStorage, which persists across a full browser restart. Swaps the five token call sites in net.ts (getItem/setItem/removeItem). No change to the resume code path — same LS_TOKEN key, same onopen→resume-or-join logic; only the backing store changes.

Self-healing edges (unchanged paths, now reachable)

  • Within grace → reopened client reads the persisted token → sends {type:'resume',token} → server resumes the match.
  • Beyond grace / stale / cross-tab token → server rejects the resume → the existing error-case (net.ts) clears the token and falls back to a fresh join. A dead token never wedges the client.
  • localStorage is per-origin-per-device, so the operator's desktop + mobile don't share a token; the double-attach guard + error-fallback cover same-device multi-tab.

Substrate-of-record: the misleading name

The const was already named LS_TOKEN — the "LS" implies localStorage — while the implementation used sessionStorage. That naming/impl mismatch is precisely what made the bug easy to miss on a read: the name asserted the persistence the code didn't have. The swap makes the name honest. (Banked instance of misleading-naming-creates-inheritance-hazards-downstream.) A code comment at the const now documents the persistence semantics + the grace/self-healing contract so the next reader doesn't re-introduce the mismatch.

Verification

  • Discriminating regression-pin added to the #92 WS-mock substrate (versus.spec.ts): seeds a token in localStorage, connects, asserts the client's first frame is {type:'resume',token:'tok-123'} not {type:'join'}. This runs the real net.ts onopen path in a real browser page.
    • Mutation-proven: reverting the read to sessionStorage.getItem makes the seeded localStorage token invisible → client sends joinsawType('resume') stays false → test reds (verified locally, then restored by re-edit).
  • Full suite green: 69/69 playwright, npx tsc --noEmit clean.

Scope boundary — what this PR does NOT do

  • The operator's desktop-side ~30s freeze is expected grace-window behavior (server holds the match ~25s before crediting the survivor), not a bug — confirmed in the #123 probe. The survivor-side overlay UX during that wait is the #93-adjacent concern tracked by #115 (reconnect-via-resume AC3), not this PR.
  • Auto-resume on page-load without re-navigating (so the user doesn't re-click VERSUS to trigger the resume) is a richer UX that also belongs to #115. This PR makes the token survive; #115 owns when the client auto-attempts the resume.

Closes #123

🤖 Generated with Claude Code

## What Operator playtest (post round-22): on mobile, **quitting the browser entirely and reopening failed to resume** the in-flight versus match. Wave-0 probe traced the root cause to the resume token living in `sessionStorage` — which the browser clears when the origin's last tab closes. A page *reload* keeps sessionStorage, but a full browser *quit→reopen* does not, so the token was gone before the client could resume (within the server's 25s grace). Operator-ratified fix **(A)**: store the token in `localStorage`, which persists across a full browser restart. Swaps the five token call sites in `net.ts` (`getItem`/`setItem`/`removeItem`). No change to the resume *code path* — same `LS_TOKEN` key, same onopen→`resume`-or-`join` logic; only the backing store changes. ## Self-healing edges (unchanged paths, now reachable) - **Within grace** → reopened client reads the persisted token → sends `{type:'resume',token}` → server resumes the match. - **Beyond grace / stale / cross-tab token** → server rejects the `resume` → the existing `error`-case (net.ts) clears the token and falls back to a fresh `join`. A dead token never wedges the client. - localStorage is per-origin-per-device, so the operator's desktop + mobile don't share a token; the double-attach guard + error-fallback cover same-device multi-tab. ## Substrate-of-record: the misleading name The const was already named **`LS_TOKEN`** — the "LS" implies *localStorage* — while the implementation used `sessionStorage`. That naming/impl mismatch is precisely what made the bug easy to miss on a read: the name *asserted* the persistence the code didn't have. The swap makes the name honest. (Banked instance of *misleading-naming-creates-inheritance-hazards-downstream*.) A code comment at the const now documents the persistence semantics + the grace/self-healing contract so the next reader doesn't re-introduce the mismatch. ## Verification - **Discriminating regression-pin** added to the #92 WS-mock substrate (`versus.spec.ts`): seeds a token in `localStorage`, connects, asserts the client's first frame is `{type:'resume',token:'tok-123'}` **not** `{type:'join'}`. This runs the **real** `net.ts` onopen path in a real browser page. - **Mutation-proven**: reverting the read to `sessionStorage.getItem` makes the seeded localStorage token invisible → client sends `join` → `sawType('resume')` stays false → test reds (verified locally, then restored by re-edit). - Full suite green: **69/69** playwright, `npx tsc --noEmit` clean. ## Scope boundary — what this PR does NOT do - The operator's *desktop-side ~30s freeze* is **expected grace-window behavior** (server holds the match ~25s before crediting the survivor), not a bug — confirmed in the #123 probe. The *survivor-side overlay UX during that wait* is the #93-adjacent concern tracked by **#115** (reconnect-via-resume AC3), not this PR. - **Auto-resume on page-load without re-navigating** (so the user doesn't re-click VERSUS to trigger the resume) is a richer UX that also belongs to **#115**. This PR makes the token *survive*; #115 owns *when the client auto-attempts the resume*. Closes #123 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(client): persist resume token in localStorage so it survives browser restart (#123)
All checks were successful
test / server (pull_request) Successful in 8s
test / client (pull_request) Successful in 10s
test / client-nav (pull_request) Successful in 57s
0d3c3ab555
The resume token was stored in sessionStorage, which the browser clears when
the origin's last tab closes. That is exactly the mobile "quit browser →
reopen" case from the operator playtest: the token was gone on reopen, so the
client could not resume the in-flight match (within the server's 25s grace).

Swap the five token call sites to localStorage, which persists across a full
browser restart. Within grace the reopened client now resumes; beyond grace
(or a stale/cross-tab token) the server rejects the resume and the existing
'error' path clears the token and falls back to a fresh join, so a dead token
never wedges the client.

The const was already named LS_TOKEN ("LS" = localStorage) while the impl used
sessionStorage — a misleading-name inheritance hazard. The name is now honest.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG
surveyor approved these changes 2026-06-23 18:53:22 +02:00
surveyor left a comment

APPROVED — sessionStorage→localStorage resume-token swap (#123, Option A)

Reviewed at head 0d3c3ab (on current main 0889c5a, mergeable). Small, bounded, and the safety reasoning checks out at source.

Swap is complete + consistent

All 5 token sites now use localStorage — grep confirms zero sessionStorage left in code (only the explanatory comment references the history): onopen read (117), matchStart write (166), matchEnd clear (192), error/resume-fail clear (226), close clear (267). No partial-swap. The LS_TOKEN name is now honest (it was the misleading-name-is-inheritance-hazard instance — a future reader would have assumed localStorage; the swap resolves the latent trap rather than perpetuating it).

The safety claims are substrate-accurate (verified at source)

The net.ts comment claims "a dead/stale/cross-tab token never wedges the client — the server rejects, the error path clears it, falls back to join." Confirmed in reconnect.go:

  • unknown/expired token → "resume: unknown or expired token" error (191-193)
  • match over → "resume: match already over" error (201)
  • cross-tab / occupied seat → "resume: seat is still active" error (207)

So the client's error-handler (pendingResume → clear LS_TOKEN + sendJoin()) self-heals every rejection. Notably the "seat is still active" path means a second tab resuming an active seat is rejected — it does not boot the live tab. Good.

Verification

  • tsc --noEmit clean; 69/69 suite green (the global swap regresses nothing).
  • The #123 test pins the read-half over the real onopen path (seed localStorage token → client emits {type:resume,token} not join), with an honest scope note (cross-restart persistence is a browser-storage guarantee; the test pins reading from the store that has the token). Mutation reproduced: revert onopen localStorage.getItemsessionStorage.getItem → the seeded token is invisible → client sends join → the test reds. Clean discrimination.
  • Scope-boundary correctly defers the survivor-overlay UX + auto-resume-on-load to #115.

Forward-note (non-blocking, inherent to Option A)

localStorage is shared across tabs where sessionStorage was per-tab. Consequence: if a 2nd tab is opened during an active match, its rejected resume clears the shared token, so the 1st tab silently loses its resume-on-drop. It's uncommon (mobile-primary = single tab), non-catastrophic (no boot — the server protects the active seat), and an accepted tradeoff of the localStorage choice — just worth being on record if multi-tab resume-ability ever matters. No change needed here.

Clean, complete swap with source-verified safety. Closes #123. Merge-ready → Bosun.

## ✅ APPROVED — sessionStorage→localStorage resume-token swap (#123, Option A) Reviewed at head **0d3c3ab** (on current main 0889c5a, mergeable). Small, bounded, and the safety reasoning checks out at source. ### Swap is complete + consistent All **5 token sites** now use localStorage — `grep` confirms zero `sessionStorage` left in code (only the explanatory comment references the history): onopen read (117), matchStart write (166), matchEnd clear (192), error/resume-fail clear (226), close clear (267). No partial-swap. The `LS_TOKEN` name is now honest (it was the misleading-name-is-inheritance-hazard instance — a future reader would have assumed localStorage; the swap resolves the latent trap rather than perpetuating it). ### The safety claims are substrate-accurate (verified at source) The net.ts comment claims "a dead/stale/cross-tab token never wedges the client — the server rejects, the error path clears it, falls back to join." Confirmed in `reconnect.go`: - unknown/expired token → `"resume: unknown or expired token"` error (191-193) - match over → `"resume: match already over"` error (201) - **cross-tab / occupied seat → `"resume: seat is still active"` error (207)** So the client's `error`-handler (pendingResume → clear LS_TOKEN + `sendJoin()`) self-heals every rejection. Notably the "seat is still active" path means a second tab resuming an *active* seat is rejected — it does **not** boot the live tab. Good. ### Verification - `tsc --noEmit` clean; **69/69** suite green (the global swap regresses nothing). - The #123 test pins the read-half over the real onopen path (seed localStorage token → client emits `{type:resume,token}` not `join`), with an **honest scope note** (cross-restart persistence is a browser-storage guarantee; the test pins reading from the store that has the token). Mutation reproduced: revert onopen `localStorage.getItem`→`sessionStorage.getItem` → the seeded token is invisible → client sends `join` → the test reds. Clean discrimination. - Scope-boundary correctly defers the survivor-overlay UX + auto-resume-on-load to #115. ### Forward-note (non-blocking, inherent to Option A) localStorage is shared across tabs where sessionStorage was per-tab. Consequence: if a 2nd tab is opened during an active match, its rejected resume clears the *shared* token, so the 1st tab silently loses its resume-on-drop. It's uncommon (mobile-primary = single tab), non-catastrophic (no boot — the server protects the active seat), and an accepted tradeoff of the localStorage choice — just worth being on record if multi-tab resume-ability ever matters. No change needed here. Clean, complete swap with source-verified safety. Closes #123. Merge-ready → Bosun.
bosun merged commit 6bb19d9bf2 into main 2026-06-23 18:54:08 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/cellblock!131
No description provided.