The rebind panel's wiring has no test: give main.ts's capture lifecycle a DOM harness (or a module) #156

changes_requested — one test in the new suite cannot fail, and it is the one that guards capture.ts's only stated invariant.

(Housekeeping: a stray review comment reading x on capture.ts:1 is mine — an API probe while I worked out that this endpoint wants an anchor. There is no delete route. Ignore it.)

The extraction itself is right, and the right call over a DOM harness — the issue named the module as the better option. The leak class is structurally gone: one persistent capture-phase listener, a bare pending flag, and every site that mutates pending (start, Reset to defaults, closing the <details>, blur, start(), resume, toMenu) followed by a repaint. refreshBindingButtons reading capture.pending instead of a saved label is what makes that hold. I checked all seven sites: none changes pending without repainting. input.ts registers its own keydown in the bubble phase, so the capture-phase interception still wins regardless of boot order — the rebind key cannot also fire the action.

I verified the rest locally: bun run check is green (tsc + 697 pass), the app boots from this branch and / answers in 7ms with the CSP, X-Content-Type-Options and Referrer-Policy floor intact, the bundle builds with the new module, no new dependency, no server surface touched, no dead code left. The screenshots at 6627777 render correctly on phone and desktop.

I also mutation-tested the suite rather than trusting the count. Four of five breakages get caught: dropping stopImmediatePropagation fails 2 tests, clearing pending on a repeat fails 1, leaving pending set after a resolve fails 4, and making start refuse to replace a pending capture fails 1. That part is a real suite.

The fifth does not. This line is the whole test:

setBindings({ ...DEFAULT_BINDINGS, jump: "KeyX" }); // rebound elsewhere mid-capture

The mid-capture write is to jump — the same action being captured — and decideBind overwrites jump either way. Stale map (jump: "Space") and fresh map (jump: "KeyX") both yield { ...DEFAULT_BINDINGS, jump: "KeyJ" }, so the assertion is true no matter when getBindings() is read. I patched createCapture to snapshot getBindings() in start and read the snapshot in handle — precisely the race the doc comment on line 44 says cannot happen — and all 12 tests still passed.

Fix this first: write to a different action, so only a fresh read can produce the right answer.

test("getBindings is read fresh on every key, not captured at start", () => {
  const { capture, bound, setBindings } = harness();
  capture.start("jump");
  setBindings({ ...DEFAULT_BINDINGS, reload: "KeyJ" }); // KeyJ taken elsewhere mid-capture
  const result = capture.handle(key("KeyJ"));
  expect(result).toEqual({ kind: "rejected", action: "jump", reason: expect.stringContaining("Reload") });
  expect(bound).toEqual([]);
});

A snapshot taken at start binds KeyJ here; a fresh read rejects it as taken. Confirm by making that snapshot mutation locally and watching this one go red before you push.

Two notes, neither blocking on its own:

• captureStatus (main.ts:257) is set on click and never set back to null, and a cancel leaves the last rejection reason on screen. That matches the old code, so it is not a regression — but now that the lifecycle has an owner, clearing the status inside cancelCapture would put the last piece of per-capture bookkeeping where the rest of it lives.
• The preview at https://cs--156--forge-trevato-apps.open-platform.sh returns a plain-text 404 not found from the proxy on /, not the app's own HTML 404, on every try over several minutes. The screenshots at this head and a local boot both say the app is fine, so I read this as the preview being asleep rather than anything in the diff — worth a glance when you push the fix.

Everything else on my list passes: no identity or write path touched, inputs still validated at the boundary, nothing shelled or interpolated, no secret added, src/tty.ts unaffected (bindings are a browser-local setting it never exposed), and the README needs nothing because no behaviour changed. Fix the one test and this is an approve.

approved — the one blocking finding is fixed, and I confirmed the fix bites rather than just reading it.

The freshness test now writes to a different action mid-capture, so only a fresh getBindings() can produce the right answer:

setBindings({ ...DEFAULT_BINDINGS, reload: "KeyJ" }); // KeyJ taken elsewhere mid-capture
expect(result).toEqual({ kind: "rejected", action: "jump", reason: expect.stringContaining("Reload") });

I re-ran the same mutation as last round — snapshot getBindings() in start, read the snapshot in handle — and this time the test goes red, alone and by name. Then I widened the battery to nine mutations of capture.ts and every one is caught: dropping stopImmediatePropagation (2 fail), dropping preventDefault (2), clearing pending on a repeat (1), keeping pending after a resolve (4), start refusing to replace (1), losing the Escape case (1), never calling onBound (3), cancel becoming a no-op (1), and claiming the key while idle (3). There is no longer a line in this module I can break without the suite saying so.

The captureStatus note is taken too: cancelCapture now clears the status line, so the last piece of per-capture bookkeeping lives with the rest. Blur, closing the <details>, start(), resume and toMenu all wipe a stale rejection reason, which is the behaviour you want from a transient message.

Verified rather than assumed, against the pull's base (a3d302b) in a scratch worktree:

• bun run check green — tsc --noEmit clean, 697 pass, 0 fail.
• All seven sites that mutate pending repaint; the Reset handler's direct capture.cancel() is still followed by its own status.textContent = "" and refreshBindingButtons(), so it is equivalent to cancelCapture.
• input.ts registers its keydown in the bubble phase, so the single capture-phase listener still wins regardless of boot order — the key that rebinds an action cannot also drive it.
• The app boots from the patched tree, / answers in 8ms, and the CSP / X-Content-Type-Options / Referrer-Policy / HttpOnly-session floor is intact. Client-only diff: no identity, write, body-cap, rate-limit, SQL or shell surface is touched, and src/tty.ts is unaffected (bindings are a browser-local setting it never exposed, so there is no door to keep level here and the README owes nothing — no behaviour changed).
• Three files, no new dependency, net main.ts shorter, one module named for one concept, nothing commented out and no TODO left. decideBind's only caller moved cleanly from main.ts to capture.ts.
• Mergeable against main as it stands now (two commits ahead of this base, no conflicts).
• The screenshots at aaed005 render correctly on both sizes — 360px with no horizontal scroll, the controls panel where it belongs.

Two things I am not blocking on, for whenever you are next in this file:

• harness(bindings = DEFAULT_BINDINGS) in capture.test.ts:31 — no caller ever passes the argument. Dead parameter; delete it or use it.
• The preview at https://cs--156--forge-trevato-apps.open-platform.sh still answers / with a plain-text 404 not found from the proxy, not the app's own HTML 404, across both review rounds and several minutes of retries. The screenshots at this exact head and a local boot both say the app is healthy, so this is the preview being asleep, not the diff. Worth a glance from whoever owns the deploy — I am not holding the pull for it.

Good change. It does what #114 asked, in the shape the issue preferred, and it leaves the rebind panel with a lifecycle that cannot silently eat the next match's first key.

x