fix(pdf-server): form-field save robustness (#591)

* fix(pdf-server): one bad form field no longer aborts saving the rest

#577 dropped the per-field try/catch in buildAnnotatedPdfBytes when it
swapped to type-dispatch, so the first field whose pdf-lib write throws
(max-length text, missing /Yes appearance, radio buttonValue mapping to
neither label nor index, ...) bubbled to the outer catch and silently
dropped every subsequent field. Compounded by getAnnotatedPdfBytes
passing every baseline field, so even an untouched problematic field in
the PDF poisoned the whole save.

- Re-wrap each field write in its own try/catch (warn + continue).
- getAnnotatedPdfBytes now only sends fields whose value differs from
  baseline (still includes explicit ''/false sentinels for cleared
  fields so pdf-lib overwrites the original /V).
- Regression test: a maxLength=2 text field fed a long string, followed
  by a normal field; assert the second one still lands.

* fix(pdf-server): radio groups misclassified as PDFCheckBox save the chosen widget

Some PDFs (e.g. third-party forms like the IRS f1040 family, and the
demo Form.pdf) omit the /Ff Radio flag bit on button fields, so pdf-lib
classifies a multi-widget radio as PDFCheckBox. The viewer stores
pdf.js's buttonValue (the widget's on-state name, e.g. '0'/'1') as a
string. The PDFCheckBox branch did 'if (value) field.check()', which
always sets the FIRST widget's on-state - so any choice saved as the
first option.

When the value is a string on a PDFCheckBox, treat it as a radio
on-value: write /V and per-widget /AS directly via the low-level
acroField (mirroring PDFAcroRadioButton.setValue minus its
first-widget-only onValues guard). Booleans keep check()/uncheck().

Test: build a radio fixture, clear the Radio flag so reload sees
PDFCheckBox, save with '1', assert /V = /1 and second widget /AS = /1.

* fix(pdf-server): clearing a radio from the panel unchecks all widgets

clearFieldInStorage wrote the same string clearValue to every widget's
storage, hitting pdf.js's inverted string-coercion bug (the one
setFieldInStorage already works around): the wrong widget rendered
checked. For radio, write {value:false} to every widget instead.

* fix(pdf-server): only treat multi-widget PDFCheckBox as misclassified radio

A single-widget checkbox getting a string value (e.g. 'Yes'/'Off' if
something upstream stores the export string instead of boolean) was
being routed through setButtonGroupValue, which no-ops when no widget's
on-state matches the value - leaving the box unchanged. Gate the
radio-path on widgets.length > 1; single-widget falls through to
check()/uncheck() with liberal truthy/'Off' semantics.

* fix(pdf-server): generate missing field appearances on save

PDFs whose checkbox/radio widgets lack an /AP/N/<onValue> stream: check()
sets /V and /AS but Preview/Acrobat can't render a state with no
appearance, so the field looks unchanged. Call form.updateFieldAppearances()
after the write loop so pdf-lib synthesizes defaults for dirty fields.

* feat(pdf-server): save PDFOptionList (multiselect listbox) values

Language field in Form.pdf is a PDFOptionList; the save loop skipped it.
Parse the viewer's comma-joined string, select() the subset that exists
in getOptions() (pdf-lib throws on unknowns and there's no edit-mode
fallback like Dropdown has).

* fix(pdf-server): capture all selected options on <select multiple>

target.value on a multiselect is only the first option; join
selectedOptions so the save path gets the full set.

Author: ochafik

examples/pdf-server/src/annotation-panel.ts

@@ -831,8 +831,15 @@ function clearFieldInStorage(name: string): void {
     meta?.[0]?.defaultValue ??
     "";
   const type = meta?.find((f) => f.type)?.type;
-  const clearValue =
-    type === "checkbox" || type === "radiobutton" ? (dv ?? "Off") : (dv ?? "");
+  // Radio: per-widget BOOLEANS, never a string. pdf.js's
+  // RadioButtonWidgetAnnotation render() has inverted string coercion (see
+  // setFieldInStorage), so writing the same string to every widget checks
+  // the wrong one. {value:false} on all = nothing selected.
+  if (type === "radiobutton") {
+    for (const id of ids) storage.setValue(id, { value: false });
+    return;
+  }
+  const clearValue = type === "checkbox" ? (dv ?? "Off") : (dv ?? "");
   for (const id of ids) storage.setValue(id, { value: clearValue });
 }
 

examples/pdf-server/src/mcp-app.ts

@@ -2971,15 +2971,23 @@ async function getAnnotatedPdfBytes(): Promise<Uint8Array> {
     }
   }
 
-  // buildAnnotatedPdfBytes gates on formFields.size > 0 and only writes
-  // entries present in the map. After clearAllItems() the map is empty →
-  // zero setText/uncheck calls → pdf-lib leaves original /V intact →
-  // the "stripped PDF" we promised keeps all its form data. To actually
-  // clear, send an explicit sentinel for every baseline field the user
-  // dropped: "" for text, false for checkbox (matching baseline type).
-  const formFieldsOut = new Map(formFieldValues);
+  // Only write fields that actually changed vs. what's already in the PDF.
+  // Unchanged fields are no-ops at best, and at worst trip pdf-lib edge
+  // cases (max-length text, missing /Yes appearance, …) on fields the user
+  // never touched — which, before the per-field catch in
+  // buildAnnotatedPdfBytes, aborted every subsequent field.
+  //
+  // Fields the user cleared (present in baseline, absent from formFieldValues
+  // after clearAllItems()) still need an explicit "" / false so pdf-lib
+  // overwrites the original /V instead of leaving it intact.
+  const formFieldsOut = new Map<string, string | boolean>();
+  for (const [name, value] of formFieldValues) {
+    if (pdfBaselineFormValues.get(name) !== value) {
+      formFieldsOut.set(name, value);
+    }
+  }
   for (const [name, baselineValue] of pdfBaselineFormValues) {
-    if (!formFieldsOut.has(name)) {
+    if (!formFieldValues.has(name)) {
       formFieldsOut.set(name, typeof baselineValue === "boolean" ? false : "");
     }
   }
@@ -3499,6 +3507,10 @@ formLayerEl.addEventListener("input", (e) => {
     if (!target.checked) return; // unchecking siblings — ignore
     const wid = target.getAttribute("data-element-id");
     value = (wid && radioButtonValues.get(wid)) ?? target.value;
+  } else if (target instanceof HTMLSelectElement && target.multiple) {
+    // .value on a <select multiple> is only the first option; join them all
+    // so save can select() the full set on a PDFOptionList.
+    value = Array.from(target.selectedOptions, (o) => o.value).join(",");
   } else {
     value = target.value;
   }

examples/pdf-server/src/pdf-annotations.test.ts

@@ -1152,6 +1152,76 @@ describe("buildAnnotatedPdfBytes", () => {
       const form = (await PDFDocument.load(out)).getForm();
       expect(form.getTextField("name").getText()).toBe("kept");
     });
+
+    it("a field that throws on write does not abort subsequent fields", async () => {
+      // Regression for #577: the per-field try/catch was dropped, so the
+      // first throwing field bubbled to the outer catch and silently dropped
+      // every field after it. setText() throws when value exceeds maxLength.
+      const doc = await PDFDocument.create();
+      const page = doc.addPage([612, 792]);
+      const form = doc.getForm();
+      const limited = form.createTextField("limited");
+      limited.setMaxLength(2);
+      limited.addToPage(page, { x: 10, y: 700 });
+      form.createTextField("after").addToPage(page, { x: 10, y: 660 });
+      const fixture = await doc.save();
+
+      const out = await buildAnnotatedPdfBytes(
+        fixture,
+        [],
+        new Map<string, string | boolean>([
+          ["limited", "way too long"], // throws
+          ["after", "kept"],
+        ]),
+      );
+
+      const saved = (await PDFDocument.load(out)).getForm();
+      expect(saved.getTextField("after").getText()).toBe("kept");
+      // The throwing field is left at whatever pdf-lib could do with it —
+      // we only assert it didn't poison "after".
+    });
+
+    it("radio misclassified as PDFCheckBox: string value selects the matching widget", async () => {
+      // Some PDFs (e.g. IRS/third-party forms) omit the /Ff Radio bit, so
+      // pdf-lib hands us a PDFCheckBox. The viewer stored pdf.js's
+      // buttonValue ("0"/"1"), not a boolean — check() would always pick
+      // the first widget. setButtonGroupValue writes /V + per-widget /AS
+      // directly so the chosen widget sticks.
+      const doc = await PDFDocument.create();
+      const page = doc.addPage([612, 792]);
+      const form = doc.getForm();
+      const rg = form.createRadioGroup("Gender");
+      rg.addOptionToPage("Male", page, { x: 10, y: 700 });
+      rg.addOptionToPage("Female", page, { x: 60, y: 700 });
+      // pdf-lib's addOptionToPage writes widget on-values "0","1". Clear the
+      // Radio flag (bit 16) so the reloaded form classifies it as checkbox.
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (rg.acroField as any).clearFlag(1 << 15);
+      const fixture = await doc.save();
+
+      // Sanity: reload sees it as PDFCheckBox now.
+      const reForm = (await PDFDocument.load(fixture)).getForm();
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const reField = reForm.getFieldMaybe("Gender") as any;
+      expect(reField?.constructor?.name).toBe("PDFCheckBox");
+
+      const out = await buildAnnotatedPdfBytes(
+        fixture,
+        [],
+        new Map<string, string | boolean>([["Gender", "1"]]),
+      );
+
+      const saved = await PDFDocument.load(out);
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const acro = (saved.getForm().getFieldMaybe("Gender") as any).acroField;
+      const v = acro.dict.get(PDFName.of("V"));
+      expect(v).toBeInstanceOf(PDFName);
+      expect((v as PDFName).decodeText()).toBe("1");
+      // Second widget /AS is the on-state, first is /Off.
+      const widgets = acro.getWidgets();
+      expect(widgets[0].getAppearanceState()?.decodeText()).toBe("Off");
+      expect(widgets[1].getAppearanceState()?.decodeText()).toBe("1");
+    });
   });
 });
 

