Skip to content

fix(extension-chrome): build content scripts as classic IIFE so Chrome injects them - #175

Merged
chrischall merged 1 commit into
mainfrom
claude/fetchproxy-reservation-mcps-debug-3724a7
Jul 29, 2026
Merged

fix(extension-chrome): build content scripts as classic IIFE so Chrome injects them#175
chrischall merged 1 commit into
mainfrom
claude/fetchproxy-reservation-mcps-debug-3724a7

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Root cause

MV3 injects content scripts as classic scripts. A built content-script file that carries a top-level import/export is not a valid classic script, so Chrome silently refuses to inject it — content.ts's chrome.runtime.onMessage listener never registers, and every service-worker→content-script sendMessage fails with "Could not establish connection. Receiving end does not exist" (surfaced by the bridge as "N URL match(es), none responded").

build.ts compiled every entry point with format: 'esm'. That was harmless until 0ce0949 (#148) added export function readDomValues to content.ts: the bundle became a real ES module ending in export { readDomValues };, and content.js stopped injecting. capture-logger.js (no exports) kept injecting — so CSRF/storage capture still worked while every fetch() through the bridge died. That is the "bridge down / Receiving end does not exist" outage seen in the field for the fetchproxy-backed cohort MCPs (opentable, tock); Resy was unaffected because it doesn't use the bridge.

Diagnosed live: on a fresh, signed-in, fully-loaded OpenTable tab, data-fetchproxy-csrf was present (capture-logger alive) yet the fetch still returned "2 URL matches, none responded" — proving the split was content.js-specific, and the built file ended in export { readDomValues };.

Fix

Split the esbuild build:

  • background + popup stay esm (genuine modules — manifest "type": "module" and <script type="module">).
  • content.ts + capture-logger.ts build as iife, which emits no top-level import/export regardless of what the source exports.

Test

tests/content-scripts-classic.test.ts asserts the content-script bundles contain no top-level import/export (and that content.js still registers its onMessage/fetchproxy-fetch listener). It fails on the old esm config and passes on iife.

Full suite green (985 tests), typecheck clean.

🤖 Generated with Claude Code

…e injects them

MV3 injects content scripts as *classic* scripts. A built content-script
file that carries a top-level `import`/`export` statement is not a valid
classic script, so Chrome silently refuses to inject it — its
`chrome.runtime.onMessage` listener never registers and every
service-worker→content-script `sendMessage` fails with "Could not
establish connection. Receiving end does not exist" (surfaced by the
bridge as "N URL match(es), none responded").

`build.ts` compiled every entry with `format: 'esm'`. That was fine
until commit 0ce0949 (#148) added `export function readDomValues` to
`content.ts`: the bundle became a real ES module ending in
`export { readDomValues };`, and `content.js` stopped injecting. Because
`capture-logger.js` has no exports it kept injecting, so storage/CSRF
capture still worked while every fetch() through the bridge died —
exactly the "bridge down / Receiving end does not exist" outage seen in
the field for the fetchproxy-backed cohort MCPs (opentable, tock).

Split the build: background + popup remain `esm` (genuine modules —
manifest `"type":"module"` and `<script type="module">`), while
content.ts + capture-logger.ts build as `iife`, which emits no top-level
import/export regardless of what the source exports.

Adds tests/content-scripts-classic.test.ts to enforce the invariant
(fails on the old esm config, passes on iife).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chrischall chrischall added the bug Something isn't working label Jul 29, 2026
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Jul 29, 2026
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 4m 35s —— View job


I'll analyze this and get back to you.

Comment on lines +63 to +68
it('background is loaded as a module service worker', async () => {
// Not asserting on export/import here — background is declared
// `"type": "module"` in the manifest and popup via
// `<script type="module">`, so ESM output is correct for them.
expect(moduleEntryOptions.format).toBe('esm');
expect(contentScriptEntryOptions.format).toBe('iife');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: this test's name ("background is loaded as a module service worker") doesn't match what it checks — it only re-asserts the static .format string on the same option objects under test, without ever building/inspecting a background.js bundle. It's tautological (true by construction from the object literals in build.ts) and doesn't add regression coverage beyond what the first describe block already exercises for content scripts. Consider either dropping this test or replacing it with an actual bundle build + assertion that the background output contains a real import/export (mirroring the classic-script checks above, inverted).

Fix this →

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review: build content scripts as classic IIFE

Root cause and fix are correct. MV3 content scripts must be classic scripts (no top-level import/export); esbuild's format: 'esm' for content.ts broke injection once readDomValues was exported (#148). Splitting the build into moduleEntryOptions (esm, for background/popup which are genuine modules per the manifest's "type": "module" and <script type="module">) vs. contentScriptEntryOptions (iife, for content/capture-logger) is the right fix, and matches manifest.json's existing classic content_scripts entries.

Other observations, all verified against the code:

  • HERE/CORE/OUT now resolve relative to build.ts's own location instead of process.cwd() — a real robustness improvement (previously relied on the script being invoked from the package directory).
  • The new process.argv[1]-based guard around void main() correctly prevents the build from running as a side effect when the test imports build.js for its exported option objects — confirmed by the passing CI run (both CI runs for this PR are green, and npm run build + npm test both exercise this).
  • tests/content-scripts-classic.test.ts directly encodes the regression (no top-level import/export in the content-script bundles, plus a sanity check that content.js still registers its onMessage listener) — good, targeted coverage of the actual bug.
  • I was unable to install dependencies / run the suite myself in this sandbox (npm/node invocations required approval that wasn't available), so I relied on static review plus the CI status API, which shows this PR's CI checks passing.

One nit (left inline): the third test ("background is loaded as a module service worker") doesn't actually build or inspect a background.js bundle — it just re-asserts the static .format field on the same option objects, which is tautological and adds no regression coverage.

No correctness, security, or convention issues found.

Verdict: pass

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

🟡 Auto-review verdict: warn — The classic-IIFE build fix correctly resolves the MV3 content-script injection bug and is backed by a targeted regression test; only a minor test-quality nit was found, no correctness/security/convention issues.
📋 Tracking follow-ups: #176

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Jul 29, 2026
@chrischall
chrischall enabled auto-merge (squash) July 29, 2026 18:39
@chrischall
chrischall merged commit f4a3728 into main Jul 29, 2026
18 checks passed
@chrischall
chrischall deleted the claude/fetchproxy-reservation-mcps-debug-3724a7 branch July 29, 2026 18:40
chrischall added a commit that referenced this pull request Jul 29, 2026
…o client (#178)

## Summary

Adds a new opt-in `graphql` capability so an MCP can invoke a
page-declared GraphQL operation through the matched tab's own
`window.__APOLLO_CLIENT__` in the MAIN world, instead of the
isolated-world `fetch()` path.

**Why:** some endpoints (OpenTable's `RestaurantsAvailability`) reject
the isolated-world `fetch` at the edge — the bot-detection telemetry
(Akamai) lives inside the page's own Apollo link chain, not on
`window.fetch`. Routing through the real client clears it. Verified live
against a signed-in opentable.com tab (La Belle Hélène, restaurant_id
1175428): the isolated-world path 403/409s; `client.query(...)` through
the page's own Apollo client returns 200 with real slots, using the
MCP's existing (unchanged) variable shape.

**Design:** the extension carries no hardcoded query text or
persisted-query hash. It captures the live `DocumentNode` the page's
Apollo client already observed for a declared `operationName` and reuses
it — so it auto-adapts when a site revises its query. Gated by
capability + a declared `graphqlOps` allowlist (approved and diffed at
pair time, like every other capability) + the existing domain allowlist
and host-or-subdomain tab match. Full design + the live PoC findings:
`docs/superpowers/specs/2026-07-29-graphql-page-apollo-capability.md`.

**Caught in review:** the extension-bridge task initially shipped the
MAIN-world bridge with a top-level ES `export`, which MV3 silently
refuses to inject as a classic content script (this branch was missing
the classic-IIFE build split from #175). Fixed by rebuilding
`extension-chrome/build.ts` with the `content`/`capture-logger` →
`format:'iife'` split, with a regression test
(`content-scripts-classic.test.ts`) pinning no top-level export in
either bundled script.

Also added size guards on `graphql_query` request `variables` / response
`data`, mirroring the existing `fetch` op's
`MAX_REQUEST_BODY_BYTES`/`MAX_RESPONSE_BODY_BYTES` convention.

No version bumps (release-please owns that).

## Test plan

- [x] `npm test` — 1042/1042 passing across 75 files
- [x] `npm run build` — clean across all 7 workspaces
- [x] `npm run typecheck` — clean
- [x] Confirmed no top-level `export` in bundled
`content.js`/`capture-logger.js`
- [x] Live-verified the Apollo-client routing approach against
opentable.com in a real signed-in tab (see spec doc)
- [ ] Follow-up (separate PRs, gated on this releasing): bump
`@fetchproxy/server` in `mcp-utils`, then switch `opentable-mcp`'s
`find_slots` to the new capability and re-verify live

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Jul 29, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.7.0](v1.6.2...v1.7.0)
(2026-07-29)


### Features

* **graphql:** route declared GraphQL ops through the tab's own Apollo
client ([#178](#178))
([0c3fdf4](0c3fdf4))


### Bug Fixes

* **extension-chrome:** build content scripts as classic IIFE so Chrome
injects them
([#175](#175))
([f4a3728](f4a3728))
* **graphql:** address all four tracked nits from PR
[#178](#178
auto-review
([#180](#180))
([9d88ac9](9d88ac9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR bug Something isn't working ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant