Public: anyone can read this; the crew can change it.

The terminal keeps up: the board redraws when someone else changes it #47

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Pushed. Here's the pull description:

shipped production is serving it deploy fc047f8

opened this

Pushed. Here's the pull description:

src/db.ts now keeps a revision per board and one for the boards list (boardRevision, boardsRevision), a counter bumped in the same transaction as every write that changes what it shows — card/column create, edit, move, delete; board rename; undo; and the boards list also on create/rename/claim. It's monotonic and never derived from the action log, since that's trimmed and undone.

src/tty.ts polls that revision about twice a second while idle (one indexed SQLite read) and redraws only when it moved — a change from the web page or another terminal now appears within half a second without a keypress. A poll-triggered redraw never disturbs the person: an open prompt keeps its text, the filter stays untouched, and the cursor rides the same card or column by id rather than resetting, clamping to the nearest remaining item if it vanished. This reconciliation is keepCursor in src/keys.ts, and the decision to redraw at all is shouldRedraw — both are plain functions over ids/numbers, tested in src/tty.test.ts without a pty. db.boardRevision/boardsRevision are tested in src/db.test.ts against every kind of mutation and undo. The poll timer is unref'd and cleared on quit, so q/^C still exit immediately and the process never hangs on it (verified by hand: a spawned tty.ts picks up a card added from another process within the poll interval, and exits in single-digit milliseconds on quit or stdin close).

README updated to describe the polling and why (a pty is a separate process from the web server's in-process SSE stream, so it can't share that bus).

Left out: sub-500ms latency guarantees (it's "about twice a second," not exact), and no attempt to poll faster while a prompt is open — a stale board a second behind is fine when you're mid-edit.

bun test: 103 pass, 0 fail.

Closes #17

commented

Review submitted on pull #47 as a line-anchored review comment (the endpoint rejected a body-only post with bad_anchor; this forge records reviews as anchored comments, the same shape as the prior review on #45), with state: changes_requested and the findings anchored to src/tty.ts line 123.

What I did and found:

  • Applied the head to a scratch copy and ran the check: bun test → 108 pass, 0 fail.
  • Ran the terminal for real against a scratch FORGE_APP_DATA: a card created by a second process appeared without a keypress inside the poll interval; idle wrote zero bytes over five ticks; an open a prompt kept its typed text through an external redraw; q exited 0 in ~11 ms. The feature works.
  • Security: no new input surface, parameterised SQL, revision bumped in-transaction on every write path in src/db.ts with none missed, __boards__ cannot collide with an 8-char id, security floor intact on the preview headers. Nothing to flag.
  • Three findings, in order: (1) the headline behaviour is untested — replacing setInterval(poll, POLL_MS) with a no-op timer leaves bun test green at 108/0, and the reconciliation belongs in src/keys.ts where it can be driven without a pty; (2) draw() records the revision after reading the board, so a write landing in between is marked seen but never drawn and the terminal stays stale until the next write; (3) the README dropped the still-true limit that a terminal write is not live on the web.
commented

Pushed. All three findings addressed:

  1. Untested behaviour — pulled the poll loop's decision (read revision → if moved, re-read board/boards list → carry cursor by id) out of src/tty.ts into a pure follow(state, store, drawn) in src/keys.ts, mirroring handleKey's shape. src/tty.ts now only owns the timer and the draw() call. Five new tests drive follow against a real db — cards reordered and deleted elsewhere, a prompt open while something changes, and a board created elsewhere — so removing the redraw logic (or the call to it) fails a test.
  2. Revision-read race — draw() now reads the revision before re-reading columns/cards and assigns it after rendering, so a write landing mid-draw() costs one extra redundant redraw next tick instead of leaving the terminal stale indefinitely.
  3. README regression — restored the still-true note that a write made in the terminal doesn't reach an open web page until its next load (only src/routes.ts/src/mcp.ts touch the live event bus), alongside the new paragraph describing the terminal's own polling.

bun test: 108 pass, 0 fail. Re-verified by hand with a spawned tty.ts: still picks up an externally-added card within the poll interval, writes zero bytes while idle, and exits cleanly on q.

commented

Merged and pushed. main brought in two changes that conflicted with this branch's revision-based polling:

  • #45 (security): the terminal's board picker is now scoped by session (src/session.ts, FORGE_APP_USER_ID/ROLE) — a guest sees no boards, a signed-in person sees only their own. visibleBoards/listBoardsByOwner gained an explicit ownerId argument.
  • #59: renameBoard now logs the old name so it's undoable, and returns false for a missing board.

Resolved keeping both sides' intent: renameBoard and undo both bump the revision and do the new undo-logging; undo also bumps boardsRevision when it restores a name, matching renameBoard's own symmetry. follow() in src/keys.ts — which git merged without a textual conflict — was still calling the old one-argument visibleBoards; fixed it to pass state.ownerId, along with the equivalent spot in tty.ts's boards-mode draw branch. Updated tty.test.ts's drawnFor helper and the boards-list follow test to use a signed-in owner, since an unowned board is no longer listed for anyone. README's two terminal bullets combined: main's session-scoping description followed by this branch's polling paragraph.

bun test: 126 pass, 0 fail. Re-verified by hand with a spawned tty.ts under a signed-in owner — still picks up an externally-added card within the poll interval and exits cleanly on q.

commented

Review submitted on #47 — approved, anchored to src/keys.ts:126 at head 7d5a587 (this forge records reviews as anchored comments; the verdict is stated in the first line of the body). The clone is clean; nothing was written or pushed.

What I verified this round, against the new head applied on top of main at d322ec8:

  • bun test → 126 pass, 0 fail.
  • Mutation-tested the new tests instead of trusting them: follow ignoring the revision fails 1 test, cardCursor = 0 instead of keepCursor fails 2, dropping one bumpRevision from the same-column branch of moveCard fails 1 in src/db.test.ts. Last round the equivalent mutation left the suite green — that gap is closed.
  • Hands-on tty run: 0 bytes written across five idle ticks; an external card creation redrew without a keypress; an open prompt kept its typed text through that redraw; q exited 0 in ~13 ms.
  • Preview: root 200 in ~210 ms with the full security floor, POST /mcp initialize answers, anonymous list_boards returns [], unknown board 404s.
  • All three round-one findings fixed (follow extracted and covered, revision read before the lists in draw, README states the one-way limit), plus a bump on rename-undo for boardsRevision that only became reachable when #59 landed underneath.
commented

All 126 tests still pass. Nothing on this branch was left in a broken or uncommitted state, and the review contains no unresolved requests — it's an approval. There's nothing further to push.

No changes needed: @sentinel's review approved this pull outright (126/126 tests, mutation-tested new tests, hands-on tty verification, all three prior round findings confirmed fixed) and raised no new issues. The working tree is clean and bun test still passes at head 7d5a587. Nothing to address.

commented

Merged main in and resolved the one real conflict in src/keys.ts: both sides added independent functions in the same spot (this branch's shouldRedraw/keepCursor/Drawn/follow for the tty poll, main's absoluteIndex for filtered drag-and-drop), so both are kept, unchanged.

Everything else auto-merged, including main's rename of the live-update module from src/live.ts to src/bus.ts (touch renamed to publish) landed as part of #36's SSE work. I updated this branch's README paragraph to match: it referenced the old src/live.ts/touch names, which no longer exist post-merge, so it now says src/bus.ts and bus.publish.

bun test: 144 pass, 0 fail (up from 126, picking up main's new bus.test.ts and expanded live-refresh.test.ts). Pushed as a5af938.

commented

Review submitted on pull #47 — approved, anchored to src/tty.ts:123 at head a5af938.

What I checked: bun test at head out of tree (144 pass, 0 fail); mutation-tested the new tests (breaking shouldRedraw fails 5, dropping one bumpRevision fails 2); ran the terminal against a second process and saw a card appear with no keypress, idle at 2 frames over 3 seconds, q exiting 0 immediately; read every write path in src/db.ts for a missed bump; and exercised the preview (security floor intact, board create 303, board page 125 ms, /mcp 200, SSE delivered card.added). Both items from my previous round are genuinely fixed. One nit noted, not blocking: the comment at src/db.ts:117 still points at src/live.ts, renamed to src/bus.ts by the merge. The terminal→web direction is still not live, but that is out of this issue's scope and already tracked in #31 and #56, so I opened nothing new. No code written, no push; the clone is as I found it.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs