[eric] browser: a failed navigation now fails loudly; a reused card that cannot move is replaced instead of answering from the old page

backend/apps/agents/browser/browser_agent.py

@@ -1150,8 +1150,25 @@ async def run_browser_agent(
             nav_result = await execute_browser_tool(
                 "BrowserNavigate", {"url": initial_url}, browser_id, tab_id,
             )
-            logger.info(f"Browser agent {session_id}: navigated to {initial_url}: {nav_result.get('text', nav_result.get('error', ''))}")
-            preloaded_perception, current_url, preloaded_reads = await p_perceive(initial_url)
+            p_nav_err = str(nav_result.get("error") or "") if isinstance(nav_result, dict) else ""
+            if p_nav_err:
+                # One retry: right after spawn the webview may still be mounting, and that transient must not become a run on the wrong page.
+                nav_result = await execute_browser_tool(
+                    "BrowserNavigate", {"url": initial_url}, browser_id, tab_id,
+                )
+                p_nav_err = str(nav_result.get("error") or "") if isinstance(nav_result, dict) else ""
+            if p_nav_err:
+                # This used to log "navigated to" and run anyway, which is how an agent asked for /wiki/Helium answered about Argon (Haik's field report).
+                logger.warning(f"Browser agent {session_id}: navigation to {initial_url} FAILED: {p_nav_err[:200]}")
+                preloaded_perception, current_url, preloaded_reads = await p_perceive("")
+                preloaded_perception = (
+                    f"\n\n[IMPORTANT: opening {initial_url} FAILED ({p_nav_err[:200]}). "
+                    f"The page actually loaded right now is {current_url or 'unknown'}. Never answer from this wrong page: "
+                    "retry BrowserNavigate yourself first, and if it still fails, report the navigation failure as your result.]"
+                ) + preloaded_perception
+            else:
+                logger.info(f"Browser agent {session_id}: navigated to {initial_url}: {nav_result.get('text', '')}")
+                preloaded_perception, current_url, preloaded_reads = await p_perceive(initial_url)
         elif not p_resumed:
             # Fresh task on an existing card: perceive the current page to learn its host (for replay) and front-load turn 1 (this path used to start cold).
             preloaded_perception, current_url, preloaded_reads = await p_perceive("")
@@ -3635,11 +3652,24 @@ async def run_browser_agents(
                 )
                 if url:
                     # a retry starts from the task's entry URL, never the failed attempt's leftover page state
+                    p_nav: dict = {}
                     try:
-                        await execute_browser_tool("BrowserNavigate", {"url": url}, browser_id)
-                    except Exception:
-                        pass
-            elif os.environ.get("OSW_PRELUDE_TRIM", "1") != "0":
+                        p_nav = await execute_browser_tool("BrowserNavigate", {"url": url}, browser_id)
+                    except Exception as p_nav_exc:
+                        p_nav = {"error": f"{type(p_nav_exc).__name__}: {p_nav_exc}"}
+                    if isinstance(p_nav, dict) and p_nav.get("error"):
+                        # A reused card that cannot move would answer confidently about the OLD page; swap it for a fresh card instead of running on a lie (Haik's field report).
+                        logger.warning(
+                            f"[browser-agent] reused card {browser_id} refused navigation to {url[:80]} "
+                            f"({str(p_nav['error'])[:140]}); evicting it and spawning a fresh card")
+                        DEAD_CARDS.add(browser_id)
+                        await evict_dead_card(dashboard_id, browser_id)
+                        async with p_card_pick_lock:
+                            browser_id = await p_create_browser_card(dashboard_id, url or entry_url, parent_session_id)
+                            ACTIVE_AGENT_CARDS.add(browser_id)
+                        reused = False
+                        task_def.pop("p_reused_note", None)
+            if not reused and os.environ.get("OSW_PRELUDE_TRIM", "1") != "0":
                 # Poll until the mounting card serves real page text instead of a blind 2s; capped, so the worst case is the old wait plus one probe.
                 p_mount_t0 = time.monotonic()
                 p_mounted = False
@@ -3668,7 +3698,7 @@ async def run_browser_agents(
                         await asyncio.sleep(0.4)
                     logger.info(f"[browser-spawn-ack] {browser_id} rebroadcast after silent mount; alive={p_mounted}")
                 logger.info(f"[browser-cold] mount poll {int((time.monotonic() - p_mount_t0) * 1000)}ms for {browser_id}")
-            else:
+            elif not reused:
                 await asyncio.sleep(2.0)
         elif browser_id and not app_mode:
             ACTIVE_AGENT_CARDS.add(browser_id)

frontend/src/shared/browserCommandHandler.ts

@@ -11,6 +11,7 @@ import { resolveInput } from './resolveUrl';
 import { rankAndCapInteractives, type RankItem } from './interactiveRanking';
 import { shouldStopWaiting, SETTLE_POLL_MS, settleProbeJs } from './browserSettle';
 import { unwrapCdpEval } from './cdpEval';
+import { navigationOutcome, sameDoc } from './navigationOutcome';
 import { typeChars, type TypedKeys } from './typeChars';
 
 let initialized = false;
@@ -369,6 +370,8 @@ async function handleNavigate(wv: BrowserWebview, params: Record<string, any>):
   const raw = params.url as string;
   if (!raw) return { error: 'url parameter is required' };
   const url = resolveInput(raw);
+  let before = '';
+  try { before = wv.getURL(); } catch { /* mid-mount; landed check degrades gracefully */ }
   // loadURL resolves only on the full 'load' event, which heavy SPAs (LinkedIn, Gmail) hold open with persistent connections long past our timeout even though the page is usable in a second. Return the moment the DOM is ready and let the agent's next wait settle the rest, the way a person clicks before every background request has finished.
   let removeReady = () => {};
   const domReady = new Promise<void>((resolve) => {
@@ -376,9 +379,10 @@ async function handleNavigate(wv: BrowserWebview, params: Record<string, any>):
     wv.addEventListener('dom-ready', onReady, { once: true });
     removeReady = () => wv.removeEventListener('dom-ready', onReady);
   });
+  let aborted = false;
   const fullyLoaded = wv.loadURL(url).catch((err: any) => {
-    // A superseded navigation aborts the old load; that's normal, not a failure.
-    if (err?.message?.includes('ERR_ABORTED')) return;
+    // ERR_ABORTED can be a benign supersede OR a nav the guest refused outright; the landed-URL check below tells them apart.
+    if (err?.message?.includes('ERR_ABORTED')) { aborted = true; return; }
     throw err;
   });
   fullyLoaded.catch(() => {}); // a late load failure shouldn't throw once dom-ready returned
@@ -400,6 +404,27 @@ async function handleNavigate(wv: BrowserWebview, params: Record<string, any>):
       data_document: true,
     };
   }
