# CLAUDE.md

This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.

## Project Overview

Chrome Debug is an MCP (Model Context Protocol) server that gives AI full control over a Chrome browser instance. It consists of:
- An MCP server for AI integration
- An HTTP API server for browser control
- A Chrome extension for visual element selection and screen recording
- A VS Code extension for IDE integration

## Key Commands

### Development
```bash
npm install          # Install dependencies
npm start           # Start MCP server (cleans up processes first)
npm run go          # Start in development mode with NODE_ENV=development
npm run dev         # Start with --watch flag for development
npm run start-http  # Start HTTP server directly (port 3000-4000)
npm run server      # Run standalone server
npm run single-server # Run with --single-server flag
npm run cleanup     # Clean up stale Chrome processes
npm run cleanup-force # Force cleanup of all Chrome processes

# Persistent server (won't be killed by other instances)
chromedebug-mcp --persistent  # Start a persistent server for manual debugging
```

### Testing
```bash
# Run all tests
npm test            # Run all Jest tests
npm run test:watch  # Run tests in watch mode
npm run test:coverage # Run tests with coverage report

# Run specific test suites
npm run test:unit   # Run unit tests only (excludes integration tests)
npm run test:integration # Run integration tests only
npm run test:chrome # Test ChromeController functionality
npm run test:http   # Test HTTP server
npm run test:db     # Test database functionality
npm run test:mcp    # Test MCP server functionality

# Recording system tests
npm run test:recording # Test comprehensive recording system
npm run test:extension # Test Chrome extension recording
npm run test:migration # Test dual ID migration
npm run test:regression # Test recording regression
npm run test:recording-all # Run all recording-related tests
```

### Configuration Management
```bash
npm run config              # Manage configuration
npm run config-show         # Show current configuration
npm run config-ports        # List configured ports
npm run update-extension-config # Update Chrome extension config
```

### Chrome Extension
```bash
# Load extension in Chrome:
# 1. Open chrome://extensions/
# 2. Enable Developer mode
# 3. Click "Load unpacked"
# 4. Select chrome-extension/ directory

# Build and package the extension (ONE build; PRO ships dormant inside it)
npm run build:extension    # Validate both manifests, then webpack into dist/extension
npm run package-extension  # Build, run scripts/verify-extension-artifact.js, zip to
                           # dist/chromedebug-extension.zip (prepublishOnly runs this)
# chrome-extension/manifest.store.json is copied into the build as manifest.json.
# chrome-extension/manifest.json is the dev manifest for "Load unpacked" and must match the
# store manifest except name, version and description (tests/manifest-parity.test.js).
# Packaging is REFUSED while any release placeholder remains (see "Release placeholders").
# (History: the FREE/PRO split - build:free, build:pro, package-pro and the two tier
#  manifests - was removed. package-webstore was removed earlier: its hand-maintained file
#  list omitted three files background.js importScripts, so its output had a dead service
#  worker.)
```

### VS Code Extension
```bash
cd vscode-chrome-pilot
npm install
npm run compile
# Press F5 in VS Code to test
```

## Architecture

### Core Components

1. **MCP Server** (`src/index.js`)
   - Entry point for AI integration via Model Context Protocol
   - Exposes Chrome control tools to AI assistants
   - Uses StdioServerTransport for communication

2. **Chrome Controller** (`src/chrome-controller.js`)
   - Central class managing Chrome browser instances
   - Uses Puppeteer for browser automation
   - Handles debugging protocol connections
   - Manages element selection and screenshots
   - Stores recordings in SQLite database

3. **HTTP Server** (`src/http-server.js`)
   - REST API on ports 3000-4000 (auto-finds available)
   - Handles screen recording uploads and workflow recording
   - CORS-enabled for cross-origin requests

4. **Database** (`src/database.js`)
   - SQLite database for persistent storage
   - Stores screen recordings and workflow data
   - Located at `~/.chromedebug/chromedebug.db` (`getCentralDbPath()`), overridable with
     `CHROMEDEBUG_DB_PATH`. NOT `data/chrome-pilot.db` - that is a pre-2026 path and the
     file of that name still on disk is 0 bytes.

5. **Capture System** (`src/capture/`)
   - Modular frame capture architecture
   - Memory management for large video recordings
   - TypeScript-style interfaces defined in `interfaces.js`
   - Optimized for AI analysis and debugging workflows

### Key Design Patterns

- **Single Chrome Instance**: Each server (MCP or HTTP) maintains one ChromeController instance
- **Per-Instance Chrome**: Each Chrome launch creates a unique user data directory
- **Automatic Cleanup**: Stale Chrome profiles are cleaned up on startup
- **Port Management**: Automatically finds available ports for debugging and HTTP server
- **Timeout Protection**: All async operations have configurable timeouts

### Chrome Extension Features

- **Element Selection**: Click elements to select for AI manipulation
- **Screen Recording**: Capture video + console logs for debugging
- **Workflow Recording**: Record user actions with screenshots for replay
- **Screenshot Recording**: Click-to-capture mode with countdown (0/3/5 seconds)
- **Frame Capture**: Optimized frame capture for videos
- **WebRTC Override**: Redirects webcam to screen capture
- **Download ZIP Export**: Export recordings as ZIP packages with screenshots and JSON metadata
- **Tab Filtering**: Filter recordings by type (All/Workflow/Screenshots)

### Chrome Extension tiers (one build)

**PRO is an honour-system tier enforced by a one-way local unlock, set once from a bundled
key-hash list or Lemon Squeezy's License API. The PRO code ships to every install and stays
dormant until unlocked. Anyone who edits extension storage or code can bypass it.**

There is one Chrome Web Store listing ("ChromeDebug MCP Assistant") and one build. Chrome Web
Store Payments no longer exists, so PRO is sold by the owner through Lemon Squeezy (merchant of
record) for $29.99, one time, and unlocked inside the same extension by pasting the licence key
from the receipt. There is no separate PRO listing, zip or installer.

**FREE** (every install until a licence is stored):
- 25 total recordings, then PRO is required (a lifetime trial, not a daily allowance)
- The counter lives in `chrome.storage.sync` with a `chrome.storage.local` mirror; reads take
  `max(local, sync)` and clamp to the limit. Clearing one area, or a second machine on the same
  Google account, does not reset it, and the 25 is shared per Google **account**. It does
  **not** survive an uninstall: Chromium clears an extension's sync storage on uninstall and
  forwards that deletion to the account (`SyncStorageBackend::DeleteStorage` then
  `SyncableSettingsStorage::Clear`;
  https://raw.githubusercontent.com/chromium/chromium/main/chrome/browser/extensions/api/storage/sync_storage_backend.cc),
  so a reinstall restarts the trial. Earlier docs claimed the opposite; that was wrong.
- All core recording features
- Watermarked images (applied at display/export time, not storage)

**PRO** is sold as exactly these, and nothing else may be advertised (listing, README, popup,
frame-editor modal):
- Unlimited recordings
- Watermark-free exports
- MP4 export and ZIP export

Modules that are NOT sold:
- `pro-only/enhanced-capture.js` ships in the bundle, is a web-accessible resource, and is
  `import()`ed by `content.js` only when the stored licence says `licensed`. It is **not
  advertised anywhere**: its React/Vue component data has never been verified in a real browser.
  When unlicensed, its popup control is hidden, not shown as a PRO upsell.
- `pro-only/function-tracker.js` is **parked**: kept in the repo for a future fix, excluded
  from the bundle, from `content_scripts` and from npm `files`. Tracing was dead code - its
  only start path sat inside a block comment in `content.js` and nothing sent
  `startFullDataRecording`. Do not advertise tracing and do not re-add the file to a manifest
  without fixing it first (the artifact guard fails a build that contains it).

**Data transmission.** No recording data leaves the machine. Activation sends the licence key
to Lemon Squeezy once, only if it is not in the bundled list.

**Where PRO applies.** In the user's own Chrome profile, including via
`connect_to_existing_chrome`. MCP-launched Chrome is not a PRO surface: branded Chrome 137+
ignores `--load-extension`, and where it does load each launch uses a throwaway profile
(`src/services/profile-manager.js`), so it is FREE and its trial restarts per launch.

#### Licence files

| File | Loaded by | Role |
|---|---|---|
| `chrome-extension/entitlement.js` | `importScripts` in `background.js`; `<script>` in `popup.html`, `capture-viewer.html`, `pro/frame-editor.html`, `options.html` (before any consumer); FIRST entry of `content_scripts[0].js` | Read-only state: `read()`, `ready()`, `getStateSync()`, `onChange()`. Single copies of `PURCHASE_URL`, `PRICE_LABEL`, `SUPPORT_CONTACT`, `PRIVACY_POLICY_URL`. Does no work at evaluation time (it runs on every page). `read()` never returns the plaintext key |
| `chrome-extension/licence-key-hashes.js` | `importScripts` in `background.js` only | Generated list of `hex(SHA-256('chromedebug-pro-v1:' + key.trim().toLowerCase()))`, padded to whole buckets of 256 |
| `chrome-extension/licence-activate.js` | `importScripts` in `background.js` only (after the two above) | Activation, mirroring, backfill; the one `PROVIDER` adapter for Lemon Squeezy; `LS_STORE_ID` / `LS_PRODUCT_IDS`. Side-effect free when evaluated; `background.js` decides when to mirror |
| `scripts/export-licence-key-hashes.js` | the owner, locally, before each release | Writes the hash list and its lock from a live-mode API key. Never deployed, never in CI, excluded from npm `files` |

Webpack COPIES the three extension files; no entry `import`s them (a bundled second copy of
`entitlement.js` would have its own `ready()`).

Storage: `chromedebug_licence` in both `storage.sync` and `storage.local` holds
`{ v:1, state:'licensed', key, keyShort, verifiedBy, provider:'lemonsqueezy', orderId|null, productId|null, activatedAt }`;
a local-only tombstone `{ v:1, state:'removed' }` ("Remove PRO from this browser"); and
`storage.local` `chromedebug_licence_last_attempt`. No customer email is stored.

Service-worker messages: `{action:'activateLicence', key}` (accepted only when
`sender.id === chrome.runtime.id` and `sender.url` starts with `chrome.runtime.getURL('')`) and
`{action:'mirrorLicence'}`.

#### The latch invariant (do not weaken)

Enforced by `tests/entitlement-latch.test.js` and `scripts/verify-extension-artifact.js`.

1. **The unlock is a pure function of stored data.** The read path has no network, clock,
   counter or device id. `entitlement.js` is checked on its parsed AST for this.
2. **Only two things write `chromedebug_licence`:** `licence-activate.js` (verified records,
   mirroring, backfill) and the options page's **Remove**, which writes the local tombstone and
   never touches sync. No catch block, migration or failed read ever deletes a licence.
3. **Mirroring.** The service worker copies a licence sync -> local and local -> sync on every
   wake, on `runtime.onStartup`/`onInstalled`, on `storage.onChanged` for sync, and when a page
   reports a sync-only record. A tombstone blocks both directions for that browser. Without it,
   uninstalling on one machine (which clears sync account-wide) would re-lock another machine
   that held only the sync copy.
4. **Readers accept any record with `v >= 1`.** Fields are only ever added; `verifiedBy` is an
   open string, so another verification source can be added later.
5. **The hash list is append-only.** `tests/fixtures/licence-key-hashes.lock.json` is the FULL
   shipped list, real hashes and padding alike. Padding entries are random digests generated by
   the export script, indistinguishable from real ones, and permanent once exported; the list
   grows only by whole buckets of 256, so neither the list nor the lock reveals the buyer count.
   A hash may be dropped only if the owner lists it in
   `tests/fixtures/licence-key-hash-removals.json` (a dropped padding entry is simply backfilled).
   Which entries are real is derivable by hashing the API keys and hand-issued keys; no separate
   padding record is kept. The one exception to append-only is the placeholder list the
   repository starts with (`CHROMEDEBUG_KEY_HASHES_META.source` is the sentinel): it never
   shipped (the guard refuses it) and nothing records how its entries were made, so the first
   export replaces it instead of keeping its entries as padding.
6. **Re-activating is idempotent and uses no Lemon Squeezy activation slots.** No expiry, device
   limits, periodic re-validation or automatic removal.

#### Licence status is tri-state; `error` is always shown

`read()` resolves `licensed`, `unlicensed` or `error`. `getStateSync()` throws an Error named
`EntitlementNotReady` until `ready()` has resolved, and there is deliberately no boolean helper,
so every call site handles `error`. `ready()` never rejects.

- `error` is never silently mapped to either tier. It favours the paying user (no cap, no
  watermark, exports allowed) **and** the UI shows "Licence status could not be read: <error>".
- `error` is wider than "chrome.storage rejected a read". A missing licence module, an
  unexpected state value, or ANY exception thrown on the read path (`entitlement.js`
  `readAndApply`, `background.js` `readEntitlementState`) also resolves as `error`, and so also
  lifts the cap and the watermark for everyone until it is fixed. A bug that makes `read()`
  throw is therefore a free unlock with only the banner to show for it; keep the storage-read
  failure cases in `tests/entitlement.test.js` and the "read() throws" case in
  `tests/licence-content-script-gates.test.js` so such a regression is visible in the suite.
- Never swallow `EntitlementNotReady` in a fail-open catch around `shouldWatermark()`,
  `processImageForDisplay()` or `processImageForExport()`: an ordering slip would ship
  unwatermarked images silently. Rethrow it, or render an error and abort.
- Nothing sends `limit: Infinity` over `sendMessage` (it serialises to `null`). Licensed usage
  replies are `{ entitlementState:'licensed', isPro:true, count:null, limit:null }`; callers
  branch on `entitlementState`, never on `limit`.

#### Artifact integrity (do not weaken)

This replaced the old "artifact purity" rule. PRO code ships to everyone now, so what must hold
is that the artifact is exactly the reviewed shape and fully configured. Enforced in
`scripts/verify-extension-artifact.js` (run by `package-extension`, so `prepublishOnly` fails
on any violation) and tested by RUNNING it in `tests/extension-artifact-integrity.test.js`,
plus `tests/manifest-parity.test.js`:

- Structural: every `importScripts` target, every HTML `<script src>`, and every file the
  manifest names (content scripts, CSS, web-accessible resources, service worker, icons,
  pages) is in the build. A missing `importScripts` target is fatal and silent - the service
  worker never installs. The FREE zip published as 2.7.7 was broken this way.
- Exact `content_scripts` (with `entitlement.js` first, and a superset of what live 2.7.4
  loads) and exact `web_accessible_resources`.
- The permission pin on the built, store AND dev manifests: `activeTab debugger notifications
  offscreen scripting sidePanel storage tabCapture tabs unlimitedStorage`. No
  `host_permissions`, `optional_*`, `externally_connectable` or `key`. A new permission can
  disable the extension for existing users until they accept a prompt; run the Extension
  Update Testing Tool before changing the pin.
- The pinned LIVE Lemon Squeezy store/product ids (`tests/fixtures/lemonsqueezy-ids.json`)
  must equal `LS_STORE_ID` / `LS_PRODUCT_IDS` in the built `licence-activate.js`. The online
  check requires both ids to match, because keys from any Lemon Squeezy store otherwise pass.
  Test-mode products have different ids and are expected to fail as `wrong_product`; never
  loosen that check to make a test pass.
- `entitlement.js` token/AST bans (no `fetch`, `XMLHttpRequest`, `Date`, `performance`,
  `lemonsqueezy`, storage `set`/`remove`/`clear`, or storage aliasing).
- The Firebase host tripwire: no `cloudfunctions.net`, `firebaseapp` or `firebaseio` in the build.
- The hash-list lock (append-only, above).
- `pro-only/` in the build is exactly `enhanced-capture.js`; `background.js` imports the
  licence files in order; the service-worker-only files are not loaded by any page; gated pages
  load `entitlement.js` before any consumer; no bundle references the deleted `__TIER__` /
  `__FEATURES__` build constants.
- npm `files`: `chrome-extension/pro-only/enhanced-capture.js` is listed explicitly, so the
  tarball's copy of `chrome-extension/` is complete against the `manifest.store.json` it also
  ships (which names the file as a web-accessible resource); `function-tracker.js` is not;
  `!scripts/export-licence-key-hashes.js` keeps the owner-only script out. Note the tarball has
  no `manifest.json` (it never had: only the store manifest is listed), so its
  `chrome-extension/` folder is not loadable unpacked as-is; the loadable npm copy is
  `dist/chromedebug-extension.zip`, which `prepublishOnly` builds and verifies.
- `chrome-extension/pro/` is a misnomer: the frame editor, video player and video exporter
  live there and ship to everyone. That is why the `files` glob `chrome-extension/pro/*.js`
  exists - and why it leaked the two paid modules to npm through 2.7.7 when they also lived in
  that directory.

#### Release placeholders

Values the owner has not supplied yet use sentinels that the guard refuses to package:
`'SET_BEFORE_RELEASE'` for strings and `0` / `[]` for ids. The guard scans every built file for
the literal `SET_BEFORE_RELEASE` (comments included - do not write the literal in a comment),
evaluates `licence-activate.js` for zero ids, and refuses the placeholder hash list.

- None as of 2026-09-17. `PRIVACY_POLICY_URL` is `https://www.dynamicupgrade.com/chromedebug/privacy` (host already
  allowlisted), and the owner's first export replaced the placeholder hash list with the Web
  Store reviewer's QA key plus padding. A real licence key that had been published in npm
  2.5.7-2.8.0 (hardcoded in `src/commands/install-pro.js`) was disabled in Lemon Squeezy first, so
  the export excluded it. Before any later export, check that no key being exported appears in git
  history or a published package.

Already supplied by the owner (2026-09-16), so no longer sentinels: `SUPPORT_CONTACT` is
`support@dynamicupgrade.com` (the Google Workspace alias must exist before release; the guard
cannot check that), and `REFUND_WINDOW_DAYS` is `14`. On 2026-09-17 the live Lemon Squeezy ids were pinned in both
`chrome-extension/licence-activate.js` and `tests/fixtures/lemonsqueezy-ids.json`: product
`662307` (from the owner) and store `223516` (read from the owner's live checkout page, which lists
it beside that product). The export still refuses any refund window
that is not a whole number of days of at least 1: "younger than 0 days" would exclude nothing.

Never replace a sentinel with a "probable" value and a comment claiming the owner confirmed it.
Fill one in only with a value the owner actually supplied, and change the test that pins the
sentinel in the same edit.

Tests override the ids by anchored substitution of the `const LS_STORE_ID = ...;` and
`const LS_PRODUCT_IDS = [...];` declarations in a copy of the source, asserting the match: test
ids for a configured build, or the sentinels `0` / `[]` to exercise the unconfigured path. Do not add runtime override hooks to
shipped code.

When `PRIVACY_POLICY_URL` (or a URL-shaped `SUPPORT_CONTACT`) is filled in, add its host to
`ALLOWED_HOSTS` in `tests/external-link-hosts.test.js` with a reason. That test scans tracked AND
untracked files under `chrome-extension/`, comments included, so never paste documentation URLs
(developer.chrome.com, docs.lemonsqueezy.com, raw.githubusercontent.com, ...) into extension
files; cite them in `tests/` or here instead.

#### Owner export of the key-hash list (before each release)

```bash
LEMONSQUEEZY_API_KEY=<new LIVE-mode key> node scripts/export-licence-key-hashes.js \
  [--hand-issued /path/OUTSIDE/the/repo/hand-issued-keys.json] [--dry-run]
```

- Owner-local only: never in CI, never deployed, excluded from npm `files`. It only issues GETs.
- It refuses to run while `LS_STORE_ID` / `LS_PRODUCT_IDS` or `REFUND_WINDOW_DAYS` are sentinels,
  while `tests/fixtures/lemonsqueezy-ids.json` disagrees with `licence-activate.js`, while the
  API key is missing, or when any key/order is test-mode or from another store/product.
- Keeps `active`/`inactive` keys on paid orders OLDER than `REFUND_WINDOW_DAYS` (D8); expired,
  disabled, refunded, unpaid and in-window keys are counted and left out. It warns (with the hash)
  when a key that already ships has since been disabled, expired or refunded: add that hash to
  `tests/fixtures/licence-key-hash-removals.json` (`{"removals": [{"hash", "reason"}]}`) if it
  should stop unlocking new browsers.
- `--hand-issued` is an off-repo JSON array of `{"key", "note"}` (for example the Web Store
  reviewer's QA key). Plaintext keys never enter the repo; only hashes are written. Each
  hand-issued key must be at least 32 characters (use a random UUID): the hash list is public,
  so a short key could be recovered from its digest.
- The FIRST export replaces the never-shipped placeholder list (256 entries of unknown
  provenance) rather than keeping it; every later export is append-only against the lock.
  Padding is generated by the script and the list only grows by whole buckets of 256. There is
  no off-repo padding register to keep or lose (`--register` is refused as unknown).
- It writes `tests/fixtures/licence-key-hashes.lock.json`, then
  `chrome-extension/licence-key-hashes.js` (whose `CHROMEDEBUG_KEY_HASHES_META.source` stops being
  the sentinel). Review the diff before committing.

#### Licence UI behaviour worth knowing

- Mouse tracking is free for everyone (D13); it is not a PRO gate.
- The popup's "I have a licence key" control sits under the trial counter for every unlicensed
  browser, not only inside the 25/25 cap card: a buyer on a new machine, a second profile or a
  reinstall still has recordings left and must find the paste box in one click.
- The enhanced-capture control in the popup is hidden unless licensed, and a stored "on" is
  forced off while unlicensed. It is never shown as a PRO upsell.
- Every programmatic content-script injection (`chrome.scripting.executeScript`) injects
  `['entitlement.js', 'content.js']`, in that order: `content.js` reads `ChromeDebugEntitlement`
  to decide whether to load enhanced capture. `entitlement.js` is safe to evaluate twice.
- The frame editor's MP4 export button is styled as locked when unlicensed but is NOT
  disabled, so a click can open the upgrade modal.
- Content scripts read licence state once per page load; tabs opened before an unlock need a
  reload ("Reload tabs that were already open.").

#### Removed: the Firebase/Cloud Functions licence layer

The Firebase/Cloud Functions licence layer, its source, and all `/chromedebug/license/*` routes
stay deleted. Activation happens once, in the extension service worker, against the bundled
hash list or api.lemonsqueezy.com: never on read, never through a server we run, never through
the local HTTP server. No expiry, device limits, periodic re-validation or automatic removal.

History: `license-helper.js`, `firebase-client.js`, `activation-manager.*`,
`firebase-config*.js`, `src/firebase-license-manager.js` and the eight `/chromedebug/license/*`
routes were deleted first. None had a consumer, and several routes were a confused deputy,
running caller-supplied ids against Firestore under an Admin service account. The backend
source (`functions/`, `firebase.json`, `.firebaserc`, `firestore.rules`,
`firestore.indexes.json`) was then removed so a single `firebase deploy` cannot recreate the
unauthenticated functions. The deployed resources are the owner's to delete out of band. The
removed `.firebaserc` named the project `chromedebug-mcp`, but the real Firebase project id was
`chrome-debug-mcp`; do not rely on the old file. `install-pro` is now an informational stub that
writes nothing. Covered by `tests/no-firebase-source.test.js`,
`tests/no-local-licence-routes.test.js` and `tests/install-pro-stub.test.js`.

#### Unchanged by the licence work (stated explicitly)

- The entire local HTTP server auth model: the loopback bypass reads
  `req.socket.remoteAddress`, `hostAllowlist` runs first, there is no JWT/Bearer path, the
  localhost role stays `ROLES.USER`, and there are no licence routes. The local HTTP server and
  the MCP server know nothing about tiers.
- "Never fabricate a clean zero in a catch": report `null` plus an `error`.

### MCP Tools Available

- `launch_chrome` - Start new Chrome instance
- `connect_to_existing_chrome` - Connect to existing Chrome with debugging enabled
- `navigate_to` - Navigate to URL
- `take_screenshot` - Capture page screenshot (AI-optimized by default)
- `get_page_content` - Extract page text, HTML, and DOM structure
- `evaluate_expression` - Execute JavaScript
- `get_selected_element` - Get currently selected element
- `apply_css_to_selected` - Apply CSS to selected element
- `execute_js_on_selected` - Run JS on selected element
- `clear_selected_element` - Clear current element selection
- `pause_execution` / `resume_execution` - Debugger control
- `step_over` - Step over in debugger
- `set_breakpoint` - Set debugging breakpoints
- `get_scopes` - Get scope variables from top call frame
- `get_logs` - Retrieve console logs
- `check_connection` - Check Chrome connection status
- `force_reset` - Force reset Chrome instance
- `get_workflow_recording` - Retrieve workflow recording with actions/logs (WARNING: Can be very large)
- `list_workflow_recordings` - List all workflow recordings
- `get_workflow_function_traces` - Get only function execution traces from a workflow
- `get_workflow_errors` - Get only error logs and error-related actions
- `get_workflow_summary` - Get a concise summary of a workflow recording
- `get_workflow_actions_filtered` - Get filtered workflow actions by type with pagination
- `get_workflow_screenshot` - Get one workflow action's screenshot as a viewable image

**Note:** the workflow tools above return `hasScreenshot` / `actionIndex` / `screenshotChars`
instead of the base64 bytes, plus a single `screenshotNote` on the response when at least one
returned action has an image. Pass that `actionIndex` to `get_workflow_screenshot` to view a
single image. Returning the bytes inline would serialize one full-quality JPEG per recorded
click into the response.
- `save_restore_point` - Save browser state restore point
- `restore_from_point` - Restore browser to saved point
- `list_restore_points` - List restore points for workflow
- `chromedebug_show_frames` - Display frame recording info
- `list_frame_sessions` - List saved screen/video recordings newest first with their session
  IDs. Paged (`limit` default 50, `0` = all) and always reports `total` / `hasMore`, plus the
  `dbPath` it read AND `dbFile` (the same `ok|missing|replaced|unknown` vocabulary
  `/chromedebug/status` uses). `dbPath` alone is a string and cannot tell you the handle is
  stale: five servers spent a week reporting the correct path while holding a DELETED copy
  of the database, and a query on a deleted handle succeeds and returns nothing. So when
  `dbFile` is `missing`/`replaced` this tool **throws** rather than returning an empty list
  (a fabricated clean zero), and returns rows with a `warning` when it has some.
  `list_workflow_recordings`, `get_frame_session_info` and `chromedebug_show_frames` now
  report it too. All seven HTTP-first MCP read paths were converted to direct reads and the
  `useHttpClient` flag deleted, so there is one handle to be honest about. The decision
  table lives once, in `src/mcp/handlers/database-identity.js`.
- `get_frame_session_info` - Get frame capture session info
- `get_frame` - Get specific frame from recording
- `get_frame_screenshot` - Get screenshot image data for a specific frame as base64
- `search_frame_logs` - Search logs across all frames
- `get_frame_logs_paginated` - Get paginated frame logs
- `play_workflow_recording` - Replay a saved workflow recording
- `play_workflow_by_name` - Replay workflow by recording name
- `capture_screenshot` - High-quality Puppeteer capture (`current_view` / `full_page` / `section`).
  `section` **requires** a `selector` and errors without one. Delegates to
  `ChromeController.captureScreenshotWithMode`.
- `get_screenshot_capture` - Retrieve one stored capture by `captureId`
- `list_screenshot_captures` - List stored capture metadata (no image bytes)

**Note:** `capture_screenshot` returns prefix-less base64 in `screenshot`, but the copy it
stores keeps a `data:image/jpeg;base64,` prefix. That asymmetry is deliberate:
`get_screenshot_capture` reads the stored form under `fallbackMimeType: null`, so a
prefix-less stored value would fail closed. Capture storage is an in-memory Map in
`src/http-server.js` and does **not** survive a server restart.

## Important Files and Locations

- Chrome user data: Temporary directories in OS temp folder
- Database: `~/.chromedebug/chromedebug.db` (SQLite), or `CHROMEDEBUG_DB_PATH` when set.
  `data/chromedebug.db` and `data/chrome-pilot.db` are abandoned local leftovers, not the
  live database; `list_frame_sessions` reports the path it actually read for this reason.
- Chrome extension: `chrome-extension/` directory (Web Store manifest: `manifest.store.json`; dev manifest: `manifest.json`)
- Extension build: `dist/extension`, zipped to `dist/chromedebug-extension.zip`
- Release pins: `tests/fixtures/lemonsqueezy-ids.json`, `tests/fixtures/licence-key-hashes.lock.json`, `tests/fixtures/licence-key-hash-removals.json`
- VS Code extension: `vscode-chrome-pilot/` directory
- HTTP server logs: `http-server.log`
- Configuration: `config/chrome-pilot-config.json`
- Test files: `test-*.js` in root directory

## Screenshot Retrieval Feature

### New MCP Tool: `get_frame_screenshot`
The `get_frame_screenshot` tool enables viewing of actual screenshots captured during frame recordings:

```javascript
// Get screenshot for frame 0 with metadata
{
  "name": "get_frame_screenshot",
  "arguments": {
    "sessionId": "frame_1756936437368_tsvn4ehi0",
    "frameIndex": 0,
    "includeMetadata": true
  }
}
```

The response carries **two** MCP content blocks: a small `text` block with the metadata, and a
real `image` block the AI client renders. The base64 is NOT in the text block — it is replaced
by a short sentinel, because emitting the payload as JSON text is both unviewable and expensive.

- `imageData`: in the text block, a sentinel such as
  `[22220 base64 chars returned as an MCP image content block]`. The actual bytes travel in the
  image block. If promotion is declined (unknown mime type, invalid base64, or over
  `MAX_IMAGE_BASE64_CHARS`), the sentinel says so and the payload is still suppressed.
- `imageFormat`: derived from the stored data URL, not hardcoded — a PNG frame reports `png`
- `metadata`: optional size, encoding, and capture time info
- Full error handling for invalid sessions/frames

### Usage Examples
- **View specific moments**: Get screenshots from any frame in a recording
- **Debug visual issues**: See exactly what was displayed when an error occurred
- **Extract UI snapshots**: Save screenshots for documentation or analysis
- **Verify user interactions**: Visual confirmation of what users clicked

## Common Workflows

### Adding New MCP Tools
1. Add the tool definition to the array in `src/mcp/tools/index.js` (NOT `src/index.js` — the
   monolithic tools array named there no longer exists).
2. Implement a handler method in the matching per-domain handler under `src/mcp/handlers/`
   (chrome / frame / workflow / element) and add a `case` for it in that handler's `handle()`
   switch, plus its name in the routing array in `RequestHandler.routeToolCall`.
3. Add the underlying method to `ChromeController` **and** a delegating method in
   `src/services/chrome-service.js`. Handlers are constructed with a **ChromeService**, not a
   ChromeController, despite the field being named `this.chromeController`. Skipping the
   delegation means every call throws `TypeError` while the whole suite stays green — that is
   exactly how `capture_screenshot` shipped broken. `tests/service-surface-contract.test.js`
   enforces both hops.
4. **If the tool returns image bytes**, add it to `IMAGE_RESULT_FIELDS` in
   `src/mcp/handlers/request-handler.js` and to the drift test in
   `tests/request-handler-image-blocks.test.js`. Without that entry the base64 is
   emitted as JSON text and no AI client can actually view the image.

### Adding New HTTP Endpoints
1. Add route in `src/http-server.js`
2. Implement logic using `chromeController` instance
3. Update README.md with endpoint documentation

### Modifying Chrome Extension
1. Edit files in `chrome-extension/` directory
2. Reload extension in Chrome (chrome://extensions/)
3. Test functionality before packaging

## Error Handling

- All async operations use `withTimeout()` wrapper
- Chrome processes are tracked and killed on cleanup
- Database operations use transactions for consistency

## Security Considerations

- CORS enabled for cross-origin requests
- Request size limits: 50MB for JSON/URL-encoded
- Chrome runs with separate user data directories
- No persistent cookies between sessions

### MCP read paths go direct to the database, not over HTTP

`src/services/http-client.js` used to cache `process.env.MCP_SERVICE_TOKEN` in its
constructor. It is a module-level singleton, and `src/index.js` imports the MCP handlers
(which import it) **before** `chrome-service.js` (which pulls in `middleware/auth.js`, which
sets that variable). ESM evaluates depth-first, so the token was `undefined` in every MCP
process, deterministically - and `request()` threw client-side before sending anything.

Every HTTP read in the MCP process therefore failed 100% of the time. Most tools survived on
their direct-DB fallbacks. The ones without a fallback silently misreported instead:

- `chromedebug_show_frames` returned `totalFound: 0` interactions for **every frame of every
  recording ever made**, because its catch substituted a hardcoded zeroed metadata object.
- `get_frame_session_info` reported `totalInteractions: 0` for every session, same cause.
- `capture_screenshot` returned `captureId: null` with a `storageError` on every call, so the
  capture store was never written to.

The token is now read per request, and a missing token no longer throws - `baseUrl` is always
`http://localhost:<port>` and `authenticate()` admits any loopback peer before it looks at a
token, so the client-side gate had no security value and acted as a self-inflicted outage.

**Rules going forward:**
- MCP handlers read via `ChromeService` / the `database` singleton **directly**. Do not
  reintroduce HTTP-first-with-fallback on a read path; it buys no auth (loopback bypass), costs
  a round trip, and drags full base64 images over the wire to read metadata.
- `httpClient` legitimately remains for the screenshot-capture endpoints, where the in-memory
  Map in `http-server.js` genuinely is the system of record.
- The token throw hid two wiring bugs that only appear once HTTP actually runs:
  `getFrameSessionInfo` pointed at `/chromedebug/frame-session/` (the full session, no
  `totalFrames`) instead of `/chromedebug/frame-session-info/`, and
  `getScreenInteractions` sent `frameIndex`/`type` as query parameters the route ignores.
- **Never fabricate a clean zero in a catch.** `{totalFound: 0}` is indistinguishable from a
  true negative, which is the only reason this survived. Report `null` plus an `error` field.

Covered by `tests/mcp-interaction-association.test.js`.

### Frame annotations: draw on a frame, and the AI sees the drawing AND the note

`chrome-extension/pro/frame-annotator.js` (UI), `frame_annotations` table, route
`POST /chromedebug/frame-annotations/:sessionId/:frameIndex`. Covered by
`tests/frame-annotations.test.js`. **Free and unconditional** - PRO is pinned to exactly four
things and nothing else may be advertised, and this is none of them.

- **A SEPARATE TABLE, never columns on `frames`.** `storeFrameBatch` writes with
  `INSERT OR REPLACE`, and SQLite's REPLACE DELETES the conflicting row and inserts a new one,
  so every column it does not name reverts to its default AND the row id changes. Measured:
  annotate frame 0, re-post the frame, annotation gone. `upload-manager.js` re-posts on retry,
  so this is routine, not exotic. The changed id is also why the FK is to `recordings`, not to
  `frames.id` - a FK there would cascade-delete the annotation just as effectively.
- **Upsert with `ON CONFLICT ... DO UPDATE`**, never `INSERT OR REPLACE`; repeating the exact
  pattern that forced the separate table would be a poor joke. An empty annotation deletes the
  row. A frame that does not exist is refused rather than creating an orphan.
- **ONE route, no `app.delete(`.** Every delete route is auto-required to carry
  `requireLiveDatabase`, whose DELETE wording is "Recordings were NOT deleted" - true of a
  recording, wrong and alarming for erasing a circle. The POST clears instead.
- **Two surfaces, deliberately different.** The modal shows a possibly-WATERMARKED `<img>`;
  the user draws on a TRANSPARENT canvas over it; the save composites onto the CLEAN original
  in `frame.imageData`. Collapsing them breaks both ways - draw on the watermarked copy and a
  later PRO buyer has a "FREE" badge burnt in forever; display the stored composite and an
  unlicensed user can right-click a clean image out of the modal. Because the overlay is
  transparent, the annotator never reads entitlement at all, so `EntitlementNotReady` cannot
  be got wrong here.
- **NOTES BELONG TO SHAPES, not to the frame.** The first version had one note per picture,
  which quietly assumed you would only ever point at one thing. The owner's first two minutes
  of real use: "there may be a few annotations that you want to add notes to". Circling three
  problems and saying what each one is, is the actual job, and a single caption cannot do it -
  a second circle has nothing to say for itself and the model must guess which sentence goes
  with which ring. Shapes are NUMBERED, each badge is drawn on the image, and the tools return
  `notes: [{shape, type, note}]` keyed to those numbers, so "shape 2" is unambiguous to the
  model and the human. A shape with no note is kept and simply omitted from `notes` - a
  pointer with nothing to say is still a valid pointer.
- **The notes travel TWICE, on purpose.** Rendered as a numbered legend BELOW the image (never
  over it - the drawing is the point) and returned as plain strings. A model reads a string
  reliably and reads words rendered into a JPEG unreliably: the drawing says WHERE, the note
  says WHAT.
- **No Annotate button and NO SAVE BUTTON. Annotations save themselves.** The owner never
  saw the Save button: they drew, typed a note, and asked "did you get this?" twice, with
  nothing stored either time. A commit gesture suits a document; the unit of work here is
  "I pointed at a thing", so the UI must not ask for one. An amber "NOT SAVED" warning was
  shipped first and was the weaker answer - it asked the user to do the work rather than
  removing the way to get it wrong. The autosave rules, each load-bearing:
  - ONE timer (~900ms) reset on every change, so typing never triggers a mid-word
    composite and the encode lands in a natural pause.
  - SINGLE-FLIGHT with exactly one coalesced follow-up, re-snapshotted when it fires. N
    changes during a slow save cost two writes and the last one wins.
  - **Emptying the row is NEVER debounced.** Zero shapes DELETE the row, and Clear was safe
    only because a human still had to press Save afterwards. A delete fires immediately, at
    a confirmed gesture, with the user present to read the receipt - and `flushAnnotationSave`
    refuses an empty payload so closing can never delete through the back door.
  - FAILURE KEEPS THE WORK: shapes stay on the canvas, go into `unsavedByFrame`, and
    reopening the frame restores them in preference to the older stored copy. Without that,
    "draw, close, the flush fails, reopen" loses the work silently - worse than the button
    ever was. A Retry button appears ONLY in the failure state; a control that shows up when
    something went wrong is the opposite of the toll booth the owner objected to.
  - The receipt carries a clock time: a bare "Saved" from four edits ago is
    indistinguishable from one a second old.
- **`reset()` is silent; `clear()` is not.** Closing the modal must wipe the canvas, or the
  shapes would still be there when the next frame opens. But `clear()` notifies, and
  `closeFrameModal` used to call it - so once saving is on a timer, EVERY Escape would
  schedule a zero-shape save and delete the row. `reset()` has the same effect with no edit
  semantics. Undo also restores a Clear now (`clear()` reassigns rather than mutates, so the
  old array survives); before that, Clear was the one gesture Undo could not reverse.
- **The annotator has TWO callbacks, and merging them is a data-loss bug.**
  `onGeometryChanged` repositions the note box (fired by `select`); `onChanged` persists
  (fired by `setNote`, `undo`, `clear`, `removeShape`, a committed shape, a finished
  resize). One hook conflated them, which is what made the close path destructive - and the
  geometry handler focuses the note input, so routing typing through it would reset the
  caret on every keystroke.
- **RESIZE uses two handle models, because the shapes are not the same kind of thing.**
  Handles appear only on the SELECTED shape and are hit-tested BEFORE `#shapeAt`, since they
  sit on the same outline band that selects - the other order would reselect and never start
  a resize. rect/ellipse get four corners with the opposite corner pinned and the box
  renormalised; an ARROW gets two endpoint handles, because `(x,y)` is the tail, `(x+w,y+h)`
  the head and w/h are SIGNED, so a corner model would silently flip arrows round. Resize
  fires `onChanged` on RELEASE, never per `pointermove` (that would be a composite and a
  POST per pixel), refuses while `#scale` is 0 like drawing does, and enforces a minimum
  size so a shape cannot be resized into something unselectable. Handles only on the
  selected shape is also what keeps the canvas drawable - see the outline-only hit test.
- **Per-shape delete is a button in the note popup, not the Delete key.** Selecting a shape
  focuses the note input, so a shortcut gated on "focus is not in the input" is unreachable
  in practice. `removeShape` clears the selection rather than clamping it, for the reason
  `undo()` records: clamping moved the note box onto a DIFFERENT shape.
- **There is no "save the shapes now, the picture later" shortcut**, and the reason is in
  the SQL: the upsert does `annotated_image_data = excluded.annotated_image_data`, so a
  shapes-only POST does not leave the stored composite alone - it overwrites it with NULL.
  A two-phase save would blank a good picture on every cheap write and leave the row
  permanently picture-less if the second phase never landed, while the receipt said "Saved".
  Note edits need a re-composite too: the notes are burnt into the legend by design.
  Measured, for anyone tempted again: the largest frame in a real database is 20 KB, 0.6% of
  the 3.4M budget, so the quality ladder essentially never fires. But watch RESOLUTION, not
  bytes - the popup exposes Format/Quality/Max Width, maxWidth is empty by default and the
  resize only runs when it is set, so a PNG at quality 100 on a retina display is ~5.7M
  pixels to encode. If that ever lags, use OffscreenCanvas + convertToBlob, not a split.
- **The tools are always on.** Opening a frame full-size is already the
  gesture that means "let me look at this one properly"; a second click to enable drawing was
  a toll booth. The note box floats anchored under the selected shape, and clicking an
  existing shape reopens its note, which is how you come back and annotate something later.
- **The toolbar is a SIBLING of `.modal-body`, not inside it, and `.modal-content` is a flex
  column.** "Always on" is a layout promise as much as a behaviour one, and it was broken
  twice over on first build: `.modal-content` is `max-height: 90vh; overflow: hidden` while
  `.modal-image` claimed `calc(90vh - 120px)`, a budget written when the header was the only
  other child - so the bar rendered and was then sliced off the bottom edge, which the owner
  reported (correctly) as the menu never appearing. The flex column gives each child its own
  track; keeping the bar OUT of `.modal-body` matters separately, because the body is the
  scroll container and a toolbar inside it can still be scrolled out of sight. Pinned by
  "the toolbar cannot be scrolled or clipped out of sight", which parses the HTML with jsdom
  and asserts `modalBody.contains(bar) === false` - a string match on the markup passes
  throughout the entire bug, because the bar was always there.
- **`[hidden]` needs an AUTHOR rule, and this page has one** (`[hidden] { display: none
  !important; }`, same form as `chrome-extension/recording-controls.html`). The attribute
  hides an element ONLY through the user-agent rule `[hidden] { display: none }`, and the
  cascade sorts by ORIGIN before specificity, so any author `display` beats it. This page
  sets `display` on `.annot-note-pop` and `.annot-tools` and toggles both with
  `el.hidden = ...`, which was therefore inert: the note box rendered the instant a frame
  opened, over the toolbar, and every "cannot annotate here" state still showed live
  drawing buttons whose handlers were never even wired. `!important` is load-bearing - an
  attribute and a class selector tie at (0,1,0), so without it the rule wins only by source
  order and the next appended `.foo { display: flex }` re-opens it. Do not test this with
  jsdom `getComputedStyle`: it does not implement the origin fight the bug turns on.
- **The extension and the server update on different clocks, so the editor asks the
  RECORDING what its server supports.** The Web Store moves the extension; only
  `npm i -g` moves the server, and a running one keeps its old code until restarted. The
  `frame-session` payload is the capability probe: `annotations` present (even `[]`) means
  supported, `annotationsError` means supported-but-unreadable, NEITHER means a server
  older than the feature. Classify by KEY PRESENCE - `data.annotations || []` collapsed all
  three into one empty Map. That is not cosmetic: every server on a machine shares ONE
  database file, so an old server on an earlier port serves the same recording's frames
  while reporting no annotations, the editor opened a blank canvas over work that exists,
  and Save (looping past that server's 404) upserted over it. Pinned by a server test that
  the route always emits one of the two keys; a future "omit empty arrays" tidy-up would
  otherwise make every new server look old to every extension. Do NOT version-compare and
  do NOT add a `capabilities` array to `/chromedebug/status` for this - key presence is
  self-describing, per-recording, and already in a request the editor makes.
- **The editor picks a server by RANK, not by arrival order.** `listServers()` in
  `background.js` (factored out of `resolveRecordingServer`, which is now its head) probes
  `/chromedebug/status` on every configured port in parallel and sorts by `dbFile`, then
  version, then port. The frame editor asks for it over a `resolveServers` message
  (`isExtensionPageSender` only) and falls back to the FULL configured list, never an empty
  one. Covered by `tests/server-resolver.test.js`.
  Four call sites in `frame-editor.js` each had their own `ports.slice(0, 5)` and took the
  first server that answered - "lowest port wins", a second selection policy that disagreed
  with the resolver, plus a blind spot for every server past the fifth port. Two routine
  consequences: a server too OLD for a feature won on port order (the Web Store moves the
  extension, only `npm i -g` moves the server, and `--persistent` is a documented workflow,
  so mixed versions are normal), and a STALE server won the same way, because reads are
  deliberately never gated so a ghost holding an unlinked copy answers happily - the editor
  showed its rows while a save went elsewhere. Measured on the owner's machine: old
  behaviour chose 3000 (2.9.2, no annotations), ranked chooses 3032 (2.9.3) - the ninth
  configured port, which the truncation had made unreachable.
  Do NOT give the editor the resolver's `preferred` shortcut: that returns the pinned server
  **ignoring `dbFile`**, which is right for a recording already in flight and wrong for
  choosing where to read from.
- **Never probe `POST /chromedebug/frame-annotations/...`.** `shapes` defaults to `[]` in
  `frameAnnotationSchema` and zero shapes DELETES the row, so a bare `POST {}` is a VALID
  request that erases the annotation on whatever frame it names. The capability probe is the
  `annotations` key on the read-only `GET /chromedebug/frame-session/:sessionId`, which the
  editor already fetches. A comment on the route says so; pinned by a test.
- **A save goes to the server the annotations were READ from, first**, then the rest of the
  ranked list. They share a database file, so saving to a different server than the editor
  read from still "works" - which is how an edit lands over rows the user was never shown.
- **The missing-route case is matched on the response BODY, never on status 404.** This one
  path produces three different 404s: the catch-all (`error: 'Endpoint not found'` plus
  `availableEndpoints`), the route-level one (`{error, port}` - no such frame HERE), and
  bad params, which the validators fail as 400. A real error from a capable server is
  preferred over "nobody had the route", and if the server that SERVED the annotations then
  calls the route unknown, the message blames the extension, not the server - the opposite
  would be another confident wrong message of the kind this file keeps recording.
- **The CALLER snapshots; `saveAnnotation` is handed the result and never reads the live
  annotator.** Taking the snapshot inside it was safe only while the trigger was a button
  click re-wired on every open. The annotator is ONE shared instance reloaded per frame, so
  a timer or a flush-on-close would composite THIS frame's picture with whatever frame is
  open NOW and upsert it onto this frame's row - both frames wrong, and the status guard
  suppressing the message. A per-open session token (`annotSession`) guards the snapshot,
  `markAnnotationSaved` and every post-save status write; `openFrameIndex` alone is not
  enough, because reopening the SAME frame passes that check while old closures are live.
  The original bug this replaced: `closeFrameModal()`
  calls `annotator.clear()`, and `buildComposite()` is slow enough to lose the race: draw,
  Save, Escape - the natural gesture - posted `shapes: []` beside a fully drawn image, and
  an empty annotation DELETES the row, so it destroyed the new drawing AND whatever the
  frame already had. The success message reports the snapshot too.
- **`renderFrames()` must re-observe its thumbnails, and a Save must not call it.**
  `setupLazyLoading()` runs ONCE from `init()` and observes the elements present then, so
  any later render leaves unobserved thumbnails stuck on "Loading..." for good. A
  `renderFrames()` added after Save therefore wiped the entire strip - to refresh an
  annotation badge that does not exist, because `createFrameElement` never reads
  `frameAnnotations`. `observeThumbnails()` now makes a re-render safe for any caller.
- **`#annotStatus` is one element shared by every frame**, so a Save resolving after the
  modal reopened elsewhere relabelled the new frame with the old one's result. Writes are
  dropped once `openFrameIndex` has moved on.
- **The contextmenu upsell matches `.modal-image-stack`.** The annotation canvas covers the
  modal `<img>` with `pointerEvents:'auto'`, so the right-click target is the CANVAS and
  the `tagName === 'IMG'` test stopped matching. Nothing clean leaked - the displayed image
  is watermarked for an unlicensed browser and the canvas holds only the transparent shape
  layer - but the prompt for a sold feature silently stopped appearing.
- **Per-ROW errors reach the editor too.** `getFrameAnnotations` returns
  `{shapes: [], error}` for a row whose JSON will not parse; the route used to map only
  `{frameIndex, shapes, note}`, dropping `error` and carrying a `note` field that stopped
  existing when notes moved onto shapes. A corrupt row arrived as "no annotation" and a
  save would upsert straight over it. Drawing is blocked on such a frame.
- **ARRAY POSITION IS NOT `frame_index`, and the editor must resolve it.** `getFrameSession`
  copies `frame_index` onto `frame.index` and deletes the raw column; `storeFrameBatch`
  honours a frame's own index (`database.js` ~958), so indices are contiguous only when
  every batch landed - and `requireLiveDatabase` exists to REJECT batches, which leaves a
  hole by design. Keyed by position, every thumbnail after a hole read and wrote its
  NEIGHBOUR's annotation, overwriting it through `ON CONFLICT DO UPDATE`, with the picture
  on screen and the row being written belonging to different frames. `dbIndex` is derived
  once in `setupAnnotator` and once in `saveAnnotation`. The fixture that makes this class
  visible is a recording with frames `0, 1, 5`; every earlier fixture was contiguous, which
  is why nothing caught it.
- **The three annotation DB methods resolve the id through `getRecording()`**, like every
  other frame read. It accepts a `frame_` prefix and the `session_id` alias, so using the
  raw argument meant `getFrameSession` returned frames for an alias while the annotations
  came back empty and a save answered "no frame" - the two halves of one feature
  disagreeing about what a recording id is.
- **`iw`/`ih` (the frame's natural size) are stored beside the shapes.** Shapes are in
  natural image pixels and nothing else recorded what that image measured, so a stored
  coordinate was uninterpretable to anything that had not decoded the JPEG - and the server
  never does. Optional, so an older client can still save; `v` stays 1 because readers
  accept `v >= 1` and fields are only ever added. It cannot be backfilled for rows written
  without it, which is why it went in before the first real annotation existed.
- **`#scale` returns 0 rather than guessing while the image decodes.** `naturalWidth` is 0
  until decode and the old fallback made the scale exactly 1, silently storing DISPLAY
  pixels in a field documented as natural pixels. The window is real: an unlicensed browser
  reassigns `src` a second time with the watermarked copy, asynchronously. `pointerdown`
  refuses to start a shape at scale 0 - a don't-know is not a 1.
- **The number badge is CLAMPED inside the canvas.** Unclamped, a shape drawn against the
  top or left edge put most of its number circle off-canvas, and circling something in a
  screenshot's top-left corner is one of the most natural gestures there is. The badge is
  the entire reason a note can say "shape 2" and be understood.
- **An oversized composite degrades to shapes-only; it does not lose the notes.** The
  shapes and their text are a few hundred bytes and are the half a model reads reliably.
  The server already accepts a null composite and `get_frame_screenshot` already explains
  that case - the client was the only layer treating it as fatal, on exactly the
  high-resolution frames most worth annotating.
- **A `null` `sessionServer` BLOCKS too.** On the `chrome.storage` fallback no server
  answered, so there is nothing to POST to. Without that branch the toolbar was fully live
  there and the failure arrived only at Save, after the drawing was done - the exact dead
  control the rest of the chain prevents.
- **Closing the modal hides it BEFORE anything that can throw.** The `setEnabled()` bug
  trapped the user because the throw preceded `classList.remove('visible')`. Ordering is
  the fix; a try/catch around the tidying would only have hidden the next one.
- **A GUARD THAT CAN NEVER PASS FAILS CLOSED AND SILENTLY.** `this.annotSession` was not
  initialised, so `++this.annotSession` was `NaN`, `NaN !== NaN` is true, and the session
  guard in `snapshotAnnotation()` refused EVERY save. The status sat on "Not saved yet -
  saving..." for ever, because the save was being DECLINED rather than failing - nothing
  threw, nothing logged, no error surfaced. Eight source pins asserted that guard EXISTED;
  not one asserted it ever PASSES, so the suite stayed green while the feature was
  completely dead. The fix is one line; the lesson is that
  `tests/frame-annotations.test.js` now constructs a REAL `FrameEditor` in jsdom (stub
  `init`, which would otherwise load a session) and drives change -> debounce -> POST with
  a stubbed `fetch` and fake timers. Any new guard on the save path needs a test that
  proves a save still happens, not merely that the guard is present.
- **The annotator is tested by RUNNING it**, not by regex over its source: the IIFE is
  loaded with `new Function('self', ...)` into a jsdom document, `getContext('2d')` returns
  null and `render()` guards on that, so everything except pixel output is reachable. Pin
  properties rather than exact source literals - a `const payload = { shapes,
  annotatedImageData }` pin broke the moment `iw`/`ih` were added, and a fixed 2600-char
  body slice broke when a comment grew.
- **"Empty" means no shapes.** A note with no shape would be a caption floating over a picture
  with nothing to indicate what it refers to, so saving with zero shapes deletes the row.
  With autosave that makes emptying the row a destructive act, which is why it is the one
  save that is never debounced and never flushed on close - see the autosave rules above.
- **Size budget.** `MAX_IMAGE_BASE64_CHARS` is 4,000,000 in `request-handler.js`; over that
  the payload is replaced by a sentinel and the AI sees nothing. The composite steps JPEG
  quality down until it fits, Joi caps it at 3.5M, and the client aims at 3.4M. An oversized
  composite is refused at the door rather than stored and silently never shown.
- **Shapes are stored in NATURAL image coordinates**, not display pixels, so an annotation
  still lines up at a different window size. Stroke width scales with image width - a 2px line
  on a 2560px frame is invisible to the model, which would make the feature a no-op on exactly
  the screenshots most worth annotating.
- **Surfaced in three places, and `get_frame_session_info` is the important one**: the frame
  editor's own Copy Prompt tells the AI to call it, so a note visible only through
  `get_frame_screenshot` would never be found. `get_frame_screenshot` serves the composite
  (`original: true` for the clean frame underneath), `chromedebug_show_frames` carries the note
  per frame, and the tool descriptions say so because the model reads those.
- **Deliberately out of scope in v1, and SAID so in the UI** rather than offering a dead
  button: workflow-action screenshots (a different table, `workflow_actions.screenshot_data`)
  and browser-only recordings (which never reach the server, so the AI cannot see the
  RECORDING, let alone a note on it).

### Stale-handle wording lives in ONE place, and `missing` never says "restart"

`src/mcp/handlers/database-identity.js`, used by `list_frame_sessions`,
`list_workflow_recordings`, `get_frame_session_info` and `chromedebug_show_frames`. Covered
by `tests/mcp-tools-report-stale-handle.test.js` and `tests/list-frame-sessions.test.js`.

- **Shared because it is safety text, not formatting.** The dangerous sentence is "restart":
  under `missing` the rows a process still serves exist ONLY inside its open descriptor, so
  a restart destroys them. Four hand-maintained copies is how one drifts.
- **`missing` + an empty list must NOT advise a restart.** The first version did, justified
  by "Nothing is held here to lose". That premise is per-TABLE and the consequence is
  per-HANDLE: one file holds sixteen tables, and `recordings` (frames) and
  `workflow_recordings` are different ones. A user with no frame sessions and forty workflow
  recordings in an unlinked handle was being told, on the strength of the frames list being
  empty, to do the one thing that destroys the forty. Under `missing` the rule is now blunt -
  never advise a restart, whatever the row count - and the message points at the sibling
  list instead. `replaced` keeps the restart advice: a different file is at the path and the
  old one may still exist under another name.
- Decision table: stale + 0 rows -> **throw**; stale + rows -> `warning` (state-specific);
  `unknown` + 0 rows -> hedge, never a calm negative; otherwise nothing, and the caller adds
  its own "nothing here, try the sibling tool" message.
- **Single-session lookups go through it too.** "Frame session not found" read off a stale
  handle is the same fabricated clean zero, one session at a time, so
  `describeSessionNotFound` refuses instead when the handle is stale and the list is empty.
  Consolidating the two copies of that block also removed a sentence that had drifted into
  one of them and was wrong ("Recordings do not persist across different Chrome Debug
  instances" - they share one file, which is the entire premise of this bug).
- **No MCP read path uses `httpClient` any more.** The eight read methods are deleted, not
  merely unused. `request()` and `resolveBaseUrl()` stay for the three screenshot-capture
  endpoints, where an in-memory Map in `http-server.js` genuinely is the system of record.
  A drift test enforces all of this; until now only prose did.
- **Behaviour change worth knowing:** on the `--single-server` / HTTP-failed-to-start
  topology, reads move from "whichever server the port file names" to "this process's own
  handle". An honest `replaced` about your own handle beats silently returning another
  process's rows; that is the semantics `list_frame_sessions` already had.
- **Tool descriptions carry this too** (`src/mcp/tools/index.js`). The model reads those, not
  this file.

### `fileState()` is read-only, and a missing identity is never guessed

`src/database.js`. `openedFile` is written **exactly once, in `init()`**. Covered by
`tests/database-startup.test.js` (including a source guard that `fileState()` contains no
assignment to `openedFile`).

- `init()` prefers the stat taken **before** the open. If the file is swapped mid-open that
  can only be wrong in the safe direction (a false `replaced`).
- `null` and `undefined` from `statFile()` are different facts and must not be merged.
  `null` is ENOENT, so this open CREATED the file and the post-open stat legitimately
  describes it; `undefined` means the stat could not be taken (EACCES, EIO, or a filesystem
  reporting inode 0 for every file) and a post-open stat would be a **guess**. Record
  nothing and answer `unknown` for this process's life. `init()` logs one warning when that
  happens - that is the visibility, rather than a fifth `dbFile` state.
- **Do not add a retry to `fileState()`.** One was added in good faith to stop a single
  unlucky stat blinding a process, and it adopted whatever was at the path - recording the
  replacement's inode against a handle holding the old file, i.e. a permanent false `ok`.
  The trade was strictly bad: the extension's `rankServer()` (`background.js` ~4343) scores
  `ok` 3, `unknown` 2, stale 1, so an honestly-`unknown` server LOSES the rotation, while a
  falsely-`ok` one ties a healthy server and can WIN it - with `requireLiveDatabase`
  disabled at exactly the moment it is chosen. The retry promoted the broken server.
- The `if (!this.openedFile) return 'unknown'` guard is load-bearing: without it the
  comparison dereferences `undefined` and throws into `getDatabaseIdentity()`.

Known and deliberately not built: when `beforeOpen` and `afterOpen` are both known and
DIFFER, `init()` keeps `beforeOpen`, which can yield a false `replaced` - and with the
upload gate in place that now refuses every upload until someone restarts. Closing and
throwing would be self-healing (the catch already closes the handle and leaves
`initialized === false`), but it needs its own error path, because `describeOpenFailure()`
would give "move it aside / .recover" advice for what is really "the file changed while I
was opening it". Nothing has observed this case. Never throw on `undefined`: that would make
the database unopenable on FAT/exFAT/SMB.

### A server that cannot save a recording must refuse it

`src/middleware/require-live-database.js`, on the eight routes that write to the database:
`POST /chromedebug/workflow-recording`, `frame-batch`, `upload/batch`, `upload/:dataType`,
`associate-logs`, `logs/stream`, `screen-interactions/:sessionId`,
`frame-annotations/:sessionId/:frameIndex`. Covered by
`tests/upload-refuses-stale-database.test.js`.

A write to an open SQLite handle whose file has been **unlinked SUCCEEDS** - the rows go into
a nameless inode the kernel frees when the process exits. So a server holding a deleted
database answered 200 to every upload and destroyed it. Nothing else holds a copy: frame
batches are deliberately not mirrored into extension storage in server mode (a 5MB quota
fills in ~12s at 4 FPS, `background.js` ~6470), and workflow actions live only in the content
script until posted. The server is the only copy from the moment of capture.

- **`missing` and `replaced` -> 503. `unknown` and `ok` -> proceed.** `unknown` is a failed
  stat (EACCES, EIO, inode 0 on FAT/SMB) or a database not yet opened; it is not evidence of
  staleness, and refusing a recording because we could not look at a file would destroy real
  data to prevent a hypothetical one.
- **Refusing reroutes rather than discards.** Every extension sender loops over candidate
  ports and continues past a non-2xx, so a 503 hands the recording to the next server. It
  also fixes an existing split: `LogStreamer` and screen-interactions iterate raw config
  order, not resolver order, so logs went to the lowest-numbered server while frames went to
  the ranked-best one. The exception is `upload/batch` via `upload-manager.js` (~187), which
  retries the SAME server 3x with backoff instead of falling through - acceptable, it is the
  unadvertised enhanced-capture path.
- **Server-side is the only place this works.** `resolveRecordingServer({preferred})`
  (`background.js` ~4371) returns the pinned server whenever it identifies as ChromeDebug,
  **ignoring `dbFile`** - so a file that goes stale MID-recording is never rerouted by the
  client.
- **`replaced` is a corruption hazard, not just a doomed copy.** If `-wal` is recreated at the
  path, a stale handle and a healthy new server can hold the SAME WAL inode - SQLite opens
  `-wal` by path and validates frames by salt and checksum, not by which main file they
  belong to - so stale writes can be checkpointed into the LIVE database. `~/.chromedebug`
  still holds an orphaned `chrome-pilot.db-wal` with no main file, that shape's fingerprint.
- **Reads are NEVER gated, and the 503 body must never say "restart".** Under `missing` the
  rows this process still serves exist ONLY inside its open handle; closing it frees them for
  good. Blocking reads, or advising a restart, would be the destructive act. Pinned by tests.
- **Fail open, narrowly.** The try/catch wraps `fileState()` alone. A bug in the detector must
  not stop every recording; a mistake anywhere else must surface as a 500, not "proceed".
- **Import the `database` singleton statically in the middleware**, never
  `activeController.database`. Server tests boot with a stub controller, and
  `{}.database.fileState()` would throw, fail open, and leave the gate silently absent.
- **No cache, no `Retry-After`, no counters, one log line per state TRANSITION.** A cache
  inside a staleness detector is the same stale-belief bug the detector exists to catch; one
  `statSync` is microseconds against a multi-MB body parse.
- It does **not** rescue data. When the only server is stale the recording is still lost
  (`serverMode === 'server'` skips the IndexedDB fallback); the gate removes the lie and the
  extension reports the true rejected count.

**Deletes are gated too, on all FOUR delete routes** (`/chromedebug/recording/:id`,
`/workflow-recording/:id`, `/recordings/all`, `/workflow-recordings/all`). On a stale handle
they ran the DELETE against the ghost file and returned its count while the real database
kept everything - a false success on a PRIVACY control.

An earlier note here said gating them "would convert a wrong success into a refusal without
fixing the lie". **That was wrong.** `popup.js` (~4399, ~4465, ~4560) and `background.js`
(~6252) loop over ports and continue past a non-2xx exactly like the upload senders, so a 503
hands the delete to the server holding the real data and the recordings genuinely go away.

**One delete caller does NOT loop**, and an earlier version of this paragraph claimed all of
them did: `popup.js` ~4865, deleting ONE screen recording from its row, fires a single DELETE
at one port and has no non-ok branch. A 503 there is silent - the row stays on screen and the
user, who just confirmed a destructive action, is told nothing. Nothing is lost and nothing
false is claimed, so it is not the privacy lie the gate exists for, but the reroute argument
does not cover it. Fix belongs with the other popup 503 work below (needs a repackage). Under
`missing` there is a second reason: those ghost rows are the only surviving copy of what was
uploaded there, so deleting them on a request aimed at the real store would lose data AND
leave the real recordings in place. Refuse; do not delete from the ghost "anyway".

The message differs by `req.method === 'DELETE'` - one ternary, no second export and no
factory, because the sentence is the only thing that differs. On an upload the failure is
"you think it saved and it did not"; on a delete it is "you think it is gone and it is not",
which is a privacy promise. The drift test counts `app.delete(` blocks and requires ALL of
them gated, so a new delete route cannot slip through by not being on a list.

**Still open in this area** (tracked, not fixed): demote a pinned server whose `dbFile` is
stale; give `LogStreamer` and screen-interactions `getServerPorts()`; a `VACUUM INTO` rescue
for rows stranded in an unlinked handle; delete the `debug-interactions` route (it only
echoes its body and has `authenticate` but no `authorize`); popup.js alerts "Failed to
connect to server" on a 503, which is misleading - it should read `error`/`reason` from the
last non-ok response (an extension change, needs a reload/repackage).

### `/chromedebug/status` answers differently depending on who asks

`src/http-server.js` ~300, covered by `tests/status-route-discloses-only-to-loopback.test.js`.

This route deliberately has **no `authenticate`** - its job is to answer before the caller
knows whether this is a ChromeDebug server at all. Three facts then combined into a leak:
the listener binds every interface (`app.listen(port, cb)`, no host argument); `hostAllowlist`
inspects the Host **header** only, which stops a rebound browser but not
`curl -H 'Host: localhost'` from the LAN; and this route had no socket check. So `dbPath`
(under the user's home directory, therefore naming the OS account) and `pid`, added in
`21bf24a` for stale-handle diagnosis, went to anyone who could reach the port.

Only 3 of 36 routes are unauthenticated (`/test`, `/health`, `/chromedebug/status`); the other
33 are safe because `authenticate` checks the socket peer.

- `status`, `service`, `protocol`, `version`, `port`, `security`, `timestamp` go to everyone -
  enough to conclude "not the server I want", and none of it describes the machine.
- `dbPath`, `dbFile`, `dbError`, `pid` are gated on `isLoopbackPeer(req)`. The extension always
  calls from loopback and loses nothing.
- Gate on the **socket peer**, never `req.ip` or a header - the same rule as the auth model
  below, and it matters more here because nothing else stands behind this route.
- The test proves the premise, not just the fix: the same request without the forged Host is
  403, and it fails against code with the gate removed (verified by deleting it).

### The listener binds loopback only, and every port probe must bind the same address

`LISTEN_HOST` in `src/port-discovery.js` (one shared constant), used by
`app.listen(port, LISTEN_HOST, ...)` and by BOTH port probes. Covered by
`tests/status-route-discloses-only-to-loopback.test.js`.

The owner's decision, in their words: "this project should only be accepting input from this
machine. accepting input from anywhere else is not the purpose of this project." Verified
before changing it: every client - extension background/popup/options, `http-client.js`, the
VS Code client (`chromePilotClient.ts` ~172) - uses `localhost`; no Dockerfile, no compose,
nothing in README/PERSISTENT-FLAG-README documents remote use.

- **IPv4 only, not both loopback addresses.** `localhost` resolves to `::1` AND `127.0.0.1`,
  and clients fall back (measured: Node 22 and curl both reach an IPv4-only bind through
  `localhost`; a forced-IPv6 request is refused). A second `::1` listener would need its own
  close path or it keeps the event loop alive - `chrome-service.js` `cleanup()` and every
  server test's `afterAll` close exactly one server.
- **`src/services/http-client.js` addresses `127.0.0.1` directly, not `localhost`.** Node
  gained Happy Eyeballs (`autoSelectFamily`) only in v20 and `package.json` declares
  `engines >=18`, so on 18 with verbatim DNS order every MCP HTTP call would fail
  ECONNREFUSED deterministically - the same shape as the `MCP_SERVICE_TOKEN` outage above.
- **Both port probes check BOTH addresses** (`isPortFreeForUs` in `port-discovery.js`), and
  neither single-address form may come back. libuv sets `SO_REUSEADDR` on every listener, so
  on BSD stacks a wildcard bind and a specific-address bind hold one port at once, and each
  probe style is blind to the other occupant. MEASURED on macOS:
  - occupant on `127.0.0.1:P` -> `listen(P)` says FREE. This blind spot silently produced MCP
    processes with **no HTTP server**: the session manager hands the port out,
    `startHttpServer` hits EADDRINUSE, `chrome-service.js` swallows that as non-fatal.
  - occupant on `0.0.0.0:P` -> `listen(P,'127.0.0.1')` says FREE. Introduced while fixing the
    first, and worse for the user: 3000, 8080 and 9000 are in our port list AND are the
    canonical third-party dev-server ports, which bind `0.0.0.0`. We would bind
    `127.0.0.1:3000` beside the user's own app, and the port then answers DIFFERENTLY by
    address - measured: `localhost` and `::1` reach the dev server, `127.0.0.1` reaches us.
    Correct behaviour is to treat it as busy and move to the next port.
  Both directions are pinned by behavioural tests, not source scans - source scans missed this
  twice.
- **Do not add a bind-host env var.** It reintroduces the path being closed, and this repo
  already has the "a client env var silently baked into a build" scar.
- **`hostAllowlist` and `trust proxy` are unchanged.** Rebinding arrives THROUGH loopback
  from the victim's own browser, so the Host check is exactly as load-bearing as before.
  Peers are now plain `127.0.0.1` rather than `::ffff:127.0.0.1`; `LOOPBACK_PEERS` has both.
- **Rollover:** the first start of this build while older wildcard-bound ChromeDebug servers
  still run can put two servers on one port, with `127.0.0.1` traffic going to the new one.
  Run `npm run cleanup` before the first start. (Third-party wildcard servers are a permanent
  condition, not a rollover one, and are handled by `isPortFreeForUs` above.)
- Now legacy, NOT removed in this change: the API-key and service-token branches in
  `middleware/auth.js` (~151-180) are unreachable, since loopback peers take the bypass
  first and no other peer can connect; the rate limiters' non-loopback `skip` never fires;
  CORS `trustedOrigins` still lists `http://0.0.0.0:*` and a `TRUSTED_ORIGINS` env.

### Auth model (do not weaken these without reading why)

- **The loopback bypass reads `req.socket.remoteAddress`, never `req.ip`.**
  `req.ip` is derived from `X-Forwarded-For` whenever `trust proxy` is truthy, so
  it is attacker-controlled. This server previously set `trust proxy: 1` and
  listened on all interfaces, which let any LAN host send
  `X-Forwarded-For: 127.0.0.1` and receive `chrome:control`. Verified live: the
  spoofed request returned HTTP 200 with real recording data, the same request
  without the header returned 401. `trust proxy` is now deliberately unset - see
  the comment at the top of `startHttpServer`.
- **The bypass is load-bearing.** All the extension's `/chromedebug/*` fetches
  send no credentials, so it cannot be removed without shipping a credential to
  the extension first.
- **`hostAllowlist` runs before everything else** and is the only control that
  stops DNS rebinding. A socket check cannot: during rebinding the browser really
  does connect to loopback. Extra names go in `CHROMEDEBUG_ALLOWED_HOSTS`.
- **There is no JWT path.** It verified against a secret hardcoded in this public
  repo and assigned the decoded payload to `req.user`, which `authorize()` unions
  into the permission set - so a forged token self-granted `admin:manage`.
  Nothing ever issued a JWT. Do not reintroduce a Bearer branch.
- **`authorize()` unions `req.user.permissions` with the role table.** That is
  only safe while every credential path builds `req.user` server-side. The
  localhost user's `role` must stay `ROLES.USER` (lowercase `'user'`) - the
  literal `'USER'` misses `ROLE_PERMISSIONS` and silently resolves to `[]`.
- **No admin API key is minted on import.** Use `npm run setup-security`.
- Rate-limit buckets come from `rateLimitKey`; returning `undefined` there
  collapses every caller into one shared bucket.

Covered by `tests/auth-localhost-bypass.test.js` and
`tests/security-controls.test.js`.

## CLI Usage

Chrome Debug provides a CLI tool that can be used directly:
```bash
chromedebug-mcp  # Runs the MCP server (equivalent to npm start)
npx chromedebug-mcp  # Run without global installation

# Persistent server mode
chromedebug-mcp --persistent  # Won't be killed by other instances
```

### Persistent Server Mode

The `--persistent` flag marks a server as persistent, preventing it from being killed when other ChromeDebug instances start. This is useful for:

- **Manual debugging servers**: Keep a debug server running while using Claude Code
- **Multiple projects**: Run persistent servers for different projects simultaneously
- **Development workflows**: Maintain a stable server instance across multiple Claude Code sessions

Example:
```bash
# Terminal 1: Start persistent debug server
chromedebug-mcp --persistent --verbose

# Terminal 2: Use Claude Code normally
# Claude's MCP server will NOT kill the persistent server
```

See `PERSISTENT-FLAG-README.md` for detailed documentation and examples.

The package name is `chromedebug-mcp` but the project is commonly referred to as Chrome Debug.

## Additional Architecture Notes

### Port Discovery
- Server automatically finds available port from configured list (default: 3001, 3000, 3002, 3028)
- Port selection stored in `config/chrome-pilot-config.json`
- Extension automatically updated with current port via `npm run update-extension-config`

### Frame Recording System
- Captures screenshots and console logs during browser sessions
- Stores frame data in SQLite with session IDs
- Supports searching logs across frames
- Optimized for AI analysis with low-res JPEG capture
- **NEW**: Screenshots can be retrieved as base64 data for viewing with `get_frame_screenshot`

### Workflow Recording
- Records user actions and browser state
- Supports restore points for reverting to previous states
- Stores DOM snapshots, form values, storage, and cookies
- Enables replay and debugging of complex workflows
- Two modes: `workflow` (action tracking) and `screenshot` (click-to-capture)
- Console logs associated with actions using timestamp windowing

### Recording Mode Database Schema
- `recording_mode` column: 'workflow' or 'screenshot'
- NULL values treated as 'workflow' for backward compatibility
- Tab filtering uses SQL: `WHERE (recording_mode = ? OR recording_mode IS NULL)`

### Testing Framework
- **Jest-based**: Uses Jest with ES modules support (`NODE_OPTIONS='--experimental-vm-modules'`)
- **Test Structure**: Tests located in `tests/` directory
- **Coverage**: Excludes legacy code and CLI from coverage reports
- **Specialized Test Suites**: Recording system, extension integration, database migration
- **Timeouts**: 30-second timeout for tests involving browser automation
- **Setup**: Custom test setup in `tests/setup.js`

## Development Guidelines

### Code Review Process
- **ALWAYS** run all plans by the second-opinion-analyst agent before implementation
- This applies to significant changes including:
  - Architectural changes (adding/removing systems, modifying core patterns)
  - Bug fixes that affect multiple components
  - Performance optimizations that alter timing or execution flow
  - State management changes (especially lease system, session management)
  - Race condition fixes
  - Error handling modifications

### Why Second Opinion Matters
The second-opinion-analyst helps prevent:
- **Symptom fixes**: Addressing surface issues without fixing root causes
- **Cascade effects**: Changes that solve one problem but create others
- **Technical debt**: Quick fixes that make future maintenance harder
- **Defensive over-engineering**: Adding unnecessary complexity "just in case"

### When to Skip Second Opinion
- Simple documentation updates
- Test additions (without changing behavior)
- Formatting/linting fixes
- Variable/function renaming

### Example Process
1. Identify the problem and proposed solution
2. Use the second-opinion-analyst agent to review the approach
3. Adjust based on feedback (addressing root causes, not symptoms)
4. Implement the approved solution
5. Test thoroughly to verify the fix

---

## Standard Operating Procedure for All Changes

### Team-Based Development Process

**All significant changes MUST follow this workflow:**

1. **Strategic Planning Phase**
   - Work with the Strategic Project Manager agent to plan implementation
   - Present plan to user for approval
   - If plan changes during implementation, alert user before proceeding

2. **Second Opinion Review**
   - Second Opinion Agent MUST approve all plans and changes
   - Review for unintended consequences and system-wide impacts
   - Ensure root causes are addressed, not symptoms

3. **TrustButVerify Integration**
   - **Before Starting:** Use `trustbutverify.impact_predict` to identify affected areas
   - **During Development:** After each major function, use `trustbutverify.microtest_generate_and_run`
   - **After Completion:** Run `trustbutverify.verify_fix` with `includeReplay: true`
   - **Quality Gate:** Only mark complete and proceed if risk score is below 0.3

4. **Specialized Agent Assignment**
   - Chrome Debug Agent: Handles all Chrome Debug codebase changes
   - Use domain-specific agents for specialized work:
     - Firebase Implementation Expert for Firebase changes
     - React Optimization Expert for React/UI changes
     - Android Platform Expert for Android work
     - etc.

5. **No TODO Lists for New Projects**
   - Build functionality directly, don't create TODO items
   - Complete implementation in single workflow
   - Use TrustButVerify for validation instead of tracking

### Quality Standards

- Risk score must be below 0.3 before proceeding
- All changes must pass microtest validation
- Impact prediction required before starting
- Replay verification required for completion