From e3cb7fd1fd44bdd1a447518ed8838370dd8c87ff Mon Sep 17 00:00:00 2001
From: "Tommy D. Rossi" <beats.by.morse@gmail.com>
Date: Fri, 2 Jan 2026 14:24:15 +0100
Subject: [PATCH] add test to find out how to remove sleep after extension
 connect

---
 docs/playwright-cdp-connection.md | 52 +++++++++++++++++++++++++++++++
 playwriter/src/mcp.test.ts        | 39 +++++++++++++++++++++++
 2 files changed, 91 insertions(+)

diff --git a/docs/playwright-cdp-connection.md b/docs/playwright-cdp-connection.md
index 75fccf8..4dabe6b 100644
--- a/docs/playwright-cdp-connection.md
+++ b/docs/playwright-cdp-connection.md
@@ -330,11 +330,25 @@ This ensures the page has a **valid URL and frame structure** before user code s
 ### Key Insights
 
 - `context.on('page')` and `context.waitForEvent('page')` fire on the **same event**
+- **`waitForEvent('page')` only waits for NEW pages** - it will NOT resolve with existing pages
+- To handle existing pages, check `context.pages()` first before waiting
 - 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
 
+### Handling Existing vs New Pages
+
+```typescript
+// waitForEvent only catches NEW pages, not existing ones
+const existingPages = context.pages();
+if (existingPages.length > 0) {
+  return existingPages[0];  // use existing
+}
+// Only wait if no pages exist yet
+const newPage = await context.waitForEvent('page');
+```
+
 ### Timing Implications for CDP Relay
 
 For `context.on('page')` to fire, the relay must:
@@ -344,3 +358,41 @@ For `context.on('page')` to fire, the relay must:
 3. Either:
    - Provide a non-empty URL (not `:`) in the frame tree
    - Or send `Page.frameNavigated` event after navigation
+
+## Empty URL Detection
+
+Empty URLs in `Target.attachedToTarget` cause Playwright to create broken pages that
+never recover. Both the extension and relay log errors when this happens:
+
+### Extension logging
+
+```typescript
+// In attachTab(), after Target.getTargetInfo
+if (!targetInfo.url || targetInfo.url === '' || targetInfo.url === ':') {
+  logger.error('WARNING: Target.attachedToTarget will be sent with empty URL!')
+}
+```
+
+### Relay logging
+
+The relay logs `logger.error` warnings when:
+- `Target.attachedToTarget` is sent/received with empty URL
+- `Target.targetCreated` is sent with empty URL
+
+### Why empty URLs break Playwright
+
+If `Target.attachedToTarget` is sent with empty URL:
+- Playwright creates a broken page
+- `_firstNonInitialNavigationCommittedPromise` may never resolve
+- `page.evaluate()` hangs forever
+- **No recovery possible**
+
+### Debugging empty URL issues
+
+Check the logs at `~/.config/opencode/playwriter.log` for:
+```
+WARNING: Target.attachedToTarget sent with empty URL
+WARNING: Target.attachedToTarget received with empty URL
+```
+
+These indicate timing issues where the page hasn't fully loaded when attached.
diff --git a/playwriter/src/mcp.test.ts b/playwriter/src/mcp.test.ts
index 4d6bce4..40bc6c9 100644
--- a/playwriter/src/mcp.test.ts
+++ b/playwriter/src/mcp.test.ts
@@ -1260,6 +1260,45 @@ describe('MCP Server Tests', () => {
         await page.close()
     }, 30000)
 
+    it('should be usable after toggle with waitForEvent', async () => {
+        // This test demonstrates the proper way to wait for a page after toggle.
+        // Instead of arbitrary sleep(), use context.waitForEvent('page').
+        // See page-ready-investigation.md for details on why this is needed.
+
+        const _browserContext = getBrowserContext()
+        const serviceWorker = await getExtensionServiceWorker(_browserContext)
+        const browser = await chromium.connectOverCDP(getCdpUrl({ port: TEST_PORT }))
+        const context = browser.contexts()[0]
+
+        const page = await _browserContext.newPage()
+        await page.goto('https://example.com', { waitUntil: 'domcontentloaded' })
+        await page.bringToFront()
+
+        // Set up page listener BEFORE toggle - this is the key pattern
+        const pagePromise = context.waitForEvent('page', {
+            predicate: p => p.url().includes('example.com'),
+            timeout: 5000
+        })
+
+        // Toggle extension - no sleep needed after!
+        await serviceWorker.evaluate(async () => {
+            await globalThis.toggleExtensionForActiveTab()
+        })
+
+        // Wait for page event - this resolves when Playwright has fully processed the page
+        const targetPage = await pagePromise
+
+        // Page is now guaranteed to be ready
+        expect(targetPage.url()).toContain('example.com')
+
+        // evaluate() works immediately
+        const result = await targetPage.evaluate(() => window.location.href)
+        expect(result).toContain('example.com')
+
+        await browser.close()
+        await page.close()
+    }, 30000)
+
     it('should maintain correct page.url() with iframe-heavy pages', async () => {
         const browserContext = getBrowserContext()
         const serviceWorker = await getExtensionServiceWorker(browserContext)