diff --git a/docs/mcp-page-ready-plan.md b/docs/mcp-page-ready-plan.md new file mode 100644 index 0000000..ff56543 --- /dev/null +++ b/docs/mcp-page-ready-plan.md @@ -0,0 +1,115 @@ +# Plan: Add Deterministic Page Waiting to mcp.ts + +## Current Problem + +In `mcp.ts`, both `connect()` and `reconnect()` functions: + +```typescript +const browser = await chromium.connectOverCDP(cdpEndpoint) +const context = contexts[0] +const pages = context.pages() // May be empty! +if (pages.length === 0) { + throw new Error(NO_TABS_ERROR) +} +``` + +This fails when: +1. Extension has tabs connected, but... +2. Playwright hasn't finished processing `Target.attachedToTarget` + `Runtime.executionContextCreated` events + +## Solution + +Use `context.waitForEvent('page')` with a timeout, similar to our test fix. + +## Implementation Plan + +### Step 1: Create a helper function + +```typescript +async function waitForPages(context: BrowserContext, options?: { + timeout?: number + minPages?: number +}): Promise { + const { timeout = 5000, minPages = 1 } = options || {} + + // Check if pages already exist + let pages = context.pages() + if (pages.length >= minPages) { + return pages + } + + // Wait for page events + const startTime = Date.now() + while (pages.length < minPages && Date.now() - startTime < timeout) { + try { + await context.waitForEvent('page', { timeout: timeout - (Date.now() - startTime) }) + pages = context.pages() + } catch { + // Timeout - check one more time + pages = context.pages() + break + } + } + + return pages +} +``` + +### Step 2: Update `connect()` function (line ~305) + +```typescript +// Before: +const pages = context.pages() +if (pages.length === 0) { + throw new Error(NO_TABS_ERROR) +} + +// After: +const pages = await waitForPages(context, { timeout: 5000 }) +if (pages.length === 0) { + throw new Error(NO_TABS_ERROR) +} +``` + +### Step 3: Update `reconnect()` function (line ~448) + +Same change as above. + +### Step 4: Consider the "createInitialTab" flow + +The relay server has a `createInitialTab` message for when no tabs exist. We should: +1. First try `waitForPages()` with a short timeout (1-2 seconds) +2. If still no pages, request `createInitialTab` +3. Then `waitForPages()` again for the new tab + +```typescript +let pages = await waitForPages(context, { timeout: 2000 }) +if (pages.length === 0) { + // No tabs - request extension to create one + await requestCreateInitialTab() + pages = await waitForPages(context, { timeout: 5000 }) +} +if (pages.length === 0) { + throw new Error(NO_TABS_ERROR) +} +``` + +## Testing + +1. Run existing tests to ensure no regression +2. Add a new test that: + - Connects immediately after extension toggle + - Verifies pages are available without sleep + +## Files to Modify + +1. `playwriter/src/mcp.ts`: + - Add `waitForPages()` helper + - Update `connect()` function + - Update `reconnect()` function + +## Edge Cases + +1. **All pages are `about:blank`**: May need to filter for real pages +2. **Extension not connected**: Existing timeout handling should work +3. **Very slow page load**: 5 second timeout should be sufficient for CDP events (actual page content load is not required) diff --git a/docs/page-ready-investigation.md b/docs/page-ready-investigation.md new file mode 100644 index 0000000..f89cc82 --- /dev/null +++ b/docs/page-ready-investigation.md @@ -0,0 +1,102 @@ +# Page Ready Investigation Summary + +## Goal +Convert undeterministic `sleep()` calls into deterministic event waits when toggling the extension. + +## Problem +After `toggleExtensionForActiveTab()` returns, the page is not immediately available in Playwright's `context.pages()`. Tests currently use arbitrary sleeps (100-400ms) to work around this. + +## Key Events Required + +For a page to appear in `context.pages()`, Playwright needs: + +1. **`Target.attachedToTarget`** - Tells Playwright a new target exists with its URL +2. **`Runtime.executionContextCreated`** - Tells Playwright the JS context is ready + +The page appears in `context.pages()` only after BOTH events are processed. + +## Current Flow (Timeline from test) + +``` + 63ms ← EVT Target.attachedToTarget # Extension sends to relay + 431ms ← EVT Runtime.executionContextCreated # Relay's proactive Runtime.enable + 437ms → CMD Runtime.enable # Playwright sends its own + 438ms Toggle completes (pageReady received) + 467ms ← EVT Runtime.executionContextCreated # Response to Playwright's Runtime.enable + 728ms Page appears in context.pages() +``` + +## Root Cause + +The ~300ms delay between toggle completing and page appearing is caused by: + +1. **Extension's `Runtime.enable` handler** (background.ts:256-272): + - Always calls `Runtime.disable` then `Runtime.enable` + - Has a 50ms `sleep()` between them + - This is needed to force Chrome to re-send `executionContextCreated` events for multiple clients + +2. **Double Runtime.enable cycle**: + - Relay proactively enables Runtime → events at ~430ms + - Playwright receives `Target.attachedToTarget`, sends its own `Runtime.enable` + - Extension does disable+enable again → events at ~470ms + - Playwright waits for ITS events before adding page to pages() + +3. **Playwright's internal processing**: + - Even after receiving events, Playwright takes time to create Page objects + - This is async and cannot be controlled from our side + +## What We Tried + +1. **Proactive Runtime.enable in relay** - Enable Runtime before forwarding `Target.attachedToTarget` + - Helps get events faster, but Playwright still calls Runtime.enable itself + +2. **Skip disable cycle if recently enabled** - Track recent enables in extension + - Breaks because relay's Runtime.enable handler waits for events that won't come + +3. **pageReady handshake** - Extension waits for relay confirmation before returning from attachTab + - Toggle now waits for executionContextCreated + - But Playwright STILL calls Runtime.enable after, causing another cycle + +## The Core Issue + +**Playwright always calls `Runtime.enable` after receiving `Target.attachedToTarget`**, regardless of whether we pre-enabled it. The extension's disable+enable cycle adds ~200ms, and we cannot skip it without breaking the multi-client case. + +## Possible Solutions + +### Option 1: Accept the delay, use proper waiting +Instead of sleep, use `context.waitForEvent('page')` with a predicate: +```typescript +await serviceWorker.evaluate(() => globalThis.toggleExtensionForActiveTab()) +const page = await context.waitForEvent('page', { + predicate: p => p.url().includes('target-url') +}) +``` + +### Option 2: Expose a "page ready" promise from the relay +Add an endpoint or event that resolves when the page is fully ready in Playwright: +```typescript +await serviceWorker.evaluate(() => globalThis.toggleExtensionForActiveTab()) +await relay.waitForPageReady(sessionId) // Waits for Playwright to process everything +``` + +### Option 3: Have extension track Runtime state per session +Skip the disable+enable if: +- This is the SAME Playwright client that just received `Target.attachedToTarget` +- The session was JUST created (within last 100ms) + +This requires tracking which client enabled Runtime and when. + +## Recommendation + +**Option 1 is the simplest and most reliable.** The delay is inherent to how Playwright processes CDP events. We cannot eliminate it, but we can wait for it properly using `context.waitForEvent('page')` instead of arbitrary sleeps. + +The test should be: +```typescript +const pagePromise = context.waitForEvent('page', { + predicate: p => p.url().includes('discord.com'), + timeout: 5000 +}) +await serviceWorker.evaluate(() => globalThis.toggleExtensionForActiveTab()) +const page = await pagePromise +// Page is now guaranteed to be ready +``` diff --git a/docs/playwright-cdp-connection.md b/docs/playwright-cdp-connection.md new file mode 100644 index 0000000..75fccf8 --- /dev/null +++ b/docs/playwright-cdp-connection.md @@ -0,0 +1,346 @@ +# Playwright CDP Connection Flow + +This document describes exactly what CDP commands, responses, and events Playwright +sends and expects when connecting to a browser via `connectOverCDP`. Understanding +this flow is critical for implementing a CDP relay that works correctly with Playwright. + +## Connection Sequence Overview + +``` +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 1. ENDPOINT RESOLUTION │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ If HTTP URL → fetch /json/version/ to get webSocketDebuggerUrl │ +│ If WS URL → use directly │ +└─────────────────────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 2. WEBSOCKET CONNECT │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ Connect to ws://host:port/devtools/browser/ │ +│ Creates root CDP session with sessionId = '' (empty string) │ +└─────────────────────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 3. BROWSER INITIALIZATION (root session) │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ Command: Browser.getVersion │ +│ Response: { protocolVersion, product, userAgent, ... } │ +│ │ +│ Command: Target.setAutoAttach │ +│ params: { autoAttach: true, waitForDebuggerOnStart: true, flatten: true } │ +│ Response: {} │ +│ │ +│ (Chrome bug workaround) │ +│ Command: Target.getTargetInfo │ +│ Response: { targetInfo: { type: 'browser', ... } } │ +└─────────────────────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 4. TARGET DISCOVERY (events from browser) │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ For each existing page/target, Chrome sends: │ +│ │ +│ Event: Target.attachedToTarget │ +│ params: { │ +│ targetInfo: { │ +│ type: 'page' | 'service_worker' | 'browser' | 'other', │ +│ targetId: string, │ +│ browserContextId: string, │ +│ url: string, ← page URL is here │ +│ openerId?: string ← for popups │ +│ }, │ +│ sessionId: string, ← CDP session ID for this target │ +│ waitingForDebugger: boolean │ +│ } │ +└─────────────────────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 5. PAGE INITIALIZATION (per-page session) │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ For each page, Playwright sends these commands on the page's sessionId: │ +│ │ +│ Parallel batch 1: │ +│ - Page.enable │ +│ - Page.getFrameTree │ +│ - Network.enable │ +│ - Runtime.enable ← triggers executionContextCreated │ +│ - Page.setLifecycleEventsEnabled { enabled: true } │ +│ - Log.enable │ +│ - Page.addScriptToEvaluateOnNewDocument { ... } │ +│ - Target.setAutoAttach { autoAttach: true, ... } ← for iframes │ +│ │ +│ Then: │ +│ - Runtime.runIfWaitingForDebugger ← unpauses the page │ +└─────────────────────────────────────────────────────────────────────────────┘ + ↓ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ 6. EXECUTION CONTEXT CREATION (events from browser) │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ After Runtime.enable, Chrome sends: │ +│ │ +│ Event: Runtime.executionContextCreated │ +│ params: { │ +│ context: { │ +│ id: number, ← contextId for Runtime.evaluate │ +│ auxData: { │ +│ frameId: string, │ +│ isDefault: boolean ← true = main world │ +│ }, │ +│ name: string ← matches utility world name if isolated │ +│ } │ +│ } │ +│ │ +│ Playwright creates a ManualPromise for each world ('main', 'utility'). │ +│ The promise resolves when executionContextCreated fires for that world. │ +└─────────────────────────────────────────────────────────────────────────────┘ +``` + +## CDP Commands Sent by Playwright + +### On Browser Connection (root session, sessionId = '') + +| Order | Command | Parameters | Purpose | +|-------|---------|------------|---------| +| 1 | `Browser.getVersion` | none | Get browser version, user agent, detect headless | +| 2 | `Target.setAutoAttach` | `autoAttach: true, waitForDebuggerOnStart: true, flatten: true` | Auto-attach to all targets | +| 3 | `Target.getTargetInfo` | none | Chrome bug workaround - ensures targets attached | + +### On Page Attachment (page session) + +| Command | Purpose | +|---------|---------| +| `Page.enable` | Enable Page domain events | +| `Page.getFrameTree` | Get frame hierarchy | +| `Network.enable` | Enable network interception | +| `Runtime.enable` | Enable JS execution contexts | +| `Page.setLifecycleEventsEnabled` | Enable load/DOMContentLoaded events | +| `Log.enable` | Enable console message capture | +| `Page.addScriptToEvaluateOnNewDocument` | Inject Playwright's utility scripts | +| `Target.setAutoAttach` | Auto-attach to iframes | +| `Runtime.runIfWaitingForDebugger` | Unpause the page | + +## CDP Events Playwright Listens For + +### Browser-Level Events (root session) + +| Event | Handler | Purpose | +|-------|---------|---------| +| `Target.attachedToTarget` | `_onAttachedToTarget` | New page/target discovered | +| `Target.detachedFromTarget` | `_onDetachedFromTarget` | Page/target closed | +| `Browser.downloadWillBegin` | `_onDownloadWillBegin` | Download started | +| `Browser.downloadProgress` | `_onDownloadProgress` | Download progress | + +### Page-Level Events (page session) + +| Event | Purpose | +|-------|---------| +| `Runtime.executionContextCreated` | JavaScript context ready | +| `Runtime.executionContextDestroyed` | Context destroyed (navigation) | +| `Page.frameAttached` | New iframe | +| `Page.frameDetached` | Iframe removed | +| `Page.navigatedWithinDocument` | SPA navigation | +| `Page.lifecycleEvent` | load, DOMContentLoaded, etc. | +| `Page.frameStoppedLoading` | Frame finished loading | +| `Network.requestWillBeSent` | Request started | +| `Network.responseReceived` | Response received | +| `Log.entryAdded` | Console message | + +## The Context Promise Mechanism + +Playwright uses a `ManualPromise` pattern for execution contexts: + +``` +Frame created + ↓ +ManualPromise created for 'main' and 'utility' worlds + ↓ +Runtime.enable sent + ↓ +Chrome sends Runtime.executionContextCreated + ↓ +_contextCreated() resolves the ManualPromise + ↓ +page.evaluate() can now execute +``` + +### How User APIs Wait for Context + +Every DOM operation in Playwright awaits `_context()` before running: + +```typescript +// frames.ts +async evaluateExpression(expression, options, arg) { + const context = await this._context(options.world ?? 'main'); // waits here + return context.evaluateExpression(expression, options, arg); +} + +_context(world: 'main' | 'utility'): Promise { + return this._contextData.get(world)!.contextPromise.then(...); +} +``` + +### Context Lifecycle on Navigation + +``` +Navigation starts + ↓ +Runtime.executionContextDestroyed fires + ↓ +_setContext(world, null) → creates NEW ManualPromise + ↓ +Any evaluate() call now waits on new promise + ↓ +New page loads, Runtime.executionContextCreated fires + ↓ +_contextCreated() resolves the new promise + ↓ +Waiting evaluate() calls proceed +``` + +## World Types + +Playwright uses two JavaScript execution worlds: + +| World | Purpose | APIs That Use It | +|-------|---------|------------------| +| **main** | Page's own JS context. User's `evaluate()` runs here. | `evaluate()`, `evaluateHandle()`, `addScriptTag()`, `addStyleTag()` | +| **utility** | Isolated world. Playwright's internal code runs here. | `$()`, `$$()`, `click()`, `fill()`, `type()`, `content()`, `waitForSelector()` | + +### Why Two Worlds? + +- **main**: User code needs access to page variables and functions +- **utility**: Playwright's selectors/actions need isolation from page code that might override `document.querySelector`, etc. + +### Element Handle Adoption + +When `page.$()` or `waitForSelector()` returns an element: +1. Query runs in **utility** world +2. Result is adopted to **main** world +3. User receives handle bound to main context + +## What Must Happen Before User Code Can Run + +For `page.evaluate()` or any DOM operation to work: + +1. **WebSocket connected** to CDP endpoint +2. **Browser.getVersion** returned successfully +3. **Target.setAutoAttach** sent and acknowledged +4. **Target.attachedToTarget** received for the page +5. **Runtime.enable** sent on page session +6. **Runtime.executionContextCreated** received for the frame +7. **ManualPromise resolved** for the appropriate world + +If any step is missing or delayed, `evaluate()` will hang waiting for the context promise. + +## Key Parameters Explained + +### Target.setAutoAttach Parameters + +| Parameter | Value | Purpose | +|-----------|-------|---------| +| `autoAttach` | `true` | Automatically attach to new targets | +| `waitForDebuggerOnStart` | `true` | Pause new targets until `Runtime.runIfWaitingForDebugger` | +| `flatten` | `true` | Use flat session IDs (not nested) | + +### Why waitForDebuggerOnStart? + +Without this, a page could start executing before Playwright injects its utility scripts. With it: +1. Chrome pauses the page immediately after creation +2. Playwright sends initialization commands +3. Playwright sends `Runtime.runIfWaitingForDebugger` +4. Page starts executing with Playwright ready + +## Implications for CDP Relay Implementation + +A CDP relay (like playwriter) must: + +1. **Forward Target.setAutoAttach** and return response before any targets attach +2. **Send Target.attachedToTarget** with correct `targetInfo` including `url` +3. **Forward Runtime.enable** and ensure `Runtime.executionContextCreated` follows +4. **Use correct sessionId** for page-specific commands/events +5. **Handle session lifecycle** - detach events when tabs close + +### Critical Timing + +The relay should ensure: +- `Target.attachedToTarget` is sent before any page commands +- `Runtime.executionContextCreated` is sent after `Runtime.enable` returns +- Events use the correct `sessionId` matching the target + +See `cdp-timing.md` for details on our relay's event synchronization. + +## When `context.on('page')` / `context.waitForEvent('page')` Fires + +Both `context.on('page')` and `context.waitForEvent('page')` listen to the **same +underlying event**. The difference is only in consumption pattern: + +| Aspect | `context.on('page', cb)` | `context.waitForEvent('page')` | +|--------|--------------------------|-------------------------------| +| Return | EventEmitter | Promise\ | +| Fires | Every time | First match only | +| Timeout | None | Built-in timeout | +| Predicate | Manual | Native `{ predicate: fn }` | +| Cleanup | Manual `off()` | Auto-removes listener | +| Close handling | Manual | Auto-rejects if context closes | + +The 'page' event on BrowserContext is NOT immediate after `Target.attachedToTarget`. +It requires page initialization to complete first. + +### Event Chain + +``` +Target.attachedToTarget (CDP event) + ↓ +CRPage created (crBrowser.ts) + ↓ +FrameSession._initialize() starts + ↓ +Parallel CDP commands sent: + - Page.enable + - Page.getFrameTree ← must complete + - Runtime.enable + - Log.enable + - Page.setLifecycleEventsEnabled + - Runtime.runIfWaitingForDebugger + ↓ +WAIT for _firstNonInitialNavigationCommittedPromise + ↓ +_initialize() resolves + ↓ +page.reportAsNew() called + ↓ +page._markInitialized() + ↓ +this.emitOnContext(BrowserContext.Events.Page, this) ← 'page' EVENT FIRES +``` + +### The Navigation Wait Condition + +The critical gate is `_firstNonInitialNavigationCommittedPromise`. The 'page' event +does NOT fire until: + +1. `Page.getFrameTree` returns the frame structure +2. **AND** one of: + - Page URL is not `:` (not an initial empty page) + - `Page.frameNavigated` event arrives for a non-initial navigation + +This ensures the page has a **valid URL and frame structure** before user code sees it. + +### Key Insights + +- `context.on('page')` and `context.waitForEvent('page')` fire on the **same event** +- The 'page' event fires AFTER page initialization, not immediately on attach +- `Runtime.executionContextCreated` is NOT required for the 'page' event +- The 'page' event guarantees the page has a valid URL and frame tree +- `page.evaluate()` may still need to wait for execution context after 'page' fires + +### Timing Implications for CDP Relay + +For `context.on('page')` to fire, the relay must: + +1. Send `Target.attachedToTarget` with valid `targetInfo.url` +2. Respond to `Page.getFrameTree` with frame structure +3. Either: + - Provide a non-empty URL (not `:`) in the frame tree + - Or send `Page.frameNavigated` event after navigation