Who else is on this board: presence beside the stream, and the name behind a move #74

LOW — optimistic. The heartbeat is 25s, but reaping depends on the enqueue actually failing, and a black-holed socket waits out the 60s idle timeout in server.ts. "within a minute" is the honest number.

LOW — once the recipient is excluded (see the routes.ts note), the same signed-in person in two tabs still renders "alice, alice" to everyone else. Either dedupe identical names here and in renderPresence() (a Set before the join), or disclose it in the README — the README discloses the no-dedup rule for guest tabs only, and a pull description is not where a reader looks it up later.

Read at head 9e6e668; bun test is green there (65 pass, 10,048 assertions) and the preview answers / in ~110 ms over the wire with the full security floor intact. The module is well made — parameterised SQL throughout, no word a client sent is ever stored as a name, the one cookie() helper so the guest cookie cannot miss HttpOnly; SameSite=Lax; Secure, SYSTEM guarded by reference equality with an impostor test, refusals instead of throws for the 20 Hz caller, window and limit clamped, and a sweep that bounds the table. The README is honest that none of it is wired yet, and the screenshots match: the page is unchanged. One security finding, one blocker to landing, three notes.

*Fix first — the actions take the guest id*, and the id is the thing you publish.** src/actions.ts:52 makes Playing an id: ensurePlayer(db, anon, { guestId }) and touchPlayer accept any string matching g_[A-Za-z0-9_-]{27} and do nothing else to establish that the caller is that guest. But the id is deliberately public — src/session.ts:95 calls it "Safe to show", the module header says it is "what the world, the database and every door see", and this pull's description says #8 hands it to the loop that publishes state to every client. So anyone who reads one frame holds everything the only checking boundary asks for: pass someone else's id and you are them, and ensurePlayer(db, me, { guestId: theirs }) on sign-in copies their score rows onto your forge player and deletes their row (absorb, src/actions.ts). "Safe to show" and "enough to act as" cannot both be true of one string. Nothing calls these yet, which is the only reason it is not live — and it is why fixing it now is one edit rather than an audit of four doors later.

The fix: take the key, never the id. Playing becomes { guestKey?: string }, the boundary check becomes isGuestKey, and the action derives guestId(key) itself; the id stays an output. Holding the HttpOnly cookie is then the only way to claim a guest, which is what session.ts already means. The tests change by one word each (guestId: g → guestKey: k), and the malformed-input test gets stronger, not weaker: a well-formed id offered as a key must now be refused too.

Second — guest= is a shared-domain cookie here. Every app and every preview on this forge lives under *.open-platform.sh, and cookies ignore the host boundary that CSP and CORS respect: any other app there can set guest=<attacker key>; Domain=open-platform.sh and the browser will send it to drift. That is session fixation on a guest — the victim swims as a guest whose key the attacker holds, and the attacker signs in later and absorbs the points. Name it __Host-guest wherever the origin is https (Path=/, Secure, no Domain — a browser refuses to store a __Host- cookie that carries a Domain attribute or arrives over http), keeping the plain name on http for localhost. One conditional in GUEST_COOKIE/cookie() and one assertion in src/session.test.ts.

Blocker to landing: head 9e6e668 conflicts with main on README.md, and the branch predates #20 — src/world.ts is not in it. Merge main and re-run bun test before this can go anywhere.

Notes, not blockers. (1) The IF NOT EXISTS guards in src/db.ts keep the shifted preview database opening, but that database skips index 2 (DROP TABLE IF EXISTS note) permanently and its recorded version numbers name different statements than production's — the note table lives on there forever. Clearing that preview's data directory is cleaner than carrying guards that make a migration list idempotent rather than ordered. (2) playerKey() mints and discards a fresh key for a cookieless anon; a door that writes with it makes one throwaway player per request. The doc comment says so, but returning undefined when there is no cookie would make the foot-gun unreachable. (3) recordRound checks for missing players outside the transaction that then writes them; harmless in one synchronous process, worth a line saying it depends on that.

Everything else — the merge being idempotent, the ladder excluding guests and tie-breaking on the earliest round, the handle shape no forge username can wear, the sweep sparing anyone with a score — checks out and is tested.

Correction first: the comment left on this pull a few minutes ago (01m2sec83mxmjfv49wv9tj9hx6, same line, under my name) is not mine in content — a shared temp file was clobbered between writing and posting and it carries another repository's review text. Ignore it entirely; this is the review.

Reviewed at eb04435: read the diff in full, ran bun test at that head (175 pass, 0 fail), and used the preview. Last round's findings are addressed — the frame now excludes the recipient's own connection, onChange's unsubscribe has its left guard with a test, the names dedupe.

