From d597c33eabfb93183452571a5fb0c5f7d835dfc4 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Thu, 26 Mar 2026 07:33:32 -0600
Subject: [PATCH] =?UTF-8?q?fix:=20review=20fixes=20=E2=80=94=20iframe=20re?=
 =?UTF-8?q?f=20scoping,=20detached=20frame=20recovery,=20state=20validatio?=
 =?UTF-8?q?n?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- snapshot.ts: ref locators, cursor-interactive scan, and cursor locator
  now use target (frame-aware) instead of page — fixes @ref clicking in iframes
- browser-manager.ts: getActiveFrameOrPage auto-recovers from detached frames
  via isDetached() check
- meta-commands.ts: state load resets activeFrame, elementHandle disposed after
  contentFrame(), state file schema validation (cookies + pages arrays),
  filter empty pipe segments in chain tokenizer
- write-commands.ts: upload command uses target.locator() for frame support

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 browse/src/browser-manager.ts | 4 ++++
 browse/src/meta-commands.ts   | 9 ++++++++-
 browse/src/snapshot.ts        | 8 ++++----
 browse/src/write-commands.ts  | 2 +-
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts
index 401b6cb0..1ef58e36 100644
--- a/browse/src/browser-manager.ts
+++ b/browse/src/browser-manager.ts
@@ -561,6 +561,10 @@ export class BrowserManager {
    * Use this for operations that work on both Page and Frame (locator, evaluate, etc.).
    */
   getActiveFrameOrPage(): import('playwright').Page | import('playwright').Frame {
+    // Auto-recover from detached frames (iframe removed/navigated)
+    if (this.activeFrame?.isDetached()) {
+      this.activeFrame = null;
+    }
     return this.activeFrame ?? this.getPage();
   }
 
diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts
index b6f89def..4388491a 100644
--- a/browse/src/meta-commands.ts
+++ b/browse/src/meta-commands.ts
@@ -219,7 +219,9 @@ export async function handleMetaCommand(
         if (!Array.isArray(commands)) throw new Error('not array');
       } catch {
         // Fallback: pipe-delimited format "goto url | click @e5 | snapshot -ic"
-        commands = jsonStr.split(' | ').map(seg => tokenizePipeSegment(seg.trim()));
+        commands = jsonStr.split(' | ')
+          .filter(seg => seg.trim().length > 0)
+          .map(seg => tokenizePipeSegment(seg.trim()));
       }
 
       const results: string[] = [];
@@ -478,7 +480,11 @@ export async function handleMetaCommand(
       if (action === 'load') {
         if (!fs.existsSync(statePath)) throw new Error(`State not found: ${statePath}`);
         const data = JSON.parse(fs.readFileSync(statePath, 'utf-8'));
+        if (!Array.isArray(data.cookies) || !Array.isArray(data.pages)) {
+          throw new Error('Invalid state file: expected cookies and pages arrays');
+        }
         // Close existing pages, then restore (replace, not merge)
+        bm.setFrame(null);
         await bm.closeAllPages();
         await bm.restoreState({
           cookies: data.cookies,
@@ -516,6 +522,7 @@ export async function handleMetaCommand(
         const locator = 'locator' in resolved ? resolved.locator : page.locator(resolved.selector);
         const elementHandle = await locator.elementHandle({ timeout: 5000 });
         frame = await elementHandle?.contentFrame() ?? null;
+        await elementHandle?.dispose();
       }
 
       if (!frame) throw new Error(`Frame not found: ${target}`);
diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts
index ae07b6b8..840cd686 100644
--- a/browse/src/snapshot.ts
+++ b/browse/src/snapshot.ts
@@ -208,11 +208,11 @@ export async function handleSnapshot(
 
     let locator: Locator;
     if (opts.selector) {
-      locator = page.locator(opts.selector).getByRole(node.role as any, {
+      locator = target.locator(opts.selector).getByRole(node.role as any, {
         name: node.name || undefined,
       });
     } else {
-      locator = page.getByRole(node.role as any, {
+      locator = target.getByRole(node.role as any, {
         name: node.name || undefined,
       });
     }
@@ -236,7 +236,7 @@ export async function handleSnapshot(
   // ─── Cursor-interactive scan (-C) ─────────────────────────
   if (opts.cursorInteractive) {
     try {
-      const cursorElements = await page.evaluate(() => {
+      const cursorElements = await target.evaluate(() => {
         const STANDARD_INTERACTIVE = new Set([
           'A', 'BUTTON', 'INPUT', 'SELECT', 'TEXTAREA', 'SUMMARY', 'DETAILS',
         ]);
@@ -290,7 +290,7 @@ export async function handleSnapshot(
         let cRefCounter = 1;
         for (const elem of cursorElements) {
           const ref = `c${cRefCounter++}`;
-          const locator = page.locator(elem.selector);
+          const locator = target.locator(elem.selector);
           refMap.set(ref, { locator, role: 'cursor-interactive', name: elem.text });
           output.push(`@${ref} [${elem.reason}] "${elem.text}"`);
         }
diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts
index f8e113b9..02413daf 100644
--- a/browse/src/write-commands.ts
+++ b/browse/src/write-commands.ts
@@ -258,7 +258,7 @@ export async function handleWriteCommand(
       if ('locator' in resolved) {
         await resolved.locator.setInputFiles(filePaths);
       } else {
-        await page.locator(resolved.selector).setInputFiles(filePaths);
+        await target.locator(resolved.selector).setInputFiles(filePaths);
       }
 
       const fileInfo = filePaths.map(fp => {