fix: resolve stale session, ref resolution and cursor-ref collision bugs (#427)

skills/agent-browser/SKILL.md

@@ -191,6 +191,35 @@ agent-browser find placeholder "Search" type "query"
 agent-browser find testid "submit-btn" click
 ```
 
+## JavaScript Evaluation (eval)
+
+Use `eval` to run JavaScript in the browser context. **Shell quoting can corrupt complex expressions** -- use `--stdin` or `-b` to avoid issues.
+
+```bash
+# Simple expressions work with regular quoting
+agent-browser eval 'document.title'
+agent-browser eval 'document.querySelectorAll("img").length'
+
+# Complex JS: use --stdin with heredoc (RECOMMENDED)
+agent-browser eval --stdin <<'EVALEOF'
+JSON.stringify(
+  Array.from(document.querySelectorAll("img"))
+    .filter(i => !i.alt)
+    .map(i => ({ src: i.src.split("/").pop(), width: i.width }))
+)
+EVALEOF
+
+# Alternative: base64 encoding (avoids all shell escaping issues)
+agent-browser eval -b "$(echo -n 'Array.from(document.querySelectorAll("a")).map(a => a.href)' | base64)"
+```
+
+**Why this matters:** When the shell processes your command, inner double quotes, `!` characters (history expansion), backticks, and `$()` can all corrupt the JavaScript before it reaches agent-browser. The `--stdin` and `-b` flags bypass shell interpretation entirely.
+
+**Rules of thumb:**
+- Single-line, no nested quotes -> regular `eval 'expression'` with single quotes is fine
+- Nested quotes, arrow functions, template literals, or multiline -> use `eval --stdin <<'EVALEOF'`
+- Programmatic/generated scripts -> use `eval -b` with base64
+
 ## Deep-Dive Documentation
 
 | Reference | When to Use |

src/actions.ts

@@ -651,7 +651,7 @@ async function handleScroll(command: ScrollCommand, browser: BrowserManager): Pr
   const page = browser.getPage();
 
   if (command.selector) {
-    const element = page.locator(command.selector);
+    const element = browser.getLocator(command.selector);
     await element.scrollIntoViewIfNeeded();
 
     if (command.x !== undefined || command.y !== undefined) {
@@ -1843,8 +1843,7 @@ async function handleScrollIntoView(
   command: ScrollIntoViewCommand,
   browser: BrowserManager
 ): Promise<Response> {
-  const page = browser.getPage();
-  await page.locator(command.selector).scrollIntoViewIfNeeded();
+  await browser.getLocator(command.selector).scrollIntoViewIfNeeded();
   return successResponse(command.id, { scrolled: true });
 }
 

src/browser.test.ts

@@ -1,5 +1,6 @@
-import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
+import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
 import { BrowserManager } from './browser.js';
+import { executeCommand } from './actions.js';
 import { chromium } from 'playwright-core';
 
 describe('BrowserManager', () => {
@@ -54,6 +55,192 @@ describe('BrowserManager', () => {
     });
   });
 
+  describe('stale session recovery (all pages closed)', () => {
+    it('should recover when all pages are closed externally', async () => {
+      const testBrowser = new BrowserManager();
+      await testBrowser.launch({ headless: true });
+
+      // Verify initial state
+      expect(testBrowser.isLaunched()).toBe(true);
+      expect(testBrowser.getPage()).toBeDefined();
+
+      // Close all pages externally (simulates stale daemon state)
+      const pages = testBrowser.getPages();
+      for (const page of [...pages]) {
+        await page.close();
+      }
+
+      // Wait for close events to propagate
+      await new Promise((resolve) => setTimeout(resolve, 100));
+
+      // isLaunched() is true but pages array is empty -- this is the stale state
+      expect(testBrowser.isLaunched()).toBe(true);
+      expect(testBrowser.getPages().length).toBe(0);
+
+      // ensurePage() should recover by creating a new page
+      await testBrowser.ensurePage();
+      expect(testBrowser.getPages().length).toBe(1);
+      expect(testBrowser.getPage()).toBeDefined();
+
+      await testBrowser.close();
+    });
+
+    it('should be a no-op when pages already exist', async () => {
+      const testBrowser = new BrowserManager();
+      await testBrowser.launch({ headless: true });
+
+      const pageBefore = testBrowser.getPage();
+      await testBrowser.ensurePage();
+      const pageAfter = testBrowser.getPage();
+
+      // Should be the same page -- no-op
+      expect(pageAfter).toBe(pageBefore);
+      expect(testBrowser.getPages().length).toBe(1);
+
+      await testBrowser.close();
+    });
+  });
+
+  describe('scrollintoview with refs', () => {
+    it('should resolve refs in scrollintoview command', async () => {
+      const page = browser.getPage();
+      await page.setContent(`
+        <html>
+          <body style="height: 3000px;">
+            <div style="height: 2000px;"></div>
+            <button id="far-button">Far Away Button</button>
+          </body>
+        </html>
+      `);
+
+      // Get snapshot to populate refs
+      const { refs } = await browser.getSnapshot({ interactive: true });
+
+      // Find the ref for our button
+      const buttonRef = Object.keys(refs).find((k) => refs[k].name === 'Far Away Button');
+      expect(buttonRef).toBeDefined();
+
+      // scrollintoview with a ref should work, not throw a CSS selector error
+      const result = await executeCommand(
+        { id: 'test-1', action: 'scrollintoview', selector: `@${buttonRef}` },
+        browser
+      );
+      expect(result.success).toBe(true);
+    });
+
+    it('should resolve refs in scroll command with selector', async () => {
+      const page = browser.getPage();
+      await page.setContent(`
+        <html>
+          <body style="height: 3000px;">
+            <div id="scroll-container" style="height: 200px; overflow: auto;">
+              <div style="height: 1000px;">Scrollable content</div>
+            </div>
+            <button id="target-btn">Target Button</button>
+          </body>
+        </html>
+      `);
+
+      const { refs } = await browser.getSnapshot({ interactive: true });
+      const buttonRef = Object.keys(refs).find((k) => refs[k].name === 'Target Button');
+      expect(buttonRef).toBeDefined();
+
+      // scroll with a ref selector should work
+      const result = await executeCommand(
+        { id: 'test-2', action: 'scroll', selector: `@${buttonRef}`, y: 100 },
+        browser
+      );
+      expect(result.success).toBe(true);
+    });
+  });
+
+  describe('cursor-ref selector uniqueness', () => {
+    it('should produce unique selectors for repeated DOM structures', async () => {
+      const page = browser.getPage();
+      // Build deeply nested identical structures where the distinguishing
+      // ancestor (div.branch) is at level 4 from the target element --
+      // beyond the previous 3-level path cutoff.
+      await page.setContent(`
+        <html>
+          <body>
+            <div class="root">
+              <div class="branch">
+                <div class="level1">
+                  <div class="level2">
+                    <div class="target" style="cursor: pointer; width: 100px; height: 30px;" onclick="void(0)">Item Alpha</div>
+                  </div>
+                </div>
+              </div>
+              <div class="branch">
+                <div class="level1">
+                  <div class="level2">
+                    <div class="target" style="cursor: pointer; width: 100px; height: 30px;" onclick="void(0)">Item Beta</div>
+                  </div>
+                </div>
+              </div>
+            </div>
+          </body>
+        </html>
+      `);
+
+      const { refs } = await browser.getSnapshot({ interactive: true, cursor: true });
+
+      // Find the cursor-interactive refs
+      const cursorRefs = Object.entries(refs).filter(([, r]) => r.role === 'clickable');
+      expect(cursorRefs.length).toBe(2);
+
+      // Each ref's selector must be unique -- clicking it should not
+      // trigger a strict mode violation.
+      for (const [refKey] of cursorRefs) {
+        const locator = browser.getLocator(`@${refKey}`);
+        const count = await locator.count();
+        expect(count).toBe(1);
+      }
+    });
+
+    it('should click the correct element when refs have repeated structure', async () => {
+      const page = browser.getPage();
+      await page.setContent(`
+        <html>
+          <body>
+            <div class="root">
+              <div class="branch">
+                <div class="level1">
+                  <div class="level2">
+                    <div class="target" style="cursor: pointer; width: 100px; height: 30px;"
+                         onclick="document.getElementById('result').textContent = 'alpha'">Item Alpha</div>
+                  </div>
+                </div>
+              </div>
+              <div class="branch">
+                <div class="level1">
+                  <div class="level2">
+                    <div class="target" style="cursor: pointer; width: 100px; height: 30px;"
+                         onclick="document.getElementById('result').textContent = 'beta'">Item Beta</div>
+                  </div>
+                </div>
+              </div>
+            </div>
+            <div id="result">none</div>
+          </body>
+        </html>
+      `);
+
+      const { refs } = await browser.getSnapshot({ interactive: true, cursor: true });
+
+      // Find the ref for "Item Beta"
+      const betaRef = Object.keys(refs).find((k) => refs[k].name === 'Item Beta');
+      expect(betaRef).toBeDefined();
+
+      // Click it -- should not throw strict mode violation
+      const locator = browser.getLocator(`@${betaRef}`);
+      await locator.click();
+
+      const result = await page.locator('#result').textContent();
+      expect(result).toBe('beta');
+    });
+  });
+
   describe('navigation', () => {
     it('should navigate to URL', async () => {
       const page = browser.getPage();

src/browser.ts

@@ -190,6 +190,43 @@ export class BrowserManager {
     return page.locator(selectorOrRef);
   }
 
+  /**
+   * Check if the browser has any usable pages
+   */
+  hasPages(): boolean {
+    return this.pages.length > 0;
+  }
+
+  /**
+   * Ensure at least one page exists. If the browser is launched but all pages
+   * were closed (stale session), creates a new page on the existing context.
+   * No-op if pages already exist.
+   */
+  async ensurePage(): Promise<void> {
+    if (this.pages.length > 0) return;
+    if (!this.browser && !this.isPersistentContext) return;
+
+    // Use the last existing context, or create a new one
+    let context: BrowserContext;
+    if (this.contexts.length > 0) {
+      context = this.contexts[this.contexts.length - 1];
+    } else if (this.browser) {
+      context = await this.browser.newContext();
+      context.setDefaultTimeout(60000);
+      this.contexts.push(context);
+      this.setupContextTracking(context);
+    } else {
+      return;
+    }
+
+    const page = await context.newPage();
+    if (!this.pages.includes(page)) {
+      this.pages.push(page);
+      this.setupPageTracking(page);
+    }
+    this.activePageIndex = this.pages.length - 1;
+  }
+
   /**
    * Get the current active page, throws if not launched
    */

src/daemon.ts

@@ -329,6 +329,17 @@ export async function startDaemon(options?: {
             }
           }
 
+          // Recover from stale state: browser is launched but all pages were closed
+          if (
+            manager instanceof BrowserManager &&
+            manager.isLaunched() &&
+            !manager.hasPages() &&
+            parseResult.command.action !== 'launch' &&
+            parseResult.command.action !== 'close'
+          ) {
+            await manager.ensurePage();
+          }
+
           // Handle close command specially - shuts down daemon
           if (parseResult.command.action === 'close') {
             const response =

src/snapshot.ts

@@ -204,7 +204,16 @@ async function findCursorInteractiveElements(
         }
         path.unshift(sel);
         current = current.parentElement;
-        if (path.length >= 3) break;
+        // Stop once the selector uniquely identifies the element (max 10 levels)
+        if (path.length >= 1) {
+          try {
+            const candidate = path.join(' > ');
+            if (document.querySelectorAll(candidate).length === 1) break;
+          } catch (e) {
+            // If selector is invalid, keep building
+          }
+        }
+        if (path.length >= 10) break;
       }
       return path.join(' > ');
     };