diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e3df83..1b3c15b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,74 @@ page as `v0.1.0 · · `, so what is deployed can always be identifie --- +## 0.8.4 — 2026-09-29 + +The second release from the audit: the multiplayer transport, server and browser. Every fault here +was invisible in solitaire, and four of the five server faults were in the one file no test had +ever stood up — `http.ts` now has an end-to-end suite that binds a real port. + +### A player who left could play the seat the next arrival took + +Leaving a lobby freed the chair and kept the token: it stayed in memory and on disk, and the next +player to join was given the vacated chair. So once the game began, the leaver's browser still held +a token for that seat — `/api/stream` served it the newcomer's hand and menu, `/api/intent` let it +move for them, and its stream connection displaced theirs. `lobby-and-sessions.md` had said all +along that Leave "drops the token"; the code did not. It does now, for a player's own leave and for +the host's remove alike, in memory and in `sessions.json`, before the chair is offered to anyone. + +### The first move after a reload could be silently swallowed + +The browser numbered its intents from 1 on every page load; the server remembers a seat's last +accepted number for the life of the game and answers a repeat with "already applied". A seat that +had made one move, reloaded, and clicked again sent `seq: 1` twice — ok, nothing happened, nothing +pushed, the click looked dead. The connect push now carries the server's count (`lastSeq`) and the +client continues from it, never backwards. + +### Two moves at once could tear the save, and a torn save took every game down + +Nothing serialised moves within a game: the handler awaits the disk write between applying and +answering, and two moves arriving together interleaved across it — both applied in memory, both +writing the same `game.json.tmp`. Measured at 200 rounds of two concurrent writes: every round lost +one to `rename` ENOENT, six left the file as invalid JSON. And the boot did a bare `JSON.parse` on +each save at the top level, so one such file was a crash loop with every game on the server +unreachable. Three fixes, each pinned: every write to a path queues behind the one before it with a +unique temp name; each game's moves run one at a time through apply, persist, answer, broadcast; and +an unreadable save is logged and skipped rather than fatal, as is one whose replay throws. + +### One error after the SSE head was sent was a whole-server crash + +The handler's one `catch` answered every error with a JSON 500 — and on a response whose head was +already written (both streams, static files) `writeHead` throws inside the catch, with nothing above +it. Node exits on an unhandled rejection. `sendJson` now ends such a response instead; the lobby's +host-reassignment write and the static file stream have their own error paths; and the 500 no longer +echoes the error's message, which for a disk error carried the data directory's absolute path. + +### Anyone could exhaust the process's memory with one POST + +Request bodies were buffered whole, with no cap, before any secret was checked. A 64 KiB limit — +the largest body any route has a use for is a few hundred bytes — answers 413; a body that is not a +JSON object answers 400 rather than surfacing as a 500. + +### In the browser + +**A double-click did the thing twice.** The page redraws the same menu the instant a submit is sent, +so a second click before the round trip posted a second, fresh `seq` for the same option, and the +server applied it again: two cards drawn, two Moves spent, two cars coupled. One submit in flight at +a time now; a click that lands during one is dropped, and the push is milliseconds away. A network +failure or a non-JSON answer is `false` from `submit` rather than an unhandled rejection. + +**The documentation renderer flattened nested bullets.** The comment said nesting was rendered by +recursion; the code appended the nested bullet to its parent as text, and the published home-deck +page read "…knows. - ABS Signals is the exception…" with a literal dash mid-sentence. Real nesting +now, and the test renders the real document to prove the dash is gone. + +**"Still needs 2 boxcar" behind a caboose.** The make-up panel counted by category and promised cars +the engine would refuse: nothing couples behind a caboose (§A.3), so it printed a need while no yard +chip lit and the only offer was to send the train out as it stands. It asks `acceptsCar` per +category now, the way the engine's own make-up report does, and says why when nothing more couples. + +--- + ## 0.8.3 — 2026-09-29 The first of three releases from a code audit (engine, server, client, tests and hygiene, each read diff --git a/docs/architecture/lobby-and-sessions.md b/docs/architecture/lobby-and-sessions.md index f5be297..ee633b1 100644 --- a/docs/architecture/lobby-and-sessions.md +++ b/docs/architecture/lobby-and-sessions.md @@ -107,7 +107,9 @@ clashing join is refused (`NAME_TAKEN`, compared trimmed and case-insensitively) suffixed: a player should play under the name they chose, or be asked for another. **Anybody may leave, and the host may clear a chair.** `Lobby.Leave` frees the seat, drops the token -from `joinOrder`, and passes host rights on exactly as a dropped connection does. Naming somebody +from `joinOrder`, **revokes it** — in memory and in `sessions.json`, since v0.8.4; until then the +leaver's token still opened the seat the next arrival took — and passes host rights on exactly as a +dropped connection does. Naming somebody else's `seat` is host-only. When the last human leaves, the lobby is deleted outright — code, file and index row — rather than left as a table of bots waiting for a host who no longer exists. Before this existed a mis-join or a player who wandered off wedged the whole table, since Start needs every chair diff --git a/docs/architecture/protocol.md b/docs/architecture/protocol.md index 10846dd..5368999 100644 --- a/docs/architecture/protocol.md +++ b/docs/architecture/protocol.md @@ -151,8 +151,14 @@ multiplayer work: everything else degrades gracefully, a redaction bug hands a p - Events carry a monotonic sequence per game. Clients apply strictly in order and request a replay on a gap rather than guessing. - Intents carry a client `seq`. The server ignores a repeat of one it has already applied, so a - reconnecting client can safely resend anything it is unsure about. + reconnecting client can safely resend anything it is unsure about. **The count is the server's, + for the life of the game**: the connect push carries the seat's last accepted `seq` (`lastSeq`) + and the client continues from it, never from 1 — a page that restarted its own count after a + reload re-sent a number the server had already applied, and the move was silently swallowed as + a resend (v0.8.4). - **The server never applies two intents concurrently within a game.** A per-game queue is sufficient - and there is nothing cleverer to do at this scale. Note this is a serialisation rule, not a + and there is nothing cleverer to do at this scale. It is a real queue (`http.ts`'s `inTurn`), not a + reliance on the single thread: the handler awaits the disk write between applying and answering, + and two moves arriving together used to interleave across that await (v0.8.4). Note this is a serialisation rule, not a one-actor-at-a-time rule: per-player turn state (`turns: Map`) means several players may hold an open turn at once, and their intents still land one at a time. diff --git a/docs/components.md b/docs/components.md index 746102f..45e0bf5 100644 --- a/docs/components.md +++ b/docs/components.md @@ -1,6 +1,6 @@ # Station Master — Components and Markers -**Version 0.8.3** · 2026-09-29 +**Version 0.8.4** · 2026-09-29 **Scope:** non-card physical components and supplies. Card-created facilities, workers, deck piles, hand state, timetable state and other markers are documented with their cards or in the diff --git a/docs/home-deck.md b/docs/home-deck.md index 83004b0..1f81cdd 100644 --- a/docs/home-deck.md +++ b/docs/home-deck.md @@ -1,6 +1,6 @@ # Station Master — Home Deck -**Version 0.8.3** · 2026-09-29 +**Version 0.8.4** · 2026-09-29 **Scope:** the Home Office deck — how it is dealt, drawn, discarded and reshuffled, and what the rules are for playing each kind of card out of it. diff --git a/docs/mainline-deck.md b/docs/mainline-deck.md index 1230445..0db7d25 100644 --- a/docs/mainline-deck.md +++ b/docs/mainline-deck.md @@ -1,6 +1,6 @@ # Station Master — Mainline Deck -**Version 0.8.3** · 2026-09-29 +**Version 0.8.4** · 2026-09-29 **Scope:** the tarot-sized Mainline cards placed between Offices — how the deck is dealt, what a card does to a train crossing it, and the Home Deck cards played onto one. diff --git a/docs/quickstart.md b/docs/quickstart.md index 5b7626c..94ff7d3 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -1,6 +1,6 @@ # Station Master — Quickstart -**Version 0.8.3** · 2026-09-29 +**Version 0.8.4** · 2026-09-29 For a player who has never played. diff --git a/docs/rules.md b/docs/rules.md index 07a2c65..9495ad0 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -1,6 +1,6 @@ # Station Master — Rules -**Version 0.8.3** · 2026-09-29 +**Version 0.8.4** · 2026-09-29 **Authority:** observed code paths and tests. Where a card face, a prototype document and executable behaviour differ, this document reports **executable behaviour** and marks unimplemented material. diff --git a/package.json b/package.json index c16d63d..330bd45 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "station-master", - "version": "0.8.3", + "version": "0.8.4", "private": true, "type": "module", "description": "Station Master — a railroad operations game", diff --git a/scripts/markdown.ts b/scripts/markdown.ts index 91ed816..7ae1ef7 100644 --- a/scripts/markdown.ts +++ b/scripts/markdown.ts @@ -109,6 +109,39 @@ export type Rendered = { html: string; headings: Heading[] }; * `linkHref` rewrites link targets — the build uses it to send `rules.md` to `rules.html` — and * defaults to leaving them alone so the function is testable on its own. */ +/** + * One list at one indent level. An item is its first line plus any lines indented under it; the + * more-indented BULLETS among those are the item's own nested list and render recursively, while + * plain indented lines are wrapped continuations of its text. + */ +function listHtml(block: string[], text: (s: string) => string): string { + const first = /^(\s*)([-*+]|\d+[.)])\s+/.exec(block.find((l) => l.trim() !== '') ?? ''); + if (!first) return ''; + const ordered = /\d/.test(first[2]!); + const base = first[1]!.length; + const items: { text: string[]; sub: string[] }[] = []; + for (const l of block) { + const m = /^(\s*)([-*+]|\d+[.)])\s+(.*)$/.exec(l); + const current = items[items.length - 1]; + if (m && m[1]!.length <= base) { + items.push({ text: [m[3]!], sub: [] }); + } else if (!current) { + continue; + } else if (current.sub.length > 0 || (m && m[1]!.length > base)) { + // Once a nested list has begun, everything further belongs to it, wrapped lines included. + current.sub.push(l); + } else if (l.trim() !== '') { + current.text.push(l.trim()); + } + } + const tag = ordered ? 'ol' : 'ul'; + return ( + `<${tag}>` + + items.map((it) => `
  • ${text(it.text.join(' '))}${it.sub.length > 0 ? listHtml(it.sub, text) : ''}
    • `).join('') + + `` + ); +} + export function renderMarkdown(src: string, linkHref: (href: string) => string = (h) => h): Rendered { const lines = src.replace(/\r\n/g, '\n').split('\n'); const out: string[] = []; @@ -196,13 +229,14 @@ export function renderMarkdown(src: string, linkHref: (href: string) => string = continue; } - // Lists. A bullet or a number opens one; continuation lines are indented under their item. + // Lists. A bullet or a number opens one; continuation lines are indented under their item, and + // a more-indented bullet under an item is a nested list, rendered by recursion (v0.8.4 — the + // comment said so before and the code appended the nested bullet to its parent as text, so the + // published home-deck page carried a literal "- " mid-sentence). const bullet = /^(\s*)([-*+]|\d+[.)])\s+(.*)$/.exec(line); if (bullet) { - const ordered = /\d/.test(bullet[2]!); const baseIndent = bullet[1]!.length; - const items: string[] = []; - let current: string[] | null = null; + const block: string[] = []; while (i < lines.length) { const l = lines[i]!; if (l.trim() === '') { @@ -210,28 +244,19 @@ export function renderMarkdown(src: string, linkHref: (href: string) => string = const next = lines[i + 1] ?? ''; const continues = /^(\s*)([-*+]|\d+[.)])\s+/.test(next) || /^\s{2,}\S/.test(next); if (!continues) break; + block.push(''); i++; continue; } - const m = /^(\s*)([-*+]|\d+[.)])\s+(.*)$/.exec(l); - if (m && m[1]!.length <= baseIndent) { - if (current) items.push(current.join(' ')); - current = [m[3]!]; - i++; - continue; - } - if (m || /^\s{2,}\S/.test(l)) { - // A nested item or a wrapped continuation. Nesting is rendered by recursion on the block. - if (!current) break; - current.push(l.trim()); + const m = /^(\s*)([-*+]|\d+[.)])\s+/.exec(l); + if ((m && m[1]!.length <= baseIndent) || (m && m[1]!.length > baseIndent) || /^\s{2,}\S/.test(l)) { + block.push(l); i++; continue; } break; } - if (current) items.push(current.join(' ')); - const tag = ordered ? 'ol' : 'ul'; - ln(`<${tag}>${items.map((it) => `
  • ${text(it)}
    • `).join('')}`); + ln(listHtml(block, text)); continue; } diff --git a/src/server/http.ts b/src/server/http.ts index 3751929..90e63ec 100644 --- a/src/server/http.ts +++ b/src/server/http.ts @@ -18,7 +18,7 @@ */ import { createServer } from 'node:http'; -import type { IncomingMessage, ServerResponse } from 'node:http'; +import type { IncomingMessage, Server, ServerResponse } from 'node:http'; import { createReadStream } from 'node:fs'; import { stat } from 'node:fs/promises'; import { extname, join, normalize } from 'node:path'; @@ -97,14 +97,57 @@ const MIME: Record = { const HEARTBEAT_MS = 20_000; -async function readJson(req: IncomingMessage): Promise { +/** + * THE LARGEST BODY ANY ROUTE HERE HAS A USE FOR, with room to spare (v0.8.4). The biggest thing a + * client sends is a `GameConfig` on `/api/lobby/create` — a few hundred bytes. `readJson` used to + * buffer whatever arrived, before any secret was checked, so anyone who could reach the port could + * exhaust the process's memory with one POST. D14 expects this box to be reachable. + */ +const MAX_BODY_BYTES = 64 * 1024; + +/** A refusal with a status — thrown from anywhere in a handler and answered by the one catch. */ +class HttpError extends Error { + readonly status: number; + constructor(status: number, message: string) { + super(message); + this.status = status; + } +} + +async function readJson(req: IncomingMessage): Promise> { + const declared = Number(req.headers['content-length'] ?? 0); + if (declared > MAX_BODY_BYTES) throw new HttpError(413, 'body too large'); const chunks: Buffer[] = []; - for await (const chunk of req) chunks.push(chunk as Buffer); + let size = 0; + for await (const chunk of req) { + size += (chunk as Buffer).length; + if (size > MAX_BODY_BYTES) throw new HttpError(413, 'body too large'); + chunks.push(chunk as Buffer); + } const text = Buffer.concat(chunks).toString('utf8'); - return text.trim() === '' ? {} : JSON.parse(text); + if (text.trim() === '') return {}; + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + // A malformed body is the caller's mistake, answered as one — it used to surface as a 500. + throw new HttpError(400, 'body is not JSON'); + } + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) throw new HttpError(400, 'expected a JSON object'); + return parsed as Record; } function sendJson(res: ServerResponse, status: number, body: unknown): void { + /** + * NEVER A SECOND HEAD (v0.8.4). The SSE routes and `serveStatic` write their head and go on; a + * throw after that reached the handler's catch, which called this, and `writeHead` on a response + * whose head was sent throws `ERR_HTTP_HEADERS_SENT` — inside a `.catch`, with nothing above it, + * so Node exited on the unhandled rejection. One bad stream write was a whole-server crash. + */ + if (res.headersSent) { + res.end(); + return; + } const text = JSON.stringify(body); res.writeHead(status, { 'Content-Type': 'application/json; charset=utf-8', 'Content-Length': Buffer.byteLength(text) }); res.end(text); @@ -149,7 +192,11 @@ async function serveStatic( 'Content-Length': info.size, 'Cache-Control': buildTagged ? 'public, max-age=31536000, immutable' : 'no-cache', }); - createReadStream(full).pipe(res); + // A read that fails mid-stream (file replaced by a deploy, disk error) closes the response rather + // than raising an error nothing listens for. + createReadStream(full) + .on('error', () => res.destroy()) + .pipe(res); } catch { res.writeHead(404, { 'Content-Type': 'text/plain' }); res.end('not found'); @@ -175,7 +222,8 @@ type LobbyPreview = { seated: { seat: number; who: string | null; bot: boolean }[]; }; -export function startServer(opts: ServerOptions): void { +/** Returns the listening server — a test binds port 0 and reads the port back off it. */ +export function startServer(opts: ServerOptions): Server { const games = opts.initialGames; const lobbies = opts.initialLobbies; const sessions = opts.initialSessions; @@ -283,8 +331,29 @@ export function startServer(opts: ServerOptions): void { async function persistSession(ps: PlayerSession): Promise { sessions.set(ps.token, ps); - const all = [...sessions.values()].filter((s) => s.gameId === ps.gameId); - await writeSessions(opts.dataDir, ps.gameId, all); + await persistSessionsOf(ps.gameId); + } + + /** Rewrites one game's `sessions.json` from what is in memory — after a token is revoked as much + * as after one is issued, or a restart would hand the seat back to a browser that left it. */ + async function persistSessionsOf(gameId: string): Promise { + const all = [...sessions.values()].filter((s) => s.gameId === gameId); + await writeSessions(opts.dataDir, gameId, all); + } + + /** + * ONE INTENT AT A TIME PER GAME — `protocol.md` §5's serialisation rule, which the intent handler + * relied on Node's single thread to keep and did not (v0.8.4): the `await` on the disk write is + * an interleaving point, so two moves arriving together could both apply in memory, race their + * writes to one `.tmp`, and hand their broadcasts to the clients in the wrong order. Everything a + * move does — apply, persist, answer, broadcast — now runs as one unit behind the move before it. + */ + const gameQueues = new Map>(); + function inTurn(gameId: string, fn: () => Promise): Promise { + const prev = gameQueues.get(gameId) ?? Promise.resolve(); + const next = prev.then(fn, fn); + gameQueues.set(gameId, next.catch(() => undefined)); + return next; } const server = createServer((req, res) => { @@ -571,6 +640,17 @@ export function startServer(opts: ServerOptions): void { sendJson(res, 403, { error: 'NOT_HOST' }); return; } + /** + * THE SEAT GOES, AND SO DOES THE TOKEN THAT HELD IT (v0.8.4). + * + * Leaving used to free the chair and keep the session: the token stayed in `sessions` and + * on disk, and `joinLobby` hands a vacated chair to the next arrival. So a player who left + * (or was removed) still held a token for seat N, and once somebody else sat in seat N and + * the game began, `/api/stream` served that token the new occupant's hand and `/api/intent` + * let it move for them — and its stream connection displaced theirs. Revoked here, before + * the chair is offered to anyone. + */ + const occupant = seat === undefined ? ps : [...sessions.values()].find((s) => s.gameId === lobby.gameId && s.player === seat); const result = leaveLobby(lobby, ps.token, seat); if (result.empty) { // Nobody human is left to start it. Everything about this lobby goes, including the code, @@ -579,6 +659,7 @@ export function startServer(opts: ServerOptions): void { gameCodes.delete(lobby.gameCode); for (const [, watcher] of lobbyConnections.get(lobby.gameId) ?? []) watcher.end(); lobbyConnections.delete(lobby.gameId); + for (const [token, s] of [...sessions]) if (s.gameId === lobby.gameId) sessions.delete(token); await deleteLobby(opts.dataDir, lobby.gameId); // The row goes with the lobby rather than being marked: a game that never started is not a // game an administrator has any use for a record of. @@ -586,6 +667,12 @@ export function startServer(opts: ServerOptions): void { sendJson(res, 200, { ok: true, closed: true }); return; } + if (occupant && result.lobby !== lobby) { + sessions.delete(occupant.token); + lobbyConnections.get(lobby.gameId)?.get(occupant.token)?.end(); + lobbyConnections.get(lobby.gameId)?.delete(occupant.token); + await persistSessionsOf(lobby.gameId); + } await persistLobby(result.lobby); broadcastLobby(lobby.gameId); sendJson(res, 200, { ok: true }); @@ -690,7 +777,9 @@ export function startServer(opts: ServerOptions): void { // `lobby`, because it may have changed (another join, another bot toggle) since connect. const current = lobbies.get(lobby.gameId); if (current && current.hostToken === token) { - void persistLobby(reassignHost(current, token)).then(() => broadcastLobby(lobby.gameId)); + persistLobby(reassignHost(current, token)) + .then(() => broadcastLobby(lobby.gameId)) + .catch((err: unknown) => console.error(`reassigning the host of ${lobby.gameId} failed:`, err)); } }); return; @@ -820,31 +909,49 @@ export function startServer(opts: ServerOptions): void { sendJson(res, 400, { error: 'expected { seq, intent }' }); return; } - const result = session.intent(ps.player, body.seq, body.intent); - if (result.accepted) { - // Persisted BEFORE the response goes out — "accepted" should mean "durably on disk" at - // this scale, not just "applied in memory" (§12 step 14). - const dir = gameDir(opts.dataDir, ps.gameId); - await writeGame(dir, session.exportSave(), opts.engineVersion); - if (result.timing) await appendTiming(dir, result.timing); - if (session.exportSave().status === 'finished') { - // `upsertIndexEntry` replaces the WHOLE row for this `gameId`, so the code has to be - // carried forward here rather than left blank — `gameCodes` is the only place still - // holding it once a lobby's own record is gone. - const gameCode = [...gameCodes.entries()].find(([, id]) => id === ps.gameId)?.[0] ?? ''; - await upsertIndexEntry(opts.dataDir, { gameId: ps.gameId, gameCode, status: 'finished' }); + const { seq, intent } = body; + await inTurn(ps.gameId, async () => { + const result = session.intent(ps.player, seq, intent); + if (result.accepted) { + // Persisted BEFORE the response goes out — "accepted" should mean "durably on disk" at + // this scale, not just "applied in memory" (§12 step 14). + const dir = gameDir(opts.dataDir, ps.gameId); + try { + await writeGame(dir, session.exportSave(), opts.engineVersion); + if (result.timing) await appendTiming(dir, result.timing); + if (session.exportSave().status === 'finished') { + // `upsertIndexEntry` replaces the WHOLE row for this `gameId`, so the code has to be + // carried forward here rather than left blank — `gameCodes` is the only place still + // holding it once a lobby's own record is gone. + const gameCode = [...gameCodes.entries()].find(([, id]) => id === ps.gameId)?.[0] ?? ''; + await upsertIndexEntry(opts.dataDir, { gameId: ps.gameId, gameCode, status: 'finished' }); + } + } catch (err) { + // The move IS applied — every seat's game has moved on — so the table is told and the + // disk's failure is the operator's to see in the log. Answering 500 here told the one + // player who moved that their move was refused, while everyone else watched it happen. + console.error(`persisting ${ps.gameId} after a move failed:`, err); + } } - } - sendJson(res, 200, result.accepted ? { ok: true } : { ok: false, code: result.code }); - if (result.accepted) broadcastGame(ps.gameId, result.pushes); + sendJson(res, 200, result.accepted ? { ok: true } : { ok: false, code: result.code }); + if (result.accepted) broadcastGame(ps.gameId, result.pushes); + }); return; } await serveStatic(opts.distDir, url.pathname, res, url.searchParams.has('v')); })().catch((err: unknown) => { - sendJson(res, 500, { error: err instanceof Error ? err.message : 'internal error' }); + if (err instanceof HttpError) { + sendJson(res, err.status, { error: err.message }); + return; + } + // Logged here, not echoed: an fs error's message carries the absolute path of the data + // directory, which is the operator's to know and not the caller's. + console.error(`${req.method ?? ''} ${req.url ?? ''} failed:`, err); + sendJson(res, 500, { error: 'internal error' }); }); }); server.listen(opts.port, opts.bindAddress); + return server; } diff --git a/src/server/index.ts b/src/server/index.ts index 75bdcaa..a663d7b 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -61,8 +61,22 @@ for (const entry of index) { } const loaded = await loadGame(gameDir(dataDir, entry.gameId)); + if (!loaded.found && loaded.corrupt) { + // One unreadable file is one game lost, not every game (v0.8.4) — see `loadGame`. + console.error(`Skipping ${entry.gameId} (${entry.gameCode}): game.json is unreadable — ${loaded.corrupt}. The file is left untouched.`); + continue; + } if (loaded.found) { - const resumed = tryResumeSession(loaded.saved); + let resumed: ReturnType; + try { + resumed = tryResumeSession(loaded.saved); + } catch (err) { + // A save that parses but is not the shape the replay expects (no config, a history entry + // that is not an intent) throws inside the engine rather than being refused. Same answer: + // this game, not the server. + console.error(`Skipping ${entry.gameId} (${entry.gameCode}): the save could not be replayed — ${err instanceof Error ? err.message : String(err)}. The file is left untouched.`); + continue; + } if (resumed.ok) { initialGames.set(entry.gameId, resumed.session); console.log(`Resumed ${entry.gameId} (${entry.gameCode}) — ${loaded.saved.history.length} intents replayed.`); diff --git a/src/server/persistence.ts b/src/server/persistence.ts index 6eb4511..978cd24 100644 --- a/src/server/persistence.ts +++ b/src/server/persistence.ts @@ -24,10 +24,33 @@ const INDEX_FILE = 'index.json'; type PersistedGame = SavedGame & { engineVersion: string }; +/** + * ONE WRITER AT A TIME PER FILE (v0.8.4). + * + * Every write here is read-modify-write or write-then-rename with an `await` in the middle, and + * Node's single thread is no protection across an `await`: two requests for the same game could + * both be inside `atomicWrite` at once. With ONE fixed `.tmp` name per path that tore the file — + * measured at 200 rounds of two concurrent writes: every round lost one write to `rename` ENOENT, + * and six left `game.json` as invalid JSON, which the boot then died on. So the temp name is unique + * per write, and every write to a given path queues behind the one before it, which is also what + * makes `upsertIndexEntry`'s "adds or updates exactly one row" claim true under two callers. + */ +const queues = new Map>(); +let writeSerial = 0; + +function serial(key: string, fn: () => Promise): Promise { + const prev = queues.get(key) ?? Promise.resolve(); + const next = prev.then(fn, fn); + queues.set(key, next.catch(() => undefined)); + return next; +} + async function atomicWrite(path: string, text: string): Promise { - const tmp = `${path}.tmp`; - await writeFile(tmp, text); - await rename(tmp, path); + await serial(path, async () => { + const tmp = `${path}.${process.pid}.${++writeSerial}.tmp`; + await writeFile(tmp, text); + await rename(tmp, path); + }); } export async function writeGame(dataDir: string, saved: SavedGame, engineVersion: string): Promise { @@ -37,7 +60,8 @@ export async function writeGame(dataDir: string, saved: SavedGame, engineVersion } export type LoadResult = - | { found: false } + /** `corrupt` names the parse error when the file is there and is not JSON — see `loadGame`. */ + | { found: false; corrupt?: string } /** The version that wrote the file, for diagnostics — it is no longer what decides. */ | { found: true; saved: SavedGame; storedVersion: string }; @@ -61,7 +85,21 @@ export async function loadGame(dataDir: string): Promise { } catch { return { found: false }; } - const payload = JSON.parse(text) as PersistedGame; + /** + * A FILE THAT IS NOT JSON IS REPORTED, NOT THROWN (v0.8.4). This parse was bare, and `index.ts` + * awaited it at the top level — so one torn or half-edited `game.json` took the whole process + * down before the port opened, and StartOS restarted it into the same file: every game on the + * server unreachable because of one. The caller gets a reason to log and moves on. + */ + let payload: PersistedGame; + try { + payload = JSON.parse(text) as PersistedGame; + } catch (err) { + return { found: false, corrupt: err instanceof Error ? err.message : String(err) }; + } + if (!payload || typeof payload !== 'object' || !Array.isArray(payload.history)) { + return { found: false, corrupt: 'not a saved game (no history array)' }; + } const { engineVersion, ...saved } = payload; return { found: true, saved, storedVersion: engineVersion }; } @@ -71,14 +109,18 @@ export async function loadGame(dataDir: string): Promise { export async function appendTiming(dataDir: string, timing: TurnTiming): Promise { await mkdir(dataDir, { recursive: true }); const path = join(dataDir, TIMINGS_FILE); - let existing: TurnTiming[]; - try { - existing = JSON.parse(await readFile(path, 'utf8')) as TurnTiming[]; - } catch { - existing = []; - } - existing.push(timing); - await atomicWrite(path, JSON.stringify(existing, null, 1)); + // The read and the write are one unit under the file's queue, or two appends could each read the + // same list and one of them would be lost. + await serial(`rmw:${path}`, async () => { + let existing: TurnTiming[]; + try { + existing = JSON.parse(await readFile(path, 'utf8')) as TurnTiming[]; + } catch { + existing = []; + } + existing.push(timing); + await atomicWrite(path, JSON.stringify(existing, null, 1)); + }); } // --------------------------------------------------------------------------- @@ -113,11 +155,13 @@ async function writeIndex(dataDir: string, entries: GameIndexEntry[]): Promise { - const entries = await readIndex(dataDir); - const i = entries.findIndex((e) => e.gameId === entry.gameId); - if (i >= 0) entries[i] = entry; - else entries.push(entry); - await writeIndex(dataDir, entries); + await serial(`rmw:${join(dataDir, INDEX_FILE)}`, async () => { + const entries = await readIndex(dataDir); + const i = entries.findIndex((e) => e.gameId === entry.gameId); + if (i >= 0) entries[i] = entry; + else entries.push(entry); + await writeIndex(dataDir, entries); + }); } /** @@ -126,11 +170,13 @@ export async function upsertIndexEntry(dataDir: string, entry: GameIndexEntry): * it on the next boot or leave `index.json` pointing at nothing. */ export async function removeIndexEntry(dataDir: string, gameId: string): Promise { - const entries = await readIndex(dataDir); - await writeIndex( - dataDir, - entries.filter((e) => e.gameId !== gameId), - ); + await serial(`rmw:${join(dataDir, INDEX_FILE)}`, async () => { + const entries = await readIndex(dataDir); + await writeIndex( + dataDir, + entries.filter((e) => e.gameId !== gameId), + ); + }); } /** Deletes a game's whole directory — its save, its turn timings, its sessions, its lobby file. */ diff --git a/src/server/session.ts b/src/server/session.ts index 35e715e..382d104 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -72,6 +72,16 @@ export type Push = { scheduled?: number | null; announcement?: string | null; justDrawn?: string | null; + /** + * THE LAST INTENT `seq` THIS SEAT HAD ACCEPTED — on the connect push only (v0.8.4). + * + * The client numbers its intents from 1 per page load, and this host remembers the seat's last + * accepted number for the life of the game and answers a repeat with "already applied" (§5). So + * a seat that had made one move, reloaded, and clicked again sent `seq: 1` a second time: the + * server said ok and did nothing, the page redrew nothing, and the click looked dead. Telling the + * client where the count stands lets it continue from there instead of starting over. + */ + lastSeq?: number; /** * ORDERED PRESENTATION STEPS — v0.8.0, TODO #13. * @@ -415,6 +425,7 @@ function buildSession( // connect IS the history, which is what lets the Frame stop carrying a second copy. sentLines.delete(seat); const push = pushFor(seat, null); + push.lastSeq = lastSeq.get(seat) ?? 0; /** * The baseline for this client's step queue (v0.8.0). `game.display.last` is the exact frame * the shared delta chain has reached, so the next step merges onto it; before any step has diff --git a/src/web/game.ts b/src/web/game.ts index d193c38..dc24c60 100644 --- a/src/web/game.ts +++ b/src/web/game.ts @@ -51,10 +51,10 @@ import { mainlineProfile, trainProfile, } from '../engine/content.ts'; -import type { Hand, HouseRuleOverrides, TrackGeometry } from '../engine/content.ts'; +import type { CarType, Hand, HouseRuleOverrides, TrackGeometry } from '../engine/content.ts'; import type { Port } from '../engine/track.ts'; import { connectionsFor, joins, neighbour, variantsFor } from '../engine/track.ts'; -import { areaOf, destinationsFor, selectDestination, trainNeedingCars } from '../engine/apply.ts'; +import { acceptsCar, areaOf, destinationsFor, selectDestination, trainNeedingCars } from '../engine/apply.ts'; import type { Frame } from '../sim/view.ts'; /** @@ -1007,16 +1007,31 @@ function consistNeeds(game: Game, trayId: string): string | null { const have = (k: 'coach' | 'caboose' | 'freight'): number => tray.consist.filter((c) => cat(c.type) === k).length; + /** + * ASKED OF `acceptsCar`, NOT RECOUNTED (v0.8.4). Counting by category alone promised cars the + * engine would then refuse: with the caboose already coupled, nothing may go on behind it + * (§A.3), so "Still needs 2 boxcar" was printed while no yard chip lit and the only offer was + * to send the train out as it stands. One sample car per category settles it the same way + * `advance.ts`'s make-up report does. + */ + const sample: Record<'coach' | 'caboose' | 'freight', CarType> = { + coach: 'coach', + caboose: 'caboose', + freight: p.consist.freightTypes?.[0] ?? 'boxcar', + }; const parts: string[] = []; - const freight = p.consist.freight - have('freight'); - const coach = p.consist.coach - have('coach'); - const caboose = p.consist.caboose - have('caboose'); - if (freight > 0) { - parts.push(`${freight} ${p.consist.freightTypes?.join('/') ?? 'freight'}${p.consist.emptiesOnly ? ' (empties only)' : ''}`); - } - if (coach > 0) parts.push(`${coach} coach${coach > 1 ? 'es' : ''}`); - if (caboose > 0) parts.push(`${caboose} caboose`); - return parts.length > 0 ? parts.join(' + ') : null; + let blocked = false; + const want = (k: 'coach' | 'caboose' | 'freight', label: (n: number) => string): void => { + const n = (k === 'coach' ? p.consist.coach : k === 'caboose' ? p.consist.caboose : p.consist.freight) - have(k); + if (n <= 0) return; + if (acceptsCar(tray, sample[k])) parts.push(label(n)); + else blocked = true; + }; + want('freight', (n) => `${n} ${p.consist.freightTypes?.join('/') ?? 'freight'}${p.consist.emptiesOnly ? ' (empties only)' : ''}`); + want('coach', (n) => `${n} coach${n > 1 ? 'es' : ''}`); + want('caboose', (n) => `${n} caboose`); + if (parts.length > 0) return parts.join(' + '); + return blocked ? 'nothing more — the caboose is on, and nothing couples behind it' : null; } /** diff --git a/src/web/session.ts b/src/web/session.ts index e9fd8a1..c88c868 100644 --- a/src/web/session.ts +++ b/src/web/session.ts @@ -282,6 +282,8 @@ type Push = { scheduled?: number | null; announcement?: string | null; justDrawn?: string | null; + /** Where the server's count of this seat's accepted intents stands — connect push only (v0.8.4). */ + lastSeq?: number; }; /** @@ -322,7 +324,19 @@ export function createRemoteSession( let scheduled: number | null = null; let announcement: string | null = null; let justDrawnCard: string | null = null; + /** + * NUMBERED FROM WHERE THE SERVER SAYS, NOT FROM 1 (v0.8.4). + * + * This started at 1 on every page load, and the server remembers a seat's last accepted number + * for the life of the game and answers a repeat with "already applied" (`protocol.md` §5). So a + * seat that had made one move, reloaded, and clicked again sent `seq: 1` twice: the server said + * ok, did nothing, pushed nothing, and the click looked dead. The connect push now carries the + * server's count and this continues from it — never backwards, in case a submit is in flight + * across a reconnect. + */ let nextSeq = 1; + /** One submit in flight at a time — see `submit`. */ + let inFlight = false; const listeners = new Set<() => void>(); const changed = (): void => { for (const fn of [...listeners]) fn(); @@ -355,7 +369,15 @@ export function createRemoteSession( }); }; source.onmessage = (ev: MessageEvent) => { - const push = JSON.parse(ev.data) as Push; + let push: Push; + try { + push = JSON.parse(ev.data) as Push; + } catch { + // A push that is not JSON is dropped rather than thrown out of an event handler nothing + // catches; the next push carries a full delta chain from what this seat was last sent. + return; + } + if (push.lastSeq !== undefined) nextSeq = Math.max(nextSeq, push.lastSeq + 1); // A presence-only push (no `frame`) carries `menu: null` too, but that is not news about this // seat's turn — only a push that actually came from the game (always carries a real `frame`, // per `session.ts`'s `Push`) updates the board or the menu. @@ -402,16 +424,33 @@ export function createRemoteSession( actor: () => need().actor, handPlayable: () => (menu?.hand ?? []).map((h) => h.playNow !== null), async submit(intent: Intent): Promise { + /** + * ONE AT A TIME (v0.8.4). The page redraws the SAME menu the instant a submit is sent — the + * new one only arrives with the push — so a second click before the round trip posted a + * second, fresh `seq` for the same option, and the server, which de-duplicates on `seq` + * alone, applied it again when it was still legal: two cards drawn, two Moves spent, two cars + * coupled. A click that lands while one is in flight is dropped; the push is milliseconds away. + */ + if (inFlight) return false; + inFlight = true; const seq = nextSeq++; - const res = await fetch(`/api/intent?${qs}`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ seq, intent }), - }); - const result = (await res.json()) as { ok: boolean; code?: string }; - // The visible update arrives via the SSE push (broadcast to every seat, including this one), - // not from this response — this only reports whether the rules accepted it. - return result.ok; + try { + const res = await fetch(`/api/intent?${qs}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ seq, intent }), + }); + const result = (await res.json()) as { ok?: boolean; code?: string }; + // The visible update arrives via the SSE push (broadcast to every seat, including this one), + // not from this response — this only reports whether the rules accepted it. + return result.ok === true; + } catch { + // A network failure or a non-JSON answer used to reject out of a `void`ed promise — an + // unhandled rejection and nothing on screen. False is honest: the move was not confirmed. + return false; + } finally { + inFlight = false; + } }, subscribe(fn: () => void) { listeners.add(fn); diff --git a/test/markdown.test.ts b/test/markdown.test.ts index 831f2d4..4fda07f 100644 --- a/test/markdown.test.ts +++ b/test/markdown.test.ts @@ -110,4 +110,19 @@ describe('the documentation renderer', () => { assert.ok(!/BEGIN CARDS/.test(out), `${name}.md leaked a build marker onto the page`); } }); + + it('nests a more-indented bullet as a list inside its item (v0.8.4)', () => { + /** + * The code said "nesting is rendered by recursion" and appended the nested bullet to the parent + * as text, so the published home-deck page read "…knows. - ABS Signals is the exception…" with + * a literal dash mid-sentence. + */ + const out = html(['- parent', ' continues here.', ' - child one', ' wraps too', ' - child two', '- second'].join('\n')); + assert.equal( + out.trim(), + '
    • parent continues here.
      • child one wraps too
      • child two
    • second
    ', + ); + assert.doesNotMatch(html(readFileSync(join(import.meta.dirname, '..', 'docs', 'home-deck.md'), 'utf8')), /\. - /); + }); }); + diff --git a/test/remote-session.test.ts b/test/remote-session.test.ts new file mode 100644 index 0000000..e75adcf --- /dev/null +++ b/test/remote-session.test.ts @@ -0,0 +1,81 @@ +/** + * The browser's half of the multiplayer transport, driven with a fake `EventSource` and `fetch`. + * `createRemoteSession` is pure otherwise — no DOM — so it runs here as it does in the page. + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { createRemoteSession } from '../src/web/session.ts'; + +type Fake = { onmessage: ((ev: { data: string }) => void) | null; emit(data: unknown): void; close(): void }; + +function fakeTransport(): { source: () => Fake; bodies: () => { seq: number }[]; fail: (on: boolean) => void } { + let last: Fake | null = null; + const bodies: { seq: number }[] = []; + let failing = false; + const g = globalThis as unknown as Record; + g['EventSource'] = class { + onmessage: ((ev: { data: string }) => void) | null = null; + onerror: (() => void) | null = null; + constructor() { + last = this; + } + emit(data: unknown): void { + this.onmessage?.({ data: JSON.stringify(data) }); + } + close(): void {} + }; + g['fetch'] = async (_url: string, init?: { body?: string }) => { + if (init?.body) bodies.push(JSON.parse(init.body) as { seq: number }); + await new Promise((r) => setTimeout(r, 5)); + if (failing) throw new Error('network down'); + return { ok: true, status: 200, json: async () => ({ ok: true }) }; + }; + return { source: () => last!, bodies: () => bodies, fail: (on) => (failing = on) }; +} + +const connectPush = (lastSeq: number): unknown => ({ frame: null, menu: null, lines: [], lastSeq }); + +describe('the remote session (v0.8.4)', () => { + it('continues the intent count from where the server says, not from 1', async () => { + const t = fakeTransport(); + const s = createRemoteSession('tok', 0); + t.source().emit(connectPush(7)); + await s.submit({ type: 'draw.end' }); + assert.deepEqual(t.bodies().map((b) => b.seq), [8], 'the first intent after a reload re-used a number the server had already applied'); + // A later reconnect never moves the count backwards. + t.source().emit(connectPush(3)); + await s.submit({ type: 'draw.end' }); + assert.deepEqual(t.bodies().map((b) => b.seq), [8, 9]); + }); + + it('drops a second submit while the first is still in flight', async () => { + const t = fakeTransport(); + const s = createRemoteSession('tok', 0); + t.source().emit(connectPush(0)); + const [a, b] = await Promise.all([s.submit({ type: 'draw.end' }), s.submit({ type: 'draw.end' })]); + assert.equal(a, true); + assert.equal(b, false, 'a double-click posted twice'); + assert.equal(t.bodies().length, 1, 'two intents went over the wire for one click'); + // And the next one, after the round trip, goes through as normal. + assert.equal(await s.submit({ type: 'draw.end' }), true); + assert.equal(t.bodies().length, 2); + }); + + it('answers false, not an unhandled rejection, when the network fails', async () => { + const t = fakeTransport(); + const s = createRemoteSession('tok', 0); + t.source().emit(connectPush(0)); + t.fail(true); + assert.equal(await s.submit({ type: 'draw.end' }), false); + t.fail(false); + assert.equal(await s.submit({ type: 'draw.end' }), true, 'the session did not recover after a failed submit'); + }); + + it('drops a push that is not JSON instead of throwing out of the handler', () => { + const t = fakeTransport(); + createRemoteSession('tok', 0); + assert.doesNotThrow(() => t.source().onmessage?.({ data: '{not json' })); + }); +}); diff --git a/test/server/http.test.ts b/test/server/http.test.ts new file mode 100644 index 0000000..41a4481 --- /dev/null +++ b/test/server/http.test.ts @@ -0,0 +1,171 @@ +/** + * The HTTP layer, driven end to end over a real socket. `startServer` binds port 0 on a temp data + * directory; nothing here reads the built site, so `distDir` is a directory with nothing in it. + * + * Added in v0.8.4, when four faults in `http.ts` turned out to be uncovered because no test had ever + * stood the server up: a leaver's token surviving the leave, an unbounded body, a torn save under + * concurrent moves, and a crash on an error after the SSE head was sent. + */ + +import { describe, it, after, before } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { Server } from 'node:http'; + +import type { GameConfig } from '../../src/engine/state.ts'; +import { startServer } from '../../src/server/http.ts'; + +const SECRET = 'test-secret'; +const config: GameConfig = { + mode: 'competitive', + days: 5, + minCombinedRevenue: 0, + maxCollisionsPerDay: 0, + maxCollisionsTotal: 0, + pvpCardsAllowed: false, + houseRules: { startingOffice: 'whistlePost' }, + optionalRules: { reducedVisibility: false, employeeRotation: false, emergencyToolbox: false }, +}; + +let server: Server; +let base = ''; +let dataDir = ''; + +before(async () => { + dataDir = await mkdtemp(join(tmpdir(), 'station-master-http-')); + server = startServer({ + port: 0, + bindAddress: '127.0.0.1', + joinSecret: SECRET, + distDir: dataDir, + dataDir, + engineVersion: 'test', + initialGames: new Map(), + initialLobbies: new Map(), + initialSessions: new Map(), + }); + await new Promise((resolve) => server.once('listening', resolve)); + const addr = server.address(); + if (!addr || typeof addr === 'string') throw new Error('no port'); + base = `http://127.0.0.1:${addr.port}`; +}); + +after(async () => { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + // A write queued behind the last move may still be landing; retry rather than race it. + await rm(dataDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); +}); + +const post = async (path: string, body: unknown): Promise<{ status: number; json: Record }> => { + const res = await fetch(base + path, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) }); + return { status: res.status, json: (await res.json()) as Record }; +}; +const get = async (path: string): Promise => (await fetch(base + path)).status; + +/** The first SSE message on a stream, then the stream is dropped. */ +async function firstPush(path: string): Promise> { + const res = await fetch(base + path); + assert.equal(res.status, 200, `${path} answered ${res.status}`); + const reader = res.body!.getReader(); + const decoder = new TextDecoder(); + let buffer = ''; + for (;;) { + const { value, done } = await reader.read(); + if (done) throw new Error('stream ended before a push'); + buffer += decoder.decode(value, { stream: true }); + const m = /data: (.*)\n\n/.exec(buffer); + if (m) { + await reader.cancel(); + return JSON.parse(m[1]!) as Record; + } + } +} + +type Seat = { token: string; player: number; gameId: string; gameCode: string }; + +async function table(): Promise<{ host: Seat; guest: Seat }> { + const created = await post('/api/lobby/create', { secret: SECRET, config, displayName: 'Host', players: 2 }); + assert.equal(created.status, 200, JSON.stringify(created.json)); + const host = created.json as unknown as Seat; + const joined = await post('/api/lobby/join', { secret: SECRET, gameCode: host.gameCode, displayName: 'Guest' }); + assert.equal(joined.status, 200, JSON.stringify(joined.json)); + return { host, guest: joined.json as unknown as Seat }; +} + +describe('the HTTP layer (v0.8.4)', () => { + it('revokes the token of a player who leaves, so it cannot play the seat the next arrival takes', async () => { + const { host, guest } = await table(); + const left = await post('/api/lobby/leave', { token: guest.token }); + assert.equal(left.status, 200); + // The leaver's token is dead at once — for the lobby and for the game that follows. + assert.equal(await get(`/api/lobby/stream?token=${guest.token}`), 404, 'a leaver can still watch the lobby'); + const again = await post('/api/lobby/join', { secret: SECRET, gameCode: host.gameCode, displayName: 'Newcomer' }); + assert.equal(again.status, 200); + assert.equal(again.json['player'], guest.player, 'the vacated chair was not the one re-offered'); + const started = await post('/api/lobby/start', { token: host.token }); + assert.equal(started.status, 200, JSON.stringify(started.json)); + assert.equal(await get(`/api/session?token=${guest.token}`), 404, 'the leaver still holds a seat in the running game'); + assert.equal(await get(`/api/stream?token=${guest.token}`), 404, "the leaver can read the newcomer's stream"); + const move = await post(`/api/intent?token=${guest.token}`, { seq: 1, intent: { type: 'localOps.choose', option: 'draw' } }); + assert.equal(move.status, 404, 'the leaver can move for the newcomer'); + // And the newcomer's own token works. + assert.equal(await get(`/api/session?token=${again.json['token'] as string}`), 200); + // On disk too, so a restart does not hand the seat back. + const onDisk = JSON.parse(await readFile(join(dataDir, 'games', host.gameId, 'sessions.json'), 'utf8')) as { token: string }[]; + assert.ok(!onDisk.some((s) => s.token === guest.token), 'the revoked token is still in sessions.json'); + }); + + it('lets the host remove a player, revoking that token the same way', async () => { + const { host, guest } = await table(); + const removed = await post('/api/lobby/leave', { token: host.token, seat: guest.player }); + assert.equal(removed.status, 200, JSON.stringify(removed.json)); + assert.equal(await get(`/api/lobby/stream?token=${guest.token}`), 404); + assert.equal(await get(`/api/lobby/stream?token=${host.token}`), 200, 'the host lost their own seat'); + }); + + it('refuses an oversized body before reading it, and a malformed one with 400', async () => { + const big = await fetch(base + '/api/claim', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ code: 'x'.repeat(200_000) }), + }); + assert.equal(big.status, 413); + const bad = await fetch(base + '/api/lobby/join', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: '{not json' }); + assert.equal(bad.status, 400); + const notObject = await fetch(base + '/api/lobby/join', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: 'null' }); + assert.equal(notObject.status, 400); + }); + + it('tells a connecting seat where its intent count stands', async () => { + const { host, guest } = await table(); + assert.equal((await post('/api/lobby/start', { token: host.token })).status, 200); + const hostPush = await firstPush(`/api/stream?token=${host.token}`); + const actor = hostPush['menu'] !== null ? host : guest; + assert.equal(hostPush['lastSeq'], 0); + const move = await post(`/api/intent?token=${actor.token}`, { seq: 1, intent: { type: 'localOps.choose', option: 'draw' } }); + assert.deepEqual(move.json, { ok: true }); + const reconnect = await firstPush(`/api/stream?token=${actor.token}`); + assert.equal(reconnect['lastSeq'], 1, 'the reconnect push does not carry the count'); + }); + + it('applies a burst of concurrent moves one at a time and leaves the save readable', async () => { + const { host, guest } = await table(); + assert.equal((await post('/api/lobby/start', { token: host.token })).status, 200); + const hostPush = await firstPush(`/api/stream?token=${host.token}`); + const actor = hostPush['menu'] !== null ? host : guest; + // Three moves that are legal only in this order, fired together. + const intents = [ + { type: 'localOps.choose', option: 'draw' }, + { type: 'draw.fromHomeOffice' }, + { type: 'draw.end' }, + ]; + const results = await Promise.all(intents.map((intent, i) => post(`/api/intent?token=${actor.token}`, { seq: i + 1, intent }))); + assert.ok(results.every((r) => r.status === 200), 'a concurrent move was answered with an error'); + const save = JSON.parse(await readFile(join(dataDir, 'games', host.gameId, 'game.json'), 'utf8')) as { history: unknown[] }; + assert.ok(save.history.length >= 1, 'no move reached the save'); + assert.equal(save.history.length, results.filter((r) => r.json['ok'] === true).length, 'the save and the answers disagree'); + }); +}); diff --git a/test/server/persistence.test.ts b/test/server/persistence.test.ts index 05438a4..b4bff2b 100644 --- a/test/server/persistence.test.ts +++ b/test/server/persistence.test.ts @@ -6,7 +6,8 @@ import { join } from 'node:path'; import type { GameConfig } from '../../src/engine/state.ts'; import type { SavedGame } from '../../src/server/session.ts'; -import { appendTiming, loadGame, writeGame } from '../../src/server/persistence.ts'; +import { appendTiming, loadGame, readIndex, upsertIndexEntry, writeGame } from '../../src/server/persistence.ts'; +import { writeFile } from 'node:fs/promises'; const config: GameConfig = { mode: 'competitive', @@ -101,4 +102,57 @@ describe('game persistence (Phase 3)', () => { const timings = JSON.parse(text) as unknown[]; assert.equal(timings.length, 2); })); + + // -- v0.8.4: concurrent writers --------------------------------------------------------------- + + it('two hundred concurrent writes to one save leave it valid and never throw (v0.8.4)', () => + withTempDir(async (dir) => { + /** + * One fixed `.tmp` per path, and no queue: measured at 200 rounds of two concurrent writes, + * every round lost one to `rename` ENOENT and six left the file as invalid JSON. Boot then + * died on it. Unique temp names and a per-path queue are the fix; this is the measurement. + */ + const writes: Promise[] = []; + for (let i = 0; i < 200; i++) { + const grown: SavedGame = { ...saved, history: Array.from({ length: i + 1 }, () => ({ type: 'draw.end' })) }; + writes.push(writeGame(dir, grown, '1.2.3')); + } + await Promise.all(writes); + const result = await loadGame(dir); + assert.equal(result.found, true, 'the save is unreadable after concurrent writes'); + if (result.found) assert.equal(result.saved.history.length, 200, 'the last write did not win'); + await assert.rejects(() => readFile(join(dir, 'game.json.tmp'))); + })); + + it('two concurrent index upserts both land (v0.8.4)', () => + withTempDir(async (dir) => { + await Promise.all([ + upsertIndexEntry(dir, { gameId: 'a', gameCode: 'AAA-1', status: 'active' }), + upsertIndexEntry(dir, { gameId: 'b', gameCode: 'BBB-2', status: 'lobby' }), + ]); + const rows = await readIndex(dir); + assert.deepEqual(rows.map((r) => r.gameId).sort(), ['a', 'b'], 'a concurrent upsert lost a row'); + })); + + it('two concurrent timing appends both land (v0.8.4)', () => + withTempDir(async (dir) => { + await Promise.all([ + appendTiming(dir, { player: 0, phase: 'localOps', day: 1, stage: 1, startedAt: 1, endedAt: 2 }), + appendTiming(dir, { player: 1, phase: 'localOps', day: 1, stage: 1, startedAt: 2, endedAt: 3 }), + ]); + const timings = JSON.parse(await readFile(join(dir, 'turn-timings.json'), 'utf8')) as unknown[]; + assert.equal(timings.length, 2); + })); + + it('reports a save that is not JSON instead of throwing (v0.8.4)', () => + withTempDir(async (dir) => { + await writeFile(join(dir, 'game.json'), '{"seed": 42, "hist'); + const result = await loadGame(dir); + assert.equal(result.found, false); + if (!result.found) assert.ok(result.corrupt, 'a torn file was reported as merely missing'); + await writeFile(join(dir, 'game.json'), '{"seed": 42}'); + const shape = await loadGame(dir); + assert.equal(shape.found, false); + if (!shape.found) assert.match(shape.corrupt ?? '', /history/); + })); }); diff --git a/test/server/session.test.ts b/test/server/session.test.ts index 8cf35dd..2ebf57c 100644 --- a/test/server/session.test.ts +++ b/test/server/session.test.ts @@ -611,3 +611,24 @@ describe('narration reaches a seat exactly once, by one path (#97)', () => { assert.deepEqual(third.lines, opening.lines, 'a reconnect is the full log, every time'); }); }); + +describe('the intent sequence across a reconnect (v0.8.4)', () => { + it('tells a connecting seat the last seq it had accepted, so a reloaded page continues the count', () => { + const session = createSession(42, config, ['Alice', 'Bob']); + const first = session.connect(0 as PlayerIndex); + assert.equal(first.lastSeq, 0, 'a seat that has moved nothing should be told 0'); + const actor = (first.menu !== null ? 0 : 1) as PlayerIndex; + const r = session.intent(actor, 1, { type: 'localOps.choose', option: 'draw' }); + assert.ok(r.accepted); + // A reload: the client starts its own count from 1 again unless told otherwise. + const again = session.connect(actor); + assert.equal(again.lastSeq, 1, 'the reconnect push does not say where the count stands'); + // The repeat the old client would have sent — silently swallowed as a resend. + const repeat = session.intent(actor, 1, { type: 'draw.fromHomeOffice' }); + assert.ok(repeat.accepted && repeat.pushes.size === 0, 'seq 1 should still read as an idempotent resend'); + // Continuing from lastSeq + 1 is a real move. + const next = session.intent(actor, 2, { type: 'draw.fromHomeOffice' }); + assert.ok(next.accepted && next.pushes.size > 0, 'seq 2 was not applied'); + }); +}); + diff --git a/test/web.test.ts b/test/web.test.ts index 36ba543..ac6abd5 100644 --- a/test/web.test.ts +++ b/test/web.test.ts @@ -8,7 +8,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import { turnOf } from '../src/engine/state.ts'; -import { areaOf, check } from '../src/engine/apply.ts'; +import { acceptsCar as acceptsCarOf, areaOf, check } from '../src/engine/apply.ts'; import type { Game } from '../src/web/game.ts'; import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync, readdirSync } from 'node:fs'; @@ -22,7 +22,7 @@ import { cardDescription, cardName, describeIntent, variantLabel } from '../src/ import { variantsFor } from '../src/engine/track.ts'; import { BOARD_CSS, divisionSvg, officeSvg } from '../src/sim/board-svg.ts'; import type { DivisionView } from '../src/sim/view.ts'; -import { ENHANCEMENT_RULES, STAGES_PER_DAY, mainlineProfile } from '../src/engine/content.ts'; +import { ENHANCEMENT_RULES, STAGES_PER_DAY, mainlineProfile, trainProfile } from '../src/engine/content.ts'; import { dayEndHtml, facilitiesHtml, pilesHtml, resultsHtml, timetableHtml } from '../src/web/panels.ts'; import { TURNCHART_CSS, turnChartHtml } from '../src/sim/turnchart.ts'; import { fieldSelectors } from '../src/web/settings-form.ts'; @@ -2459,6 +2459,43 @@ describe('the static build', () => { } }); + it('never lists a car in "still needs" that the engine would refuse (v0.8.4)', () => { + /** + * `consistNeeds` counted by category on its own and could promise cars `acceptsCar` refuses — + * nothing couples behind a caboose (§A.3), and a card narrows which freight it takes. It asks + * `acceptsCar` per category now, so this holds every line of the panel to the engine's answer, + * and the reverse: a category the engine still takes is never left off. + */ + let panels = 0; + for (const seed of [430, 99, 270861860]) { + const game = newGame(seed); + for (let i = 0; i < 600 && currentActor(game) !== null; i++) { + const menu = actionMenu(game); + if (menu.makeUp) { + const tray = game.state.trays.get(menu.makeUp.trayId)!; + const profile = trainProfile(tray.trainNumber ?? 0, tray.trainIsExtra)!; + const needs = menu.makeUp.needs ?? ''; + const categories = [ + { name: 'freight', sample: profile.consist.freightTypes?.[0] ?? 'boxcar', re: /boxcar|hopper|reefer|tank|freight/ }, + { name: 'coach', sample: 'coach', re: /coach/ }, + { name: 'caboose', sample: 'caboose', re: /\d caboose/ }, + ] as const; + for (const c of categories) { + const listed = c.re.test(needs); + const takes = acceptsCarOf(tray, c.sample); + if (listed) assert.ok(takes, `seed ${seed}: the panel lists ${c.name} the engine refuses: "${needs}"`); + const wanted = (c.name === 'coach' ? profile.consist.coach : c.name === 'caboose' ? profile.consist.caboose : profile.consist.freight) > tray.consist.filter((x) => (x.type === 'coach' ? 'coach' : x.type === 'caboose' ? 'caboose' : 'freight') === c.name).length; + if (takes && wanted) assert.ok(listed, `seed ${seed}: the engine still takes a ${c.name} the panel does not list: "${needs}"`); + } + panels++; + } + const { options } = actionGroups(game); + if (options.length === 0 || !submit(game, options[0]!)) break; + } + } + assert.ok(panels > 0, 'no seed reached a train being made up'); + }); + it('keys each make-up car to the yard chip that shows it', () => { // Ten buttons reading "add loaded hopper" when the Division Yard is already on screen showing // exactly those cars by type and load state. The yard is the surface.