Skip to content

fix(chrome): parse PDFs in offscreen document - #2983

Merged
esokullu merged 5 commits into
webbrain-one:mainfrom
16Miku:fix/read-pdf-mv3-offscreen
Sep 6, 2026
Merged

fix(chrome): parse PDFs in offscreen document#2983
esokullu merged 5 commits into
webbrain-one:mainfrom
16Miku:fix/read-pdf-mv3-offscreen

Conversation

@16Miku

@16Miku 16Miku commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move PDF.js loading out of the Chrome MV3 service worker and into the shared offscreen document
  • keep the agent-facing read_pdf result shape and Claude-compatible native PDF passthrough
  • add focused coverage plus a real Chrome offscreen PDF extraction assertion

Why

Chrome MV3 rejects dynamic import() from ServiceWorkerGlobalScope, so read_pdf failed when pdf-tools.js tried to lazily load PDF.js. The offscreen extension page can load PDF.js and its worker normally, while avoiding the startup cost for users who never read PDFs.

This change is Chrome-specific because the failure is caused by the MV3 service-worker runtime; Firefox's MV2 background context is not affected.

Testing

  • npm run test:pdf-read — 3 passed
  • npm run test:pdf-mime-handler — passed with Chrome 152.0.7977.76
  • node test/run.js — passed
  • manually loaded the unpacked Chrome extension and read both pages of The Bitter Lesson; read_page returned success: true, totalPages: 2, and pageCount: 2, with no ServiceWorkerGlobalScope import error

@vercel

vercel Bot commented Sep 5, 2026

Copy link
Copy Markdown

@16Miku is attempting to deploy a commit to the esokullu's projects Team on Vercel.

A member of the Team first needs to authorize it.

@esokullu

esokullu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Right architecture for MV3 (PDF.js out of the service worker). Three concrete issues:

Cold-start race. ensureOffscreen() returns before the module host’s onMessage is registered — first read_pdf can get “returned no result.” Add a ready handshake or short retry.
Open credentialed fetch. Offscreen accepts offscreen-pdf-extract from any extension context and fetches with credentials: 'include', no scheme allowlist. Restrict to http/https/file and gate senders.
Dual-fetch on Claude passthrough. SW and offscreen each fetch the same URL — signed/cookie-rotated URLs can desync text vs bytes. One fetch for that path.

@16Miku

16Miku commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed all three points in 9ab4a12f:

  • Cold-start race: read_pdf now waits for an explicit offscreen-host ready response with a bounded retry before sending the extraction request.
  • Credentialed fetch boundary: the host accepts PDF messages only from this extension's src/background.js service worker (matching extension ID, exact sender URL, and no tab), and rejects URLs outside http:, https:, and file: before fetch.
  • Claude dual fetch: the offscreen host now fetches once, derives both extracted text and the size-bounded Claude base64 document from those same bytes, and encodes before PDF.js can transfer/detach its input buffer.

Added regression coverage for the ready retry, sender and scheme rejection, pre-fetch blocking, preserved byte length, one-request Claude extraction, and byte-for-byte document identity.

Validation:

  • npm run test:pdf-read — 4 passed
  • npm run test:pdf-mime-handler — passed in Chrome 152.0.7977.76
  • node test/run.js — 2198 passed, 0 failed
  • Manual cold-start test after reloading the unpacked extension successfully read both pages of The Bitter Lesson on the first read_pdf call.

The current WebMCP smoke failure is the same pre-existing fixture page error timeout reproduced by main at e6556ed2; the PDF MIME/offscreen test itself passes.

@esokullu

esokullu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Re-reviewed at high effort against the branch head. 9ab4a12f2 resolves all three of my review comments; no open findings from me.

(Correcting my own earlier line here: I said the gh pr diff view was stale and omitted 9ab4a12f2. That was wrong — the diff includes both commits.)

Cold-start racewaitForPdfExtractionHost() now waits on a PDF_EXTRACTION_READY_MESSAGE handshake with a 40 × 25ms probe, rather than assuming the offscreen document is listening.

Open credentialed fetchnormalizePdfUrl() restricts to http/https/file and validates before the fetch, and isTrustedPdfExtractionSender() gates the sender to chrome-extension://<id>/src/background.js, which matches the manifest's service_worker entry. The e2e proves a settings page is rejected and never reaches the network.

Dual fetch — the offscreen host base64-encodes the same buffer it parses, before PDF.js can detach it, and returns it as _pdfBase64; byteLength is captured pre-getDocument for the same reason. The e2e asserts exactly one server request.

One thing worth knowing rather than changing: a 16MB PDF now crosses chrome.runtime.sendMessage as roughly 21MB of base64. That's the cost of the single-fetch guarantee, and it seems like the right trade here, but it's worth a comment in the code so the next person doesn't try to "optimize" the encode back into a second fetch.

🤖 Generated with Claude Code

https://claude.ai/code/session_0196wte9FS1SnsrKvrQUbB7m

@16Miku

16Miku commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Added the suggested why-comment in commit ee4e104. It documents that the approximately 4/3 base64 message overhead is an intentional tradeoff for preserving the single-fetch, byte-identical guarantee. Validation after the comment-only change: npm run test:pdf-read (4 passed); npm run test:pdf-mime-handler (passed in Chrome 152.0.7977.76); git diff --check (passed).

Four fixes from a review pass over the offscreen extraction path.

- waitForPdfExtractionHost() treated every non-ready response as transient.
  An "Unauthorized PDF extraction sender." refusal is permanent, but it
  burned all 40 attempts and then surfaced as "did not become ready", which
  points a debugger at the wrong subsystem. It also never re-ensured the
  document, so an offscreen page that went away between ensureOffscreen()
  and the probe failed read_pdf permanently instead of recreating the host.
- isTrustedPdfExtractionSender() hardcoded 'src/background.js', duplicating
  the manifest. Renaming the service worker entry would have rejected every
  extraction with that same misleading error, and no test would have caught
  it. The path now comes from getManifest().
- The scheme allowlist hard-failed on our own viewer tabs. With the native
  MIME handler on, a PDF tab's URL is src/ui/pdf-handler.html?url=..., and
  read_pdf falls back to the tab URL, so it returned "PDF URL must use
  http:, https:, or file:." normalizePdfUrl() now unwraps the inner URL for
  that page only, and the unwrapped URL still passes the allowlist.
- The passthrough base64 was encoded before parsing, so a corrupt PDF that
  threw in getDocument() still paid for a string nobody reads. The bytes are
  copied before parsing, preserving the single-fetch guarantee, and encoded
  only after extraction succeeds.

Also waits for the service worker in the MIME-handler e2e: serviceWorkers()
is a snapshot, so the lookup could miss a worker Chrome had idled out.

node test/run.js: 2198 passed, 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0196wte9FS1SnsrKvrQUbB7m
@esokullu

esokullu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Ran a second review pass scoped to this PR alone — the first one was shared with a much larger PR and I didn't trust "no findings" from it. Five issues, four now fixed in aa8c6fb8f. node test/run.js is 2198/0.

Rebased onto ee4e10445 rather than over it, and folded your base64-overhead comment into the block I was rewriting — the reasoning you wrote still applies, so it's preserved verbatim alongside the new bit.

Fixed in aa8c6fb8f

Readiness probe retried permanent failures (pdf-tools.js:44) — the loop treated every non-ready response as transient. An Unauthorized PDF extraction sender. refusal is permanent, but it burned all 40 attempts and then surfaced as The offscreen PDF parser did not become ready. Unauthorized PDF extraction sender., which sends whoever is debugging to the wrong subsystem. The loop also never re-called ensureOffscreen(), so if the offscreen document went away between ensureOffscreen() seeing it via hasDocument() and the probe firing, read_pdf failed permanently instead of recreating the host. It now aborts on an explicit error and re-ensures on a connection error.

Sender gate hardcoded the background path (pdf-extraction.js:33) — 'src/background.js' duplicated manifest.json's background.service_worker. Renaming that entry would have rejected every extraction with the same misleading "not ready" error above, and nothing would have caught it, since test/pdf-read.mjs hardcoded the identical string. It now derives from getManifest(), with a test that renames the entry and asserts the old path stops being trusted.

The scheme allowlist hard-failed on our own viewer tabs (pdf-extraction.js:23) — this is the one worth a look. With the native MIME handler from #2980 enabled, a PDF tab's URL is chrome-extension://<id>/src/ui/pdf-handler.html?url=<real>&tabId=N (background.js:1582), and read_pdf with no url argument falls back to chrome.tabs.get(tabId).url (agent.js:27774). So the new check returned PDF URL must use http:, https:, or file:. on the product's own default PDF path. Not a regression — before this PR it fetched the handler HTML and died inside PDF.js — but since the hunk was already normalizing the URL, normalizePdfUrl() now unwraps searchParams.get('url') for that one page. The unwrapped URL still goes through the allowlist, so a handler URL wrapping javascript: is still rejected; tests cover both that and the case of another extension page trying to use ?url= as a bypass.

Base64 encoded before parsing (pdf-extraction-host.js:40) — a corrupt PDF that threw in getDocument() still paid the full encode. The bytes are now copied before parsing, which keeps your single-fetch, byte-identical guarantee intact, and the encode happens only after extraction succeeds.

Flaky service-worker lookup (pdf-mime-handler-e2e.mjs:182) — context.serviceWorkers() is a synchronous snapshot with no wait, so the assertion could fail spuriously if Playwright hadn't observed the worker yet or Chrome idled it out during the preceding settings.evaluate calls. Falls back to waitForEvent('serviceworker') now.

Still open

_isPdfTab can't recognize handler URLs (agent.js:5371) — same root cause as the third fix above, but on the other side: the automatic read_pageread_pdf redirect tests the tab URL against isPdfUrl, which a chrome-extension://.../pdf-handler.html?url=... tab won't match. So read_pdf now works on those tabs when called directly, but the agent won't be routed there on its own. I left it alone because the fix belongs with the redirect logic rather than in this PR's extraction path, and it wants its own e2e — happy to do it here if you'd rather not split it.

Checked, not findings

fetchPdfBytes has no remaining importer in the Chrome tree; the _pdfBytes_pdfBase64 rename is complete; {type: 'offscreen-pdf-extract'} can't be intercepted by a sibling listener, since background.js:2777 gates on msg.target !== 'background' and every other offscreen host gates on its own type set; the CSP permits both the dynamic import() and PDF.js's module worker; and leaving the Firefox tree untouched is correct here, since MV2 allows dynamic import() in the background and no parity check covers these files.

Worth crediting: encoding before getDocument() fixes a latent bug on main, where byteLength and _pdfBytes were read after PDF.js may already have detached the buffer — the Claude passthrough could ship an empty document block. That wasn't in the PR description.

🤖 Generated with Claude Code

https://claude.ai/code/session_0196wte9FS1SnsrKvrQUbB7m

@esokullu

esokullu commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

@16Miku can you review the latest changes, do you think it's merge-ready? particularly aa8c6fb

Thanks!

@16Miku

16Miku commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the additional review and for the fixes in aa8c6fb8. I synced the updated branch locally, reviewed the new diff, and reran both automated and manual validation.

Review of aa8c6fb8

I reviewed the changes around:

  • aborting the readiness loop immediately on an explicit host refusal;
  • re-running ensureOffscreen() when the ready probe loses its listener;
  • deriving the trusted background service-worker path from manifest.json instead of hardcoding src/background.js;
  • unwrapping only WebBrain's own pdf-handler.html?url=... URL while still applying the http: / https: / file: allowlist to the inner URL;
  • copying the passthrough bytes before PDF.js can detach the input, while deferring base64 encoding until parsing succeeds;
  • waiting for the service worker in the MIME-handler E2E instead of relying only on the synchronous serviceWorkers() snapshot.

The changes match the issues described in the review. I did not find an additional blocking issue in the updated extraction path.

Automated validation

Tested locally on Node.js v23.10.0 and Chrome 152.0.7977.76:

  • npm run test:pdf-read4 passed, 0 failed
  • npm run test:pdf-mime-handlerpassed
  • node test/run.js2198 passed, 0 failed
  • git diff --checkpassed

The PDF tests cover the ready retry/re-ensure behavior, permanent refusal handling, manifest-derived sender path, handler URL unwrapping and scheme rejection, single-fetch behavior, byte identity, and the MIME-handler service-worker wait.

Manual Chrome validation

I reloaded the unpacked extension from the updated branch and tested The Bitter Lesson PDF in Ask mode with WebBrain 34.1.6 / WebBrain Compass.

The following real-browser requests all completed successfully:

  1. “请读取并概括整个 PDF” (Read and summarize the entire PDF.)

    • WebBrain called read_pdf.
    • Result: success: true, totalPages: 2, pageCount: 2.
    • Both pages were read and summarized.
  2. “读取并说明本页面全部内容” (Read and explain all content on this page.)

    • WebBrain automatically selected read_pdf.
    • Result: success: true, all 2 pages read.
  3. “读取当前页面” (Read the current page.)

    • WebBrain automatically selected read_pdf.
    • Result: success: true, all 2 pages read.
  4. Explicit WebBrain viewer path.

    • From the PDF page, I used Open PDF with WebBrain and confirmed that the tab URL changed to WebBrain's chrome-extension://.../src/ui/pdf-handler.html?url=...&tabId=... page.
    • On that extension page, WebBrain resolved the wrapped original URL and successfully called read_pdf.
    • Result: success: true, totalPages: 2, pageCount: 2, with a correct summary.
    • No offscreen readiness, sender authorization, URL-scheme, dynamic import, or PDF.js parsing error appeared in these runs.

Remaining _isPdfTab routing gap

I also tried to force the explicit handler-page case with the original Chinese prompt: “请使用 read_page 工具读取当前页面,不要直接调用 read_pdf。” (Please use read_page to read the current page; do not call read_pdf directly.) The model still selected read_pdf, following the PDF-specific tool guidance, and the PDF read succeeded.

Therefore, the manual run verifies that the explicit pdf-handler.html URL is correctly unwrapped and readable, but it does not directly exercise the internal read_page -> read_pdf redirect or prove that _isPdfTab recognizes the handler URL. I agree that this remaining routing gap is separate from the offscreen extraction work in this PR and would be better handled in a follow-up change with a dedicated E2E that invokes the read_page path deterministically.

Conclusion

Based on the code review, automated tests, and real Chrome validation, I consider PR #2983 merge-ready for its current offscreen PDF extraction scope. The remaining _isPdfTab handler-page auto-routing case is non-blocking for this PR and should be tracked and tested separately.

…sponse

Three follow-ups on the offscreen PDF extraction host:

getPdfjs() memoized its rejection as well as its result. The old pdf-tools.js
assigned the module only after the await succeeded, so a failed import retried
on the next call. Because the offscreen document outlives any single read, one
transient failure of the vendor/pdfjs/pdf.mjs import made every later read_pdf
fail with the same stale error until the document was torn down. Clear the
promise on rejection to restore the retry.

PDF_HANDLER_PAGE hardcoded the viewer path, duplicating the manifest's
mime_types_handler handler_url — the same duplication we removed for
background.service_worker. Renaming the viewer would have made read_pdf on our
own PDF tabs fail with a misleading scheme error. Derive it from getManifest(),
keeping the literal as a fallback for runtime stubs without getManifest.

A throwing sendResponse on the success path fell through to the catch, which
responded a second time on a closed channel; that second throw became an
unhandled rejection and swallowed the original error. Route both replies
through a single-shot respond().

Also hoists the per-message message-type array into a module-level Set, matching
vision-inference-host.js.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrohYGnD3GTHtRXsA1vhh1
@esokullu
esokullu merged commit 85db978 into webbrain-one:main Sep 6, 2026
0 of 2 checks passed
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