examples/pdf-server/src/pdf-annotations.ts

@@ -21,7 +21,9 @@ import {
   PDFTextField,
   PDFCheckBox,
   PDFDropdown,
+  PDFOptionList,
   PDFRadioGroup,
+  type PDFForm,
 } from "pdf-lib";
 
 // =============================================================================
@@ -800,6 +802,40 @@ export async function addAnnotationDicts(
   }
 }
 
+/**
+ * Select a radio-style button group by widget on-value, bypassing pdf-lib's
+ * type-level guards. Used when pdf-lib classifies a radio as `PDFCheckBox`
+ * (PDF lacks the /Ff Radio bit) — `check()` would always pick the first
+ * widget. Mirrors `PDFAcroRadioButton.setValue` minus its `onValues` throw.
+ */
+function setButtonGroupValue(
+  field: PDFCheckBox | PDFRadioGroup,
+  onValue: string,
+): void {
+  const acro = field.acroField;
+  const off = PDFName.of("Off");
+  const widgets = acro.getWidgets();
+  // Match by PDFName identity (pdf-lib interns names) — the viewer stored
+  // pdf.js's buttonValue, which IS the widget's /AP /N on-state name.
+  let target = onValue && onValue !== "Off" ? PDFName.of(onValue) : off;
+  if (
+    target !== off &&
+    !widgets.some((w: { getOnValue(): PDFName | undefined }) => {
+      return w.getOnValue() === target;
+    })
+  ) {
+    // No widget has this on-state — leave as-is rather than corrupt /V.
+    return;
+  }
+  acro.dict.set(PDFName.of("V"), target);
+  for (const w of widgets) {
+    const on = (w as { getOnValue(): PDFName | undefined }).getOnValue();
+    (w as { setAppearanceState(s: PDFName): void }).setAppearanceState(
+      on === target ? target : off,
+    );
+  }
+}
+
 /**
  * Build annotated PDF bytes from the original document.
  * Applies user annotations and form fills, returns Uint8Array of the new PDF.
@@ -815,44 +851,96 @@ export async function buildAnnotatedPdfBytes(
   await addAnnotationDicts(pdfDoc, annotations);
 
   // Apply form fills. Dispatch on actual field type — getTextField(name) throws
-  // for dropdowns/radios, so the old try/catch silently dropped those on save.
+  // for dropdowns/radios, so we look up the generic field and instanceof it.
+  // Each field is wrapped in its own try/catch: pdf-lib can throw on
+  // length-constrained text, radios whose buttonValue maps to neither label
+  // nor index, checkboxes missing a /Yes appearance, etc. One bad field must
+  // not abort the rest of the loop (regressed in #577 when the inner catch
+  // was dropped along with the type-specific getters).
   if (formFields.size > 0) {
+    let form: PDFForm | undefined;
     try {
-      const form = pdfDoc.getForm();
+      form = pdfDoc.getForm();
+    } catch {
+      // No AcroForm in this PDF
+    }
+    if (form) {
       for (const [name, value] of formFields) {
-        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 idx = Number(s);
-            if (Number.isInteger(idx) && idx >= 0 && idx < opts.length) {
-              field.select(opts[idx]);
+        try {
+          const field = form.getFieldMaybe(name);
+          if (!field) continue;
+
+          if (field instanceof PDFCheckBox) {
+            const widgets = field.acroField.getWidgets();
+            if (typeof value === "string" && widgets.length > 1) {
+              // Multi-widget "checkbox" with a string value = pdf-lib
+              // misclassified a radio group (PDF lacks the /Ff Radio flag).
+              // The viewer stored pdf.js's buttonValue (the widget's on-state
+              // name, e.g. "0"/"1"); check()/uncheck() would hit the FIRST
+              // widget regardless. Write /V and per-widget /AS directly.
+              setButtonGroupValue(field, value);
+            } else {
+              // Single-widget (real) checkbox. Viewer normally stores boolean,
+              // but be liberal: any truthy non-"Off" string counts as checked.
+              const on =
+                value === true ||
+                (typeof value === "string" && value !== "" && value !== "Off");
+              if (on) field.check();
+              else field.uncheck();
             }
-            // else: value is neither label nor index — leave unset
+          } 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 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
+            }
+          } 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 PDFOptionList) {
+            // Viewer stores multiselect listboxes as a comma-joined string
+            // (pdf.js's annotationStorage value for /Ch is string|string[];
+            // our Map<string,string|boolean> flattens arrays). Only select
+            // values that are actually in the option list — pdf-lib throws
+            // on unknowns and there's no edit-mode fallback like Dropdown.
+            const opts = field.getOptions();
+            const wanted = String(value)
+              .split(",")
+              .map((s) => s.trim())
+              .filter((s) => opts.includes(s));
+            if (wanted.length > 0) field.select(wanted);
+            else field.clear();
+          } else if (field instanceof PDFTextField) {
+            field.setText(String(value));
           }
-        } 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, PDFSignature: no fill_form support yet
+        } catch (err) {
+          // Skip this field; carry on with the rest. Surfacing per-field
+          // failures is the caller's job (see fill_form result), not save's.
+          console.warn(`buildAnnotatedPdfBytes: skipped field "${name}":`, err);
         }
-        // PDFButton, PDFOptionList, PDFSignature: no fill_form support yet
       }
-    } catch {
-      // pdfDoc.getForm() throws if the PDF has no AcroForm
+      // Some PDFs ship widgets with no on-state appearance stream (no
+      // /AP/N/<onValue>). pdf-lib's check()/select()/setText() set /V and /AS
+      // but don't synthesize the missing appearance, so other readers
+      // (Preview, Acrobat) show the field as if unchanged. This generates a
+      // default appearance for any field marked dirty above.
+      try {
+        form.updateFieldAppearances();
+      } catch (err) {
+        console.warn("buildAnnotatedPdfBytes: updateFieldAppearances:", err);
+      }
     }
   }