aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoramankumarsingh77 <amankumarsingh7702@gmail.com>2026-07-28 20:13:30 +0530
committerAman Kumar <145339995+amankumarsingh77@users.noreply.github.com>2026-08-02 19:12:46 +0530
commit2b3eb8ad79d2fed770cf665cff1cc746951a95d3 (patch)
tree4e51409f9d4ef5a453200a751417cbc9eed14094
parent758be304747974f84418f66efa4353362ac1edcf (diff)
docs: document the layer rules and tool authoring contract
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
-rw-r--r--.gitignore3
-rw-r--r--CONTRIBUTING.md61
-rw-r--r--docs/ARCHITECTURE.md400
3 files changed, 426 insertions, 38 deletions
diff --git a/.gitignore b/.gitignore
index fcb90d8..a36ffce 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,7 +4,8 @@ benchmark/
*.tsbuildinfo
.DS_Store
*.log
-docs/
+docs/*
+!docs/ARCHITECTURE.md
scripts/*
!scripts/check-boundaries.ts
!scripts/boundary-allowlist.ts
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 2c9d7a4..93ae71e 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -29,34 +29,25 @@ npm install
### 3. Run the local checks
```bash
-npm run typecheck
+npm run check
npm pack --dry-run
```
-`npm run typecheck` validates the TypeScript source. `npm pack --dry-run` verifies that the package can be packed for publishing and shows which files will be included.
+`npm run check` runs the typecheck, the type-safety boundary scan, and the test suite. All three must pass before a PR. `npm pack --dry-run` verifies that the package can be packed for publishing and shows which files will be included.
---
-## Project structure
+## Architecture
-```text
-.
-├── src/
-│ ├── index.ts # Extension entry point and command registration
-│ ├── tools.ts # Browser tool schemas, handlers, and result rendering hooks
-│ ├── daemon.ts # Connection lifecycle for the browser-harness daemon
-│ ├── protocol.ts # Types for browser/daemon protocol messages
-│ ├── setup.ts # /browser-setup implementation and Chrome detection
-│ ├── state.ts # Session state persistence
-│ ├── prompt.ts # Browser usage guidance injected into the agent prompt
-│ └── renderers.ts # TUI renderers for screenshots and tab listings
-├── skills/
-│ └── pi-browser-harness/
-│ └── SKILL.md # Agent skill documentation
-├── README.md # User-facing documentation
-├── CHANGELOG.md # Release notes
-└── package.json # npm package metadata and pi manifest
-```
+Read [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) before making a non-trivial change. It covers:
+
+- the layer diagram and which directories a `domains/*` file may and may not import from,
+- the typed and runtime-validated CDP boundary in `src/cdp/commands.ts` and `src/cdp/events.ts`, and how to add a command,
+- the nine-rule tool authoring contract,
+- an end-to-end worked example of adding a tool and testing it against the fake client,
+- the conventions in this codebase that look like mistakes but are not.
+
+The source carries almost no comments by policy, so that document is where the non-obvious conventions live.
---
@@ -93,8 +84,10 @@ Use the normal browser workflow when testing: screenshot → act → screenshot
## Code conventions
-- Write TypeScript and keep the project passing `npm run typecheck`.
+- Write TypeScript and keep the project passing `npm run check`.
- Keep `strict` TypeScript mode clean. Do not introduce implicit `any` values.
+- No unchecked casts. `npm run boundaries` fails the build on `as any`, `as {`, `as unknown as`, `@ts-ignore`, and non-null assertions in `src/`. A green `tsc` is not evidence of type safety — see `docs/ARCHITECTURE.md` section 3.
+- Prefer no comments. Explain a non-obvious convention in `docs/ARCHITECTURE.md`, not inline.
- Prefer small, focused changes over broad rewrites.
- Keep tool descriptions, parameter descriptions, and prompt guidelines clear and user-facing.
- Preserve the existing error-handling style: return useful, actionable messages to the agent instead of leaking low-level details when possible.
@@ -105,24 +98,16 @@ Use the normal browser workflow when testing: screenshot → act → screenshot
## Adding a new browser tool
-Most browser tools are registered in `src/tools.ts` through `pi.registerTool`.
+The full worked example is in [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) — section 4 (the contract) and section 5 (the steps). In short: one file under `src/domains/`, a typebox args schema at the top, a `defineBrowserTool` call carrying its own `concurrency`, an entry in `ALL_TOOLS` in `src/registry.ts`, and a test under `test/domains/` written against `createFakeClient`.
-When adding a tool:
+Beyond that:
-1. Add or reuse the underlying daemon method in `src/daemon.ts` and protocol types in `src/protocol.ts` when needed.
-2. Register the pi tool in `src/tools.ts` with:
- - a stable `browser_*` name,
- - a concise label and description,
- - a TypeBox parameter schema,
- - prompt snippets and guidelines that explain when to use it,
- - clear result text and structured details where useful.
-3. Update prompt guidance in `src/prompt.ts` if the tool changes recommended workflows.
-4. Add or update a renderer in `src/renderers.ts` if the tool returns visual or rich output.
-5. Document the tool in `README.md` and note the change in `CHANGELOG.md`.
-6. Run:
+1. Update prompt guidance in `src/prompt.ts` if the tool changes recommended workflows.
+2. Document the tool in `README.md` and note the change in `CHANGELOG.md`.
+3. Run:
```bash
- npm run typecheck
+ npm run check
npm pack --dry-run
```
@@ -143,10 +128,12 @@ Keep tool behavior predictable. The agent should be able to verify actions with
4. Run local checks:
```bash
- npm run typecheck
+ npm run check
npm pack --dry-run
```
+ `npm run check` must be green before the PR is opened.
+
5. Test in pi with a local path install when the change affects runtime behavior.
6. Update documentation and CHANGELOG entries when user-visible behavior changes.
7. Open a pull request against `main` and describe what changed, how it was tested, and any follow-up work.
diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md
new file mode 100644
index 0000000..c1efae2
--- /dev/null
+++ b/docs/ARCHITECTURE.md
@@ -0,0 +1,400 @@
+# Architecture
+
+How `pi-browser-harness` is put together, what each layer may depend on, and the rules a new
+tool has to follow. Read the "Conventions that look like mistakes" section before you clean
+anything up — several deliberate choices in this codebase look like typos.
+
+The codebase carries almost no comments by policy. That makes this file the only place the
+non-obvious conventions live.
+
+---
+
+## 1. Layers
+
+Dependencies point downward only.
+
+```
+index.ts extension entry: flags, slash commands, session lifecycle hooks
+ |
+registry.ts flat tool catalog (ALL_TOOLS, 40 tools): imports only, zero per-tool config
+ |
+util/tool.ts tool runtime: liveness guard -> concurrency lock -> handler
+ -> Result-to-AgentToolResult conversion
+ |
+domains/* one file per capability. Pure tool definitions.
+ The only layer expected to grow.
+ |
+client.ts BrowserClient facade: session, ownership, mutation mutex,
+ profile pin
+ |
+cdp/session.ts typed call/callOnTarget/callBrowser, event routing,
+ console + network buffers
+ |
+cdp/daemon-transport.ts -> daemon/* out-of-process daemon owning the Chrome socket
+ |
+Chrome (CDP over WebSocket)
+```
+
+Side stacks:
+
+| Directory | Role | Who may import it |
+|---|---|---|
+| `profile/*` | decides *which* Chrome and which profile to drive | `index.ts`, `setup.ts`, `client.ts` — never a domain |
+| `schemas/*` | shared typebox arg fragments (`Coords`, `MouseButton`) and `parseJson` | anything |
+| `util/*` | leaves — they import nothing from this repo except each other (`result`, `guards`, `mutex`, `time`, `keycodes`, `truncate`, `paths`, `js-template`, `sharp-shim`) | anything |
+
+`src/util/tool.ts` sits in `util/` for locality but is not a leaf — it is the tool runtime layer
+in the diagram above and imports `client` and `daemon/spawn`.
+
+### The import rule for domains
+
+A `domains/*` file may import from `schemas/`, `util/`, other `domains/`, the `BrowserClient`
+type, and the *typed, session-level* parts of `cdp/` (`cdp/commands` types, `cdp/attach`,
+`cdp/window`, `cdp/evaluate`, `cdp/target-factory`, the buffers, the `CdpSession` type).
+
+**It must never import from `daemon/`, `cdp/transport`, or `profile/`.** A domain that reaches
+the raw transport has escaped the validated boundary in section 2; a domain that reaches
+`profile/` has coupled a tool to Chrome-launch policy.
+
+This currently holds across all of `src/domains/`. It is **not** machine-enforced —
+`scripts/check-boundaries.ts` checks casts, not imports. Verify it by eye in review.
+
+---
+
+## 2. The CDP boundary
+
+`src/cdp/commands.ts` (34 commands) and `src/cdp/events.ts` (12 events) are the single source
+of truth for **both** the TypeScript types and the runtime validation. One typebox schema per
+entry generates both, so they cannot drift.
+
+```ts
+const cmd = <P extends TSchema, R extends TSchema>(params: P, result: R) => ({
+ params,
+ result,
+ validate: Compile(result),
+});
+
+export const COMMANDS = {
+ "Target.attachToTarget": cmd(
+ Type.Object({ targetId: Type.String(), flatten: Type.Optional(Type.Boolean()) }),
+ Type.Object({ sessionId: Type.String() }),
+ ),
+ // ...
+};
+
+export type CdpMethod = keyof typeof COMMANDS;
+export type ParamsOf<M extends CdpMethod> = Static<(typeof COMMANDS)[M]["params"]>;
+export type ResultOf<M extends CdpMethod> = Static<(typeof COMMANDS)[M]["result"]>;
+```
+
+`decodeResult` validates every response before resolving; `decodeEvent` does the same for
+subscribed events. A malformed payload becomes `err({ kind: "invalid_response", ... })` instead
+of a `TypeError` three frames later.
+
+### Adding a command
+
+1. Add one `cmd(params, result)` entry to `COMMANDS`, keyed by the CDP method name.
+2. Call it. `session.call(method, params)` and `cdpCall(client, method, params)` are already
+ typed against the table. **No cast anywhere.**
+
+### `additionalProperties: true` — the part that bites
+
+`additionalProperties: true` makes the **runtime validator** permissive. It does **not** widen
+the derived `Static<>` **type**. The consequence differs by direction, and getting it backwards
+produces two different bugs:
+
+| Schema | Under-specify | Over-specify |
+|---|---|---|
+| `params` | **compile error at the call site** — the field is not in `ParamsOf<M>`, so you cannot pass it | harmless |
+| `result` / event params | field is simply unavailable to read | **silently dropped event or failed decode** when a future Chrome changes that field |
+
+So:
+
+- **Enumerate every field a call site passes.** Four of the six compile errors this refactor
+ had to fix were exactly this: `Runtime.callFunctionOn`'s params schema omitted `arguments`,
+ `returnByValue`, and `awaitPromise`, which the code genuinely sends.
+- **On results and events, enumerate only what a consumer actually reads.** `Page.frameNavigated`
+ and `Page.loadEventFired` are declared as fully-open objects with no enumerated fields
+ (`ev(Anything)`) and never call `decodeEvent` at all — the session only needs to know they
+ fired.
+
+### `cdpCall` vs `session.call`
+
+`cdpCall(client, method, params)` (`src/domains/cdp-call.ts`) is `session().call` plus the
+`CdpError -> ToolErr` mapping, retiring the `if (!r.success) return err({ kind: "cdp_error" })`
+couplet after every call. Domains use it by default.
+
+`params` is **required**, unlike `session.call`. A no-argument CDP method needs an explicit `{}`:
+
+```ts
+const r = await cdpCall(client, "Page.reload", {});
+```
+
+`src/domains/snapshot.ts` **deliberately mixes both styles.** Its three `session.call` sites —
+two in `liveValue`, one in the optional-screenshot block — swallow the failure on purpose:
+`liveValue` returns `undefined` so the caller falls back to the accessibility-tree value, and a
+failed screenshot just omits the path. Neither should surface as a tool error. This is
+intentional, not drift — do not convert them to `cdpCall`.
+
+---
+
+## 3. Keeping the type safety
+
+### A green `tsc` proves nothing about unchecked casts
+
+`as { … }` between structurally compatible object types compiles **silently**. At one point this
+codebase held 33 such casts and produced only 6 compile errors when they were removed. Strict
+flags do not see them.
+
+Likewise, **`JSON.parse` returns `any`, so an annotated assignment is an invisible cast.**
+`let msg: CdpRawMessage = JSON.parse(raw)` typechecked cleanly in `src/daemon/bridge.ts` while
+being completely unguarded — a `null` frame from Chrome would have thrown inside a `ws` handler
+with no `try`/`catch` above it and killed the daemon. That is why `src/schemas/parse.ts` exists:
+`parseJson(raw, validator)` parses to `unknown`, validates, and returns a `Result`. Every parse
+boundary goes through it or an equivalent guard.
+
+### The boundary scanner
+
+`npm run boundaries` runs `scripts/check-boundaries.ts` over every `.ts` file under `src/` and
+fails the build on five line-level patterns:
+
+| Rule | Matches |
+|---|---|
+| `as-any` | `as any` |
+| `ts-ignore` | `@ts-ignore`, `@ts-expect-error`, `@ts-nocheck` |
+| `non-null-assertion` | `x!.`, `x!(`, `x!;` … |
+| `inline-object-cast` | `as {` |
+| `double-cast` | `as unknown as` |
+
+Exemptions live in an **external allowlist**, `scripts/boundary-allowlist.ts`, as
+`{ path, rule, count, reason }` entries. The `count` is a budget, not a flag: a *second*
+violation of the same rule in an already-exempt file still fails the build.
+
+The allowlist is deliberately external rather than a comment marker in the source, because the
+zero-comments policy would eventually delete an in-source marker and turn CI red for an
+unrecognisable reason. `unusedExemptions` also reports an entry whose code is gone, so the list
+cannot rot.
+
+**Limitations, stated rather than implied:** the scan is **line-based**, so a cast split across
+two lines evades every rule. And it only walks `src/` — `test/` and `scripts/` are unscanned.
+
+### The five surviving assertions in `src/`
+
+All five are boundary assertions where a runtime check has already established the shape and
+TypeScript cannot express the correlation.
+
+| Location | Why |
+|---|---|
+| `decodeResult` in `src/cdp/commands.ts` | `validate.Check(raw)` proves `raw` matches `COMMANDS[method].result`; TS cannot correlate a generic key to its mapped value |
+| `decodeEvent` in `src/cdp/events.ts` | same, for `EVENTS[name].params` |
+| `loadSharp` in `src/util/sharp-shim.ts` (probe for `.default`) | the module comes from a dynamic import of an optional dependency, so its shape is unknown at compile time; the cast only claims an optional `.default`, and every value read off it is re-checked with `typeof` |
+| `loadSharp` in `src/util/sharp-shim.ts` (`candidate` to `SharpFactory`) | `typeof candidate === "function"` was checked immediately above |
+| `registerBrowserTool` in `src/util/tool.ts` (`renderCall` re-widening) | `AnyBrowserToolDefinition` is type-erased so the registry can hold a heterogeneous array; `defineBrowserTool` guarantees handler, parameters, and `renderCall` were built from the same `S` |
+
+Only the `sharp-shim.ts` `.default` probe needs an allowlist entry, because it is the one written
+as `as { … }`. The other four are `as SomeNamedType`, which no scanner rule matches. **Do not add
+allowlist entries for them** — they are not required, and `unusedExemptions` would flag them.
+
+---
+
+## 4. The tool authoring contract
+
+Nine rules. Every `domains/*` file follows them.
+
+| # | Rule | Exemplar |
+|---|---|---|
+| 1 | **One domain per file**, exporting only `<name>Tool` consts plus helpers it owns. Past roughly 250 lines, split by capability. | `src/domains/history.ts` |
+| 2 | **Args schema at the top of the file**, named `<Name>Args`, built with typebox, with a `description` on every field. Those strings are the agent's only documentation of the tool. | `src/domains/keyboard.ts` (`TypeArgs`, `PressKeyArgs`) |
+| 3 | **`defineBrowserTool({ … })` holds all metadata, including `concurrency`.** A tool's concurrency class must not live in a different file from the tool. | every domain file |
+| 4 | **Fixed handler signature** `(args, ctx) => Promise<Result<ToolOk, ToolErr>>`. Never throw, never `console.log`, never touch `process`. Errors are values. | `src/util/tool.ts` (`ToolHandler`) |
+| 5 | **Reach Chrome only via `ctx.client`.** No domain imports the transport. | see section 1 |
+| 6 | **Element targeting goes through the shared `resolveTarget(client, args)`** in `src/domains/target.ts` — never a hand-rolled ref-vs-x/y branch. | `src/domains/click.ts` |
+| 7 | **CDP calls go through `cdpCall(client, method, params)`.** | `src/domains/history.ts` |
+| 8 | **Naming:** tool name `browser_<verb>`, `label` title-case, `promptSnippet` one line, `promptGuidelines` at most six agent-facing bullets. | pinned by `test/domains/registry-test.ts` |
+| 9 | **Every tool has a test** covering its schema and its error paths, written against a fake client so no browser is needed. | `test/domains/form-errors-test.ts` |
+
+Standing exceptions to rule 1: `src/domains/snapshot.ts` (477) and `src/domains/form.ts` (409)
+are over the 250-line guidance. Both are single coherent capabilities; splitting them is
+tracked work, not licence to add a third.
+
+Rule 6 currently binds only `src/domains/click.ts`, because it is the sole tool taking
+*either* a ref *or* coordinates. `drag.ts` takes explicit coordinates only and `scroll.ts`
+defaults to the viewport centre; neither resolves refs. Any *new* ref-or-coords tool must use
+`resolveTarget`.
+
+Rules 8 and 9 are partly machine-checked. `test/domains/registry-test.ts` asserts every tool
+declares a concurrency class, that names are unique, that every name is `browser_`-prefixed, and
+that the set of `serialized` tools is **exactly** a pinned list — so adding a serialized tool
+means adding its name to `SERIALIZED` in that file.
+
+---
+
+## 5. How to add a tool
+
+`src/domains/history.ts` (80 lines, three tools) is the whole pattern in one file. A minimal
+tool, abridged from `reloadTool` in that file — the real one also branches on an open JS dialog:
+
+```ts
+import { Type } from "typebox";
+import { type Result, err, ok } from "../util/result";
+import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool";
+import { cdpCall } from "./cdp-call";
+
+export const reloadTool = defineBrowserTool({
+ name: "browser_reload",
+ label: "Browser Reload",
+ description: "Reload the current page.",
+ promptSnippet: "Reload the page",
+ promptGuidelines: ["Use to refresh the page, e.g. after server-side changes."],
+ parameters: Type.Object({}),
+ concurrency: "serialized",
+ async handler(_a, { client }): Promise<Result<ToolOk, ToolErr>> {
+ const r = await cdpCall(client, "Page.reload", {});
+ if (!r.success) return r;
+ const info = await client.pageInfo();
+ if (!info.success) return err({ kind: "cdp_error", message: info.error.message });
+ return ok({ text: `Reloaded: ${info.data.title}`, details: { page: info.data } });
+ },
+});
+```
+
+Note `if (!r.success) return r;` — a failed `cdpCall` is already a `Result<_, ToolErr>`, so it
+returns unchanged. No re-wrapping.
+
+Steps:
+
+1. Create `src/domains/foo.ts`. Declare `FooArgs` at the top with a `description` on every field.
+2. If the CDP method you need is not in `COMMANDS`, add a table entry first (section 2).
+3. Call `defineBrowserTool` with an explicit `concurrency`. Pick `serialized` if the tool mutates
+ page or session state; `parallel` only for pure reads.
+4. Add the export to `ALL_TOOLS` in `src/registry.ts`.
+5. If it is `serialized`, add its name to `SERIALIZED` in `test/domains/registry-test.ts`.
+6. Write `test/domains/foo-test.ts`.
+
+### Testing a handler
+
+Use **`createFakeClient()`** from `test/domains/fake-client.ts`. It builds a real `CdpSession`
+over a **stub transport** — canned per-method results, recorded calls, and an injectable event
+stream:
+
+```ts
+const ctxFor = (fake: FakeClient): HandlerContext => ({
+ client: fake.client,
+ signal: undefined,
+ onUpdate: () => {},
+ extensionCtx: undefined as never,
+});
+
+const fake = await createFakeClient({
+ refs: [{ ref: "e1", backendId: 101 }],
+ canned: {
+ "DOM.resolveNode": ok({ object: { type: "object", objectId: "obj-1" } }),
+ "Runtime.callFunctionOn": ok({
+ result: { type: "undefined" },
+ exceptionDetails: { text: "Uncaught TypeError: el.dispatchEvent is not a function" },
+ }),
+ },
+});
+const r = await fillTool.handler({ ref: "e1", value: "x" }, ctxFor(fake));
+assert.equal(r.success, false);
+if (r.success) return;
+assert.equal(r.error.kind, "cdp_error");
+assert.match(r.error.message, /dispatchEvent is not a function/);
+```
+
+`canned` maps a CDP method to a `Result` (or a queue of them, consumed in order); `refs` seeds the
+session's ref map so a `ref`-taking tool resolves without a snapshot; `evaluate` stubs
+`client.evaluateJs`. `fake.calls` / `fake.callsTo(method)` are the recorded requests, and
+`fake.emit(event)` injects a CDP event. `test/domains/form-errors-test.ts` is the reference usage.
+
+**Do not mock the `BrowserClient` instead.** A mocked client bypasses the real session, the real
+decoders, and the real error mapping, which produces a test that cannot fail. Two tasks on this
+refactor shipped exactly that before `createFakeClient` existed.
+
+It also exports `createStubTransport` (for session-level tests) and `axNode` (for
+accessibility-tree fixtures).
+
+---
+
+## 6. Conventions that look like mistakes
+
+Each of these has been, or would be, "fixed" by someone who did not know why it was written
+that way.
+
+**`src/util/tool.ts` gates the lock on `if (def.concurrency !== "parallel")`, not
+`=== "serialized"`.** Deliberate fail-closed default: an unrecognised or missing concurrency
+value **over-serialises** rather than racing on shared CDP session and page state. A test in
+`test/domains/registry-test.ts` ("a definition with no concurrency class takes the mutation
+mutex") pins it.
+
+**`src/util/sharp-shim.ts` routes its dynamic import through
+`const SHARP_SPECIFIER: string = "sharp"`** so TypeScript cannot resolve the specifier
+statically. `sharp` is an **optional** dependency. **Anyone who "simplifies" this back to a
+static `import("sharp")` breaks the build wherever sharp is absent.** This is the single most
+likely thing in the codebase to be broken by a well-meaning cleanup.
+
+**`npm test` passes `--conditions=import`.** A test can import a tool definition *only* because
+of it: `@mariozechner/pi-coding-agent` publishes no `"require"` condition, and this repo is not
+`"type": "module"`, so tsx would otherwise compile `src/` to CJS and the specifier would be
+unresolvable. It also *matches* production — pi loads the extension with jiti and aliases that
+package to `dist/index.js` by absolute path, which is what the `import` condition selects.
+
+**The test glob is `test/{profile,deep-research,domains,cdp,util}/**/*-test.ts`.** A test placed
+in any other directory — `test/schemas/`, `test/daemon/` — **silently never runs.** `test/manual/`
+is excluded on purpose: those need live Chrome.
+
+**`Box` (`src/domains/box.ts`) has six fields — `x, y, width, height, cx, cy` — and all six are
+consumed.** `cx`/`cy` are the click point. `width`/`height` reach the model through
+`browser_snapshot` with `format: "json"`, which serialises each node's `box` wholesale.
+Narrowing the type would silently change tool output.
+
+---
+
+## 7. The failure mode that cost the most rework
+
+**Adopting a type guard where a loose truthiness check lived can invert a safety default.**
+
+`if (guard(x)) { if (!x.ok) fail }` is **not** equivalent to `if (!x.ok) fail`. With an unchecked
+cast, a malformed payload left the field `undefined`, so `!field` was **true** and the failure
+path fired. Fold the guard into the boolean gate and a rejected shape now **skips the check
+entirely** — the code proceeds in exactly the case where it knows least.
+
+The structural tell, verified across six sites:
+
+- **SAFE** — the guard produces a **value** that inherits a pre-existing failure path:
+ ```ts
+ const v = guard(x) ? x : undefined;
+ if (v === undefined || !v.ok) return err(…);
+ ```
+- **UNSAFE** — the guard is **ANDed into a boolean gate**:
+ ```ts
+ if (ok && guard(x)) { … }
+ ```
+
+`typeTool` in `src/domains/keyboard.ts` has both halves within six lines. The inner line is the
+safe form — `isFocusProbe(focused.data) ? focused.data : { ok: false, tag: null }` turns a bad
+shape into a **value** meaning "not focused", which the existing `if (!f.ok)` then rejects. The
+outer `if (focused.success)` is the same mistake in its other guise: a gate, so when the focus
+probe itself fails the entire check is skipped and the tool types anyway. That is a known open bug
+(F9 in the bug audit), left in place here as the clearest illustration in the tree.
+
+Rule of thumb: **a check must default to the failure path when its input cannot be understood.**
+If adding a guard moved a case from "fails loudly" to "not checked", the guard is in the wrong
+place.
+
+---
+
+## 8. Running the checks
+
+```bash
+npm run check # typecheck + boundaries + test
+```
+
+- `npm run typecheck` — `tsc --noEmit` over `src/`, then `tsc -p tsconfig.test.json` over
+ `src/`, `test/`, and `scripts/`. Flags live in `tsconfig.base.json`; the notable ones beyond
+ `strict` are `noUncheckedIndexedAccess`, `exactOptionalPropertyTypes`,
+ `noPropertyAccessFromIndexSignature`, and `verbatimModuleSyntax`.
+- `npm run boundaries` — the cast scanner (section 3).
+- `npm test` — `node:test` over the glob above.
+
+`npm run check` must be green before a PR.