Bind playwright execute page to Chrome's active tab

[dprevoznik]

Summary

browsers.playwright.execute injected page as context.pages()[0] — the oldest tab in the context — regardless of which tab was active. In any multi-tab session this bound page to the wrong tab, so calls like page.pdf(), page.title(), etc. operated on a different tab than the one the automation had just navigated to.

Change

• server/runtime/playwright-daemon.ts: added resolveActivePage, which resolves the browser's actual active tab over CDP — Target.getTargets filtered to tab targets, matched on TargetInfo.embedderData.tabActive (exposed by the Chrome 152.0.7977.42 build this image now ships, per Upgrade Chromium to 152.0.7977.42 #340), then mapped to a Playwright Page via Target.autoAttachRelated. Every CDP session it opens is temporary and detached before returning, so the daemon stays stateless across requests. page is bound to that resolved tab directly; resolution failures surface like any other daemon error, with no ordering heuristic or fallback.
• While scanning context.pages() for the matching tab, a page whose CDP session setup or Target.getTargetInfo call fails is skipped rather than aborting the whole search, so one crashed or closing tab can't block resolution of the real active tab.
• server/openapi.yaml: documents the selection policy on /playwright/execute — page is bound to an active tab reported by Chrome. In single-window sessions that's the foreground tab; with multiple browser windows open, Chrome reports one active tab per window and the selected window is unspecified. context.pages() remains the explicit escape hatch.
• server/e2e/e2e_playwright_test.go: extends the existing TestPlaywrightExecuteAPI (same container/warm daemon connection, no second container) to open a second tab via a data: URL and assert page binds to it, then bring the original tab — selected by URL rather than context.pages()[0] — back to the foreground and assert binding follows focus, not tab-creation order.

Scope

Depends on this image's Chrome 152.0.7977.42 exposing TargetInfo.embedderData.tabActive; single-tab sessions are unaffected. Cross-context foreground-tab resolution is out of scope — resolveActivePage searches the same context (contexts()[0]) already exposed to user code, matching this endpoint's existing single-context design rather than introducing a new limitation.

Testing

• go vet ./e2e/... passes.
• esbuild bundles the daemon cleanly.
• TestPlaywrightExecuteAPI (extended) passes in CI against the image's real Chrome 152 build, covering both the new-tab and refocus cases; requires docker, skips otherwise.

[masnwilliams]

requesting changes on two maintainability issues: the new default relies on an undocumented page-ordering contract and the regression test duplicates a full container setup for a one-line selection policy.

[masnwilliams]

BrowserContext.pages() only documents that it returns all open pages; it does not guarantee creation order. This makes Playwright's current internal Set insertion order part of our public page contract, while the comment also conflates newest with current/active (open B, bring A forward, and this still selects B). Can we define the selection policy in the OpenAPI contract and either own the intended state or describe this precisely as a last-discovered-page heuristic? We should not ship the behavior change while leaving callers to infer the invariant.

[dprevoznik]

Replaced the ordering-based heuristic with a CDP lookup of the browser's real foreground tab (Target.getTargets + tabActive, mapped to a Playwright Page via Target.autoAttachRelated). This is now only a fallback, used when the underlying Chrome build doesn't expose that signal — documented both in code (see resolveActivePage) and in the OpenAPI description for /playwright/execute, which now states the policy precisely: foreground tab when available, otherwise most-recently-opened, with context.pages() as the escape hatch for explicit selection.

[masnwilliams]

This adds 59 lines and another Docker container to verify a one-line policy change, duplicating the setup already in TestPlaywrightExecuteAPI. Can we fold this scenario into that existing warm-daemon test (or extract a shared harness if isolation matters)? While simplifying it, use deterministic data: URLs and compare JSON200.Result directly rather than adding another network dependency and JSON-marshal/substring assertion.

[dprevoznik]

Folded this into TestPlaywrightExecuteAPI, reusing its container/warm daemon connection instead of spinning up a second one. Swapped the second tab's https://example.net navigation for a data: URL so the tab-binding check no longer needs its own outbound request, and replaced the JSON-marshal/Contains check with a direct require.Equal against JSON200.Result.

[dprevoznik]

Fixed in 59c3e11: moved newBrowserCDPSession() inside the try so a session-creation failure falls back to the heuristic like every other failure path in resolveActivePage, instead of rejecting and failing the whole execute request.

[cursor]

Existing pages may not attach

High Severity

resolveActivePage fills relatedPageIds only from Target.attachedToTarget after Target.autoAttachRelated. CDP documents that command as monitoring related-target creation and reporting newly created related targets. For already-open tabs, the set can stay empty, so the loop never matches and execution fails with foreground tab has no matching Playwright page.

 

^{Reviewed by Cursor Bugbot for commit 5d9bab0. Configure here.}

[dprevoznik]

Verified this doesn't reproduce against the image's actual Chrome (152.0.7977.42): the e2e test added in this PR (TestPlaywrightExecuteAPI, https://github.com/kernel/kernel-images/actions/runs/32164219559/job/95800815565) explicitly covers a pre-existing related target — it opens a second tab, then several execute calls later brings the original tab (created well before the autoAttachRelated call in question, not a target created in that same request) back to the foreground and asserts the daemon binds to it. That assertion passed in CI. In practice, autoAttachRelated on this target's browser session fires attachedToTarget for already-open related targets at call time, not just future creations, so relatedPageIds is populated correctly for this case. Leaving as-is; will revisit if this surfaces on a real session.

[cursor]

Active tab ignored across contexts

Medium Severity

resolveActivePage picks the browser-wide foreground tab via CDP, but only searches for a matching Playwright Page inside the single context from contexts()[0]. If that tab lives in another context, binding throws and /playwright/execute fails before user code runs, so callers cannot recover via browser or context.pages().

Additional Locations (1)

• server/runtime/playwright-daemon.ts#L217-L225

 

^{Reviewed by Cursor Bugbot for commit f2de954. Configure here.}

[dprevoznik]

Leaving this as-is: contexts()[0] is the pre-existing selection this whole endpoint has always used for the 'context' variable exposed to user code — it predates this PR and applies equally to the empty-context newPage() path a few lines up. resolveActivePage searching only within that same context is consistent with what 'context' means for this daemon, not a new limitation this PR introduces. If the true foreground tab lives in a different browser context, the old heuristic would have silently bound 'page' to the wrong context's tab; this now fails loudly instead, which is arguably safer. Multi-context support for this endpoint would be a bigger, separate change to how 'context' itself is resolved.

[rgarcia]

reviewed — the active-tab resolution direction makes sense, and the test covers both opening a new tab and refocusing an older one. two things to address:

• server/openapi.yaml:1342 — replace the current page description with: “page is bound to an active tab reported by Chrome. In single-window sessions, this is the foreground tab. When multiple browser windows are open, Chrome reports one active tab per window and the selected window is unspecified. Use context.pages() to select a page explicitly.”

• server/e2e/e2e_playwright_test.go:119 — this test relies on context.pages()[0] being the oldest page, which is the same undocumented ordering assumption this change removes. Select the known original page instead:

  const first = context.pages().find(candidate =>
    candidate.url().includes('example.com')
  );
  if (!first) throw new Error('original page not found');
  await first.bringToFront();
  return first.url();

[dprevoznik]

Pushed 5195138 addressing both: reworded the openapi.yaml description to your exact multi-window wording, and swapped the e2e test's context.pages()[0] for a URL-based lookup (throws if the original page isn't found).

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 6703f7f. Configure here.}

[dprevoznik]

Both addressed and pushed (eb7424b, rebased on the latest main-merge): openapi.yaml now uses your exact multi-window wording, and the e2e test selects the original page by URL instead of context.pages()[0]. Also fixed BugBot's low-severity finding from the latest pass (a crashed/closing page could abort the whole foreground-tab search) and replied to its medium finding (cross-context foreground tab) explaining why it's a pre-existing constraint of this endpoint's single-context design rather than something this PR introduces. All CI checks pass; BugBot is clean (no new findings) on the current commit.