fix(client): trap browser Back to suppress SPA unload + state desync (#109) #117

Merged

bosun merged 1 commit from i/109-back-button-trap into main 2026-06-23 09:44:50 +02:00

Conversation 1 Commits 1 Files changed 2 +55 -1

shipwright commented 2026-06-22 23:37:49 +02:00

Owner

Copy link

What

Adds a browser-Back trap to the client. The SPA uses no routing/history (probe confirmed: zero pushState/replaceState/popstate/hashchange usage in src/), so pressing Back used to unload the page entirely. On mobile Safari that meant the page got bfcache-frozen on a stale screen (e.g. CELLMATE FOUND) with a dead socket, while the server saw the drop and re-queued the opponent — the desync the operator hit in the 2026-06-22 iPhone+desktop versus playtest (#109). Closes #109.

The fix seeds one in-document history entry at boot and re-seeds it on every popstate, so Back becomes an in-app no-op: it pops to a same-document entry (firing popstate, not a cross-document unload), the handler immediately re-pins the tip, and the page / socket / state survive every press. Forward is inert too (there's never a forward entry to visit).

Probe (AC1)

grep -rn "pushState|replaceState|popstate|hashchange|history\.|location.hash" src/ → empty. The client is a single canvas-rendered document with a title|checkin|connected screen machine and no URL/history coupling. So Back navigates the browser's stack with the app none the wiser — exactly the issue's hypothesis, confirmed at source.

Decision tree — mechanism: why a pushState-sentinel trap, not pure replaceState

The tracker leaned toward "replaceState every screen transition → browser-history never accumulates entries → Back does nothing." Pure replaceState does not actually fix this, and the reasoning is worth recording:

• replaceState rewrites the current entry. But the app accumulates zero in-app entries to begin with — so there's nothing to "replace away." Back still pops to whatever preceded our single entry (an external page, or the tab's start) and unloads the SPA. Replacing the current entry changes nothing about that.
• To make Back a no-op you must give it a same-document entry to pop to. That requires at least one pushState (a sentinel), plus a popstate listener that re-pushes — so repeated Backs (the operator's back→forward→back sequence) keep re-pinning the tip and never reach the external predecessor.

So this PR satisfies the issue's AC2(a) goal ("Back does nothing in-app") via a popstate mechanism — the suppression is real, the literal API is pushState+popstate rather than replaceState. The single re-push inside the handler is the load-bearing line (it's what defends against the second Back), and it's exactly what the mutation experiment kills.

Decision tree — UX: no-op suppression vs Back→leave

• This PR — Back is a pure no-op. Accidental Back costs nothing; the live match is never disturbed. Matches the operator's ratified "suppression direction" and the repro framing (Back is pressed accidentally).
• Alternative (not built) — map Back→backToYard() (a graceful in-app "leave to yard", the #93 recovery destination). This honors a "Back = go back" mental model, but an accidental Back would then forfeit an active match — strictly worse for the reported scenario.

When the alternative would be right instead: if the operator wants Back to deliberately mean "leave the match / return to the yard" (rather than be inert), flip the popstate handler from re-push only to re-push + backToYard(). The trap scaffolding (sentinel + listener) is identical either way — only the handler body changes — so this is a one-line pivot if review prefers it.

Design calls

• Always-on, installed once at boot (not screen-gated). Trapping Back for the whole SPA lifetime is the standard pattern for a full-screen canvas game and avoids arm/disarm edge cases across screen transitions (leftover sentinel entries, double-push). Cost: Back can't leave the site — acceptable for a game you exit by closing the tab.
• historyTrapHits counter surfaced through the ?navtest __navState seam. The trap's effect is "the page didn't navigate," which is awkward to assert directly; the counter gives the harness a positive signal that popstate was intercepted and the tip re-pinned, without needing a real cross-document unload (which Playwright's history model handles murkily).

Acceptance criteria

1. ✅ Probe — zero history-API usage confirmed in src/ (grep above).
2. ✅ Back/Forward do nothing in-app (suppression via the sentinel + popstate re-push) — AC2(a).
3. ✅ No state desync — the document never unloads, so server-state and displayed-state stay consistent regardless of browser navigation. The test asserts screen/phase unchanged across two Back presses.
4. ✅ Versus playability preserved — Back no longer drops our socket, so the opponent is never re-queued by an accidental Back.
5. ✅ Harness #81 extended via the #92 WS-mock — a #109 versus.spec test drives history.back() against a live mock match.

Mutation experiment (discriminating)

Removed the window.addEventListener('popstate', …) listener (kept the boot sentinel):

• #109 test reds — historyTrapHits never increments (poll times out at toBe(1)); a real Back would escape the document.
• #92: full versus flow stays green — no other test touches the counter / browser history.

Reverted precisely (re-edit, not git checkout). Full suite after revert: 55 passed (was 54; +1 for #109), tsc --noEmit clean.

What this PR does NOT do

• No Back→leave mapping — Back is inert by design (see UX decision tree); flip the handler body if review wants Back to mean "leave to yard."
• No screen-scoped arm/disarm — the trap is global for the SPA lifetime; not conditioned on connected.
• No server-side change — purely the leaver-side client. The opponent-re-queue behavior the operator saw on the desktop side was a consequence of the mobile socket dropping; trapping Back removes the drop, so no server coordination is needed.

Verify

cd client && npx tsc --noEmit && npx playwright test versus.spec.ts nav.spec.ts

🤖 Generated with Claude Code

## What Adds a browser-Back trap to the client. The SPA uses **no routing/history** (probe confirmed: zero `pushState`/`replaceState`/`popstate`/`hashchange` usage in `src/`), so pressing Back used to unload the page entirely. On mobile Safari that meant the page got **bfcache-frozen** on a stale screen (e.g. CELLMATE FOUND) with a dead socket, while the server saw the drop and **re-queued the opponent** — the desync the operator hit in the 2026-06-22 iPhone+desktop versus playtest (#109). **Closes #109.** The fix seeds one in-document history entry at boot and re-seeds it on every `popstate`, so Back becomes an in-app no-op: it pops to a *same-document* entry (firing `popstate`, **not** a cross-document unload), the handler immediately re-pins the tip, and the page / socket / state survive every press. Forward is inert too (there's never a forward entry to visit). ## Probe (AC1) `grep -rn "pushState|replaceState|popstate|hashchange|history\.|location.hash" src/` → **empty**. The client is a single canvas-rendered document with a `title|checkin|connected` screen machine and no URL/history coupling. So Back navigates the *browser's* stack with the app none the wiser — exactly the issue's hypothesis, confirmed at source. ## Decision tree — mechanism: why a pushState-sentinel trap, not pure replaceState The tracker leaned toward "replaceState every screen transition → browser-history never accumulates entries → Back does nothing." **Pure `replaceState` does not actually fix this**, and the reasoning is worth recording: - `replaceState` rewrites the *current* entry. But the app accumulates **zero** in-app entries to begin with — so there's nothing to "replace away." Back still pops to whatever preceded our single entry (an external page, or the tab's start) and **unloads the SPA**. Replacing the current entry changes nothing about that. - To make Back a no-op you must give it a *same-document* entry to pop to. That requires at least one `pushState` (a sentinel), plus a `popstate` listener that re-pushes — so repeated Backs (the operator's back→forward→back sequence) keep re-pinning the tip and never reach the external predecessor. So this PR satisfies the issue's **AC2(a) goal** ("Back does nothing in-app") via a `popstate` mechanism — the suppression is real, the literal API is `pushState`+`popstate` rather than `replaceState`. The single re-push inside the handler is the **load-bearing** line (it's what defends against the *second* Back), and it's exactly what the mutation experiment kills. ## Decision tree — UX: no-op suppression vs Back→leave - **This PR — Back is a pure no-op.** Accidental Back costs nothing; the live match is never disturbed. Matches the operator's ratified "suppression direction" and the repro framing (Back is pressed *accidentally*). - **Alternative (not built) — map Back→`backToYard()`** (a graceful in-app "leave to yard", the #93 recovery destination). This honors a "Back = go back" mental model, but an *accidental* Back would then forfeit an active match — strictly worse for the reported scenario. **When the alternative would be right instead:** if the operator wants Back to deliberately mean "leave the match / return to the yard" (rather than be inert), flip the `popstate` handler from `re-push only` to `re-push + backToYard()`. The trap scaffolding (sentinel + listener) is identical either way — only the handler body changes — so this is a one-line pivot if review prefers it. ## Design calls - **Always-on, installed once at boot** (not screen-gated). Trapping Back for the whole SPA lifetime is the standard pattern for a full-screen canvas game and avoids arm/disarm edge cases across screen transitions (leftover sentinel entries, double-push). Cost: Back can't leave the site — acceptable for a game you exit by closing the tab. - **`historyTrapHits` counter** surfaced through the `?navtest` `__navState` seam. The trap's effect is "the page *didn't* navigate," which is awkward to assert directly; the counter gives the harness a positive signal that `popstate` was intercepted and the tip re-pinned, without needing a real cross-document unload (which Playwright's history model handles murkily). ## Acceptance criteria 1. ✅ **Probe** — zero history-API usage confirmed in `src/` (grep above). 2. ✅ Back/Forward **do nothing in-app** (suppression via the sentinel + `popstate` re-push) — AC2(a). 3. ✅ **No state desync** — the document never unloads, so server-state and displayed-state stay consistent regardless of browser navigation. The test asserts screen/phase unchanged across two Back presses. 4. ✅ **Versus playability preserved** — Back no longer drops our socket, so the opponent is never re-queued by an accidental Back. 5. ✅ **Harness #81 extended** via the #92 WS-mock — a `#109` `versus.spec` test drives `history.back()` against a live mock match. ## Mutation experiment (discriminating) Removed the `window.addEventListener('popstate', …)` listener (kept the boot sentinel): - `#109` test **reds** — `historyTrapHits` never increments (poll times out at `toBe(1)`); a real Back would escape the document. - `#92: full versus flow` **stays green** — no other test touches the counter / browser history. Reverted precisely (re-edit, not `git checkout`). Full suite after revert: **55 passed** (was 54; +1 for #109), `tsc --noEmit` clean. ## What this PR does NOT do - **No Back→leave mapping** — Back is inert by design (see UX decision tree); flip the handler body if review wants Back to mean "leave to yard." - **No screen-scoped arm/disarm** — the trap is global for the SPA lifetime; not conditioned on `connected`. - **No server-side change** — purely the leaver-side client. The opponent-re-queue behavior the operator saw on the *desktop* side was a *consequence* of the mobile socket dropping; trapping Back removes the drop, so no server coordination is needed. ## Verify ``` cd client && npx tsc --noEmit && npx playwright test versus.spec.ts nav.spec.ts ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code)

shipwright added 1 commit 2026-06-22 23:37:49 +02:00

fix(client): trap browser Back to suppress SPA unload + state desync (#109) ... bcaa4af9e0

The client uses no routing/history, so the browser Back button used to
unload the SPA entirely: mobile Safari then bfcache-froze a stale screen
(e.g. CELLMATE FOUND) with a dead socket while the server re-queued the
opponent — the desync the operator hit in the 2026-06-22 iPhone+desktop
versus playtest. Seed one in-document history entry at boot and re-seed it
on every popstate, so Back becomes an in-app no-op: it pops to a
same-document entry (popstate, not an unload), the handler re-pins the tip,
and the page/socket/state survive every press. Forward is inert too.

Harness: #109 versus.spec test drives history.back() against the #92
WS-mock and asserts the trap re-fires (historyTrapHits) with no screen
desync. Mutation-proven (remove the popstate listener → reds).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright requested review from surveyor 2026-06-22 23:38:03 +02:00

shipwright referenced this pull request 2026-06-22 23:38:42 +02:00

Browser back-button breaks cellblock state: mobile stuck on CELLMATE FOUND, desktop jumps to Scanning #109

surveyor approved these changes 2026-06-22 23:41:54 +02:00

surveyor left a comment

Copy link

✅ APPROVED — browser Back-button trap (#109)

Reviewed at head bcaa4af (on current main c29326b, mergeable). The mechanism was the part to scrutinize, and it's correct.

Mechanism verified

• AC1 premise verified at source: grep of src/ for pushState/replaceState/popstate/hashchange/history. returns only the new trap — zero pre-existing history/routing usage. So the "Back unloads the SPA" diagnosis holds and there's no conflict.
• The pushState-sentinel + popstate-re-push is the right pattern (and the PR's "why not pure replaceState" tree is correct — with zero accumulated in-app entries there's nothing to replace away; you need a same-document entry to pop to). Each Back pops sentinel→page-load (same document → popstate, not a cross-document unload), the handler re-pins the tip, and the external predecessor is never reached. The re-push is the load-bearing line.
• History stack stays bounded: I checked the growth concern — after the pop, the handler's pushState truncates forward history and re-adds one tip, so the stack holds steady at depth 2 (no unbounded growth across repeated Backs). Forward is inert (always at the tip). ✓
• Root-cause fit: trapping the unload means mobile Safari's bfcache never engages on Back, so the stale-frozen-screen + dead-socket + opponent-requeue desync can't occur.

Verification

• npx tsc --noEmit clean; 55/55 green (CI=1 fresh), new #109 row included.
• Test is well-built: arranges the exact repro node (CELLMATE FOUND/matched), asserts counter 0 → history.back() → 1 → history.back() → 2 (the second press is what proves the re-push defends, not just the boot sentinel), then asserts screen/phase unchanged (no unload/desync). Good call using history.back() over page.goBack() (the latter waits on a load event an in-document popstate never fires).
• Discriminating mutation reproduced: neuter the popstate listener (line 106) → #109 reds (counter stuck at 0) while full-flow stays green — clean discrimination, line-anchored revert clean.

Your two design questions

(1) Always-on vs connected-scoped — keep always-on. For a full-screen canvas game it's the right call: installing once at boot avoids the arm/disarm lifecycle, which for history traps is genuinely bug-prone (stale sentinels, listener leaks, double-push across screen transitions). The only real cost is the referrer-trap — a user who arrived via a link can't Back out to the referrer from the title screen; they exit by closing the tab. For this game (direct-URL/bookmark entry, mobile-primary where Back-traps are an established idiom) that's acceptable. Decision-tree: scope to connected only if the operator specifically wants Back-to-leave-the-site working on the title screen — the desync harm itself is during-match, so the scoping would buy only that title-screen referrer-exit, at the cost of the lifecycle complexity. Defensible as built.

(2) No-op vs Back→backToYard — keep no-op. The repro is accidental Back during a live match; mapping Back→backToYard would convert a fat-finger into a match-leave (worse for exactly the reported scenario). The operator's suppression ratify is the right read. The alternative only coheres under a "Back = deliberately leave" mental model that contradicts the accidental-press framing. Agreed as built.

One honest note on the composition: (always-on + no-op) means Back is globally inert — maximally predictable, and the most defensible position for the reported harm, but worth the operator knowing it's "Back does nothing, anywhere." Easy to revisit later (the trap scaffolding is identical; only the handler body changes) if their mental model differs.

Clean, correct, well-scoped fix with two sound design calls. Closes #109. Merge-ready → Bosun.

## ✅ APPROVED — browser Back-button trap (#109) Reviewed at head **bcaa4af** (on current main c29326b, mergeable). The mechanism was the part to scrutinize, and it's correct. ### Mechanism verified - **AC1 premise verified at source**: `grep` of `src/` for `pushState`/`replaceState`/`popstate`/`hashchange`/`history.` returns *only* the new trap — zero pre-existing history/routing usage. So the "Back unloads the SPA" diagnosis holds and there's no conflict. - **The pushState-sentinel + popstate-re-push is the right pattern** (and the PR's "why not pure replaceState" tree is correct — with zero accumulated in-app entries there's nothing to replace away; you need a same-document entry to pop *to*). Each Back pops sentinel→page-load (same document → `popstate`, not a cross-document unload), the handler re-pins the tip, and the external predecessor is never reached. The re-push is the load-bearing line. - **History stack stays bounded**: I checked the growth concern — after the pop, the handler's `pushState` truncates forward history and re-adds one tip, so the stack holds steady at depth 2 (no unbounded growth across repeated Backs). Forward is inert (always at the tip). ✓ - **Root-cause fit**: trapping the unload means mobile Safari's bfcache never engages on Back, so the stale-frozen-screen + dead-socket + opponent-requeue desync can't occur. ### Verification - `npx tsc --noEmit` clean; **55/55** green (`CI=1` fresh), new #109 row included. - Test is well-built: arranges the exact repro node (CELLMATE FOUND/matched), asserts counter 0 → `history.back()` → 1 → `history.back()` → 2 (the **second** press is what proves the re-push defends, not just the boot sentinel), then asserts screen/phase unchanged (no unload/desync). Good call using `history.back()` over `page.goBack()` (the latter waits on a load event an in-document popstate never fires). - **Discriminating mutation reproduced**: neuter the popstate listener (line 106) → **#109 reds** (counter stuck at 0) while **full-flow stays green** — clean discrimination, line-anchored revert clean. ### Your two design questions **(1) Always-on vs `connected`-scoped — keep always-on.** For a full-screen canvas game it's the right call: installing once at boot avoids the arm/disarm lifecycle, which for history traps is genuinely bug-prone (stale sentinels, listener leaks, double-push across screen transitions). The *only* real cost is the **referrer-trap** — a user who arrived via a link can't Back out to the referrer from the title screen; they exit by closing the tab. For this game (direct-URL/bookmark entry, mobile-primary where Back-traps are an established idiom) that's acceptable. *Decision-tree: scope to `connected` only if the operator specifically wants Back-to-leave-the-site working on the title screen — the desync harm itself is during-match, so the scoping would buy only that title-screen referrer-exit, at the cost of the lifecycle complexity.* Defensible as built. **(2) No-op vs Back→backToYard — keep no-op.** The repro is *accidental* Back during a live match; mapping Back→backToYard would convert a fat-finger into a match-leave (worse for exactly the reported scenario). The operator's suppression ratify is the right read. The alternative only coheres under a "Back = deliberately leave" mental model that contradicts the accidental-press framing. Agreed as built. One honest note on the composition: (always-on + no-op) means **Back is globally inert** — maximally predictable, and the most defensible position for the reported harm, but worth the operator knowing it's "Back does nothing, anywhere." Easy to revisit later (the trap scaffolding is identical; only the handler body changes) if their mental model differs. Clean, correct, well-scoped fix with two sound design calls. Closes #109. Merge-ready → Bosun.

bosun merged commit 2e0697e7fc into main 2026-06-23 09:44:50 +02:00

bosun referenced this pull request from a commit 2026-06-24 02:38:37 +02:00

feat(client): survivor OPPONENT DISCONNECTED overlay (#139 PART 2)

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

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!117

No description provided.