Skip to content

fix(browser): drop session injection from extension exec results#1518

Merged
jackwener merged 1 commit into
jackwener:mainfrom
hansnow:fix/evaluate-unpacks-bridge-data
May 13, 2026
Merged

fix(browser): drop session injection from extension exec results#1518
jackwener merged 1 commit into
jackwener:mainfrom
hansnow:fix/evaluate-unpacks-bridge-data

Conversation

@hansnow
Copy link
Copy Markdown
Contributor

@hansnow hansnow commented May 13, 2026

Summary

Fixes browser-backed searches that can visually render results in Chrome but return empty / NOT_FOUND from adapters because pageScopedResult() in the extension was injecting the lease's session into the result data. For the exec action this contaminated arbitrary user-JavaScript returns:

  • Array / primitive returns came back as { session, data: <value> } envelopes — Array.isArray(result) was false, adapters extracted nothing.
  • Plain-object returns had an extra session key spliced in (and any user session field was silently overwritten by the lease).

Originally proposed (this PR's first commit) as a client-side unwrap in Page.evaluate(). After upstream review the scope changed to fix the root cause in the extension and drop the client-side workaround.

This PR now:

  • Reverts pageScopedResult() to its pre-refactor(browser): replace workspace with explicit sessions #1461 form ({ id, ok, data, page }). No session injection. Result.session is not added to the protocol — no consumers exist and Result.page already exposes routing identity. Bumps extension to 1.0.14.
  • Waits for #rso a h3 (5s) before extracting Google SERP, falling back to the existing wait 2. Avoids empty extraction on Chrome 148 / Linux Wayland when DOM settles before SERP anchors populate.
  • Extracts visible Xiaohongshu cards before scrolling, then merges post-scroll rows by URL. Xiaohongshu's virtualized masonry can evict initial note cards from the DOM after scroll.

Original commit attribution kept on @hansnow; scope rewritten with maintainer push.

Repro environment (from original report)

  • OpenCLI: 1.7.18
  • Browser Bridge extension: 1.0.121.0.14
  • Google Chrome: 148.0.7778.96
  • OS/session: Linux 7.0.0-15-generic x86_64, Wayland
  • Node.js: 22.22.1, npm: 9.2.0

Observed failure mode:

  • opencli google search "美食" --limit 5 --lang zh -f yaml
  • opencli xiaohongshu search "美食" --limit 5 -f yaml

Chrome showed normal public search results, but adapters received non-array values and treated the extraction as empty.

Validation

  • npm run typecheck
  • npm run build
  • npx vitest run --project unit — 1083 passed (one unrelated EADDRINUSE from daemon.test.ts port contention on local machine)
  • npx vitest run --project extension — 64 passed
  • npx vitest run --project adapter clis/xiaohongshu/ clis/google/ — 113 passed
  • Extension vite build — 75.49 kB

Root cause history

The session injection in pageScopedResult was added by #1461 (refactor(browser): replace workspaces with sessions). The same refactor also caused the doctor connectivity regression fixed in 1.7.18 — both were silent breakages from a partial refactor that wasn't end-to-end smoke-tested. Lesson recorded for future large refactors.

`pageScopedResult()` in extension/src/background.ts was spreading the
lease's session into the result `data` for every page-scoped command. For
the `exec` action — which routes user JavaScript through page.evaluate()
— this contaminated arbitrary user-JS returns:

* Array / primitive returns came back as `{ session, data: <value> }`
  envelopes. Adapters that did `Array.isArray(result)` got `false` and
  treated the page as having no rows. Visible repro:
  `opencli google search ...` and `opencli xiaohongshu search ...` —
  Chrome rendered results correctly but adapters extracted an empty array
  (reported in jackwener#1518 from the Browser Bridge v1.0.12 envelope).
* Plain-object returns had an extra `session` key spliced in, silently
  overwriting any user `session` field with the lease's value.

Fix in the extension layer instead of compensating client-side:
`pageScopedResult` now returns `{ id, ok, data, page }` — the same form
it had before jackwener#1461 added the workspace→session refactor. Client-side
unwrapping is no longer needed and the original PR jackwener#1518 `Page.evaluate`
heuristic is dropped (it only covered the array path and would have
missed the plain-object path).

Two adapter improvements kept from the original PR:

* `clis/google/search.js` — wait for `#rso a h3` (with a 5s timeout)
  before extracting. On Chrome 148 / Linux Wayland the DOM can settle
  before SERP anchors are populated, so the existing fixed `wait 2`
  could return empty even with the envelope fix.
* `clis/xiaohongshu/search.js` — extract initially visible cards before
  scrolling, then merge post-scroll rows by URL. Xiaohongshu's
  virtualized masonry can evict the initial note cards from the DOM
  after scroll, causing extraction to return [] even though the
  browser had rendered results correctly.

Extension version bumped to 1.0.14.

Repro environment (from jackwener#1518):

* OpenCLI 1.7.18
* Browser Bridge extension 1.0.12 → 1.0.14
* Chrome 148.0.7778.96
* Linux Wayland, Node 22.22.1

Tests: extension/src/background.test.ts navigate same-url assertion
updated to no longer expect `session` in `data`. Three Page.evaluate
unwrap test cases removed.
@jackwener jackwener force-pushed the fix/evaluate-unpacks-bridge-data branch from efb0619 to 9c64a54 Compare May 13, 2026 09:27
@jackwener jackwener changed the title fix(browser): unwrap Browser Bridge exec envelopes fix(browser): drop session injection from extension exec results May 13, 2026
Copy link
Copy Markdown
Owner

@jackwener jackwener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified the extension-side fix keeps exec/evaluate data raw by reverting pageScopedResult to { id, ok, data, page }, with no Page-side unwrap residue. Extension 1.0.14 version files and dist are synced; Google/Xiaohongshu adapter fixes are scoped and reasonable. Local checks: extension background 52/52, Page unit 21/21, Xiaohongshu adapter 19/19, root typecheck/build, extension typecheck/build, diff-check.

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.

2 participants