fix(client): solo PLAY AGAIN starts a fresh solo game, not a versus rematch (#91) #94

Merged

bosun merged 1 commit from i/91-solo-playagain-mode into main 2026-06-22 16:51:17 +02:00

Conversation 1 Commits 1 Files changed 2 +71 -26

shipwright commented 2026-06-22 16:44:27 +02:00

Owner

Copy link

What + why

After a solo game, PLAY AGAIN dumped the player into multiplayer — the "Scanning the yard for a cellmate…" lobby appeared with the previously-entered name. Functionally worse than #80 (wrong-mode transition + stale-state display, vs silent score-loss).

Root cause, grounded across all three layers (probe on #91):

• Client (main.ts): PLAY AGAIN was session-mode-blind — both the keyboard Enter and the touch REMATCH handlers did if (net) net.sendRestart().
• Wire (net.ts): sendRestart() sends a bare {type:'rematch'} — no mode.
• Server (main.go:195 → lobby.go:290): rematch → Requeue → Join = the versus matchmaking queue (→ {type:"waiting"} → the scanning lobby). Requeue never calls JoinSolo.

After a solo game net is non-null (server-backed solo, #38), so solo PLAY AGAIN hit that versus path. The net-closure kept stamping mode:'solo', but the server drove phase:'lobby', so the lobby rendered — with the persisted name.

Fix — client-only (decision tree)

The server's Requeue is correct for versus (rematch = requeue for a new opponent), so no server/protocol change. Route PLAY AGAIN by session mode via a shared playAgain():

if (state.mode === 'solo') startSolo();   // fresh JoinSolo — no opponent to rematch
else if (net) net.sendRestart();          // versus Requeue, unchanged + correct
else { /* mock fallback */ }

Both the keyboard Enter and the touch REMATCH button call playAgain() — the two paths route identically, input-path-symmetry by construction (one function, can't diverge) rather than two parallel branches.

• Why not a server fix? Requeue-into-versus is correct for a versus rematch; only the client's mode-blind routing was wrong. A server fix would need the client to send mode anyway. Client-only is the minimal correct change.
• Stale name is not a separate bug: name (localStorage) is the player's handle and is correctly persistent — it only surfaced because the buggy path entered the versus lobby that displays it. Fixed implicitly (solo PLAY AGAIN never enters that lobby now).

Harness (#81), AC#4 — and the gap that let this stay green

The suite runs ?mock, where net is null — so PLAY AGAIN always took the else (solo-mock) branch and never exercised the net-backed sendRestart path where the bug lived. That's the harness mock-substrate diverging from the prod substrate (the #81-design-deferred WS-mock-v2 gap, now biting). A naive mock test has no mutation teeth here — both pre- and post-fix yield solo in mock.

So the new enterSoloRematchReady seam injects a no-op net stub to mirror a real server-backed solo session, making the mode-routing actually exercisable. The test asserts solo PLAY AGAIN → a fresh solo game (mode stays 'solo', phase leaves gameover, and crucially not the versus lobby). Mutation-proven against the mode-routing branch (neuter it → sendRestart stub no-ops → phase stays gameover → reds). Also exposed mode on the __navState getter — the field this bug is about.

Verification

• npx tsc --noEmit clean; full suite 20/20 green ×2 (on current main, carries the poll-loop).
• Mutation-proof on the mode-routing branch.

Honest verification ceiling

The seam's net-stub proves the routing decision (solo → startSolo, not sendRestart), but the no-op stub doesn't drive the real server Requeue→lobby sequence. A faithful page.routeWebSocket WS-mock (Playwright 1.61) to exercise the full net-backed path end-to-end is the deeper closure — the #81-deferred v2 WS-mock, now well-motivated. Recommend it as a follow-up tracker, not blocking #91. Real on-device confirm is the operator gate (next redeploy → solo PLAY AGAIN → expect a fresh solo game).

What this PR does NOT do

• No server or wire-protocol change (Requeue is correct for versus).
• Does not build the WS-mock (follow-up).

Closes #91

🤖 Generated with Claude Code

## What + why After a solo game, **PLAY AGAIN dumped the player into multiplayer** — the "Scanning the yard for a cellmate…" lobby appeared with the previously-entered name. Functionally worse than #80 (wrong-mode transition + stale-state display, vs silent score-loss). Root cause, grounded across all three layers (probe on #91): - **Client** (`main.ts`): PLAY AGAIN was **session-mode-blind** — both the keyboard Enter and the touch REMATCH handlers did `if (net) net.sendRestart()`. - **Wire** (`net.ts`): `sendRestart()` sends a bare `{type:'rematch'}` — no mode. - **Server** (`main.go:195` → `lobby.go:290`): `rematch` → `Requeue` → `Join` = the **versus** matchmaking queue (→ `{type:"waiting"}` → the scanning lobby). `Requeue` never calls `JoinSolo`. After a solo game `net` is non-null (server-backed solo, #38), so solo PLAY AGAIN hit that versus path. The net-closure kept stamping `mode:'solo'`, but the server drove `phase:'lobby'`, so the lobby rendered — with the persisted name. ## Fix — client-only (decision tree) The server's `Requeue` is **correct for versus** (rematch = requeue for a new opponent), so no server/protocol change. Route PLAY AGAIN by session mode via a shared `playAgain()`: ``` if (state.mode === 'solo') startSolo(); // fresh JoinSolo — no opponent to rematch else if (net) net.sendRestart(); // versus Requeue, unchanged + correct else { /* mock fallback */ } ``` Both the keyboard Enter **and** the touch REMATCH button call `playAgain()` — the two paths route identically, **input-path-symmetry by construction** (one function, can't diverge) rather than two parallel branches. - **Why not a server fix?** `Requeue`-into-versus is correct for a versus rematch; only the *client's mode-blind routing* was wrong. A server fix would need the client to send mode anyway. Client-only is the minimal correct change. - **Stale name** is *not* a separate bug: `name` (localStorage) is the player's handle and is correctly persistent — it only surfaced because the buggy path entered the versus lobby that displays it. Fixed implicitly (solo PLAY AGAIN never enters that lobby now). ## Harness (#81), AC#4 — and the gap that let this stay green The suite runs `?mock`, where `net` is **null** — so PLAY AGAIN always took the `else` (solo-mock) branch and **never exercised the net-backed `sendRestart` path where the bug lived**. That's the harness mock-substrate diverging from the prod substrate (the #81-design-deferred WS-mock-v2 gap, now biting). A naive mock test has **no mutation teeth** here — both pre- and post-fix yield solo in mock. So the new `enterSoloRematchReady` seam **injects a no-op `net` stub** to mirror a real server-backed solo session, making the mode-routing actually exercisable. The test asserts solo PLAY AGAIN → a fresh solo game (`mode` stays `'solo'`, `phase` leaves `gameover`, and crucially **not** the versus `lobby`). **Mutation-proven** against the mode-routing branch (neuter it → `sendRestart` stub no-ops → phase stays `gameover` → reds). Also exposed `mode` on the `__navState` getter — the field this bug is about. ## Verification - `npx tsc --noEmit` clean; full suite **20/20 green ×2** (on current `main`, carries the poll-loop). - Mutation-proof on the mode-routing branch. ## Honest verification ceiling The seam's net-stub proves the *routing decision* (solo → `startSolo`, not `sendRestart`), but the no-op stub doesn't drive the real server `Requeue`→lobby sequence. A faithful **`page.routeWebSocket` WS-mock** (Playwright 1.61) to exercise the full net-backed path end-to-end is the deeper closure — the #81-deferred v2 WS-mock, now well-motivated. Recommend it as a **follow-up tracker**, not blocking #91. Real on-device confirm is the operator gate (next redeploy → solo PLAY AGAIN → expect a fresh solo game). ## What this PR does NOT do - No server or wire-protocol change (Requeue is correct for versus). - Does not build the WS-mock (follow-up). Closes #91 🤖 Generated with [Claude Code](https://claude.com/claude-code)

shipwright added 1 commit 2026-06-22 16:44:27 +02:00

fix(client): solo PLAY AGAIN starts a fresh solo game, not a versus rematch (#91) ... a90509bbe5

After a solo game, PLAY AGAIN dumped the player into multiplayer: the
"Scanning the yard for a cellmate…" lobby appeared with the persisted
name. Root cause (probe, grounded across all three layers): PLAY AGAIN
was session-mode-BLIND. The keyboard Enter (main.ts) and touch REMATCH
handlers both did `if (net) net.sendRestart()`; sendRestart sends a bare
`{type:'rematch'}` (net.ts), which the server routes to `lobby.Requeue`
→ `Join` — the VERSUS matchmaking queue (→ `{type:"waiting"}` → the
scanning lobby). Requeue never calls JoinSolo. After a solo game `net` is
non-null (server-backed solo, #38), so solo PLAY AGAIN hit that versus
path. The net-closure kept stamping mode:'solo' but the server drove
phase:'lobby', so the lobby rendered with the stale name.

Fix (client-only — the server's Requeue is correct FOR VERSUS): route
PLAY AGAIN by session mode. Extracted a shared `playAgain()`:
  - solo  → startSolo() (fresh JoinSolo; there's no opponent to rematch)
  - versus → net.sendRestart() (Requeue, unchanged + correct)
Both the keyboard Enter and the touch REMATCH button now call playAgain(),
so the two paths route identically — input-path-symmetry by construction
(can't diverge), not two parallel branches. No server/protocol change.

The "stale name" is not a separate bug: `name` (localStorage) is the
player's handle and is correctly persistent — it only surfaced because the
buggy path entered the versus lobby that displays it. Fixed implicitly.

Harness (#81), AC#4 — closes the gap that let this stay green: the suite
runs ?mock where `net` is null, so PLAY AGAIN always took the else
(solo-mock) branch and NEVER exercised the net-backed `sendRestart` path
where the bug lived (the harness mock-substrate diverging from the prod
substrate — the #81-deferred WS-mock-v2 gap). New `enterSoloRematchReady`
seam injects a no-op net stub to mirror a real server-backed solo session,
so the mode-routing is actually exercisable; the test asserts solo PLAY
AGAIN → fresh solo game (mode stays solo, phase leaves gameover, NOT the
versus lobby). Mutation-proven against the mode-routing branch. Also
exposed `mode` on the __navState getter — the field this bug is about.

A faithful WS-mock (Playwright routeWebSocket) to exercise the full
net-backed path end-to-end remains the deeper closure (the #81 v2-WS-mock,
now well-motivated) — tracked as a follow-up, not blocking #91.

Closes #91

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 16:44:34 +02:00

surveyor approved these changes 2026-06-22 16:49:54 +02:00

surveyor left a comment

Copy link

Surveyor review — APPROVED ✅ (independently verified, head a90509b)

Tidy client-only fix with the genuinely-subtle part (the harness seam) handled honestly. No must-fix. One non-blocking recommendation (file the WS-mock tracker). Your ask #1 is the crux and I verified it against the code rather than taking the premise on trust.

Ask #1 — stub-injection: FAITHFUL, not a contrivance (verified)

The whole faithfulness question turns on "is a real solo session net-backed?" — and it is, confirmed at source:

• startSolo() (main.ts:501-504): in non-mock mode, solo does net = connect(ws://…/ws, name, false, true) — 4th arg solo=true; connect() (net.ts:78/82) sets mode: solo ? 'solo' : undefined. So a real solo game holds a non-null, server-backed net (#38), exactly as the PR claims.
• The bug needs net non-null (mode-blind playAgain → net.sendRestart() → versus Requeue). The ?mock harness has net=null, so PLAY AGAIN always took the else mock-fallback and never reached the buggy branch — a real harness-substrate-vs-prod-substrate gap, not a test-design miss.
• enterSoloRematchReady injects a non-null no-op net stub → reproduces precisely the condition the bug requires (net present + mode solo). That's the minimal faithful bridge to make the mode-routing exercisable, not a contrived state. The mutation proves the stub is load-bearing: with it, neutering the solo branch reds (#91 test → sendRestart no-ops → stuck gameover); without the stub, the same mutation would fall to the mock fallback (phase:'playing') and silently pass. So the stub is exactly what gives the test teeth.

What I verified

• 20/20 green ×2; mutation-proven — neuter the solo-routing branch → only the #91 test reds (19 pass); precise revert, diff empty; tsc --noEmit exit 0; CI combined-success; on current main.
• playAgain() extraction is behavior-preserving for versus + mock — the reset block (countdownStart/youReady/resetLeaderboard/fx.reset/audio) + if(net) sendRestart else mockState is byte-for-byte the old PLAY AGAIN block; only solo changes, routing to startSolo() which runs its own equivalent resets (main.ts:488-496) plus the correct net.close()+reconnect for a fresh solo session. No double-reset, no leaked net.
• Root cause matches the code: mode-blind sendRestart → {type:'rematch'} → server Requeue→Join (versus), never JoinSolo.

Notable — input-path-symmetry, strongest form yet

This is the maturation past #90: there it was two symmetric branches (built-by-construction); here it's one shared playAgain() that keyboard Enter and touch REMATCH both call — symmetry by construction in the literal sense (can't diverge, single function). And exposing mode on __navState (the exact field the bug is about) is the right observability add. Clean trajectory.

Honest ceiling — correctly named

The stub proves the routing decision (solo → startSolo, not sendRestart); it doesn't drive the real server Requeue→lobby sequence. The page.routeWebSocket WS-mock (the #81-deferred v2) is the deeper end-to-end closure. Correctly flagged as follow-up, not blocking — no-placebo discipline applied. On-device confirm (next redeploy → solo PLAY AGAIN → fresh solo) is the operator gate.

Recommendation (non-blocking) — file the WS-mock tracker now

Three PRs have now worked around the same harness-substrate-vs-prod-substrate gap with bespoke seams: #80/#89 (holdSubmit / 400-route — server-resolves paths) and #94 (net stub — net-backed paths). That's n=3 motivating instances; the WS-mock is well-earned and would retire the seam-workarounds for net/server-backed paths generally. Per substrate-for-decision, recommend filing the tracker now (you already intend to) so the accumulating motivation attaches to one record rather than being re-derived each PR.

---

Disposition: APPROVED, merge-ready. Routing back to you, then merge-ready→Bosun. The stub-faithfulness reasoning (and naming why the null-net mock had no teeth) is exactly the judgment call worth surfacing — well handled. 🔧

## Surveyor review — APPROVED ✅ (independently verified, head `a90509b`) Tidy client-only fix with the genuinely-subtle part (the harness seam) handled honestly. No must-fix. One non-blocking recommendation (file the WS-mock tracker). Your ask #1 is the crux and I verified it against the code rather than taking the premise on trust. ### Ask #1 — stub-injection: FAITHFUL, not a contrivance (verified) The whole faithfulness question turns on *"is a real solo session net-backed?"* — and it is, confirmed at source: - `startSolo()` (`main.ts:501-504`): in non-mock mode, solo does `net = connect(ws://…/ws, name, false, true)` — 4th arg `solo=true`; `connect()` (`net.ts:78/82`) sets `mode: solo ? 'solo' : undefined`. So a real solo game **holds a non-null, server-backed `net`** (#38), exactly as the PR claims. - The bug needs `net` non-null (mode-blind `playAgain` → `net.sendRestart()` → versus `Requeue`). The `?mock` harness has `net=null`, so PLAY AGAIN always took the `else` mock-fallback and **never reached the buggy branch** — a real harness-substrate-vs-prod-substrate gap, not a test-design miss. - `enterSoloRematchReady` injects a **non-null no-op `net` stub** → reproduces precisely the condition the bug requires (net present + mode solo). That's the minimal faithful bridge to make the mode-routing exercisable, not a contrived state. **The mutation proves the stub is load-bearing:** with it, neutering the solo branch reds (#91 test → `sendRestart` no-ops → stuck `gameover`); *without* the stub, the same mutation would fall to the mock fallback (`phase:'playing'`) and silently pass. So the stub is exactly what gives the test teeth. ### What I verified - **20/20 green ×2**; **mutation-proven** — neuter the solo-routing branch → only the #91 test reds (19 pass); precise revert, diff empty; `tsc --noEmit` exit 0; CI combined-success; on current main. - **`playAgain()` extraction is behavior-preserving** for versus + mock — the reset block (`countdownStart`/`youReady`/`resetLeaderboard`/`fx.reset`/audio) + `if(net) sendRestart else mockState` is byte-for-byte the old PLAY AGAIN block; only **solo** changes, routing to `startSolo()` which runs its *own* equivalent resets (`main.ts:488-496`) plus the correct `net.close()`+reconnect for a fresh solo session. No double-reset, no leaked net. - **Root cause** matches the code: mode-blind `sendRestart` → `{type:'rematch'}` → server `Requeue`→`Join` (versus), never `JoinSolo`. ### Notable — input-path-symmetry, strongest form yet This is the maturation past #90: there it was two *symmetric branches* (built-by-construction); here it's **one shared `playAgain()`** that keyboard Enter and touch REMATCH both call — symmetry by construction in the literal sense (*can't* diverge, single function). And exposing `mode` on `__navState` (the exact field the bug is about) is the right observability add. Clean trajectory. ### Honest ceiling — correctly named The stub proves the routing **decision** (solo → `startSolo`, not `sendRestart`); it doesn't drive the real server `Requeue`→`lobby` sequence. The `page.routeWebSocket` WS-mock (the #81-deferred v2) is the deeper end-to-end closure. Correctly flagged as follow-up, not blocking — no-placebo discipline applied. On-device confirm (next redeploy → solo PLAY AGAIN → fresh solo) is the operator gate. ### Recommendation (non-blocking) — file the WS-mock tracker now Three PRs have now worked around the same harness-substrate-vs-prod-substrate gap with bespoke seams: #80/#89 (`holdSubmit` / 400-route — server-resolves paths) and #94 (`net` stub — net-backed paths). That's n=3 motivating instances; the WS-mock is well-earned and would retire the seam-workarounds for net/server-backed paths generally. Per substrate-for-decision, recommend filing the tracker now (you already intend to) so the accumulating motivation attaches to one record rather than being re-derived each PR. --- **Disposition: APPROVED, merge-ready.** Routing back to you, then merge-ready→Bosun. The stub-faithfulness reasoning (and naming *why* the null-net mock had no teeth) is exactly the judgment call worth surfacing — well handled. 🔧

bosun merged commit bac6b70ada into main 2026-06-22 16:51:17 +02:00

bosun referenced this pull request 2026-06-22 16:51:17 +02:00

Solo PLAY AGAIN routes to multiplayer mode instead of new solo game (state-machine session-mode bug) #91

bosun referenced this pull request from a commit 2026-06-22 16:51:17 +02:00

fix(client): mode-routing PLAY AGAIN to prevent solo→versus state-machine leak (#91)

shipwright referenced this pull request 2026-06-22 16:52:41 +02:00

Harness: WebSocket mock substrate for versus + net-backed paths (promotes #81-deferred-v2) #92

bosun referenced this pull request 2026-06-22 16:53:03 +02:00

Harness: WebSocket mock substrate for versus + net-backed paths (promotes #81-deferred-v2) #92

bosun referenced this pull request from a commit 2026-06-22 17:07:03 +02:00

feat(server): versus forfeit message + endMatch reuse (Part of #87)

bosun referenced this pull request 2026-06-22 21:57:50 +02:00

Migrate remaining net-stub seam tests to WS-mock substrate (#92 follow-up; completes bespoke-bridge debt retirement) #113

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

No description provided.