diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index db6c56ba..52075069 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -1054,4 +1054,142 @@ describe("interact tool", () => { await server.close(); }); }); + + describe("viewer liveness", () => { + // get_screenshot/get_text fail fast when the iframe never polled, instead + // of waiting 45s for a viewer that isn't there. Reproduces the case where + // the host goes idle before the iframe reaches startPolling(). + + it("get_screenshot fails fast when viewer never polled", async () => { + const { server, client } = await connect(); + const uuid = "never-polled-screenshot"; + + const started = Date.now(); + const r = await client.callTool({ + name: "interact", + arguments: { viewUUID: uuid, action: "get_screenshot", page: 1 }, + }); + const elapsed = Date.now() - started; + + expect(r.isError).toBe(true); + expect(firstText(r)).toContain("never connected"); + expect(firstText(r)).toContain(uuid); + expect(firstText(r)).toContain("display_pdf again"); // recovery hint + // Fast-fail bound (~8s grace), well under the 45s page-data timeout. + // 15s upper bound leaves slack for CI scheduling. + expect(elapsed).toBeLessThan(15_000); + + await client.close(); + await server.close(); + }, 20_000); + + it("get_screenshot waits full timeout when viewer polled then went silent", async () => { + // Viewer polled once (proving it mounted) then hung on a heavy render. + // The grace check passes, so we fall through to the 45s page-data wait — + // verified here by racing against a 12s deadline that should NOT win. + const { server, client } = await connect(); + const uuid = "polled-then-silent"; + + // Viewer's first poll: drain whatever's there so it returns fast. + // Enqueue a trivial command first so poll returns via the batch-wait + // path (~200ms) instead of blocking on the 30s long-poll. + await client.callTool({ + name: "interact", + arguments: { viewUUID: uuid, action: "navigate", page: 1 }, + }); + await poll(client, uuid); + + // Now get_screenshot — viewer has polled, so no fast-fail. But viewer + // never calls submit_page_data → should wait beyond the grace period. + const outcome = await Promise.race([ + client + .callTool({ + name: "interact", + arguments: { viewUUID: uuid, action: "get_screenshot", page: 1 }, + }) + .then(() => "completed" as const), + new Promise<"still-waiting">((r) => + setTimeout(() => r("still-waiting"), 12_000), + ), + ]); + + expect(outcome).toBe("still-waiting"); + + await client.close(); + await server.close(); + }, 20_000); + + it("get_screenshot succeeds when viewer polls during grace window", async () => { + // Model calls interact before the viewer has polled — but the viewer + // shows up within the grace period and completes the roundtrip. + const { server, client } = await connect(); + const uuid = "late-arriving-viewer"; + + const interactPromise = client.callTool({ + name: "interact", + arguments: { viewUUID: uuid, action: "get_screenshot", page: 1 }, + }); + + // Viewer connects 500ms late — well inside the grace window. + await new Promise((r) => setTimeout(r, 500)); + const cmds = await poll(client, uuid); + const getPages = cmds.find((c) => c.type === "get_pages"); + expect(getPages).toBeDefined(); + + // Viewer responds with the page data. + await client.callTool({ + name: "submit_page_data", + arguments: { + requestId: getPages!.requestId as string, + pages: [ + { page: 1, image: Buffer.from("fake-jpeg").toString("base64") }, + ], + }, + }); + + const r = await interactPromise; + expect(r.isError).toBeFalsy(); + expect((r.content as Array<{ type: string }>)[0].type).toBe("image"); + + await client.close(); + await server.close(); + }, 15_000); + + it("batch failure puts error message first", async () => { + // When [fill_form, get_screenshot] runs and get_screenshot times out, + // content[0] must describe the failure — not the earlier success. Some + // hosts flatten isError results to content[0].text only, which previously + // showed "Queued: Filled N fields" with isError:true and dropped the + // actual timeout entirely. + const { server, client } = await connect(); + const uuid = "batch-error-ordering"; + + const r = await client.callTool({ + name: "interact", + arguments: { + viewUUID: uuid, + commands: [ + { action: "navigate", page: 3 }, // succeeds → "Queued: ..." + { action: "get_screenshot", page: 1 }, // never-polled → fast-fail + ], + }, + }); + + expect(r.isError).toBe(true); + const texts = (r.content as Array<{ type: string; text: string }>).map( + (c) => c.text, + ); + // content[0]: batch-failed summary naming the culprit + expect(texts[0]).toContain("failed"); + expect(texts[0]).toContain("2/2"); + expect(texts[0]).toContain("get_screenshot"); + // content[1]: the actual error + expect(texts[1]).toContain("never connected"); + // content[2]: the earlier success, pushed to the back + expect(texts[2]).toContain("Queued"); + + await client.close(); + await server.close(); + }, 15_000); + }); }); diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 5bc2502f..b216d49c 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -196,6 +196,14 @@ export type { PdfCommand }; // reject first and return a real error instead of the client cancelling us. const GET_PAGES_TIMEOUT_MS = 45_000; +/** + * Grace period for the viewer's first poll. If interact() arrives before the + * iframe has ever polled, we wait this long for it to show up (iframe mount + + * PDF load + startPolling). If no poll comes, the viewer almost certainly + * never rendered — failing fast beats a silent 45s hang. + */ +const VIEWER_FIRST_POLL_GRACE_MS = 8_000; + interface PageDataEntry { page: number; text?: string; @@ -232,6 +240,33 @@ function waitForPageData( }); } +/** + * Wait for the viewer's first poll_pdf_commands call. + * + * Called before waitForPageData() so a viewer that never mounted fails in ~8s + * with a specific message instead of a generic 45s "Timeout waiting for page + * data" that gives no hint why. + * + * Intentionally does NOT touch pollWaiters: piggybacking on that single-slot + * Map races with poll_pdf_commands' batch-wait branch (which never cancels the + * prior waiter) and with concurrent interact calls (which would overwrite each + * other). A plain check loop on viewsPolled is stateless — multiple callers + * can wait independently and all observe the same add() when it happens. + */ +async function ensureViewerIsPolling(uuid: string): Promise { + const deadline = Date.now() + VIEWER_FIRST_POLL_GRACE_MS; + while (!viewsPolled.has(uuid)) { + if (Date.now() >= deadline) { + throw new Error( + `Viewer never connected for viewUUID ${uuid} (no poll within ${VIEWER_FIRST_POLL_GRACE_MS / 1000}s). ` + + `The iframe likely failed to mount — this happens when the conversation ` + + `goes idle before the viewer finishes loading. Call display_pdf again to get a fresh viewUUID.`, + ); + } + await new Promise((r) => setTimeout(r, 100)); + } +} + interface QueueEntry { commands: PdfCommand[]; /** Timestamp of the most recent enqueue or dequeue */ @@ -243,6 +278,15 @@ const commandQueues = new Map(); /** Waiters for long-poll: resolve callback wakes up a blocked poll_pdf_commands */ const pollWaiters = new Map void>(); +/** + * viewUUIDs that have been polled at least once. A view missing from this set + * means the iframe never reached startPolling() — usually because it wasn't + * mounted yet, or ontoolresult threw before the poll loop started. Used to + * fail fast in get_screenshot/get_text instead of waiting the full 45s for + * a viewer that was never there. + */ +const viewsPolled = new Set(); + /** Valid form field names per viewer UUID (populated during display_pdf) */ const viewFieldNames = new Map>(); @@ -288,6 +332,7 @@ function pruneStaleQueues(): void { commandQueues.delete(uuid); viewFieldNames.delete(uuid); viewFieldInfo.delete(uuid); + viewsPolled.delete(uuid); stopFileWatch(uuid); } } @@ -812,6 +857,10 @@ interface FormFieldInfo { y: number; width: number; height: number; + /** Radio button export value (buttonValue) — distinguishes widgets that share a field name. */ + exportValue?: string; + /** Dropdown/listbox option values, as seen in the widget's `options` array. */ + options?: string[]; } /** @@ -864,6 +913,16 @@ async function extractFormFieldInfo( // Convert to model coords (top-left origin): modelY = pageHeight - pdfY - height const modelY = pageHeight - y2; + // Choice widgets (combo/listbox) carry `options` as + // [{exportValue, displayValue}]. Expose export values — that's + // what fill_form needs. + let options: string[] | undefined; + if (Array.isArray(ann.options) && ann.options.length > 0) { + options = ann.options + .map((o: { exportValue?: string }) => o?.exportValue) + .filter((v: unknown): v is string => typeof v === "string"); + } + fields.push({ name: fieldName, type: fieldType, @@ -873,6 +932,12 @@ async function extractFormFieldInfo( width: Math.round(width), height: Math.round(height), ...(ann.alternativeText ? { label: ann.alternativeText } : undefined), + // Radio: buttonValue is the per-widget export value — the only + // thing distinguishing three `size [Btn]` lines from each other. + ...(ann.radioButton && ann.buttonValue != null + ? { exportValue: String(ann.buttonValue) } + : undefined), + ...(options?.length ? { options } : undefined), }); } } @@ -1282,6 +1347,14 @@ Set \`elicit_form_inputs\` to true to prompt the user to fill form fields before y: z.number(), width: z.number(), height: z.number(), + exportValue: z + .string() + .optional() + .describe("Radio button value — pass this to fill_form"), + options: z + .array(z.string()) + .optional() + .describe("Dropdown/listbox option values"), }), ) .optional() @@ -1453,8 +1526,14 @@ URL: ${normalized}`, for (const f of fields) { const label = f.label ? ` "${f.label}"` : ""; const nameStr = f.name || "(unnamed)"; + // Radio: = tells the model what value to pass. + // Dropdown: options:[...] lists valid choices. + const exportSuffix = f.exportValue ? `=${f.exportValue}` : ""; + const optsSuffix = f.options + ? ` options:[${f.options.join(", ")}]` + : ""; lines.push( - ` ${nameStr}${label} [${f.type}] at (${f.x},${f.y}) ${f.width}×${f.height}`, + ` ${nameStr}${exportSuffix}${label} [${f.type}] at (${f.x},${f.y}) ${f.width}×${f.height}${optsSuffix}`, ); } } @@ -1925,6 +2004,7 @@ URL: ${normalized}`, let pageData: PageDataEntry[]; try { + await ensureViewerIsPolling(uuid); pageData = await waitForPageData(requestId, signal); } catch (err) { return { @@ -1973,6 +2053,7 @@ URL: ${normalized}`, let pageData: PageDataEntry[]; try { + await ensureViewerIsPolling(uuid); pageData = await waitForPageData(requestId, signal); } catch (err) { return { @@ -2202,7 +2283,7 @@ Example — add a signature image and a stamp, then screenshot to verify: // Process commands sequentially, collecting all content parts const allContent: ContentPart[] = []; - let hasError = false; + let failedAt = -1; for (let i = 0; i < commandList.length; i++) { const result = await processInteractCommand( @@ -2211,15 +2292,27 @@ Example — add a signature image and a stamp, then screenshot to verify: extra.signal, ); if (result.isError) { - hasError = true; + // Error content first. Some hosts flatten isError results to + // content[0].text — if we push the error after prior successes, + // the user sees "Queued: Filled 7 fields" with isError:true and + // the actual failure is silently dropped. + allContent.unshift(...result.content); + failedAt = i; + break; } allContent.push(...result.content); - if (hasError) break; // Stop on first error + } + + if (failedAt >= 0 && commandList.length > 1) { + allContent.unshift({ + type: "text", + text: `Batch failed at step ${failedAt + 1}/${commandList.length} (${commandList[failedAt].action}):`, + }); } return { content: allContent, - ...(hasError ? { isError: true } : {}), + ...(failedAt >= 0 ? { isError: true } : {}), }; }, ); @@ -2280,6 +2373,7 @@ Example — add a signature image and a stamp, then screenshot to verify: _meta: { ui: { visibility: ["app"] } }, }, async ({ viewUUID: uuid }): Promise => { + viewsPolled.add(uuid); // If commands are already queued, wait briefly to let more accumulate if (commandQueues.has(uuid)) { await new Promise((r) => setTimeout(r, POLL_BATCH_WAIT_MS)); diff --git a/examples/pdf-server/src/mcp-app.ts b/examples/pdf-server/src/mcp-app.ts index 6a59407c..b20d74b1 100644 --- a/examples/pdf-server/src/mcp-app.ts +++ b/examples/pdf-server/src/mcp-app.ts @@ -2708,6 +2708,60 @@ async function buildFieldNameMap( log.info(`Built field name map: ${fieldNameToIds.size} fields`); } +/** + * Set one form field's value in pdf.js's annotationStorage, in the format + * AnnotationLayer expects to READ when it re-renders. + * + * Radio buttons need per-widget booleans: pdf.js's RadioButtonWidgetAnnotation + * render() has inverted string coercion (`value !== buttonValue` → true for + * every NON-matching widget), so a string value on all widgets checks the + * first rendered one and clears the rest regardless of what you asked for. + * Match pdf.js's own change handler instead: `{value: true}` on the widget + * whose buttonValue matches, `{value: false}` on the siblings. + * + * Also patches the live DOM element for the current page so the user sees the + * change without waiting for a full re-render. + */ +function setFieldInStorage(name: string, value: string | boolean): void { + if (!pdfDocument) return; + const ids = fieldNameToIds.get(name); + if (!ids) return; + const storage = pdfDocument.annotationStorage; + + // Radio group: at least one widget ID has a buttonValue recorded. + const isRadio = ids.some((id) => radioButtonValues.has(id)); + if (isRadio) { + const want = String(value); + for (const id of ids) { + const checked = radioButtonValues.get(id) === want; + storage.setValue(id, { value: checked }); + const el = formLayerEl.querySelector( + `input[data-element-id="${id}"]`, + ) as HTMLInputElement | null; + if (el) el.checked = checked; + } + return; + } + + // Text / checkbox / select: same value on every widget (a field can have + // multiple widget annotations sharing one /V). + const storageValue = typeof value === "boolean" ? value : String(value); + for (const id of ids) { + storage.setValue(id, { value: storageValue }); + const el = formLayerEl.querySelector(`[data-element-id="${id}"]`) as + | HTMLInputElement + | HTMLSelectElement + | HTMLTextAreaElement + | null; + if (!el) continue; + if (el instanceof HTMLInputElement && el.type === "checkbox") { + el.checked = !!value; + } else { + el.value = String(value); + } + } +} + /** Sync formFieldValues into pdfDocument.annotationStorage so AnnotationLayer renders pre-filled values. * Skips values that match the PDF's baseline — those are already in storage * in pdf.js's native format (which may differ from our string/bool repr, @@ -2715,17 +2769,9 @@ async function buildFieldNameMap( * form can break the Reset button's ability to restore defaults. */ function syncFormValuesToStorage(): void { if (!pdfDocument || fieldNameToIds.size === 0) return; - const storage = pdfDocument.annotationStorage; for (const [name, value] of formFieldValues) { if (pdfBaselineFormValues.get(name) === value) continue; - const ids = fieldNameToIds.get(name); - if (ids) { - for (const id of ids) { - storage.setValue(id, { - value: typeof value === "boolean" ? value : String(value), - }); - } - } + setFieldInStorage(name, value); } } @@ -4093,6 +4139,15 @@ app.ontoolresult = async (result: CallToolResult) => { } catch (err) { log.error("Error loading PDF:", err); showError(err instanceof Error ? err.message : String(err)); + // Poll anyway. The server's interact tool has no way to know we choked — + // without a poll it waits 45s on every get_screenshot against this + // viewUUID. handleGetPages already null-guards pdfDocument, so a failed + // load just means empty page data → server returns "No screenshot + // returned" (fast, actionable) instead of "Timeout waiting for page data + // from viewer" (slow, opaque). + if (viewUUID && interactEnabled) { + startPolling(); + } } }; @@ -4213,44 +4268,10 @@ async function processCommands(commands: PdfCommand[]): Promise { case "fill_form": for (const field of cmd.fields) { formFieldValues.set(field.name, field.value); - // Set in PDF.js annotation storage and update DOM elements directly - if (pdfDocument) { - const ids = fieldNameToIds.get(field.name); - if (ids) { - for (const id of ids) { - pdfDocument.annotationStorage.setValue(id, { - value: - typeof field.value === "boolean" - ? field.value - : String(field.value), - }); - // Update the live DOM element if it exists on the current page - const el = formLayerEl.querySelector( - `[data-element-id="${id}"]`, - ) as - | HTMLInputElement - | HTMLSelectElement - | HTMLTextAreaElement - | null; - if (el) { - if ( - el instanceof HTMLInputElement && - el.type === "checkbox" - ) { - el.checked = !!field.value; - } else if (el instanceof HTMLSelectElement) { - el.value = String(field.value); - } else { - el.value = String(field.value); - } - } - } - } else { - log.info( - `fill_form: no annotation IDs for field "${field.name}"`, - ); - } + if (!fieldNameToIds.has(field.name)) { + log.info(`fill_form: no annotation IDs for field "${field.name}"`); } + setFieldInStorage(field.name, field.value); } // Re-render to show updated form values (handles fields on other pages) renderPage(); @@ -4440,16 +4461,27 @@ app.onteardown = async () => { app.onhostcontextchanged = handleHostContextChanged; // Connect to host -app.connect().then(() => { - log.info("Connected to host"); - const ctx = app.getHostContext(); - if (ctx) { - handleHostContextChanged(ctx); - } - // Restore annotations early using toolInfo.id (available before tool result) - restoreAnnotations(); - updateAnnotationsBadge(); -}); +app + .connect() + .then(() => { + log.info("Connected to host"); + const ctx = app.getHostContext(); + if (ctx) { + handleHostContextChanged(ctx); + } + // Restore annotations early using toolInfo.id (available before tool result) + restoreAnnotations(); + updateAnnotationsBadge(); + }) + .catch((err: unknown) => { + // ui/initialize failed or transport rejected. Without a catch this is an + // unhandled rejection — iframe shows blank, server times out on every + // interact call with no clue why. + log.error("Failed to connect to host:", err); + showError( + `Failed to connect to host: ${err instanceof Error ? err.message : String(err)}`, + ); + }); // Debug helper: dump all annotation state. Run in DevTools console as // `__pdfDebug()` to diagnose ghost annotations (visible on canvas but not diff --git a/examples/pdf-server/src/pdf-annotations.test.ts b/examples/pdf-server/src/pdf-annotations.test.ts index c451cc19..fe34d835 100644 --- a/examples/pdf-server/src/pdf-annotations.test.ts +++ b/examples/pdf-server/src/pdf-annotations.test.ts @@ -1031,6 +1031,81 @@ describe("buildAnnotatedPdfBytes", () => { const bytes2 = await doc.save(); expect(bytes2.length).toBeGreaterThan(0); }); + + describe("form field persistence", () => { + // One fixture with every field type we support. pdf-lib's addOptionToPage + // writes radio buttonValues as numeric index strings ("0","1","2"), which + // is the stress case — the viewer stores those, but .select() wants labels. + let formPdfBytes: Uint8Array; + beforeAll(async () => { + const doc = await PDFDocument.create(); + const page = doc.addPage([612, 792]); + const form = doc.getForm(); + form.createTextField("name").addToPage(page, { x: 10, y: 700 }); + form.createCheckBox("agree").addToPage(page, { x: 10, y: 660 }); + const dd = form.createDropdown("country"); + dd.addOptions(["USA", "UK", "Canada"]); + dd.addToPage(page, { x: 10, y: 620 }); + const rg = form.createRadioGroup("size"); + rg.addOptionToPage("small", page, { x: 10, y: 580 }); + rg.addOptionToPage("medium", page, { x: 50, y: 580 }); + rg.addOptionToPage("large", page, { x: 90, y: 580 }); + formPdfBytes = await doc.save(); + }); + + it("writes text, checkbox, dropdown, and radio (by label) in one pass", async () => { + const out = await buildAnnotatedPdfBytes( + formPdfBytes, + [], + new Map([ + ["name", "Alice"], + ["agree", true], + ["country", "Canada"], + ["size", "medium"], + ]), + ); + const form = (await PDFDocument.load(out)).getForm(); + expect(form.getTextField("name").getText()).toBe("Alice"); + expect(form.getCheckBox("agree").isChecked()).toBe(true); + expect(form.getDropdown("country").getSelected()).toEqual(["Canada"]); + expect(form.getRadioGroup("size").getSelected()).toBe("medium"); + }); + + it("maps numeric radio buttonValue to option label by index", async () => { + // The viewer stores what pdf.js reports as buttonValue ("2"), not the + // label. Save must translate or the radio is silently dropped. + const out = await buildAnnotatedPdfBytes( + formPdfBytes, + [], + new Map([["size", "2"]]), + ); + const form = (await PDFDocument.load(out)).getForm(); + expect(form.getRadioGroup("size").getSelected()).toBe("large"); + }); + + it("leaves radio unset when value is neither label nor valid index", async () => { + const out = await buildAnnotatedPdfBytes( + formPdfBytes, + [], + new Map([["size", "bogus"]]), + ); + const form = (await PDFDocument.load(out)).getForm(); + expect(form.getRadioGroup("size").getSelected()).toBeUndefined(); + }); + + it("skips unknown field names without throwing", async () => { + const out = await buildAnnotatedPdfBytes( + formPdfBytes, + [], + new Map([ + ["nonexistent", "x"], + ["name", "kept"], + ]), + ); + const form = (await PDFDocument.load(out)).getForm(); + expect(form.getTextField("name").getText()).toBe("kept"); + }); + }); }); // ============================================================================= diff --git a/examples/pdf-server/src/pdf-annotations.ts b/examples/pdf-server/src/pdf-annotations.ts index 0628d825..25d9ebb6 100644 --- a/examples/pdf-server/src/pdf-annotations.ts +++ b/examples/pdf-server/src/pdf-annotations.ts @@ -18,6 +18,10 @@ import { PDFString, PDFHexString, StandardFonts, + PDFTextField, + PDFCheckBox, + PDFDropdown, + PDFRadioGroup, } from "pdf-lib"; // ============================================================================= @@ -810,26 +814,45 @@ export async function buildAnnotatedPdfBytes( // Add proper PDF annotation objects await addAnnotationDicts(pdfDoc, annotations); - // Apply form fills + // Apply form fills. Dispatch on actual field type — getTextField(name) throws + // for dropdowns/radios, so the old try/catch silently dropped those on save. if (formFields.size > 0) { try { const form = pdfDoc.getForm(); for (const [name, value] of formFields) { - try { - if (typeof value === "boolean") { - const checkbox = form.getCheckBox(name); - if (value) checkbox.check(); - else checkbox.uncheck(); + const field = form.getFieldMaybe(name); + if (!field) continue; + + if (field instanceof PDFCheckBox) { + if (value) field.check(); + else field.uncheck(); + } else if (field instanceof PDFRadioGroup) { + // The viewer stores pdf.js's buttonValue, which for PDFs with an + // /Opt array is a numeric index ("0","1","2") rather than the + // option label pdf-lib's select() expects. Try the label first, + // then fall back to indexing into getOptions(). + const opts = field.getOptions(); + const s = String(value); + if (opts.includes(s)) { + field.select(s); } else { - const textField = form.getTextField(name); - textField.setText(value); + const idx = Number(s); + if (Number.isInteger(idx) && idx >= 0 && idx < opts.length) { + field.select(opts[idx]); + } + // else: value is neither label nor index — leave unset } - } catch { - // Field not found or wrong type — skip + } else if (field instanceof PDFDropdown) { + // select() auto-enables edit mode for values outside getOptions(), + // so this works for both enumerated and free-text combos. + field.select(String(value)); + } else if (field instanceof PDFTextField) { + field.setText(String(value)); } + // PDFButton, PDFOptionList, PDFSignature: no fill_form support yet } } catch { - // Form not available — skip + // pdfDoc.getForm() throws if the PDF has no AcroForm } }