What I verified working, not assumed

• Identity is the forge's, never the client's: I opened the preview's /b/:id/events with a hand-written x-forge-identity: eviluser|MalloryFake|admin|99999999999999 and a junk signature — the frame came back {"people":[]}. A typed name cannot enter presence.
• Two concurrent streams against the preview: A receives {"people":[null]} when B joins and {"people":[]} within a second of B aborting, each excluding itself — the same set a fresh page load renders. Presence works, live, as the issue asked.
• No id on the wire (by: by?.name ?? null), names escaped server-side (presenceText, with the <script> test) and written with textContent client-side, presence scoped per board, no new dependency, no dead code, and a README honest about what presence is not.

The one thing to fix: the presence fan-out is an unmetered push path.

GET /b/:id/events carries no rate limit, and every join and leave writes a frame to every other subscriber inline, each frame rebuilding the whole list. Measured against this branch locally, on a board with 20 viewers: 25 connect/abort cycles from one stranger with the link (578 ms, a few KB of requests) pushed 667 frames / 133 KB into the other viewers' connections — about 27 frames and 5 KB per single request, growing with the subscriber count, so near the 100-tab cap it is roughly 5x that with longer payloads. src/limits.ts states the rule this crosses: MAX_WRITES_PER_MINUTE exists so one caller cannot "push unlimited live events" by writing; this adds a way to push them without writing at all. It bites without malice too — after a deploy, 100 tabs reconnecting produce a 100x100 notify burst.

Concrete fix, small and local, and still well inside "within a second": coalesce in src/presence.ts. Have notify() set a per-board timer instead of calling listeners inline — at most one notification per board per ~250 ms, cleared when the board empties — with each listener reading list() at fire time so the frame is still current. A test that N joins in one tick produce one notification would fail without it. If you want a second lock, charge a stream open to writeBucket keyed with callerKey(who, ip); the coalescing is the part that matters.

Two small things while you are in there

1. The pull description contradicts the code and the README: it says "no dedup of the same signed-in person across two tabs (each open connection counts as one)", but presenceText and renderPresence both dedupe by name and two tests assert it. Fix the description — it is what people read after this lands.
2. It does not merge: src/routes.ts conflicts with main's cb2e92a (the cleanText move into src/text.ts, plus the import block). Trivial, but it has to be rebased before it can land.

Not blocking, worth knowing

• The README's "dropped within a minute — the connection's own idle timeout" is not what the deployment does: I held one /events connection on the preview open for 170 s, heartbeats flowing, and it was never dropped. A tab that dies without closing can linger in the list well past a minute. Soften the sentence rather than chase the code.
• Presence is web-only: someone working in src/tty.ts neither appears in the list nor sees it, since a separate process cannot read this map. Defensible for a thing defined as stream membership, but the terminal section of the README should say so.

Everything else — the escaping, the id staying off the wire, the self-exclusion, the bus unsubscribe guard and its test, the attribution mark and its aria-live announcement, the fixed-height slots that keep it from shifting — reads right and is covered by tests that would fail without it.

probe2

p

MEDIUM — this unsubscribe has no left guard, unlike join()'s (which you test in presence.test.ts). stop() in routes.ts' eventStream is reachable more than once — the abort listener, the stream's cancel(), and a failed write() all call it — and a second call after this board's listener set was emptied and re-created runs changeListeners.delete(boardId) against the new set, silently freezing presence for the tabs still connected: exactly the "a lie on the page" risk the issue names. I could not trigger it on the preview (a later connection was still seen by an older one after two came and went), so it is hardening, not a break. The same one-line if (left) return; closes it; src/bus.ts has the identical shape, so guard it there too or say why not.

HIGH — fix this first. This frame is per-board, so it counts the recipient's own connection; the first paint at line 250 does not. On the preview, one /events connection with nobody else on the board receives event: presence / {"people":[null]}, while GET /b/:id for a visitor who is not streaming renders an empty slot. A person who opens a board alone therefore watches the slot fill in with "Guest" about a second after load, and a signed-in person alone sees their own username under the tooltip "Who has this board open right now" — the page reports someone who is not there, and it contradicts both the README you added ("who else has this board's /events stream open") and the issue title.

Fix: make the frame per-recipient, not per-board. Have join() hand back its token (or return { leave, others }) and give list(boardId, except?) an exclusion, then send presence.list(boardId, myToken) here — exactly the set line 250 computes, so the two paths agree and a lone visitor's slot stays empty. Tests that would fail today: with one connection open, its first presence frame is {"people":[]}; with two open, each sees exactly one person.