Files
Jesse.MarkowitzandClaude Opus 5.5 8a29de54ee Docs: the StartOS package is the only place the game is served
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
2026-10-07 06:05:59 -04:00

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:

  1. web/remote-store.ts — RemoteRecord, readStore/writeStore, loadRemote/saveRemote/ forgetRemote/knownRemote, and lobbyHandlers. Pure functions over localStorage; testable with a Map. Today they are untestable except through the page.
  2. web/url-options.ts — RULE_PARAMS/VICTORY_PARAMS/OPTIONAL_PARAMS, gameOptionsFromUrl, solitaireDefaults, rulesToUrl. A pure round trip; the audit found the location.search = '' no-op assumption (#114) by reading this code, and a test on the round trip would have found it.
  3. web/setup-screen.ts — wireGameTypeBlock, commitNewGame, runSolitaireSetup.
  4. A Selection object replacing the five lets (selected, mode, pendingAt, selectedCrew, peekPlayer) that every click handler mutates. One value, one reset(), passed to render rather than read from module scope — which is what lets a test call render(frame, selection) and assert on the HTML.
  5. 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 check by phase — checkLocalOps, checkNewTrain, checkLoadUnload, checkMainline — each a switch over its own intents, dispatched by inPhase. The shared guards (ownTray, the option-chosen test, the Moves test) become the first lines of checkLocalOps rather than four repeats. reduce splits the same way.
  • One exported carCategory(type) in content.ts, used by acceptsCar, newTrainPhase, consistNeeds and isFreight/carriesLoad.
  • maneuver.flyingSwitch is a weaker copy of switch.dropCars (no switchingRefusal, no engineAt clamp, no standingWest handling). 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.