Skip to content

Fix pdf-server saved-page restore (dead pageCount guard since #329) - #828

Merged
ochafik merged 2 commits into
mainfrom
pdf-server-restore-saved-page
Oct 27, 2026
Merged

ochafik merged 2 commits into
mainfrom
pdf-server-restore-saved-page

Conversation

@ochafik

@ochafik ochafik commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Summary

The pdf-server viewer saves the current page to localStorage (keyed by viewUUID) and is meant to restore it when a host re-renders the same tool call — reopening a conversation, refreshing the page. In practice annotations came back but the page didn't. Root cause: the restore guard

currentPage = savedPage && savedPage <= parsed.pageCount ? savedPage : parsed.initialPage;

compares against structuredContent.pageCount, a field display_pdf stopped sending in #329 when server-side PDF parsing was removed. savedPage <= undefined is always false, so the saved page has been read and discarded on every load since — dead code for ~9 months.

Changes

  • Restore the saved page unconditionally (loadSavedPage() already validates >= 1) and clamp to totalPages once the document loads — the same clamp reloadPdf() already has; the bounds check simply can't happen before the document is parsed.
  • Resolve viewUUID from _meta with a structuredContent fallback. The server sends it in both places, and some hosts drop _meta when replaying cached tool results — in that case the viewer previously had no storage key at all (so neither page nor, pre-Add get_current_pdf to the pdf-server example (app tool + interact action) #820-era keys aside, anything else keyed on it could restore). This also tracks the broader "dual-channel critical fields" hardening direction.
  • Drop pageCount from the parsed-result type — it was never sent, and the lying type is what hid the bug.

Test

New e2e (tests/e2e/pdf-page-restore.spec.ts) that exercises the real restore path: the basic-host test UI doesn't replay tool results (#642), so the test plays the host — it captures the display_pdf CallToolResult from the result panel, navigates to another page via the view's own navigate tool (which persists it), then re-posts the captured result into the app iframe as a ui/notifications/tool-result notification and asserts the viewer comes back on the saved page, not initialPage. Two variants: with the original _meta, and with _meta stripped (covers the structuredContent fallback).

Validation

  • tsc --noEmit (example tsconfig) — clean
  • bun test examples/pdf-server src — 441 tests, 0 failures
  • Both new e2e tests pass; the pre-existing pdf e2e suites pass apart from unrelated server-boot timing flakes on the (slow) validation machine — they are green on main's CI and this diff doesn't touch boot paths.

Generated by Claude Code

claude and others added 2 commits October 27, 2026 01:04
The viewer saves the current page to localStorage and is meant to
restore it when a host re-renders the same tool call, but the restore
bounds-checked the saved page against structuredContent.pageCount — a
field display_pdf stopped sending in #329 (the server no longer
pre-parses the PDF). The comparison was always false, so the viewer
always fell back to initialPage: annotations survived a re-render,
the page never did.

- Restore the saved page unconditionally and clamp to totalPages once
  the document loads (same clamp reloadPdf already has).
- Resolve viewUUID from _meta with a structuredContent fallback: the
  server sends it in both, and some hosts drop _meta on replayed tool
  results — without the fallback the page had no storage key at all.
- Drop the pageCount field from the parsed-result type (never sent).

The e2e test plays the host: it captures the display_pdf result from
the result panel, navigates via the view's tool, re-posts the result
as a ui/notifications/tool-result notification, and asserts the viewer
comes back on the saved page — with and without _meta.
@pkg-pr-new

pkg-pr-new Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/ext-apps

npm i https://pkg.pr.new/@modelcontextprotocol/ext-apps@828

@modelcontextprotocol/server-basic-preact

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-preact@828

@modelcontextprotocol/server-basic-react

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-react@828

@modelcontextprotocol/server-basic-solid

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-solid@828

@modelcontextprotocol/server-basic-svelte

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-svelte@828

@modelcontextprotocol/server-basic-vanillajs

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-vanillajs@828

@modelcontextprotocol/server-basic-vue

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-vue@828

@modelcontextprotocol/server-budget-allocator

npm i https://pkg.pr.new/@modelcontextprotocol/server-budget-allocator@828

@modelcontextprotocol/server-cohort-heatmap

npm i https://pkg.pr.new/@modelcontextprotocol/server-cohort-heatmap@828

@modelcontextprotocol/server-customer-segmentation

npm i https://pkg.pr.new/@modelcontextprotocol/server-customer-segmentation@828

@modelcontextprotocol/server-debug

npm i https://pkg.pr.new/@modelcontextprotocol/server-debug@828

@modelcontextprotocol/server-lazy-auth

npm i https://pkg.pr.new/@modelcontextprotocol/server-lazy-auth@828

@modelcontextprotocol/server-map

npm i https://pkg.pr.new/@modelcontextprotocol/server-map@828

@modelcontextprotocol/server-pdf

npm i https://pkg.pr.new/@modelcontextprotocol/server-pdf@828

@modelcontextprotocol/server-scenario-modeler

npm i https://pkg.pr.new/@modelcontextprotocol/server-scenario-modeler@828

@modelcontextprotocol/server-shadertoy

npm i https://pkg.pr.new/@modelcontextprotocol/server-shadertoy@828

@modelcontextprotocol/server-sheet-music

npm i https://pkg.pr.new/@modelcontextprotocol/server-sheet-music@828

@modelcontextprotocol/server-system-monitor

npm i https://pkg.pr.new/@modelcontextprotocol/server-system-monitor@828

@modelcontextprotocol/server-threejs

npm i https://pkg.pr.new/@modelcontextprotocol/server-threejs@828

@modelcontextprotocol/server-transcript

npm i https://pkg.pr.new/@modelcontextprotocol/server-transcript@828

@modelcontextprotocol/server-video-resource

npm i https://pkg.pr.new/@modelcontextprotocol/server-video-resource@828

@modelcontextprotocol/server-wiki-explorer

npm i https://pkg.pr.new/@modelcontextprotocol/server-wiki-explorer@828

commit: e880dfe

@ochafik
ochafik merged commit 517512c into main Oct 27, 2026
20 checks passed
@ochafik
ochafik deleted the pdf-server-restore-saved-page branch October 27, 2026 04:15
@ochafik ochafik mentioned this pull request Oct 27, 2026
ochafik added a commit that referenced this pull request Oct 27, 2026
Patch release: the pdf-server example defaults to a single interact app
tool with the per-action tools behind --split-app-tools (#830), the
map-server example gains a get-screenshot app tool (#821), the
pdf-server viewer restores the saved page on tool-result re-delivery
(#828), and the dependency refresh clears the open npm audit findings
(solid-js 1.9.17, sharp ^0.35.5 and in-range transitive updates, #827).

Co-authored-by: Claude <noreply@anthropic.com>
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