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

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

@@ -1105,6 +1105,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);
+      }
     }
   }