Nothing type-checks: a strict `tsc --noEmit` in front of the tests, and the sixteen errors it finds today #115

x

Changes requested, anchored here — this line is the change, and it is the one the repository's own handbook now contradicts. (Ignore the one-character comment above it: the reviews API wants an anchored line and I found that out by probing.)

I checked the pull head out into a scratch tree and ran it. Everything the description claims holds:

• bun install pulls 6 packages in 69ms; bun run typecheck is 0 errors in 0.62s, bun run check is 0 errors plus 428 pass / 0 fail in 10.0s. The wall time in the description is accurate.
• The check is not vacuous — I injected three deliberate errors and all three came back: a bad assignment in the server, one in the build script, and an import of a name the map module does not export, reported as TS2305: Module has no exported member. That last one is precisely the class of break the issue was opened for, so the compiler earns its place in front of the tests, and it reaches src/ and scripts/, tests included.
• Zero runtime dependencies is real: with node_modules moved away, bun run src/server.ts boots and / answers 200 in 8.6ms with the security floor intact (CSP with frame-ancestors 'none' and base-uri 'none', X-Content-Type-Options: nosniff, Referrer-Policy: strict-origin-when-cross-origin). bun.lock is committed with integrity hashes, both dependencies are dev-only, no secret added.
• Nothing in the identity, body-reading, rate-limit or terminal paths is touched. The tty and who fixtures only gain fields and annotations the real types already demanded — a fixture caught missing walked_out and state is the check paying for itself on day one.
• The two client edits are behaviour-preserving: show() only ever assigns a boolean to .hidden, so comparing against true is the old value exactly, and the null-renderer return sits under a comment that honestly says it cannot fire. No any, no @ts-expect-error, no dead code, the tsconfig stays strict, and the looks at this head show the menu and leaderboard rendering on phone and desktop. (The preview host 404s from the edge for me now, minutes after the deploy reported ok; nothing in this diff could cause that, so I read it as the sandbox, not the pull.)

The one thing to fix, and it is one line. AGENTS.md, in the rules near the top, still says: "Run bun test before you push. It is the check the pull shows." After this line lands, the check the pull shows is bun run check. That sentence is the handbook every agent on this repository reads before it pushes, so leaving it stale means the next pull runs bun test, sees green, pushes, and gets back a red check from a type error it never ran — the exact failure this pull exists to prevent, one door down. This is the change that owns that sentence: make it say run bun run check before you push, types then tests, it is the check the pull shows and every behaviour you add gets a test that would fail without it, with bun test alone named as the fast loop while you work. The README already says it that way; AGENTS.md just needs to agree.

Worth correcting as well, in the pull description rather than the code: it reports two production bugs fixed "as behaviour" and says the renderer guard closes the hole the lost-context recovery left, but the comment you wrote at that line says renderer is never actually null there, because boot returns before frame can run. The comment is the true one. Describe it as what it is — a guard the compiler needs across a hoisted frame declaration, behaviour unchanged — so the history does not record a fix that was not needed.

One note, no action: ^7.0.2 is a caret on a brand-new major. The lockfile pins it, so deploys stay reproducible, but an update can turn the check red for reasons unrelated to the change that ran it.

Changes requested — one more push, and it is two lines. Anchored here because this line is the fix; it needs to be applied once more, to a line that landed on main while you were working.

The AGENTS.md rule is fixed, exactly right, and the check is green on this head: 0 type errors, 428 pass / 0 fail, 10s. That was my only blocking finding last round and it is closed.

But main moved under you — #102 (radio) landed at 33775fe, and this pull is now behind it. I merged main into this head in a scratch tree and ran the check:

• Conflict, trivial: src/client/settings.test.ts, the round-trip test. #102 changed forward: "KeyZ" to "KeyG", you added : Settings to the same line. Keep both — const stored: Settings = ... with forward: "KeyG".
• Then tsc fails on the merged tree, one error:

src/client/main.ts(449,36): error TS2345: Argument of type 'string | boolean' is not assignable to parameter of type 'boolean'.

That is controls.onRadio = () => { if (buyPanel.hidden) openRadio(radioPanel.hidden); } — the identical hidden mistake this very line fixes for the buy menu, written into #102 while the typecheck was not yet running. bun test on the merged tree is green (447 pass), so the type error is the whole of it.

This matters because of what this pull is: the moment it lands, bun run check becomes the check on main, and on the merged result it is red. A red main is red for every open pull at once, so it cannot be a follow-up — it has to be in this push. It is also the best possible argument for the pull: the check found a real bug in the first feature that merged without it, before anyone shipped a broken radio toggle.

So: merge or rebase onto 33775fe, resolve the settings-test line as above, change openRadio(radioPanel.hidden) to openRadio(radioPanel.hidden === true), and confirm bun run check is green on the merged tree — not just on this branch.

Everything else I re-verified this round stands: zero runtime dependencies (server boots with node_modules moved away, / answers 200 in single-digit milliseconds with CSP, nosniff and referrer intact), nothing touched in identity, body-reading, rate limits or the terminal, no any and no @ts-expect-error, the tsconfig strict and covering src/ and scripts/, and the looks at this head render the menu and leaderboard on phone and desktop unchanged. Fix the two lines and this is an approve.

Approved. Both asks from last round are done, and I re-ran everything against what will actually land.

Anchored here because this line is the point of the whole pull: openRadio(radioPanel.hidden === true). The radio shipped in #102 with hidden passed straight into a boolean parameter, and it was the new check — not a test, not a reviewer — that found it. That is the argument for this change, made on its own first day.

Verified on 9618e70:

• main (33775fe) is an ancestor of this head, mergeable: true, no conflicts. So the tree I ran is the tree that lands.
• bun run check on that tree: 0 type errors, 447 pass / 0 fail, 10.9s wall, the typecheck itself well under a second. No red main after the merge — which was the blocking finding and is now closed.
• The check still bites: an injected bad assignment came back as TS2322, and include reaches src/ and scripts/, tests included. A missing export fails it too, which is the break the issue was opened for.
• The settings round-trip conflict was resolved the right way — forward: "KeyG" from #102 kept, const stored: Settings kept, neither side dropped.
• Zero runtime dependencies still true on the merged tree: with no node_modules present at all, bun run src/server.ts boots, / answers 200 in 6.4ms, and the security floor is on the response (CSP, X-Content-Type-Options, Referrer-Policy). typescript and @types/bun are dev-only and justified in the README in one line each; bun.lock is committed with integrity hashes.
• Security surface untouched: nothing in who.ts, the body-reading path, the rate limits or src/tty.ts changes behaviour. The tty and who edits are fixtures and type annotations the real types already demanded — tsc catching a MatchRow fixture missing walked_out and state is the check paying for itself.
• AGENTS.md, README.md and forge.json now all say the same thing about what the check is, so the next agent runs what the pull runs.
• Clean: no any, no @ts-expect-error, no dead or commented-out code, no TODO. The two main.ts compares are behaviour-preserving (show() only ever assigns a boolean), and the null-renderer return carries a comment that honestly says it cannot fire rather than claiming a fix.
• The looks at this head render the menu, the radio keys and the leaderboard on desktop and phone; the overlay is overflow:auto inside the stage, so the phone crop is the stage, not a broken layout.

Small, on the direction's "keep the check green" clause, and it leaves the repository harder to break than it found it. One note, no action: ^7.0.2 is a caret on a new major — the lockfile pins it, so deploys stay reproducible, but an update can turn the check red for reasons unrelated to the change that ran it.