docs: bug audit report

Thirty findings plus six investigated-not-bugs from the read-every-file sweep of
src/, with resolution status for the six the user approved for fixing and the
live-Chrome experiment F12 still needs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

1 files changed, 1482 insertions, 0 deletions

diff --git a/docs/superpowers/reports/2026-07-26-bug-audit.md b/docs/superpowers/reports/2026-07-26-bug-audit.md
new file mode 100644
index 0000000..4ebdd66
--- /dev/null
+++ b/docs/superpowers/reports/2026-07-26-bug-audit.md
@@ -0,0 +1,1482 @@
+# Bug audit sweep — `src/` read-every-file pass
+
+Task 15, Steps 1–2. Branch `refactor/codebase-typesafety`, HEAD `4909192`.
+No `src/` or `test/` file was modified by this pass. This document is the deliverable.
+
+**Coverage.** Every `.ts` file under `src/` was read in full (80 files, ~10.8 kLOC):
+`cdp/` (17), `daemon/` (5), `domains/` (30 incl. `search/`, `readpage/`), `profile/` (7),
+`schemas/` (2), `util/` (10), and the 7 root files. Also read: `scripts/check-boundaries.ts`,
+`package.json`, and the test tree layout. Depth notes are in *Coverage and confidence* at the end.
+
+**Numbering.** F1…F36, ordered by severity. The summary table is at the bottom.
+
+---
+
+## Resolution status
+
+The user ruled on 2026-07-28: fix six findings, leave the rest documented.
+
+| Finding | Status | Commit |
+|---|---|---|
+| — | test infrastructure (`makeFakeClient`) | `1d4ae38` test: fake browser client over a stub transport |
+| F3 | **fixed** | `d10e729` fix(cdp): re-enable domains when re-attaching to a known tab |
+| F4 | **fixed** | `7c6b3fd` fix(cdp): route background-tab events to the tab they belong to |
+| F1 | **fixed** | `2ff5459` fix(tool): stop the socket gate rejecting every tool call on Windows |
+| F6 | **fixed** | `fe90a9c` fix(domains): keep element refs stable across a mutation |
+| F5 + F10 | **fixed** (one engine) | `16e7a30` fix(domains): one fill engine, so browser_fill stops lying about checkboxes |
+| F2 | **fixed** | `77086bb` fix(domains): report a page-side throw instead of blaming the ref |
+| F12 | **left unfixed by ruling** — needs one live-Chrome experiment | — |
+| all others | documented only, unchanged | — |
+
+Every fix was driven by a test that first failed for the reason recorded here.
+Test count went 206 → 251 passing (8 skipped, 0 failing); `npm run check` is green.
+
+### Observable behaviour changes shipped
+
+Three, all in the fill engine commit, and all deliberate:
+
+1. **`browser_fill` on a disabled input now errors instead of returning success.**
+   User-approved. The old success was the bug, and `browser_fill_form` already
+   refused it.
+2. **`browser_fill` on a checkbox/radio now toggles `checked`** instead of writing
+   the submit value and reporting a no-op as success (F5).
+3. **`browser_fill_form` on a contenteditable now uses `execCommand("insertText")`**
+   rather than assigning `textContent`, so it drives ProseMirror/Slack-style
+   editors the way `browser_fill` already did. This was not in the ruling's
+   enumerated list — it is the unavoidable consequence of the two tools sharing
+   one engine, and it is the better of the two behaviours, so it stayed. Flagging
+   it because it changes what `browser_fill_form` does to a rich-text editor.
+
+Minor, non-behavioural: both fill tools now return `tag` in `details`; the six
+domain-enable round trips now also run on a re-attach (F3), and tab registration
+moved ahead of them (F4).
+
+Deliberately *not* changed while fixing these: the object-handle leak (F29) still
+exists on the fill/focus/select ref paths — routing them through `resolveAndCall`
+would have fixed it for free, and I kept them on `runOnElement` precisely so this
+pass did not quietly fix an out-of-scope finding.
+
+**Reproducibility.** `test/cdp/session-call-test.ts` already contains a stub `CdpTransport`
+with a controllable `events()` iterable and a per-method canned-response map. That harness is
+enough to reach most of the `cdp/` and `domains/` findings below without a live browser, which is
+why so many are marked reproducible. There is no equivalent fake for `BrowserClient`; findings
+that need one say so.
+
+---
+
+## F1 — Every browser tool fails on Windows: `access()` on a named pipe
+
+> **FIXED** in `2ff5459`. Both call sites now share `daemonSocketMayExist(platform, socketPath)`
+> in `src/daemon/spawn.ts`, following the precedent the same file already set.
+> Tests: `test/util/daemon-socket-test.ts` pins the platform matrix — a `\\.\pipe\…`
+> path on `win32` is never judged absent, while POSIX still consults the filesystem.
+> **Still unverified on Windows hardware**: the tests pin the platform *branch*, not
+> Chrome's behaviour on Windows. One run of any `browser_*` tool on a real Windows
+> box is still the only thing that closes this out.
+> The gate's own branch could not be tested deterministically — it calls
+> `daemonSocketMayExist()` with no arguments, and this machine has a live
+> `/tmp/pi-browser-daemon.sock`, so a gate test asserting "handler skipped" passes or
+> fails depending on the developer's machine. I dropped that assertion rather than
+> ship an environment-dependent test; the gate is covered only through the helper.
+
+**Location** `src/util/tool.ts:143-157` (guard), `src/daemon/protocol.ts:143-146` (the path).
+
+**What breaks.** `registerBrowserTool`'s `execute` gates every tool on
+`await access(DAEMON_SOCKET_PATH)`. On Windows `DAEMON_SOCKET_PATH` is
+`\\.\pipe\pi-browser-daemon` — a named pipe, which is not a filesystem entry, so
+`GetFileAttributesW` (what libuv's `access` resolves to) cannot see it and `access` rejects.
+The guard therefore fails for a perfectly healthy daemon and every `browser_*` call returns
+`not_connected: "Browser harness not initialized. Run /browser-setup first"`.
+
+The codebase already knows this. `src/daemon/spawn.ts:44-51` skips the identical pre-check
+for exactly this reason, with the comment *"Windows named pipes are not filesystem entries —
+`access()` always fails there — so skip the pre-check and probe directly."* `tool.ts` is the
+same mistake in the one place it is fatal.
+
+**Trigger.** Any `browser_*` tool call on `process.platform === "win32"`, with or without a
+running daemon. `browser_setup` and `browser_http_get` still work (`ensureAlive: false`).
+
+**Impact** The harness is non-functional on Windows, and the error message misdirects the user
+to a wizard that cannot fix it. Windows is clearly meant to be supported — the code carries
+`.cmd` shim handling, `windowsVerbatimArguments`, PowerShell CIM process detection, and
+`LOCALAPPDATA` profile paths.
+
+**Severity** crash (total loss of function on one platform).
+
+**Proposed fix.** Replace the `access` pre-check with the platform-correct liveness probe that
+already exists: on POSIX keep `access`, on Windows skip straight to `client.ensureAlive()`
+(whose transport `connect` already reports `ENOENT` as "Daemon not running"). Factoring the
+POSIX-only branch out of `spawn.ts:45-51` into one shared helper would prevent a third copy.
+
+**Observable behaviour change.** Yes, on Windows only: tools start working. On POSIX, none.
+
+**Reproducible in a test?** Yes, but only after extraction. As written the check is inline in
+`execute` and reads `process.platform` transitively through a module constant. Extract
+`daemonEndpointReachable(): Promise<boolean>` (or pass the path in) and a test can assert that a
+`\\.\pipe\…`-shaped path is not rejected on the strength of `access` alone. Asserting the real
+Windows behaviour needs Windows.
+
+**Confidence.** High on the mechanism, **unverified on Windows hardware** — I could not run it
+here. Corroborated by `spawn.ts`'s own comment, which the earlier work treated as a verified fact.
+
+---
+
+## F2 — `runOnElement` ignores `exceptionDetails`: a page-side throw is reported as "No element matched"
+
+**Location** `src/domains/form.ts:30-62`; live at `:142` (fill), `:205` (focus), `:290` (select).
+
+> **FIXED** in `77086bb`. `runOnElement` now checks `exceptionDetails` and returns
+> `cdp_error` carrying the exception text. Per the ruling, the error is
+> distinguishable from both neighbours: `cdp_error` for a throw, `invalid_state`
+> "No element matched" for a genuine miss, `invalid_state` "unknown or stale" for a
+> bad ref — all three asserted in `test/domains/form-errors-test.ts`, which failed
+> first with `invalid_state` for all three tools. The fix deliberately did **not**
+> route the ref path through `resolveAndCall`, which would also have silently fixed
+> the out-of-scope handle leak (F29).
+
+*Pre-confirmed item — I agree it is real. I do not rate it the single most severe item; see
+"Ranking" below.*
+
+**What breaks.** The ref path calls `cdpCall(client, "Runtime.callFunctionOn", …)`. `cdpCall`
+(`src/domains/cdp-call.ts:11-13`) only inspects transport success. A CDP command that *succeeds*
+while the page function *throws* returns `{ result: {type:"undefined"}, exceptionDetails: {…} }`.
+`runOnElement` returns `ok(r.data.result.value)` = `ok(undefined)`; the handler's
+`isFillResult(undefined)` is false, so it takes the `res === undefined` branch and reports
+`` `No element matched: ${target}` ``.
+
+**Trigger.** Any exception inside `FILL_FN` / `FOCUS_FN` / `SELECT_FN` on the ref path. Concretely:
+`SELECT_FN` starts with `arg.wv` — if the argument ever arrives as `undefined` the very first
+statement throws; `FILL_FN` calls `el.dispatchEvent`, which throws if a page overrode it or if a
+CSP/permissions error surfaces; a cross-origin `contenteditable` focus can throw. Also every
+future edit to those function bodies.
+
+**Impact** A **wrong diagnosis**. The agent is told the ref is bad and re-snapshots a ref that was
+fine, then loops. The real error text is discarded. Compare `resolveAndCall`
+(`form.ts:361-363`), which does check `exceptionDetails` and reports
+`` `page function threw: …` `` — the two code paths in the same file disagree.
+
+**Severity** wrong result (misdiagnosis, agent-visible loop).
+
+**Proposed fix.** Check `r.data.exceptionDetails` in `runOnElement` before reading
+`result.value` and return `{ kind: "cdp_error", message: \`page function threw: …\` }`. Better:
+delete `runOnElement`'s ref branch and route it through `resolveAndCall`, which already checks
+`exceptionDetails` *and* releases the object handle (F29).
+
+**Observable behaviour change.** Yes — a previously-mislabelled failure now returns
+`cdp_error` with the page's own message.
+
+**Reproducible in a test?** Yes. Needs a fake `BrowserClient` whose `session().call` returns
+`ok({ result: { type: "undefined" }, exceptionDetails: { text: "boom" } })`. No such fake exists
+yet; the stub transport in `test/cdp/session-call-test.ts` plus a thin client shim gets there.
+
+---
+
+## F3 — `switchTo` back to a previously-visited tab silently stops collecting console, network, and page events
+
+> **FIXED** in `d10e729`. `enableDomains` is now called on every attach, not only a
+> tab's first visit. Tests: `test/cdp/session-switch-test.ts`, which failed first
+> with **zero** `.enable` calls on the returned-to tab's new session.
+
+**Location** `src/cdp/session.ts:350-382`, specifically `368-377`.
+
+**What breaks.** `switchTo` calls `attachTo` (`Target.attachToTarget` with `flatten: true`),
+which mints a **new** `sessionId` — the code knows this: `:369` says *"Each
+Target.attachToTarget produces a new sessionId — update it."* CDP domain enablement is
+**per-session**. But `enableDomains(attachedSessionId)` runs only in the `else` branch, i.e. only
+the first time a tab is visited. On a revisit the new session is left with `Page`, `DOM`,
+`Runtime`, `Network`, `Accessibility`, and `Log` all **disabled**.
+
+**Trigger.** `browser_switch_tab` to A → `browser_switch_tab` to B → `browser_switch_tab` back
+to A. From that moment, on tab A: `Runtime.consoleAPICalled`, `Log.entryAdded`, and every
+`Network.*` event stop arriving, so `browser_console` and `browser_network_requests` return
+"_No matching … captured yet._" for a tab that is generating traffic. `Page.loadEventFired` /
+`Page.frameNavigated` also stop, so `pageInfoDirty` is never set and `client.pageInfo()` serves
+the 1-second-TTL cache without invalidation. `Page.javascriptDialogOpening` stops too, so a
+dialog on the re-visited tab is never captured and `browser_page_info` cannot warn about it.
+
+**Impact** Silent, permanent-for-that-tab loss of all diagnostic data, plus a missed dialog
+(which blocks every subsequent interaction with no explanation). The tool descriptions promise
+the opposite: *"Buffer is reset on tab switch"* implies collection resumes after a switch.
+
+**Severity** wrong result (silent data loss) — with a plausible path to a hard hang via the
+missed dialog.
+
+**Proposed fix.** Call `enableDomains(attachedSessionId)` unconditionally in `switchTo`, for the
+`existing` branch as well. Re-enabling an already-enabled domain is a no-op in CDP, so hoisting
+the call above the if/else is safe and simplest.
+
+**Observable behaviour change.** Yes — events resume; up to six extra CDP round-trips per
+re-visit (the `TODO(perf)` at `:207` already notes these are sequential).
+
+**Reproducible in a test?** Yes, cleanly, with the existing stub transport: `switchTo("a")`,
+`switchTo("b")`, `switchTo("a")`, then assert `calls` contains `Log.enable` for the third
+`sessionId`. This is the single easiest finding in the report to pin down.
+
+**Confidence.** High on the code shape. The premise "a second `attachToTarget` mints a new
+sessionId and domain state is per-session" is standard CDP behaviour and is asserted by the
+code's own comment, but I did not confirm it against a live browser.
+
+---
+
+## F4 — The event loop discards every event from non-active tabs, defeating the per-tab buffers
+
+> **FIXED** in `7c6b3fd`. Handled with the extra care the ruling asked for: before
+> widening the filter I traced every `switch` arm against a background-tab event.
+> Three arms needed to change together, and widening the filter alone would have
+> introduced a new bug —
+> - `Page.javascriptDialogOpening` used the *active* `targetId`, so a background
+>   tab's dialog would have been recorded against the foreground tab and
+>   `browser_page_info` would have pointed the agent at the wrong tab. Now
+>   `resolveTab(ev.sessionId)`.
+> - `Page.frameNavigated` / `loadEventFired` had the same shape: the wrong tab's
+>   page-info cache invalidated and the right tab's left stale. Now `resolveTab`.
+> - `Target.targetCreated` / `targetDestroyed` are browser-level with no sessionId,
+>   so they are unaffected either way; the `targetDestroyed` exemption was kept.
+> - The `Network.*` / console arms already used `resolveTab` and simply start
+>   receiving events.
+>
+> One ordering hazard the widening exposed: with the filter keyed on
+> `sessionIdToTargetId`, events arriving during the six domain-enable round trips
+> would have been dropped as "unknown", so tab registration moved ahead of
+> `enableDomains` in both `attachFirstPage` and `switchTo`.
+> Tests: `test/cdp/session-events-test.ts` — four failed first (console, network,
+> dialog and load events from a background tab all discarded); the fifth, asserting
+> that a session this instance never attached is still ignored, passed before and
+> after and is the guard that the filter did not become a free-for-all.
+> No design decision turned out to be needed, so I did not stop.
+
+**Location** `src/cdp/session.ts:88-94`.
+
+```ts
+if (ev.method !== "Target.targetDestroyed") {
+  if (ev.sessionId && ev.sessionId !== sessionId) continue;
+}
+```
+
+**What breaks.** `sessionId` is the *active* tab's session. Every other attached tab's events are
+dropped at the top of the loop. Yet the same loop then routes through
+`resolveTab(ev.sessionId)` (`:151-178`), and `TabSession` carries a per-tab `networkBuffer` and
+`consoleBuffer` explicitly so that *"switching tabs preserves the previous tab's data in its
+TabSession instead of clearing it"* (`:13-16`). Those two designs contradict each other: the
+per-tab buffers can only ever hold data captured while their tab was the active one.
+
+Tabs stay attached after a switch (`tabs` and `sessionIdToTargetId` are never pruned on switch),
+and `browser_open_urls` / `browser_read_page` attach extra sessions of their own, so there is
+real traffic being thrown away.
+
+**Trigger.** Switch to tab B while tab A is loading; switch back to A; call `browser_console` or
+`browser_network_requests`. Everything A logged or fetched while B was active is absent, with no
+`bufferOverflowed` flag or any other signal that data is missing.
+
+**Impact** wrong result. Silently incomplete diagnostics — the failure mode the console and
+network tools exist to prevent. Note this **masks F3**: with the filter in place, the missing
+domain-enable on a revisit is partly invisible, and fixing either one alone gives a partial fix.
+
+**Severity** wrong result (silent data loss).
+
+**Proposed fix.** Narrow the filter to what it is actually for. Keep dropping events for
+sessions this `CdpSession` does not know (`!sessionIdToTargetId.has(ev.sessionId)`), and let
+known sessions through to `resolveTab`. Then move the three handlers that use
+`targetId ? tabs.get(targetId)` (`javascriptDialogOpening` at `:97`, `frameNavigated` /
+`loadEventFired` at `:117`) onto `resolveTab(ev.sessionId)` too, or a background tab's dialog
+will be recorded against the foreground tab.
+
+**Observable behaviour change.** Yes — background tabs accumulate console/network records, and
+a background tab's dialog is attributed to the right tab.
+
+**Reproducible in a test?** Yes. The stub transport's `events()` is a generator; replace it with
+a pushable queue, `switchTo("a")`, `switchTo("b")`, push a `Runtime.consoleAPICalled` carrying
+tab A's sessionId, `switchTo("a")`, assert `drainConsoleBuffer` returns it.
+
+---
+
+## F5 — `browser_fill` on a checkbox sets `value`, not `checked`, and reports success
+
+> **FIXED** in `16e7a30`, together with F10 — one engine in
+> `src/domains/fill-engine.ts`, used by both tools. A checkbox now goes through the
+> click/`checked` path instead of the `value` setter.
+> Tests: `test/domains/fill-engine-test.ts` executes the real in-page engine source
+> in Node against fake DOM elements — the "pure branch-decision seam" this report
+> recommended. 9 of its 16 assertions failed first, including the headline one:
+> after `browser_fill(checkbox, "true")`, `checked` was still `false`.
+
+**Location** `src/domains/form.ts:90-105` (`FILL_FN`).
+
+*Pre-confirmed item — verified, and I agree it is the worst row of the two-engines table.*
+
+**What breaks.** `FILL_FN` branches on `el instanceof HTMLInputElement` and, for any input,
+fires `HTMLInputElement.prototype.value`'s setter. For `type="checkbox"` / `type="radio"` that
+writes the element's *`value` attribute* — the string submitted when the box is ticked — and
+leaves `checked` untouched. It then dispatches `input` + `change` (so a framework listener runs
+and reads the *unchanged* checked state) and returns
+`{ status: "ok", kind: "checkbox", value: el.value }`, which the handler renders as
+`` Filled e7 = "true" ``.
+
+**Trigger.** `browser_fill({ ref: <a checkbox>, value: "true" })`. There is nothing in the tool
+description or `promptGuidelines` steering the agent away from it — `browser_fill` is
+described as the field-filling default, and `browser_snapshot` gives checkboxes refs like every
+other interactive element.
+
+**Impact** A **silent no-op reported as success**, plus corruption of the checkbox's submit
+value. The agent reads `Filled e7 = "true"`, believes the box is ticked, and moves on. The
+appended `interactiveDiff` does compare `state`, so a careful reader might notice no
+`checked` transition — but the primary result line says success.
+
+**Severity** wrong result — the most damaging failure mode in the report, because it is silent
+and the agent has no reason to doubt it.
+
+**Proposed fix.** Add a checkbox/radio branch to `FILL_FN` that either delegates to the
+`CHECK_FN` logic (`form.ts:379-394`, which clicks and falls back to `el.checked = want`) or
+returns `{ status: "not_fillable", tag }` with a message pointing at `browser_set_checked`.
+Failing loudly is strictly better than the current silence; delegating is better still, since
+`browser_fill_form` already accepts a boolean for the same element (F10).
+
+**Observable behaviour change.** Yes, and deliberately: a call that reported success now either
+works or errors.
+
+**Reproducible in a test?** The JS body is a string with no direct unit-test seam. Two options:
+(a) reproduce in `test/manual/forms-test.ts` against live Chrome — proves it, doesn't guard it;
+(b) restructure so the branch decision is a pure function of `{ tag, type, isContentEditable }`
+and unit-test that. I recommend (b) — it is the only way this class of bug stays fixed, and it
+covers F7 in the same seam.
+
+---
+
+## F6 — Every mutation silently renumbers the ref map, so refs held by the agent can point at different elements
+
+> **FIXED** in `fe90a9c`. `interactiveDiff` now carries each ref over by
+> `backendNodeId` (via a new `session.refMappings()`) and only mints a new `eN` for an
+> element it has not seen; the counter starts above every existing number so a new
+> element can never take a live ref.
+>
+> **Does this make refs genuinely stable, or narrow the window?** The ruling asked for
+> an explicit answer. **Genuinely stable for any element that persists across the
+> mutation** — which is the case the contract is about and the case that was silently
+> wrong. Two honest residuals:
+> - An element that disappears and later returns gets a **new** ref, because its old
+>   ref is dropped from the map when it vanishes. The agent sees it as new (`*[eN]`),
+>   which is truthful.
+> - `browser_snapshot` still renumbers from scratch, by design: an explicit
+>   re-snapshot is the documented way to get a fresh ref set.
+>
+> So the F6 divergence between `browser_snapshot`'s `interestingOnly`/`maxNodes` and
+> `interactiveDiff`'s hardcoded values no longer *renumbers* anything — a node absent
+> from the diff's narrower tree simply keeps no ref until the next snapshot.
+> Tests: `test/domains/ref-stability-test.ts`. Two of four failed first, with `e1`
+> resolving to backendNodeId 999 (the newly inserted element) instead of 101.
+> While writing them I found my own first fixture was wrong, not the product: real
+> CDP AX nodes carry `parentId`, and without it `buildTree` treats every child as an
+> additional root and collects it twice. Fixture corrected; no product bug there.
+
+**Location** `src/domains/ref-resolve.ts:75-94` (`interactiveDiff`), called from
+`form.ts:160` (fill), `form.ts:307` (select), `click.ts:83` (click).
+Compare `src/domains/snapshot.ts:332-344,371-376`.
+
+**What breaks.** After each mutation, `interactiveDiff` re-reads the AX tree, re-runs
+`collectInteractiveTargets`, and assigns refs by **positional index** — `e1, e2, …` in tree
+order — then calls `session.setRefMap(refMap, refSig)`, replacing the map wholesale. If the
+mutation changed the interactive element set (an error message with a link appears, a
+conditional field renders, a button unmounts), every ref at or after the insertion point now
+resolves to a **different element**. Not stale — *wrong*. `resolveRefToObjectId` cannot detect
+this: it looks up `e6`, gets a live `backendNodeId`, resolves it, and fills the wrong field.
+
+There is a second, independent divergence: `browser_snapshot` builds the tree with the
+caller's `args.interestingOnly` / `args.maxNodes`, while `interactiveDiff` hardcodes
+`{ interestingOnly: true, maxNodes: 1000 }`. A snapshot taken with `interestingOnly: false` or
+`maxNodes: 5000` produces a *different numbering*, which the first mutation then replaces —
+so refs shift even when the DOM did not change at all.
+
+**Trigger.** The documented happy path in the system prompt (`src/prompt.ts:64-72`):
+snapshot once, then `browser_fill(e7)`, `browser_select_option(e9)`, `browser_click(e12)`.
+Any validation message or conditional field appearing after the first fill shifts `e9` and
+`e12`. Reproducible deterministically with `browser_snapshot({ interestingOnly: false })`
+followed by any fill.
+
+**Impact** wrong result, silent, and it contradicts the contract the prompt and all seven tool
+descriptions state: *"refs survive re-renders"*, *"refs are keyed to element identity"*.
+What actually survives is the *backendNodeId behind a ref*, not the ref's meaning.
+
+**Severity** wrong result. High: it targets the wrong element while reporting success, and it
+sits directly on the recommended workflow.
+
+**Proposed fix.** Make refs identity-keyed rather than positional. Keep a per-tab
+`backendNodeId → ref` table and, when re-publishing in `interactiveDiff`, reuse the existing
+ref for any backendNodeId already known, minting `eN` only for genuinely new nodes. That
+preserves the diff (which already keys on `role|name`) and makes the "refs survive" claim true.
+Minimum viable alternative: have `interactiveDiff` *not* replace the map — only add newly
+appeared nodes — and use the same `buildTree` options the last snapshot used.
+
+**Observable behaviour change.** Yes — ref numbers stay stable across mutations, and the
+`*[eN]` markers in the diff change to reflect stable numbering.
+
+**Reproducible in a test?** Yes, at the pure level and without a browser:
+`buildTree(fixtureA)` → `collectInteractiveTargets` → refs; `buildTree(fixtureB)` where B is A
+plus one interactive node near the top → assert the trailing refs still map to the same
+`backendDOMNodeId`s. `test/deep-research/` already keeps AX-shaped JSON fixtures, so the
+pattern exists.
+
+---
+
+## F7 — `browser_fill` and `browser_select_option` write through a **disabled** control and report `status: "ok"`
+
+**Location** `src/domains/form.ts:90-122` (`FILL_FN`), `:254-268` (`SELECT_FN`).
+
+*Pre-confirmed item — verified.*
+
+**What breaks.** Neither function body checks `el.disabled`. `FILL_FN` fires the prototype
+`value` setter, which works fine on a disabled input; `SELECT_FN` assigns `el.value` on a
+disabled `<select>`. Both dispatch `input`/`change` and return `ok`. A real user cannot type
+into either control, so the page's own state machine never expected the value, and the value is
+not submitted.
+
+**Trigger.** `browser_fill` on a field the form disables until a precondition is met — a "Submit"
+step that stays disabled until terms are accepted, a dependent select that unlocks after a
+country is chosen.
+
+**Impact** wrong result. Same silent-success class as F5. `browser_fill_form` on the very same
+element returns `{ ok: false, reason: "element is disabled" }` (`form-batch.ts:10`), so the two
+tools give opposite answers about the same DOM.
+
+**Severity** wrong result.
+
+**Proposed fix.** Add `if (el.disabled) return { status: "not_fillable", tag }` — or a dedicated
+`{ status: "disabled" }` so the message can say *why* — to both bodies, matching
+`form-batch.ts:10` and `CHECK_FN` (`form.ts:385`), which both already do this.
+
+**Observable behaviour change.** Yes: a previously "successful" write now errors. That is the
+point, and it is the behaviour `browser_fill_form` already has.
+
+**Reproducible in a test?** Same seam as F5 — pure branch-decision function, or live Chrome.
+
+---
+
+## F8 — `browser_fill` reports `verified` without ever comparing it to the requested value
+
+**Location** `src/domains/form.ts:160-164`.
+
+*Pre-confirmed item — verified, and I rate it lower than the pre-confirmed list implies.*
+
+**What breaks.** The handler returns `details: { value: args.value, verified: res.value }` and
+text `` Filled ${target} = ${JSON.stringify(res.value)} ``. `res.value` is `el.value` read back
+after the write, but nothing compares it to `args.value`, and no mismatch is flagged. The tool
+description promises *"Returns the field's value after writing, plus a compact diff"* — literally
+true — while `promptGuidelines` says *"confirm both match what you intended"*, i.e. it delegates
+verification to the agent's own reading of a string.
+
+**Trigger.** Any input that transforms its value: `<input type="number">` given `"abc"` (becomes
+`""`), a masked phone input, `maxlength` truncation, an `<input type="date">` given a
+non-ISO string, or a controlled component that rejects and reverts.
+
+**Impact** wrong result, but **mitigated**: the correct value *is* in the output text, so an
+attentive agent can catch it. `browser_fill_form` computes an explicit `mismatch` flag
+(`form-batch.ts:100-106`) and surfaces it in the summary line, which is what `browser_fill`
+should do.
+
+**Severity** wrong result (mild — the data is present but unjudged).
+
+**Proposed fix.** Mirror `form-batch.ts`: compute `mismatch = res.value !== args.value`, put it
+in `details`, and prefix the text (e.g. `` Filled … = "12" (requested "abc" — MISMATCH) ``).
+Do not turn it into an error: legitimate normalisation (trimming, date reformatting) would
+start failing.
+
+**Observable behaviour change.** Yes — an extra `mismatch` field and a warning in the text.
+
+**Reproducible in a test?** Yes, and this one is easy: the mismatch computation is pure over
+`(args.value, res.value)`. Extract it (or share `form-batch`'s) and unit-test it.
+
+---
+
+## F9 — `browser_type`'s focus guard is fail-open when the probe itself fails
+
+**Location** `src/domains/keyboard.ts:32-50`.
+
+*Pre-confirmed as a coverage hole. My independent call: it is also a live fail-open, of exactly
+the shape the brief warns about.*
+
+```ts
+const focused = await client.evaluateJs(safeJs`…`);
+if (focused.success) {          // ← probe failure skips the guard entirely
+  const f = isFocusProbe(focused.data) ? focused.data : { ok: false, tag: null };
+  if (!f.ok) return err({ kind: "invalid_state", message: "No editable element is focused — typed text would be lost." });
+}
+const r = await cdpCall(client, "Input.insertText", { text: args.text });
+```
+
+The guard exists because typed text goes nowhere when nothing editable is focused. When the
+probe cannot answer — the case where we know *least* about focus — the code proceeds to type
+anyway and returns `` Typed: "…" ``. Note the inner fallback is correctly fail-closed
+(`{ ok: false }` on a bad shape), which makes the outer fail-open look unintended rather than
+chosen.
+
+**Trigger.** Anything that makes `client.evaluateJs` fail: a page-level CSP that blocks
+evaluation, a detached/navigating frame, a transient timeout, or a JS dialog freezing the page
+(the most likely one — a dialog is exactly when typed text is lost).
+
+**Impact** wrong result. Text silently discarded and reported as typed. Same class as F5,
+narrower trigger.
+
+**Severity** wrong result.
+
+**Proposed fix.** Treat probe failure as "not focused": add an `else` returning
+`invalid_state` with the probe's error message. If the team prefers not to block on a flaky
+probe, keep typing but say so in the result text — never return an unqualified success.
+
+**Observable behaviour change.** Yes — `browser_type` starts failing in a case where it
+previously claimed success.
+
+**Reproducible in a test?** Yes, but it needs a fake `BrowserClient` (the check runs through
+`client.evaluateJs`). Return `err(cdpError("timeout", …))` from the fake and assert the handler
+returns `invalid_state` rather than calling `Input.insertText`. A `makeFakeClient()` helper is
+the missing piece; several other findings here (F2, F14) want the same helper.
+
+---
+
+## F10 — Two fill engines coexist and disagree on the same element
+
+**Location** `src/domains/form.ts:90-122` (`browser_fill`) vs `src/domains/form-batch.ts:6-58`
+(`browser_fill_form`).
+
+> **FIXED** in `16e7a30`, per the ruling: one engine, `browser_fill` delegates, and its
+> documented `<select>` rejection survives as an explicit `rejectSelect` pre-check
+> rather than as a happy accident of the old code falling through to "not fillable".
+> Per-tool focus behaviour is a `focusFirst` parameter, so `browser_fill`'s documented
+> "does not focus" promise still holds and `browser_fill_form` still focuses.
+> `browser_fill` keeps appending `interactiveDiff` and republishing refs;
+> `browser_fill_form` still does neither. The merged engine is the **union** of the two
+> columns, so `browser_fill_form` gained the `execCommand` contenteditable path — the
+> third behaviour change listed under Resolution status.
+
+*Pre-confirmed item — verified; the table in the brief matches the code exactly.* Both are
+registered and agent-callable (`src/registry.ts:29-31`).
+
+| case | `browser_fill` | `browser_fill_form` |
+|---|---|---|
+| disabled input | writes, `ok` (F7) | `{ ok:false, reason:"element is disabled" }` |
+| `<select>` | error, "use browser_select_option" | matches by `value`/`label`/`text`, fills |
+| **checkbox** | prototype `value` setter, `ok` (F5) | flips `checked` via `click()` |
+| contenteditable | `execCommand("insertText")` — drives ProseMirror/Slack | `el.textContent =` — does not |
+| result | appends `interactiveDiff`, republishes refs | neither |
+| verification | reports `verified`, no comparison (F8) | explicit `mismatch` flag |
+
+**Impact** The agent cannot reason about which tool to use, and the tool descriptions do not
+warn it. `browser_fill_form` is documented as *"Preferred for forms"*, but it is the **weaker**
+engine for contenteditable (the `textContent` assignment does not drive rich-text editors,
+which is the one thing `FILL_FN`'s `execCommand` path exists for) and it returns no page diff.
+Meanwhile `browser_fill` is the weaker engine for checkboxes, selects, and disabled controls.
+
+**Severity** wrong result (umbrella; F5, F7, F8 are its individually-testable instances).
+
+**Proposed fix.** One engine. Extract the per-element-type decision + write into a single JS
+body used by both tools, with `browser_fill` as the single-field entry point (keeping the diff)
+and `browser_fill_form` as the batch entry point (keeping `mismatch`). The union of the two
+tables is strictly better than either column. This is a bigger change than the other fixes and
+should be planned, not folded into a bug-fix commit.
+
+**Observable behaviour change.** Yes, substantially, in both tools. Needs the user's sign-off on
+the merged semantics, not just on "is it a bug".
+
+**Reproducible in a test?** Once the decision logic is a pure function, yes — one table-driven
+test per row. Today, no: both bodies are opaque strings and neither tool has any automated
+coverage (F30).
+
+---
+
+## F11 — Concurrent liveness probes can spawn a second daemon that unlinks the live one's socket
+
+**Location** `src/daemon/spawn.ts:41-101` (`isDaemonRunning`), `src/daemon/server.ts:83-89`
+(`unlinkSync`), `:121-135` (duplicate-clientId rejection),
+`src/daemon/protocol.ts:165` (`DAEMON_STALE_SOCKET_CLEANUP = true`).
+
+**What breaks.** Three facts compose into a bad outcome:
+
+1. `isDaemonRunning` registers with the **fixed** clientId `"_liveness_probe"` (`spawn.ts:70`).
+2. `server.ts:126-134` rejects a registration whose clientId is already connected — it writes a
+   `shutdown` control and destroys the socket.
+3. On rejection the probe's `close` handler fires `done(false)` (`spawn.ts:96-99`), so
+   `isDaemonRunning` reports **"not running"** for a daemon that is running and healthy.
+   `ensureDaemon` then spawns a second daemon, whose `start()` unconditionally
+   `unlinkSync(DAEMON_SOCKET_PATH)` (`server.ts:83-89`) — deleting the *live* daemon's socket
+   inode — and listens on a fresh one.
+
+The `close` handler's comment says *"daemon alive but rejected us (e.g. max clients); don't
+unlink"* — so the author anticipated rejection, but the return value still says "not running",
+which is what drives the spawn.
+
+**Trigger.** Two pi sessions calling `/browser-setup` (or `browser_setup`, or any first tool
+call) within the ~2 s probe window. The `_liveness_probe` slot is also never freed on the
+reject path, so a probe that races itself in one process can do it too.
+
+**Impact** Two daemons, two WebSockets to Chrome, two "Allow Remote Debugging" prompts — the
+exact thing the daemon exists to eliminate. Sessions already connected keep working (their FD
+survives the unlink), so the split is invisible until the user is prompted again. The orphaned
+daemon lingers until its 30-minute idle timeout, if it ever reaches zero clients.
+
+**Severity** resource leak, with user-visible fallout (repeat Chrome prompts).
+
+**Proposed fix.** Two independent parts:
+- Make the probe clientId unique: `` `_liveness_probe-${randomUUID()}` ``. Removes the collision
+  entirely and costs nothing.
+- Distinguish "rejected by a live daemon" from "nothing listening". A received `shutdown`
+  control means the daemon is alive — `isDaemonRunning` should return `true`. And gate
+  `unlinkSync` on the socket not being live rather than doing it unconditionally.
+
+**Observable behaviour change.** No, in the normal single-session path. In the race, one daemon
+instead of two.
+
+**Reproducible in a test?** Yes — `test/manual/ipc-server-test.ts` and
+`live-daemon-test.ts` already drive the real server over a real socket. A test that registers
+`_liveness_probe` and then calls `isDaemonRunning()` and asserts `true` is straightforward.
+It lands in `test/manual/`, which the `npm test` glob excludes, so it would not run in CI
+without the daemon-test infrastructure F30 asks for.
+
+---
+
+## F12 — `openerId` is probably not a `Target.createTarget` parameter, so unpinned new tabs are placed by focus, not by ownership
+
+**Location** `src/cdp/commands.ts:90-99` (schema), `src/cdp/target-factory.ts:205-214`
+(`openHarnessTab`, unpinned path).
+
+> **LEFT UNFIXED BY RULING — this is the highest-value thing a human with a browser can
+> check.** Neither I nor the reviewer can run live Chrome. The experiment: call
+> `Target.createTarget` with `openerId` set to an owned tab, then read the new target's
+> `TargetInfo.openerId` and which window it joined. If Chrome ignores the parameter,
+> then unpinned `browser_open_urls` / `browser_new_tab` tabs land in whichever window has
+> **focus** — the user's window, not the harness's — and the session-boundary promise in
+> `src/prompt.ts:98-105` is false. The fix would then be to pass `windowId:
+> ownership.harnessWindowId()`, a documented parameter the harness already tracks for
+> exactly this purpose and already has in its schema.
+
+*Pre-confirmed item, flagged for verification. My finding: it looks wrong, and I could not
+confirm it either way from documentation.*
+
+**What breaks.** The command table declares `openerId: Type.Optional(Type.String())` as a
+`Target.createTarget` parameter and `openHarnessTab` passes it to place a new tab "beside"
+`openerTargetId`. Querying the CDP protocol definitions, `openerId` appears on **`TargetInfo`**
+("Opener target Id") and I found no `openerId` in `CreateTargetRequest`. The documented
+`Target.createTarget` parameters are `url`, `left`, `top`, `width`, `height`, `windowId`,
+`browserContextId`, `enableBeginFrameControl`, `newWindow`, `background`, `forTab`.
+
+If it is not a parameter, Chrome ignores the unknown field and the new tab lands wherever a
+bare `createTarget` lands — the focus-derived window. `target-factory.ts`'s own header says
+*"`openerId` for siblings inside it"* while also stating (verified, for the pinned case)
+that *"`openerId` does NOT inherit the opener's context"* — which is consistent with the field
+being ignored rather than honoured. That the unpinned path appears to work in practice is
+explained by the harness window usually being the focused one.
+
+**Trigger.** Unpinned profile, `browser_open_urls` or `browser_new_tab`, while a user window
+has focus. The tabs would open in the user's window — still `ownership.add`-ed, so the harness
+believes they are in its dedicated window.
+
+**Impact** wrong result, and it breaks the session-boundary promise in `prompt.ts:98-105`
+(*"you operate only inside this session's own Chrome window"*). Also `windowId` — an actual,
+documented parameter — is in the schema and available (`ownership.harnessWindowId()`) but is
+never passed.
+
+**Severity** wrong result. **Unverified** — needs one live-Chrome experiment.
+
+**Proposed fix.** First verify: call `Target.createTarget` with `openerId` against live Chrome
+and check whether the new target's `TargetInfo.openerId` is set and which window it joins. If
+ignored, replace it with `windowId: ownership.harnessWindowId()`, which is exactly the
+documented mechanism for "put this tab in that window" and which the harness already tracks
+for this purpose. If it turns out to be honoured, delete the stale comment and note it.
+
+**Observable behaviour change.** Yes if changed — new tabs would reliably land in the harness
+window rather than the focused one.
+
+**Reproducible in a test?** Not as a unit test — the question is about Chrome's behaviour. A
+`test/manual/window-binding-test.ts` addition (that file already exists) is the right home.
+The *schema* claim can be tested trivially once known: assert the table matches the protocol.
+
+---
+
+## F13 — A `session_tree` event before client creation resets the namespace to `"default"`
+
+**Location** `src/index.ts:195-198`; `src/state.ts:51-68` (`restoreState`).
+
+*Pre-confirmed item, flagged "confirm whether the fallback makes these equivalent". It does
+not, in one specific case.*
+
+**What breaks.** `session_tree` passes `client?.namespace`, which is `undefined` before the
+client exists (the client is created in `session_start`). Inside `restoreState`:
+
+```ts
+const fallback = defaultState(currentNamespace);   // namespace = undefined ?? "default"
+…
+namespace: currentNamespace ?? data.namespace ?? fallback.namespace
+```
+
+With a persisted entry present, `data.namespace` wins and the two call sites *are* equivalent.
+With **no** persisted entry, the loop falls through to `return fallback` — whose namespace is
+`"default"`, not the `pi-<random>` value `index.ts:40` generated. `persistState` then writes
+`"default"`, and the subsequent `session_start` reads it back and creates the client (and the
+daemon transport) with `namespace: "default"`.
+
+**Trigger.** A `session_tree` event arriving before `session_start` in a session with no prior
+`browser-harness-state` entry — a fresh session on a fresh branch. Whether pi actually orders
+the events that way I could not determine from this repo; the ordering is the host's.
+
+**Impact** Every affected pi session shares the daemon clientId `"default"`, so the second one
+to connect is rejected by `server.ts:126-134` ("clientId default is already connected") and
+gets its socket destroyed. Multi-session browser control breaks, and the error surfaces as a
+generic transport failure. If `--browser-namespace` was passed, `flagNs` is not consulted here
+either, so an explicit user choice is discarded.
+
+**Severity** wrong result (conditional on host event ordering; **unverified** that the ordering
+occurs).
+
+**Proposed fix.** Pass the namespace that is authoritative, not the one that happens to be
+reachable: `restoreState(ctx, client?.namespace ?? state.namespace)`. `state.namespace` is
+initialised from the flag or the generated value at `:40-42`, so it is never `undefined` — which
+also makes the two call sites genuinely equivalent and removes the trap.
+
+**Observable behaviour change.** None in the ordering where `session_start` runs first. In the
+other ordering, the namespace stops being clobbered.
+
+**Reproducible in a test?** Yes, and cheaply — `restoreState` takes only an `ExtensionContext`
+whose `sessionManager.getBranch()` returns an array. A test asserting
+`restoreState(ctxWithNoEntries, undefined).namespace` is not silently `"default"` when the
+caller had a real namespace pins the contract. Reproducing the *event ordering* needs a pi
+host fake, which does not exist here.
+
+---
+
+## F14 — Both `AbortController` time budgets are dead code: the pre-`await` check can never fire
+
+**Location** `src/domains/snapshot.ts:346-366` (1 500 ms box budget) and
+`src/domains/network.ts:249-266` (5 000 ms `BODY_BUDGET_MS`).
+
+*This is the "signal.aborted checked before an await, not re-checked after" item from the
+brief's list — present in two places, in a slightly worse form than described.*
+
+**What breaks.** Both sites build the whole array of promises with `.map(async …)` and then
+await them together. `.map` starts every async function **synchronously**, so the guard at the
+top of each callback runs before the timer can possibly have fired:
+
+```ts
+const timer = setTimeout(() => ac.abort(), BODY_BUDGET_MS);
+const fills = drained.records.map(async (r) => {
+  if (ac.signal.aborted) return;                  // always false — same tick
+  const bodyRes = await session.call("Network.getResponseBody", …);   // never re-checked
+  …
+});
+await Promise.allSettled(fills);
+```
+
+`session.call` takes no `AbortSignal`, so aborting cannot cancel an in-flight request either.
+Net effect in `network.ts`: the 5 s budget bounds nothing at all — all N `getResponseBody`
+calls are issued, and the tool waits for every one (each up to the 15 s transport timeout).
+In `snapshot.ts` the *first* check is likewise dead, so no `DOM.getBoxModel` is ever skipped;
+only the **second** check (`:360`, after the `boxOf` await) does anything, and it only skips the
+`liveValue` follow-up.
+
+**Trigger.** `browser_network_requests({ includeResponseBodies: true })` matching many records
+on a slow page. `browser_snapshot` on a page with many interactive elements.
+
+**Impact** wrong result via latency: a tool documented as budgeted can block far past its
+budget, and the caller sees a plain timeout with no partial data. Not a crash; nothing is
+mis-stated in the output.
+
+**Severity** wrong result (latency/robustness). Low-to-medium.
+
+**Proposed fix.** Bound the whole batch instead of guarding inside each task:
+`await Promise.race([Promise.allSettled(tasks), sleep(BUDGET_MS)])`, and record per-record
+completion so partial results are reported honestly. If per-task cancellation is wanted, thread
+a `timeoutMs` (already supported by `session.call`) derived from the remaining budget —
+`src/util/time.ts` already exports a `deadline()` helper built for exactly this and currently
+has no callers in `src/`.
+
+**Observable behaviour change.** Yes — the tool returns within its budget with partial bodies
+instead of eventually returning all of them.
+
+**Reproducible in a test?** Yes for `network.ts`, with a fake client whose
+`session().call("Network.getResponseBody")` resolves after a controllable delay: assert the
+handler returns within ~`BODY_BUDGET_MS` for 50 records at 200 ms each. Needs the same
+`makeFakeClient()` helper as F2/F9.
+
+---
+
+## F15 — `asArrayOf` is all-or-nothing, so one malformed record blanks an entire TUI render
+
+**Location** `src/util/guards.ts:13-25`; consumers `src/domains/console.ts:139` and
+`src/domains/network.ts:209`.
+
+*Pre-confirmed item — verified. Severity: cosmetic, not wrong-result, because it is
+render-only.*
+
+**What breaks.** `asArrayOf` returns `undefined` if **any** element fails its item guard. In
+`asConsoleDetails` / `asNetworkDetails` a single bad record makes the whole `records` array
+`undefined`, which makes the details `undefined`, which makes `renderResult` return
+`"console: no details"` / `"network: no details"` — hiding all the good records.
+
+**Trigger.** A record that fails the item guard. Both item guards require few fields
+(`asConsoleRecord` needs a valid `level` and a string `text`; `asNetworkRecord` needs `method`,
+`url`, `type`), and the producing buffers always populate them, so this is hard to hit from
+inside the harness. It becomes reachable if a transcript is replayed after a schema change, or
+if `details` is round-tripped through a lossy serializer.
+
+**Impact** cosmetic — the LLM's copy of the data comes from `ToolOk.text`, which is built
+directly from `drained.records` and unaffected. Only the human's TUI panel goes blank.
+
+**Severity** cosmetic.
+
+**Proposed fix.** Add a lenient sibling — `asArrayOfLenient` / `compactMap`, which drops
+unparseable elements and keeps the rest — and use it in the two render paths only. Leave
+`asArrayOf` strict where a partial array would be wrong (`state.ts:39`, `ownedTargetIds`:
+silently dropping a target id there would un-own a live tab, so strict is correct and should
+stay).
+
+**Observable behaviour change.** Only in the failure case: a partial render instead of a blank one.
+
+**Reproducible in a test?** Yes, directly — `test/util/guards-test.ts` already exists and
+`asArrayOf` is pure.
+
+---
+
+## F16 — A strict `AxNode` schema means one bad node fails the whole `browser_snapshot`
+
+**Location** `src/cdp/commands.ts:55-76,239-242`; consumed by `snapshot.ts:313` and
+`ref-resolve.ts:79`.
+
+*Pre-confirmed item — verified. My judgement: leave it, add a guard rail.*
+
+**What breaks.** `Accessibility.getFullAXTree`'s result is now `Type.Array(AxNode)`. A single
+node failing validation makes `decodeResult` reject the entire response, so `browser_snapshot`
+returns `invalid_response` and `interactiveDiff` silently returns `""`. Before the schema work
+this was `Array(Unknown())` and could not fail.
+
+**Trigger.** A node where `nodeId` is absent or not a string — the only required field. CDP
+always sends `nodeId` as a string, and everything else is optional with
+`additionalProperties: true`, so I could not construct a realistic trigger. A future Chrome
+change to the AX payload could.
+
+**Impact** If it ever fires, the harness's **default** page-inspection tool stops working
+entirely, and the `interactiveDiff` failure is invisible (returns `""`, callers append nothing).
+The blast radius is large; the probability is low.
+
+**Severity** cosmetic today (unreachable), crash-class if it ever fires.
+
+**Proposed fix.** The low-cost mitigation is per-node leniency at the consumer, not a looser
+schema: keep `Type.Array(AxNode)`, but if the decode fails, fall back to filtering nodes that
+individually validate and proceed with those, noting the count in `details`. Alternatively add
+a `depth` cap. I would **not** revert to `Array(Unknown())` — that discards the type safety
+Task 13 bought for a risk that has no demonstrated trigger.
+
+**Observable behaviour change.** Only in the failure case.
+
+**Reproducible in a test?** Yes — `test/cdp/commands-test.ts` exists; `decodeResult` is pure.
+Feed a nodes array with one `{ nodeId: 42 }` and assert the whole decode fails today.
+
+---
+
+## F17 — Every `Input.*` command's parameters are unvalidated
+
+**Location** `src/cdp/commands.ts:228-231`; worst consumer `src/domains/drag.ts:39-59`.
+
+*Pre-confirmed item — verified.*
+
+**What breaks.** `Input.dispatchMouseEvent`, `Input.dispatchKeyEvent`, and
+`Input.dispatchDragEvent` are all `cmd(Type.Object({}, { additionalProperties: true }), Empty)`.
+Any object satisfies them, so `ParamsOf<M>` is effectively `Record<string, unknown>` and a typo
+(`deltaY` → `delatY`, `clickCount` → `clickcount`) typechecks, passes the boundary scanner, and
+reaches Chrome, which ignores the unknown field. `drag.ts`'s `InputCall` tuple types its second
+element as `Record<string, unknown>` for the same reason, and `type InputCall` is declared inside
+the handler body rather than at module scope.
+
+**Trigger.** A typo or a wrong-typed value in any of the ~10 `Input.*` call sites
+(`click.ts:42-51`, `scroll.ts:48-55`, `keyboard.ts:90-108`, `drag.ts:41-58`). Nothing catches it;
+the action silently does the wrong thing or nothing.
+
+**Impact** No live bug found — I checked every call site against the CDP parameter names and
+they are correct. This is a missing guard rail, not a defect.
+
+**Severity** cosmetic (latent).
+
+**Proposed fix.** Write out the real parameter sets: `type`, `x`, `y`, `button`, `buttons`,
+`clickCount`, `deltaX`, `deltaY`, `modifiers`, `pointerType` for `dispatchMouseEvent`; `type`,
+`key`, `code`, `text`, `unmodifiedText`, `windowsVirtualKeyCode`, `nativeVirtualKeyCode`,
+`modifiers` for `dispatchKeyEvent`; `type`, `x`, `y`, `data`, `modifiers` for
+`dispatchDragEvent`. Keep `additionalProperties: true` so a future CDP field is not a hard
+error. Then `drag.ts`'s tuple can be typed properly and `InputCall` hoisted to module scope.
+
+**Observable behaviour change.** None — same wire payloads.
+
+**Reproducible in a test?** Not as a runtime test (the current code is correct). It is a
+compile-time guarantee: a `// @ts-expect-error`-style negative test, or simply the typecheck.
+
+---
+
+## F18 — `browser_snapshot` silently omits `screenshotPath` when the screenshot fails
+
+**Location** `src/domains/snapshot.ts:383-390`, with `:399` and the renderer at `:419`.
+
+*Pre-confirmed item — verified.*
+
+**What breaks.**
+
+```ts
+if (args.includeScreenshot) {
+  const shot = await session.call("Page.captureScreenshot", …);
+  if (shot.success) { shotPath = …; writeFileSync(…); }   // no else
+}
+```
+
+A failed capture leaves `shotPath` undefined, `details.screenshotPath` is omitted, and the
+renderer prints `"screenshot omitted"` — the identical wording it uses when the caller never
+asked for one. The caller who explicitly passed `includeScreenshot: true` is told nothing.
+
+**Trigger.** `browser_snapshot({ includeScreenshot: true })` where `Page.captureScreenshot`
+fails — a detached target, a page mid-navigation, or a GPU/compositor hiccup.
+
+**Impact** cosmetic: the AX tree, the refs, and everything else the tool exists for are correct.
+Only the screenshot's absence is unexplained.
+
+**Severity** cosmetic.
+
+**Proposed fix.** Add `details.screenshotError = shot.error.message` on the failure path and
+render `"screenshot failed: …"` instead of `"screenshot omitted"`. Do not fail the tool — the
+snapshot itself succeeded.
+
+**Observable behaviour change.** Yes — a new `details` field and a distinct message.
+
+**Reproducible in a test?** Yes with a fake client (`Page.captureScreenshot` → `err`). Same
+missing helper as F2/F9/F14.
+
+---
+
+## F19 — A bad `outputPath` throws out of the handler instead of returning `io_error`
+
+**Location** `src/domains/files.ts:212-221` (`browser_print_to_pdf`);
+also `src/domains/snapshot.ts:388` (`writeFileSync`) and
+`src/domains/screenshot.ts:40` (`writeFile`).
+
+**What breaks.** `printToPdfTool` does `await writeFile(path, …)` with `path = args.outputPath ?? pdfPath(…)`
+— caller-supplied, unvalidated, and outside any try/catch. An `ENOENT`/`EACCES`/`EISDIR`
+rejection escapes the handler as a thrown exception instead of the `Result<ToolOk, ToolErr>`
+every tool is contracted to return. `registerBrowserTool`'s `execute` has no try/catch around
+`def.handler` (`util/tool.ts:178-196`) — only a `finally` that releases the mutex — so the
+rejection propagates into pi's tool runner. The mutex *is* released, so there is no deadlock.
+
+The other two sites write to `screenshotPath()`/`pdfPath()` paths under `tmpdir()`, so they are
+only reachable on a full or read-only tmpdir — but they are the same unguarded shape.
+
+**Trigger.** `browser_print_to_pdf({ outputPath: "/nonexistent/dir/out.pdf" })`, or a path the
+process cannot write, or a directory path.
+
+**Impact** The agent gets a raw host-level error instead of a typed `io_error` it can act on.
+Every sibling in `files.ts` (`downloadTool`, `verifyReadable`) validates its path carefully and
+returns `io_error` — this one path does not.
+
+**Severity** crash (of the tool call), no data loss.
+
+**Proposed fix.** Wrap the three writes and map to `{ kind: "io_error", message }`. Separately,
+add a try/catch around `def.handler` in `util/tool.ts:178` that maps an unexpected throw to
+`{ kind: "internal" }` — a defence-in-depth fix that covers all 39 tools at once and is the
+better first move.
+
+**Observable behaviour change.** Yes — a typed tool error instead of a thrown exception.
+
+**Reproducible in a test?** Yes for the `util/tool.ts` guard: register a tool whose handler
+throws and assert `execute` resolves to an `isError` result. For `printToPdf` specifically it
+needs the fake client.
+
+---
+
+## F20 — Aborting `browser_wait_for` throws instead of returning a typed error
+
+**Location** `src/domains/page.ts:132` (`await sleep(50, signal)`); `src/util/time.ts:1-16`.
+
+**What breaks.** `sleep` **rejects** when the signal aborts. `waitTool` wraps its `sleep` in
+try/catch (`page.ts:52-57`); `waitForTool` calls it bare inside the poll loop. The loop's own
+`if (signal?.aborted)` check runs at the top of each iteration, so aborting *during* the 50 ms
+sleep rejects out of the handler. Same unguarded-throw path as F19.
+
+Note `waitForLoadTool` (`:87`) uses a raw `new Promise(setTimeout)` rather than `sleep`, so it
+is not affected — the three wait tools use three different sleep strategies.
+
+**Trigger.** Cancelling `browser_wait_for` (Esc / agent cancellation) within a 50 ms window.
+
+**Impact** A thrown exception instead of the `{ kind: "internal", message: "aborted" }` the
+same function returns when the check wins the race. Inconsistent, not damaging.
+
+**Severity** cosmetic.
+
+**Proposed fix.** Wrap the loop body, or make the three wait tools share one abortable-sleep
+helper that returns a `Result` rather than throwing. The blanket handler try/catch from F19
+also covers it.
+
+**Observable behaviour change.** Yes — a typed error on cancellation.
+
+**Reproducible in a test?** Yes — `sleep` is pure and needs no client; the handler needs a fake
+client for `evaluateJs`. The `sleep`-rejects half is testable today.
+
+---
+
+## F21 — `openIsolatedTab` can adopt and then close the harness window's only tab
+
+**Location** `src/domains/isolated-tab.ts:35-63` and `:112-114` (`closeIsolatedTab`);
+`src/cdp/target-factory.ts:144-182`.
+
+**What breaks.** When `ensureHarnessWindow` reports `freshlyCreated: true`,
+`openIsolatedTab` uses the **seed tab of the newly created harness window** as its isolated tab.
+`ensureHarnessWindow` has, by then, recorded that same target as `harnessWindow` and added it to
+ownership. `closeIsolatedTab` closes it in the `finally`, so the window that was just
+established is destroyed and the window binding is dropped (`ownership.remove` clears
+`harnessWindow` when the ids match, `ownership.ts:54-57`).
+
+**Trigger.** `browser_web_search` or `browser_read_page({ url })` at a moment when no owned tab
+is live. `ensureAlive` normally runs first and seeds a window, so the window usually already
+exists and `freshlyCreated` is false — the exposure is the narrow window where the user closes
+the harness tabs between `ensureAlive` and the isolated-tab call. Also reachable when the
+concurrency coordinator hands `freshlyCreated: true` to the creating caller while a second
+concurrent `browser_read_page` is in flight.
+
+**Impact** With a pinned profile, the next tool call re-runs `seedPinnedProfileWindow`, which
+**launches a browser window via the command line** — visible, slow (up to 15 s of sentinel
+polling), and disruptive. If the session was attached to that seed tab, the next call also gets
+`session_not_found` and re-attaches. It contradicts the tool's documented promise:
+*"Runs in its own isolated tab and never disturbs the user's current tab."*
+
+**Severity** resource leak / lifecycle churn (no wrong data).
+
+**Proposed fix.** Never let an isolated tab *be* the harness window's seed tab: after
+`ensureHarnessWindow` returns `freshlyCreated: true`, still call `openHarnessTab` to get a
+sibling, and leave the seed alone. That is a two-line change and makes the seed tab's role
+(a durable window anchor) consistent across all callers.
+
+**Observable behaviour change.** One extra tab open+close per isolated read in the fresh-window
+case; the harness window survives.
+
+**Reproducible in a test?** Only with a fake client that can report "no owned tabs" and record
+`Target.closeTarget` calls. Fiddly but doable; the race variant is not deterministically
+reproducible. **Partly unverified** — I traced it in the code but did not observe it.
+
+---
+
+## F22 — The daemon never releases session ownership on `Target.detachFromTarget`
+
+**Location** `src/daemon/bridge.ts:386-391`; caller `src/client.ts:375-383`.
+
+**What breaks.** The bridge does `if (isDetach && req.sessionId) router.release(req.sessionId)`.
+But `client.detach()` sends `Target.detachFromTarget` with the session id in **`params`**, at
+browser level (`{ sessionId: null }` transport option), so `req.sessionId` on the envelope is
+`undefined` and the release never runs. `EventRouter.owners` keeps the mapping for a session
+that no longer exists.
+
+**Trigger.** `session_shutdown` (which calls `client.detach()`), every session, every time.
+
+**Impact** A slow leak in a long-lived daemon: `owners` grows by one entry per detached session
+and is only pruned by an `Inspector.detached` event (`bridge.ts:199-201`) or `removeClient` on
+disconnect. Since the transport deliberately survives session shutdown
+(`index.ts:184-187`), `removeClient` does not fire. Events for a dead session would also still
+be routed to the stale owner — harmless, since Chrome stops sending them.
+
+**Severity** resource leak (small, bounded by daemon lifetime and the 30-minute idle exit).
+
+**Proposed fix.** Read the id from either place:
+`const sid = req.sessionId ?? asString(req.params?.["sessionId"])`. `asString` is already
+imported in `bridge.ts`.
+
+**Observable behaviour change.** None observable; the map shrinks.
+
+**Reproducible in a test?** Yes — `handleRequest` is a plain function taking a `WireRequest`
+and a `send` callback; `getSessionOwner` is exported for the assertion. It lands in
+`test/manual/bridge-test.ts`, outside the CI glob (F30).
+
+---
+
+## F23 — `handleRequest`'s promise is dropped with no `.catch`
+
+**Location** `src/daemon/index.ts:24-32`.
+
+**What breaks.** `cdpBridge.handleRequest(msg, client.id, send)` returns a `Promise<void>` that
+is neither awaited nor `.catch`-ed, inside a `readline` `"line"` handler. Any rejection is an
+unhandled rejection, which on Node ≥ 15 terminates the process by default — taking the daemon,
+and every connected pi session's browser control, with it.
+
+**Trigger.** No reachable throw found. `handleRequest`'s only `await` is a `setTimeout` sleep;
+`JSON.stringify(payload)` and `ws.send` are inside a try/catch; `mux.allocate` cannot throw.
+A future edit that adds an unguarded `await` in that function makes it reachable.
+
+**Impact** None today. This is the same latent shape as the `bridge.ts:156` `JSON.parse` bug
+that Task 4 fixed — an unguarded failure path inside a socket handler with nothing above it —
+and it sits in the same file's blast radius.
+
+**Severity** cosmetic today, crash-class if a throw is ever introduced.
+
+**Proposed fix.** `void cdpBridge.handleRequest(msg, client.id, send).catch((e) => { … send an
+error response … });` — or make it explicit with `process.on("unhandledRejection")` logging in
+the daemon entry point. Cheap insurance for a process whose whole job is to stay up.
+
+**Reproducible in a test?** Yes at the seam — inject a bridge whose `handleRequest` rejects and
+assert the process/handler survives. Needs the daemon test infrastructure from F30.
+
+---
+
+## F24 — `interactiveDiff` keys the diff on `role|name`, so duplicate-named elements collapse
+
+**Location** `src/domains/ref-resolve.ts:96-126`.
+
+**What breaks.** The diff builds `prevByKey` / `curByKey` maps keyed on `` `${role}|${name}` ``.
+Any two interactive elements sharing a role and an accessible name overwrite each other, so only
+the last one is compared. The comment explains the choice (*"Compare by stable identity
+(role|name) rather than ref position, since refs renumber"*) — which is itself the symptom of F6.
+
+**Trigger.** Very common: a table with a "Delete" button per row; several unlabelled textboxes
+(all `textbox|`); repeated "Add to cart" buttons.
+
+**Impact** The "Page changes" block under-reports. A value change on row 3's field can be
+attributed to row 7's, or dropped. Since the whole point of the diff is *"read it to confirm the
+change landed"* (`prompt.ts:30-31`), an agent trusting it can be misled — but the primary result
+line (`Filled e7 = "…"`) is still correct, so nothing depends solely on the diff.
+
+**Severity** cosmetic (misleading advisory output).
+
+**Proposed fix.** Falls out of F6's fix for free. With identity-stable refs, key the diff on
+`backendNodeId` and the collapse disappears. Not worth fixing independently.
+
+**Reproducible in a test?** Yes, pure, from two AX fixtures with duplicate `role|name` pairs.
+
+---
+
+## F25 — `limit: 0` would make both buffers return everything (investigated — not a live bug)
+
+**Location** `src/cdp/console-buffer.ts:227-228`, `src/cdp/network-buffer.ts:174-175`.
+
+```ts
+const limit = Math.min(filter.limit ?? 50, 500);
+const limited = matched.slice(-limit);
+```
+
+With `limit === 0`, `-0` is `0` and `matched.slice(0)` returns the **entire** array — the exact
+opposite of "return nothing".
+
+**Why it is not a live bug.** Both tool schemas set `minimum: 1` on `limit`
+(`console.ts:35`, `network.ts:80`), so a validated `0` cannot arrive. `drainConsoleBuffer` /
+`drainNetworkBuffer` have no other callers in `src/`.
+
+**Note for the record.** This is a **guard made dead by a schema constraint** — the category the
+brief flags. Here the guard is fine and the schema is right; the fragility is that the buffer's
+own contract does not hold without the schema. If the buffers are ever reused (a script via
+`browser_run_script` gets `daemon` = the full `BrowserClient` and can reach
+`client.session().drainConsoleBuffer({ limit: 0 })` **today**), it fires.
+
+**Severity** cosmetic (latent; reachable only via `browser_run_script`).
+
+**Proposed fix.** One line in each: `if (limit <= 0) return { records: [], total, bufferOverflowed }`
+— or `matched.slice(Math.max(0, matched.length - limit))`.
+
+**Reproducible in a test?** Yes, trivially — both buffers are pure and dependency-free, and
+this is a 5-line test. Worth doing purely because it is so cheap.
+
+---
+
+## F26 — `"Provide either \`ref\` or \`selector\`."` appears three times; at most two can share a helper
+
+**Location** `src/domains/form.ts:46-48`, `src/domains/keyboard.ts:167-169`,
+`src/domains/files.ts:127-129`.
+
+*Pre-confirmed item — verified, including the caveat.* `form.ts` and `keyboard.ts` both resolve
+a ref to an **objectId** via `resolveRefToObjectId` and both fall back to
+`document.querySelector`, so those two can share. `files.ts` resolves to a **backendNodeId**
+(`resolveRefToBackendId`, for `DOM.setFileInputFiles`) and its selector path is a genuinely
+different three-stage fallback (`DOM.querySelector` → CDP set → JS `DataTransfer`). Unifying all
+three would force the file-upload shape into a mould that does not fit.
+
+Also note `src/domains/target.ts:34` carries the sibling message
+`"Provide either \`ref\` or both \`x\` and \`y\`."` for the coordinate-based tools — already
+correctly factored into one place, which is the pattern to copy.
+
+**Severity** cosmetic.
+
+**Proposed fix.** Extract the shared guard + message as a constant or a small
+`requireRefOrSelector(args)` helper used by `form.ts` and `keyboard.ts`. Leave `files.ts`
+referencing the same message constant but keeping its own control flow.
+
+**Observable behaviour change.** None.
+
+**Reproducible in a test?** Not a bug; no test needed. A test asserting all three produce the
+identical message would lock the wording.
+
+---
+
+## F27 — The boundary scanner has no rule for `: any`
+
+**Location** `scripts/check-boundaries.ts:13-19` (`RULES`).
+
+*Pre-confirmed item — verified, including the false-positive hazard.* The five rules cover
+`as any`, `@ts-ignore`/`expect-error`/`nocheck`, non-null assertions, `as {`, and
+`as unknown as`. Nothing covers a bare annotation: `const x: any = …`, `(p: any) => …`,
+`Promise<any>`.
+
+`src/` has no `: any` today, so nothing is broken — but the whole point of the scanner is that
+the property cannot silently regress, and this one can.
+
+**The false-positive hazard is real.** A naive `/:\s*any\b/` matches the long prose
+`description` strings throughout `src/domains/` — e.g. `snapshot.ts:296`
+(*"or (b) browser_snapshot and browser_execute_js cannot answer your question"* is safe, but
+`js.ts:48` contains *"is this element visible / enabled / checked / focused"* and
+`network.ts:74`'s resource-type list and several `promptGuidelines` lines contain `any` after a
+colon in ordinary English). Anchoring on a terminator is required.
+
+**Severity** cosmetic (process guard rail).
+
+**Proposed fix.** Add `{ rule: "any-annotation", re: /:\s*any\s*(?:[=;,)\]>|&]|$)/ }` — a colon,
+`any`, then a type-position terminator. Verify against the current tree: the rule must report
+zero violations on `src/` as it stands. `test/util/check-boundaries-test.ts` already exists and
+`checkSource` takes injected files, so both the positive and negative cases are directly
+testable.
+
+**Observable behaviour change.** None to runtime; `npm run boundaries` gains a rule.
+
+---
+
+## F28 — `sharp-shim.ts`'s indirection has invisible intent and invites a breaking "simplification"
+
+**Location** `src/util/sharp-shim.ts:23,34`.
+
+*Pre-confirmed item — verified. Functionally correct; a maintenance trap.*
+
+`const SHARP_SPECIFIER: string = "sharp";` followed by `await import(SHARP_SPECIFIER)` exists so
+TypeScript cannot statically resolve the specifier and therefore does not require `sharp`'s
+types — which is what removed the `@ts-ignore`. Nothing in the file says so. The explicit
+`: string` annotation (redundant on a string literal) is the entire mechanism: it widens the
+literal type so the import is dynamic to the compiler.
+
+A reader applying an "inline the constant" cleanup — plausibly Task 17, which strips comments —
+restores a statically-resolvable `await import("sharp")` and breaks the build in any workspace
+where the optional dependency is absent.
+
+**Severity** cosmetic (maintainability).
+
+**Proposed fix.** This is the one place in the file that needs a comment to survive Task 17.
+Alternatively make the intent structural and self-evident:
+`const specifier = ["sh", "arp"].join("")` is worse; a named export
+`const OPTIONAL_SHARP_SPECIFIER` plus one line of explanation is the honest answer. Flag it as
+comment-exempt for Task 17 either way.
+
+**Reproducible in a test?** Partly. A test asserting `loadSharp()` returns `{ kind: "missing" }`
+rather than throwing when `sharp` is absent guards the runtime behaviour, but nothing can
+prevent the compile-time regression except the comment.
+
+---
+
+## F29 — Ref-based fill/focus/select leak a CDP object handle on every call
+
+**Location** `src/domains/form.ts:30-45` (`runOnElement`, ref branch);
+also `src/domains/keyboard.ts:152-165` (`browser_dispatch_key`, ref branch) and
+`src/domains/snapshot.ts:263-281` (`liveValue`).
+
+*Pre-confirmed item — verified, and it is in **three** places, not one.*
+
+**What breaks.** `resolveRefToObjectId` produces a `Runtime.RemoteObject` handle via
+`DOM.resolveNode`. Chrome retains that object (it cannot be GC'd) until
+`Runtime.releaseObject` is called or the execution context is destroyed. `runOnElement` never
+releases it. `resolveAndCall` in the same file **does** (`form.ts:369-371`, in a `finally`),
+so the correct pattern already exists two hundred lines away, and `Runtime.releaseObject` is
+already in the typed command table (`commands.ts:193`).
+
+`liveValue` in `snapshot.ts` is the worst site by volume: it resolves one handle **per
+interactive form control per snapshot** and releases none.
+
+**Trigger.** Every `browser_fill`, `browser_focus`, `browser_select_option`, and
+`browser_dispatch_key` on the ref path, plus every `browser_snapshot` (once per
+`VALUE_ROLES` control).
+
+**Impact** Renderer memory growth in the user's real browser tab, proportional to how long the
+session runs. Released on navigation (context destroyed), so it is bounded per page. A
+long-running SPA session doing hundreds of snapshots and fills is the bad case.
+
+**Severity** resource leak.
+
+**Proposed fix.** Wrap all three in `try { … } finally { await session.call("Runtime.releaseObject", { objectId }) }`,
+exactly as `resolveAndCall` does. For `runOnElement` specifically, F2's suggested fix
+(route the ref branch through `resolveAndCall`) fixes the leak and the missing
+`exceptionDetails` check in one change.
+
+**Observable behaviour change.** One extra CDP round-trip per ref operation; no user-visible
+difference.
+
+**Reproducible in a test?** Yes with a fake client that records calls: assert
+`Runtime.releaseObject` appears after `Runtime.callFunctionOn`. Same missing `makeFakeClient()`
+helper as F2.
+
+---
+
+## F30 — Coverage holes where the type system cannot help
+
+*Pre-confirmed item — verified against `package.json` and the test tree.*
+
+The `npm test` glob is `test/{profile,deep-research,domains,cdp,util}/**/*-test.ts`. Confirmed
+gaps:
+
+- **`test/manual/` is excluded entirely** — 10 files, including `bridge-test.ts`,
+  `ipc-server-test.ts`, `daemon-transport-test.ts`, `socket-guard-test.ts`,
+  `window-binding-test.ts`, `forms-test.ts`. All the daemon and forms coverage that exists is
+  outside CI.
+- **`test/daemon/` does not exist.** `daemon/bridge.ts:156` (the `isCdpRawMessage` guard that
+  stops a `null` CDP frame from killing the daemon) has no test and no type protection —
+  `onChromeMessage` is a closure reachable only through a live WebSocket. Fix direction: extract
+  `onChromeMessage`'s body as a pure `handleChromeFrame(raw, deps)` and test it directly. That
+  also makes F22 and F23 testable.
+- **`domains/` has three test files** (`box`, `registry`, `target`). `form.ts` (425 lines),
+  `form-batch.ts`, `keyboard.ts`, `snapshot.ts` (477 lines), and `ref-resolve.ts` have **zero**
+  automated coverage — i.e. F2, F5, F6, F7, F8, F9, F24, F29 all live in untested files.
+  `fillFormTool` and `setCheckedTool` are newly agent-callable with no coverage at all.
+- **`test/manual/forms-test.ts` does not drive `browser_fill_form` at a `<select>`**, so it
+  would not have caught the select-by-label class of bug (`dcfe4f3`) even if it were run.
+- **The missing enabler is a `makeFakeClient()` test helper.** Eight findings above need one and
+  none can be reached without it. `test/cdp/session-call-test.ts`'s stub transport is the
+  precedent; a `BrowserClient` shim over it is the single highest-leverage piece of test
+  infrastructure this codebase is missing.
+
+**Severity** cosmetic (process), but it is the reason most findings in this report are latent
+rather than caught.
+
+---
+
+## Investigated — not bugs
+
+Recorded because the reasoning is worth as much as a finding.
+
+**F31 — `spawn.ts` resolving the daemon entry as `index.ts`.** The comment at `spawn.ts:107-112`
+says the entry is `dist/daemon/index.js` when running compiled, but the code only ever builds
+`join(moduleDir, "index.ts")`. Checked `package.json`: `files` ships `src/` (no `dist/`) and
+`pi.extensions` is `./src/index.ts`. The package runs from TypeScript source via `tsx` in every
+distribution path, so the code is right and only the comment is stale.
+
+**F32 — `picker.ts:61` mapping a selected label back to a profile by `options.indexOf`.** Looked
+for an off-by-one or a collision: `rows[i]` is built from `labels[i]` which is built from
+`profiles[i]`, and `CLEAR_SELECTION_LABEL` is appended last and handled before the `indexOf`.
+`disambiguate` guarantees distinct labels, and the pinned row's `"✓ "` prefix keeps it distinct
+from an unpinned duplicate. Correct.
+
+**F33 — `restoreState`'s fallback making the two call sites equivalent.** They *are* equivalent
+whenever a persisted entry exists. The non-equivalence is narrow and specific — see F13, which
+is the real finding this investigation produced.
+
+**F34 — `util/mutex.ts` and `util/result.ts`.** Read closely for a lost-release or a chain that
+can reject. The mutex's queue is only ever advanced by `resolve`, `registerBrowserTool` releases
+in a `finally`, and no handler holds the lock across a `return` without it. Correct.
+
+**F35 — `console-buffer.clear()` resetting `nextSeq` to 1.** This would invalidate every
+outstanding `nextCursor` (a stale cursor would suppress new records whose seq restarted below
+it). Grepped: `clear()` has **no caller** in `src/`. Dead code, not a bug. Worth deleting or
+making seq-monotonic if it is ever wired up.
+
+**F36 — `bind.ts:111` `if (seed.browserContextId) client.setProfileContextId(...)`.** If a
+sentinel page arrived without a `browserContextId`, `profileContextId()` stays undefined,
+`canReuse` in `ensureHarnessWindowUncoordinated` stays `false` forever, and **every**
+`ensureHarnessWindow` call would command-line-launch a new profile window. That would be severe.
+But `TargetInfo.browserContextId` is populated for targets in a non-default browser context —
+which a pinned non-default profile's window is by construction — and the launched window is
+always non-default relative to the connected browser's default context. Classified **not a bug,
+low residual risk**: if the pinned profile happens to *be* the browser's default context, the
+field may be omitted and the degenerate loop becomes reachable. Unverified; a one-line
+`else` returning a clear error would remove the doubt cheaply.
+
+---
+
+## Ranking: is `exceptionDetails` the most severe item?
+
+**No — I put it fourth.** F2 is real and should be fixed, but its failure is *loud*: the agent
+gets an error, the wrong error. It re-snapshots and retries. Nothing downstream is corrupted.
+
+Above it I place three findings whose failure is **silent**:
+
+1. **F1** — the harness does not work at all on Windows. Nothing else matters on that platform.
+2. **F5** — `browser_fill` on a checkbox reports success for a no-op. The agent proceeds on a
+   false belief, submits a form with the box unticked, and reports the task done. No error is
+   ever surfaced. This is the single worst *behaviour* in the codebase.
+3. **F6** — refs renumber under the agent's feet, so a mutation can hit the wrong element while
+   reporting success — on the exact workflow the system prompt recommends.
+
+F3 and F4 are close behind for the same reason (silent data loss), and they are the two easiest
+findings in the report to reproduce and fix.
+
+The ordering principle: **silence beats severity**. A wrong error message costs a retry; a false
+success costs correctness with no signal.
+
+---
+
+## Coverage and confidence
+
+**Read thoroughly, line by line** — `cdp/` (all 17), `daemon/` (all 5), `util/` (all 10),
+`schemas/` (both), the 7 root files, and in `domains/`: `form.ts`, `form-batch.ts`,
+`keyboard.ts`, `ref-resolve.ts`, `snapshot.ts`, `cdp-call.ts`, `click.ts`, `navigate.ts`,
+`tabs.ts`, `page.ts`, `files.ts`, `js.ts`, `network.ts`, `console.ts`, `drag.ts`,
+`isolated-tab.ts`, `screenshot.ts`, `target.ts`, `box.ts`.
+
+**Read, lower scrutiny** — `profile/` (7 files: read in full, but the platform-specific process
+detection in `browser-process.ts` (437 lines of `ps`/PowerShell/registry parsing) was assessed
+for shape rather than probed per-platform; it also has the best test coverage in the repo,
+`test/profile/browser-process-test.ts`). Same for `domains/search/` and `domains/readpage/` —
+read in full, and their extraction/scoring logic is already pure and fixture-tested, so I looked
+for integration bugs (F21) rather than re-auditing the parsers.
+
+**Skimmed** — the long `description` / `promptGuidelines` string blocks in every tool
+definition, except where a claim contradicted the code (F5, F6, F12, F21 all turn on such a
+contradiction, so the ones that mattered were read).
+
+**Not covered** — `test/` beyond its layout and `session-call-test.ts`'s stub; `skills/`;
+`docs/`. All out of scope for a `src/` sweep.
+
+**Unverified, called out in place** — F1 (needs Windows), F12 (needs live Chrome), F13 (needs
+the host's event ordering), F21 (traced, not observed), F36 (needs a live pinned profile in the
+default browser context). F3's premise (per-session domain enablement) is standard CDP behaviour
+asserted by the code's own comment but not confirmed against a browser here.
+
+---
+
+## Summary table
+
+| # | Finding | Location | Severity | Pre-confirmed? | Reproducible in a test? | Fix changes behaviour? |
+|---|---|---|---|---|---|---|
+| **F1 ✔ fixed** | `access()` on a Windows named pipe fails ⇒ **every** browser tool returns "not initialized" | `util/tool.ts:145` | crash (platform-wide) | no | after extracting the check; real Windows behaviour needs Windows | yes (Windows only) |
+| **F5 ✔ fixed** | `browser_fill` on a checkbox sets `value` not `checked`, returns `ok` | `domains/form.ts:90-105` | wrong result (silent no-op) | yes — agree, worst behaviour | needs a pure branch-decision seam, or live Chrome | yes |
+| **F6 ✔ fixed** | Every mutation renumbers the ref map ⇒ held refs can target the wrong element | `domains/ref-resolve.ts:85-94` | wrong result (silent) | no | yes, pure, from AX fixtures | yes |
+| **F2 ✔ fixed** | `runOnElement` ignores `exceptionDetails` ⇒ a page throw reports "No element matched" | `domains/form.ts:37-44` (3 sites) | wrong result (misdiagnosis) | yes — agree; I rank it 4th, not 1st | yes, needs a `makeFakeClient()` helper | yes |
+| **F3 ✔ fixed** | `switchTo` back to a visited tab skips `enableDomains` ⇒ console/network/page events stop | `cdp/session.ts:368-377` | wrong result (silent data loss) | no | **yes — easiest in the report**, existing stub transport | yes |
+| **F4 ✔ fixed** | Event loop drops all non-active-tab events, defeating the per-tab buffers | `cdp/session.ts:93` | wrong result (silent data loss) | no | yes, stub transport + pushable event queue | yes |
+| F7 | Fill/select write through a **disabled** control and report `ok` | `domains/form.ts:90,254` | wrong result (silent) | yes — agree | same seam as F5 | yes |
+| F9 | `browser_type` focus guard is **fail-open** when the probe itself fails | `domains/keyboard.ts:40` | wrong result (silent) | yes (as a coverage hole) — I also call it a live fail-open | yes, needs the fake client | yes |
+| **F10 ✔ fixed** | Two fill engines diverge on disabled / select / **checkbox** / contenteditable / diff | `form.ts` vs `form-batch.ts` | wrong result (umbrella) | yes — table verified exactly | per-row, once the decision logic is pure | yes, substantially — needs design sign-off |
+| F8 | `verified` is reported but never compared to the requested value | `domains/form.ts:161-164` | wrong result (mild — data present, unjudged) | yes — agree it's real, rank it lower | yes, the comparison is pure | yes |
+| F11 | Concurrent liveness probes ⇒ second daemon unlinks the live socket, two Chrome prompts | `daemon/spawn.ts:70` + `server.ts:83` | resource leak (user-visible) | no | yes, but lands in excluded `test/manual/` | no (normal path) |
+| **F12 ⚠ needs live Chrome** | `openerId` is likely not a `createTarget` param ⇒ unpinned tabs placed by focus | `cdp/commands.ts:96`, `target-factory.ts:210` | wrong result — **unverified** | yes (flagged to verify) — looks wrong; could not confirm | no; needs one live-Chrome experiment | yes if changed |
+| F13 | `session_tree` before client creation resets namespace to `"default"` | `index.ts:196` | wrong result — **unverified ordering** | yes — not equivalent, in one narrow case | `restoreState` yes; the event ordering no | no (normal ordering) |
+| F19 | Bad `outputPath` throws out of the handler instead of `io_error` | `domains/files.ts:219` (+2) | crash (tool call) | no | yes, via a handler-level try/catch test | yes |
+| F14 | Both abort budgets are dead — `.map` starts every task before the timer can fire | `snapshot.ts:351`, `network.ts:253` | wrong result (latency) | yes (as the abort-path item) | yes for `network.ts`, needs the fake client | yes |
+| F29 | Ref fill/focus/select/dispatch_key + `liveValue` leak CDP object handles (3 sites) | `form.ts:37`, `keyboard.ts:155`, `snapshot.ts:273` | resource leak | yes — agree; it is 3 sites, not 1 | yes, assert `releaseObject` is called | no |
+| F21 | `openIsolatedTab` can adopt then close the harness window's only tab | `domains/isolated-tab.ts:41-47` | resource leak / churn | no | partly — race variant not deterministic | yes (one extra tab open/close) |
+| F22 | Daemon misses `detachFromTarget` session release (id is in `params`, not the envelope) | `daemon/bridge.ts:389` | resource leak (small) | no | yes, but in excluded `test/manual/` | no |
+| F15 | `asArrayOf` all-or-nothing blanks a whole TUI render | `util/guards.ts:13` | cosmetic (render only) | yes — agree it's real; **render-only, not wrong result** | yes, pure, test file exists | only in the failure case |
+| F18 | `screenshotPath` silently omitted when the screenshot fails | `domains/snapshot.ts:384-390` | cosmetic | yes — agree | yes, needs the fake client | yes (new `details` field) |
+| F16 | Strict `AxNode` array ⇒ one bad node fails all of `browser_snapshot` | `cdp/commands.ts:241` | cosmetic today; crash-class if it fires | yes — no realistic trigger found; **mitigate, don't revert** | yes, `decodeResult` is pure | only in the failure case |
+| F17 | All `Input.*` params unvalidated; `drag.ts` `InputCall` untyped + declared in a handler | `cdp/commands.ts:228-231` | cosmetic (latent) | yes — verified; **no live bug**, every call site is correct | compile-time only | no |
+| F23 | `handleRequest` promise dropped with no `.catch` in a socket handler | `daemon/index.ts:31` | cosmetic today; crash-class if a throw appears | no | yes, with daemon test infra | no |
+| F24 | Diff keyed on `role\|name` collapses duplicate-named elements | `domains/ref-resolve.ts:98-105` | cosmetic (misleading advisory) | no | yes, pure — but fixed for free by F6 | no (subsumed) |
+| F20 | Aborting `browser_wait_for` throws instead of returning a typed error | `domains/page.ts:132` | cosmetic | no | yes (`sleep` half is testable today) | yes |
+| F25 | `limit: 0` ⇒ `slice(-0)` returns everything | `console-buffer.ts:228`, `network-buffer.ts:175` | cosmetic (latent) | no | **yes, 5-line pure test** — reachable via `browser_run_script` | no |
+| F26 | `"Provide either ref or selector"` ×3; only 2 can share a helper | `form.ts:47`, `keyboard.ts:168`, `files.ts:128` | cosmetic | yes — agree, including the caveat | n/a | no |
+| F27 | Boundary scanner has no `: any` rule; a naive regex hits prose `description`s | `scripts/check-boundaries.ts:13-19` | cosmetic (guard rail) | yes — agree, anchoring needed | yes, `checkSource` takes injected files | no |
+| F28 | `sharp-shim.ts` indirection's intent is invisible; a "simplification" breaks the build | `util/sharp-shim.ts:23,34` | cosmetic (maintainability) | yes — agree; **flag comment-exempt for Task 17** | runtime half only | no |
+| F30 | Coverage holes: `test/manual/` excluded, no `test/daemon/`, `form`/`snapshot`/`keyboard` untested, no `makeFakeClient()` | `package.json` test glob | cosmetic (process) | yes — verified and expanded | n/a | no |
+| F31–F36 | **Investigated, not bugs**: stale `dist/` comment in `spawn.ts`; `picker.ts` index mapping; `restoreState` equivalence (⇒ F13); `mutex`/`result`; `console-buffer.clear()` is uncalled dead code; `bind.ts` missing-`browserContextId` degenerate loop (low residual risk, unverified) | various | — | — | — | — |
+
+**Fixed in this pass (2026-07-28 ruling):** F1, F2, F3, F4, F5+F10, F6 — six findings in
+six commits, each preceded by a test that failed for the reason recorded above. Test suite
+206 → 251 passing, 8 skipped, 0 failing; `npm run check` green. See *Resolution status* at
+the top for commits and the three shipped behaviour changes. **F12 remains open and needs
+one live-Chrome experiment.** Everything else is documented only, by ruling.
+
+**Counts.** 30 findings + 6 investigated-not-bugs.
+By severity: **crash 2** (F1, F19) plus 2 crash-class-if-triggered (F16, F23) ·
+**wrong result 11** (F2, F3, F4, F5, F6, F7, F8, F9, F10, F12, F13, F14 — 12 including F14's
+latency class) · **resource leak 4** (F11, F21, F22, F29) · **cosmetic 11**.
+By verification: 25 verified by reading · **5 unverified** (F1 needs Windows, F12 needs live
+Chrome, F13 needs the host's event ordering, F21 traced-not-observed, F36 needs a live pinned
+default-context profile).
+Pre-confirmed items: 12 reviewed, **12 agreed real**, 0 rejected — but three re-rated
+(F8 down, F15 down to cosmetic/render-only, F17 confirmed as latent-only with no live bug),
+one broadened (F29 is 3 sites, not 1), and one, F12, remains **unverified and probably wrong
+as written**.