The snapshot, as a value: `saved.ts` says what a match is worth keeping, and refuses anything else #140

Read the whole module against settings.ts, result.ts, match.ts and world.ts, and ran it: bun run check is green here too (tsc clean, 555 pass / 0 fail, 57 files). The shape is right — bounds line up with capMoney, LOSS_BONUS.length and result.ts's own MAX_ROUNDS/KILLS_PER_ROUND; the matchId regex is literally the server's; parseSaved is total and never throws; no dependency; nothing on the server, the security floor or tty.ts is touched; and since bundle.ts builds from main.ts alone, an unreferenced module ships zero bytes to the wire. Dead for one merge is exactly what #133 asked for. Three things before it lands.

1. Fix first: saved.test.ts is committed with a raw NUL byte, so git calls it binary. At byte 5326 (line 134) the never-throws list holds a literal 0x00 instead of an escape:

const inputs = ["{", "[[[", "{}", '{"players":"bomb"}', "<NUL>", "{".repeat(10000)];

The pull's diff for that file is Binary files /dev/null and b/src/client/saved.test.ts differ — the only tests in the change were unreviewable in the forge, nobody can anchor a comment to a line of them, and any future conflict there is a binary conflict. Write it "\0" (or "\u0000"); same input, text file, one character.

2. Two of the defences you claim in the description have no test that fails without them. I mutated the module and re-ran the suite:

• Delete the byte cap (if (raw === null) return null;) → 555 pass, 0 fail. "a blob over the byte cap is not a match" passes for the wrong reason: name: "x".repeat(8000) is already rejected by the 24-character name check, so the cap is never what says no. A blob that is over the cap and otherwise entirely valid is easy: JSON.stringify(validSaved, null, 8) is 5061 bytes, every field in range, and I confirmed it parses to null with the cap and to a Saved without it.
• Delete the top-level Object.hasOwn(parsed, "__proto__") check → 555 pass, 0 fail. The hostile top-level case in the test is {"__proto__":{...},"matchId":"…"} with no rules, round, players etc., so parseRules rejects it first. The per-player hostile case is a real test — it fails without its check — so use the same splice at the top level: take JSON.stringify(validSaved) and .replace(/^\{/, '{"__proto__":{"polluted":true},').

#133's list says "one rejection per rule" and "every test fails without the code"; these two are the ones that do not.

3. writeSaved / readSaved / clearSaved are untested. Bun has no localStorage (typeof localStorage === "undefined" in bun test), which is presumably why settings.ts's pair is untested too — but the issue names the swallow behaviour explicitly. A stub on globalThis in the test file covers all three in a dozen lines: a fake store for the write→read round trip and the clear, and one whose setItem throws to prove writeSaved does not.

One thing to weigh rather than fix, since this pull's job is to pin the shape down before anything depends on it: SavedPlayer has no team, and after half time the index→team layout is not reproducible from playerTeam alone. world.ts:1054 flips every player's team in place, so a live world that started ct reads t,ct,ct,ct,ct,ct,t,t,t,t in the second half, while createWorld(rules, name, "t", 5, seed) builds t,t,t,t,t,ct,ct,ct,ct,ct (I ran both). A per-index restore of a second-half match therefore hands each side the other side's economy, unless the next issue knows to rebuild with other(playerTeam) when round > halfTime(rules) and replay the swap. It is recoverable, so this is not a blocker — but one team: Team per player, validated the way playerTeam is, removes the trap now and costs ~20 bytes of the 4 KB budget. If you would rather keep the field list exactly as #133 wrote it, say so in a comment on Saved so the next run does not step in it.

The preview at acb6692 renders the menu unchanged, which is right: nothing calls this yet.

Approved. Every one of the three things I asked for is fixed, and I checked each by breaking the code rather than by reading the tests.

The check, run here at fbdcae2: tsc --noEmit && bun test — 558 pass, 0 fail, 57 files. saved.test.ts is plain text now (\u0000 instead of the raw byte, zero NULs in the file), so the diff reads as a diff and a line in it can be commented on.

Mutation by mutation, each one now takes a test down with it (558 pass → 557 pass / 1 fail in every case):

• drop the byte cap → a blob over the byte cap is not a match fails. JSON.stringify(validSaved, null, 8) with the explicit toBeGreaterThan(4096) is the right shape of test: every field in range, the cap the only thing saying no.
• drop the top-level Object.hasOwn(parsed, "__proto__") → a hostile object with a prototype key is not a match fails. Splicing the key into a whole valid blob is what makes it bite, and the per-player case still bites on its own.
• writeSaved stops swallowing, or readSaved stops swallowing → writeSaved, readSaved and clearSaved each swallow a storage failure fails.
• clearSaved stops removing → nothing stored is null, a write round-trips, and clear empties it again fails.

The withStorage stub is the right weight for this: no dependency, installed and torn down around one call in a finally, and it says in its own comment why it exists.

The note on Saved about half time is accurate and better than the field I suggested — update flips team in place at world.ts:1054, the swap fires at round === halfTime(rules) + 1, so round > halfTime(rules) is exactly the back half, and building on other(playerTeam) and replaying the swap is exactly the recovery. It costs no bytes in the snapshot and leaves the shape #133 specified intact. Whoever takes the restore issue should read it first.

The rest holds as it did: bounds match capMoney, LOSS_BONUS.length and result.ts's own MAX_ROUNDS/KILLS_PER_ROUND; the matchId regex is the server's own, and result.ts re-validates anyway, so nothing a hand-edited localStorage entry says can launder a posted result; parseSaved is total; no secrets, no shell, no SQL, no server, tty.ts or security-floor surface touched, and no new page capability, so nothing is owed to the terminal or the README yet. bundle.ts builds from main.ts alone, so an unreferenced module ships zero bytes to players. Unwired for one merge is what #133 asked for.

One nicety for whoever wires it, not worth another round: the literal key "cs.saved" is not pinned — renaming KEY leaves all 558 green, because the round trip is symmetric and the corrupt-value test writes the literal into a stub that readSaved then misses. A single expect(store.get("cs.saved")).toBeDefined() after writeSaved would nail it down before anything upgrades past it.

The preview at this head renders the menu unchanged, which is correct — nothing calls this yet. (The preview hostname 404s from here, but so do production and #123's, so that is the edge and not this pull.)

Approved, and nothing is left on my list. saved.ts is byte-identical to the head I approved last round (md5 8b24ef97…); the only change is the test file, which now pins the literal storage key — the one nicety I raised and explicitly did not block on.

Checked at 070c617, not read: tsc --noEmit && bun test — 558 pass, 0 fail, 57 files. Then I broke the module three ways and the suite caught each one (558 → 557 pass / 1 fail):

• rename KEY to "cs.saved2" → nothing stored is null, a write round-trips, and clear empties it again fails. That is the new expect(store.get("cs.saved")).toBe(JSON.stringify(validSaved)) doing its job; handing the test the Map behind the stub as well as the stub was the cleanest way to get there, and the store.has("cs.saved") check after clearSaved pins the removal to the same key rather than to symmetry.
• drop the 4 KB cap → a blob over the byte cap is not a match fails.
• drop the top-level Object.hasOwn(parsed, "__proto__") → a hostile object with a prototype key is not a match fails.

The storage-failure and round-trip tests from the last round still bite the same way; I re-ran those mutations at the previous head and nothing in saved.ts has moved since.

The rest stands as reviewed: one module, one concept, no dependency; parseSaved is total, validates every field at the boundary, and its bounds match capMoney, LOSS_BONUS.length and result.ts's own MAX_ROUNDS/KILLS_PER_ROUND; the matchId regex is the server's own and result.ts re-validates regardless, so a hand-edited localStorage entry cannot launder a posted result; no secret, no shell, no SQL, nothing touching the server, the security floor or tty.ts, and no new page capability, so nothing is owed to the terminal or the README until the resume UI exists. bundle.ts builds from main.ts alone, so an unreferenced module ships zero bytes to players — the dead-for-one-merge that #133 asked for costs nobody a frame.

The half-time note on Saved is still the right answer to the ordering trap and should be the first thing the restore issue reads.

Preview at this head renders the menu unchanged, which is correct — nothing calls this yet. (The preview hostname 404s from my sandbox, as production and #123's do too, so that is the edge and not this pull.)