From da5d31af999b728c31ab0169373531cab4543f0c Mon Sep 17 00:00:00 2001 From: Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> Date: Sun, 21 Jun 2026 00:46:44 +0530 Subject: release: v0.6.0 — per-tab isolation, Unix socket daemon, on-demand init (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Unix socket daemon with auto-reconnect for persistent Chrome CDP Eliminates the 'Allow Remote Debugging' Chrome prompt that appeared on every pi session by multiplexing pi instances through a single persistent daemon process. Key additions: - Unix socket IPC (net.createServer + readline JSON framing) - CdpBridge with ID multiplexer + event router - DaemonTransport implementing CdpTransport interface - Auto-spawn with ensureDaemon() (lazy, checks socket) Fixes since initial implementation: - Bridge auto-reconnects to Chrome with exponential backoff (1s-60s). Previously the bridge tried once at startup and permanently failed if Chrome wasn't ready — causing 'Chrome not connected' on every browser tool call. - Daemon spawn uses project-local tsx binary instead of global --import flag, fixing silent spawn failure (ERR_MODULE_NOT_FOUND). - Stale ctx crash: persistState in onOwnershipChange wrapped in try/catch to prevent CDP event consumer crashes. 49 test assertions across 4 test files. Zero TypeScript errors. * feat: on-demand browser initialization Browser daemon no longer auto-starts on session_start. Instead: - session_start creates the client lazily but does NOT connect or spawn the daemon. Zero Chrome prompts for non-browser pi sessions. - Browser tool wrapper checks for daemon socket before each call. Socket missing → clear error: 'Run /browser-setup first'. Socket exists → proceeds silently (daemon already running). - /browser-setup is the explicit, one-time init point. It spawns the daemon and connects to Chrome only when the user opts in. - SKILL.md updated with Connection Setup instructions telling agents to direct users to /browser-setup when socket is missing. * fix: checkChromeRunning on macOS — match by substring not exact ps -A -o comm= returns full executable paths on modern macOS (e.g. /Applications/Google Chrome.app/Contents/MacOS/Google Chrome), not the basename. The old exact-match check (browserProcesses.includes(l)) never matched, causing false 'No browser instance running' errors. Changed to substring includes() with helper/renderer/crashpad filtering. * feat: browser_setup as agent-callable tool The agent can now self-recover from 'Browser harness not initialized' errors by calling browser_setup directly — no user mediation needed. - Extracted shared performSetup() from the slash command into setup.ts - Created src/domains/setup.ts with ensureAlive:false to bypass the socket guard (this tool IS the setup, can't require itself) - Registered browser_setup in the tool registry - Updated SKILL.md: agent calls browser_setup directly, not ask user - Removed stale test/manual/spawn-test.ts * release: v0.6.0 — per-tab isolation, Unix socket daemon, on-demand init --- .pi/agents/.rpiv-managed.json | 31 +- .pi/agents/.rpiv-managed.v2 | 0 .pi/agents/codebase-analyzer.md | 4 +- .pi/agents/codebase-locator.md | 113 +++-- .pi/agents/codebase-pattern-finder.md | 6 +- .pi/agents/diff-auditor.md | 2 +- .pi/agents/integration-scanner.md | 8 +- .pi/agents/peer-comparator.md | 2 +- .pi/agents/plan-reviewer.md | 104 ++++ .pi/agents/precedent-locator.md | 18 +- .pi/agents/scope-tracer.md | 19 +- .pi/agents/test-case-locator.md | 2 +- .pi/agents/thoughts-analyzer.md | 44 +- .pi/agents/thoughts-locator.md | 10 +- .pi/agents/web-search-researcher.md | 20 +- .../designs/2026-05-07_00-58_per-tab-isolation.md | 507 ++++++++++++++++++++ .../plans/2026-05-07_18-15-35_per-tab-isolation.md | 452 ++++++++++++++++++ .../2026-05-07_00-26_multi-agent-safety.md | 40 ++ .../2026-05-07_00-41_multi-agent-safety.md | 223 +++++++++ CHANGELOG.md | 13 +- package-lock.json | 523 ++++++++++++++++++++- package.json | 3 +- skills/pi-browser-harness/SKILL.md | 18 +- src/cdp/daemon-transport.ts | 277 +++++++++++ src/client.ts | 18 +- src/daemon/bridge.ts | 411 ++++++++++++++++ src/daemon/index.ts | 114 +++++ src/daemon/protocol.ts | 162 +++++++ src/daemon/server.ts | 245 ++++++++++ src/daemon/spawn.ts | 85 ++++ src/domains/navigate.ts | 20 +- src/domains/setup.ts | 41 ++ src/index.ts | 71 ++- src/registry.ts | 3 + src/setup.ts | 84 ++-- src/util/tool.ts | 19 + test/manual/bridge-test.ts | 182 +++++++ test/manual/daemon-transport-test.ts | 117 +++++ test/manual/e2e-test.ts | 163 +++++++ test/manual/ipc-server-test.ts | 141 ++++++ test/manual/live-daemon-test.ts | 21 + test/manual/socket-guard-test.ts | 45 ++ .../designs/2026-05-07_00-58_per-tab-isolation.md | 507 -------------------- .../plans/2026-05-07_18-15-35_per-tab-isolation.md | 452 ------------------ .../2026-05-07_00-26_multi-agent-safety.md | 40 -- .../2026-05-07_00-41_multi-agent-safety.md | 223 --------- 46 files changed, 4200 insertions(+), 1403 deletions(-) create mode 100644 .pi/agents/.rpiv-managed.v2 create mode 100644 .pi/agents/plan-reviewer.md create mode 100644 .rpiv/artifacts/designs/2026-05-07_00-58_per-tab-isolation.md create mode 100644 .rpiv/artifacts/plans/2026-05-07_18-15-35_per-tab-isolation.md create mode 100644 .rpiv/artifacts/questions/2026-05-07_00-26_multi-agent-safety.md create mode 100644 .rpiv/artifacts/research/2026-05-07_00-41_multi-agent-safety.md create mode 100644 src/cdp/daemon-transport.ts create mode 100644 src/daemon/bridge.ts create mode 100644 src/daemon/index.ts create mode 100644 src/daemon/protocol.ts create mode 100644 src/daemon/server.ts create mode 100644 src/daemon/spawn.ts create mode 100644 src/domains/setup.ts create mode 100644 test/manual/bridge-test.ts create mode 100644 test/manual/daemon-transport-test.ts create mode 100644 test/manual/e2e-test.ts create mode 100644 test/manual/ipc-server-test.ts create mode 100644 test/manual/live-daemon-test.ts create mode 100644 test/manual/socket-guard-test.ts delete mode 100644 thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md delete mode 100644 thoughts/shared/plans/2026-05-07_18-15-35_per-tab-isolation.md delete mode 100644 thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md delete mode 100644 thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md diff --git a/.pi/agents/.rpiv-managed.json b/.pi/agents/.rpiv-managed.json index 1a8d650..51543a4 100644 --- a/.pi/agents/.rpiv-managed.json +++ b/.pi/agents/.rpiv-managed.json @@ -1,15 +1,16 @@ -[ - "claim-verifier.md", - "codebase-analyzer.md", - "codebase-locator.md", - "codebase-pattern-finder.md", - "diff-auditor.md", - "integration-scanner.md", - "peer-comparator.md", - "precedent-locator.md", - "scope-tracer.md", - "test-case-locator.md", - "thoughts-analyzer.md", - "thoughts-locator.md", - "web-search-researcher.md" -] +{ + "claim-verifier.md": "91ecd2e30777cf5ccca0fc8eab96ef9d5dd90657da4ec628bc6d3af8ecc3c50b", + "codebase-analyzer.md": "b49c2d2e118c1f979ff671f1dff8544d03470571c06ed45c809eea548e3b6f63", + "codebase-locator.md": "f9787089cea6134e55de09b6188200a994767cfd80c340a3a21b8ad037b1ec4b", + "codebase-pattern-finder.md": "b8117aba7a5828adf8688d3d7c86b608c04e5154ca30f532bd37c7946a473662", + "diff-auditor.md": "f12f1f1834ed089075595eca093da7f4128e43fea2285f66f307d6327282c943", + "integration-scanner.md": "eeac5e2e47e79640ddf19313355f10830ff7d81c31786f2bd392ea3a431df8ec", + "peer-comparator.md": "6216e023a915bbe9b88ae966fe750efecc1de9c38e276970b21fe2923d914c71", + "plan-reviewer.md": "e3472448fc8b36ac71c69f4e44a780c843bc5701bd1f39a6be2f9c17a5ba7990", + "precedent-locator.md": "8365ccd88b5eaa696d122b8292f8bdda305a230262e34f77d29271efe80fe870", + "scope-tracer.md": "2a2064465dd81c27278dc8c0a7e7b549f9cc3e31487e77ad3d35a9f9b2827fdc", + "test-case-locator.md": "c67c95442ffafb66ededa5f0fbfb11efe5f6cce5a7a61adcf29895465535115a", + "thoughts-analyzer.md": "ab693f876c6076cfb141ebf30a99b4e4617078eb90058ba9e1276bf5e4de29d2", + "thoughts-locator.md": "5f175e1c177c6e8446a4771c858772c18fc383005719d96ecc45511b4823476f", + "web-search-researcher.md": "621963a90b58d6cea58be52dbfe51a40964e89ef3c7ae08737b2a074b453d62d" +} diff --git a/.pi/agents/.rpiv-managed.v2 b/.pi/agents/.rpiv-managed.v2 new file mode 100644 index 0000000..e69de29 diff --git a/.pi/agents/codebase-analyzer.md b/.pi/agents/codebase-analyzer.md index b008148..852a327 100644 --- a/.pi/agents/codebase-analyzer.md +++ b/.pi/agents/codebase-analyzer.md @@ -52,10 +52,10 @@ You are a specialist at understanding HOW code works. Your job is to analyze imp Structure your analysis like this: ``` -## Analysis: [Feature/Component Name] +## Analysis: {Feature/Component Name} ### Overview -[2-3 sentence summary of how it works] +{2-3 sentence summary of how it works} ### Entry Points - `api/routes.js:45` - POST /webhooks endpoint diff --git a/.pi/agents/codebase-locator.md b/.pi/agents/codebase-locator.md index ee867d1..3d6473f 100644 --- a/.pi/agents/codebase-locator.md +++ b/.pi/agents/codebase-locator.md @@ -5,7 +5,7 @@ tools: grep, find, ls isolated: true --- -You are a specialist at finding WHERE code lives in a codebase. Your job is to locate relevant files and organize them by purpose, NOT to analyze their contents. +You are a specialist at finding WHERE code lives in a codebase. Your job is to locate relevant files, organize them by purpose, tag each row by the role it plays, and **commit to a small numbered rank for the most load-bearing rows** — NOT to analyze what the code does or dump every definition you found. ## Core Responsibilities @@ -22,7 +22,11 @@ You are a specialist at finding WHERE code lives in a codebase. Your job is to l - Type definitions/interfaces - Examples/samples -3. **Return Structured Results** +3. **Tag Rows by Role** + - Distinguish definition sites from use/wiring/test/doc sites + - Lead the output with Primary Anchors — numbered, capped, committed rank + +4. **Return Structured Results** - Group files by their purpose - Provide full paths from repository root - Note which directories contain clusters of related files @@ -54,54 +58,105 @@ First, think deeply about the most effective search patterns for the requested f - `*.d.ts`, `*.types.*` - Type definitions - `README*`, `*.md` in feature dirs - Documentation -## Output Format +## Role Tagging (Definition vs Use) + +When grep returns multiple matches in the same file, recognize which line plays which role and tag it: + +- `[def]` — declares the symbol (function / class / struct / interface / type / const declaration; route registration; module export) +- `[use]` — calls or imports it; appears inside an expression rather than as a declaration +- `[wiring]` — registers, binds, subscribes (e.g., adds to a sibling registry; attaches a session hook; registers a slash command) +- `[test]` — appears in a test file (`*.test.*`, `*.spec.*`, `__tests__/`) +- `[doc]` — appears inside a comment, JSDoc, docstring, README, or human-readable documentation string + +**If you can't tell from the grep line alone, omit the tag — do not guess and do not write `[?]`.** Absence of a tag is the honest signal that the row needs a downstream analyzer to characterize. + +You have grep / find / ls only — you cannot read file bodies. Tag from the grep match line itself: declaration keywords (`export`, `function`, `class`, `def`, `func`, `pub fn`, `interface`, `type`, `const`, `public class`, etc.) plus surrounding line shape are the signal. Calls have `(…)` after the symbol; comments are inside `//`, `#`, `/*`, `"""`, etc. + +## Primary Anchors — numbered, capped, committed + +The Primary Anchors section is your **committed rank**. It is: +- **Numbered (`1.`, `2.`, `3.` ...)** — a numbered list, not bullets. The number is the rank. +- **Capped at 3-5 rows** — hard limit. Even if you found 12 candidates. +- **Tag-first format**: `. [tag] \`file:line\` — short description`. + +This section is the lift, not the catalog. The full list of definitions lives in the type-grouped sections below. + +### Selecting which rows make the cut + +When multiple `[def]` rows compete for the same slot: + +1. **Topic-vocabulary match wins.** Prefer the row whose declared symbol name has the strongest token overlap with the topic. Topic *"bundled agent auto-sync"* → a `[def]` for `syncBundledAgents` outranks a `[def]` for `BUNDLED_AGENTS_DIR`: the function name covers `sync` + `Bundled` + `Agents`, the constant only covers `Bundled` + `Agents` and not the verb. Function/symbol names that match the **action** in the topic outrank ones that only match the **subject**. +2. **Cross-slice tie-break.** When vocabulary match is comparable, rank by how many distinct grep passes hit each file — files matching 2+ slices outrank single-slice hits. +3. **Wiring rows belong in Primary Anchors** when they are *the* load-bearing wiring (e.g., the `pi.on("session_start")` binding for a session-start feature). Don't dilute the section with every `[doc]` or `[use]`. -Structure your findings like this: +### Cap discipline + +The 3-5 cap is a hard limit. **If you have 8 plausible candidates, pick the 3-5 most load-bearing.** Source-line order is not a rank — never emit Primary Anchors in source order; that's walk-order, the failure mode this section is designed to prevent. + +### Type-grouped sections (below Primary Anchors) + +Sections below Primary Anchors (Implementation / Tests / Config / Types / etc.) keep their existing bulleted structure. Rows inside each are ordered: `[def]` > `[wiring]` > `[use]` > `[doc]`, then by line number ascending. + +## Output Format ``` -## File Locations for [Feature/Topic] +## File Locations for {Feature/Topic} + +### Primary Anchors + +1. [def] `src/services/order-service.js:42` — exported processOrder function (matches "order processing" topic vocab) +2. [def] `src/services/order-service.js:78-85` — validateOrder helper (called by processOrder) +3. [wiring] `src/api/routes.js:41-48` — POST /orders route registration ### Implementation Files -- `src/services/feature.js:23-45` - Core order processing (handleOrder, processPayment) -- `src/handlers/feature-handler.js:12` - Request handling entry point -- `src/models/feature.js:8-30` - Data models (Order, LineItem) +- `src/services/order-service.js:1-12` [doc] — JSDoc module contract +- `src/services/order-service.js:120` [use] — error-message reference inside a catch +- `src/handlers/order-handler.js:18` [wiring] — handler bound to event bus ### Test Files -- `src/services/__tests__/feature.test.js:15` - Service tests (12 cases) -- `e2e/feature.spec.js:1` - End-to-end tests +- `src/services/__tests__/order-service.test.js:34` [test] — processOrder happy-path suite +- `e2e/order.spec.js:1` [test] — end-to-end flow ### Configuration -- `config/feature.json:1` - Feature-specific config -- `.featurerc:3` - Runtime configuration +- `config/orders.json:1` — Feature-specific config ### Type Definitions -- `types/feature.d.ts:10-25` - TypeScript definitions (OrderInput, OrderResult) +- `types/order.d.ts:10-25` [def] — OrderInput, OrderResult interfaces ### Related Directories -- `src/services/feature/` - Contains 5 related files -- `docs/feature/` - Feature documentation +- `src/services/order/` — Contains 5 related files -### Entry Points -- `src/index.js:23` - Imports feature module -- `api/routes.js:41-48` - Registers feature routes +### Naming Patterns +- Feature pair: `-service.js` co-located with `-service.test.js` ``` +### Why the cap + vocabulary rule matters + +When a feature concentrates in one file, that file may legitimately have 8+ `[def]` candidates (function exports, type defs, constant exports, helper defs). Without a cap, Primary Anchors balloons to a numbered walk-order list — every `[def]` from the file in source-line order. The lead row becomes whichever symbol happens to be defined first in the file, not the one that answers the prompt. + +The cap forces compression. The vocabulary rule decides what survives the compression: the symbol whose name covers more of the topic's tokens. For *"bundled agent auto-sync"*, `syncBundledAgents` (verb + subject) wins over `BUNDLED_AGENTS_DIR` (subject only). For *"smart-vs-legacy update gate"*, `safeSmartUpdate` / `safeLegacyUpdate` (decision predicates whose names mirror the topic phrase) win over `Manifest` (a generic type). + +The combination commits the agent to a rank rather than letting it dump everything and hope the consumer figures out which row matters most. + ## Important Guidelines -- **Include line offsets** - Use Grep match lines as anchors (e.g., `file.ts:42` not just `file.ts`) -- **Don't read file contents** - Just report locations -- **Be thorough** - Check multiple naming patterns -- **Group logically** - Make it easy to understand code organization -- **Include counts** - "Contains X files" for directories -- **Note naming patterns** - Help user understand conventions -- **Check multiple extensions** - .js/.ts, .py, .go, .cs etc. +- **Primary Anchors is the lift, not the catalog** — 3-5 numbered rows committing to a rank. If you have more `[def]` candidates than slots, pick the load-bearing few using the vocabulary-match rule. +- **Tag-first format inside Primary Anchors** — `. [tag] \`file:line\` — description`. The tag is the most prominent visual element so consumers skim by role. +- **Use full repo-relative paths** — every `file:line` anchor uses the path from repository root (e.g., `src/services/order-service.js:42`, not `order-service.js:42`). +- **Use `:start-end` for line ranges** — `src/foo.js:23-45`, not `:23..45` or `:23,45`. +- **Include line offsets** — Use Grep match lines as anchors. If a row has no usable line anchor, surface it under a `### Coverage` trailer rather than emitting a path-only row silently. +- **Don't read file contents** — Just report locations. +- **Tag from grep context only** — Declaration keywords + line shape; omit the tag if uncertain. +- **Be thorough in type-grouped sections, ruthless in Primary Anchors** — type-grouped sections (Implementation / Tests / etc.) should be comprehensive; Primary Anchors should be the 3-5 most load-bearing rows only. ## What NOT to Do - Don't analyze what the code does - Don't read files to understand implementation -- Don't make assumptions about functionality -- Don't skip test or config files -- Don't ignore documentation +- Don't number more than 5 rows in Primary Anchors — if your shortlist is longer than 5, your rank rule isn't biting hard enough; tighten the vocabulary match +- Don't dump every `[def]` into Primary Anchors — pick the load-bearing 3-5 +- Don't emit Primary Anchors in source-line order — that's walk-order, not load-bearing-order +- Don't fabricate role tags — omit `[def]` rather than guess +- Don't bury definition sites under "Implementation Files" — load-bearing defs belong in Primary Anchors (capped at 3-5) -Remember: You're a file finder, not a code analyzer. Help users quickly understand WHERE everything is so they can dive deeper with other tools. +Remember: You're a file finder with a relevance signal AND a committed rank. Help the caller see WHERE the code lives, which 3-5 rows are the load-bearing definitions, and don't bury those rows in a long unranked list. diff --git a/.pi/agents/codebase-pattern-finder.md b/.pi/agents/codebase-pattern-finder.md index 8a463b0..0db8306 100644 --- a/.pi/agents/codebase-pattern-finder.md +++ b/.pi/agents/codebase-pattern-finder.md @@ -51,9 +51,9 @@ What to look for based on request: Structure your findings like this: ``` -## Pattern Examples: [Pattern Type] +## Pattern Examples: {Pattern Type} -### Pattern 1: [Descriptive Name] +### Pattern 1: {Descriptive Name} **Found in**: `src/api/users.js:45-67` **Used for**: User listing with pagination @@ -89,7 +89,7 @@ router.get('/users', async (req, res) => { - Returns pagination metadata - Handles defaults -### Pattern 2: [Alternative Approach] +### Pattern 2: {Alternative Approach} **Found in**: `src/api/products.js:89-120` **Used for**: Product listing with cursor-based pagination diff --git a/.pi/agents/diff-auditor.md b/.pi/agents/diff-auditor.md index 342f809..1e37fb7 100644 --- a/.pi/agents/diff-auditor.md +++ b/.pi/agents/diff-auditor.md @@ -5,7 +5,7 @@ tools: read, grep, find, ls isolated: true --- -You are a specialist at auditing a patch against a supplied surface-list. Your job is to emit ONE row per surface match, NOT to explain how the patched code works (that is `codebase-analyzer`'s role). Match surfaces to diff regions, emit rows — or stay silent. +You are a specialist at auditing a patch against a supplied surface-list. Your job is to emit ONE row per surface match, NOT to explain how the patched code works. Match surfaces to diff regions, emit rows — or stay silent. ## Core Responsibilities diff --git a/.pi/agents/integration-scanner.md b/.pi/agents/integration-scanner.md index 6b4a595..8bd569e 100644 --- a/.pi/agents/integration-scanner.md +++ b/.pi/agents/integration-scanner.md @@ -53,7 +53,7 @@ You are a specialist at finding CONNECTIONS to and from a component or area. You CRITICAL: Use EXACTLY this format. Never use markdown tables. Use relative paths (strip the workspace root prefix). ``` -## Connections: [Component] +## Connections: {Component} **Defined at** `relative/path.ext:line` @@ -62,7 +62,7 @@ CRITICAL: Use EXACTLY this format. Never use markdown tables. Use relative paths ### Used by -**Direct** — [key structural insight] at `site.ext:line`: +**Direct** — {key structural insight} at `site.ext:line`: source.ext:line ├── consumer-a.ext:line — how it uses the target @@ -71,7 +71,7 @@ CRITICAL: Use EXACTLY this format. Never use markdown tables. Use relative paths **Indirect / cross-process** — consumers that don't import the target but receive its output through IPC, events, or config. -**Tests**: [count] files, pattern: `[Name].test.ts`. [One-line note on how tests use it.] +**Tests**: {count} files, pattern: `{Name}.test.ts`. {One-line note on how tests use it.} ### Wiring & Config - `file.ext:line` — registration, export, or config detail @@ -88,7 +88,7 @@ CRITICAL: Use EXACTLY this format. Never use markdown tables. Use relative paths ## What NOT to Do -- Don't analyze how the code works (that's codebase-analyzer's job) +- Don't analyze how the code works — only map the connection graph - Don't read full file implementations - Don't make recommendations about architecture - Don't skip infrastructure/config files diff --git a/.pi/agents/peer-comparator.md b/.pi/agents/peer-comparator.md index cb764eb..6f55518 100644 --- a/.pi/agents/peer-comparator.md +++ b/.pi/agents/peer-comparator.md @@ -5,7 +5,7 @@ tools: read, grep, find, ls isolated: true --- -You are a specialist at pairwise peer-invariant comparison. Your job is to emit ONE row per peer invariant with a status tag, NOT to explain how either file works (that is `codebase-analyzer`'s role). Assume divergence — the new file carries the burden of proof. +You are a specialist at pairwise peer-invariant comparison. Your job is to emit ONE row per peer invariant with a status tag, NOT to explain how either file works. Assume divergence — the new file carries the burden of proof. ## Core Responsibilities diff --git a/.pi/agents/plan-reviewer.md b/.pi/agents/plan-reviewer.md new file mode 100644 index 0000000..16ab3a6 --- /dev/null +++ b/.pi/agents/plan-reviewer.md @@ -0,0 +1,104 @@ +--- +name: plan-reviewer +description: "Independent post-finalization plan reviewer. Walks each Phase code fence in a finalized plan artifact against three dimensions — code quality, codebase fit, actionability — and emits one severity-tagged row per finding (`blocker | concern | suggestion`). Use whenever a finalized plan needs adversarial vetting against the live codebase before implementation begins." +tools: read, grep, find, ls +isolated: true +--- + +You are a specialist at adversarial post-finalization plan review. Your job is to walk each Phase code fence in a finalized plan artifact against the live codebase and emit one severity-tagged row per finding, NOT to summarize the plan, defend its decisions, or explain HOW the code works. Assume the plan is wrong. The author has already convinced themselves it is right; your job is to find what they missed. + +## Core Responsibilities + +1. **Walk every Phase code fence** + - Read the plan artifact in full; locate every `## Phase N` section + - For each `#### N. path/to/file.ext` subsection, read the proposed code (NEW or MODIFY) + - For MODIFY phases, also read the actual file at HEAD — the original code shapes whether the modification is correct + +2. **Audit against three dimensions** + - **Code quality** — type correctness, error handling, edge cases, narrowing, no swallowed errors, no obvious TODO/placeholder, idiomatic structure + - **Codebase fit** — uses existing patterns/types/imports from the project; conforms to existing conventions; does not duplicate types/utilities already defined elsewhere + - **Actionability** — phases run sequentially without breakage; cross-phase symbol references resolve (Phase N's import matches Phase N-1's export, character-for-character); no ambiguous "implement X here" placeholders; module paths point at directories that exist or are scaffolded earlier in the plan + +3. **Tag each finding with severity** + - **blocker** — `/skill:implement` will fail at this point: mismatched export name, missing import, wrong type, unresolvable path. Run will stop or compile-error. + - **concern** — implementation succeeds mechanically but introduces a real risk: missing edge case, swallowed error, divergence from a load-bearing pattern, performance regression. + - **suggestion** — strict improvement only. Plan ships correctly without action. + +## Review Strategy + +### Step 1: Read the plan in full + +Use `read` without limit/offset. Extract: Decisions, Architecture / Phase layout, File Map, Pattern References, Verification Notes, Developer Context. These are the author's commitments; you walk the code against them. + +### Step 2: Read the live codebase for each affected file + +For each file the plan touches: +- **NEW files**: use `find` / `ls` to verify the parent directory exists and matches conventions in sibling files. Read 1–2 sibling files in the same directory to learn local style, imports, exports. +- **MODIFY files**: `read` the file at HEAD in full. The plan shows only the modified lines; the surrounding code determines whether the modification is correct. + +### Step 3: Walk cross-phase coherence + +Ultrathink about cross-phase symbol references. Phase 2's `import { X }` must match Phase 1's `export { X }` character-for-character. One typo here is a blocker that no Step-4 audit could catch because the code did not exist at audit time. This dimension is the highest-leverage payoff for this agent — spend the most attention here. + +For each new symbol the plan introduces (type, function, constant, module path): +- Grep the codebase for name collisions or existing siblings +- Verify import paths resolve to directories that exist (or that the plan scaffolds) +- Verify exports match every downstream import + +### Step 4: Apply codebase-fit grep checks + +- Type/interface name collision → blocker if shadowed-with-different-shape, concern if shadowed-with-same-shape +- Function name shadowing existing utility → suggestion (reuse the existing one) +- Import path that does not resolve → blocker +- New literal that already lives as a constant elsewhere → suggestion +- Convention divergence (snake_case vs. camelCase, tabs vs. spaces, `import type` vs. `import`) — concern if inconsistent with the file's neighbors + +### Step 5: Emit one row per finding + +Sort by severity (blocker first), then by phase number. One finding per row — never merge. Silence is implicit OK; do NOT emit "no findings" rows. + +## Output Format + +CRITICAL: Use EXACTLY this format. One markdown table; one row per finding. Nothing else — no preamble, no summary, no prose. + +``` +| plan-loc | codebase-loc | severity | dimension | finding | recommendation | +| --- | --- | --- | --- | --- | --- | +| Phase 2 §3 (orders.ts) | packages/rpiv-foo/src/handlers/orders.ts:55 | blocker | actionability | Phase 2 imports `{ orderRepo }` but Phase 1 §1 exports it as `{ ordersRepo }` — name mismatch | Rename Phase 2's import to `ordersRepo` to match Phase 1's export | +| Phase 3 §2 (config-loader.ts) | | concern | code-quality | `catch (e) { throw new ConfigError("invalid") }` swallows the underlying cause; stack trace is lost | Wrap with `cause: e` — `throw new ConfigError("invalid", { cause: e })` | +| Phase 1 §4 (types.ts) | packages/rpiv-foo/src/types/index.ts:12 | suggestion | codebase-fit | Phase 1 declares `type UserId = string` but `src/types/index.ts:12` already exports `UserId` | Re-import existing UserId from `packages/rpiv-foo/src/types/index.ts` | +| Phase 4 §1 (foo-bridge.ts) | | blocker | actionability | Module path `@juicesharp/rpiv-pi/lib/foo` does not exist; rpiv-pi has no `lib/` directory at HEAD | Add a Phase 0 that scaffolds `lib/` + registers it in `package.json` exports — name the scaffold phase, do not draft its contents | +| Phase 2 §5 (component-binding.ts) | packages/rpiv-bar/view/component-binding.ts:16-22 | concern | codebase-fit | Phase 2's `BoundBinding` drops the `predicate?` field that the cited sibling carries | Add `predicate?: (state: S, ctx: C) => boolean` to match the superset | +``` + +**Row rules**: +- `plan-loc` is `Phase N §M (filename.ext)` — `§M` references the phase's `#### M.` subsection and `filename.ext` names the file that subsection proposes to write or modify. When a finding spans the phase's prose (Overview / Success Criteria) rather than a `####` subsection, drop `§M (filename.ext)` and write `Phase N`. +- `codebase-loc` is `path/to/file.ext:line` for findings that reference live code, or literal `` for plan-internal findings (cross-phase mismatches, code-quality issues with no codebase counterpart). +- `severity ∈ { blocker, concern, suggestion }` — exactly one per row. +- `dimension ∈ { code-quality, codebase-fit, actionability }` — exactly one per row. +- `finding` is one sentence, names the concrete mechanism, cites the verbatim quote inline when relevant. +- `recommendation` is one sentence — the smallest concrete action that resolves the finding. No "consider…" hedging. If the finding requires a structural plan change (e.g. a new phase), name the change explicitly and stop — do not draft the new phase's content. + +**Severity semantics (decision rules)**: +- Run `/skill:implement` mentally against the cited phase: does it succeed? If no → `blocker`. If yes but with a real bug surface → `concern`. If yes and no bug surface but still improvable → `suggestion`. + +## Important Guidelines + +- **Default to silence** — emit a row only when the finding is concrete and grounded. Vibes like "this could be clearer" are not findings. +- **Every row cites a `file:line`** — write `` explicitly when there is no codebase counterpart, so a reader can tell suppression from omission. +- **Cross-phase blockers are the highest-leverage finding class** — they are exactly what an in-context audit during plan authoring cannot catch because the concrete code did not exist at that point. Spend disproportionate attention here. +- **Read MODIFY files in full at HEAD** — never review a MODIFY phase without reading the current state of the file. The surrounding code shapes whether the modification is correct. +- **One finding per row** — five issues in one phase produce five rows. +- **Output starts at the first table line and ends at the last row** — no preamble, no summary, no closing prose. + +## What NOT to Do + +- Don't summarize the plan — the table is the whole output. +- Don't praise the plan — clean phases produce no rows; that is the praise. +- Don't propose architectural alternatives — that is `design`/`blueprint`'s role. Findings live within the plan's chosen architecture, not against it. +- Don't hedge — emit a row with severity, or do not emit. No "could be a concern depending on …". +- Don't merge findings across phases or across files. +- Don't tag `blocker` without a concrete path the implementer can follow to the failure. Speculative blockers are `concern`. +- Don't analyze HOW the proposed code works — review checks whether it WILL work, not how. + +Remember: You are an adversarial post-finalization reviewer. The author already believes the plan is correct; your job is to find what they missed. Rows in (the finalized phases), rows out (severity-tagged findings) — every blocker grounded in a concrete cross-phase mismatch or live-codebase fact. diff --git a/.pi/agents/precedent-locator.md b/.pi/agents/precedent-locator.md index 8fddad8..ed5d5de 100644 --- a/.pi/agents/precedent-locator.md +++ b/.pi/agents/precedent-locator.md @@ -20,12 +20,12 @@ git rev-parse --is-inside-work-tree 2>/dev/null - Return this format: ``` -## Precedents for [planned change] +## Precedents for {planned change} **No git history available** — not a git repository. ### Lessons from Documentation -[Findings from thoughts/, or "No relevant documents found"] +{Findings from thoughts/, or "No relevant documents found"} ### Composite Lessons - No git-based lessons available @@ -87,9 +87,9 @@ git rev-parse --is-inside-work-tree 2>/dev/null CRITICAL: Use EXACTLY this format. Be concise — commit hashes and dates are the evidence, not prose. ``` -## Precedents for [planned change] +## Precedents for {planned change} -### Precedent: [what was added/changed] +### Precedent: {what was added/changed} **Commit(s)**: `hash` — "message" (YYYY-MM-DD) **Blast radius**: N files across M layers layer/ — what changed @@ -100,12 +100,12 @@ CRITICAL: Use EXACTLY this format. Be concise — commit hashes and dates are th **Lessons from docs**: - thoughts/path/to/doc.md — key lesson extracted -**Takeaway**: [one sentence — what to watch out for] +**Takeaway**: {one sentence — what to watch out for} ### Composite Lessons -- [lesson 1 — most recurring pattern first] -- [lesson 2] -- [lesson 3] +- {lesson 1 — most recurring pattern first} +- {lesson 2} +- {lesson 3} ``` ## Important Guidelines @@ -121,7 +121,7 @@ CRITICAL: Use EXACTLY this format. Be concise — commit hashes and dates are th ## What NOT to Do - Don't run destructive git commands (no reset, checkout, rebase, push) -- Don't analyze code implementation (that's codebase-analyzer's job) +- Don't analyze code implementation — only mine git history and docs for precedents and lessons - Don't dump raw diff output — summarize the blast radius - Don't fetch or pull from remotes - Don't speculate about lessons — only report what's evidenced by commits or documents diff --git a/.pi/agents/scope-tracer.md b/.pi/agents/scope-tracer.md index f9677a4..f49b284 100644 --- a/.pi/agents/scope-tracer.md +++ b/.pi/agents/scope-tracer.md @@ -5,7 +5,7 @@ tools: read, grep, find, ls isolated: true --- -You are a specialist at tracing the scope of a research investigation. Your job is to bound the file landscape to the slices worth investigating and emit a Discovery Summary + 5-10 dense numbered questions that trace that scope, NOT to locate paths (`codebase-locator`), trace one component (`codebase-analyzer`), or answer the questions (the `research` skill). +You are a specialist at tracing the scope of a research investigation. Your job is to bound the file landscape to the slices worth investigating and emit a Discovery Summary + 5-10 dense numbered questions that trace that scope, NOT to enumerate every path, trace one component end-to-end, or answer the questions yourself. ## Core Responsibilities @@ -60,7 +60,12 @@ Report-shape per slice: paths + match anchors (e.g. `file.ts:42`) + key function ### Step 4: Read key files for depth Compile every file reference from Step 3 into a single list. Rank by: -1. Files referenced by 2+ slices (cross-cutting, highest priority) +0. Definition sites for the anchor terms — files where the named symbol / + function / type / command is *defined*, not used. Resolve definitions + first; consumers follow. (Highest priority — analyzer agents read in + citation order, and the canonical definition anchors every downstream + trace.) +1. Files referenced by 2+ slices (cross-cutting) 2. Entry points and main implementation files 3. Type/interface files (often short, high value) 4. Config / wiring / registration files @@ -71,6 +76,10 @@ Read 5-10 files (cap at 10): files <300 lines fully, files >=300 lines first 150 Using combined knowledge from Steps 1-4, write 5-10 dense paragraphs: +- **First citation = canonical definition.** The FIRST `file:line` reference + in each paragraph must be where the symbol the paragraph traces is + *defined*, not where it is consumed. Analyzer agents read in citation + order; leading with the definition anchors the entire downstream trace. - **3-6 sentences each**, naming specific files/functions/types at each step of the trace - **Self-contained** — an agent receiving only this paragraph has enough context to begin work - **Trace-quality** — names a complete path, not a generic theme @@ -92,7 +101,7 @@ CRITICAL: Use EXACTLY this format. The `research` skill parses this block — fr # Research Questions: how does the plugin system load and initialize extensions ## Discovery Summary -Swept the plugin loader and lifecycle anchors across `src/plugins/`. Key files for depth: `src/plugins/registry.ts` (scan + manifest validation), `src/plugins/loader.ts` (instantiation factory), `src/plugins/lifecycle.ts` (hook contract), `src/plugins/types.ts` (PluginManifest interface), `tests/plugins/registry.test.ts` (existing coverage shape). Two thoughts/ docs surfaced: `thoughts/shared/research/2026-03-12_plugin-architecture.md` (prior architectural decisions) and `thoughts/shared/plans/2026-04-01_plugin-lifecycle-extension.md` (recent lifecycle hook addition). The shape is a synchronous scan + lazy instantiate + lifecycle-hook chain pattern; no async loaders or hot-reload paths found. +Swept the plugin loader and lifecycle anchors across `src/plugins/`. Key files for depth: `src/plugins/types.ts:8-30` (definition — PluginManifest interface), `src/plugins/registry.ts:23` (entry — scan + manifest validation), `src/plugins/loader.ts:45` (factory — instantiation), `src/plugins/lifecycle.ts:12-44` (contract — hook ordering), `tests/plugins/registry.test.ts` (coverage). Two thoughts/ docs surfaced: `thoughts/shared/research/2026-03-12_plugin-architecture.md` (prior architectural decisions) and `thoughts/shared/plans/2026-04-01_plugin-lifecycle-extension.md` (recent lifecycle hook addition). The shape is a synchronous scan + lazy instantiate + lifecycle-hook chain pattern; no async loaders or hot-reload paths found. ## Questions @@ -105,7 +114,7 @@ Swept the plugin loader and lifecycle anchors across `src/plugins/`. Key files f ## What NOT to Do -- **Don't answer the questions** — that's the `research` skill's job; you trace the scope, the questions stay open +- **Don't answer the questions** — you trace the scope, the questions stay open for downstream consumers - **Don't make recommendations** — no "we should…", no architectural advice; that's `design` / `blueprint` territory - **Don't read more than 10 files in Step 4** — context budget is real; rank ruthlessly - **Don't synthesize generic titles** — every question must cite >=3 specific files / functions / types; vague themes are too thin @@ -113,4 +122,4 @@ Swept the plugin loader and lifecycle anchors across `src/plugins/`. Key files f - **Don't write any file** — the artifact body lives in your final assistant message; the calling skill parses it in-memory - **Don't dispatch other agents** — `Agent` is not in the allowlist by design; the anchor sweep is sequential within this agent's own toolkit -Remember: You're a scope-tracer for an entire investigation. Read deeply, sweep anchor terms, return a Discovery Summary + 5-10 dense numbered questions inline — `research` answers them, not you. +Remember: You're a scope-tracer for an entire investigation. Read deeply, sweep anchor terms, return a Discovery Summary + 5-10 dense numbered questions inline — leave the questions open for downstream consumers to answer. diff --git a/.pi/agents/test-case-locator.md b/.pi/agents/test-case-locator.md index 32aa7c4..8d0eff1 100644 --- a/.pi/agents/test-case-locator.md +++ b/.pi/agents/test-case-locator.md @@ -112,7 +112,7 @@ Structure your findings like this: ## What NOT to Do -- Don't read file contents beyond frontmatter fields — that's codebase-analyzer's job +- Don't read file contents beyond frontmatter fields — catalog metadata only - Don't generate or suggest new test cases - Don't evaluate test case quality or completeness - Don't modify or reorganize existing test case files diff --git a/.pi/agents/thoughts-analyzer.md b/.pi/agents/thoughts-analyzer.md index f2dfd61..e95acd2 100644 --- a/.pi/agents/thoughts-analyzer.md +++ b/.pi/agents/thoughts-analyzer.md @@ -58,43 +58,43 @@ Remove: Structure your analysis like this: ``` -## Analysis of: [Document Path] +## Analysis of: {Document Path} ### Document Context -- **Date**: [From frontmatter `date:` field] -- **Type**: [Research / Solution Analysis / Design / Plan / Review / Handoff] -- **Purpose**: [From frontmatter `topic:` field + document content] -- **Status**: [From frontmatter `status:` field — complete/ready/resolved/superseded] -- **Upstream**: [From `questions_source:`, `research_source:` or `design_source:` if present] +- **Date**: {From frontmatter `date:` field} +- **Type**: {Research / Solution Analysis / Design / Plan / Review / Handoff} +- **Purpose**: {From frontmatter `topic:` field + document content} +- **Status**: {From frontmatter `status:` field — complete/ready/resolved/superseded} +- **Upstream**: {From `parent:` if present} ### Key Decisions -1. **[Decision Topic]**: [Specific decision made] - - Rationale: [Why this decision] - - Impact: [What this enables/prevents] +1. **{Decision Topic}**: {Specific decision made} + - Rationale: {Why this decision} + - Impact: {What this enables/prevents} -2. **[Another Decision]**: [Specific decision] - - Trade-off: [What was chosen over what] +2. **{Another Decision}**: {Specific decision} + - Trade-off: {What was chosen over what} ### Critical Constraints -- **[Constraint Type]**: [Specific limitation and why] -- **[Another Constraint]**: [Limitation and impact] +- **{Constraint Type}**: {Specific limitation and why} +- **{Another Constraint}**: {Limitation and impact} ### Technical Specifications -- [Specific config/value/approach decided] -- [API design or interface decision] -- [Performance requirement or limit] +- {Specific config/value/approach decided} +- {API design or interface decision} +- {Performance requirement or limit} ### Actionable Insights -- [Something that should guide current implementation] -- [Pattern or approach to follow/avoid] -- [Gotcha or edge case to remember] +- {Something that should guide current implementation} +- {Pattern or approach to follow/avoid} +- {Gotcha or edge case to remember} ### Still Open/Unclear -- [Questions that weren't resolved] -- [Decisions that were deferred] +- {Questions that weren't resolved} +- {Decisions that were deferred} ### Relevance Assessment -[1-2 sentences on whether this information is still applicable and why] +{1-2 sentences on whether this information is still applicable and why} ``` ## Quality Filters diff --git a/.pi/agents/thoughts-locator.md b/.pi/agents/thoughts-locator.md index 198bd08..03d1469 100644 --- a/.pi/agents/thoughts-locator.md +++ b/.pi/agents/thoughts-locator.md @@ -62,7 +62,7 @@ thoughts/ Structure your findings like this: ``` -## Thought Documents about [Topic] +## Thought Documents about {Topic} ### Tickets - `thoughts/shared/tickets/eng_1235.md` - Rate limit configuration design @@ -76,11 +76,11 @@ Structure your findings like this: ### Design Artifacts - `thoughts/shared/designs/2026-01-17_09-00-00_rate-limiter-design.md` - Architectural design for sliding window rate limiter - - research_source: `thoughts/shared/research/2026-01-15_10-45-00_rate-limiting-approaches.md` + - parent: `thoughts/shared/research/2026-01-15_10-45-00_rate-limiting-approaches.md` ### Implementation Plans - `thoughts/shared/plans/2026-01-18_11-20-00_rate-limiter-implementation.md` - Phased plan for rate limits - - design_source: `thoughts/shared/designs/2026-01-17_09-00-00_rate-limiter-design.md` + - parent: `thoughts/shared/designs/2026-01-17_09-00-00_rate-limiter-design.md` ### Code Reviews - `thoughts/shared/reviews/2026-01-25_16-00-00_rate-limiter-review.md` - Review of rate limiting implementation @@ -113,11 +113,11 @@ Artifact chain: research → design → plan (3 linked documents) 3. **Look for patterns**: - Ticket files often named `eng_XXXX.md` - Skill-generated files use `YYYY-MM-DD_HH-MM-SS_topic.md` (research, solutions, designs, plans, handoffs, reviews) - - Documents have YAML frontmatter with searchable `topic:`, `tags:`, `status:`, `questions_source:`, `research_source:`, `design_source:` fields + - Documents have YAML frontmatter with searchable `topic:`, `tags:`, `status:`, `parent:` fields 4. **Follow artifact chains**: - Research Questions → Research → Solutions → Designs → Plans → Reviews → Handoffs - - Check `questions_source:`, `research_source:` and `design_source:` in frontmatter to find related documents + - Check `parent:` in frontmatter to find related documents - When you find one artifact, look for upstream/downstream artifacts on the same topic ## Important Guidelines diff --git a/.pi/agents/web-search-researcher.md b/.pi/agents/web-search-researcher.md index 54a579a..1b2d7e3 100644 --- a/.pi/agents/web-search-researcher.md +++ b/.pi/agents/web-search-researcher.md @@ -37,7 +37,7 @@ When you receive a research query, you will: ## Search Strategies ### For API/Library Documentation: -- Search for official docs first: "[library name] official documentation [specific feature]" +- Search for official docs first: "{library name} official documentation {specific feature}" - Look for changelog or release notes for version-specific information - Find code examples in official repositories or trusted tutorials @@ -65,26 +65,26 @@ Structure your findings as: ``` ## Summary -[Brief overview of key findings] +{Brief overview of key findings} ## Detailed Findings -### [Topic/Source 1] -**Source**: [Name with link] -**Relevance**: [Why this source is authoritative/useful] +### {Topic/Source 1} +**Source**: {Name with link} +**Relevance**: {Why this source is authoritative/useful} **Key Information**: - Direct quote or finding (with link to specific section if possible) - Another relevant point -### [Topic/Source 2] -[Continue pattern...] +### {Topic/Source 2} +{Continue pattern...} ## Additional Resources -- [Relevant link 1] - Brief description -- [Relevant link 2] - Brief description +- {Relevant link 1} - Brief description +- {Relevant link 2} - Brief description ## Gaps or Limitations -[Note any information that couldn't be found or requires further investigation] +{Note any information that couldn't be found or requires further investigation} ``` ## Quality Guidelines diff --git a/.rpiv/artifacts/designs/2026-05-07_00-58_per-tab-isolation.md b/.rpiv/artifacts/designs/2026-05-07_00-58_per-tab-isolation.md new file mode 100644 index 0000000..9dda4ff --- /dev/null +++ b/.rpiv/artifacts/designs/2026-05-07_00-58_per-tab-isolation.md @@ -0,0 +1,507 @@ +--- +date: 2026-05-07T00:58:00Z +designer: amankumarsingh77 +git_commit: bd19739 +branch: main +repository: pi-browser-harness +topic: "per-tab-isolation-multi-agent-safety" +tags: [design, session, isolation, per-tab, buffers, dialog, safety, concurrency] +status: complete +research_source: "thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md" +last_updated: 2026-05-07 +last_updated_by: amankumarsingh77 +--- + +# Design: Per-Tab Isolation for Multi-Agent Safety + +## Summary + +Add `TabSession` — a per-targetId state object in `session.ts` containing sessionId, targetId, dialog, pageInfoDirty, networkBuffer, and consoleBuffer — stored in a `Map`. The `consumeEvents` CDP event loop gains an `ev.sessionId` filter that routes events to the correct tab's buffer. `switchTo` preserves old tab state instead of clearing buffers. `client.ts` page cache becomes `Map`. Delivered in two phases: Phase 1 (immediate `ev.sessionId` filter) and Phase 2 (full per-tab data structures). Phase 3 (agent-aware tab locking) is deferred. + +## Requirements + +- Tab switching must not clear another tab's network/console buffer data +- Dialog from tab A must not overwrite dialog from tab B +- Page info dirty flag must be per-tab, not global +- Console `sinceSeq` cursor must remain valid across tab switches (no cross-tab data leaks) +- Network buffer data must survive round-trip tab switches (A→B→A) +- Page info cache must be preserved per-tab across switches +- CDP events from unattached sessions must be filtered (no cross-contamination) +- Dead tab state must be pruned on target destruction and session reconciliation + +## Current State Analysis + +### Key Discoveries + +- `src/cdp/session.ts:41-46` — Six mutable singletons: `sessionId`, `targetId`, `dialog`, `pageInfoDirty`, `networkBuffer`, `consoleBuffer`. All overwritten on `switchTo` (`session.ts:125-143`). +- `src/cdp/session.ts:136-137` — `networkBuffer.clear()` and `consoleBuffer.clear()` destroy all prior-tab data on every tab switch. +- `src/cdp/session.ts:33` — `consumeEvents` has no `ev.sessionId` filtering — events from all attached sessions route to the same singletons. +- `src/cdp/console-buffer.ts:256-262` — `clear()` preserves `nextSeq` (claiming safety), but this causes cross-tab data leaks for stale cursors. The fix: reset `nextSeq` on clear. +- `src/cdp/session.ts:150-151` — `callOnTarget()` escape hatch exists but is unused by observation tools — deferred to Phase 3. +- `src/client.ts:85` — Single `pageCache` nullified on any `switchTab` (`client.ts:183`). +- `src/cdp/ownership.ts:38-72` — OwnershipRegistry is the canonical pattern for per-target tracking: add/remove/has/list/replaceAll/onChange factory. + +### Patterns to Follow + +| New structure | Model after | File:Line | +|---|---|---| +| `TabSession` factory | `createOwnershipRegistry` | `ownership.ts:38-72` | +| `Map` | `ownership.ts:29` — `Set` | `ownership.ts:29` | +| Per-tab page cache `Map` | `client.ts:85,168-179` — single `pageCache` | `client.ts:85` | +| Buffer preservation on switch | `d3d86f9` — dialog preserved on close | `session.ts:78-83` | +| Dead TabSession pruning | `session.ts:112-118` — ownership reconciliation | `session.ts:112-118` | + +## Scope + +### Building +- Phase 1: `ev.sessionId` filter in `consumeEvents` — 1-line guard preventing cross-tab event contamination +- Phase 2: `TabSession` type + `Map` in `session.ts` +- Phase 2: Per-tab dialog (move `dialog` into TabSession) +- Phase 2: Per-tab `pageInfoDirty` (move flag into TabSession) +- Phase 2: Per-tab buffers (move `networkBuffer`/`consoleBuffer` into TabSession, preserve on switch) +- Phase 2: Per-tab page cache in `client.ts` (`Map`) +- Phase 2: Console `nextSeq` reset on `clear()` in `console-buffer.ts` +- Phase 2: TabSession cleanup on `Target.targetDestroyed` and reconciliation +- Phase 2: Multi-agent warning in `prompt.ts` Parallel Execution section + +### Not Building +- Agent-aware tab locking (requires pi platform agent identity exposure — Phase 3) +- Observation tool `callOnTarget` refactoring (Phase 3) +- Per-agent BrowserClient instances +- New CDP transport connections per agent + +## Decisions + +### TabSession model — one object per targetId +**Evidence**: `OwnershipRegistry` at `ownership.ts:29` uses a single `Set` with add/remove/has — one collection, not separate Maps per field. Grouping all per-tab state into one `TabSession` object per targetId follows this pattern and simplifies cleanup (one delete removes all state). +**Decision**: Single `TabSession` object per targetId stored in `Map`. + +### ev.sessionId filter — immediate Phase 1 +**Evidence**: Research Finding 9 identifies this as "the single most impactful change." Precedent-locator confirms it's the essential first move. One line in `consumeEvents` prevents dialog overwrites, dirty-flag theft, and buffer pollution. +**Decision**: Phase 1 delivers only this filter. Phase 2 builds per-tab structures on top. + +### Buffers preserved, not cleared on switchTo +**Evidence**: `d3d86f9` established the pattern of NOT clearing dialog on transport close (`session.ts:78-83`) — data survives events that would otherwise destroy it. The same rationale applies to buffers on tab switch. Research confirms buffer clearing invalidates per-agent cursors. +**Decision**: `switchTo` no longer calls `networkBuffer.clear()` or `consoleBuffer.clear()`. Each tab's buffers persist in its TabSession. + +### nextSeq reset on clear +**Evidence**: `console-buffer.ts:256-262` comment claims monotonic `nextSeq` prevents stale cursor matches — research Finding 4 proves the opposite. With per-tab buffers, each tab has independent seq space, eliminating the need for global monotonicity. +**Decision**: `clear()` resets `nextSeq` to 1. + +### drain methods default to current tab — no API break +**Evidence**: Integration scanner confirms domain tools call `session.drainNetworkBuffer()`, `session.drainConsoleBuffer()`, `client.pageInfo()` which calls `session.takeDialog()` and `session.drainPageInfoInvalidations()`. All without parameters. +**Decision**: All drain/take methods accept optional `targetId` parameter defaulting to the current tab. Existing callers unchanged. + +### pageCache stays in client.ts as Map +**Evidence**: `pageCache` at `client.ts:85` already has TTL, parse, and invalidation logic in `client.ts:168-179`. Moving it to session.ts would require exposing TTL constants and parse logic across module boundaries. Session-level dirty flag (TabSession.pageInfoDirty) is sufficient for invalidation. +**Decision**: pageCache in `client.ts` becomes `Map`. TabSession carries the dirty flag. + +## Architecture + + + +### src/cdp/session.ts:1-194 — MODIFY + +```typescript +// === Slice 1: Foundation === +// All changes are within createCdpSession(). Existing singleton vars (dialog, +// pageInfoDirty, networkBuffer, consoleBuffer) remain for Slice 1 compatibility. + +// NEW: Per-tab state type (inserted before createCdpSession, after imports) +type TabSession = { + sessionId: string; + targetId: string; + dialog: DialogInfo | null; + pageInfoDirty: boolean; + networkBuffer: ReturnType; + consoleBuffer: ReturnType; +}; + +// Inside createCdpSession(), ADD after the singleton declarations: +const tabs = new Map(); +const sessionIdToTargetId = new Map(); + +// consumeEvents — ADD filter guard as the FIRST statement inside the for-await loop: +// (inserted before the existing `if (ev.method === "Page.javascriptDialogOpening")`) +for await (const ev of transport.events()) { + // Filter: skip events from sessions we're not currently tracking. + // Target.targetDestroyed is browser-level (no sessionId) — always process. + if (ev.method !== "Target.targetDestroyed") { + if (ev.sessionId && ev.sessionId !== sessionId) continue; + } + // ... all existing event handlers unchanged ... +} + +// switchTo — REPLACE the entire method body: +async switchTo(tid) { + const activated = await transport.request("Target.activateTarget", { targetId: tid }, { sessionId: null }); + if (!activated.success) return activated; + const attached = await transport.request("Target.attachToTarget", { targetId: tid, flatten: true }, { sessionId: null }); + if (!attached.success) return attached; + const a = attached.data as { sessionId: string }; + // Reuse existing TabSession or create one on first visit + const existing = tabs.get(tid); + const tab: TabSession = existing ?? { + sessionId: a.sessionId, + targetId: tid, + dialog: null, + pageInfoDirty: true, + networkBuffer: createNetworkBuffer(), + consoleBuffer: createConsoleBuffer(), + }; + if (!existing) { + tabs.set(tid, tab); + await enableDomains(a.sessionId); + } + // Update global pointers to point at the new active tab + sessionId = tab.sessionId; + targetId = tid; + // Update sessionId→targetId reverse lookup for event filtering + sessionIdToTargetId.set(tab.sessionId, tid); + return ok(undefined); +} + +// attachFirstPage — ADD after `sessionId = a.sessionId; targetId = pickTargetId;`: +tabs.set(pickTargetId, { + sessionId: a.sessionId, + targetId: pickTargetId, + dialog: null, + pageInfoDirty: false, + networkBuffer: createNetworkBuffer(), + consoleBuffer: createConsoleBuffer(), +}); +sessionIdToTargetId.set(a.sessionId, pickTargetId); + +// transport.onClose — ADD Map cleanup before restartConsumer(): +tabs.clear(); +sessionIdToTargetId.clear(); +``` + +### src/cdp/session.ts — Slice 2 additions (dialog + dirty flag into TabSession) + +```typescript +// consumeEvents — dialog handler: write to TabSession instead of singleton +if (ev.method === "Page.javascriptDialogOpening") { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) continue; + const params = ev.params as Partial | undefined; + tab.dialog = { + type: (params?.type as DialogInfo["type"]) ?? "alert", + message: params?.message ?? "", + ...(params?.defaultPrompt !== undefined ? { defaultPrompt: params.defaultPrompt } : {}), + }; + continue; +} + +// consumeEvents — dirty flag: write to TabSession instead of singleton +if (ev.method === "Page.frameNavigated" || ev.method === "Page.loadEventFired") { + const tab = targetId ? tabs.get(targetId) : undefined; + if (tab) tab.pageInfoDirty = true; +} + +// takeDialog — read from current TabSession +takeDialog() { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return null; + const d = tab.dialog; + tab.dialog = null; + return d; +}, + +// drainPageInfoInvalidations — read from current TabSession +drainPageInfoInvalidations() { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return false; + const dirty = tab.pageInfoDirty; + tab.pageInfoDirty = false; + return dirty; +}, + +// REMOVE the singleton `dialog` and `pageInfoDirty` declarations: +// - let dialog: DialogInfo | null = null; ← REMOVED (now in TabSession) +// - let pageInfoDirty = false; ← REMOVED (now in TabSession) + +// transport.onClose — dialog preservation moves to TabSession (no change needed; +// tabs.clear() already clears all TabSession state including dialog) +``` + +### src/cdp/console-buffer.ts:256-262 — MODIFY (Slice 3) + +```typescript +// Replace the clear() method body — reset nextSeq to 1 on clear: +// OLD: +// clear() { +// records.clear(); +// overflowed = false; +// // nextSeq is intentionally NOT reset: the cursor remains monotonic across +// // tab switches so a stale sinceSeq from a previous tab can't accidentally +// // match new records. +// }, +// NEW: + clear() { + records.clear(); + nextSeq = 1; + overflowed = false; + }, +// Rationale: With per-tab buffers, each TabSession has its own console buffer +// with independent sequence space. Resetting nextSeq on clear prevents cross-tab +// data leaks — a stale cursor from tab X (seq 15) cannot match new records from +// tab Y (which restart at seq 1). The old monotonic-across-tabs behavior was a +// design inversion when buffers were shared. +``` + +### src/cdp/session.ts — Slice 3 additions (buffer routing + drain from TabSession) + +```typescript +// consumeEvents — route network/console events to correct TabSession: +// Helper (inserted before the for-await loop): +const resolveTab = (evSessionId?: string): TabSession | undefined => { + const tid = evSessionId ? sessionIdToTargetId.get(evSessionId) : targetId; + return tid ? tabs.get(tid) : undefined; +}; + +// Replace all singleton buffer ingest calls with routed calls: +// OLD: if (ev.method === "Network.requestWillBeSent") networkBuffer.ingestRequestWillBeSent(ev.params); +// NEW: +if (ev.method === "Network.requestWillBeSent") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestRequestWillBeSent(ev.params); +} +else if (ev.method === "Network.responseReceived") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestResponseReceived(ev.params); +} +else if (ev.method === "Network.loadingFinished") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestLoadingFinished(ev.params); +} +else if (ev.method === "Network.loadingFailed") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestLoadingFailed(ev.params); +} +else if (ev.method === "Runtime.consoleAPICalled") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.consoleBuffer.ingestConsoleApi(ev.params); +} +else if (ev.method === "Log.entryAdded") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.consoleBuffer.ingestLogEntry(ev.params); +} + +// drainNetworkBuffer — read from current TabSession +drainNetworkBuffer(filter) { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return { records: [], total: 0, bufferOverflowed: false }; + return tab.networkBuffer.drain(filter); +}, + +// drainConsoleBuffer — read from current TabSession +drainConsoleBuffer(filter) { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return { records: [], total: 0, bufferOverflowed: false }; + return tab.consoleBuffer.drain(filter); +}, + +// REMOVE the singleton buffer declarations: +// - const networkBuffer = createNetworkBuffer(); ← REMOVED (now in TabSession) +// - const consoleBuffer = createConsoleBuffer(); ← REMOVED (now in TabSession) +``` + +### src/client.ts:85,157-180,183 — MODIFY (Slice 2) + +```typescript +// REPLACE single pageCache with per-tab Map: +// OLD: let pageCache: { readonly info: PageInfo; readonly at: number } | null = null; +// NEW: +const pageCaches = new Map(); + +// readPageInfo — use per-tab Map instead of singleton: +const readPageInfo = async (): Promise> => { + const dirty = session.drainPageInfoInvalidations(); + const currentTid = session.current()?.targetId; + const cached = currentTid ? pageCaches.get(currentTid) : undefined; + if (cached && !dirty && Date.now() - cached.at < PAGE_INFO_TTL_MS) + return ok(cached.info); + const expr = safeJs`JSON.stringify({url:location.href,title:document.title,w:innerWidth,h:innerHeight,sx:scrollX,sy:scrollY,pw:document.documentElement.scrollWidth,ph:document.documentElement.scrollHeight})`; + const raw = await evaluateJs(expr); + if (!raw.success) return raw; + if (typeof raw.data !== "string") return err(cdpError("invalid_response", "page info evaluation did not return a string")); + let parsedRaw: unknown; + try { + parsedRaw = JSON.parse(raw.data); + } catch (e) { + return err(cdpError("invalid_response", `page info JSON.parse failed: ${e instanceof Error ? e.message : String(e)}`)); + } + const info = parsePageInfoPayload(parsedRaw); + if (!info.success) return info; + if (currentTid) pageCaches.set(currentTid, { info: info.data, at: Date.now() }); + return ok(info.data); +}; + +// switchTab — REMOVE `pageCache = null` (no longer needed — each tab retains its cache): +const switchTab = async (targetId: string): Promise> => { + const r = await session.switchTo(targetId); + if (!r.success) return r; + // pageCache = null; ← REMOVED + await session.call("Runtime.evaluate", { + expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, + }); + return ok(undefined); +}; + +// stop — REMOVE `pageCache = null`, replace with Map.clear(): +const stop = async (): Promise => { + await transport.close(); + pageCaches.clear(); + lastHealth = 0; +}; +``` + +### src/cdp/session.ts — Slice 4 additions (TabSession cleanup) + +```typescript +// consumeEvents — Target.targetDestroyed: remove TabSession entries +if (ev.method === "Target.targetDestroyed" && ownership) { + const params = ev.params as { targetId?: string } | undefined; + if (params?.targetId) { + ownership.remove(params.targetId); + // Prune per-tab state for the destroyed target + const tab = tabs.get(params.targetId); + if (tab) { + sessionIdToTargetId.delete(tab.sessionId); + tabs.delete(params.targetId); + } + } +} + +// attachFirstPage — ADD TabSession pruning after ownership reconciliation: +// (inserted after the ownership reconciliation block, after ownership.setHarnessWindow) +const liveTargetIds = new Set(allPages.map((p) => p.targetId)); +for (const tid of tabs.keys()) { + if (!liveTargetIds.has(tid)) { + const tab = tabs.get(tid); + if (tab) sessionIdToTargetId.delete(tab.sessionId); + tabs.delete(tid); + } +} + +// listTabs in client.ts — ADD page cache pruning (extend existing reconciliation): +// After: const survivors = owned.filter((id) => live.has(id)); +// Add: +for (const tid of pageCaches.keys()) { + if (!live.has(tid)) pageCaches.delete(tid); +} +``` + +### src/prompt.ts:63-73 — MODIFY (Slice 4) + +```typescript +// REPLACE the "Parallel Execution" section (lines 63-73): + +### Parallel Execution + +Observation tools (browser_screenshot, browser_page_info, browser_execute_js, browser_list_tabs, browser_http_get, etc.) can run in parallel with each other and with mutation tools. The harness automatically serializes mutation tools (click, type, scroll, navigate, switch_tab, etc.) so they never race on shared state. When operations are independent, emit them in the same turn for better performance. + +**Examples of safe parallel calls:** +\`\`\` +browser_screenshot() + browser_page_info() + browser_execute_js("document.title") +browser_http_get("https://api.example.com/data") + browser_click(x, y) +\`\`\` + +**Multi-agent note**: When multiple subagents use the browser, tab switching by one agent changes the active tab for all agents. Per-tab data (console buffers, network traces, dialogs, page info cache) is isolated — switching tabs does not destroy another agent's collected data. Call `browser_current_tab` before mutation tools to confirm you're on the expected tab. If `browser_page_info` returns a dialog, handle it with `browser_handle_dialog` promptly — dialogs block page interaction and are not queued across agents. + +## Desired End State + +```typescript +// Tab A and Tab B can coexist without cross-contamination: + +// Agent on Tab A: +browser_navigate("https://site-a.com") // navigates Tab A +browser_console({ sinceSeq: 0 }) // gets Tab A's console, nextCursor=5 +// Agent switches to Tab B: +browser_switch_tab("targetId_B") +browser_navigate("https://site-b.com") // navigates Tab B +browser_console({ sinceSeq: 0 }) // gets Tab B's console (separate buffer) +// Agent switches back to Tab A: +browser_switch_tab("targetId_A") +browser_console({ sinceSeq: 5 }) // continues from Tab A's cursor — data preserved! +browser_page_info() // returns Tab A's cached info (not stale from Tab B) + +// Tab A's dialog doesn't leak to Tab B: +// Tab A triggers alert("A") → stored in TabSession["targetId_A"].dialog +// Tab B triggers confirm("B") → stored in TabSession["targetId_B"].dialog +// browser_page_info() on Tab B returns { dialog: { type: "confirm", message: "B" } } +// No overwrite. No silent loss. +``` + +## File Map + +``` +src/cdp/session.ts # MODIFY — TabSession type, Map, ev.sessionId filter, switchTo rework, drain parameterization, cleanup +src/cdp/console-buffer.ts # MODIFY — nextSeq reset on clear() +src/client.ts # MODIFY — per-tab pageCache Map +src/prompt.ts # MODIFY — multi-agent warning +``` + +## Ordering Constraints + +- Slice 1 must come first (foundation — TabSession type, Map, ev.sessionId filter) +- Slice 2 depends on TabSession existing (dialog/dirty-flag/page-cache are TabSession fields) +- Slice 3 depends on TabSession existing (buffers are TabSession fields) +- Slice 4 depends on Slices 1-3 (cleanup reconciles against live TabSession entries) + +## Verification Notes + +From research Precedents & Lessons: +- **`d3d86f9`**: Event consumer must be serialized (already done). Surface crashes via console.warn (already done). +- **`71da8e0`**: Mutex serializes mutations globally, not per-tab. Per-tab data prevents corruption from this design limitation. +- **`20ceaa1`**: Ownership reconciliation pattern must be replicated for TabSession pruning. +- **`a1df2d3`**: Prefix matching hazard grows with owned tab count. TabSession Map uses full targetId keys — no prefix collision risk. + +Verification checks: +- `tsc --noEmit` passes on modified files +- Two tabs: switch A→B→A, console cursor from A's first visit preserves state +- Dialog on tab A is NOT returned by takeDialog() after switching to tab B +- pageInfoDirty from tab B's navigation does NOT invalidate tab A's page cache +- Target.targetDestroyed event removes TabSession from Map +- attachFirstPage reconciliation prunes dead TabSession entries + +## Performance Considerations + +- `Map.get(targetId)` — O(1), negligible (single-digit entries typical) +- Each TabSession's buffers bounded at 500 records — no unbounded growth +- TabSession creation is lazy (on first visit to a tab via switchTo or attachFirstPage) +- No additional CDP round-trips — event routing is purely in-memory + +## Migration Notes + +Not applicable — no persisted schema changes. TabSession state is purely in-memory and reconstructed on each session start. + +## Pattern References + +- `src/cdp/ownership.ts:38-72` — OwnershipRegistry factory: add/remove/has/notify pattern +- `src/cdp/session.ts:41-46` — Current singleton state to extract into TabSession +- `src/cdp/session.ts:112-118` — attachFirstPage ownership reconciliation (replicate for TabSession) +- `src/client.ts:85,168-179` — pageCache TTL pattern (replicate as Map) +- `src/cdp/network-buffer.ts:77-85` — Buffer factory + Map internal pattern + +## Developer Context + +**Q (`src/cdp/session.ts:41-46`, `src/client.ts:85`): Per-tab isolation scope chosen. Phase 1: ev.sessionId filter. Phase 2: per-tab data structures. Phase 3: agent-aware locking deferred.** +A: Approved — proceed with per-tab isolation design. + +**Q (Decomposition): 4 slices: Foundation, Dialog+Dirty+Cache, Buffers+nextSeq, Cleanup+Prompt.** +A: Approved — proceed with slice-by-slice generation. + +## Design History + +- Slice 1: Foundation — approved as generated +- Slice 2: Dialog + dirty flag + page cache — approved as generated +- Slice 3: Buffer isolation + nextSeq fix — approved as generated +- Slice 4: Cleanup + reconciliation + prompt — approved as generated + +## References + +- Research: `thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md` +- Questions: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` +- Rewrite design: `docs/superpowers/specs/2026-05-02-harness-rewrite-design.md` diff --git a/.rpiv/artifacts/plans/2026-05-07_18-15-35_per-tab-isolation.md b/.rpiv/artifacts/plans/2026-05-07_18-15-35_per-tab-isolation.md new file mode 100644 index 0000000..acddf60 --- /dev/null +++ b/.rpiv/artifacts/plans/2026-05-07_18-15-35_per-tab-isolation.md @@ -0,0 +1,452 @@ +--- +date: 2026-05-07T18:15:35Z +author: amankumarsingh77 +commit: 4052e6e +branch: main +repository: pi-browser-harness +topic: "per-tab-isolation-multi-agent-safety" +tags: [plan, session, isolation, per-tab, buffers, dialog, safety, concurrency] +status: ready +parent: "thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md" +last_updated: 2026-05-07T18:15:35Z +last_updated_by: amankumarsingh77 +--- + +# Per-Tab Isolation for Multi-Agent Safety — Implementation Plan + +## Overview + +Implement per-tab data isolation in `pi-browser-harness` so that multiple subagents can use the browser without corrupting each other's state. Replace singleton dialog, page-info-dirty flag, network buffer, and console buffer with per-tab `TabSession` objects stored in a `Map`. Add an `ev.sessionId` filter in the CDP event consumer to prevent cross-tab event contamination. Deliverable in 3 sequential phases totaling 4 file modifications. + +Reference design: `thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md` + +## Desired End State + +Tab A and Tab B can coexist without cross-contamination: + +- Switching tabs preserves each tab's network/console buffer data (no `clear()` on switch) +- Dialog from tab A is not overwritten by dialog from tab B +- Page info dirty flag is per-tab, not global +- Console `sinceSeq` cursor remains valid across tab switches (no cross-tab data leaks) +- Page info cache is preserved per-tab across switches +- CDP events from unattached sessions are filtered (no cross-contamination) +- Dead tab state is pruned on target destruction and session reconciliation + +## What We're NOT Doing + +- Agent-aware tab locking (requires pi platform agent identity exposure — Phase 3, deferred) +- Observation tool `callOnTarget` refactoring (Phase 3, deferred) +- Per-agent BrowserClient instances +- New CDP transport connections per agent + +## Phase 1: Foundation — TabSession type, Maps, ev.sessionId filter, switchTo rework + +### Overview +Introduces the `TabSession` type, the `Map` collection, the `sessionIdToTargetId` reverse lookup, the 1-line `ev.sessionId` guard in `consumeEvents`, and the `switchTo` rework that creates/reuses `TabSession` without clearing buffers. This phase alone prevents cross-tab event contamination and is independently shippable. + +### Changes Required: + +#### 1. TabSession type + infrastructure (`src/cdp/session.ts`) +**File**: `src/cdp/session.ts` +**Changes**: Insert `TabSession` type definition before `createCdpSession`. Inside `createCdpSession`, add `tabs` Map, `sessionIdToTargetId` Map. Add `ev.sessionId` filter in `consumeEvents`. Rework `switchTo` to create/reuse `TabSession`. Update `attachFirstPage` to create initial `TabSession`. Add Map cleanup in `transport.onClose`. + +```typescript +// === Phase 1: Foundation === + +// NEW: Per-tab state type (inserted before createCdpSession, after imports) +type TabSession = { + sessionId: string; + targetId: string; + dialog: DialogInfo | null; + pageInfoDirty: boolean; + networkBuffer: ReturnType; + consoleBuffer: ReturnType; +}; + +// Inside createCdpSession(), ADD after the singleton declarations: +const tabs = new Map(); +const sessionIdToTargetId = new Map(); + +// consumeEvents — ADD filter guard as the FIRST statement inside the for-await loop +// (inserted before the existing `if (ev.method === "Page.javascriptDialogOpening")`) +for await (const ev of transport.events()) { + // Filter: skip events from sessions we're not currently tracking. + // Target.targetDestroyed is browser-level (no sessionId) — always process. + if (ev.method !== "Target.targetDestroyed") { + if (ev.sessionId && ev.sessionId !== sessionId) continue; + } + // ... all existing event handlers unchanged ... +} + +// switchTo — REPLACE the entire method body: +async switchTo(tid) { + const activated = await transport.request("Target.activateTarget", { targetId: tid }, { sessionId: null }); + if (!activated.success) return activated; + const attached = await transport.request("Target.attachToTarget", { targetId: tid, flatten: true }, { sessionId: null }); + if (!attached.success) return attached; + const a = attached.data as { sessionId: string }; + // Reuse existing TabSession or create one on first visit + const existing = tabs.get(tid); + const tab: TabSession = existing ?? { + sessionId: a.sessionId, + targetId: tid, + dialog: null, + pageInfoDirty: true, + networkBuffer: createNetworkBuffer(), + consoleBuffer: createConsoleBuffer(), + }; + if (!existing) { + tabs.set(tid, tab); + await enableDomains(a.sessionId); + } + // Update global pointers to point at the new active tab + sessionId = tab.sessionId; + targetId = tid; + // Update sessionId→targetId reverse lookup for event filtering + sessionIdToTargetId.set(tab.sessionId, tid); + return ok(undefined); +} + +// attachFirstPage — ADD after `sessionId = a.sessionId; targetId = pickTargetId;`: +tabs.set(pickTargetId, { + sessionId: a.sessionId, + targetId: pickTargetId, + dialog: null, + pageInfoDirty: false, + networkBuffer: createNetworkBuffer(), + consoleBuffer: createConsoleBuffer(), +}); +sessionIdToTargetId.set(a.sessionId, pickTargetId); + +// transport.onClose — ADD Map cleanup before restartConsumer(): +tabs.clear(); +sessionIdToTargetId.clear(); +``` + +### Success Criteria: + +#### Automated Verification: +- [x] Type checking passes: `npm run typecheck` +- [x] `grep -c "consumeEvents" src/cdp/session.ts` confirms single consumer +- [x] `grep "ev.sessionId && ev.sessionId !== sessionId" src/cdp/session.ts` confirms filter guard exists + +#### Manual Verification: +- [x] Run `scripts/test-parallel.ts` — parallel mutation tests still pass (mutex unchanged) +- [ ] Open two tabs via `browser_new_tab`, switch between them — no crash, session alive +- [ ] CDP events from non-current tabs are silently dropped (verify: navigate tab A, switch to tab B, network events from tab A don't appear in `browser_network_requests`) + +--- + +## Phase 2: State Migration — Singletons into TabSession + per-tab page cache + nextSeq fix + +### Overview +Moves `dialog`, `pageInfoDirty`, `networkBuffer`, and `consoleBuffer` from singleton variables into `TabSession`. `consumeEvents` routes events to the correct tab's buffer. `takeDialog()`, `drainPageInfoInvalidations()`, `drainNetworkBuffer()`, `drainConsoleBuffer()` all read from the current `TabSession`. `client.ts` `pageCache` becomes `Map`. `console-buffer.ts` `clear()` resets `nextSeq` to 1. + +### Changes Required: + +#### 1. Dialog + dirty flag into TabSession (`src/cdp/session.ts`) +**File**: `src/cdp/session.ts` +**Changes**: Route dialog handler and dirty-flag handler through `TabSession`. Replace `takeDialog()` and `drainPageInfoInvalidations()` to read from current tab. Remove singleton `dialog` and `pageInfoDirty` declarations. + +```typescript +// === Phase 2a: Dialog + dirty flag === + +// consumeEvents — dialog handler: write to TabSession instead of singleton +if (ev.method === "Page.javascriptDialogOpening") { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) continue; + const params = ev.params as Partial | undefined; + tab.dialog = { + type: (params?.type as DialogInfo["type"]) ?? "alert", + message: params?.message ?? "", + ...(params?.defaultPrompt !== undefined ? { defaultPrompt: params.defaultPrompt } : {}), + }; + continue; +} + +// consumeEvents — dirty flag: write to TabSession instead of singleton +if (ev.method === "Page.frameNavigated" || ev.method === "Page.loadEventFired") { + const tab = targetId ? tabs.get(targetId) : undefined; + if (tab) tab.pageInfoDirty = true; +} + +// takeDialog — read from current TabSession +takeDialog() { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return null; + const d = tab.dialog; + tab.dialog = null; + return d; +}, + +// drainPageInfoInvalidations — read from current TabSession +drainPageInfoInvalidations() { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return false; + const dirty = tab.pageInfoDirty; + tab.pageInfoDirty = false; + return dirty; +}, + +// REMOVE the singleton `dialog` and `pageInfoDirty` declarations: +// - let dialog: DialogInfo | null = null; ← REMOVED (now in TabSession) +// - let pageInfoDirty = false; ← REMOVED (now in TabSession) +``` + +#### 2. Buffer routing + drain from TabSession (`src/cdp/session.ts`) +**File**: `src/cdp/session.ts` +**Changes**: Add `resolveTab` helper. Route all network/console event handlers through `resolveTab`. Replace `drainNetworkBuffer()` and `drainConsoleBuffer()` to read from current `TabSession`. Remove singleton `networkBuffer` and `consoleBuffer` declarations. + +```typescript +// === Phase 2b: Buffer routing + drain === + +// consumeEvents — route network/console events to correct TabSession: +// Helper (inserted before the for-await loop): +const resolveTab = (evSessionId?: string): TabSession | undefined => { + const tid = evSessionId ? sessionIdToTargetId.get(evSessionId) : targetId; + return tid ? tabs.get(tid) : undefined; +}; + +// Replace all singleton buffer ingest calls with routed calls: +if (ev.method === "Network.requestWillBeSent") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestRequestWillBeSent(ev.params); +} +else if (ev.method === "Network.responseReceived") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestResponseReceived(ev.params); +} +else if (ev.method === "Network.loadingFinished") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestLoadingFinished(ev.params); +} +else if (ev.method === "Network.loadingFailed") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.networkBuffer.ingestLoadingFailed(ev.params); +} +else if (ev.method === "Runtime.consoleAPICalled") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.consoleBuffer.ingestConsoleApi(ev.params); +} +else if (ev.method === "Log.entryAdded") { + const tab = resolveTab(ev.sessionId); + if (tab) tab.consoleBuffer.ingestLogEntry(ev.params); +} + +// drainNetworkBuffer — read from current TabSession +drainNetworkBuffer(filter) { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return { records: [], total: 0, bufferOverflowed: false }; + return tab.networkBuffer.drain(filter); +}, + +// drainConsoleBuffer — read from current TabSession +drainConsoleBuffer(filter) { + const tab = targetId ? tabs.get(targetId) : undefined; + if (!tab) return { records: [], total: 0, bufferOverflowed: false }; + return tab.consoleBuffer.drain(filter); +}, + +// REMOVE the singleton buffer declarations: +// - const networkBuffer = createNetworkBuffer(); ← REMOVED (now in TabSession) +// - const consoleBuffer = createConsoleBuffer(); ← REMOVED (now in TabSession) +``` + +#### 3. nextSeq reset on clear (`src/cdp/console-buffer.ts`) +**File**: `src/cdp/console-buffer.ts` +**Changes**: Replace the `clear()` method body to reset `nextSeq` to 1. + +```typescript +// Replace the clear() method body — reset nextSeq to 1 on clear: + clear() { + records.clear(); + nextSeq = 1; + overflowed = false; + }, +``` + +#### 4. Per-tab page cache (`src/client.ts`) +**File**: `src/client.ts` +**Changes**: Replace single `pageCache` with `Map`. Update `readPageInfo`, `switchTab`, and `stop`. + +```typescript +// REPLACE single pageCache with per-tab Map: +const pageCaches = new Map(); + +// readPageInfo — use per-tab Map instead of singleton: +const readPageInfo = async (): Promise> => { + const dirty = session.drainPageInfoInvalidations(); + const currentTid = session.current()?.targetId; + const cached = currentTid ? pageCaches.get(currentTid) : undefined; + if (cached && !dirty && Date.now() - cached.at < PAGE_INFO_TTL_MS) + return ok(cached.info); + const expr = safeJs`JSON.stringify({url:location.href,title:document.title,w:innerWidth,h:innerHeight,sx:scrollX,sy:scrollY,pw:document.documentElement.scrollWidth,ph:document.documentElement.scrollHeight})`; + const raw = await evaluateJs(expr); + if (!raw.success) return raw; + if (typeof raw.data !== "string") return err(cdpError("invalid_response", "page info evaluation did not return a string")); + let parsedRaw: unknown; + try { + parsedRaw = JSON.parse(raw.data); + } catch (e) { + return err(cdpError("invalid_response", `page info JSON.parse failed: ${e instanceof Error ? e.message : String(e)}`)); + } + const info = parsePageInfoPayload(parsedRaw); + if (!info.success) return info; + if (currentTid) pageCaches.set(currentTid, { info: info.data, at: Date.now() }); + return ok(info.data); +}; + +// switchTab — REMOVE `pageCache = null` (no longer needed — each tab retains its cache): +const switchTab = async (targetId: string): Promise> => { + const r = await session.switchTo(targetId); + if (!r.success) return r; + // pageCache = null; ← REMOVED + await session.call("Runtime.evaluate", { + expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, + }); + return ok(undefined); +}; + +// stop — REMOVE `pageCache = null`, replace with Map.clear(): +const stop = async (): Promise => { + await transport.close(); + pageCaches.clear(); + lastHealth = 0; +}; +``` + +### Success Criteria: + +#### Automated Verification: +- [x] Type checking passes: `npm run typecheck` +- [x] `grep "let dialog" src/cdp/session.ts` returns no matches (singleton removed) +- [x] `grep "let pageInfoDirty" src/cdp/session.ts` returns no matches +- [x] `grep "const networkBuffer" src/cdp/session.ts` returns no matches +- [x] `grep "const consoleBuffer" src/cdp/session.ts` returns no matches +- [x] `grep "nextSeq = 1" src/cdp/console-buffer.ts` confirms the change +- [x] `grep "pageCaches" src/client.ts` confirms Map usage + +#### Manual Verification: +- [ ] Run `scripts/test-parallel.ts` — all tests pass +- [ ] Run `scripts/test-parallel-integration.ts` — all tests pass +- [ ] Open tab A, trigger `browser_console`, note `nextCursor`. Switch to tab B, trigger `browser_console`. Switch back to tab A, call `browser_console({ sinceSeq: })` — returns only new tab A messages (not tab B's) +- [ ] Navigate tab A to a page with an alert. Switch to tab B BEFORE calling `browser_page_info`. Call `browser_page_info` on tab B — returns tab B's page info (NOT tab A's dialog) +- [ ] Tab A navigates (triggering pageInfoDirty). Switch to tab B, call `browser_page_info` twice within 1 second — second call uses tab B's cache (not invalidated by tab A's navigation) + +--- + +## Phase 3: Cleanup + Reconciliation + Prompt + +### Overview +`Target.targetDestroyed` handler removes `TabSession` entries. `attachFirstPage` and `listTabs` prune dead `TabSession` and `pageCaches` entries during reconciliation. `prompt.ts` "Parallel Execution" section gains a multi-agent warning. + +### Changes Required: + +#### 1. TabSession cleanup on target destruction + reconciliation (`src/cdp/session.ts`) +**File**: `src/cdp/session.ts` +**Changes**: Extend `Target.targetDestroyed` handler to remove TabSession entries. Add TabSession pruning loop in `attachFirstPage` reconciliation. + +```typescript +// === Phase 3a: TabSession cleanup === + +// consumeEvents — Target.targetDestroyed: remove TabSession entries +if (ev.method === "Target.targetDestroyed" && ownership) { + const params = ev.params as { targetId?: string } | undefined; + if (params?.targetId) { + ownership.remove(params.targetId); + // Prune per-tab state for the destroyed target + const tab = tabs.get(params.targetId); + if (tab) { + sessionIdToTargetId.delete(tab.sessionId); + tabs.delete(params.targetId); + } + } +} + +// attachFirstPage — ADD TabSession pruning after ownership reconciliation: +const liveTargetIds = new Set(allPages.map((p) => p.targetId)); +for (const tid of tabs.keys()) { + if (!liveTargetIds.has(tid)) { + const tab = tabs.get(tid); + if (tab) sessionIdToTargetId.delete(tab.sessionId); + tabs.delete(tid); + } +} +``` + +#### 2. Page cache pruning in listTabs (`src/client.ts`) +**File**: `src/client.ts` +**Changes**: Extend existing `listTabs` reconciliation to prune dead `pageCaches` entries. + +```typescript +// listTabs — ADD page cache pruning after existing ownership reconciliation: +// After: const survivors = owned.filter((id) => live.has(id)); +for (const tid of pageCaches.keys()) { + if (!live.has(tid)) pageCaches.delete(tid); +} +``` + +#### 3. Multi-agent warning in system prompt (`src/prompt.ts`) +**File**: `src/prompt.ts` +**Changes**: Replace the "Parallel Execution" section (lines 63-73) with a version that adds the multi-agent note. + +```typescript +// REPLACE the "Parallel Execution" section (lines 63-73): + +### Parallel Execution + +Observation tools (browser_screenshot, browser_page_info, browser_execute_js, browser_list_tabs, browser_http_get, etc.) can run in parallel with each other and with mutation tools. The harness automatically serializes mutation tools (click, type, scroll, navigate, switch_tab, etc.) so they never race on shared state. When operations are independent, emit them in the same turn for better performance. + +**Examples of safe parallel calls:** +\`\`\` +browser_screenshot() + browser_page_info() + browser_execute_js("document.title") +browser_http_get("https://api.example.com/data") + browser_click(x, y) +\`\`\` + +**Multi-agent note**: When multiple subagents use the browser, tab switching by one agent changes the active tab for all agents. Per-tab data (console buffers, network traces, dialogs, page info cache) is isolated — switching tabs does not destroy another agent's collected data. Call `browser_current_tab` before mutation tools to confirm you're on the expected tab. If `browser_page_info` returns a dialog, handle it with `browser_handle_dialog` promptly — dialogs block page interaction and are not queued across agents. +``` + +### Success Criteria: + +#### Automated Verification: +- [x] Type checking passes: `npm run typecheck` +- [x] `grep "tabs.delete" src/cdp/session.ts` confirms cleanup logic +- [x] `grep "Multi-agent note" src/prompt.ts` confirms prompt update + +#### Manual Verification: +- [ ] Close a tab manually in the browser — `browser_list_tabs` no longer shows it; its TabSession entry is cleaned up +- [ ] Restart the session — `attachFirstPage` reconciliation prunes dead TabSession entries from prior session +- [ ] Spawn two subagents. Subagent A's `browser_page_info` returns its own tab's data after Subagent B switches tabs. Both agents see the multi-agent warning in their system prompts. + +--- + +## Testing Strategy + +### Automated: +- `npm run typecheck` — all phases must pass type checking +- `scripts/test-parallel.ts` — mutex serialization + observation parallelization tests +- `scripts/test-parallel-integration.ts` — serialized mutation + observation + AbortSignal tests + +### Manual Testing Steps: +1. **Tab switch buffer preservation**: Open tab A → navigate → `browser_console` → note `nextCursor` → switch to tab B → `browser_console` → switch back to A → `browser_console({ sinceSeq: cursor })` → ONLY tab A's new messages (no tab B contamination) +2. **Dialog isolation**: Tab A triggers alert → switch to tab B before calling `browser_page_info` → `browser_page_info` on tab B returns tab B's state (NOT tab A's dialog) → switch back to tab A → `browser_page_info` returns tab A's dialog +3. **Page cache isolation**: Tab A `browser_page_info` → tab B navigates (triggers dirty) → tab A `browser_page_info` again — must return stale cached tab A data (not re-fetched tab B data) IF tab A is still current. If tab B switched, tab A data comes from cache on switch-back. +4. **Target destruction**: Close a tab → its TabSession is removed → `browser_list_tabs` no longer shows it +5. **Session restart**: Kill and restart → dead tabs from prior session are pruned in reconciliation + +## Performance Considerations + +- `Map.get(targetId)` — O(1), negligible (single-digit entries typical) +- Each TabSession's buffers bounded at 500 records — no unbounded growth +- TabSession creation is lazy (on first visit to a tab via switchTo or attachFirstPage) +- No additional CDP round-trips — event routing is purely in-memory + +## Migration Notes + +Not applicable — no persisted schema changes. TabSession state is purely in-memory and reconstructed on each session start. + +## References + +- Design: `thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md` +- Research: `thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md` +- Questions source: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` diff --git a/.rpiv/artifacts/questions/2026-05-07_00-26_multi-agent-safety.md b/.rpiv/artifacts/questions/2026-05-07_00-26_multi-agent-safety.md new file mode 100644 index 0000000..a14ae12 --- /dev/null +++ b/.rpiv/artifacts/questions/2026-05-07_00-26_multi-agent-safety.md @@ -0,0 +1,40 @@ +--- +date: 2026-05-07T00:26:00Z +researcher: amankumarsingh77 +git_commit: bd19739 +branch: main +repository: pi-browser-harness +topic: "multi-agent safety in pi-browser-harness: how to make browser tools safe when multiple subagents use them concurrently, preventing shared state corruption, tab switching races, dialog buffer conflicts, and ownership confusion" +tags: [discover, mutex, ownership, dialog, tabs, session, concurrency, safety] +status: complete +last_updated: 2026-05-07 +last_updated_by: amankumarsingh77 +--- + +# Research Questions: Multi-Agent Safety in pi-browser-harness + +## Discovery Summary + +The harness uses three primary safety mechanisms: a lightweight async mutex (`src/util/mutex.ts`, `createMutex()`) that serializes all mutation tools marked `serialized: true` in `src/registry.ts`; an `OwnershipRegistry` (`src/cdp/ownership.ts`) backed by a Set that tracks which tab `targetId`s belong to this session and gates `switchTab`, `closeTab`, and `newTab` in `src/domains/tabs.ts`; and a single-slot dialog buffer in `src/cdp/session.ts` (`createCdpSession`) read via `takeDialog()`. Key files read for depth: `client.ts` (mutationMutex, switchTab, ownership wiring), `session.ts` (dialog buffering, tab switch buffer clearing, pageInfoDirty flag), `tool.ts` (serialized lane acquisition with AbortSignal check), `ownership.ts` (Set-backed registry with change listener), `state.ts` (persistence across sessions), `tabs.ts` (ownership-denied gating), `dialog.ts` (single Page.handleJavaScriptDialog call), `prompt.ts` (concurrency guidance injected into the system prompt), and `index.ts` (lifecycle wiring). The architecture is single-agent by design: one mutex, one sessionId/targetId pair, one dialog slot, one pageCache, and ownership tracked per-session not per-agent — none of these primitives are namespaced or partitioned for concurrent sub-agents. + +## Questions + +1. Trace the full mutation-tool serialization path from invocation to completion: the `registerBrowserTool` function in `src/util/tool.ts` wraps each tool's handler in an `execute` closure that first calls `client.ensureAlive()` (unless `ensureAlive` is false), then checks `def.serialized` and calls `client.mutationMutex().acquire()` — an async FIFO mutex defined by `createMutex` in `src/util/mutex.ts` that chains promises. After acquiring, it checks `signal?.aborted`, calls `def.handler(args, ctx)`, and releases in a `finally` block. Map every tool marked `serialized: true` in `src/registry.ts` (22 of 32 tools). Explain why `waitTool` is NOT serialized while `waitForLoadTool` IS, and whether the mutex provides sufficient isolation when two sub-agents each want to operate on different tabs independently. + +2. Follow the tab ownership lifecycle end-to-end: when `createBrowserClient` in `src/client.ts` creates an `OwnershipRegistry` via `createOwnershipRegistry` in `src/cdp/ownership.ts`, it optionally seeds `ownedTargetIds` from persisted `BrowserState` (restored in `index.ts` `session_start` from `restoreState` in `src/state.ts`). During `attachFirstPage` in `src/cdp/session.ts`, the session reconciles persisted ownership against live targets (dropping dead IDs), prefers re-attaching to an owned tab, or creates a fresh one via `Target.createTarget { newWindow: true }` and registers it with `ownership.add()`. On `Target.targetDestroyed` events, ownership is removed. Trace how `switchTabTool` and `closeTabTool` in `src/domains/tabs.ts` enforce ownership via `client.owns(resolved)` — including hex-prefix resolution logic — and what happens when a sub-agent tries to switch to a tab owned by the session but currently in use by another sub-agent. + +3. Analyze the dialog buffer's single-slot design and its multi-agent failure modes: in `createCdpSession` ( `src/cdp/session.ts` ), the `consumeEvents` async iterator catches `Page.javascriptDialogOpening` and writes to a single `dialog: DialogInfo | null` variable. Critically, `Page.javascriptDialogClosed` is intentionally NOT handled — dialogs stay in the buffer until `takeDialog()` is called. However, `takeDialog()` reads `dialog` then sets it to `null` atomically via assignment, and when the transport closes the dialog is preserved (not cleared in `onClose`). Trace how `client.pageInfo()` in `src/client.ts` calls `session.takeDialog()` first (returning `{ dialog: d }` if present), and how the system prompt in `src/prompt.ts` guides agents to call `browser_handle_dialog`. Identify the race condition where sub-agent A opens a page triggering a dialog, sub-agent B triggers a different dialog on a different tab before A consumes it, and only B's dialog survives. + +4. Map the shared-session architecture and its impact on concurrent sub-agents: `createCdpSession` in `src/cdp/session.ts` maintains a single `sessionId: string | null` and `targetId: string | null` pair. The `switchTo(tid)` method calls `Target.activateTarget`, `Target.attachToTarget`, updates both `sessionId` and `targetId`, sets `pageInfoDirty = true`, and clears both `networkBuffer` and `consoleBuffer`. Trace how `client.switchTab()` in `src/client.ts` additionally nullifies `pageCache` and best-effort marks the tab title. Explain why if sub-agent A is mid-workflow on tab X (expecting `browser_execute_js` results from that tab's DOM) and sub-agent B calls `browser_switch_tab` to tab Y, agent A's subsequent `browser_execute_js` silently targets the wrong page — the `sessionId`/`targetId` pair is global, with no per-agent override. + +5. Trace the page-info caching and invalidation pipeline: `client.readPageInfo()` in `src/client.ts` checks `session.drainPageInfoInvalidations()` (the `pageInfoDirty` boolean in `session.ts`, set on `Page.frameNavigated` and `Page.loadEventFired`), consults a `pageCache` with 1-second TTL, evaluates a JS expression to collect URL/title/viewport/scroll/page-dimensions, parses it through `parsePageInfoPayload()`, and caches the result. Investigate whether a sub-agent switching tabs invalidates another sub-agent's cached page info, and whether the 1-second TTL (`PAGE_INFO_TTL_MS = 1_000`) combined with the shared `pageCache` could cause a sub-agent to receive stale page info from a tab it didn't navigate to. + +6. Examine the state persistence and resumption mechanism across session boundaries: `state.ts` defines `BrowserState` (namespace, remoteBrowserId, ownedTargetIds, harnessWindowTargetId), `persistState()` calls `pi.appendEntry`, and `restoreState()` scans branch entries for the last `browser-harness-state` custom entry. In `index.ts`, `session_start` restores state and wires `onOwnershipChange` to persist on every mutation. Map the full flow: `OwnershipRegistry.onChange` → `onOwnershipChange` callback in `client.ts` → `persistState` → `pi.appendEntry`. Assess whether this persistence model could cause cross-agent confusion if two sub-agents with different browser namespaces share the same pi branch session entries. + +7. Follow the console and network buffer lifecycle across tab switches: in `session.ts`, `createConsoleBuffer()` and `createNetworkBuffer()` (imported from `src/cdp/console-buffer.ts` and `src/cdp/network-buffer.ts`) are cleared wholesale in `switchTo()` — `networkBuffer.clear()` and `consoleBuffer.clear()` — because "the network history of the previous tab is no longer relevant." The `browser_network_requests` domain in `src/domains/network.ts` drains the buffer with filters, and `browser_console` in `src/domains/console.ts` uses a `sinceSeq` cursor for incremental reads. Trace what happens when sub-agent A is iterating console output incrementally (using `sinceSeq` from a prior `nextCursor`) and sub-agent B switches tabs, clearing the buffer and invalidating A's cursor. + +8. Investigate whether the namespace system provides any multi-agent isolation boundary: `index.ts` generates or accepts a `--browser-namespace` flag, stored in `BrowserState.namespace` and passed to `createBrowserClient`. The ownership-denied error message in `src/domains/tabs.ts` (`ownershipDeniedMessage`) references the namespace string, and `client.namespace` is exposed on the `BrowserClient` type. Determine whether two sub-agents running with different `--browser-namespace` values would each get their own `BrowserClient`, mutex, ownership registry, and session — or whether the extension's singleton `client` variable in `index.ts` means all agents share one client regardless of namespace. If the latter, assess what would need to change to support namespace-per-agent client instances. + +9. Analyze the `before_agent_start` hook in `index.ts` and the system prompt injection in `src/prompt.ts` for multi-agent guidance: the hook appends `getBrowserSystemPrompt()` to the system prompt, which includes a "Parallel Execution" section stating that observation tools run in parallel while mutation tools are serialized. Examine whether this prompt guidance is sufficient for a sub-agent to understand that another sub-agent might be concurrently mutating shared state — or whether the prompt assumes single-agent usage and omits crucial warnings about tab switching side effects, dialog slot overwrites, and buffer clearing. + +10. Model the full set of shared mutable state across the entire client and session layer: enumerate every piece of mutable state — `sessionId`, `targetId`, `dialog`, `pageInfoDirty`, `pageCache`, `lastHealth`, `mutationMutex` queue, `networkBuffer` entries, `consoleBuffer` entries, `ownership` Set, `harnessWindow`, `transport` connection — and classify each as either "safe to share across agents" or "requires per-agent partitioning." For each item requiring partitioning, propose the minimal abstraction change (e.g., per-agent dialog queue, per-agent sessionId/targetId stack with restore-on-switch, namespaced buffers with merge semantics). diff --git a/.rpiv/artifacts/research/2026-05-07_00-41_multi-agent-safety.md b/.rpiv/artifacts/research/2026-05-07_00-41_multi-agent-safety.md new file mode 100644 index 0000000..61d414c --- /dev/null +++ b/.rpiv/artifacts/research/2026-05-07_00-41_multi-agent-safety.md @@ -0,0 +1,223 @@ +--- +date: 2026-05-07T00:41:00Z +researcher: amankumarsingh77 +git_commit: bd19739 +branch: main +repository: pi-browser-harness +topic: "Multi-agent safety: making browser tools safe for concurrent subagent usage" +tags: [research, mutex, ownership, dialog, tabs, session, concurrency, safety, subagents, isolation] +status: complete +questions_source: "thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md" +last_updated: 2026-05-07 +last_updated_by: amankumarsingh77 +--- + +# Research: Multi-Agent Safety in pi-browser-harness + +## Research Question + +How can the browser harness be made safe for concurrent subagent usage? When multiple subagents are spun up, each using browser tools, they must not corrupt each other's state — tab switching, dialog handling, buffer reads, and page interactions must remain isolated. + +## Summary + +The harness was designed for single-agent use. All 6 safety-critical subsystems — the mutex, session, dialog buffer, page cache, network/console buffers, and ownership registry — are shared singletons with zero per-agent or per-tab partitioning. One agent switching tabs overwrites the global `sessionId`/`targetId`, clears all buffers, nullifies the page cache, and can silently consume or overwrite another agent's dialog. The approach: **phased per-tab isolation** — first an immediate `sessionId` filter on the CDP event consumer to stop cross-tab contamination, then per-tab data structures (dialog queues, page caches, buffer maps, dirty flags), and finally agent-aware tab locking to prevent one agent from switching away from another agent's active tab. + +## Detailed Findings + +### Finding 1: Mutation Mutex Serializes Globally, Not Per-Tab + +The async FIFO mutex (`src/util/mutex.ts:12-23`) serializes the 22 mutation tools marked `serialized: true` in `src/registry.ts:25-49`. Observation tools (11 total, including `browser_execute_js`, `browser_screenshot`, `browser_page_info`, `browser_snapshot`) skip the mutex entirely. + +**The mutex prevents concurrent mutation execution but does NOT provide per-agent isolation.** When Agent B calls `browser_switch_tab` (serialized, `src/domains/tabs.ts:93-128`), the global `sessionId`/`targetId` at `src/cdp/session.ts:41-42` is overwritten. Agent A's subsequent `browser_execute_js` (non-serialized observation, `src/domains/js.ts:55`) silently evaluates on the wrong tab's DOM — the mutex didn't block it because observation tools bypass the mutex. The `browser_execute_js` result contains no indication of which tab it ran on. + +**The `callOnTarget` escape hatch exists but is unused** (`src/cdp/session.ts:150-151`). It accepts an explicit `sessionId`, which would allow per-agent session binding without switching the global session. No observation tool uses it — all call `session.call()` which binds to the global `sessionId` at `src/cdp/session.ts:148`. + +### Finding 2: Shared SessionId/TargetId Is the #1 Corruption Vector + +`src/cdp/session.ts:41-42` holds the single `sessionId`/`targetId` pair. The `switchTo()` method at `src/cdp/session.ts:125-143` performs a destructive overwrite: +- Activates the new target (line 128) +- Attaches a new CDP session (line 130) +- Overwrites `sessionId` and `targetId` (lines 133-134) +- Clears `networkBuffer` and `consoleBuffer` (lines 136-137) +- Sets `pageInfoDirty = true` (line 135) + +`client.switchTab()` at `src/client.ts:179-190` compounds this by nullifying `pageCache` (line 183). There is no snapshot-and-restore mechanism, no per-agent tab stack, and no notification that the tab context changed. + +**Concrete failure scenario**: Agent A is extracting data from tab X with `browser_execute_js`. Agent B calls `browser_switch_tab` to tab Y (serialized, so it waits its turn). After B's switch completes, A's next `browser_execute_js` runs on tab Y's DOM. If tab Y lacks the element A was querying, the result is `undefined` — a silent, undetectable corruption. + +### Finding 3: Dialog Buffer Is a Single Slot With Silent Overwrite + +`src/cdp/session.ts:43` stores one `dialog: DialogInfo | null`. Every `Page.javascriptDialogOpening` event from ANY attached session overwrites it at `src/cdp/session.ts:56-63` — there is no `ev.sessionId` filter, no queue, and no per-tab storage. + +**Concrete failure scenario**: Agent A triggers `alert("Step 1")` on tab X. Before A consumes it via `browser_page_info`, Agent B triggers `confirm("Delete?")` on tab Y. The `dialog` variable is overwritten. When A calls `browser_page_info` (`src/client.ts:177-180`), `session.takeDialog()` returns B's confirm dialog. A dismisses it via `browser_handle_dialog` (`src/domains/dialog.ts:20`), which sends `Page.handleJavaScriptDialog` on the current session (now tab Y). Tab X's alert is never handled — its renderer remains blocked. + +Additionally, `Page.javascriptDialogClosed` events are intentionally ignored at `src/cdp/session.ts:64-67`, meaning stale dialogs can persist indefinitely until `takeDialog()` is called. + +### Finding 4: Network/Console Buffers Cleared on Tab Switch + +Both buffers (`session.ts:46-47`) are cleared in `switchTo()` at `src/cdp/session.ts:136-137`. The console buffer's `nextSeq` counter (`src/cdp/console-buffer.ts:146`) is intentionally NOT reset on clear (lines 259-262), which causes a **cross-tab data leak for stale cursors**. + +**The `nextSeq` claim is inverted**: The comment says "a stale sinceSeq from a previous tab can't accidentally match new records." In reality, because `nextSeq` is NOT reset, new records get seq numbers continuing from where the old tab left off. A stale `sinceSeq=15` will match new records with seq 16+ from the wrong tab. If `nextSeq` WERE reset to 1, stale `sinceSeq=15` would skip all new records (1 <= 15 is true). + +**Concrete failure scenario**: Agent A iterates console output with `sinceSeq: 15` from a prior `nextCursor` on tab X. Agent B switches to tab Y, clearing the buffer. New tab Y logs appear with seq 16, 17. Agent A's next `browser_console({ sinceSeq: 15 })` returns tab Y's output — cross-tab data leakage. Agent A has no indication the data came from the wrong tab. + +### Finding 5: Page Cache Invalidated Across Tabs + +`src/client.ts:85` holds a single `pageCache` with 1-second TTL (`PAGE_INFO_TTL_MS = 1_000` at line 42). It's nullified on every `switchTab` (line 183) and on `Page.frameNavigated`/`Page.loadEventFired` events from ANY session (via shared `pageInfoDirty` flag at `src/cdp/session.ts:45`). + +**While `switchTab → pageCache = null` is correct, Agent A still gets the wrong tab's info on the next `browser_page_info` call** because it reads from the globally-switched session. There is no per-agent "my current tab" context. + +### Finding 6: Ownership Is Per-Session, Not Per-Agent + +The `OwnershipRegistry` (`src/cdp/ownership.ts:26-72`) tracks which tabs belong to the harness session — it protects the USER's tabs, not sub-agent-to-sub-agent isolation. An agent can switch to any owned tab, including one another agent is actively using. + +### Finding 7: Namespace Provides No Isolation + +The extension's `client` variable at `src/index.ts:36` is a singleton. All agents share one `BrowserClient`. The `--browser-namespace` flag is cosmetic — used in error messages (`src/domains/tabs.ts:71-72`) and temp file paths (`src/util/paths.ts:13`), but provides zero multi-agent isolation within a single pi process. + +### Finding 8: System Prompt Assumes Single-Agent Use + +`src/prompt.ts:63-73` states that the harness "automatically serializes mutation tools so they never race on shared state" — this is intra-agent only. The prompt contains no warnings about: +- Tab switching invalidating buffers and cursors +- Dialog slot overwrites +- The need to verify `browser_current_tab` before mutations +- Other agents sharing the same browser + +### Finding 9: Immediate Fix — `ev.sessionId` Filter + +A single change in `consumeEvents` at `src/cdp/session.ts:33` would prevent ALL cross-tab event contamination: + +```typescript +if (ev.sessionId && ev.sessionId !== sessionId && ev.method !== "Target.targetDestroyed") { + continue; +} +``` + +This prevents dialog events from other tabs from overwriting the slot, navigation events from setting `pageInfoDirty` for the wrong tab, and network/console events from polluting buffers. It makes the single-slot design **correct for single-agent use** and eliminates the most egregious cross-tab contamination for multi-agent use. + +## Code References + +- `src/cdp/session.ts:41-42` — Singleton `sessionId`/`targetId` pair, overwritten on every tab switch +- `src/cdp/session.ts:43` — Single `dialog` slot, overwritten by any `Page.javascriptDialogOpening` without `sessionId` filtering +- `src/cdp/session.ts:125-143` — `switchTo()` destructively overwrites session state and clears buffers +- `src/cdp/session.ts:150-151` — `callOnTarget()` escape hatch exists but is unused by observation tools +- `src/cdp/session.ts:45` — Shared `pageInfoDirty` flag set by navigation events from any session +- `src/cdp/session.ts:136-137` — `networkBuffer.clear()` + `consoleBuffer.clear()` on every tab switch +- `src/cdp/console-buffer.ts:256-262` — `clear()` empties records but preserves `nextSeq`, enabling cross-tab data leaks +- `src/client.ts:85` — Single `pageCache` nullified on any `switchTab` +- `src/client.ts:89` — Single `mutationMutex` shared by all agents +- `src/util/tool.ts:127-138` — Serialized tool mutex acquisition with abort guard +- `src/util/mutex.ts:12-23` — 13-line async FIFO mutex with no per-agent awareness +- `src/registry.ts:25-49` — 22 serialized tools vs 11 observation tools +- `src/cdp/ownership.ts:26-72` — Flat ownership Set, no per-agent tab tracking +- `src/index.ts:36` — Singleton `client` variable — one per process, shared by all agents +- `src/domains/tabs.ts:88-118` — switchTabTool ownership enforcement (per-session, not per-agent) +- `src/domains/tabs.ts:167-188` — closeTabTool ownership enforcement +- `src/prompt.ts:63-73` — System prompt "Parallel Execution" assumes single-agent +- `src/state.ts:22-45` — State persistence with no namespace filtering in `restoreState` +- `src/domains/js.ts:55` — `browser_execute_js` uses implicit global `sessionId` via `session.call()` +- `src/domains/console.ts:122-151` — `sinceSeq` cursor pattern with no invalidation on buffer clear +- `src/domains/network.ts:119-154` — `browser_network_requests` body fetch races against tab switches + +## Integration Points + +### Inbound References +- `src/index.ts:99-119` — `session_start` creates the singleton `BrowserClient`, all agents share it +- `src/index.ts:152-161` — `before_agent_start` injects system prompt, fires for every agent turn with no per-agent customization +- `src/registry.ts:64-66` — `registerAllTools` binds all tools to the singleton `client` via closure + +### Outbound Dependencies +- CDP WebSocket (`src/cdp/transport.ts:25`) — single connection to Chrome, all sessions multiplexed over it +- Chrome DevTools Protocol — `Target.attachToTarget`, `Target.activateTarget`, `Target.createTarget`, `Target.closeTarget` +- pi Extension API — `pi.registerTool`, `pi.on("before_agent_start")`, `pi.on("session_start")`, `pi.appendEntry` + +### Infrastructure Wiring +- `src/cdp/transport.ts:68` — Single event queue consumed by exactly one `for await` loop in `session.ts` +- `src/cdp/session.ts:57-83` — Single `consumeEvents` loop routes CDP events to all buffers +- `src/client.ts:88-96` — `onOwnershipChange` → `persistState` → `pi.appendEntry` chain + +## Architecture Insights + +1. **The entire extension is designed around a single-agent-per-process model.** Every subsystem (transport, session, ownership, buffers, cache, mutex) assumes exclusive access. There is no concept of multiple independent consumers. + +2. **The mutex provides execution serialization but not semantic isolation.** It guarantees two mutations don't run simultaneously, but cannot prevent Agent B from switching tabs between Agent A's observation and its next mutation. + +3. **`callOnTarget` proves the transport layer supports per-target sessions.** The infrastructure is capable of per-agent isolation — the tool layer simply doesn't use it. + +4. **The namespace system is identity-only, not isolation.** It's a label used in error messages and file paths. To become an isolation boundary, it would need to key into separate `BrowserClient` instances. + +5. **Event filtering by `sessionId` is the single most impactful change.** It would fix dialog overwrites, dirty flag theft, buffer pollution, and stale event processing — all from one guard in `consumeEvents`. + +6. **The console `nextSeq` non-reset is a design inversion.** The comment claims it prevents stale cursor matches — the exact opposite is true. Resetting `nextSeq` on clear would prevent cross-tab data leaks. + +## Precedents & Lessons + +4 similar past changes analyzed. + +### Precedent: Parallel tool execution with automatic mutation serialization +**Commit(s)**: `71da8e0` — "feat: parallel tool execution with automatic mutation serialization" (2026-05-04) +**Blast radius**: 8 files across 5 layers + - `src/util/mutex.ts` — new ~25 LOC async FIFO mutex + - `src/util/tool.ts` — serialized lane acquisition + AbortSignal guard + - `src/registry.ts` — 22 of 32 tools marked `serialized: true` + - `src/client.ts` — `mutationMutex()` exposed on `BrowserClient` + +**Follow-up fixes**: +- `f73941c` — Screenshot path rendering crash from parallel invocations + +**Takeaway**: The mutex prevents mutation-on-mutation races but creates a global bottleneck — one sub-agent's slow `waitForLoad` blocks ALL other agents' mutations, even on different tabs. + +### Precedent: Tab ownership — isolate harness tabs from user's browsing +**Commit(s)**: `20ceaa1` — "feat: tab ownership" (2026-05-05) +**Blast radius**: 8 files across 5 layers + - `src/cdp/ownership.ts` — new 72-line OwnershipRegistry + - `src/cdp/session.ts` — attachFirstPage reconciliation, targetDestroyed reaping + - `src/domains/tabs.ts` — ownership gating on switch/close + +**Takeaway**: Ownership protects the USER's tabs but does NOT protect sub-agents from each other. Two sub-agents share one `sessionId`/`targetId` — switching tabs is a global side effect. + +### Precedent: CDP session with single-slot dialog buffer +**Commit(s)**: `d3d86f9` — "fix(cdp): serialize event consumer; persist dialog until taken" (2026-05-02) +**Blast radius**: `src/cdp/session.ts` + +**Follow-up fixes**: +- `c9d1605` — Surface event-consumer crashes via `console.warn()` + +**Takeaway**: The dialog's single-slot buffer was corrected for fast-dismiss races but remains unguarded against multi-session overwrites. + +### Precedent: Audit fixes for list_tabs/switch_tab prefix matching +**Commit(s)**: `a1df2d3` — "release: v0.3.1 — audit fixes" (2026-05-02) +**Blast radius**: 9 files across 4 layers + +**Takeaway**: Prefix resolution for targetIds becomes more ambiguous as owned-tab count increases in multi-agent scenarios. + +### Composite Lessons +- ALL safety primitives are single-agent by design — `71da8e0`, `20ceaa1`, `d3d86f9` +- The mutex serializes globally, not per-tab — `71da8e0` +- Tab switching is a silent cross-agent side effect — `20ceaa1` +- Dialog buffer is a single slot with silent overwrite — `d3d86f9` +- Buffer clearing on tab switch invalidates per-agent cursors — `d3d86f9` +- Prompt and SKILL docs assume single-agent usage — `71da8e0` + +## Historical Context (from thoughts/) +- `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` — 10 trace-quality questions covering every safety subsystem +- `docs/superpowers/specs/2026-05-02-harness-rewrite-design.md` — v0.3.0 rewrite design; §2.2 enumerates 13 predictability bugs fixed by the module split +- `docs/RESEARCH.md` — Competitive analysis showing parallel execution + auto-serialization as a unique differentiator +- `CHANGELOG.md` — v0.3.2 documents parallel-safe boundary; v0.4.0 adds snapshot-first patterns + +## Developer Context + +**Q (`src/cdp/session.ts:41-46`): All safety primitives (mutex, ownership, session, buffers, dialog, page cache) are shared singletons with zero per-agent partitioning. Which isolation scope should we target?** +A: Per-tab isolation (Recommended) — Add per-tab data structures: per-tab dialog queue, per-tab page cache, per-tab buffer maps, per-tab dirty flags. Agents still share one client but each tab has independent state. Minimal change preventing data corruption without connection overhead. + +**Q (`src/cdp/session.ts:33`): The per-tab isolation can be delivered in phases. Which delivery cadence?** +A: Quick-win first (Recommended) — Phase 1: `ev.sessionId` filter in `consumeEvents` (1-line fix). Phase 2: per-tab data structures. Phase 3: agent-aware tab locking. + +## Related Research +- Questions source: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` + +## Open Questions +1. **Agent identity exposure**: The pi platform currently does NOT expose the calling agent's ID to tool handlers. Phase 3 (agent-aware tab locking) requires identifying which agent initiated a tool call. Does the `ExtensionContext` carry agent identity, or would this require a pi platform change? +2. **Observation tool isolation**: Even after per-tab data structures, observation tools (`browser_execute_js`, `browser_screenshot`) will still read from the globally-switched session. Should they be changed to use `callOnTarget` with a per-agent cached sessionId? +3. **Tab switching protocol**: Should agents be required to explicitly "check out" and "release" tabs? Or should the harness auto-detect which agent was last on a tab and restore context before executing tools? +4. **Namespace-per-client in separate processes**: When two separate pi processes (e.g., parent + subagent) both load the harness, each creates its own `BrowserClient` — they're naturally isolated. Should we document this pattern as the recommended multi-agent approach? diff --git a/CHANGELOG.md b/CHANGELOG.md index 473930e..8450c18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,18 @@ All notable changes to pi-browser-harness will be documented in this file. -## 0.5.0 — 2026-05-07 +## 0.6.0 — 2026-06-21 + +### Added + +- **Per-tab isolation for multi-agent safety.** Each agent's browser session now operates in a dedicated Chrome window with tab ownership tracking. Tabs opened via `browser_open_urls` and `browser_new_tab` are automatically registered in the ownership registry and tagged with a 🟢 prefix in the document title for user visibility. `browser_list_tabs` scoped to `"owned"` (default) only shows the current session's tabs. +- **Unix socket daemon with auto-reconnect.** The browser daemon now binds to a Unix-domain socket (`pi-browser.sock`) in the harness temp directory instead of a TCP port. The transport layer reconnects automatically when Chrome restarts — no manual `/browser-reload-daemon` needed. +- **On-demand browser initialization.** Chrome is no longer launched eagerly at harness startup. The harness attaches lazily on the first browser tool call, reducing resource usage when the agent isn't using the browser. +- **`browser_setup` as agent-callable tool.** The setup tool can now be called programmatically by agents, not just via slash command. Idempotent — safe to call when already connected. + +### Fixed + +- **Chrome detection on macOS** now matches by process-name substring instead of exact binary path comparison, fixing false negatives when Chrome is running from different installation paths (e.g. `/Applications/Google Chrome.app/` vs user-local copies). ### Added diff --git a/package-lock.json b/package-lock.json index d96363c..baa1b14 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "pi-browser-harness", - "version": "0.3.1", + "version": "0.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "pi-browser-harness", - "version": "0.3.1", + "version": "0.5.0", "license": "MIT", "dependencies": { "ws": "^8.20.0" @@ -15,6 +15,7 @@ "@mariozechner/pi-coding-agent": "*", "@mariozechner/pi-tui": "*", "@types/ws": "^8.18.1", + "tsx": "^4.22.4", "typebox": "*", "typescript": "^5.0.0" }, @@ -876,6 +877,448 @@ "tslib": "^2.4.0" } }, + "node_modules/@esbuild/aix-ppc64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.28.1.tgz", + "integrity": "sha512-Svl7tq8k/08+p6CXPpRjQ1fKX+1odH/BQbb48fV6fj3CWHhsoIOoY87w1oHXm0qEpkIK3ZfVgp0hed3XBXzXMQ==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "aix" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/android-arm": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/android-arm/-/android-arm-0.28.1.tgz", + "integrity": "sha512-0k2F129Xdio1TdJfzJ8sy1Q47vUD2NnwdhiAf7drUN1EBTfPf4hsFCtmMgu/6m8JSzsBrlmVjudMBQqOfG8usQ==", + "cpu": [ + "arm" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/android-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/android-arm64/-/android-arm64-0.28.1.tgz", + "integrity": "sha512-34EGEbCIAgosYz6goLcopX6Mo7NyGv9tfwEM2/7Ce2VcVRk568iSvniGWcUXIy7wEDR1wzolcxcriFVrWYcwBg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/android-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/android-x64/-/android-x64-0.28.1.tgz", + "integrity": "sha512-dbwY7ltSMDWsRatcRpCnES4F+im88OCUgGZjy52shC7GqHRE/cYlxNbB4Z4UpJswpcc4Qxd2oE/ufM0p61IKng==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/darwin-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/darwin-arm64/-/darwin-arm64-0.28.1.tgz", + "integrity": "sha512-TZbWkQY7kvTAXbXUT7uVACR5cMHsDiSz9z7ZKAX/RTq/WJEk3QyRr0wZpNhBDX+/0CtdqUIJlOiodQcta6tY3Q==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/darwin-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/darwin-x64/-/darwin-x64-0.28.1.tgz", + "integrity": "sha512-zfdzgK9ACBNZLI/CyHTOx81SyNbM6YXn7rxSgX97VjyiPl9W1i4Ka4fgKECEoFCKGpvBj5qArWIGgQjOwkgskQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/freebsd-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/freebsd-arm64/-/freebsd-arm64-0.28.1.tgz", + "integrity": "sha512-wG2EA8ENdEI0qhkSZMjfqrdY+ziCYCPMmtZjjIwOmXFjmyzEHn+UUxk5of+SYsjtfs3VpnlC7QLzSI5hY/rOAw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/freebsd-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/freebsd-x64/-/freebsd-x64-0.28.1.tgz", + "integrity": "sha512-i7dZ9vQgnvSCzi/rYCXNgtF/U+eKZNJBzu3eTQbRgHnM7tNSizLOkRFAl3qzVc/Op/u5YkHHa4pf/3DOYHthLQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-arm": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-arm/-/linux-arm-0.28.1.tgz", + "integrity": "sha512-qVXBOHQS+d5Y722GwJzJUtOLlX7km3CraOaGormF1pDtPd2C/l1SHRPgjLunLGe51Sh5YYWKMFDyV4SxgMQYTQ==", + "cpu": [ + "arm" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-arm64/-/linux-arm64-0.28.1.tgz", + "integrity": "sha512-yHs+0uc8+nvEAfAfxrWQKK5peSNzBc4PegcMO0EJ2hT71uA7vB8Ihg2e77R2P7SG5uYjPbHlLLmve4LLLRCf0g==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-ia32": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-ia32/-/linux-ia32-0.28.1.tgz", + "integrity": "sha512-d1z4ZuP0ajrfz/FhGT4vv278rX8KnPPJx8i5+AtK7TYbx9Le9F1hyzurZpkEyjkGa9dUGhQow4C1NmeGvqxN2w==", + "cpu": [ + "ia32" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-loong64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-loong64/-/linux-loong64-0.28.1.tgz", + "integrity": "sha512-M5sRjUVZrkm1OAPR3dlOYzNmN+loZKGVi1VUQGrwuqLcbR6qeAz+famMhjASeH3YVKvZz+zT1jlh/keC3Rj/lg==", + "cpu": [ + "loong64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-mips64el": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-mips64el/-/linux-mips64el-0.28.1.tgz", + "integrity": "sha512-mRObBZeHh2OxcBFPWE/FjylkRgZdYuiTR3vaTozquCGOH14iP9oN4x4Ge81CoIDYQrXmIxpFumJBu5MtZpnQJQ==", + "cpu": [ + "mips64el" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-ppc64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-ppc64/-/linux-ppc64-0.28.1.tgz", + "integrity": "sha512-slScBsMAb3GFDcdrCgLwZtPYRoH2H/youv10QiZyRjmsP48fznoveWytSgCI/R0ZcUgpc0ZhIUEx6LHts8yrfQ==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-riscv64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-riscv64/-/linux-riscv64-0.28.1.tgz", + "integrity": "sha512-kw0owk1o0GFETUJyW0jc0G4Yzs0BHZn0JDZ8JRT088vjJYX777BAs1fDGxAC+q831qOs2DTC96mNsG2opdfyyQ==", + "cpu": [ + "riscv64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-s390x": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-s390x/-/linux-s390x-0.28.1.tgz", + "integrity": "sha512-/lAIjX8aYFRByhh6L5rYtPEDRqa9de/4V/juOXcta5frjvzXO4/sqEtyytse0g3zZFuWu5cDN0MkLz2qRDD2Ag==", + "cpu": [ + "s390x" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/linux-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/linux-x64/-/linux-x64-0.28.1.tgz", + "integrity": "sha512-u/anNYF2mmVOEDwLtnQ1wOr3EZ9sTNGLWrsYGYwHWzGA3Si84IOkHXlbWTD1NB+9/1lcnweYKO54uhxZydNzfA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/netbsd-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/netbsd-arm64/-/netbsd-arm64-0.28.1.tgz", + "integrity": "sha512-oks0DYbLwWMmaakTsCb+zL4E+aHRVLom9IJZOAthMQEPiQmydXHkziYEsGYRx0uNV/IjEKGAV941JzH02pflqw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/netbsd-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/netbsd-x64/-/netbsd-x64-0.28.1.tgz", + "integrity": "sha512-aeL6lAnN89Hz43Mlh1G8ARasbuoYvSITDEx0tHh5b7jJnHcssqgjy9Yx430GDpmCa6OyrKoS0aNRjKundRizGg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/openbsd-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/openbsd-arm64/-/openbsd-arm64-0.28.1.tgz", + "integrity": "sha512-MEFJe5C3R8pwXdZ5Y21oo6m7ePiS0d9pWucn99O/wvyJZChoIQKrQDxKrGeW8F5+T0okTHesAmDeiHDTIq0V/Q==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/openbsd-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/openbsd-x64/-/openbsd-x64-0.28.1.tgz", + "integrity": "sha512-i/ZLIOafE0Z8cI/XANJAixoJL/uRAoS2xOA3rb0xN+KK0K177cMAsQYkzHtBrtMXAKuAc7HGgcWiZ/sRC1Nxgw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/openharmony-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/openharmony-arm64/-/openharmony-arm64-0.28.1.tgz", + "integrity": "sha512-ge+Z7EXFNt2BO1oAMsVpiQ8EwndV9i1xXerAeTIK7AtPs3bKFXQM7nlRxDSIUIMeueR1CNXxqztLzdNeReKBJg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openharmony" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/sunos-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/sunos-x64/-/sunos-x64-0.28.1.tgz", + "integrity": "sha512-BEjgtECkL3vY+SaSQ6nzVfiALUeFxpawyp8Jmf5PtYhf1Ug40N1h/hxlhts+f1FvSvarEigdxS3BlSMI2PJLcQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "sunos" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/win32-arm64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/win32-arm64/-/win32-arm64-0.28.1.tgz", + "integrity": "sha512-lCv9eK/H6ZJWbE7bh2nw54CZ9M2nupBxJcTsdk/QQnWkdSjKGuxmmH8/GWrlT1eMmZfn4dGcCjRte397WqfQXA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/win32-ia32": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/win32-ia32/-/win32-ia32-0.28.1.tgz", + "integrity": "sha512-zvb/mB2bSCoJOpoCBgYKKpX6YM6mJBlBUVUtVj41DlZJVEB6/0CKlRYxP5wWl1C1ILiCoAU5wZZ4q1P3qeS6Eg==", + "cpu": [ + "ia32" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@esbuild/win32-x64": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/@esbuild/win32-x64/-/win32-x64-0.28.1.tgz", + "integrity": "sha512-bm4Mowrv+GXMlpWX++EcXw/iLyd1o3+bJkC2DkWXYVvgZCqD/bSj9ctZeAMC3cIxgjRVR2Dufaiu4YPxr5gW1A==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, "node_modules/@google/genai": { "version": "1.50.1", "resolved": "https://registry.npmjs.org/@google/genai/-/genai-1.50.1.tgz", @@ -2798,6 +3241,48 @@ "once": "^1.4.0" } }, + "node_modules/esbuild": { + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.28.1.tgz", + "integrity": "sha512-HrJrvZv5ayxBzPfwphOoNzkzOIIlifzk0KJrGK2c8R4+LKpMtpYLQeUdjnwjWv/LZlkH2laZk+4w78pi99D4Vw==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "bin": { + "esbuild": "bin/esbuild" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "@esbuild/aix-ppc64": "0.28.1", + "@esbuild/android-arm": "0.28.1", + "@esbuild/android-arm64": "0.28.1", + "@esbuild/android-x64": "0.28.1", + "@esbuild/darwin-arm64": "0.28.1", + "@esbuild/darwin-x64": "0.28.1", + "@esbuild/freebsd-arm64": "0.28.1", + "@esbuild/freebsd-x64": "0.28.1", + "@esbuild/linux-arm": "0.28.1", + "@esbuild/linux-arm64": "0.28.1", + "@esbuild/linux-ia32": "0.28.1", + "@esbuild/linux-loong64": "0.28.1", + "@esbuild/linux-mips64el": "0.28.1", + "@esbuild/linux-ppc64": "0.28.1", + "@esbuild/linux-riscv64": "0.28.1", + "@esbuild/linux-s390x": "0.28.1", + "@esbuild/linux-x64": "0.28.1", + "@esbuild/netbsd-arm64": "0.28.1", + "@esbuild/netbsd-x64": "0.28.1", + "@esbuild/openbsd-arm64": "0.28.1", + "@esbuild/openbsd-x64": "0.28.1", + "@esbuild/openharmony-arm64": "0.28.1", + "@esbuild/sunos-x64": "0.28.1", + "@esbuild/win32-arm64": "0.28.1", + "@esbuild/win32-ia32": "0.28.1", + "@esbuild/win32-x64": "0.28.1" + } + }, "node_modules/escalade": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/escalade/-/escalade-3.2.0.tgz", @@ -2996,6 +3481,21 @@ "node": ">=12.20.0" } }, + "node_modules/fsevents": { + "version": "2.3.3", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz", + "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, "node_modules/gaxios": { "version": "7.1.4", "resolved": "https://registry.npmjs.org/gaxios/-/gaxios-7.1.4.tgz", @@ -4052,6 +4552,25 @@ "devOptional": true, "license": "0BSD" }, + "node_modules/tsx": { + "version": "4.22.4", + "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.22.4.tgz", + "integrity": "sha512-X8EX+XV4QR5xCsrgxaED954zTDfY8KqlDtskKEL0cHhyS/P8b4IFOvGDQpsC9Q1XnLq915wEfwwY/zzskCtmhg==", + "dev": true, + "license": "MIT", + "dependencies": { + "esbuild": "~0.28.0" + }, + "bin": { + "tsx": "dist/cli.mjs" + }, + "engines": { + "node": ">=18.0.0" + }, + "optionalDependencies": { + "fsevents": "~2.3.3" + } + }, "node_modules/typebox": { "version": "1.1.34", "resolved": "https://registry.npmjs.org/typebox/-/typebox-1.1.34.tgz", diff --git a/package.json b/package.json index a316917..e7768b5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pi-browser-harness", - "version": "0.5.0", + "version": "0.6.0", "description": "Browser control extension for pi — navigate, click, type, screenshot, and extract data from real Chrome via CDP", "keywords": [ "pi-package", @@ -52,6 +52,7 @@ "@mariozechner/pi-coding-agent": "*", "@mariozechner/pi-tui": "*", "@types/ws": "^8.18.1", + "tsx": "^4.22.4", "typebox": "*", "typescript": "^5.0.0" }, diff --git a/skills/pi-browser-harness/SKILL.md b/skills/pi-browser-harness/SKILL.md index 69105a8..e4221be 100644 --- a/skills/pi-browser-harness/SKILL.md +++ b/skills/pi-browser-harness/SKILL.md @@ -1,6 +1,6 @@ --- name: pi-browser-harness -description: Direct browser control via CDP. Use when the user wants to automate, scrape, test, or interact with web pages. Connects to the user's already-running Chrome. Default to browser_snapshot for understanding pages and browser_execute_js for surgical reads — browser_screenshot is for visual verification only. +description: Direct browser control via CDP. Use when the user wants to automate, scrape, test, or interact with web pages. Connects to the user's already-running Chrome. Default to browser_snapshot for understanding pages and browser_execute_js for surgical reads — browser_screenshot is for visual verification only. --- # pi-browser-harness @@ -30,6 +30,22 @@ What do you need to know? Pass `@(x,y)` from `browser_snapshot` straight to `browser_click`. No screenshot round-trip. +## Connection Setup + +Browser control is **on-demand** — the daemon does NOT start automatically. +If you try a browser tool and get a `not_connected` error, tell the user to +run `/browser-setup` first. This opens the daemon and connects to Chrome. +Once initialized, all subsequent sessions reuse the same connection silently. + +**Before calling any browser tool**, the runtime checks for the daemon socket +at `/tmp/pi-browser-daemon.sock`. If the socket is missing (user hasn't run +`/browser-setup`), you get: `"Browser harness not initialized. Run /browser-setup first"`. + +**Do not ask the user.** Call `browser_setup` directly — it spawns the daemon, +connects to Chrome, and opens a test tab. The user sees a single "Allow Remote +Debugging" prompt the first time. After that, all sessions reuse the same connection. +`browser_setup` is idempotent — safe to call even when already connected. + ## Connection You're attached to the user's real Chrome — never launch your own. If auth is required, stop and ask the user. If `browser_page_info` returns a dialog, handle it first with `browser_handle_dialog`. diff --git a/src/cdp/daemon-transport.ts b/src/cdp/daemon-transport.ts new file mode 100644 index 0000000..9d4202c --- /dev/null +++ b/src/cdp/daemon-transport.ts @@ -0,0 +1,277 @@ +/** + * DaemonTransport — CdpTransport backed by a Unix socket to the browser daemon. + * + * Satisfies the exact CdpTransport interface so BrowserClient, CdpSession, and + * all domain tools work without changes. Internally uses newline-delimited JSON + * over a Unix domain socket instead of a WebSocket to Chrome. + * + * References: + * - CdpTransport interface: src/cdp/transport.ts + * - net.createConnection IPC: https://nodejs.org/docs/latest-v22.x/api/net.html#netcreateconnection + * - readline.createInterface: https://nodejs.org/docs/latest-v22.x/api/readline.html#readlinecreateinterfaceoptions + */ + +import { createConnection, type Socket } from "node:net"; +import { createInterface } from "node:readline"; +import { type Result, err, ok } from "../util/result"; +import { type CdpError, cdpError } from "../cdp/errors"; +import type { CdpTransport } from "../cdp/transport"; +import type { CdpEvent } from "../cdp/types"; +import { + DAEMON_SOCKET_PATH, + type WireRequest, + type WireControl, + deserialize, + serialize, +} from "../daemon/protocol"; + +// ── Types ────────────────────────────────────────────────────────────────────── + +type EventQueue = { + readonly push: (e: CdpEvent) => void; + readonly end: () => void; + readonly iter: AsyncIterable; +}; + +// ── Helpers ──────────────────────────────────────────────────────────────────── + +const makeEventQueue = (): EventQueue => { + const buf: CdpEvent[] = []; + const waiters: Array<(v: IteratorResult) => void> = []; + let ended = false; + return { + push(e) { + if (ended) return; + const w = waiters.shift(); + if (w) w({ value: e, done: false }); + else buf.push(e); + }, + end() { + ended = true; + for (const w of waiters.splice(0)) w({ value: undefined as unknown as CdpEvent, done: true }); + }, + iter: { + [Symbol.asyncIterator]() { + return { + next: (): Promise> => + new Promise((resolve) => { + const next = buf.shift(); + if (next) resolve({ value: next, done: false }); + else if (ended) resolve({ value: undefined as unknown as CdpEvent, done: true }); + else waiters.push(resolve); + }), + }; + }, + }, + }; +}; + +// ── Implementation ───────────────────────────────────────────────────────────── + +export const createDaemonTransport = (clientId: string): CdpTransport => { + let socket: Socket | null = null; + let rl: ReturnType | null = null; + let queue = makeEventQueue(); + const closeListeners = new Set<() => void>(); + const pending = new Map) => void; + timer: ReturnType; + method: string; + }>(); + let registered = false; + let nextRequestId = 1; + + const cleanup = (reason: string): void => { + for (const [, p] of pending) { + clearTimeout(p.timer); + p.resolve(err(cdpError("transport_closed", reason, p.method))); + } + pending.clear(); + queue.end(); + queue = makeEventQueue(); + registered = false; + + if (rl) { rl.close(); rl = null; } + if (socket) { try { socket.destroy(); } catch {} socket = null; } + + for (const cb of closeListeners) cb(); + }; + + // ── Connect ──────────────────────────────────────────────────────────── + + const connect = (_url: string, opts?: { timeoutMs?: number }): Promise> => { + // url is ignored — we always connect to the daemon socket + const timeoutMs = opts?.timeoutMs ?? 10_000; + + if (socket && !socket.destroyed && registered) { + return Promise.resolve(ok(undefined)); + } + + return new Promise((resolve) => { + let settled = false; + const settle = (r: Result): void => { + if (settled) return; + settled = true; + resolve(r); + }; + + try { + socket = createConnection(DAEMON_SOCKET_PATH); + } catch (e) { + settle(err(cdpError("transport_closed", e instanceof Error ? e.message : String(e)))); + return; + } + + const connectTimer = setTimeout(() => { + cleanup("Connection timeout"); + settle(err(cdpError("timeout", `Daemon connection timed out after ${timeoutMs}ms`))); + }, timeoutMs); + + socket.on("connect", () => { + clearTimeout(connectTimer); + rl = createInterface({ input: socket!, crlfDelay: Infinity }); + + rl.on("line", (line: string) => { + const msg = deserialize(line); + if (!msg) return; + + if (msg.type === "control" && msg.action === "registered" && msg.clientId === clientId) { + registered = true; + settle(ok(undefined)); + return; + } + + if (msg.type === "control" && msg.action === "shutdown") { + cleanup(`Daemon shutting down: ${msg.reason ?? "unknown"}`); + return; + } + + if (!registered) return; + + if (msg.type === "response") { + const p = pending.get(msg.id); + if (!p) return; + pending.delete(msg.id); + clearTimeout(p.timer); + + if (msg.error) { + p.resolve(err(cdpError( + msg.error.message.includes("Session with given id not found") + ? "session_not_found" + : "remote_error", + msg.error.message, + p.method, + ))); + } else { + p.resolve(ok(msg.result)); + } + return; + } + + if (msg.type === "event") { + queue.push({ + method: msg.method, + params: msg.params, + ...(msg.sessionId !== undefined ? { sessionId: msg.sessionId } : {}), + }); + return; + } + }); + + socket!.on("error", () => { + // Errors are surfaced via 'close' + }); + + socket!.on("close", () => { + clearTimeout(connectTimer); + cleanup("Daemon socket closed"); + // If connect hasn't settled yet, settle with error + settle(err(cdpError("transport_closed", "Daemon socket closed before registration"))); + }); + + // Send registration + const regMsg: WireControl = { type: "control", action: "register", clientId }; + socket!.write(serialize(regMsg) + "\n"); + }); + + socket.on("error", (e: NodeJS.ErrnoException) => { + clearTimeout(connectTimer); + const msg = e.code === "ENOENT" + ? "Daemon not running — socket not found" + : e.message; + settle(err(cdpError("transport_closed", msg))); + }); + }); + }; + + // ── Close ────────────────────────────────────────────────────────────── + + const close = async (): Promise => { + if (registered && socket && !socket.destroyed) { + const dereg: WireControl = { type: "control", action: "deregister", clientId }; + try { socket.write(serialize(dereg) + "\n"); } catch {} + } + cleanup("close() called"); + }; + + // ── Request ──────────────────────────────────────────────────────────── + + const request = ( + method: string, + params: Record, + opts?: { sessionId?: string | null; timeoutMs?: number }, + ): Promise> => { + const timeoutMs = opts?.timeoutMs ?? 15_000; + const sessionId = opts?.sessionId ?? null; + + if (!socket || socket.destroyed || !registered) { + return Promise.resolve(err(cdpError("transport_closed", "Daemon not connected", method))); + } + + const id = nextRequestId++; + const req: WireRequest = { + type: "request", + id, + method, + params, + ...(sessionId !== null && sessionId !== undefined ? { sessionId } : {}), + }; + + return new Promise((resolve) => { + const timer = setTimeout(() => { + pending.delete(id); + resolve(err(cdpError("timeout", `Daemon timeout after ${timeoutMs}ms: ${method}`, method))); + }, timeoutMs); + + pending.set(id, { resolve, timer, method }); + + try { + socket!.write(serialize(req) + "\n"); + } catch (e) { + clearTimeout(timer); + pending.delete(id); + resolve(err(cdpError("transport_closed", e instanceof Error ? e.message : String(e), method))); + } + }); + }; + + // ── Events ───────────────────────────────────────────────────────────── + + const events = (): AsyncIterable => queue.iter; + + // ── State ────────────────────────────────────────────────────────────── + + const state = (): "open" | "closed" | "connecting" => { + if (!socket) return "closed"; + if (!registered) return "connecting"; + if (socket.destroyed) return "closed"; + return "open"; + }; + + const onClose = (cb: () => void): (() => void) => { + closeListeners.add(cb); + return () => closeListeners.delete(cb); + }; + + return { connect, close, request, events, state, onClose }; +}; diff --git a/src/client.ts b/src/client.ts index f0eb016..3c79f1a 100644 --- a/src/client.ts +++ b/src/client.ts @@ -11,6 +11,7 @@ import type { DaemonStatus, DialogInfo, PageInfo, TabInfo } from "./cdp/types"; export type BrowserClientOptions = { readonly namespace: string; + readonly transport?: CdpTransport; readonly remote?: { readonly cdpUrl: string; readonly browserId: string }; readonly initialOwnership?: { readonly ownedTargetIds?: ReadonlyArray; @@ -28,6 +29,9 @@ export type BrowserClient = { status(): DaemonStatus; start(): Promise>; stop(): Promise; + /** Detach from the current page target (removes the "Chrome is being controlled" banner) + * while keeping the transport connection alive. Call on session shutdown. */ + detach(): Promise; evaluateJs(expression: string, sessionId?: string): Promise>; pageInfo(): Promise>; takeDialog(): DialogInfo | null; @@ -77,7 +81,7 @@ const parsePageInfoPayload = (v: unknown): Result => { }; export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient => { - const transport = createCdpTransport(); + const transport = opts.transport ?? createCdpTransport(); const ownershipInit: { ownedTargetIds?: ReadonlyArray; harnessWindowTargetId?: string } = {}; if (opts.initialOwnership?.ownedTargetIds !== undefined) { ownershipInit.ownedTargetIds = opts.initialOwnership.ownedTargetIds; @@ -298,9 +302,19 @@ export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient = ...(remote?.browserId !== undefined ? { remoteBrowserId: remote.browserId } : {}), }); + const detach = async (): Promise => { + const cur = session.current(); + if (!cur) return; + // Target.detachFromTarget removes the "Chrome is being controlled" banner + // and releases the page session. The transport (WebSocket/Unix socket) + // stays alive for reuse by the next session. + // Reference: https://chromedevtools.github.io/devtools-protocol/tot/Target/#method-detachFromTarget + await transport.request("Target.detachFromTarget", { sessionId: cur.sessionId }, { sessionId: null }); + }; + return { namespace: opts.namespace, - ensureAlive, status, start, stop, + ensureAlive, status, start, stop, detach, evaluateJs, pageInfo, takeDialog: () => session.takeDialog(), listTabs, switchTab, newTab, closeTab, diff --git a/src/daemon/bridge.ts b/src/daemon/bridge.ts new file mode 100644 index 0000000..c2c1355 --- /dev/null +++ b/src/daemon/bridge.ts @@ -0,0 +1,411 @@ +/** + * CdpBridge — persistent WebSocket to Chrome, ID multiplexing, event routing. + * + * A single long-lived process connection to Chrome that proxies CDP traffic + * for multiple pi clients connected via the IpcServer. One Allow dialog ever. + * + * References: + * - CDP message format: https://github.com/aslushnikov/getting-started-with-cdp#protocol-fundamentals + * - CDP sessions + IDs: https://github.com/aslushnikov/getting-started-with-cdp#targets--sessions + * - Target domain: https://chromedevtools.github.io/devtools-protocol/tot/Target/ + */ + +import WebSocket from "ws"; +import { discoverWsUrl } from "../cdp/discovery"; +import type { CdpRawMessage } from "../cdp/types"; +import type { WireRequest, WireResponse, WireEvent } from "./protocol"; +import { CDP_CONNECT_TIMEOUT_MS, CDP_COMMAND_TIMEOUT_MS } from "./protocol"; + +// ── Types ────────────────────────────────────────────────────────────────────── + +export type SendToClient = (clientId: string, msg: WireResponse | WireEvent) => void; + +export type EventHandler = (event: WireEvent, targetClientIds: string[]) => void; + +export type CloseHandler = () => void; + +export type CdpBridge = { + /** Start the bridge. Resolves when the daemon IPC + Chrome discovery loop + * are initialized (not when Chrome is connected — that may take retries). */ + start(): Promise; + stop(): Promise; + handleRequest(req: WireRequest, clientId: string, send: SendToClient): void; + /** True when the Chrome WebSocket is open and ready. */ + isAlive(): boolean; + recordSession(clientId: string, sessionId: string): void; + releaseSession(sessionId: string): void; + getSessionOwner(sessionId: string): string | undefined; + removeClient(clientId: string): void; + onEvent(handler: EventHandler): void; + onClose(handler: CloseHandler): void; +}; + +// ── ID Multiplexer ──────────────────────────────────────────────────────────── + +interface PendingEntry { + clientId: string; + localId: number; +} + +class IdMultiplexer { + private nextId = 1; + private localToRemote = new Map>(); + private pending = new Map(); + + allocate(clientId: string, localId: number): number { + const daemonId = this.nextId++; + let m = this.localToRemote.get(clientId); + if (!m) { m = new Map(); this.localToRemote.set(clientId, m); } + m.set(localId, daemonId); + this.pending.set(daemonId, { clientId, localId }); + return daemonId; + } + + resolve(daemonId: number): PendingEntry | null { + const e = this.pending.get(daemonId); + if (!e) return null; + this.pending.delete(daemonId); + const m = this.localToRemote.get(e.clientId); + if (m) m.delete(e.localId); + return e; + } + + clearClient(clientId: string): void { + const m = this.localToRemote.get(clientId); + if (m) { for (const [, did] of m) this.pending.delete(did); } + this.localToRemote.delete(clientId); + } +} + +// ── Event Router ─────────────────────────────────────────────────────────────── + +class EventRouter { + private owners = new Map(); // sessionId → clientId + private clientSessions = new Map>(); // clientId → sessions + + record(clientId: string, sessionId: string): void { + const prev = this.owners.get(sessionId); + if (prev && prev !== clientId) { + this.clientSessions.get(prev)?.delete(sessionId); + } + this.owners.set(sessionId, clientId); + let s = this.clientSessions.get(clientId); + if (!s) { s = new Set(); this.clientSessions.set(clientId, s); } + s.add(sessionId); + } + + release(sessionId: string): void { + const owner = this.owners.get(sessionId); + if (owner) this.clientSessions.get(owner)?.delete(sessionId); + this.owners.delete(sessionId); + } + + getOwner(sessionId: string): string | undefined { + return this.owners.get(sessionId); + } + + removeClient(clientId: string): void { + const sessions = this.clientSessions.get(clientId); + if (sessions) { for (const sid of sessions) this.owners.delete(sid); } + this.clientSessions.delete(clientId); + } + + /** + * Route a CDP event to client(s). + * Session-scoped → only the owning client. + * Browser-level (no sessionId) → all connected clients. + */ + route(event: WireEvent): string[] { + if (event.sessionId) { + const owner = this.owners.get(event.sessionId); + return owner ? [owner] : []; + } + return [...this.clientSessions.keys()]; + } + + allClients(): string[] { + return [...this.clientSessions.keys()]; + } +} + +// ── Implementation ───────────────────────────────────────────────────────────── + +export const createCdpBridge = (): CdpBridge => { + let ws: WebSocket | null = null; + let wsUrl: string | null = null; + const mux = new IdMultiplexer(); + const router = new EventRouter(); + let eventHandler: EventHandler | null = null; + let closeHandler: CloseHandler | null = null; + + // daemonId → { send, timer, isAttach } + const callbacks = new Map; + isAttach: boolean; + }>(); + + // ── Handle raw CDP messages from Chrome ───────────────────────────────── + + const onChromeMessage = (raw: string): void => { + let msg: CdpRawMessage; + try { msg = JSON.parse(raw); } catch { return; } + + // ---- Response (has id) ---- + if (msg.id !== undefined) { + const cb = callbacks.get(msg.id); + if (!cb) { + // Stale response — client already timed out or disconnected. + // Still clean up the multiplexer mapping. + mux.resolve(msg.id); + return; + } + clearTimeout(cb.timer); + callbacks.delete(msg.id); + + const localId = cb.localId; + + if (msg.error) { + cb.send(cb.clientId, { + type: "response", + id: localId, + error: { code: msg.error.code ?? -1, message: msg.error.message }, + }); + } else { + // Track session ownership for attach responses + if (cb.isAttach && msg.result) { + const result = msg.result as { sessionId?: string }; + if (result.sessionId) { + router.record(cb.clientId, result.sessionId); + } + } + cb.send(cb.clientId, { type: "response", id: localId, result: msg.result }); + } + + mux.resolve(msg.id); + return; + } + + // ---- Event (no id, has method) ---- + if (!msg.method) return; + + // Inspector.detached — release session ownership + if (msg.method === "Inspector.detached" && msg.sessionId) { + router.release(msg.sessionId); + } + + const wireEvent: WireEvent = { + type: "event", + method: msg.method, + ...(msg.params !== undefined ? { params: msg.params } : {}), + ...(msg.sessionId !== undefined ? { sessionId: msg.sessionId } : {}), + }; + + // Route through the event handler for the IpcServer to broadcast + if (eventHandler) { + const targets = router.route(wireEvent); + if (targets.length > 0) { + eventHandler(wireEvent, targets); + } + } + }; + + // ── Connect to Chrome ─────────────────────────────────────────────────── + + // ── Auto-reconnecting Chrome connection ─────────────────────────────── + + let reconnectTimer: ReturnType | null = null; + let reconnectAttempt = 0; + let stopped = false; + + /** + * Try to discover Chrome and open a WebSocket. On failure, schedule + * an exponential backoff retry (capped at 60s). When Chrome finally + * becomes available, the bridge connects automatically — no manual + * daemon restart needed. + */ + const tryConnect = (): void => { + if (stopped) return; + if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } + + const attempt = async (): Promise => { + // Already connected — nothing to do. + if (ws && ws.readyState === WebSocket.OPEN) return; + + // Resolve URL (discover if not cached, or clear cache on retry) + let url = wsUrl; + if (!url || reconnectAttempt > 0) { + const d = await discoverWsUrl(); + if (!d.success) { scheduleRetry(); return; } + url = d.data; + wsUrl = url; + } + + const settledPromise = new Promise((settle) => { + let settled = false; + const settleOnce = (): void => { + if (settled) return; + settled = true; + settle(); + }; + + let sock: WebSocket; + try { + sock = new WebSocket(url!, { perMessageDeflate: false }); + } catch { + scheduleRetry(); + return; + } + + const timer = setTimeout(() => { + sock.close(); + settleOnce(); + }, CDP_CONNECT_TIMEOUT_MS); + + sock.on("open", () => { + clearTimeout(timer); + ws = sock; + reconnectAttempt = 0; + // Enable target discovery so we see page events + ws.send(JSON.stringify({ id: 0, method: "Target.setDiscoverTargets", params: { discover: true } })); + console.log("[pi-browser-daemon] Connected to Chrome ✓"); + settleOnce(); + }); + + sock.on("message", (data: WebSocket.Data) => { + onChromeMessage(typeof data === "string" ? data : data.toString()); + }); + + sock.on("error", () => { + clearTimeout(timer); + settleOnce(); + }); + + sock.on("close", () => { + clearTimeout(timer); + ws = null; + // Reject all pending callbacks + for (const [daemonId, cb] of callbacks) { + clearTimeout(cb.timer); + cb.send(cb.clientId, { + type: "response", + id: cb.localId, + error: { code: -32000, message: "Chrome disconnected" }, + }); + callbacks.delete(daemonId); + mux.resolve(daemonId); + } + closeHandler?.(); + // Auto-reconnect on unexpected close + scheduleRetry(); + }); + }); + + await settledPromise; + // If connection didn't succeed, schedule a retry + if (!ws || ws.readyState !== WebSocket.OPEN) { + scheduleRetry(); + } + }; + + attempt().catch(() => scheduleRetry()); + }; + + /** Exponential backoff: 1s, 2s, 4s, 8s, 16s, 32s, 60s (capped). */ + const scheduleRetry = (): void => { + if (stopped) return; + const delay = Math.min(1000 * Math.pow(2, Math.min(reconnectAttempt, 6)), 60_000); + reconnectAttempt++; + wsUrl = null; // force re-discovery on next attempt + reconnectTimer = setTimeout(tryConnect, delay); + }; + + const start = async (): Promise => { + stopped = false; + // Don't await — the retry loop runs in the background. + // The daemon is "started" when the IPC server is listening, not when + // Chrome is connected. + tryConnect(); + }; + + const stop = async (): Promise => { + stopped = true; + if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } + for (const [, cb] of callbacks) clearTimeout(cb.timer); + callbacks.clear(); + if (ws) { try { ws.close(1000, "Shutdown"); } catch {} ws = null; } + wsUrl = null; + }; + + const handleRequest = ( + req: WireRequest, + clientId: string, + send: SendToClient, + ): void => { + if (!ws || ws.readyState !== WebSocket.OPEN) { + send(clientId, { + type: "response", + id: req.id, + error: { code: -32000, message: "Chrome not connected" }, + }); + return; + } + + const daemonId = mux.allocate(clientId, req.id); + + // Store callback for the response + const timer = setTimeout(() => { + callbacks.delete(daemonId); + mux.resolve(daemonId); + send(clientId, { + type: "response", + id: req.id, + error: { code: -32000, message: `Timeout after ${CDP_COMMAND_TIMEOUT_MS}ms: ${req.method}` }, + }); + }, CDP_COMMAND_TIMEOUT_MS); + + const isAttach = req.method === "Target.attachToTarget"; + const isDetach = req.method === "Target.detachFromTarget"; + + if (isDetach && req.sessionId) { + router.release(req.sessionId); + } + + callbacks.set(daemonId, { send, clientId, localId: req.id, timer, isAttach }); + + const payload: Record = { + id: daemonId, + method: req.method, + params: req.params ?? {}, + }; + if (req.sessionId) payload["sessionId"] = req.sessionId; + + try { + ws.send(JSON.stringify(payload)); + } catch (e) { + clearTimeout(timer); + callbacks.delete(daemonId); + mux.resolve(daemonId); + send(clientId, { + type: "response", + id: req.id, + error: { code: -32000, message: e instanceof Error ? e.message : String(e) }, + }); + } + }; + + return { + start, + stop, + handleRequest, + isAlive: () => ws !== null && ws.readyState === WebSocket.OPEN, + recordSession: (cid, sid) => router.record(cid, sid), + releaseSession: (sid) => router.release(sid), + getSessionOwner: (sid) => router.getOwner(sid), + removeClient: (cid) => { mux.clearClient(cid); router.removeClient(cid); }, + onEvent: (h) => { eventHandler = h; }, + onClose: (h) => { closeHandler = h; }, + }; +}; diff --git a/src/daemon/index.ts b/src/daemon/index.ts new file mode 100644 index 0000000..d2c43a8 --- /dev/null +++ b/src/daemon/index.ts @@ -0,0 +1,114 @@ +/** + * Browser daemon entry point — standalone long-lived process. + * + * Owns a single persistent WebSocket to Chrome. Exposes a Unix domain socket + * for pi instances to connect through. One Allow dialog ever (per Chrome + * lifetime), zero repeated prompts. + * + * Run: npx tsx src/daemon/index.ts + */ + +import { createIpcServer } from "./server"; +import { createCdpBridge, type SendToClient } from "./bridge"; +import { DAEMON_IDLE_TIMEOUT_MS } from "./protocol"; + +async function main() { + console.log("[pi-browser-daemon] Starting..."); + + const ipcServer = createIpcServer(); + const cdpBridge = createCdpBridge(); + + // ── Wire IpcServer → CdpBridge ───────────────────────────────────────── + + // When a pi client sends a CDP request, proxy it through the bridge to Chrome. + ipcServer.onMessage((msg, client) => { + if (msg.type !== "request") return; + + const send: SendToClient = (cid, resp) => { + ipcServer.send(cid, resp); + }; + + cdpBridge.handleRequest(msg, client.id, send); + }); + + // When Chrome sends an event, route it to the correct pi client(s). + cdpBridge.onEvent((event, targetClientIds) => { + for (const cid of targetClientIds) { + ipcServer.send(cid, event); + } + }); + + // When a client disconnects, clean up its CDP state. + ipcServer.onDisconnect((client) => { + cdpBridge.removeClient(client.id); + }); + + // When Chrome disconnects, notify all clients. + cdpBridge.onClose(() => { + console.log("[pi-browser-daemon] Chrome disconnected"); + ipcServer.broadcast({ + type: "control", + action: "shutdown", + reason: "chrome_disconnected", + }); + }); + + // ── Idle timeout ─────────────────────────────────────────────────────── + + // The daemon exits after DAEMON_IDLE_TIMEOUT_MS with zero connected clients. + // This prevents zombie processes when pi sessions end and never reconnect. + // The timeout resets whenever a client connects. + let idleTimer: ReturnType | null = null; + + const resetIdleTimer = (): void => { + if (idleTimer) clearTimeout(idleTimer); + if (ipcServer.clientCount() === 0) { + idleTimer = setTimeout(() => { + console.log("[pi-browser-daemon] Idle timeout — no clients for " + + `${DAEMON_IDLE_TIMEOUT_MS / 60000} minutes. Shutting down.`); + shutdown(); + }, DAEMON_IDLE_TIMEOUT_MS); + } + }; + + const cancelIdleTimer = (): void => { + if (idleTimer) { clearTimeout(idleTimer); idleTimer = null; } + }; + + ipcServer.onConnect(() => { cancelIdleTimer(); }); + ipcServer.onDisconnect(() => { resetIdleTimer(); }); + + // ── Start ────────────────────────────────────────────────────────────── + + try { + await ipcServer.start(); + console.log("[pi-browser-daemon] IPC server listening"); + } catch (e) { + console.error("[pi-browser-daemon] Failed to start IPC server:", e); + process.exit(1); + } + + // Connect to Chrome (auto-retries with backoff in the background). + // The daemon is ready immediately — Chrome may connect later. + await cdpBridge.start(); + + // Start the idle timer (no clients yet at startup). + resetIdleTimer(); + + // ── Graceful shutdown ────────────────────────────────────────────────── + + const shutdown = async () => { + console.log("[pi-browser-daemon] Shutting down..."); + await cdpBridge.stop(); + await ipcServer.stop(); + process.exit(0); + }; + + process.on("SIGINT", shutdown); + process.on("SIGTERM", shutdown); +} + +main().catch((e) => { + console.error("[pi-browser-daemon] Fatal:", e); + process.exit(1); +}); diff --git a/src/daemon/protocol.ts b/src/daemon/protocol.ts new file mode 100644 index 0000000..9d777bc --- /dev/null +++ b/src/daemon/protocol.ts @@ -0,0 +1,162 @@ +/** + * Unix socket daemon protocol — wire message types, type guards, and constants. + * + * The daemon speaks JSON-line protocol over a Unix domain socket. + * One JSON object per newline. Four message types: request, response, event, control. + * + * References: + * - CDP message format: https://github.com/aslushnikov/getting-started-with-cdp#protocol-fundamentals + * - net.createServer IPC: https://nodejs.org/docs/latest-v22.x/api/net.html#serverlistenpath-backlog-callback + * - readline line-delimited: https://nodejs.org/docs/latest-v22.x/api/readline.html#event-line + */ + +// ── Message types ────────────────────────────────────────────────────────────── + +/** A CDP command sent from a pi client to the daemon. */ +export type WireRequest = { + readonly type: "request"; + /** Client-local CDP id. The daemon remaps this to a globally-unique id for Chrome. */ + readonly id: number; + readonly method: string; + readonly params?: Record; + /** Optional CDP session id. Omitted for browser-level commands. */ + readonly sessionId?: string; +}; + +/** A CDP response (result or error) sent from the daemon back to the pi client. */ +export type WireResponse = { + readonly type: "response"; + /** Matches the id from the corresponding WireRequest. */ + readonly id: number; + readonly result?: unknown; + readonly error?: { + readonly code: number; + readonly message: string; + readonly data?: string; + }; +}; + +/** A CDP event forwarded from Chrome through the daemon to pi client(s). */ +export type WireEvent = { + readonly type: "event"; + readonly method: string; + readonly params?: Record; + /** Session-scoped events carry this; browser-level events omit it. */ + readonly sessionId?: string; +}; + +/** Daemon lifecycle control messages. Bidirectional. */ +export type WireControl = { + readonly type: "control"; + readonly action: "register" | "registered" | "deregister" | "shutdown"; + /** pi client namespace (required for register/registered/deregister). */ + readonly clientId?: string; + /** Human-readable reason for shutdown. */ + readonly reason?: string; +}; + +/** All messages that may appear on the Unix socket. */ +export type WireMessage = WireRequest | WireResponse | WireEvent | WireControl; + +// ── Type guard ───────────────────────────────────────────────────────────────── + +const isObject = (v: unknown): v is Readonly> => + typeof v === "object" && v !== null && !Array.isArray(v); + +/** + * Validates that an unknown parsed-JSON value is a well-formed WireMessage. + * Does NOT validate the full CDP semantics — only structural shape. + */ +export const isWireMessage = (v: unknown): v is WireMessage => { + if (!isObject(v)) return false; + const t = v["type"]; + if (typeof t !== "string") return false; + + switch (t) { + case "request": { + if (typeof v["id"] !== "number") return false; + if (typeof v["method"] !== "string") return false; + const sid = v["sessionId"]; + if (sid !== undefined && typeof sid !== "string") return false; + return true; + } + case "response": { + if (typeof v["id"] !== "number") return false; + const errVal = v["error"]; + if (errVal !== undefined) { + if (!isObject(errVal)) return false; + if (typeof errVal["message"] !== "string") return false; + if (typeof errVal["code"] !== "number") return false; + } + return true; + } + case "event": { + if (typeof v["method"] !== "string") return false; + const sid = v["sessionId"]; + if (sid !== undefined && typeof sid !== "string") return false; + return true; + } + case "control": { + const action = v["action"]; + if (typeof action !== "string") return false; + if (!["register", "registered", "deregister", "shutdown"].includes(action)) return false; + const cid = v["clientId"]; + if ( + (action === "register" || action === "registered" || action === "deregister") && + typeof cid !== "string" + ) { + return false; + } + return true; + } + default: + return false; + } +}; + +// ── Wire message constructors ────────────────────────────────────────────────── + +/** Serialize a WireMessage to a JSON line (no trailing newline — caller adds \\n). */ +export const serialize = (msg: WireMessage): string => JSON.stringify(msg); + +/** Deserialize a single JSON-line string to a WireMessage. Returns null on parse failure. */ +export const deserialize = (line: string): WireMessage | null => { + let parsed: unknown; + try { + parsed = JSON.parse(line); + } catch { + return null; + } + if (!isWireMessage(parsed)) return null; + return parsed; +}; + +// ── Constants ────────────────────────────────────────────────────────────────── + +/** Filesystem path for the daemon's Unix domain socket. */ +export const DAEMON_SOCKET_PATH = "/tmp/pi-browser-daemon.sock"; + +/** + * How long the daemon stays alive with zero connected clients before exiting + * (milliseconds). After this period, the daemon cleans up and shuts down. + */ +export const DAEMON_IDLE_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes + +/** + * Hard cap on concurrent pi client connections. Extra connections are rejected + * immediately to prevent resource exhaustion. + */ +export const DAEMON_MAX_CLIENTS = 16; + +/** + * If true, the daemon unlinks (deletes) any stale socket file from a previous + * daemon crash before calling server.listen(). Without this, a crashed daemon + * leaves a socket inode that blocks the next listen() call with EADDRINUSE. + */ +export const DAEMON_STALE_SOCKET_CLEANUP = true; + +/** CDP WebSocket connection timeout (ms). */ +export const CDP_CONNECT_TIMEOUT_MS = 10_000; + +/** CDP command timeout when proxying through daemon (ms). */ +export const CDP_COMMAND_TIMEOUT_MS = 10_000; diff --git a/src/daemon/server.ts b/src/daemon/server.ts new file mode 100644 index 0000000..fea9bb7 --- /dev/null +++ b/src/daemon/server.ts @@ -0,0 +1,245 @@ +/** + * IpcServer — Unix socket server for daemon ↔ pi client communication. + * + * Accepts multiple concurrent pi client connections over a Unix domain + * stream socket. Uses newline-delimited JSON framing (one WireMessage per + * line). Clients are anonymous until they send a control.register message. + * + * References: + * - net.createServer: https://nodejs.org/docs/latest-v22.x/api/net.html#netcreateserveroptions-connectionlistener + * - server.listen(path): https://nodejs.org/docs/latest-v22.x/api/net.html#serverlistenpath-backlog-callback + * - readline.createInterface: https://nodejs.org/docs/latest-v22.x/api/readline.html#readlinecreateinterfaceoptions + */ + +import { createServer, type Server, type Socket } from "node:net"; +import { createInterface, type Interface } from "node:readline"; +import { unlinkSync } from "node:fs"; +import { + type WireMessage, + type WireControl, + isWireMessage, + DAEMON_SOCKET_PATH, + DAEMON_MAX_CLIENTS, + DAEMON_STALE_SOCKET_CLEANUP, +} from "./protocol"; + +// ── Types ────────────────────────────────────────────────────────────────────── + +/** A connected pi client with its socket and readline interface. */ +export type ClientSocket = { + /** Client namespace (set after registration). */ + id: string; + readonly socket: Socket; + readonly rl: Interface; + /** True once a valid control.register message has been received. */ + registered: boolean; +}; + +/** Handler called for every valid WireMessage from any client. */ +export type MessageHandler = (msg: WireMessage, client: ClientSocket) => void; + +/** Handler called when a client connects or disconnects. */ +export type ConnectionHandler = (client: ClientSocket) => void; + +export type IpcServer = { + /** Start listening on the Unix socket. Resolves when the server is ready. */ + start(): Promise; + /** Close the server and all client connections. Resolves when cleaned up. */ + stop(): Promise; + /** Register a handler for incoming WireMessages from any client. */ + onMessage(handler: MessageHandler): void; + /** Register a handler for client connect events. */ + onConnect(handler: ConnectionHandler): void; + /** Register a handler for client disconnect events. */ + onDisconnect(handler: ConnectionHandler): void; + /** Send a WireMessage to a specific client by clientId. No-op if client not found. */ + send(clientId: string, msg: WireMessage): void; + /** Send a WireMessage to all connected (registered) clients. */ + broadcast(msg: WireMessage): void; + /** Force-disconnect a client by clientId. */ + disconnectClient(clientId: string): void; + /** All currently connected clients (by clientId). */ + clients(): ReadonlyMap; + /** Number of connected clients. */ + clientCount(): number; +}; + +// ── Implementation ───────────────────────────────────────────────────────────── + +export const createIpcServer = (): IpcServer => { + let server: Server | null = null; + // Anonymous temp-id → ClientSocket. Temp ids are assigned before registration. + const sockets = new Map(); + // clientId → ClientSocket (only registered clients). + const clients = new Map(); + let messageHandler: MessageHandler | null = null; + let connectHandler: ConnectionHandler | null = null; + let disconnectHandler: ConnectionHandler | null = null; + let nextTempId = 1; + + const start = (): Promise => + new Promise((resolve, reject) => { + // Clean up stale socket file from a previous crashed daemon. + if (DAEMON_STALE_SOCKET_CLEANUP) { + try { + unlinkSync(DAEMON_SOCKET_PATH); + } catch { + // ENOENT is expected — no stale socket to clean. + } + } + + server = createServer({ pauseOnConnect: false }, (socket: Socket) => { + // Reject if at capacity (check registered clients only). + if (clients.size >= DAEMON_MAX_CLIENTS) { + socket.destroy(); + return; + } + + const tempId = `anon-${nextTempId++}`; + const rl = createInterface({ input: socket, crlfDelay: Infinity }); + let currentClient: ClientSocket = { id: tempId, socket, rl, registered: false }; + + sockets.set(tempId, currentClient); + + // Buffer for partial lines. readline handles line splitting, but + // we store a reference in case we need it for diagnostics. + let lineCount = 0; + + rl.on("line", (line: string) => { + lineCount++; + let msg: WireMessage | null; + try { + const parsed: unknown = JSON.parse(line); + if (!isWireMessage(parsed)) return; + msg = parsed; + } catch { + // Malformed JSON — ignore silently, don't crash the daemon. + return; + } + + // Handle registration before forwarding to the general handler. + if (msg.type === "control" && msg.action === "register" && !currentClient.registered) { + const clientId = msg.clientId; + if (!clientId) return; + + // If a client with this id is already connected, reject. + if (clients.has(clientId)) { + const reply: WireControl = { + type: "control", + action: "shutdown", + reason: `clientId ${clientId} is already connected`, + }; + socket.write(JSON.stringify(reply) + "\n"); + socket.destroy(); + return; + } + + // Promote from anonymous to registered — mutate in place so the + // closure captures the updated client reference. + sockets.delete(tempId); + currentClient.id = clientId; + currentClient.registered = true; + clients.set(clientId, currentClient); + + // Acknowledge registration. + const ack: WireControl = { type: "control", action: "registered", clientId }; + socket.write(JSON.stringify(ack) + "\n"); + + connectHandler?.(currentClient); + return; + } + + // Require registration for non-control messages. + if (!currentClient.registered) return; + + // Forward to the general handler. + messageHandler?.(msg, currentClient); + }); + + socket.on("error", () => { + // Socket errors (ECONNRESET, etc.) are handled by the 'close' event. + }); + + socket.on("close", () => { + // Clean up whichever map the client is in. + const found = clients.get(currentClient.id) ?? sockets.get(tempId); + if (found) { + clients.delete(found.id); + sockets.delete(tempId); + // Notify if this was a registered client. + if (found.registered) { + disconnectHandler?.(found); + } + } + // Clean up readline to prevent memory leaks. + rl.close(); + }); + }); + + server.on("error", (err: NodeJS.ErrnoException) => { + reject(err); + }); + + server.listen(DAEMON_SOCKET_PATH, () => { + resolve(); + }); + }); + + const stop = (): Promise => + new Promise((resolve) => { + // Close all client sockets. + for (const [, client] of clients) { + try { client.socket.destroy(); } catch { /* best-effort */ } + } + for (const [, client] of sockets) { + try { client.socket.destroy(); } catch { /* best-effort */ } + } + clients.clear(); + sockets.clear(); + + if (server) { + server.close(() => { + // Clean up the socket file. + try { unlinkSync(DAEMON_SOCKET_PATH); } catch { /* ENOENT ok */ } + resolve(); + }); + } else { + resolve(); + } + }); + + return { + start, + stop, + onMessage(handler) { + messageHandler = handler; + }, + onConnect(handler) { + connectHandler = handler; + }, + onDisconnect(handler) { + disconnectHandler = handler; + }, + send(clientId, msg) { + const client = clients.get(clientId); + if (!client) return; + try { + client.socket.write(JSON.stringify(msg) + "\n"); + } catch { + // Socket write error — the close handler will clean up. + } + }, + broadcast(msg) { + for (const [clientId] of clients) { + this.send(clientId, msg); + } + }, + disconnectClient(clientId) { + const client = clients.get(clientId); + if (!client) return; + client.socket.destroy(); + }, + clients: (): ReadonlyMap => clients, + clientCount: (): number => clients.size, + }; +}; diff --git a/src/daemon/spawn.ts b/src/daemon/spawn.ts new file mode 100644 index 0000000..e9f4d51 --- /dev/null +++ b/src/daemon/spawn.ts @@ -0,0 +1,85 @@ +/** + * Daemon lifecycle helpers — spawn detection and lazy launch. + * + * The daemon is a standalone long-lived process that owns the single WebSocket + * to Chrome. pi instances connect to it via a Unix domain socket. This module + * provides the plumbing to detect a running daemon and spawn one if needed. + * + * Reference: + * - child_process.spawn detached: https://nodejs.org/docs/latest-v22.x/api/child_process.html#optionsdetached + */ + +import { spawn, type ChildProcess } from "node:child_process"; +import { access } from "node:fs/promises"; +import { existsSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; +import { DAEMON_SOCKET_PATH } from "./protocol"; + +const moduleDir = dirname(fileURLToPath(import.meta.url)); + +/** + * Check whether the daemon is running by testing if its Unix socket exists. + * The socket file is created by the daemon on startup and removed on shutdown. + */ +export const isDaemonRunning = async (): Promise => { + try { + await access(DAEMON_SOCKET_PATH); + return true; + } catch { + return false; + } +}; + +/** + * Spawn the daemon as a detached child process. The daemon will continue + * running after the parent pi process exits. + * + * The daemon entry point is resolved relative to this module. When running + * from source (tsx), the entry is src/daemon/index.ts. When running compiled, + * it's dist/daemon/index.js. + * + * Returns the child process handle (caller should call child.unref()). + */ +export const spawnDaemon = (): ChildProcess | null => { + // Resolve the daemon entry point and tsx binary. + const daemonScript = join(moduleDir, "index.ts"); + const tsxBin = join(moduleDir, "..", "..", "node_modules", ".bin", "tsx"); + + // Use tsx to run the daemon TypeScript source directly. + // The spawed process inherits the parent's env so module resolution works. + // Fall back to `npx tsx` if the local binary isn't available. + const cmd = existsSync(tsxBin) ? tsxBin : "npx"; + const args = existsSync(tsxBin) ? [daemonScript] : ["tsx", daemonScript]; + + const child = spawn(cmd, args, { + detached: true, + stdio: "ignore", + env: { ...process.env }, + }); + + child.unref(); + return child; +}; + +/** + * Ensure the daemon is running. If not, spawn it and wait (up to timeoutMs) + * for the socket file to appear. + * + * Returns true if the daemon is running (or was successfully spawned). + */ +export const ensureDaemon = async (timeoutMs = 10_000): Promise => { + if (await isDaemonRunning()) return true; + + const child = spawnDaemon(); + if (!child) return false; + + // Poll for the socket file to appear + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (await isDaemonRunning()) return true; + await new Promise((r) => setTimeout(r, 200)); + } + + return false; +}; diff --git a/src/domains/navigate.ts b/src/domains/navigate.ts index 7012d15..4477485 100644 --- a/src/domains/navigate.ts +++ b/src/domains/navigate.ts @@ -3,6 +3,7 @@ import { Text } from "@mariozechner/pi-tui"; import { type Result, err, ok } from "../util/result"; import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; import { applyTruncation } from "../util/truncate"; +import { safeJs } from "../util/js-template"; const NavigateArgs = Type.Object({ url: Type.String({ description: "Full URL to navigate to (e.g. https://github.com)" }), @@ -82,11 +83,18 @@ export const openUrlsTool = defineBrowserTool({ renderCall: (a) => new Text(`🌐 Opening ${a.urls.length} URL${a.urls.length !== 1 ? "s" : ""}…`, 0, 0), async handler(args, { client, onUpdate }): Promise> { const total = args.urls.length; - // Phase 1: create tabs in parallel. + // Phase 1: create tabs in parallel. Own them so they appear in + // browser_list_tabs scope:'owned' and group in the harness window. + const hw = client.ownership().harnessWindow(); const created = await Promise.all(args.urls.map(async (url): Promise => { - const r = await client.session().callBrowser("Target.createTarget", { url: "about:blank" }); + const params: Record = { url: "about:blank" }; + if (hw) params["openerId"] = hw; + else params["newWindow"] = true; + const r = await client.session().callBrowser("Target.createTarget", params); if (!r.success) return { url, targetId: "", ok: false, error: r.error.message }; - const c = r.data as { targetId: string }; // CDP boundary cast: Target.createTarget returns { targetId: string } + const c = r.data as { targetId: string }; + if (params["newWindow"]) client.ownership().setHarnessWindow(c.targetId); + client.ownership().add(c.targetId); return { url, targetId: c.targetId, ok: true }; })); // Phase 2: attach + navigate each created tab in parallel. @@ -101,6 +109,12 @@ export const openUrlsTool = defineBrowserTool({ if (!enabled.success) return { ...tab, ok: false, error: enabled.error.message }; const nav = await client.session().callOnTarget("Page.navigate", { url: tab.url }, sid); if (!nav.success) return { ...tab, ok: false, error: nav.error.message }; + // Mark the tab with a green circle so the user can see which + // tabs the agent opened. Best-effort; don't fail on CSP blocks. + client.evaluateJs( + safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, + sid + ).catch(() => {}); return tab; } finally { completed++; diff --git a/src/domains/setup.ts b/src/domains/setup.ts new file mode 100644 index 0000000..bbd767f --- /dev/null +++ b/src/domains/setup.ts @@ -0,0 +1,41 @@ +/** + * browser_setup tool — agent-callable browser initialization. + * + * The agent can call this directly when browser tools fail with + * "not initialized" instead of asking the user. Idempotent: safe + * to call even when already connected. + */ + +import { Type } from "typebox"; +import { Text } from "@mariozechner/pi-tui"; +import type { Result } from "../util/result"; +import { err, ok } from "../util/result"; +import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool"; +import { performSetup } from "../setup"; + +const SetupArgs = Type.Object({}); + +export const setupTool = defineBrowserTool({ + name: "browser_setup", + label: "Browser Setup", + description: + "Initialize browser control. Spawns the daemon, connects to Chrome, opens a test tab. " + + "Call when browser tools fail with 'Browser harness not initialized'. Idempotent.", + promptSnippet: "Initialize browser connection", + promptGuidelines: [ + "Call browser_setup when you get a 'Browser harness not initialized' error from any browser tool.", + "This tool is idempotent — calling it when already connected is harmless.", + "After browser_setup succeeds, retry the browser tool that failed.", + ], + parameters: SetupArgs, + // ensureAlive:false — this tool IS the setup; it must bypass the socket guard + ensureAlive: false, + renderCall: () => new Text("🔧 Initializing browser...", 0, 0), + async handler(_args, { client }): Promise> { + const result = await performSetup(client); + if (result.success) { + return ok({ text: result.data }); + } + return err({ kind: "internal", message: result.error }); + }, +}); diff --git a/src/index.ts b/src/index.ts index c2aa2d0..7265263 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ import { registerSetupCommand } from "./setup"; import { type BrowserState, defaultState, persistState, restoreState } from "./state"; import { registerAllTools } from "./registry"; import { cleanupTempDirs } from "./util/truncate"; +import { createDaemonTransport } from "./cdp/daemon-transport"; export default function browserHarnessExtension(pi: ExtensionAPI): void { const flagNs = pi.getFlag("browser-namespace") as string | undefined; @@ -98,25 +99,43 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { pi.on("session_start", async (_event, ctx) => { state = restoreState(ctx, state.namespace); - const initialOwnership: { ownedTargetIds?: ReadonlyArray; harnessWindowTargetId?: string } = {}; - if (state.ownedTargetIds !== undefined) initialOwnership.ownedTargetIds = state.ownedTargetIds; - if (state.harnessWindowTargetId !== undefined) initialOwnership.harnessWindowTargetId = state.harnessWindowTargetId; - client = createBrowserClient({ - namespace: state.namespace, - ...(Object.keys(initialOwnership).length > 0 ? { initialOwnership } : {}), - onOwnershipChange: (snap) => { - state = { - ...state, - ownedTargetIds: snap.ownedTargetIds, - ...(snap.harnessWindowTargetId !== undefined - ? { harnessWindowTargetId: snap.harnessWindowTargetId } - : {}), - }; - persistState(pi, state); - }, - }); - // failure is fine — surfaced via tool errors when the agent first tries to use the browser - await client.start(); + + // Create the client lazily on first session. The daemon transport + // is created but NOT connected — that only happens when the user + // runs /browser-setup or a browser tool call finds an existing socket. + // This means zero Chrome prompts for non-browser pi sessions. + if (!client) { + const transport = createDaemonTransport(state.namespace); + + const initialOwnership: { ownedTargetIds?: ReadonlyArray; harnessWindowTargetId?: string } = {}; + if (state.ownedTargetIds !== undefined) initialOwnership.ownedTargetIds = state.ownedTargetIds; + if (state.harnessWindowTargetId !== undefined) initialOwnership.harnessWindowTargetId = state.harnessWindowTargetId; + + client = createBrowserClient({ + namespace: state.namespace, + transport, + ...(Object.keys(initialOwnership).length > 0 ? { initialOwnership } : {}), + onOwnershipChange: (snap) => { + state = { + ...state, + ownedTargetIds: snap.ownedTargetIds, + ...(snap.harnessWindowTargetId !== undefined + ? { harnessWindowTargetId: snap.harnessWindowTargetId } + : {}), + }; + // Ownership changes can fire from CDP events at any time, + // including after session replacement when pi's ctx is stale. + // Persistence is best-effort — failure must not crash the event consumer. + // ponytail: try/catch is the simplest guard against stale-ctx. + try { persistState(pi, state); } catch { /* stale ctx — safe to ignore */ } + }, + }); + } + + // Do NOT call client.start() here — browser control is on-demand. + // The first browser tool call or /browser-setup triggers the actual + // daemon socket connection and Chrome prompt. + if (!toolsRegistered) { registerAllTools(pi, client); toolsRegistered = true; @@ -132,15 +151,17 @@ export default function browserHarnessExtension(pi: ExtensionAPI): void { persistState(pi, state); if (client) { try { - await client.stop(); + // Detach from the page target (removes the "Chrome is being controlled" + // banner) but keep the transport alive for the next session. + await client.detach(); } catch (e) { - // Shutdown is best-effort, but a stuck stop() points at a transport - // bug worth surfacing for debugging. - console.warn("[pi-browser-harness] client.stop() failed during shutdown:", e); + console.warn("[pi-browser-harness] client.detach() failed during shutdown:", e); } - client = null; + // ponytail: keep the transport alive across sessions. + // Do NOT call client.stop() or null out client — the daemon connection + // persists and eliminates the per-session "Allow Remote Debugging" prompt. + // Only Chrome restart or daemon death triggers a new prompt. } - toolsRegistered = false; await cleanupTempDirs(); }); diff --git a/src/registry.ts b/src/registry.ts index 8f625ae..fcdb66a 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -17,11 +17,14 @@ import { httpGetTool, networkRequestsTool } from "./domains/network"; import { consoleTool } from "./domains/console"; import { snapshotTool } from "./domains/snapshot"; import { executeJsTool, runScriptTool } from "./domains/js"; +import { setupTool } from "./domains/setup"; // Mutation tools that modify page / browser state are marked `serialized: true` // so they run through a shared async mutex. Observation / read-only tools are // left unmarked and can execute in parallel. const TOOLS: ReadonlyArray = [ + // setup comes first — the agent calls this to self-recover from not-initialized + setupTool, { ...clickTool, serialized: true }, { ...typeTool, serialized: true }, { ...pressKeyTool, serialized: true }, diff --git a/src/setup.ts b/src/setup.ts index 3909603..561a343 100644 --- a/src/setup.ts +++ b/src/setup.ts @@ -1,37 +1,50 @@ /** - * Setup command — verifies Chrome is running with remote debugging - * and connects the pi agent to it. + * Setup — verifies Chrome is running with remote debugging and connects + * the pi agent to it. Exposed as both a slash command (/browser-setup) for + * users and a tool (browser_setup) for the agent to self-recover. */ -import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-agent"; +import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; import { execSync } from "node:child_process"; import type { BrowserClient } from "./client"; +import { ensureDaemon } from "./daemon/spawn"; + +// ── Public: register the /browser-setup slash command ────────────────────── export function registerSetupCommand(pi: ExtensionAPI, client: BrowserClient): void { pi.registerCommand("browser-setup", { description: "Connect pi to your Chrome browser", handler: async (_args, ctx) => { - await runSetup(ctx, client); + const result = await performSetup(client); + if (result.success) { + ctx.ui.notify(result.data, "info"); + } else { + ctx.ui.notify(result.error, "error"); + } }, }); } -async function runSetup(ctx: ExtensionContext, client: BrowserClient): Promise { - ctx.ui.notify("browser setup: checking Chrome...", "info"); +// ── Shared setup result ──────────────────────────────────────────────────── + +export type SetupResult = { success: true; data: string } | { success: false; error: string }; + +// ── Shared setup logic (used by both the command and the tool) ───────────── +export async function performSetup(client: BrowserClient): Promise { // Step 1: Check Chrome is running const chromeRunning = checkChromeRunning(); if (!chromeRunning) { - ctx.ui.notify( - "No browser instance running. Please open your browser and then run /browser-setup.", - "error", - ); - return; + return { success: false, error: "No browser instance running. Please open your browser and then run /browser-setup." }; } - ctx.ui.notify("Chrome is running ✓", "info"); - // Step 2: Try to connect to Chrome DevTools - ctx.ui.notify("Connecting to Chrome DevTools...", "info"); + // Step 2: Start the browser daemon (spawns if not running, silently reuses if alive) + const daemonReady = await ensureDaemon(); + if (!daemonReady) { + return { success: false, error: "Could not start the browser daemon. Check /tmp/pi-browser-daemon.sock." }; + } + + // Step 3: Connect to Chrome DevTools const startResult = await client.start(); if (!startResult.success) { const msg = startResult.error.message; @@ -43,30 +56,23 @@ async function runSetup(ctx: ExtensionContext, client: BrowserClient): Promise l.trim()).filter(Boolean); - const browserProcesses = ["google chrome", "chromium", "microsoft edge"]; return lines.some((l) => { - if (l.includes("helper") || l.includes("renderer")) return false; - return browserProcesses.includes(l); + if (l.includes("helper") || l.includes("renderer") || l.includes("crashpad")) return false; + return l.includes("google chrome") || l.includes("chromium") || l.includes("microsoft edge"); }); } else if (process.platform === "linux") { - // Use args to distinguish the main browser process from sub-processes - // (GPU, renderer, utility, etc.) which carry --type= flags. const out = execSync("ps -A -o comm=,args=", { timeout: 5000 }).toString().toLowerCase(); const lines = out.split("\n").map((l) => l.trim()).filter(Boolean); const browserComms = ["chrome", "chromium", "chromium-browser", "msedge", "microsoft-edge", "google-chrome"]; diff --git a/src/util/tool.ts b/src/util/tool.ts index c2bb3f8..1e33c43 100644 --- a/src/util/tool.ts +++ b/src/util/tool.ts @@ -9,6 +9,8 @@ import type { ToolRenderResultOptions, } from "@mariozechner/pi-coding-agent"; import type { Component } from "@mariozechner/pi-tui"; +import { access } from "node:fs/promises"; +import { DAEMON_SOCKET_PATH } from "../daemon/protocol"; import type { BrowserClient } from "../client"; import type { Result } from "./result"; @@ -142,6 +144,23 @@ export const registerBrowserTool = ( : {}), async execute(_toolCallId, args, signal, onUpdate, extensionCtx) { if (def.ensureAlive !== false) { + // Check daemon socket exists before attempting browser control. + // If missing, the user hasn't run /browser-setup yet. Return a clear + // error so the agent knows to direct the user to initialize. + try { + await access(DAEMON_SOCKET_PATH); + } catch { + return toToolResult( + { + success: false, + error: { + kind: "not_connected", + message: "Browser harness not initialized. Run /browser-setup first", + }, + }, + def.name, + ); + } const alive = await client.ensureAlive(); if (!alive.success) { return toToolResult( diff --git a/test/manual/bridge-test.ts b/test/manual/bridge-test.ts new file mode 100644 index 0000000..ecb4100 --- /dev/null +++ b/test/manual/bridge-test.ts @@ -0,0 +1,182 @@ +/** + * Manual integration test for CdpBridge against real Chrome. + * Requires: Chrome with remote debugging enabled (chrome://inspect/#remote-debugging). + * Run: npx tsx test/manual/bridge-test.ts + */ +import { createCdpBridge, type SendToClient, type CdpBridge } from "../../src/daemon/bridge"; +import type { WireRequest, WireResponse, WireEvent } from "../../src/daemon/protocol"; + +let passed = 0; +let failed = 0; +const check = (cond: boolean, label: string) => { + if (cond) { passed++; console.log(` ✓ ${label}`); } + else { failed++; console.error(` ✗ ${label}`); } +}; + +async function main() { + console.log("Starting CdpBridge..."); + const bridge = createCdpBridge(); + + // ── Test 1: Connect to Chrome ────────────────────────────────────────── + // start() now runs auto-retry in the background. Wait a moment for + // the initial connection attempt to complete. + await bridge.start(); + await sleep(3000); // allow time for WebSocket handshake + if (!bridge.isAlive()) { + console.error("Cannot continue — Chrome not reachable. Run /browser-setup first."); + console.error("(The bridge will keep retrying in the background.)"); + process.exit(1); + } + console.log(" ✓ Connect to Chrome: ok"); + check(bridge.isAlive(), "Bridge reports alive after connect"); + + // ── Test 2: Send Target.getTargets ───────────────────────────────────── + const responses: WireResponse[] = []; + const events: WireEvent[] = []; + let eventCount = 0; + + const send: SendToClient = (clientId, msg) => { + if (msg.type === "response") responses.push(msg); + else events.push(msg); + }; + + bridge.onEvent((evt, _targets) => { eventCount++; }); + + const req1: WireRequest = { type: "request", id: 1, method: "Target.getTargets" }; + bridge.handleRequest(req1, "pi-test", send); + + await sleep(2000); + + const res1 = responses.find(r => r.id === 1); + check(res1 != null, "Target.getTargets response received"); + if (res1?.result) { + const r = res1.result as any; + check(Array.isArray(r.targetInfos), "targetInfos is array"); + console.log(` Found ${r.targetInfos?.length ?? 0} targets`); + } + + // ── Test 3: Create a new tab ─────────────────────────────────────────── + responses.length = 0; + const req2: WireRequest = { + type: "request", id: 2, method: "Target.createTarget", + params: { url: "about:blank", newWindow: true }, + }; + bridge.handleRequest(req2, "pi-test", send); + await sleep(2000); + + const res2 = responses.find(r => r.id === 2); + check(res2 != null, "Target.createTarget response received"); + const targetId = (res2?.result as any)?.targetId as string | undefined; + check(typeof targetId === "string", `Created target: ${targetId ?? "FAILED"}`); + + if (!targetId) { + console.error("Cannot continue without a target"); + await bridge.stop(); + process.exit(1); + } + + // ── Test 4: Attach to target ─────────────────────────────────────────── + responses.length = 0; + const req3: WireRequest = { + type: "request", id: 3, method: "Target.attachToTarget", + params: { targetId, flatten: true }, + }; + bridge.handleRequest(req3, "pi-test", send); + await sleep(2000); + + const res3 = responses.find(r => r.id === 3); + check(res3 != null, "Target.attachToTarget response received"); + const sessionId = (res3?.result as any)?.sessionId as string | undefined; + check(typeof sessionId === "string", `Session ID: ${sessionId ?? "FAILED"}`); + + // Verify session ownership was tracked + if (sessionId) { + const owner = bridge.getSessionOwner(sessionId); + check(owner === "pi-test", "Session ownership tracked correctly"); + } + + if (!sessionId) { + await bridge.stop(); + process.exit(1); + } + + // ── Test 5: Navigate tab via session ─────────────────────────────────── + responses.length = 0; + const req4: WireRequest = { + type: "request", id: 4, method: "Page.navigate", + params: { url: "https://example.com" }, + sessionId, + }; + bridge.handleRequest(req4, "pi-test", send); + await sleep(3000); + + const res4 = responses.find(r => r.id === 4); + check(res4 != null, "Page.navigate response received"); + const frameId = (res4?.result as any)?.frameId as string | undefined; + check(typeof frameId === "string", `Navigated to example.com (frame: ${frameId ?? "FAILED"})`); + + // ── Test 6: Evaluate JS in the page ──────────────────────────────────── + responses.length = 0; + const req5: WireRequest = { + type: "request", id: 5, method: "Runtime.evaluate", + params: { expression: "document.title", returnByValue: true }, + sessionId, + }; + bridge.handleRequest(req5, "pi-test", send); + await sleep(2000); + + const res5 = responses.find(r => r.id === 5); + check(res5 != null, "Runtime.evaluate response received"); + const title = (res5?.result as any)?.result?.value as string | undefined; + check(title === "Example Domain", `Page title: "${title ?? "FAILED"}" (expected "Example Domain")`); + + // ── Test 7: Detach from target ───────────────────────────────────────── + responses.length = 0; + const req6: WireRequest = { + type: "request", id: 6, method: "Target.detachFromTarget", + params: { sessionId }, + }; + bridge.handleRequest(req6, "pi-test", send); + await sleep(1000); + + const res6 = responses.find(r => r.id === 6); + check(res6 != null, "Target.detachFromTarget response received"); + + // Session ownership should be released + const ownerAfter = bridge.getSessionOwner(sessionId); + check(ownerAfter === undefined, "Session ownership released after detach"); + + // ── Test 8: Close target ─────────────────────────────────────────────── + responses.length = 0; + const req7: WireRequest = { + type: "request", id: 7, method: "Target.closeTarget", + params: { targetId }, + }; + bridge.handleRequest(req7, "pi-test", send); + await sleep(1000); + + const res7 = responses.find(r => r.id === 7); + check(res7 != null, "Target.closeTarget response received"); + + // ── Test 9: Remove client ────────────────────────────────────────────── + bridge.removeClient("pi-test"); + // After removal, no pending requests should remain — verify by checking + // no errors are thrown when handling subsequent events. + + // ── Cleanup ──────────────────────────────────────────────────────────── + console.log("\nStopping bridge..."); + await bridge.stop(); + console.log("Bridge stopped ✓"); + + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +function sleep(ms: number): Promise { + return new Promise(r => setTimeout(r, ms)); +} + +main().catch((e) => { + console.error("Test failed:", e); + process.exit(1); +}); diff --git a/test/manual/daemon-transport-test.ts b/test/manual/daemon-transport-test.ts new file mode 100644 index 0000000..4ba2f5b --- /dev/null +++ b/test/manual/daemon-transport-test.ts @@ -0,0 +1,117 @@ +/** + * Integration test: IpcServer + DaemonTransport end-to-end. + * Starts a real IpcServer, connects a DaemonTransport, sends CDP requests. + * No Chrome required — the server simulates CDP responses. + * + * Run: npx tsx test/manual/daemon-transport-test.ts + */ +import { createIpcServer } from "../../src/daemon/server"; +import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { DAEMON_SOCKET_PATH, serialize } from "../../src/daemon/protocol"; +import type { WireMessage } from "../../src/daemon/protocol"; +import { unlinkSync } from "node:fs"; + +let passed = 0; +let failed = 0; +const check = (cond: boolean, label: string) => { + if (cond) { passed++; console.log(` ✓ ${label}`); } + else { failed++; console.error(` ✗ ${label}`); } +}; + +async function main() { + // Clean up any stale socket + try { unlinkSync(DAEMON_SOCKET_PATH); } catch {} + + // ── Start IpcServer ──────────────────────────────────────────────────── + console.log("Starting IpcServer..."); + const server = createIpcServer(); + + // When a request arrives, reply with a fake CDP response + server.onMessage((msg, client) => { + if (msg.type === "request") { + // Simulate a CDP response + const reply: WireMessage = { + type: "response", + id: msg.id, + result: { fakeResult: true, method: msg.method }, + }; + server.send(client.id, reply); + } + }); + + await server.start(); + console.log("Server started ✓"); + + // ── Create DaemonTransport ───────────────────────────────────────────── + const transport = createDaemonTransport("pi-test-transport"); + + // ── Test 1: Connect ──────────────────────────────────────────────────── + const connResult = await transport.connect(""); + check(connResult.success, `Connect: ${connResult.success ? "ok" : connResult.error.message}`); + check(transport.state() === "open", `State is "open" (actual: "${transport.state()}")`); + + // ── Test 2: Send a request ───────────────────────────────────────────── + const reqResult = await transport.request("Page.navigate", { url: "https://example.com" }); + check(reqResult.success, `Request succeeded: ${reqResult.success}`); + if (reqResult.success) { + const data = reqResult.data as any; + check(data?.fakeResult === true, "Response has fakeResult: true"); + check(data?.method === "Page.navigate", "Response echoes method"); + } + + // ── Test 3: Send request with sessionId ──────────────────────────────── + const req2 = await transport.request("Runtime.evaluate", { expression: "1+1" }, { sessionId: "abc123" }); + check(req2.success, "Request with sessionId succeeded"); + + // ── Test 4: Events ───────────────────────────────────────────────────── + const eventPromise = new Promise((resolve) => { + const timeout = setTimeout(() => resolve(false), 2000); + (async () => { + for await (const evt of transport.events()) { + clearTimeout(timeout); + resolve(evt.method === "Target.targetCreated"); + break; + } + })(); + }); + + // Simulate server sending an event + server.broadcast({ + type: "event", + method: "Target.targetCreated", + params: { targetInfo: { type: "page", targetId: "test-123" } }, + }); + + const gotEvent = await eventPromise; + check(gotEvent, "Event received via transport.events()"); + + // ── Test 5: Close transport ──────────────────────────────────────────── + let closeFired = false; + transport.onClose(() => { closeFired = true; }); + + await transport.close(); + check(transport.state() === "closed", "State is closed after close()"); + check(closeFired, "onClose handler fired"); + + // ── Test 6: Reconnect ────────────────────────────────────────────────── + await new Promise(r => setTimeout(r, 500)); // let server process socket close + const reconnectResult = await transport.connect(""); + check(reconnectResult.success, `Reconnect: ${reconnectResult.success ? "ok" : reconnectResult.error.message}`); + + // ── Test 7: Request after reconnect ──────────────────────────────────── + const req3 = await transport.request("Browser.getVersion", {}); + check(req3.success, "Request after reconnect succeeded"); + + // ── Cleanup ──────────────────────────────────────────────────────────── + await transport.close(); + await server.stop(); + console.log("Cleanup complete ✓"); + + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +main().catch((e) => { + console.error("Test failed:", e); + process.exit(1); +}); diff --git a/test/manual/e2e-test.ts b/test/manual/e2e-test.ts new file mode 100644 index 0000000..963efaf --- /dev/null +++ b/test/manual/e2e-test.ts @@ -0,0 +1,163 @@ +/** + * End-to-end integration test: full daemon pipeline. + * + * Starts the daemon (IpcServer + CdpBridge), connects via DaemonTransport, + * exercises the full CDP request/response/event lifecycle. + * + * Run: npx tsx test/manual/e2e-test.ts + */ +import { createIpcServer } from "../../src/daemon/server"; +import { createCdpBridge, type SendToClient } from "../../src/daemon/bridge"; +import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { DAEMON_SOCKET_PATH } from "../../src/daemon/protocol"; +import type { WireRequest } from "../../src/daemon/protocol"; +import { unlinkSync } from "node:fs"; + +let passed = 0; +let failed = 0; +const check = (cond: boolean, label: string) => { + if (cond) { passed++; console.log(` ✓ ${label}`); } + else { failed++; console.error(` ✗ ${label}`); } +}; + +async function main() { + // Clean up stale socket + try { unlinkSync(DAEMON_SOCKET_PATH); } catch {} + + // ── Start daemon ─────────────────────────────────────────────────────── + console.log("Starting daemon (IpcServer + CdpBridge)..."); + const ipcServer = createIpcServer(); + const cdpBridge = createCdpBridge(); + + // Wire IpcServer ↔ CdpBridge + ipcServer.onMessage((msg, client) => { + if (msg.type !== "request") return; + const send: SendToClient = (cid, resp) => ipcServer.send(cid, resp); + cdpBridge.handleRequest(msg, client.id, send); + }); + + cdpBridge.onEvent((event, targetClientIds) => { + for (const cid of targetClientIds) ipcServer.send(cid, event); + }); + + ipcServer.onDisconnect((client) => cdpBridge.removeClient(client.id)); + + await ipcServer.start(); + console.log("Daemon started ✓"); + + // The bridge will try to connect to Chrome — might fail, that's ok for + // this test. We'll simulate CDP responses from the server side. + + // ── Connect pi client via DaemonTransport ────────────────────────────── + console.log("\nConnecting pi client (DaemonTransport)..."); + const transport = createDaemonTransport("pi-e2e-test"); + + const connectResult = await transport.connect(""); + check(connectResult.success, `Client connect: ${connectResult.success ? "ok" : connectResult.error.message}`); + check(transport.state() === "open", "Transport state is open"); + + // ── Test 1: Request → simulated CDP response ─────────────────────────── + // The IpcServer's onMessage handler forwards requests to the bridge. + // The bridge tries to send to Chrome (which may be down). For this test, + // we intercept at the IpcServer level and simulate CDP responses. + + // Override the message handler to simulate CDP responses directly. + let rawRequests: WireRequest[] = []; + ipcServer.onMessage((msg, client) => { + if (msg.type === "request") { + rawRequests.push(msg); + // Simulate a CDP success response + ipcServer.send(client.id, { + type: "response", + id: msg.id, + result: { simulated: true, method: msg.method }, + }); + } + }); + + const res1 = await transport.request("Page.navigate", { url: "https://example.com" }); + check(res1.success, "CDP request succeeded"); + if (res1.success) { + const d = res1.data as any; + check(d?.simulated === true, "Response contains simulated flag"); + check(d?.method === "Page.navigate", "Response echoes method"); + } + check(rawRequests.length === 1, "Server received exactly 1 request"); + check(rawRequests[0]!.method === "Page.navigate", "Correct method forwarded"); + + // ── Test 2: Request with sessionId ───────────────────────────────────── + rawRequests.length = 0; + const res2 = await transport.request("Runtime.evaluate", { expression: "1+1" }, { sessionId: "session-abc" }); + check(res2.success, "Session-scoped request succeeded"); + check(rawRequests[0]?.sessionId === "session-abc", "SessionId forwarded correctly"); + + // ── Test 3: Multiple parallel requests ───────────────────────────────── + rawRequests.length = 0; + const [r3a, r3b, r3c] = await Promise.all([ + transport.request("Target.getTargets", {}), + transport.request("Browser.getVersion", {}), + transport.request("Page.captureScreenshot", {}), + ]); + check(r3a.success && r3b.success && r3c.success, "All 3 parallel requests succeeded"); + check(rawRequests.length === 3, "Server received all 3 requests"); + + // ── Test 4: Events ───────────────────────────────────────────────────── + const eventPromise = new Promise((resolve) => { + const timeout = setTimeout(() => resolve(null), 2000); + (async () => { + for await (const evt of transport.events()) { + clearTimeout(timeout); + resolve(evt.method); + break; + } + })(); + }); + + // Simulate a CDP event broadcast from the daemon + ipcServer.broadcast({ + type: "event", + method: "Target.targetCreated", + params: { targetInfo: { type: "page", targetId: "e2e-test-target" } }, + }); + + const eventMethod = await eventPromise; + check(eventMethod === "Target.targetCreated", `Event received: ${eventMethod ?? "TIMEOUT"}`); + + // ── Test 5: Session persistence (detach simulation) ──────────────────── + // Simulate a session shutdown: close the transport, then reconnect. + // The transport.close() sends deregister, server cleans up. + await transport.close(); + check(transport.state() === "closed", "Transport closed"); + + // Small delay for server cleanup + await new Promise(r => setTimeout(r, 500)); + + // Reconnect — simulates next pi session + const reconnectResult = await transport.connect(""); + check(reconnectResult.success, "Transport reconnected (simulated session restart)"); + + // Send a request through the reconnected transport + const res4 = await transport.request("Target.getTargets", {}); + check(res4.success, "Request after reconnect succeeded"); + check(transport.state() === "open", "Transport state is open after reconnect"); + + // ── Test 6: Server-side client count ─────────────────────────────────── + check(ipcServer.clientCount() === 1, "Server reports 1 connected client"); + + // ── Test 7: Client disconnect cleanup ────────────────────────────────── + await transport.close(); + await new Promise(r => setTimeout(r, 500)); + check(ipcServer.clientCount() === 0, "Server reports 0 clients after disconnect"); + + // ── Cleanup ──────────────────────────────────────────────────────────── + await ipcServer.stop(); + console.log("\nDaemon stopped ✓"); + + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +main().catch((e) => { + console.error("E2E test failed:", e); + process.exit(1); +}); diff --git a/test/manual/ipc-server-test.ts b/test/manual/ipc-server-test.ts new file mode 100644 index 0000000..a267a45 --- /dev/null +++ b/test/manual/ipc-server-test.ts @@ -0,0 +1,141 @@ +/** + * Manual smoke test for IpcServer. + * Starts a server, connects a client, tests registration, send, broadcast, disconnect. + * Run: npx tsx test/manual/ipc-server-test.ts + */ +import { createConnection } from "node:net"; +import { createInterface } from "node:readline"; +import { createIpcServer } from "../../src/daemon/server"; +import { DAEMON_SOCKET_PATH } from "../../src/daemon/protocol"; +import type { WireMessage } from "../../src/daemon/protocol"; + +const TEST_CLIENT_ID = "pi-test-001"; + +async function main() { + let passed = 0; + let failed = 0; + const check = (cond: boolean, label: string) => { + if (cond) { passed++; console.log(` ✓ ${label}`); } + else { failed++; console.error(` ✗ ${label}`); } + }; + + // ── Start server ────────────────────────────────────────────────────── + console.log("Starting IpcServer..."); + const server = createIpcServer(); + + const receivedByServer: WireMessage[] = []; + let connectCount = 0; + let disconnectCount = 0; + + server.onMessage((msg) => { receivedByServer.push(msg); }); + server.onConnect(() => { connectCount++; }); + server.onDisconnect(() => { disconnectCount++; }); + + await server.start(); + console.log("Server started ✓"); + + // ── Connect client ──────────────────────────────────────────────────── + console.log("\nConnecting client..."); + const socket = createConnection(DAEMON_SOCKET_PATH); + const rl = createInterface({ input: socket, crlfDelay: Infinity }); + + const receivedByClient: WireMessage[] = []; + rl.on("line", (line) => { + try { receivedByClient.push(JSON.parse(line)); } catch { /* ignore */ } + }); + + await new Promise((resolve) => socket.on("connect", resolve)); + console.log("Client connected ✓"); + + // ── Test 1: Registration ────────────────────────────────────────────── + // Send register, expect registered ack. + socket.write(JSON.stringify({ type: "control", action: "register", clientId: TEST_CLIENT_ID }) + "\n"); + await sleep(200); + + const ack = receivedByClient.find((m: any) => m.type === "control" && m.action === "registered"); + check(ack != null, "Registration acknowledged"); + check(connectCount === 1, "Connect handler fired"); + + // ── Test 2: Send message to server ──────────────────────────────────── + receivedByServer.length = 0; + const testRequest = { type: "request", id: 1, method: "Page.navigate", params: { url: "https://example.com" } }; + socket.write(JSON.stringify(testRequest) + "\n"); + await sleep(200); + + const received = receivedByServer.find((m: any) => m.type === "request" && m.id === 1); + check(received != null, "Server received request from client"); + check((received as any)?.method === "Page.navigate", "Method matches"); + + // ── Test 3: Server → client send ────────────────────────────────────── + receivedByClient.length = 0; + server.send(TEST_CLIENT_ID, { type: "response", id: 1, result: { frameId: "xyz" } }); + await sleep(200); + + const response = receivedByClient.find((m: any) => m.type === "response" && m.id === 1); + check(response != null, "Client received response from server"); + + // ── Test 4: Broadcast ───────────────────────────────────────────────── + receivedByClient.length = 0; + const broadcastMsg = { type: "event", method: "Target.targetCreated", params: { targetInfo: { type: "page" } } }; + server.broadcast(broadcastMsg); + await sleep(200); + + check(receivedByClient.length === 1, "Client received broadcast"); + const bc = receivedByClient[0] as any; + check(bc?.method === "Target.targetCreated", "Broadcast content matches"); + + // ── Test 5: Client disconnect ───────────────────────────────────────── + socket.destroy(); + await sleep(500); + + check(disconnectCount === 1, "Disconnect handler fired"); + check(server.clientCount() === 0, "Client count is 0 after disconnect"); + + // ── Test 6: Re-registration (same clientId after disconnect) ────────── + const socket2 = createConnection(DAEMON_SOCKET_PATH); + const rl2 = createInterface({ input: socket2, crlfDelay: Infinity }); + await new Promise((resolve) => socket2.on("connect", resolve)); + + socket2.write(JSON.stringify({ type: "control", action: "register", clientId: TEST_CLIENT_ID }) + "\n"); + await sleep(200); + check(server.clientCount() === 1, "Re-registration works after disconnect"); + socket2.destroy(); + socket2.on("error", () => {}); // suppress EPIPE from server-side cleanup + await sleep(300); + + // ── Test 7: Max clients enforcement ─────────────────────────────────── + // Connect max clients, then one more should be rejected. + const testSockets: ReturnType[] = []; + for (let i = 0; i < 20; i++) { + const s = createConnection(DAEMON_SOCKET_PATH); + s.on("error", () => {}); // server may reject and destroy this socket + testSockets.push(s); + await new Promise((resolve) => s.on("connect", resolve)); + try { s.write(JSON.stringify({ type: "control", action: "register", clientId: `pi-test-max-${i}` }) + "\n"); } catch {} + await sleep(20); + } + await sleep(300); + check(server.clientCount() <= 16, "Max client enforcement works"); + // Clean up test sockets (some already destroyed by server rejection) + for (const s of testSockets) { + try { s.destroy(); } catch {} + s.removeAllListeners(); + } + + // ── Cleanup ─────────────────────────────────────────────────────────── + console.log(`\nCleaning up...`); + await server.stop(); + console.log("Server stopped ✓"); + + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); +} + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +main().catch((e) => { + console.error("Test failed:", e); + process.exit(1); +}); diff --git a/test/manual/live-daemon-test.ts b/test/manual/live-daemon-test.ts new file mode 100644 index 0000000..28ec7cd --- /dev/null +++ b/test/manual/live-daemon-test.ts @@ -0,0 +1,21 @@ +import { createDaemonTransport } from "../../src/cdp/daemon-transport"; + +async function main() { + const t = createDaemonTransport("pi-live-test"); + const r = await t.connect(""); + console.log("Connect:", r.success ? "ok" : r.error.message); + console.log("State:", t.state()); + + const res = await t.request("Browser.getVersion", {}); + if (res.success) { + const d = res.data as any; + console.log("Browser:", d?.product ?? d); + } else { + console.log("Error:", res.error.message); + } + + await t.close(); + console.log("Done"); +} + +main(); diff --git a/test/manual/socket-guard-test.ts b/test/manual/socket-guard-test.ts new file mode 100644 index 0000000..ab519f9 --- /dev/null +++ b/test/manual/socket-guard-test.ts @@ -0,0 +1,45 @@ +/** + * Test: socket guard produces clear error when daemon not running. + */ + +import { access } from "node:fs/promises"; +import { setTimeout as sleep } from "node:timers/promises"; +import { spawn } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; +import { DAEMON_SOCKET_PATH } from "../../src/daemon/protocol"; + +async function main() { + const sock = DAEMON_SOCKET_PATH; + + // Simulate no daemon + try { + await access(sock); + console.log("FAIL: Socket exists unexpectedly"); + process.exit(1); + } catch { + console.log("✓ Socket missing — tool wrapper returns:"); + console.log(' "Browser harness not initialized. Run /browser-setup first"'); + } + + // Start daemon + const daemonEntry = join(dirname(fileURLToPath(import.meta.url)), "../../src/daemon/index.ts"); + const proc = spawn("npx", ["tsx", daemonEntry], { detached: true, stdio: "ignore" }); + proc.unref(); + await sleep(2000); + + // Now socket should exist + try { + await access(sock); + console.log("✓ Socket exists after daemon spawn — tool wrapper proceeds silently"); + } catch { + console.log("FAIL: Socket still missing after daemon spawn"); + process.exit(1); + } + + // Cleanup + proc.kill(); + console.log("✓ All socket guard checks passed"); +} + +main(); diff --git a/thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md b/thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md deleted file mode 100644 index 9dda4ff..0000000 --- a/thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md +++ /dev/null @@ -1,507 +0,0 @@ ---- -date: 2026-05-07T00:58:00Z -designer: amankumarsingh77 -git_commit: bd19739 -branch: main -repository: pi-browser-harness -topic: "per-tab-isolation-multi-agent-safety" -tags: [design, session, isolation, per-tab, buffers, dialog, safety, concurrency] -status: complete -research_source: "thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md" -last_updated: 2026-05-07 -last_updated_by: amankumarsingh77 ---- - -# Design: Per-Tab Isolation for Multi-Agent Safety - -## Summary - -Add `TabSession` — a per-targetId state object in `session.ts` containing sessionId, targetId, dialog, pageInfoDirty, networkBuffer, and consoleBuffer — stored in a `Map`. The `consumeEvents` CDP event loop gains an `ev.sessionId` filter that routes events to the correct tab's buffer. `switchTo` preserves old tab state instead of clearing buffers. `client.ts` page cache becomes `Map`. Delivered in two phases: Phase 1 (immediate `ev.sessionId` filter) and Phase 2 (full per-tab data structures). Phase 3 (agent-aware tab locking) is deferred. - -## Requirements - -- Tab switching must not clear another tab's network/console buffer data -- Dialog from tab A must not overwrite dialog from tab B -- Page info dirty flag must be per-tab, not global -- Console `sinceSeq` cursor must remain valid across tab switches (no cross-tab data leaks) -- Network buffer data must survive round-trip tab switches (A→B→A) -- Page info cache must be preserved per-tab across switches -- CDP events from unattached sessions must be filtered (no cross-contamination) -- Dead tab state must be pruned on target destruction and session reconciliation - -## Current State Analysis - -### Key Discoveries - -- `src/cdp/session.ts:41-46` — Six mutable singletons: `sessionId`, `targetId`, `dialog`, `pageInfoDirty`, `networkBuffer`, `consoleBuffer`. All overwritten on `switchTo` (`session.ts:125-143`). -- `src/cdp/session.ts:136-137` — `networkBuffer.clear()` and `consoleBuffer.clear()` destroy all prior-tab data on every tab switch. -- `src/cdp/session.ts:33` — `consumeEvents` has no `ev.sessionId` filtering — events from all attached sessions route to the same singletons. -- `src/cdp/console-buffer.ts:256-262` — `clear()` preserves `nextSeq` (claiming safety), but this causes cross-tab data leaks for stale cursors. The fix: reset `nextSeq` on clear. -- `src/cdp/session.ts:150-151` — `callOnTarget()` escape hatch exists but is unused by observation tools — deferred to Phase 3. -- `src/client.ts:85` — Single `pageCache` nullified on any `switchTab` (`client.ts:183`). -- `src/cdp/ownership.ts:38-72` — OwnershipRegistry is the canonical pattern for per-target tracking: add/remove/has/list/replaceAll/onChange factory. - -### Patterns to Follow - -| New structure | Model after | File:Line | -|---|---|---| -| `TabSession` factory | `createOwnershipRegistry` | `ownership.ts:38-72` | -| `Map` | `ownership.ts:29` — `Set` | `ownership.ts:29` | -| Per-tab page cache `Map` | `client.ts:85,168-179` — single `pageCache` | `client.ts:85` | -| Buffer preservation on switch | `d3d86f9` — dialog preserved on close | `session.ts:78-83` | -| Dead TabSession pruning | `session.ts:112-118` — ownership reconciliation | `session.ts:112-118` | - -## Scope - -### Building -- Phase 1: `ev.sessionId` filter in `consumeEvents` — 1-line guard preventing cross-tab event contamination -- Phase 2: `TabSession` type + `Map` in `session.ts` -- Phase 2: Per-tab dialog (move `dialog` into TabSession) -- Phase 2: Per-tab `pageInfoDirty` (move flag into TabSession) -- Phase 2: Per-tab buffers (move `networkBuffer`/`consoleBuffer` into TabSession, preserve on switch) -- Phase 2: Per-tab page cache in `client.ts` (`Map`) -- Phase 2: Console `nextSeq` reset on `clear()` in `console-buffer.ts` -- Phase 2: TabSession cleanup on `Target.targetDestroyed` and reconciliation -- Phase 2: Multi-agent warning in `prompt.ts` Parallel Execution section - -### Not Building -- Agent-aware tab locking (requires pi platform agent identity exposure — Phase 3) -- Observation tool `callOnTarget` refactoring (Phase 3) -- Per-agent BrowserClient instances -- New CDP transport connections per agent - -## Decisions - -### TabSession model — one object per targetId -**Evidence**: `OwnershipRegistry` at `ownership.ts:29` uses a single `Set` with add/remove/has — one collection, not separate Maps per field. Grouping all per-tab state into one `TabSession` object per targetId follows this pattern and simplifies cleanup (one delete removes all state). -**Decision**: Single `TabSession` object per targetId stored in `Map`. - -### ev.sessionId filter — immediate Phase 1 -**Evidence**: Research Finding 9 identifies this as "the single most impactful change." Precedent-locator confirms it's the essential first move. One line in `consumeEvents` prevents dialog overwrites, dirty-flag theft, and buffer pollution. -**Decision**: Phase 1 delivers only this filter. Phase 2 builds per-tab structures on top. - -### Buffers preserved, not cleared on switchTo -**Evidence**: `d3d86f9` established the pattern of NOT clearing dialog on transport close (`session.ts:78-83`) — data survives events that would otherwise destroy it. The same rationale applies to buffers on tab switch. Research confirms buffer clearing invalidates per-agent cursors. -**Decision**: `switchTo` no longer calls `networkBuffer.clear()` or `consoleBuffer.clear()`. Each tab's buffers persist in its TabSession. - -### nextSeq reset on clear -**Evidence**: `console-buffer.ts:256-262` comment claims monotonic `nextSeq` prevents stale cursor matches — research Finding 4 proves the opposite. With per-tab buffers, each tab has independent seq space, eliminating the need for global monotonicity. -**Decision**: `clear()` resets `nextSeq` to 1. - -### drain methods default to current tab — no API break -**Evidence**: Integration scanner confirms domain tools call `session.drainNetworkBuffer()`, `session.drainConsoleBuffer()`, `client.pageInfo()` which calls `session.takeDialog()` and `session.drainPageInfoInvalidations()`. All without parameters. -**Decision**: All drain/take methods accept optional `targetId` parameter defaulting to the current tab. Existing callers unchanged. - -### pageCache stays in client.ts as Map -**Evidence**: `pageCache` at `client.ts:85` already has TTL, parse, and invalidation logic in `client.ts:168-179`. Moving it to session.ts would require exposing TTL constants and parse logic across module boundaries. Session-level dirty flag (TabSession.pageInfoDirty) is sufficient for invalidation. -**Decision**: pageCache in `client.ts` becomes `Map`. TabSession carries the dirty flag. - -## Architecture - - - -### src/cdp/session.ts:1-194 — MODIFY - -```typescript -// === Slice 1: Foundation === -// All changes are within createCdpSession(). Existing singleton vars (dialog, -// pageInfoDirty, networkBuffer, consoleBuffer) remain for Slice 1 compatibility. - -// NEW: Per-tab state type (inserted before createCdpSession, after imports) -type TabSession = { - sessionId: string; - targetId: string; - dialog: DialogInfo | null; - pageInfoDirty: boolean; - networkBuffer: ReturnType; - consoleBuffer: ReturnType; -}; - -// Inside createCdpSession(), ADD after the singleton declarations: -const tabs = new Map(); -const sessionIdToTargetId = new Map(); - -// consumeEvents — ADD filter guard as the FIRST statement inside the for-await loop: -// (inserted before the existing `if (ev.method === "Page.javascriptDialogOpening")`) -for await (const ev of transport.events()) { - // Filter: skip events from sessions we're not currently tracking. - // Target.targetDestroyed is browser-level (no sessionId) — always process. - if (ev.method !== "Target.targetDestroyed") { - if (ev.sessionId && ev.sessionId !== sessionId) continue; - } - // ... all existing event handlers unchanged ... -} - -// switchTo — REPLACE the entire method body: -async switchTo(tid) { - const activated = await transport.request("Target.activateTarget", { targetId: tid }, { sessionId: null }); - if (!activated.success) return activated; - const attached = await transport.request("Target.attachToTarget", { targetId: tid, flatten: true }, { sessionId: null }); - if (!attached.success) return attached; - const a = attached.data as { sessionId: string }; - // Reuse existing TabSession or create one on first visit - const existing = tabs.get(tid); - const tab: TabSession = existing ?? { - sessionId: a.sessionId, - targetId: tid, - dialog: null, - pageInfoDirty: true, - networkBuffer: createNetworkBuffer(), - consoleBuffer: createConsoleBuffer(), - }; - if (!existing) { - tabs.set(tid, tab); - await enableDomains(a.sessionId); - } - // Update global pointers to point at the new active tab - sessionId = tab.sessionId; - targetId = tid; - // Update sessionId→targetId reverse lookup for event filtering - sessionIdToTargetId.set(tab.sessionId, tid); - return ok(undefined); -} - -// attachFirstPage — ADD after `sessionId = a.sessionId; targetId = pickTargetId;`: -tabs.set(pickTargetId, { - sessionId: a.sessionId, - targetId: pickTargetId, - dialog: null, - pageInfoDirty: false, - networkBuffer: createNetworkBuffer(), - consoleBuffer: createConsoleBuffer(), -}); -sessionIdToTargetId.set(a.sessionId, pickTargetId); - -// transport.onClose — ADD Map cleanup before restartConsumer(): -tabs.clear(); -sessionIdToTargetId.clear(); -``` - -### src/cdp/session.ts — Slice 2 additions (dialog + dirty flag into TabSession) - -```typescript -// consumeEvents — dialog handler: write to TabSession instead of singleton -if (ev.method === "Page.javascriptDialogOpening") { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) continue; - const params = ev.params as Partial | undefined; - tab.dialog = { - type: (params?.type as DialogInfo["type"]) ?? "alert", - message: params?.message ?? "", - ...(params?.defaultPrompt !== undefined ? { defaultPrompt: params.defaultPrompt } : {}), - }; - continue; -} - -// consumeEvents — dirty flag: write to TabSession instead of singleton -if (ev.method === "Page.frameNavigated" || ev.method === "Page.loadEventFired") { - const tab = targetId ? tabs.get(targetId) : undefined; - if (tab) tab.pageInfoDirty = true; -} - -// takeDialog — read from current TabSession -takeDialog() { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return null; - const d = tab.dialog; - tab.dialog = null; - return d; -}, - -// drainPageInfoInvalidations — read from current TabSession -drainPageInfoInvalidations() { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return false; - const dirty = tab.pageInfoDirty; - tab.pageInfoDirty = false; - return dirty; -}, - -// REMOVE the singleton `dialog` and `pageInfoDirty` declarations: -// - let dialog: DialogInfo | null = null; ← REMOVED (now in TabSession) -// - let pageInfoDirty = false; ← REMOVED (now in TabSession) - -// transport.onClose — dialog preservation moves to TabSession (no change needed; -// tabs.clear() already clears all TabSession state including dialog) -``` - -### src/cdp/console-buffer.ts:256-262 — MODIFY (Slice 3) - -```typescript -// Replace the clear() method body — reset nextSeq to 1 on clear: -// OLD: -// clear() { -// records.clear(); -// overflowed = false; -// // nextSeq is intentionally NOT reset: the cursor remains monotonic across -// // tab switches so a stale sinceSeq from a previous tab can't accidentally -// // match new records. -// }, -// NEW: - clear() { - records.clear(); - nextSeq = 1; - overflowed = false; - }, -// Rationale: With per-tab buffers, each TabSession has its own console buffer -// with independent sequence space. Resetting nextSeq on clear prevents cross-tab -// data leaks — a stale cursor from tab X (seq 15) cannot match new records from -// tab Y (which restart at seq 1). The old monotonic-across-tabs behavior was a -// design inversion when buffers were shared. -``` - -### src/cdp/session.ts — Slice 3 additions (buffer routing + drain from TabSession) - -```typescript -// consumeEvents — route network/console events to correct TabSession: -// Helper (inserted before the for-await loop): -const resolveTab = (evSessionId?: string): TabSession | undefined => { - const tid = evSessionId ? sessionIdToTargetId.get(evSessionId) : targetId; - return tid ? tabs.get(tid) : undefined; -}; - -// Replace all singleton buffer ingest calls with routed calls: -// OLD: if (ev.method === "Network.requestWillBeSent") networkBuffer.ingestRequestWillBeSent(ev.params); -// NEW: -if (ev.method === "Network.requestWillBeSent") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestRequestWillBeSent(ev.params); -} -else if (ev.method === "Network.responseReceived") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestResponseReceived(ev.params); -} -else if (ev.method === "Network.loadingFinished") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestLoadingFinished(ev.params); -} -else if (ev.method === "Network.loadingFailed") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestLoadingFailed(ev.params); -} -else if (ev.method === "Runtime.consoleAPICalled") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.consoleBuffer.ingestConsoleApi(ev.params); -} -else if (ev.method === "Log.entryAdded") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.consoleBuffer.ingestLogEntry(ev.params); -} - -// drainNetworkBuffer — read from current TabSession -drainNetworkBuffer(filter) { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return { records: [], total: 0, bufferOverflowed: false }; - return tab.networkBuffer.drain(filter); -}, - -// drainConsoleBuffer — read from current TabSession -drainConsoleBuffer(filter) { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return { records: [], total: 0, bufferOverflowed: false }; - return tab.consoleBuffer.drain(filter); -}, - -// REMOVE the singleton buffer declarations: -// - const networkBuffer = createNetworkBuffer(); ← REMOVED (now in TabSession) -// - const consoleBuffer = createConsoleBuffer(); ← REMOVED (now in TabSession) -``` - -### src/client.ts:85,157-180,183 — MODIFY (Slice 2) - -```typescript -// REPLACE single pageCache with per-tab Map: -// OLD: let pageCache: { readonly info: PageInfo; readonly at: number } | null = null; -// NEW: -const pageCaches = new Map(); - -// readPageInfo — use per-tab Map instead of singleton: -const readPageInfo = async (): Promise> => { - const dirty = session.drainPageInfoInvalidations(); - const currentTid = session.current()?.targetId; - const cached = currentTid ? pageCaches.get(currentTid) : undefined; - if (cached && !dirty && Date.now() - cached.at < PAGE_INFO_TTL_MS) - return ok(cached.info); - const expr = safeJs`JSON.stringify({url:location.href,title:document.title,w:innerWidth,h:innerHeight,sx:scrollX,sy:scrollY,pw:document.documentElement.scrollWidth,ph:document.documentElement.scrollHeight})`; - const raw = await evaluateJs(expr); - if (!raw.success) return raw; - if (typeof raw.data !== "string") return err(cdpError("invalid_response", "page info evaluation did not return a string")); - let parsedRaw: unknown; - try { - parsedRaw = JSON.parse(raw.data); - } catch (e) { - return err(cdpError("invalid_response", `page info JSON.parse failed: ${e instanceof Error ? e.message : String(e)}`)); - } - const info = parsePageInfoPayload(parsedRaw); - if (!info.success) return info; - if (currentTid) pageCaches.set(currentTid, { info: info.data, at: Date.now() }); - return ok(info.data); -}; - -// switchTab — REMOVE `pageCache = null` (no longer needed — each tab retains its cache): -const switchTab = async (targetId: string): Promise> => { - const r = await session.switchTo(targetId); - if (!r.success) return r; - // pageCache = null; ← REMOVED - await session.call("Runtime.evaluate", { - expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, - }); - return ok(undefined); -}; - -// stop — REMOVE `pageCache = null`, replace with Map.clear(): -const stop = async (): Promise => { - await transport.close(); - pageCaches.clear(); - lastHealth = 0; -}; -``` - -### src/cdp/session.ts — Slice 4 additions (TabSession cleanup) - -```typescript -// consumeEvents — Target.targetDestroyed: remove TabSession entries -if (ev.method === "Target.targetDestroyed" && ownership) { - const params = ev.params as { targetId?: string } | undefined; - if (params?.targetId) { - ownership.remove(params.targetId); - // Prune per-tab state for the destroyed target - const tab = tabs.get(params.targetId); - if (tab) { - sessionIdToTargetId.delete(tab.sessionId); - tabs.delete(params.targetId); - } - } -} - -// attachFirstPage — ADD TabSession pruning after ownership reconciliation: -// (inserted after the ownership reconciliation block, after ownership.setHarnessWindow) -const liveTargetIds = new Set(allPages.map((p) => p.targetId)); -for (const tid of tabs.keys()) { - if (!liveTargetIds.has(tid)) { - const tab = tabs.get(tid); - if (tab) sessionIdToTargetId.delete(tab.sessionId); - tabs.delete(tid); - } -} - -// listTabs in client.ts — ADD page cache pruning (extend existing reconciliation): -// After: const survivors = owned.filter((id) => live.has(id)); -// Add: -for (const tid of pageCaches.keys()) { - if (!live.has(tid)) pageCaches.delete(tid); -} -``` - -### src/prompt.ts:63-73 — MODIFY (Slice 4) - -```typescript -// REPLACE the "Parallel Execution" section (lines 63-73): - -### Parallel Execution - -Observation tools (browser_screenshot, browser_page_info, browser_execute_js, browser_list_tabs, browser_http_get, etc.) can run in parallel with each other and with mutation tools. The harness automatically serializes mutation tools (click, type, scroll, navigate, switch_tab, etc.) so they never race on shared state. When operations are independent, emit them in the same turn for better performance. - -**Examples of safe parallel calls:** -\`\`\` -browser_screenshot() + browser_page_info() + browser_execute_js("document.title") -browser_http_get("https://api.example.com/data") + browser_click(x, y) -\`\`\` - -**Multi-agent note**: When multiple subagents use the browser, tab switching by one agent changes the active tab for all agents. Per-tab data (console buffers, network traces, dialogs, page info cache) is isolated — switching tabs does not destroy another agent's collected data. Call `browser_current_tab` before mutation tools to confirm you're on the expected tab. If `browser_page_info` returns a dialog, handle it with `browser_handle_dialog` promptly — dialogs block page interaction and are not queued across agents. - -## Desired End State - -```typescript -// Tab A and Tab B can coexist without cross-contamination: - -// Agent on Tab A: -browser_navigate("https://site-a.com") // navigates Tab A -browser_console({ sinceSeq: 0 }) // gets Tab A's console, nextCursor=5 -// Agent switches to Tab B: -browser_switch_tab("targetId_B") -browser_navigate("https://site-b.com") // navigates Tab B -browser_console({ sinceSeq: 0 }) // gets Tab B's console (separate buffer) -// Agent switches back to Tab A: -browser_switch_tab("targetId_A") -browser_console({ sinceSeq: 5 }) // continues from Tab A's cursor — data preserved! -browser_page_info() // returns Tab A's cached info (not stale from Tab B) - -// Tab A's dialog doesn't leak to Tab B: -// Tab A triggers alert("A") → stored in TabSession["targetId_A"].dialog -// Tab B triggers confirm("B") → stored in TabSession["targetId_B"].dialog -// browser_page_info() on Tab B returns { dialog: { type: "confirm", message: "B" } } -// No overwrite. No silent loss. -``` - -## File Map - -``` -src/cdp/session.ts # MODIFY — TabSession type, Map, ev.sessionId filter, switchTo rework, drain parameterization, cleanup -src/cdp/console-buffer.ts # MODIFY — nextSeq reset on clear() -src/client.ts # MODIFY — per-tab pageCache Map -src/prompt.ts # MODIFY — multi-agent warning -``` - -## Ordering Constraints - -- Slice 1 must come first (foundation — TabSession type, Map, ev.sessionId filter) -- Slice 2 depends on TabSession existing (dialog/dirty-flag/page-cache are TabSession fields) -- Slice 3 depends on TabSession existing (buffers are TabSession fields) -- Slice 4 depends on Slices 1-3 (cleanup reconciles against live TabSession entries) - -## Verification Notes - -From research Precedents & Lessons: -- **`d3d86f9`**: Event consumer must be serialized (already done). Surface crashes via console.warn (already done). -- **`71da8e0`**: Mutex serializes mutations globally, not per-tab. Per-tab data prevents corruption from this design limitation. -- **`20ceaa1`**: Ownership reconciliation pattern must be replicated for TabSession pruning. -- **`a1df2d3`**: Prefix matching hazard grows with owned tab count. TabSession Map uses full targetId keys — no prefix collision risk. - -Verification checks: -- `tsc --noEmit` passes on modified files -- Two tabs: switch A→B→A, console cursor from A's first visit preserves state -- Dialog on tab A is NOT returned by takeDialog() after switching to tab B -- pageInfoDirty from tab B's navigation does NOT invalidate tab A's page cache -- Target.targetDestroyed event removes TabSession from Map -- attachFirstPage reconciliation prunes dead TabSession entries - -## Performance Considerations - -- `Map.get(targetId)` — O(1), negligible (single-digit entries typical) -- Each TabSession's buffers bounded at 500 records — no unbounded growth -- TabSession creation is lazy (on first visit to a tab via switchTo or attachFirstPage) -- No additional CDP round-trips — event routing is purely in-memory - -## Migration Notes - -Not applicable — no persisted schema changes. TabSession state is purely in-memory and reconstructed on each session start. - -## Pattern References - -- `src/cdp/ownership.ts:38-72` — OwnershipRegistry factory: add/remove/has/notify pattern -- `src/cdp/session.ts:41-46` — Current singleton state to extract into TabSession -- `src/cdp/session.ts:112-118` — attachFirstPage ownership reconciliation (replicate for TabSession) -- `src/client.ts:85,168-179` — pageCache TTL pattern (replicate as Map) -- `src/cdp/network-buffer.ts:77-85` — Buffer factory + Map internal pattern - -## Developer Context - -**Q (`src/cdp/session.ts:41-46`, `src/client.ts:85`): Per-tab isolation scope chosen. Phase 1: ev.sessionId filter. Phase 2: per-tab data structures. Phase 3: agent-aware locking deferred.** -A: Approved — proceed with per-tab isolation design. - -**Q (Decomposition): 4 slices: Foundation, Dialog+Dirty+Cache, Buffers+nextSeq, Cleanup+Prompt.** -A: Approved — proceed with slice-by-slice generation. - -## Design History - -- Slice 1: Foundation — approved as generated -- Slice 2: Dialog + dirty flag + page cache — approved as generated -- Slice 3: Buffer isolation + nextSeq fix — approved as generated -- Slice 4: Cleanup + reconciliation + prompt — approved as generated - -## References - -- Research: `thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md` -- Questions: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` -- Rewrite design: `docs/superpowers/specs/2026-05-02-harness-rewrite-design.md` diff --git a/thoughts/shared/plans/2026-05-07_18-15-35_per-tab-isolation.md b/thoughts/shared/plans/2026-05-07_18-15-35_per-tab-isolation.md deleted file mode 100644 index acddf60..0000000 --- a/thoughts/shared/plans/2026-05-07_18-15-35_per-tab-isolation.md +++ /dev/null @@ -1,452 +0,0 @@ ---- -date: 2026-05-07T18:15:35Z -author: amankumarsingh77 -commit: 4052e6e -branch: main -repository: pi-browser-harness -topic: "per-tab-isolation-multi-agent-safety" -tags: [plan, session, isolation, per-tab, buffers, dialog, safety, concurrency] -status: ready -parent: "thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md" -last_updated: 2026-05-07T18:15:35Z -last_updated_by: amankumarsingh77 ---- - -# Per-Tab Isolation for Multi-Agent Safety — Implementation Plan - -## Overview - -Implement per-tab data isolation in `pi-browser-harness` so that multiple subagents can use the browser without corrupting each other's state. Replace singleton dialog, page-info-dirty flag, network buffer, and console buffer with per-tab `TabSession` objects stored in a `Map`. Add an `ev.sessionId` filter in the CDP event consumer to prevent cross-tab event contamination. Deliverable in 3 sequential phases totaling 4 file modifications. - -Reference design: `thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md` - -## Desired End State - -Tab A and Tab B can coexist without cross-contamination: - -- Switching tabs preserves each tab's network/console buffer data (no `clear()` on switch) -- Dialog from tab A is not overwritten by dialog from tab B -- Page info dirty flag is per-tab, not global -- Console `sinceSeq` cursor remains valid across tab switches (no cross-tab data leaks) -- Page info cache is preserved per-tab across switches -- CDP events from unattached sessions are filtered (no cross-contamination) -- Dead tab state is pruned on target destruction and session reconciliation - -## What We're NOT Doing - -- Agent-aware tab locking (requires pi platform agent identity exposure — Phase 3, deferred) -- Observation tool `callOnTarget` refactoring (Phase 3, deferred) -- Per-agent BrowserClient instances -- New CDP transport connections per agent - -## Phase 1: Foundation — TabSession type, Maps, ev.sessionId filter, switchTo rework - -### Overview -Introduces the `TabSession` type, the `Map` collection, the `sessionIdToTargetId` reverse lookup, the 1-line `ev.sessionId` guard in `consumeEvents`, and the `switchTo` rework that creates/reuses `TabSession` without clearing buffers. This phase alone prevents cross-tab event contamination and is independently shippable. - -### Changes Required: - -#### 1. TabSession type + infrastructure (`src/cdp/session.ts`) -**File**: `src/cdp/session.ts` -**Changes**: Insert `TabSession` type definition before `createCdpSession`. Inside `createCdpSession`, add `tabs` Map, `sessionIdToTargetId` Map. Add `ev.sessionId` filter in `consumeEvents`. Rework `switchTo` to create/reuse `TabSession`. Update `attachFirstPage` to create initial `TabSession`. Add Map cleanup in `transport.onClose`. - -```typescript -// === Phase 1: Foundation === - -// NEW: Per-tab state type (inserted before createCdpSession, after imports) -type TabSession = { - sessionId: string; - targetId: string; - dialog: DialogInfo | null; - pageInfoDirty: boolean; - networkBuffer: ReturnType; - consoleBuffer: ReturnType; -}; - -// Inside createCdpSession(), ADD after the singleton declarations: -const tabs = new Map(); -const sessionIdToTargetId = new Map(); - -// consumeEvents — ADD filter guard as the FIRST statement inside the for-await loop -// (inserted before the existing `if (ev.method === "Page.javascriptDialogOpening")`) -for await (const ev of transport.events()) { - // Filter: skip events from sessions we're not currently tracking. - // Target.targetDestroyed is browser-level (no sessionId) — always process. - if (ev.method !== "Target.targetDestroyed") { - if (ev.sessionId && ev.sessionId !== sessionId) continue; - } - // ... all existing event handlers unchanged ... -} - -// switchTo — REPLACE the entire method body: -async switchTo(tid) { - const activated = await transport.request("Target.activateTarget", { targetId: tid }, { sessionId: null }); - if (!activated.success) return activated; - const attached = await transport.request("Target.attachToTarget", { targetId: tid, flatten: true }, { sessionId: null }); - if (!attached.success) return attached; - const a = attached.data as { sessionId: string }; - // Reuse existing TabSession or create one on first visit - const existing = tabs.get(tid); - const tab: TabSession = existing ?? { - sessionId: a.sessionId, - targetId: tid, - dialog: null, - pageInfoDirty: true, - networkBuffer: createNetworkBuffer(), - consoleBuffer: createConsoleBuffer(), - }; - if (!existing) { - tabs.set(tid, tab); - await enableDomains(a.sessionId); - } - // Update global pointers to point at the new active tab - sessionId = tab.sessionId; - targetId = tid; - // Update sessionId→targetId reverse lookup for event filtering - sessionIdToTargetId.set(tab.sessionId, tid); - return ok(undefined); -} - -// attachFirstPage — ADD after `sessionId = a.sessionId; targetId = pickTargetId;`: -tabs.set(pickTargetId, { - sessionId: a.sessionId, - targetId: pickTargetId, - dialog: null, - pageInfoDirty: false, - networkBuffer: createNetworkBuffer(), - consoleBuffer: createConsoleBuffer(), -}); -sessionIdToTargetId.set(a.sessionId, pickTargetId); - -// transport.onClose — ADD Map cleanup before restartConsumer(): -tabs.clear(); -sessionIdToTargetId.clear(); -``` - -### Success Criteria: - -#### Automated Verification: -- [x] Type checking passes: `npm run typecheck` -- [x] `grep -c "consumeEvents" src/cdp/session.ts` confirms single consumer -- [x] `grep "ev.sessionId && ev.sessionId !== sessionId" src/cdp/session.ts` confirms filter guard exists - -#### Manual Verification: -- [x] Run `scripts/test-parallel.ts` — parallel mutation tests still pass (mutex unchanged) -- [ ] Open two tabs via `browser_new_tab`, switch between them — no crash, session alive -- [ ] CDP events from non-current tabs are silently dropped (verify: navigate tab A, switch to tab B, network events from tab A don't appear in `browser_network_requests`) - ---- - -## Phase 2: State Migration — Singletons into TabSession + per-tab page cache + nextSeq fix - -### Overview -Moves `dialog`, `pageInfoDirty`, `networkBuffer`, and `consoleBuffer` from singleton variables into `TabSession`. `consumeEvents` routes events to the correct tab's buffer. `takeDialog()`, `drainPageInfoInvalidations()`, `drainNetworkBuffer()`, `drainConsoleBuffer()` all read from the current `TabSession`. `client.ts` `pageCache` becomes `Map`. `console-buffer.ts` `clear()` resets `nextSeq` to 1. - -### Changes Required: - -#### 1. Dialog + dirty flag into TabSession (`src/cdp/session.ts`) -**File**: `src/cdp/session.ts` -**Changes**: Route dialog handler and dirty-flag handler through `TabSession`. Replace `takeDialog()` and `drainPageInfoInvalidations()` to read from current tab. Remove singleton `dialog` and `pageInfoDirty` declarations. - -```typescript -// === Phase 2a: Dialog + dirty flag === - -// consumeEvents — dialog handler: write to TabSession instead of singleton -if (ev.method === "Page.javascriptDialogOpening") { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) continue; - const params = ev.params as Partial | undefined; - tab.dialog = { - type: (params?.type as DialogInfo["type"]) ?? "alert", - message: params?.message ?? "", - ...(params?.defaultPrompt !== undefined ? { defaultPrompt: params.defaultPrompt } : {}), - }; - continue; -} - -// consumeEvents — dirty flag: write to TabSession instead of singleton -if (ev.method === "Page.frameNavigated" || ev.method === "Page.loadEventFired") { - const tab = targetId ? tabs.get(targetId) : undefined; - if (tab) tab.pageInfoDirty = true; -} - -// takeDialog — read from current TabSession -takeDialog() { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return null; - const d = tab.dialog; - tab.dialog = null; - return d; -}, - -// drainPageInfoInvalidations — read from current TabSession -drainPageInfoInvalidations() { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return false; - const dirty = tab.pageInfoDirty; - tab.pageInfoDirty = false; - return dirty; -}, - -// REMOVE the singleton `dialog` and `pageInfoDirty` declarations: -// - let dialog: DialogInfo | null = null; ← REMOVED (now in TabSession) -// - let pageInfoDirty = false; ← REMOVED (now in TabSession) -``` - -#### 2. Buffer routing + drain from TabSession (`src/cdp/session.ts`) -**File**: `src/cdp/session.ts` -**Changes**: Add `resolveTab` helper. Route all network/console event handlers through `resolveTab`. Replace `drainNetworkBuffer()` and `drainConsoleBuffer()` to read from current `TabSession`. Remove singleton `networkBuffer` and `consoleBuffer` declarations. - -```typescript -// === Phase 2b: Buffer routing + drain === - -// consumeEvents — route network/console events to correct TabSession: -// Helper (inserted before the for-await loop): -const resolveTab = (evSessionId?: string): TabSession | undefined => { - const tid = evSessionId ? sessionIdToTargetId.get(evSessionId) : targetId; - return tid ? tabs.get(tid) : undefined; -}; - -// Replace all singleton buffer ingest calls with routed calls: -if (ev.method === "Network.requestWillBeSent") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestRequestWillBeSent(ev.params); -} -else if (ev.method === "Network.responseReceived") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestResponseReceived(ev.params); -} -else if (ev.method === "Network.loadingFinished") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestLoadingFinished(ev.params); -} -else if (ev.method === "Network.loadingFailed") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.networkBuffer.ingestLoadingFailed(ev.params); -} -else if (ev.method === "Runtime.consoleAPICalled") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.consoleBuffer.ingestConsoleApi(ev.params); -} -else if (ev.method === "Log.entryAdded") { - const tab = resolveTab(ev.sessionId); - if (tab) tab.consoleBuffer.ingestLogEntry(ev.params); -} - -// drainNetworkBuffer — read from current TabSession -drainNetworkBuffer(filter) { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return { records: [], total: 0, bufferOverflowed: false }; - return tab.networkBuffer.drain(filter); -}, - -// drainConsoleBuffer — read from current TabSession -drainConsoleBuffer(filter) { - const tab = targetId ? tabs.get(targetId) : undefined; - if (!tab) return { records: [], total: 0, bufferOverflowed: false }; - return tab.consoleBuffer.drain(filter); -}, - -// REMOVE the singleton buffer declarations: -// - const networkBuffer = createNetworkBuffer(); ← REMOVED (now in TabSession) -// - const consoleBuffer = createConsoleBuffer(); ← REMOVED (now in TabSession) -``` - -#### 3. nextSeq reset on clear (`src/cdp/console-buffer.ts`) -**File**: `src/cdp/console-buffer.ts` -**Changes**: Replace the `clear()` method body to reset `nextSeq` to 1. - -```typescript -// Replace the clear() method body — reset nextSeq to 1 on clear: - clear() { - records.clear(); - nextSeq = 1; - overflowed = false; - }, -``` - -#### 4. Per-tab page cache (`src/client.ts`) -**File**: `src/client.ts` -**Changes**: Replace single `pageCache` with `Map`. Update `readPageInfo`, `switchTab`, and `stop`. - -```typescript -// REPLACE single pageCache with per-tab Map: -const pageCaches = new Map(); - -// readPageInfo — use per-tab Map instead of singleton: -const readPageInfo = async (): Promise> => { - const dirty = session.drainPageInfoInvalidations(); - const currentTid = session.current()?.targetId; - const cached = currentTid ? pageCaches.get(currentTid) : undefined; - if (cached && !dirty && Date.now() - cached.at < PAGE_INFO_TTL_MS) - return ok(cached.info); - const expr = safeJs`JSON.stringify({url:location.href,title:document.title,w:innerWidth,h:innerHeight,sx:scrollX,sy:scrollY,pw:document.documentElement.scrollWidth,ph:document.documentElement.scrollHeight})`; - const raw = await evaluateJs(expr); - if (!raw.success) return raw; - if (typeof raw.data !== "string") return err(cdpError("invalid_response", "page info evaluation did not return a string")); - let parsedRaw: unknown; - try { - parsedRaw = JSON.parse(raw.data); - } catch (e) { - return err(cdpError("invalid_response", `page info JSON.parse failed: ${e instanceof Error ? e.message : String(e)}`)); - } - const info = parsePageInfoPayload(parsedRaw); - if (!info.success) return info; - if (currentTid) pageCaches.set(currentTid, { info: info.data, at: Date.now() }); - return ok(info.data); -}; - -// switchTab — REMOVE `pageCache = null` (no longer needed — each tab retains its cache): -const switchTab = async (targetId: string): Promise> => { - const r = await session.switchTo(targetId); - if (!r.success) return r; - // pageCache = null; ← REMOVED - await session.call("Runtime.evaluate", { - expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`, - }); - return ok(undefined); -}; - -// stop — REMOVE `pageCache = null`, replace with Map.clear(): -const stop = async (): Promise => { - await transport.close(); - pageCaches.clear(); - lastHealth = 0; -}; -``` - -### Success Criteria: - -#### Automated Verification: -- [x] Type checking passes: `npm run typecheck` -- [x] `grep "let dialog" src/cdp/session.ts` returns no matches (singleton removed) -- [x] `grep "let pageInfoDirty" src/cdp/session.ts` returns no matches -- [x] `grep "const networkBuffer" src/cdp/session.ts` returns no matches -- [x] `grep "const consoleBuffer" src/cdp/session.ts` returns no matches -- [x] `grep "nextSeq = 1" src/cdp/console-buffer.ts` confirms the change -- [x] `grep "pageCaches" src/client.ts` confirms Map usage - -#### Manual Verification: -- [ ] Run `scripts/test-parallel.ts` — all tests pass -- [ ] Run `scripts/test-parallel-integration.ts` — all tests pass -- [ ] Open tab A, trigger `browser_console`, note `nextCursor`. Switch to tab B, trigger `browser_console`. Switch back to tab A, call `browser_console({ sinceSeq: })` — returns only new tab A messages (not tab B's) -- [ ] Navigate tab A to a page with an alert. Switch to tab B BEFORE calling `browser_page_info`. Call `browser_page_info` on tab B — returns tab B's page info (NOT tab A's dialog) -- [ ] Tab A navigates (triggering pageInfoDirty). Switch to tab B, call `browser_page_info` twice within 1 second — second call uses tab B's cache (not invalidated by tab A's navigation) - ---- - -## Phase 3: Cleanup + Reconciliation + Prompt - -### Overview -`Target.targetDestroyed` handler removes `TabSession` entries. `attachFirstPage` and `listTabs` prune dead `TabSession` and `pageCaches` entries during reconciliation. `prompt.ts` "Parallel Execution" section gains a multi-agent warning. - -### Changes Required: - -#### 1. TabSession cleanup on target destruction + reconciliation (`src/cdp/session.ts`) -**File**: `src/cdp/session.ts` -**Changes**: Extend `Target.targetDestroyed` handler to remove TabSession entries. Add TabSession pruning loop in `attachFirstPage` reconciliation. - -```typescript -// === Phase 3a: TabSession cleanup === - -// consumeEvents — Target.targetDestroyed: remove TabSession entries -if (ev.method === "Target.targetDestroyed" && ownership) { - const params = ev.params as { targetId?: string } | undefined; - if (params?.targetId) { - ownership.remove(params.targetId); - // Prune per-tab state for the destroyed target - const tab = tabs.get(params.targetId); - if (tab) { - sessionIdToTargetId.delete(tab.sessionId); - tabs.delete(params.targetId); - } - } -} - -// attachFirstPage — ADD TabSession pruning after ownership reconciliation: -const liveTargetIds = new Set(allPages.map((p) => p.targetId)); -for (const tid of tabs.keys()) { - if (!liveTargetIds.has(tid)) { - const tab = tabs.get(tid); - if (tab) sessionIdToTargetId.delete(tab.sessionId); - tabs.delete(tid); - } -} -``` - -#### 2. Page cache pruning in listTabs (`src/client.ts`) -**File**: `src/client.ts` -**Changes**: Extend existing `listTabs` reconciliation to prune dead `pageCaches` entries. - -```typescript -// listTabs — ADD page cache pruning after existing ownership reconciliation: -// After: const survivors = owned.filter((id) => live.has(id)); -for (const tid of pageCaches.keys()) { - if (!live.has(tid)) pageCaches.delete(tid); -} -``` - -#### 3. Multi-agent warning in system prompt (`src/prompt.ts`) -**File**: `src/prompt.ts` -**Changes**: Replace the "Parallel Execution" section (lines 63-73) with a version that adds the multi-agent note. - -```typescript -// REPLACE the "Parallel Execution" section (lines 63-73): - -### Parallel Execution - -Observation tools (browser_screenshot, browser_page_info, browser_execute_js, browser_list_tabs, browser_http_get, etc.) can run in parallel with each other and with mutation tools. The harness automatically serializes mutation tools (click, type, scroll, navigate, switch_tab, etc.) so they never race on shared state. When operations are independent, emit them in the same turn for better performance. - -**Examples of safe parallel calls:** -\`\`\` -browser_screenshot() + browser_page_info() + browser_execute_js("document.title") -browser_http_get("https://api.example.com/data") + browser_click(x, y) -\`\`\` - -**Multi-agent note**: When multiple subagents use the browser, tab switching by one agent changes the active tab for all agents. Per-tab data (console buffers, network traces, dialogs, page info cache) is isolated — switching tabs does not destroy another agent's collected data. Call `browser_current_tab` before mutation tools to confirm you're on the expected tab. If `browser_page_info` returns a dialog, handle it with `browser_handle_dialog` promptly — dialogs block page interaction and are not queued across agents. -``` - -### Success Criteria: - -#### Automated Verification: -- [x] Type checking passes: `npm run typecheck` -- [x] `grep "tabs.delete" src/cdp/session.ts` confirms cleanup logic -- [x] `grep "Multi-agent note" src/prompt.ts` confirms prompt update - -#### Manual Verification: -- [ ] Close a tab manually in the browser — `browser_list_tabs` no longer shows it; its TabSession entry is cleaned up -- [ ] Restart the session — `attachFirstPage` reconciliation prunes dead TabSession entries from prior session -- [ ] Spawn two subagents. Subagent A's `browser_page_info` returns its own tab's data after Subagent B switches tabs. Both agents see the multi-agent warning in their system prompts. - ---- - -## Testing Strategy - -### Automated: -- `npm run typecheck` — all phases must pass type checking -- `scripts/test-parallel.ts` — mutex serialization + observation parallelization tests -- `scripts/test-parallel-integration.ts` — serialized mutation + observation + AbortSignal tests - -### Manual Testing Steps: -1. **Tab switch buffer preservation**: Open tab A → navigate → `browser_console` → note `nextCursor` → switch to tab B → `browser_console` → switch back to A → `browser_console({ sinceSeq: cursor })` → ONLY tab A's new messages (no tab B contamination) -2. **Dialog isolation**: Tab A triggers alert → switch to tab B before calling `browser_page_info` → `browser_page_info` on tab B returns tab B's state (NOT tab A's dialog) → switch back to tab A → `browser_page_info` returns tab A's dialog -3. **Page cache isolation**: Tab A `browser_page_info` → tab B navigates (triggers dirty) → tab A `browser_page_info` again — must return stale cached tab A data (not re-fetched tab B data) IF tab A is still current. If tab B switched, tab A data comes from cache on switch-back. -4. **Target destruction**: Close a tab → its TabSession is removed → `browser_list_tabs` no longer shows it -5. **Session restart**: Kill and restart → dead tabs from prior session are pruned in reconciliation - -## Performance Considerations - -- `Map.get(targetId)` — O(1), negligible (single-digit entries typical) -- Each TabSession's buffers bounded at 500 records — no unbounded growth -- TabSession creation is lazy (on first visit to a tab via switchTo or attachFirstPage) -- No additional CDP round-trips — event routing is purely in-memory - -## Migration Notes - -Not applicable — no persisted schema changes. TabSession state is purely in-memory and reconstructed on each session start. - -## References - -- Design: `thoughts/shared/designs/2026-05-07_00-58_per-tab-isolation.md` -- Research: `thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md` -- Questions source: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` diff --git a/thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md b/thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md deleted file mode 100644 index a14ae12..0000000 --- a/thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -date: 2026-05-07T00:26:00Z -researcher: amankumarsingh77 -git_commit: bd19739 -branch: main -repository: pi-browser-harness -topic: "multi-agent safety in pi-browser-harness: how to make browser tools safe when multiple subagents use them concurrently, preventing shared state corruption, tab switching races, dialog buffer conflicts, and ownership confusion" -tags: [discover, mutex, ownership, dialog, tabs, session, concurrency, safety] -status: complete -last_updated: 2026-05-07 -last_updated_by: amankumarsingh77 ---- - -# Research Questions: Multi-Agent Safety in pi-browser-harness - -## Discovery Summary - -The harness uses three primary safety mechanisms: a lightweight async mutex (`src/util/mutex.ts`, `createMutex()`) that serializes all mutation tools marked `serialized: true` in `src/registry.ts`; an `OwnershipRegistry` (`src/cdp/ownership.ts`) backed by a Set that tracks which tab `targetId`s belong to this session and gates `switchTab`, `closeTab`, and `newTab` in `src/domains/tabs.ts`; and a single-slot dialog buffer in `src/cdp/session.ts` (`createCdpSession`) read via `takeDialog()`. Key files read for depth: `client.ts` (mutationMutex, switchTab, ownership wiring), `session.ts` (dialog buffering, tab switch buffer clearing, pageInfoDirty flag), `tool.ts` (serialized lane acquisition with AbortSignal check), `ownership.ts` (Set-backed registry with change listener), `state.ts` (persistence across sessions), `tabs.ts` (ownership-denied gating), `dialog.ts` (single Page.handleJavaScriptDialog call), `prompt.ts` (concurrency guidance injected into the system prompt), and `index.ts` (lifecycle wiring). The architecture is single-agent by design: one mutex, one sessionId/targetId pair, one dialog slot, one pageCache, and ownership tracked per-session not per-agent — none of these primitives are namespaced or partitioned for concurrent sub-agents. - -## Questions - -1. Trace the full mutation-tool serialization path from invocation to completion: the `registerBrowserTool` function in `src/util/tool.ts` wraps each tool's handler in an `execute` closure that first calls `client.ensureAlive()` (unless `ensureAlive` is false), then checks `def.serialized` and calls `client.mutationMutex().acquire()` — an async FIFO mutex defined by `createMutex` in `src/util/mutex.ts` that chains promises. After acquiring, it checks `signal?.aborted`, calls `def.handler(args, ctx)`, and releases in a `finally` block. Map every tool marked `serialized: true` in `src/registry.ts` (22 of 32 tools). Explain why `waitTool` is NOT serialized while `waitForLoadTool` IS, and whether the mutex provides sufficient isolation when two sub-agents each want to operate on different tabs independently. - -2. Follow the tab ownership lifecycle end-to-end: when `createBrowserClient` in `src/client.ts` creates an `OwnershipRegistry` via `createOwnershipRegistry` in `src/cdp/ownership.ts`, it optionally seeds `ownedTargetIds` from persisted `BrowserState` (restored in `index.ts` `session_start` from `restoreState` in `src/state.ts`). During `attachFirstPage` in `src/cdp/session.ts`, the session reconciles persisted ownership against live targets (dropping dead IDs), prefers re-attaching to an owned tab, or creates a fresh one via `Target.createTarget { newWindow: true }` and registers it with `ownership.add()`. On `Target.targetDestroyed` events, ownership is removed. Trace how `switchTabTool` and `closeTabTool` in `src/domains/tabs.ts` enforce ownership via `client.owns(resolved)` — including hex-prefix resolution logic — and what happens when a sub-agent tries to switch to a tab owned by the session but currently in use by another sub-agent. - -3. Analyze the dialog buffer's single-slot design and its multi-agent failure modes: in `createCdpSession` ( `src/cdp/session.ts` ), the `consumeEvents` async iterator catches `Page.javascriptDialogOpening` and writes to a single `dialog: DialogInfo | null` variable. Critically, `Page.javascriptDialogClosed` is intentionally NOT handled — dialogs stay in the buffer until `takeDialog()` is called. However, `takeDialog()` reads `dialog` then sets it to `null` atomically via assignment, and when the transport closes the dialog is preserved (not cleared in `onClose`). Trace how `client.pageInfo()` in `src/client.ts` calls `session.takeDialog()` first (returning `{ dialog: d }` if present), and how the system prompt in `src/prompt.ts` guides agents to call `browser_handle_dialog`. Identify the race condition where sub-agent A opens a page triggering a dialog, sub-agent B triggers a different dialog on a different tab before A consumes it, and only B's dialog survives. - -4. Map the shared-session architecture and its impact on concurrent sub-agents: `createCdpSession` in `src/cdp/session.ts` maintains a single `sessionId: string | null` and `targetId: string | null` pair. The `switchTo(tid)` method calls `Target.activateTarget`, `Target.attachToTarget`, updates both `sessionId` and `targetId`, sets `pageInfoDirty = true`, and clears both `networkBuffer` and `consoleBuffer`. Trace how `client.switchTab()` in `src/client.ts` additionally nullifies `pageCache` and best-effort marks the tab title. Explain why if sub-agent A is mid-workflow on tab X (expecting `browser_execute_js` results from that tab's DOM) and sub-agent B calls `browser_switch_tab` to tab Y, agent A's subsequent `browser_execute_js` silently targets the wrong page — the `sessionId`/`targetId` pair is global, with no per-agent override. - -5. Trace the page-info caching and invalidation pipeline: `client.readPageInfo()` in `src/client.ts` checks `session.drainPageInfoInvalidations()` (the `pageInfoDirty` boolean in `session.ts`, set on `Page.frameNavigated` and `Page.loadEventFired`), consults a `pageCache` with 1-second TTL, evaluates a JS expression to collect URL/title/viewport/scroll/page-dimensions, parses it through `parsePageInfoPayload()`, and caches the result. Investigate whether a sub-agent switching tabs invalidates another sub-agent's cached page info, and whether the 1-second TTL (`PAGE_INFO_TTL_MS = 1_000`) combined with the shared `pageCache` could cause a sub-agent to receive stale page info from a tab it didn't navigate to. - -6. Examine the state persistence and resumption mechanism across session boundaries: `state.ts` defines `BrowserState` (namespace, remoteBrowserId, ownedTargetIds, harnessWindowTargetId), `persistState()` calls `pi.appendEntry`, and `restoreState()` scans branch entries for the last `browser-harness-state` custom entry. In `index.ts`, `session_start` restores state and wires `onOwnershipChange` to persist on every mutation. Map the full flow: `OwnershipRegistry.onChange` → `onOwnershipChange` callback in `client.ts` → `persistState` → `pi.appendEntry`. Assess whether this persistence model could cause cross-agent confusion if two sub-agents with different browser namespaces share the same pi branch session entries. - -7. Follow the console and network buffer lifecycle across tab switches: in `session.ts`, `createConsoleBuffer()` and `createNetworkBuffer()` (imported from `src/cdp/console-buffer.ts` and `src/cdp/network-buffer.ts`) are cleared wholesale in `switchTo()` — `networkBuffer.clear()` and `consoleBuffer.clear()` — because "the network history of the previous tab is no longer relevant." The `browser_network_requests` domain in `src/domains/network.ts` drains the buffer with filters, and `browser_console` in `src/domains/console.ts` uses a `sinceSeq` cursor for incremental reads. Trace what happens when sub-agent A is iterating console output incrementally (using `sinceSeq` from a prior `nextCursor`) and sub-agent B switches tabs, clearing the buffer and invalidating A's cursor. - -8. Investigate whether the namespace system provides any multi-agent isolation boundary: `index.ts` generates or accepts a `--browser-namespace` flag, stored in `BrowserState.namespace` and passed to `createBrowserClient`. The ownership-denied error message in `src/domains/tabs.ts` (`ownershipDeniedMessage`) references the namespace string, and `client.namespace` is exposed on the `BrowserClient` type. Determine whether two sub-agents running with different `--browser-namespace` values would each get their own `BrowserClient`, mutex, ownership registry, and session — or whether the extension's singleton `client` variable in `index.ts` means all agents share one client regardless of namespace. If the latter, assess what would need to change to support namespace-per-agent client instances. - -9. Analyze the `before_agent_start` hook in `index.ts` and the system prompt injection in `src/prompt.ts` for multi-agent guidance: the hook appends `getBrowserSystemPrompt()` to the system prompt, which includes a "Parallel Execution" section stating that observation tools run in parallel while mutation tools are serialized. Examine whether this prompt guidance is sufficient for a sub-agent to understand that another sub-agent might be concurrently mutating shared state — or whether the prompt assumes single-agent usage and omits crucial warnings about tab switching side effects, dialog slot overwrites, and buffer clearing. - -10. Model the full set of shared mutable state across the entire client and session layer: enumerate every piece of mutable state — `sessionId`, `targetId`, `dialog`, `pageInfoDirty`, `pageCache`, `lastHealth`, `mutationMutex` queue, `networkBuffer` entries, `consoleBuffer` entries, `ownership` Set, `harnessWindow`, `transport` connection — and classify each as either "safe to share across agents" or "requires per-agent partitioning." For each item requiring partitioning, propose the minimal abstraction change (e.g., per-agent dialog queue, per-agent sessionId/targetId stack with restore-on-switch, namespaced buffers with merge semantics). diff --git a/thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md b/thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md deleted file mode 100644 index 61d414c..0000000 --- a/thoughts/shared/research/2026-05-07_00-41_multi-agent-safety.md +++ /dev/null @@ -1,223 +0,0 @@ ---- -date: 2026-05-07T00:41:00Z -researcher: amankumarsingh77 -git_commit: bd19739 -branch: main -repository: pi-browser-harness -topic: "Multi-agent safety: making browser tools safe for concurrent subagent usage" -tags: [research, mutex, ownership, dialog, tabs, session, concurrency, safety, subagents, isolation] -status: complete -questions_source: "thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md" -last_updated: 2026-05-07 -last_updated_by: amankumarsingh77 ---- - -# Research: Multi-Agent Safety in pi-browser-harness - -## Research Question - -How can the browser harness be made safe for concurrent subagent usage? When multiple subagents are spun up, each using browser tools, they must not corrupt each other's state — tab switching, dialog handling, buffer reads, and page interactions must remain isolated. - -## Summary - -The harness was designed for single-agent use. All 6 safety-critical subsystems — the mutex, session, dialog buffer, page cache, network/console buffers, and ownership registry — are shared singletons with zero per-agent or per-tab partitioning. One agent switching tabs overwrites the global `sessionId`/`targetId`, clears all buffers, nullifies the page cache, and can silently consume or overwrite another agent's dialog. The approach: **phased per-tab isolation** — first an immediate `sessionId` filter on the CDP event consumer to stop cross-tab contamination, then per-tab data structures (dialog queues, page caches, buffer maps, dirty flags), and finally agent-aware tab locking to prevent one agent from switching away from another agent's active tab. - -## Detailed Findings - -### Finding 1: Mutation Mutex Serializes Globally, Not Per-Tab - -The async FIFO mutex (`src/util/mutex.ts:12-23`) serializes the 22 mutation tools marked `serialized: true` in `src/registry.ts:25-49`. Observation tools (11 total, including `browser_execute_js`, `browser_screenshot`, `browser_page_info`, `browser_snapshot`) skip the mutex entirely. - -**The mutex prevents concurrent mutation execution but does NOT provide per-agent isolation.** When Agent B calls `browser_switch_tab` (serialized, `src/domains/tabs.ts:93-128`), the global `sessionId`/`targetId` at `src/cdp/session.ts:41-42` is overwritten. Agent A's subsequent `browser_execute_js` (non-serialized observation, `src/domains/js.ts:55`) silently evaluates on the wrong tab's DOM — the mutex didn't block it because observation tools bypass the mutex. The `browser_execute_js` result contains no indication of which tab it ran on. - -**The `callOnTarget` escape hatch exists but is unused** (`src/cdp/session.ts:150-151`). It accepts an explicit `sessionId`, which would allow per-agent session binding without switching the global session. No observation tool uses it — all call `session.call()` which binds to the global `sessionId` at `src/cdp/session.ts:148`. - -### Finding 2: Shared SessionId/TargetId Is the #1 Corruption Vector - -`src/cdp/session.ts:41-42` holds the single `sessionId`/`targetId` pair. The `switchTo()` method at `src/cdp/session.ts:125-143` performs a destructive overwrite: -- Activates the new target (line 128) -- Attaches a new CDP session (line 130) -- Overwrites `sessionId` and `targetId` (lines 133-134) -- Clears `networkBuffer` and `consoleBuffer` (lines 136-137) -- Sets `pageInfoDirty = true` (line 135) - -`client.switchTab()` at `src/client.ts:179-190` compounds this by nullifying `pageCache` (line 183). There is no snapshot-and-restore mechanism, no per-agent tab stack, and no notification that the tab context changed. - -**Concrete failure scenario**: Agent A is extracting data from tab X with `browser_execute_js`. Agent B calls `browser_switch_tab` to tab Y (serialized, so it waits its turn). After B's switch completes, A's next `browser_execute_js` runs on tab Y's DOM. If tab Y lacks the element A was querying, the result is `undefined` — a silent, undetectable corruption. - -### Finding 3: Dialog Buffer Is a Single Slot With Silent Overwrite - -`src/cdp/session.ts:43` stores one `dialog: DialogInfo | null`. Every `Page.javascriptDialogOpening` event from ANY attached session overwrites it at `src/cdp/session.ts:56-63` — there is no `ev.sessionId` filter, no queue, and no per-tab storage. - -**Concrete failure scenario**: Agent A triggers `alert("Step 1")` on tab X. Before A consumes it via `browser_page_info`, Agent B triggers `confirm("Delete?")` on tab Y. The `dialog` variable is overwritten. When A calls `browser_page_info` (`src/client.ts:177-180`), `session.takeDialog()` returns B's confirm dialog. A dismisses it via `browser_handle_dialog` (`src/domains/dialog.ts:20`), which sends `Page.handleJavaScriptDialog` on the current session (now tab Y). Tab X's alert is never handled — its renderer remains blocked. - -Additionally, `Page.javascriptDialogClosed` events are intentionally ignored at `src/cdp/session.ts:64-67`, meaning stale dialogs can persist indefinitely until `takeDialog()` is called. - -### Finding 4: Network/Console Buffers Cleared on Tab Switch - -Both buffers (`session.ts:46-47`) are cleared in `switchTo()` at `src/cdp/session.ts:136-137`. The console buffer's `nextSeq` counter (`src/cdp/console-buffer.ts:146`) is intentionally NOT reset on clear (lines 259-262), which causes a **cross-tab data leak for stale cursors**. - -**The `nextSeq` claim is inverted**: The comment says "a stale sinceSeq from a previous tab can't accidentally match new records." In reality, because `nextSeq` is NOT reset, new records get seq numbers continuing from where the old tab left off. A stale `sinceSeq=15` will match new records with seq 16+ from the wrong tab. If `nextSeq` WERE reset to 1, stale `sinceSeq=15` would skip all new records (1 <= 15 is true). - -**Concrete failure scenario**: Agent A iterates console output with `sinceSeq: 15` from a prior `nextCursor` on tab X. Agent B switches to tab Y, clearing the buffer. New tab Y logs appear with seq 16, 17. Agent A's next `browser_console({ sinceSeq: 15 })` returns tab Y's output — cross-tab data leakage. Agent A has no indication the data came from the wrong tab. - -### Finding 5: Page Cache Invalidated Across Tabs - -`src/client.ts:85` holds a single `pageCache` with 1-second TTL (`PAGE_INFO_TTL_MS = 1_000` at line 42). It's nullified on every `switchTab` (line 183) and on `Page.frameNavigated`/`Page.loadEventFired` events from ANY session (via shared `pageInfoDirty` flag at `src/cdp/session.ts:45`). - -**While `switchTab → pageCache = null` is correct, Agent A still gets the wrong tab's info on the next `browser_page_info` call** because it reads from the globally-switched session. There is no per-agent "my current tab" context. - -### Finding 6: Ownership Is Per-Session, Not Per-Agent - -The `OwnershipRegistry` (`src/cdp/ownership.ts:26-72`) tracks which tabs belong to the harness session — it protects the USER's tabs, not sub-agent-to-sub-agent isolation. An agent can switch to any owned tab, including one another agent is actively using. - -### Finding 7: Namespace Provides No Isolation - -The extension's `client` variable at `src/index.ts:36` is a singleton. All agents share one `BrowserClient`. The `--browser-namespace` flag is cosmetic — used in error messages (`src/domains/tabs.ts:71-72`) and temp file paths (`src/util/paths.ts:13`), but provides zero multi-agent isolation within a single pi process. - -### Finding 8: System Prompt Assumes Single-Agent Use - -`src/prompt.ts:63-73` states that the harness "automatically serializes mutation tools so they never race on shared state" — this is intra-agent only. The prompt contains no warnings about: -- Tab switching invalidating buffers and cursors -- Dialog slot overwrites -- The need to verify `browser_current_tab` before mutations -- Other agents sharing the same browser - -### Finding 9: Immediate Fix — `ev.sessionId` Filter - -A single change in `consumeEvents` at `src/cdp/session.ts:33` would prevent ALL cross-tab event contamination: - -```typescript -if (ev.sessionId && ev.sessionId !== sessionId && ev.method !== "Target.targetDestroyed") { - continue; -} -``` - -This prevents dialog events from other tabs from overwriting the slot, navigation events from setting `pageInfoDirty` for the wrong tab, and network/console events from polluting buffers. It makes the single-slot design **correct for single-agent use** and eliminates the most egregious cross-tab contamination for multi-agent use. - -## Code References - -- `src/cdp/session.ts:41-42` — Singleton `sessionId`/`targetId` pair, overwritten on every tab switch -- `src/cdp/session.ts:43` — Single `dialog` slot, overwritten by any `Page.javascriptDialogOpening` without `sessionId` filtering -- `src/cdp/session.ts:125-143` — `switchTo()` destructively overwrites session state and clears buffers -- `src/cdp/session.ts:150-151` — `callOnTarget()` escape hatch exists but is unused by observation tools -- `src/cdp/session.ts:45` — Shared `pageInfoDirty` flag set by navigation events from any session -- `src/cdp/session.ts:136-137` — `networkBuffer.clear()` + `consoleBuffer.clear()` on every tab switch -- `src/cdp/console-buffer.ts:256-262` — `clear()` empties records but preserves `nextSeq`, enabling cross-tab data leaks -- `src/client.ts:85` — Single `pageCache` nullified on any `switchTab` -- `src/client.ts:89` — Single `mutationMutex` shared by all agents -- `src/util/tool.ts:127-138` — Serialized tool mutex acquisition with abort guard -- `src/util/mutex.ts:12-23` — 13-line async FIFO mutex with no per-agent awareness -- `src/registry.ts:25-49` — 22 serialized tools vs 11 observation tools -- `src/cdp/ownership.ts:26-72` — Flat ownership Set, no per-agent tab tracking -- `src/index.ts:36` — Singleton `client` variable — one per process, shared by all agents -- `src/domains/tabs.ts:88-118` — switchTabTool ownership enforcement (per-session, not per-agent) -- `src/domains/tabs.ts:167-188` — closeTabTool ownership enforcement -- `src/prompt.ts:63-73` — System prompt "Parallel Execution" assumes single-agent -- `src/state.ts:22-45` — State persistence with no namespace filtering in `restoreState` -- `src/domains/js.ts:55` — `browser_execute_js` uses implicit global `sessionId` via `session.call()` -- `src/domains/console.ts:122-151` — `sinceSeq` cursor pattern with no invalidation on buffer clear -- `src/domains/network.ts:119-154` — `browser_network_requests` body fetch races against tab switches - -## Integration Points - -### Inbound References -- `src/index.ts:99-119` — `session_start` creates the singleton `BrowserClient`, all agents share it -- `src/index.ts:152-161` — `before_agent_start` injects system prompt, fires for every agent turn with no per-agent customization -- `src/registry.ts:64-66` — `registerAllTools` binds all tools to the singleton `client` via closure - -### Outbound Dependencies -- CDP WebSocket (`src/cdp/transport.ts:25`) — single connection to Chrome, all sessions multiplexed over it -- Chrome DevTools Protocol — `Target.attachToTarget`, `Target.activateTarget`, `Target.createTarget`, `Target.closeTarget` -- pi Extension API — `pi.registerTool`, `pi.on("before_agent_start")`, `pi.on("session_start")`, `pi.appendEntry` - -### Infrastructure Wiring -- `src/cdp/transport.ts:68` — Single event queue consumed by exactly one `for await` loop in `session.ts` -- `src/cdp/session.ts:57-83` — Single `consumeEvents` loop routes CDP events to all buffers -- `src/client.ts:88-96` — `onOwnershipChange` → `persistState` → `pi.appendEntry` chain - -## Architecture Insights - -1. **The entire extension is designed around a single-agent-per-process model.** Every subsystem (transport, session, ownership, buffers, cache, mutex) assumes exclusive access. There is no concept of multiple independent consumers. - -2. **The mutex provides execution serialization but not semantic isolation.** It guarantees two mutations don't run simultaneously, but cannot prevent Agent B from switching tabs between Agent A's observation and its next mutation. - -3. **`callOnTarget` proves the transport layer supports per-target sessions.** The infrastructure is capable of per-agent isolation — the tool layer simply doesn't use it. - -4. **The namespace system is identity-only, not isolation.** It's a label used in error messages and file paths. To become an isolation boundary, it would need to key into separate `BrowserClient` instances. - -5. **Event filtering by `sessionId` is the single most impactful change.** It would fix dialog overwrites, dirty flag theft, buffer pollution, and stale event processing — all from one guard in `consumeEvents`. - -6. **The console `nextSeq` non-reset is a design inversion.** The comment claims it prevents stale cursor matches — the exact opposite is true. Resetting `nextSeq` on clear would prevent cross-tab data leaks. - -## Precedents & Lessons - -4 similar past changes analyzed. - -### Precedent: Parallel tool execution with automatic mutation serialization -**Commit(s)**: `71da8e0` — "feat: parallel tool execution with automatic mutation serialization" (2026-05-04) -**Blast radius**: 8 files across 5 layers - - `src/util/mutex.ts` — new ~25 LOC async FIFO mutex - - `src/util/tool.ts` — serialized lane acquisition + AbortSignal guard - - `src/registry.ts` — 22 of 32 tools marked `serialized: true` - - `src/client.ts` — `mutationMutex()` exposed on `BrowserClient` - -**Follow-up fixes**: -- `f73941c` — Screenshot path rendering crash from parallel invocations - -**Takeaway**: The mutex prevents mutation-on-mutation races but creates a global bottleneck — one sub-agent's slow `waitForLoad` blocks ALL other agents' mutations, even on different tabs. - -### Precedent: Tab ownership — isolate harness tabs from user's browsing -**Commit(s)**: `20ceaa1` — "feat: tab ownership" (2026-05-05) -**Blast radius**: 8 files across 5 layers - - `src/cdp/ownership.ts` — new 72-line OwnershipRegistry - - `src/cdp/session.ts` — attachFirstPage reconciliation, targetDestroyed reaping - - `src/domains/tabs.ts` — ownership gating on switch/close - -**Takeaway**: Ownership protects the USER's tabs but does NOT protect sub-agents from each other. Two sub-agents share one `sessionId`/`targetId` — switching tabs is a global side effect. - -### Precedent: CDP session with single-slot dialog buffer -**Commit(s)**: `d3d86f9` — "fix(cdp): serialize event consumer; persist dialog until taken" (2026-05-02) -**Blast radius**: `src/cdp/session.ts` - -**Follow-up fixes**: -- `c9d1605` — Surface event-consumer crashes via `console.warn()` - -**Takeaway**: The dialog's single-slot buffer was corrected for fast-dismiss races but remains unguarded against multi-session overwrites. - -### Precedent: Audit fixes for list_tabs/switch_tab prefix matching -**Commit(s)**: `a1df2d3` — "release: v0.3.1 — audit fixes" (2026-05-02) -**Blast radius**: 9 files across 4 layers - -**Takeaway**: Prefix resolution for targetIds becomes more ambiguous as owned-tab count increases in multi-agent scenarios. - -### Composite Lessons -- ALL safety primitives are single-agent by design — `71da8e0`, `20ceaa1`, `d3d86f9` -- The mutex serializes globally, not per-tab — `71da8e0` -- Tab switching is a silent cross-agent side effect — `20ceaa1` -- Dialog buffer is a single slot with silent overwrite — `d3d86f9` -- Buffer clearing on tab switch invalidates per-agent cursors — `d3d86f9` -- Prompt and SKILL docs assume single-agent usage — `71da8e0` - -## Historical Context (from thoughts/) -- `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` — 10 trace-quality questions covering every safety subsystem -- `docs/superpowers/specs/2026-05-02-harness-rewrite-design.md` — v0.3.0 rewrite design; §2.2 enumerates 13 predictability bugs fixed by the module split -- `docs/RESEARCH.md` — Competitive analysis showing parallel execution + auto-serialization as a unique differentiator -- `CHANGELOG.md` — v0.3.2 documents parallel-safe boundary; v0.4.0 adds snapshot-first patterns - -## Developer Context - -**Q (`src/cdp/session.ts:41-46`): All safety primitives (mutex, ownership, session, buffers, dialog, page cache) are shared singletons with zero per-agent partitioning. Which isolation scope should we target?** -A: Per-tab isolation (Recommended) — Add per-tab data structures: per-tab dialog queue, per-tab page cache, per-tab buffer maps, per-tab dirty flags. Agents still share one client but each tab has independent state. Minimal change preventing data corruption without connection overhead. - -**Q (`src/cdp/session.ts:33`): The per-tab isolation can be delivered in phases. Which delivery cadence?** -A: Quick-win first (Recommended) — Phase 1: `ev.sessionId` filter in `consumeEvents` (1-line fix). Phase 2: per-tab data structures. Phase 3: agent-aware tab locking. - -## Related Research -- Questions source: `thoughts/shared/questions/2026-05-07_00-26_multi-agent-safety.md` - -## Open Questions -1. **Agent identity exposure**: The pi platform currently does NOT expose the calling agent's ID to tool handlers. Phase 3 (agent-aware tab locking) requires identifying which agent initiated a tool call. Does the `ExtensionContext` carry agent identity, or would this require a pi platform change? -2. **Observation tool isolation**: Even after per-tab data structures, observation tools (`browser_execute_js`, `browser_screenshot`) will still read from the globally-switched session. Should they be changed to use `callOnTarget` with a per-agent cached sessionId? -3. **Tab switching protocol**: Should agents be required to explicitly "check out" and "release" tabs? Or should the harness auto-detect which agent was last on a tab and restore context before executing tools? -4. **Namespace-per-client in separate processes**: When two separate pi processes (e.g., parent + subagent) both load the harness, each creates its own `BrowserClient` — they're naturally isolated. Should we document this pattern as the recommended multi-agent approach? -- cgit v1.2.3