The static site on the File Browser host was retired with the 0.4.9 line on 2026-09-29. README, TODO and the structure plan no longer describe a site deploy as the release step or the zero-dependency rule as what keeps the site a static upload. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019rwKTmug58sEsJ72AuWsEi
6.5 KiB
Structure — a proposal (2026-09-29)
What the 2026-09-29 audit found about the SHAPE of the code, as distinct from its faults, and what to do about it. Nothing here changes behaviour; each item is a seam that would let the next fault be found by a test instead of by a reviewer reading three thousand lines. Ordered by payoff.
The faults the audit fixed in 0.8.3 and 0.8.4 were nearly all in the three largest functions or in the one file no test stood up. That is the argument: the structure is where the bugs were.
1. http.ts — a route table, and one place for authentication
Today. startServer is one 600-line async closure of if (pathname === … && method === …)
chains. The token → session → lobby-or-game resolution is copy-pasted eight times; the admin-secret
check, the join-secret check and the host check are each inline where they are needed. The catch at
the bottom is the only error path, and until 0.8.4 it was itself a crash.
Proposed. A table of routes, each { method, path, auth, handler }, with three auth wrappers:
withToken(handler) // resolves { ps, lobby?, game? } from the token, 404s if none
withHost(handler) // withToken, then refuses anyone but lobby.hostToken
withAdmin(handler) // the x-admin-secret gate; the whole prefix is absent when unset
Handlers become ten to thirty lines each and take a typed context. The one dispatcher owns
readJson, sendJson, the HttpError catch and the headersSent guard — one place, so 0.8.4's
crash fix cannot be forgotten by the next route. The per-game inTurn queue becomes a property of
the game context rather than something a handler has to remember to call.
Cost. A day. Every route is exercised by test/server/http.test.ts now, so the move is
mechanical and verifiable. Do it before adding the next route (the seatless display stream, #20).
2. main.ts — five extractions, and a Selection value
Today. 3200 lines, ~21 module-level lets (session, selected, mode, pendingAt,
selectedCrew, peekPlayer, zoom, soundOn, gameCode, remoteToken, …), and start() runs at
import. render() is ~470 lines; renderActions ~360. test/web.test.ts has to stub the DOM before
importing, can never build two pages, and cannot call render() with a Frame of its choosing —
which is why every screen fault in the audit was found by reading, not by a test.
Proposed, in the order they pay:
web/remote-store.ts—RemoteRecord,readStore/writeStore,loadRemote/saveRemote/forgetRemote/knownRemote, andlobbyHandlers. Pure functions overlocalStorage; testable with a Map. Today they are untestable except through the page.web/url-options.ts—RULE_PARAMS/VICTORY_PARAMS/OPTIONAL_PARAMS,gameOptionsFromUrl,solitaireDefaults,rulesToUrl. A pure round trip; the audit found thelocation.search = ''no-op assumption (#114) by reading this code, and a test on the round trip would have found it.web/setup-screen.ts—wireGameTypeBlock,commitNewGame,runSolitaireSetup.- A
Selectionobject replacing the fivelets (selected,mode,pendingAt,selectedCrew,peekPlayer) that every click handler mutates. One value, onereset(), passed torenderrather than read from module scope — which is what lets a test callrender(frame, selection)and assert on the HTML. web/game-shell.ts— session routing:beginRemote,abandonRemote,claimSeat,start,applyCapabilities, the handoff. This is where the animation loop outliving the session (#114) lives, and it is easier to see once it is not surrounded by rendering.
Keep render() in main.ts but split the board-and-hand wiring (~240 lines of addEventListener)
into wireBoard(selection) so what is DRAWN and what is CLICKABLE are separate functions.
Cost. Two to three days across the five, each its own commit; the first two are an afternoon each and carry no risk.
3. The engine — check per phase, and one car category
Today. check (~600 lines) and reduce (~700) are single switches mixing every phase;
moveTrain nests four position kinds by three decision kinds. Car category (coach / caboose /
freight) is computed in three places with three spellings. The seat guard 0.8.3 added to the four
switching intents is the same four lines four times.
Proposed.
- Split
checkby phase —checkLocalOps,checkNewTrain,checkLoadUnload,checkMainline— each a switch over its own intents, dispatched byinPhase. The shared guards (ownTray, the option-chosen test, the Moves test) become the first lines ofcheckLocalOpsrather than four repeats.reducesplits the same way. - One exported
carCategory(type)incontent.ts, used byacceptsCar,newTrainPhase,consistNeedsandisFreight/carriesLoad. maneuver.flyingSwitchis a weaker copy ofswitch.dropCars(noswitchingRefusal, noengineAtclamp, nostandingWesthandling). It should CALL the dropCars path with a flag, or be deleted until the card is dealt (it is at 0 copies). Deciding is #115's job; the structure point is that there must be one cut-dropping reducer.
Cost. The check/reduce split is a day and is pure motion — every test in apply.test.ts
and advance.test.ts runs unchanged. carCategory is an hour.
4. One Push type
web/session.ts mirrors server/session.ts's Push by hand so the browser bundle never imports
from src/server/. Every envelope change (0.8.4 added lastSeq) is two edits. Move the wire types
— Push, LobbyPush, LobbyPreview — into src/sim/wire.ts, which both sides already import
from, and delete the mirror.
Cost. An hour.
5. Tests that would then exist
Each extraction above names the test it makes possible:
| after | test |
|---|---|
| route table | one test per route for the 404/403/400 answers, from a table |
remote-store.ts |
the RemoteRecord round trip, the "seat tied to this browser" notice (#33) |
url-options.ts |
rulesToUrl(gameOptionsFromUrl(x)) === x, and the search = '' case |
Selection |
render(frame, selection) snapshot tests for each screen state |
check per phase |
the seat guard once, as a property over every switching intent |
What not to do
- Do not refactor
render()and fix a screen bug in the same commit. The audit's value was that each finding could be verified against unchanged code. - Do not introduce a framework or a runtime dependency to do any of this. The zero-dependency rule
is what keeps the package a 63 MB
.s9pk.