feat(server): pre-game cancel protocol — {type:"cancel"} + opponentCancelled (#151) #164

Merged

bosun merged 1 commit from i/151-cancel-protocol into main 2026-06-24 11:22:54 +02:00

Conversation 1 Commits 1 Files changed 4 +127

engineer commented 2026-06-24 11:19:21 +02:00

Owner

Copy link

Closes #151. Engine-room half of #142 (CELLMATE-FOUND back/cancel); consumed by Shipwright's #142 client half (PR #161). #151 + #161 ship together.

A player on the matched/ready-up CELLMATE FOUND screen needs a graceful bail, and the opponent needs an explicit "OPPONENT CANCELLED — returning to lobby" closure — not a silent re-pair that reads as a crash.

Why a new verb (neither existing signal fit)

• forfeit means "opponent wins a game" (endMatch/gameover) — wrong, no game has started on the ready screen.
• raw socket-close already requeues the survivor via cancelPendingLocked, but with no cancel-reason, so the survivor can't tell "peer bailed" from "matchmaking re-paired me".

Contract

direction	message	when
client→server	{type:"cancel"}	BACK on the matched/ready screen, pre-matchStart only (p.pending != nil); no-op after matchStart + when unpaired. Canceller returns to title on its own side (no server ack).
server→survivor	{type:"opponentCancelled"}	sent immediately before the requeue → survivor sees opponentCancelled then exactly one of waiting/matched.

Ordering — the load-bearing invariant

cancelPendingLocked emits the survivor's waiting/matched via sendCritical (async/unordered). So opponentCancelled is a direct synchronous send before the requeue — buffered first → the survivor sees the closure before the transition, deterministically (the direct send occupies the channel slot before the async requeue goroutine is even spawned). Same ordering rule as solo.go's matchStart-before-first-state. Pre-game + buffered(64) + near-idle survivor → the direct send doesn't block under l.mu.

Dedicated message (not a reason-tag)

Symmetric with the existing opponentDisconnect/opponentReconnect notifies; keeps the closure beat clean; avoids muddying the re-pair case ("opponent cancelled" + "here's your new match" in one frame). reason-tag would only win if the client couldn't hold a transient closure-beat distinct from the transition — it can (Shipwright owns the brief-beat-before-instant-repair render timing).

ACs

1. cancel in pending/matched → survivor gets opponentCancelled then waiting/matched, in order ✓ (tested both requeue branches).
2. canceller fully detached (p.pending cleared, no server→canceller msg) ✓.
3. cancel after matchStart / no pending pair → no-op, no panic, no spurious notify ✓.
4. existing close-during-pending requeue unchanged (purely additive) ✓.

Verification

• cd server && go test ./... (exact CI) — green; -race — green; gofmt -l/vet clean.
• Mutation-verified the ordering invariant on BOTH requeue branches — drop the opponentCancelled send → the survivor's first message becomes the requeue:

  --- FAIL: TestCancel_NotifiesSurvivorBeforeRequeue
      cancel_test.go:25: first survivor message = main.WaitingMessage, want OpponentCancelledMessage (closure must precede requeue)
  --- FAIL: TestCancel_RepairsSurvivorAfterNotify
      cancel_test.go:52: first survivor message = main.MatchedMessage, want OpponentCancelledMessage
  

  reverted by re-edit; suite green (cache-confirmed byte-identical).

What this PR does NOT do

• No client change — Shipwright's #161 builds the BACK/Esc affordance, the closure render, and the brief-beat-before-instant-repair timing.
• No change to forfeit / disconnect / the existing requeue — cancelPendingLocked is reused untouched; the only addition is the pre-requeue notify.

Closes #151. Engine-room half of **#142** (CELLMATE-FOUND back/cancel); consumed by Shipwright's #142 client half (**PR #161**). **#151 + #161 ship together.** A player on the matched/ready-up **CELLMATE FOUND** screen needs a graceful bail, and the opponent needs an explicit "OPPONENT CANCELLED — returning to lobby" closure — not a silent re-pair that reads as a crash. ## Why a new verb (neither existing signal fit) - **`forfeit`** means "opponent wins a **game**" (endMatch/gameover) — wrong, no game has started on the ready screen. - **raw socket-close** already requeues the survivor via `cancelPendingLocked`, but with **no cancel-reason**, so the survivor can't tell "peer bailed" from "matchmaking re-paired me". ## Contract | direction | message | when | |-----------|---------|------| | client→server | `{type:"cancel"}` | BACK on the matched/ready screen, **pre-matchStart only** (`p.pending != nil`); no-op after matchStart + when unpaired. Canceller returns to title on its own side (no server ack). | | server→survivor | `{type:"opponentCancelled"}` | sent **immediately before** the requeue → survivor sees `opponentCancelled` **then** exactly one of `waiting`/`matched`. | ## Ordering — the load-bearing invariant `cancelPendingLocked` emits the survivor's `waiting`/`matched` via `sendCritical` (**async/unordered**). So `opponentCancelled` is a **direct synchronous send before** the requeue — buffered first → the survivor sees the closure before the transition, **deterministically** (the direct send occupies the channel slot before the async requeue goroutine is even spawned). Same ordering rule as `solo.go`'s matchStart-before-first-state. Pre-game + buffered(64) + near-idle survivor → the direct send doesn't block under `l.mu`. ## Dedicated message (not a reason-tag) Symmetric with the existing `opponentDisconnect`/`opponentReconnect` notifies; keeps the closure beat clean; avoids muddying the re-pair case ("opponent cancelled" + "here's your new match" in one frame). *reason-tag would only win if the client couldn't hold a transient closure-beat distinct from the transition — it can (Shipwright owns the brief-beat-before-instant-repair render timing).* ## ACs 1. cancel in pending/matched → survivor gets `opponentCancelled` **then** `waiting`/`matched`, in order ✓ (tested both requeue branches). 2. canceller fully detached (`p.pending` cleared, no server→canceller msg) ✓. 3. cancel after matchStart / no pending pair → no-op, no panic, no spurious notify ✓. 4. existing close-during-pending requeue unchanged (purely additive) ✓. ## Verification - `cd server && go test ./...` (exact CI) — **green**; `-race` — **green**; `gofmt -l`/`vet` clean. - **Mutation-verified the ordering invariant on BOTH requeue branches** — drop the `opponentCancelled` send → the survivor's first message becomes the requeue: ``` --- FAIL: TestCancel_NotifiesSurvivorBeforeRequeue cancel_test.go:25: first survivor message = main.WaitingMessage, want OpponentCancelledMessage (closure must precede requeue) --- FAIL: TestCancel_RepairsSurvivorAfterNotify cancel_test.go:52: first survivor message = main.MatchedMessage, want OpponentCancelledMessage ``` reverted by re-edit; suite green (cache-confirmed byte-identical). ## What this PR does NOT do - **No client change** — Shipwright's #161 builds the BACK/Esc affordance, the closure render, and the brief-beat-before-instant-repair timing. - **No change to forfeit / disconnect / the existing requeue** — `cancelPendingLocked` is reused untouched; the only addition is the pre-requeue notify.

engineer added 1 commit 2026-06-24 11:19:21 +02:00

feat(server): pre-game cancel protocol — {type:"cancel"} + opponentCancelled (#151) ... 08f99ea7a4

Closes #151. Engine-room half of #142 (CELLMATE-FOUND back/cancel); consumed by
Shipwright's #142 client half (PR #161). A player on the matched/ready-up screen
needs a graceful bail, and the opponent needs an explicit "OPPONENT CANCELLED —
returning to lobby" closure rather than a silent re-pair that reads as a crash.

Why a new verb (neither fit):
- forfeit means "opponent wins a GAME" (endMatch/gameover) — wrong, no game has
  started on the ready screen.
- raw socket-close already requeues the survivor via cancelPendingLocked, but
  with no cancel-reason, so the survivor can't distinguish "peer bailed" from
  "matchmaking re-paired me".

Contract:
- client→server {type:"cancel"} — valid pre-matchStart only (p.pending != nil);
  no-op after matchStart + when unpaired. Canceller's client returns to title on
  its own side (no server ack).
- server→survivor {type:"opponentCancelled"} — sent immediately before the
  requeue, so the survivor sees opponentCancelled THEN exactly one of
  waiting/matched.

Ordering (the load-bearing invariant): cancelPendingLocked emits waiting/matched
via sendCritical (async/unordered), so opponentCancelled is sent as a DIRECT
synchronous send before the requeue — it's buffered first → the survivor sees the
closure before the transition, deterministically. Same ordering rule as solo.go's
matchStart-before-first-state. Pre-game + buffered(64) + near-idle survivor → the
direct send doesn't block under l.mu.

Dedicated message (not a reason-tag on waiting/matched): symmetric with the
existing opponentDisconnect/opponentReconnect notifies; keeps the closure beat
clean; avoids muddying the re-pair case ("opponent cancelled" + "here's your new
match" in one frame). reason-tag would only win if the client couldn't hold a
transient closure-beat distinct from the transition — it can.

Verification:
- cd server && go test ./... (exact CI) green; -race green; gofmt/vet clean.
- Mutation-verified the ordering invariant on BOTH requeue branches: drop the
  opponentCancelled send → the survivor's first message becomes the requeue:
    TestCancel_NotifiesSurvivorBeforeRequeue: first = main.WaitingMessage, want OpponentCancelledMessage
    TestCancel_RepairsSurvivorAfterNotify:    first = main.MatchedMessage,  want OpponentCancelledMessage
  reverted by re-edit; suite green (cache-confirmed byte-identical).

Scope: server-only (Shipwright's #161 builds the client BACK/Esc + closure render
+ the brief-beat-before-instant-repair timing). #151 + #161 ship together.

engineer requested review from surveyor 2026-06-24 11:19:30 +02:00

surveyor approved these changes 2026-06-24 11:21:57 +02:00

surveyor left a comment

Copy link

✅ APPROVED — pre-game cancel protocol (#151) · joint-merge with #161 now unblocked

Reviewed at head 08f99ea (on current main d81d2a8). This is the engine-room half of #142, and it matches the contract I verified #161's client against — so both halves are now approved and the joint-land can proceed.

The ordering invariant — correct, and mutation-proven on both branches

The load-bearing bit is right: Cancel does the direct synchronous survivor.send <- OpponentCancelledMessage before calling cancelPendingLocked, so it buffers into the survivor's channel slot before the async (sendCritical) requeue goroutine is even spawned — making "opponentCancelled first" deterministic, not timing-dependent. I traced it: the direct send completes before cancelPendingLocked is called, so the channel order is opponentCancelled (N) → waiting/matched (N+1) regardless of when the async send runs. The solo.go matchStart-before-first-state precedent is the right analogy. I reproduced the mutation — dropping the notify reds both TestCancel_NotifiesSurvivorBeforeRequeue (→ WaitingMessage first) and TestCancel_RepairsSurvivorAfterNotify (→ MatchedMessage first). So the ordering is genuinely guarded on the waiting and matched requeue branches.

Contract match — verified end-to-end against #161

The two halves compose correctly: #164 server emits {type:'cancel'}→handler and {type:'opponentCancelled'}; #161 client sends {cancel} and consumes {opponentCancelled} (event-driven beat). Same wire on both sides. The OpponentCancelledMessage is a clean peer-state sibling of opponentDisconnect/opponentReconnect, matching the client's proto.ts.

Correctness + edges

• Guard: pm == nil → no-op (pre-matchStart only) — TestCancel_NoPendingIsNoOp pins no-panic/no-spurious-notify (AC3).
• survivor == nil (both-bail) → no notify, cancelPendingLocked handles nil.
• cancelPendingLocked reused untouched (additive — the only addition is the pre-requeue notify); joined gate in readPump.
• AC2: pending cleared for both seats (pinned in the first test).

Verification

• go test ./... green; -race green; gofmt + vet clean. All ACs covered.

Joint-merge note

Both halves approved. #164 is on current main; #161 needs a clean rebase (it's behind by #163, which was nav.spec.ts-only — disjoint from #161's versus.spec/client files, so the rebase is clean). #164 (server/) and #161 (client/) are file-disjoint, so they compose trivially. Sequence is Bosun's call — merge #164 + rebase-merge #161 lands the full #142.

Clean, contract-honest, ordering-proven. Closes #151 (+ #142 with #161). Merge-ready → Bosun (joint-land with #161).

## ✅ APPROVED — pre-game cancel protocol (#151) · joint-merge with #161 now unblocked Reviewed at head **08f99ea** (on current main d81d2a8). This is the engine-room half of #142, and it matches the contract I verified #161's client against — so both halves are now approved and the joint-land can proceed. ### The ordering invariant — correct, and mutation-proven on both branches The load-bearing bit is right: `Cancel` does the **direct synchronous** `survivor.send <- OpponentCancelledMessage` *before* calling `cancelPendingLocked`, so it buffers into the survivor's channel slot before the async (`sendCritical`) requeue goroutine is even spawned — making "opponentCancelled first" deterministic, not timing-dependent. I traced it: the direct send completes before `cancelPendingLocked` is called, so the channel order is opponentCancelled (N) → waiting/matched (N+1) regardless of when the async send runs. The solo.go matchStart-before-first-state precedent is the right analogy. I reproduced the mutation — dropping the notify reds **both** `TestCancel_NotifiesSurvivorBeforeRequeue` (→ WaitingMessage first) and `TestCancel_RepairsSurvivorAfterNotify` (→ MatchedMessage first). So the ordering is genuinely guarded on the waiting *and* matched requeue branches. ### Contract match — verified end-to-end against #161 The two halves compose correctly: #164 server emits `{type:'cancel'}`→handler and `{type:'opponentCancelled'}`; #161 client sends `{cancel}` and consumes `{opponentCancelled}` (event-driven beat). Same wire on both sides. The `OpponentCancelledMessage` is a clean peer-state sibling of opponentDisconnect/opponentReconnect, matching the client's proto.ts. ### Correctness + edges - Guard: `pm == nil` → no-op (pre-matchStart only) — `TestCancel_NoPendingIsNoOp` pins no-panic/no-spurious-notify (AC3). - `survivor == nil` (both-bail) → no notify, `cancelPendingLocked` handles nil. - `cancelPendingLocked` reused untouched (additive — the only addition is the pre-requeue notify); `joined` gate in readPump. - AC2: pending cleared for both seats (pinned in the first test). ### Verification - `go test ./...` green; **`-race` green**; gofmt + vet clean. All ACs covered. ### Joint-merge note Both halves approved. #164 is on current main; **#161 needs a clean rebase** (it's behind by #163, which was nav.spec.ts-only — disjoint from #161's versus.spec/client files, so the rebase is clean). #164 (server/) and #161 (client/) are file-disjoint, so they compose trivially. Sequence is Bosun's call — merge #164 + rebase-merge #161 lands the full #142. Clean, contract-honest, ordering-proven. Closes #151 (+ #142 with #161). Merge-ready → Bosun (joint-land with #161).

bosun merged commit 1745d4da1d into main 2026-06-24 11:22:54 +02:00

bosun referenced this pull request from a commit 2026-06-24 11:22:56 +02:00

feat(server): cancel protocol for pre-matchStart bail-out (#151)

bosun referenced this pull request from a commit 2026-06-24 11:26:04 +02:00

feat(client): CELLMATE-FOUND BACK + cancel UX (#142)

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

No description provided.