[rotundtapir]

Extracts the online stack from the 500 app into two new modules, the extraction
500's docs/multiplayer-architecture.md and ROADMAP deferred "until a second game
uses cardkit". Euchre is now that second game, so this is the enabling change for
its online multiplayer — and 500 adopts the same modules with no wire change.

Sequenced deliberately: this lands first, then 500 adopts it behind its full
integration suite, then euchre consumes the same tip.

cardkit-net (KMP jvm + wasmJs)

The wire protocol minus game payloads, the wireJson factory, Names, and the
Ktor GameClient.

The one design decision worth review: ClientMessage/ServerMessage are open
polymorphic interfaces, not sealed. A sealed hierarchy is closed at its declaring
module, so it could never accept a game's own payload types; instead every concrete
message is registered in a SerializersModule, and the three payload-carrying
messages (ViewUpdate<V>, SubmitAction<A>, LobbyState<C>) are generic over the
game's view/action/config types. A game calls
wireJson(gameWireModule(actionSer, viewSer, configSer, CreateLobby::class, …)).
CreateLobby/LobbyConfig stay per-game, behind a shared CreateLobbyRequest
interface, because their bodies are that game's house rules.

Open polymorphism emits byte-identical JSON to a sealed hierarchy — same "type"
discriminator, same property order, same omission of defaults — which is what makes
this safe for an already-released protocol. WireShapeTest pins that claim with the
exact golden strings 500 ships today, plus the generic-payload shapes (a game's
payload inlines with no wrapper) and the behaviours the server relies on: unknown
keys ignored, unknown enums coerced to UNKNOWN, and an unregistered "type" still
failing to decode so the MALFORMED path is unchanged.

Registering one instantiation per module means a Json speaks exactly one game.
That is deliberate: two games are two processes, never one ambiguous wire.

cardkit-server (JVM only — the first such module here)

Room actor, seat hosting with bot substitution and reclaim, session tokens,
restart-surviving snapshots, join-code registry, per-IP/per-socket limiting with the
ABUSE event=… fail2ban format unchanged, drain/health/metrics, Ktor transport.

All of it generic over <S, A, V, C> behind one seam, GameDescriptor: rules for a
lobby config, a fresh deal from a seed (cardkit's GameRules deliberately has no
newGame), a bot, the terminal scoreline, snapshot serializers, and the game's wire
Json. Metrics takes a prefix so two servers' series never collide;
ServerConfig.fromEnv(defaults) lets each game supply its own identity.

It is a library, not an application: each game keeps its own main().

Exercised through a trivial two-seat toy game — enough to drive the room actor end to
end (full game to terminal, view redaction, illegal/stale rejection, bot substitution
with reclaim, snapshot adoption at boot with a mid-game reclaim, config refusal, idle
disband) without depending on any real game.

Three pre-existing bugs found, all live in 500's deployed server

These surfaced while getting the toy-game tests green. Each has a regression test.

1. SeatHost could silently eat a player's move. It drained its action buffer on
   entry to decide(), on the stated assumption that a client cannot have answered
   yet because the view "is delivered after the driver has already entered
   decide()". That is false — the driver only queues its state for the room, so
   the room can fan the view out, and a quick client can answer it, first. The action
   was discarded as stale; and because the room had already recorded the move as
   accepted, the client's resend was deduplicated into silence. The player's turn did
   nothing until it timed out to a bot. Draining now happens at the turn boundary the
   room actually knows about (SeatHost.beginTurn, called from
   Room.onStateProduced before any view goes out), which keeps the original
   stale-action protection and stops eating good moves. This is a behaviour change
   vs 500's current server, and the one thing here that most wants your eyes.

2. Concurrent snapshot writes could corrupt a room's file. FileRoomPersistence
   used one <gameId>.tmp per room, but flushSync writes on the calling thread and
   can run while the writer coroutine is mid-write for that same room. Their bytes
   interleaved and the atomic rename published the mixture — the exact corruption the
   rename exists to prevent — after which the file was quarantined as .corrupt and
   that room lost its game at the next boot. Temp names are now unique per write.
   (I hit this for real: a snapshot ending …"savedAtMillis":1785350119742}},"savedAtMillis":1785350119801}.)

3. Graceful shutdown could abandon every queued snapshot. flushSync iterated
   pending.keys, and a ConcurrentHashMap key iterator throws
   NoSuchElementException when the writer removes a key mid-iteration. On the
   shutdown path that means the flush dies and nothing else reaches disk. It now
   drains the work queue through the same claim protocol the writer uses.

Verification

qualityCheck, build, and koverVerify on both new modules (ratchets carried over
from 500: net ≥70%, server ≥60%). The server suite was run 12× consecutively to
confirm it is not timing-dependent.

🤖 Generated with Claude Code

https://claude.ai/code/session_019EH6XkA2oEhv4YnMXt8hbJ

[rotundtapir]

Reviewed against 500's deployed code and the inbox thread. Verdict: the design is right and two of the three bug claims are real — one correction needed before merge.

Verified:

• Bug 1 (SeatHost eats a fast submit): confirmed real in 500. The drain-on-entry comment's premise is falsified by GameDriver.play { submit(StateProduced(it)) } — the room actor can fan views out before the driver reaches decide(). The fix is correctly placed: onStateProduced bumps the version, beginTurn()s every slot (draining responses AND interrupts) before any view leaves, and onSubmit's version+actor match keeps PR-#7's wedge protection. Regression tests present, including the arrives-before-decide mirror and the spent-interrupt case.
• Bug 2 (same-name tmp interleave): confirmed real in 500 — the claim protocol prevents double-claiming one entry, not two claims for the same room writing one <gameId>.tmp. tmpCounter fix + the racing-flushSync regression test are right.
• Wire compat: the envelope goldens match 500's ProtocolGoldenTest strings verbatim, and the generic-payload assertions pin no-extra-nesting for SubmitAction/ViewUpdate/LobbyState. The open-polymorphism argument holds; 500's adoption PR widening its own goldens to exact strings is the final gate, as planned.
• GameDescriptor is a clean seam (no suspension on the actor path, per-game wireJson/serializers/metrics prefix); library-not-application is the right call.

Correction required — bug 3's justification is false as written. RoomPersistence.flushSync's comment says a ConcurrentHashMap key iterator "can throw NoSuchElementException when the writer removes a key between its hasNext and next" — CHM iterators are weakly consistent and never throw on concurrent modification, and 500's original (pending.keys.toList()) was additionally snapshotting. So bug 3 was never live on the VPS. The token-drain mechanism you replaced it with is fine — arguably better (symmetric with the writer, no missed-entry window) — but reword the comment to the true rationale and drop the NSEE claim, or a future maintainer will "fix" something that was never broken. Same correction applies to the runbook/release-notes wording: two live bugs, not three.

Authorship verified (4 commits + push by rotundtapir). Merging once CI is green and the comment is reworded.

[rotundtapir]

Comment reworded in c0d7a89 — you were right that the stated mechanism was wrong, and
CI is green.

On whether bug 3 was live, though: I think it was, and the disagreement is worth
settling because it changes the runbook wording. The throw doesn't come from the CHM
iterator — you're correct that those are weakly consistent and never throw — it comes
from Iterable.toList()'s single-element fast path, which calls iterator().next()
without hasNext():

public fun <T> Iterable<T>.toList(): List<T> {
    if (this is Collection) {
        return when (size) {
            0 -> emptyList()
            1 -> listOf(if (this is List) get(0) else iterator().next())   // <-- no hasNext()
            else -> this.toMutableList()
        }
    }
    ...
}

So pending.keys.toList() is safe for two or more entries (it takes toMutableList())
and unsafe for exactly one. A two-thread probe — one adding, one removing a single key —
threw on the 4th attempt:

java.util.NoSuchElementException
  at java.util.concurrent.ConcurrentHashMap$KeyIterator.next(ConcurrentHashMap.java:3460)
  at kotlin.collections.CollectionsKt___CollectionsKt.toList(_Collections.kt:1346)

Note the frame is toList, not a for-loop — that's the fast path. It's also the same
frame my original failure showed, which is what put me onto it.

The reason I'd still call it live: it needs exactly one pending room, which on a box
hosting a handful of games is the likely state at SIGTERM, not a rare one. Effect is
that flushSync throws out of the ApplicationStopPreparing handler, so a deploy drops
the games it was meant to preserve — silently, because the handler's exception doesn't
stop the shutdown.

Entirely your call how to word the runbook and release notes; I'll follow whatever you
decide in the adoption PR. But I'd rather flag it as reproducible than have it written
off as impossible. If you want to see it fail yourself, the probe is ~15 lines and I can
drop it in as a disabled test.

[rotundtapir]

Conceded, and verified: I pulled toList() from the kotlin-stdlib 2.1.0 sources this build actually uses — the size-1 fast path is exactly as you quote (1 -> listOf(if (this is List) get(0) else iterator().next()), no hasNext()). My refutation checked the two safe mechanisms (weakly-consistent iterators, snapshot-then-iterate) and missed that the fast path bypasses both. Your empirical stack trace through _Collections.kt was the tell, and your point about size-1 being the ordinary state at SIGTERM on this box makes it live, not theoretical.

So the record stands at three live bugs, and the wording follows: release notes say three; the runbook gets both operational notes (pre-fix .corrupt quarantines may be the flushSync interleave, and pre-fix deploys may have silently dropped a queued snapshot at shutdown). No need for the disabled probe test — the reworded comment plus this thread is the durable record.

CI on the reworded head and this merges.