+  // "Navigated to" used to be unconditional, so a wedged guest whose load was silently aborted reported success while still parked on the old page (Haik's 2-of-4 repro). Only claim success once the document provably moved.
+  if (before && !sameDoc(url, before)) {
+    let landed = before;
+    const settleDeadline = Date.now() + (aborted ? 1500 : 400);
+    for (;;) {
+      try { landed = wv.getURL(); } catch { /* keep last */ }
+      if (landed && !sameDoc(landed, before)) break;
+      if (Date.now() >= settleDeadline) break;
+      await new Promise((r) => setTimeout(r, 150));
+    }
+    const verdict = navigationOutcome(url, before, landed);
+    if (verdict.kind === 'stuck') {
+      return {
+        error: `Navigation to ${url} did NOT happen; the page is still on ${before}. The webview may be wedged: retry once, and if it fails again report the failure instead of answering from the current page.`,
+        url: verdict.url,
+      };
+    }
+    if (verdict.kind === 'redirected') {
+      return { text: `Navigated to ${landed} (redirected from requested ${url})`, url: landed };
+    }
+  }
   // Route-count is sampled on the next READ (handleGetText), not here: at navigate-return the SPA's XHRs haven't fired yet, so this would always be ~0.
   return { text: `Navigated to ${url}`, url };
 }

frontend/src/shared/navigationOutcome.test.ts

@@ -0,0 +1,41 @@
+// Haik's repro: 2 of 4 navigations never moved yet all 4 reported OK. These pin the verdict table
+// so a nav that leaves the document parked on the old URL can only ever read as a failure.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { navigationOutcome, sameDoc } from './navigationOutcome';
+
+test('a landed URL equal to the old page is stuck, never ok', () => {
+  const v = navigationOutcome('https://en.wikipedia.org/wiki/Helium', 'https://en.wikipedia.org/wiki/Argon', 'https://en.wikipedia.org/wiki/Argon');
+  assert.equal(v.kind, 'stuck');
+});
+
+test('an empty landed URL (webview gave nothing back) is stuck', () => {
+  const v = navigationOutcome('https://a.com/x', 'https://a.com/y', '');
+  assert.equal(v.kind, 'stuck');
+});
+
+test('landing on the requested URL is ok', () => {
+  const v = navigationOutcome('https://a.com/x', 'https://a.com/y', 'https://a.com/x');
+  assert.deepEqual(v, { kind: 'ok', url: 'https://a.com/x' });
+});
+
+test('landing elsewhere is reported as a redirect, not a silent ok', () => {
+  const v = navigationOutcome('https://site.com/settings', 'https://site.com/home', 'https://site.com/login?next=settings');
+  assert.equal(v.kind, 'redirected');
+  assert.equal(v.url, 'https://site.com/login?next=settings');
+});
+
+test('re-navigating to the current page is ok (reload, no moved-document requirement)', () => {
+  const v = navigationOutcome('https://a.com/x', 'https://a.com/x/', 'https://a.com/x');
+  assert.equal(v.kind, 'ok');
+});
+
+test('no before URL (mid-mount card) degrades to ok rather than a false failure', () => {
+  const v = navigationOutcome('https://a.com/x', '', '');
+  assert.deepEqual(v, { kind: 'ok', url: 'https://a.com/x' });
+});
+
+test('sameDoc ignores only the trailing slash', () => {
+  assert.equal(sameDoc('https://a.com', 'https://a.com/'), true);
+  assert.equal(sameDoc('https://a.com/x', 'https://a.com/x#frag'), false);
+});

frontend/src/shared/navigationOutcome.ts

@@ -0,0 +1,18 @@
+// Pure verdict for one navigation attempt, decided from URLs alone, so "Navigated to" can never
+// again be printed while the document is provably still parked on the old page (Haik's field report).
+export type NavOutcome =
+  | { kind: 'ok'; url: string }
+  | { kind: 'redirected'; url: string; requested: string }
+  | { kind: 'stuck'; url: string; requested: string };
+
+// Trailing-slash-insensitive, so loadURL("https://x.com") over "https://x.com/" doesn't read as "never moved".
+export function sameDoc(a: string, b: string): boolean {
+  return a.replace(/\/$/, '') === b.replace(/\/$/, '');
+}
+
+export function navigationOutcome(requested: string, before: string, landed: string): NavOutcome {
+  if (!before || sameDoc(requested, before)) return { kind: 'ok', url: landed || requested };
+  if (!landed || sameDoc(landed, before)) return { kind: 'stuck', url: landed || before, requested };
+  if (!sameDoc(landed, requested)) return { kind: 'redirected', url: landed, requested };
+  return { kind: 'ok', url: landed };
+}