Skip to content

fix(browser): recover stale page before evaluate - #2222

Open
shaomingbo wants to merge 2 commits into
jackwener:mainfrom
shaomingbo:agent/recover-stale-page-evaluate
Open

fix(browser): recover stale page before evaluate#2222
shaomingbo wants to merge 2 commits into
jackwener:mainfrom
shaomingbo:agent/recover-stale-page-evaluate

Conversation

@shaomingbo

Copy link
Copy Markdown

Summary

  • recover a stale cached page identity when it fails immediately before Page.evaluate()
  • rebind through the last known URL without the dead target before retrying once
  • keep explicitly selected pages fail-closed when no trustworthy URL is available

Root cause

The Browser Bridge can return a page identity from navigation that is evicted before the adapter's next evaluate call. The extension rejects that identity in resolveTabId() before any page code runs, but Page.evaluate() only retried target-navigation errors and kept resending the stale target. Adapters then surfaced a generic command execution failure even though the failure was recoverable at the page lifecycle boundary.

Page.goto() already had equivalent stale-identity recovery for later navigations. This change closes the missing evaluate path without adding adapter-specific retry logic.

Impact

Cookie-backed adapters can recover when a page target becomes stale between navigation and their first API evaluation. Other evaluate failures remain unchanged, and caller-selected tabs without a known URL are never replaced implicitly.

Validation

  • npm run typecheck
  • npm run build
  • npx vitest run --project unit src/browser — 28 files, 427 tests passed
  • focused stale-page replay — failed before the fix and passed twice after it

The full sandbox suite reached 6,546 passing tests; its remaining 40 failures were unrelated EPERM writes to the real user home, so they were not retried against live user configuration.

@shaomingbo
shaomingbo marked this pull request as ready for review August 2, 2026 07:51
@shaomingbo

Copy link
Copy Markdown
Author

@jackwener This is ready for review. The failure was reproduced from a retained Browser Bridge trace without additional site calls: navigation returned a target that became stale before the adapter's first evaluate(). The regression is at the generic Page seam, and the BOSS adapter remains unchanged.

Focused browser tests, typecheck, and build are green; the PR also keeps explicitly selected tabs fail-closed when no trustworthy URL is available.

@shaomingbo

Copy link
Copy Markdown
Author

Canary follow-up: the first patch covered stale identities during navigate and public evaluate, but an isolated real adapter run exposed one more page-bridge path.

The sequence was:

  1. navigate succeeded and returned a page identity.
  2. The post-navigation stealth/DOM-settle exec hit Inspected target navigated or closed.
  3. Its settle retry reused the now-stale page identity and failed at Page.goto with Page not found: … — stale page identity.

The trace had no failed network requests, and the stack pointed to Page.goto rather than the BOSS adapter. Commit 9d5e27a now rebinds once at the known URL when either settle attempt exposes a stale identity, then reruns settle with the fresh page. A second recovery failure is propagated; there is no retry loop.

Validation after the update:

  • exact regression: red before, green after, replayed green twice
  • src/browser/page.test.ts: 29/29
  • src/browser: 28 files, 428/428
  • npm run typecheck
  • npm run build
  • git diff --check

@shaomingbo

Copy link
Copy Markdown
Author

Second isolated canary on 9d5e27a provided positive boundary evidence for this PR:

  • fresh identity capture and activation succeeded
  • boss/search navigation completed without the previous stale-page failure
  • the collection reached bossFetch / Page.evaluate
  • it then failed later with a distinct detached_mid_command (Boss API request failed: Detached while handling command.)
  • the trace again reported no failed network requests

This confirms the stale identity path addressed here was crossed in the real adapter flow. The later failure has a different safety boundary: generic Page.evaluate must not replay an unknown mid-command outcome, while the BOSS adapter can identify an idempotent GET. That separate read-scoped fix is in draft PR #2224. No live retry was performed after that adapter patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant