fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter#579
Merged
fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter#579
Conversation
interact's get_screenshot/get_text waited the full 45s for a viewer that never mounted (host went idle before iframe reached startPolling), then timed out with a generic message. Now tracks viewsPolled and fails in ~8s with a recovery hint. Batch errors were also ordered success-first — hosts that flatten isError to content[0] showed "Queued: Filled N fields" and dropped the actual failure entirely. Viewer side: poll even when PDF load throws, and catch connect() rejection instead of silently blanking.
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-preact
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-solid
@modelcontextprotocol/server-basic-svelte
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-basic-vue
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-debug
@modelcontextprotocol/server-map
@modelcontextprotocol/server-pdf
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-shadertoy
@modelcontextprotocol/server-sheet-music
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-transcript
@modelcontextprotocol/server-video-resource
@modelcontextprotocol/server-wiki-explorer
commit: |
…veness # Conflicts: # examples/pdf-server/server.test.ts # examples/pdf-server/server.ts
…annotation updates Three issues from a hung interact in production: 1. Batch error swallowed by host flattening. The earlier fix unshifted the error before successes, but the prefix went on top of THAT — so content[0] was just "Batch failed at step 2/2 (get_screenshot):" with the actual timeout text in content[1], which the host dropped. Now all text joins into one block; images survive after. 2. update_annotations silent no-op. The viewer's processCommands does `if (!existing) continue` for unknown ids. If a prior add_annotations failed mid-batch the model gets a happy "Queued" while nothing renders. Track ids server-side; warn (not error — user might have drawn the annotation manually in the iframe). 3. Per-def isolation in the viewer. A throw in addAnnotation killed the whole processCommands batch including any get_pages later in the queue, so submit_page_data never fired and the server waited the full 45s. Now each def gets its own try/catch. Also: stderr logging on interact failure (only — not chatty per-call) so the next hang shows step timing in mcp-server-*.log instead of nowhere.
The host's content[0] flattening drops images on successful batches just the same: [add_annotations, get_screenshot] → content = ["Queued: ...", <image>] → model sees only the text. Observed in audit logs where the exact same iframe + viewUUID returned an image when the batch was [get_screenshot, get_screenshot] but only text when add_annotations came first. Stable-sort images to the front. The Queued lines still arrive, just after what the model actually asked to see.
This reverts commit 129a2dc.
…mething to lose LocalAgentMode SDK collapses isError:true results to a bare string of content[0].text. The squash-everything-into-one-block defense from 0dbb8ed worked but lost the positional contract — the model couldn't tell which command produced which result. Cleaner: content[i] is the result of commands[i]. If a step fails the batch stops there with that slot's text starting with "ERROR", and content.length tells the model how far it got. No isError on multi-step failures — there's no way to set it without the SDK eating the prior images. Single-command failures keep isError:true since there's nothing before content[0] to lose; the SDK's flatten-to-content[0].text is exactly the ERROR text we want shown. Tool description documents the contract.
…At===0
failedAt===0 in a multi-step batch still has a positional contract to
honor — content.length=1 vs commandList.length=3 is a signal the model
loses if the SDK flattens the array to a string. The ERROR text encodes
"step 1/3" but the simpler rule ("batches never set isError") is
easier to document and reason about.
…tiny measurements currentDisplayMode defaults to "inline" and handleHostContextChanged only writes it `if (ctx.displayMode)`. If the host's initial context omits the field — or the update lands one tick after a renderPage() — requestFitToContent measures a near-empty pageWrapper and sends ~85px (toolbar + padding only), shrinking a fullscreen iframe to a sliver. Workaround was exit + re-enter fullscreen to force a context update. Read app.getHostContext().displayMode directly (always fresh), and separately refuse to send when pageWrapper measures smaller than the toolbar — that's never a real layout, just an early render.
The viewer's annotationMap is baseline (loadBaselineAnnotations from the PDF) + diff restored from localStorage + manual edits + model adds. viewAnnotationIds only tracks the last category, so the warning fires for any update to an annotation the model didn't itself add — including the entire restored state. That's not "best-effort" miss rate, it's structurally wrong about where the data lives. Model would learn to ignore the warning. The viewer-side log.error on the silent skip (mcp-app.ts:4251) is the right layer for this.
ochafik
added a commit
that referenced
this pull request
Apr 1, 2026
Changes since 1.3.2: - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) - feat(pdf-server): add save_as interact action (#580) - fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix(pdf-server): render page before O(numPages) annotation scans (#581) - fix(pdf-server): radio + dropdown in fill_form/save (#577) - fix(deps): bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576)
ochafik
added a commit
that referenced
this pull request
Apr 2, 2026
Changes since 1.3.2: - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) - feat(pdf-server): add save_as interact action (#580) - feat(pdf-server): fit-to-page on fullscreen + pinch-to-zoom (#583) - fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix(pdf-server): render page before O(numPages) annotation scans (#581) - fix(pdf-server): radio + dropdown in fill_form/save (#577) - fix(deps): bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576)
ochafik
added a commit
that referenced
this pull request
Apr 2, 2026
Changes since 1.3.2: - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) - feat(pdf-server): add save_as interact action (#580) - feat(pdf-server): fit-to-page on fullscreen + pinch-to-zoom (#583) - fix(pdf-server): npx DOMMatrix crash + broken MCPB bundle (#584) - fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix(pdf-server): render page before O(numPages) annotation scans (#581) - fix(pdf-server): radio + dropdown in fill_form/save (#577) - fix(deps): bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576)
ochafik
added a commit
that referenced
this pull request
Apr 2, 2026
Changes since 1.3.2: - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) - feat(pdf-server): add save_as interact action (#580) - feat(pdf-server): fit-to-page on fullscreen + pinch-to-zoom (#583) - fix(pdf-server): npx DOMMatrix crash + broken MCPB bundle (#584) - fix(pdf-server): viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix(pdf-server): render page before O(numPages) annotation scans (#581) - fix(pdf-server): radio + dropdown in fill_form/save (#577) - fix(deps): bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576) - chore(deps): npm audit fix — sdk 1.29.0, systeminformation 5.31.5, +13 transitives (#585, #586)
ochafik
pushed a commit
that referenced
this pull request
Apr 2, 2026
Changes since 1.3.2: SDK - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) pdf-server - feat: add save_as interact action (#580) - feat: fit-to-page on fullscreen + pinch-to-zoom (#583) - fix: npx DOMMatrix crash + broken MCPB bundle (#584) - fix: viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix: render page before O(numPages) annotation scans (#581) - fix: radio + dropdown in fill_form/save (#577) Dependencies - chore: npm audit fix — sdk 1.29.0, systeminformation 5.31.5, +13 transitives (#585, #586) - fix: bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576)
ochafik
added a commit
that referenced
this pull request
Apr 2, 2026
Changes since 1.3.2: SDK - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) pdf-server - feat: add save_as interact action (#580) - feat: fit-to-page on fullscreen + pinch-to-zoom (#583) - fix: npx DOMMatrix crash + broken MCPB bundle (#584) - fix: viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix: render page before O(numPages) annotation scans (#581) - fix: radio + dropdown in fill_form/save (#577) Dependencies - chore: npm audit fix — sdk 1.29.0, systeminformation 5.31.5, +13 transitives (#585, #586) - fix: bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576)
ochafik
added a commit
that referenced
this pull request
Apr 2, 2026
* chore: bump ext-apps to 1.4.0 Changes since 1.3.2: SDK - feat: add addEventListener/removeEventListener with DOM-model on* semantics (#573) pdf-server - feat: add save_as interact action (#580) - feat: fit-to-page on fullscreen + pinch-to-zoom (#583) - fix: npx DOMMatrix crash + broken MCPB bundle (#584) - fix: viewer liveness, 1:1 batch results, fullscreen jitter (#579) - fix: render page before O(numPages) annotation scans (#581) - fix: radio + dropdown in fill_form/save (#577) Dependencies - chore: npm audit fix — sdk 1.29.0, systeminformation 5.31.5, +13 transitives (#585, #586) - fix: bump path-to-regexp 8.3.0 → 8.4.1 to patch ReDoS CVEs (#576) * chore: update e2e snapshots [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was breaking
45s hangs on
get_screenshot/get_text. When the iframe never reachesstartPolling()— host goes idle before mount finishes —interactwaited the fullGET_PAGES_TIMEOUT_MSfor a viewer that isn't there, then returned a generic timeout with no hint why.Batch errors swallowed. LocalAgentMode SDK collapses
isError:trueresults to a bare string ofcontent[0].text. A[fill_form, get_screenshot]batch where the screenshot timed out returned a 3-item content array, but the model only sawcontent[0]— which after the original ordering fix was the"Batch failed at step 2/2 (get_screenshot):"prefix with nothing after the colon. The actual error was incontent[1], dropped.Fullscreen iframe shrinking to ~85px.
requestFitToContent()reads cachedcurrentDisplayModewhich defaults to"inline"and only updatesif (ctx.displayMode)— if the host omits the field or the update lands one tick late, the function measures a near-empty pageWrapper and sends toolbar-height-only. Workaround was exit + re-enter fullscreen.Fixes
server.tsensureViewerIsPolling()—viewsPolledSet tracks which UUIDs have polled at least once. Called beforewaitForPageData()inget_screenshot/get_text. Throws after 8s with a"Viewer never connected... call display_pdf again"hint if no poll arrives. Stateless polling loop onviewsPolled.has()— doesn't touchpollWaiters(would race the batch-wait branch and concurrentinteractcalls).1:1 batch result contract —
content[i]is the result ofcommands[i]. If a step fails, the batch stops there and that slot's text starts withERROR;content.lengthtells the model how far it got. Multi-step batches don't setisError(the SDK's flatten would eat any prior images); single-command failures still set it since there's nothing beforecontent[0]to lose. Tool description documents this.console.error("[interact] ... failed after Xms")— only on failure. Goes to stderr →mcp-server-*.log, which previously had zero trace of SDK-proxied tool calls.mcp-app.tsPer-def
try/catchinadd_annotations/update_annotations— a throw on one def used to kill the wholeprocessCommandsbatch including anyget_pageslater in the queue, sosubmit_page_datanever fired and the server waited the full 45s.log.errorwhenupdate_annotationsskips a missing id — wasif (!existing) continuewith no trace.requestFitToContentreadsapp.getHostContext().displayModedirectly — and refuses to send whenpageWrapperHeight < toolbarHeight(never a real layout, just an early render).Existing fixes from earlier on this branch (poll-on-load-error,
.catch()onapp.connect()) unchanged.Tests
server.test.ts:get_screenshot fails fast when viewer never polled— verifies <15s + recovery hintget_screenshot waits full timeout when viewer polled then went silent— distinguishes never-mounted from hung-renderget_screenshot succeeds when viewer polls during grace window— late-arriving viewer still worksbatch step failure: 1:1 content, no isError, ERROR-prefixed slot— 3-command batch where step 2 fails:content.length === 2, slot 0 untouched, slot 1 starts withERROR, noisError