M1: keep unknown trailer entries on a full save (#1338) (#1431)

* M1: keep unknown trailer entries on a full save (#1338)

A full rewrite built a new trailer from a few known keys, so accepted custom metadata disappeared without a repair note. Copy every other entry and write objects that only those entries reference.

* Run a tool after a background open of the file it asked for

A tool chosen with no document opened the picked file on the worker, but only ran when that open finished inside the picker callback. Remember the command on the pending open, and let the command test wait for the worker.

* Wait for background opens in the command tests

Opening a picked file or a recent file finishes on the worker, so a fixed handful of frames can assert before the tab exists.

* Compare batch copies to the PDF that was written

A new text PDF stamps the current second, so a later pdf() call is a different file. The numbered copy and the source must match the bytes that were saved.

* Drop the unused batch fixture left by the merge

Main already keeps the written PDF in original, so the older source binding is unread and fails -D warnings.

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

ROADMAP.md

@@ -190,6 +190,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
 - **2026-10-13 (M6, choice indices):** Multi-select choice fields write ascending `/I` option indices while preserving the caller's `/V` order. Synthetic forms and headless engine regressions cover reverse selection and save/reopen. Estimates unchanged.
 
 - **2026-10-12 (M10, Windows printing without PowerShell, #1107):** craft-print now prints on Windows in-process: printers from EnumPrinters/GetDefaultPrinter, jobs drawn as vectors by Windows.Data.Pdf into the XPS print API with a driver-validated PrintTicket (exact paper, orientation, no scaling), so printing works where App Control for Business or AppLocker run PowerShell in Constrained Language Mode. Portrait and landscape sheets still print as one job (turned with /Rotate); a sheet size the driver lacks is refused rather than shrunk. Pins craft-libs f03ec27 (craft-libs#28); deny.toml allows the storytold/winprint.rs fork and adds a webpki-root-certs licence exception and a RUSTSEC-2024-0370 ignore for winprint's build-dependencies. An earlier craft-print revision was checked in Windows Sandbox under enforced WDAC: printers listed, CLI and desktop jobs spooled, and no powershell.exe in the 4688 audit. The final pinned revision passed craft-libs Windows CI, including a real paused-queue job; that Sandbox run has not been repeated. Estimates unchanged.
+- **2026-10-12 (M1, #1338):** A full rewrite used to build a new trailer from `/Root`, `/Info`, `/Encrypt` and `/ID`, so accepted unknown entries such as a custom string or `/Version` disappeared, including when an incremental save was forced to rewrite. Those entries are copied, and objects reachable only from them are written and renumbered with the rest of the file. Ordinary incremental saves are unchanged. Estimates unchanged.
 - **2026-10-12 (M4, #1310, #1311, #1312):** Undo back to the opened file left it marked unsaved, so recovery and close treated unchanged work as dirty. After a save, Revert followed the restored snapshot's older base and the next save dropped the latest saved revision. The last saved file is remembered on its own: undo and redo are clean only when they land on it, Revert opens it, and a later save appends to it. Estimates unchanged.
 - **2026-10-12 (M13, CLI robustness scans):** `pdfcraft-cli check` visits a directory once even when input roots overlap or directory symlinks lead back into the tree. PDFs reached through file symlinks remain accepted. Three real CLI regressions cover overlapping roots, a directory cycle and directory/file aliases. Estimates unchanged.
 - **2026-10-12 (M4, Type 3 extraction fidelity):** Extract and Split preserve shared image/resource dictionaries reached through Type 3 glyphs, including nested forms, patterns and soft masks. A bounded object-graph walk protects unknown glyph content; unreadable pages/forms retain their glyph resources, and shared explicit fonts are visited once. Separate page dictionaries still prune unused images. Synthetic save/reopen regressions and an opt-in Poppler pixel oracle cover resource ownership, nested forms, cycles, limits and repeated fonts. Estimates unchanged.

crates/cos/src/writer.rs

@@ -4,7 +4,8 @@
 //!   as the file's last one (table or stream) and a trailer with `/Prev`. The original bytes are
 //!   never modified, which keeps digital signatures over earlier revisions valid.
 //! * `write_full` writes only objects reachable from the trailer, renumbered densely, with a
-//!   cross-reference table or stream — the "Save As" / garbage-collecting path.
+//!   cross-reference table or stream — the "Save As" / garbage-collecting path. Unknown trailer
+//!   entries are copied, including objects reachable only from them.
 
 use std::borrow::Cow;
 use std::collections::{BTreeMap, HashMap, VecDeque};
@@ -257,6 +258,13 @@ pub fn write_full(doc: &Document, opts: &SaveOptions) -> Result<Vec<u8>, CosErro
             queue.push_back(r);
         }
     }
+    // Unknown trailer entries are document data. Objects they alone reference must be written
+    // too, or a full rewrite would keep the key and drop its value.
+    for (key, value) in trailer_in.iter() {
+        if !trailer_key_rewritten(key) {
+            collect_refs(value, &mut |r| queue.push_back(r));
+        }
+    }
     while let Some(r) = queue.pop_front() {
         if map.contains_key(&r) {
             continue;
@@ -337,6 +345,11 @@ pub fn write_full(doc: &Document, opts: &SaveOptions) -> Result<Vec<u8>, CosErro
     if let Some(id) = trailer_in.get(b"ID") {
         trailer.set(b"ID".to_vec(), id.clone());
     }
+    for (key, value) in trailer_in.iter() {
+        if !trailer_key_rewritten(key) {
+            trailer.set(key.clone(), renumber(value, &map));
+        }
+    }
     if reader.document.output_security().0.is_none() {
         ensure_id(&mut trailer, opts, reader.document.bytes());
     }
@@ -400,6 +413,15 @@ impl ObjectStreamBatch {
     }
 }
 
+/// Trailer keys a full rewrite generates itself. Everything else is copied, with references
+/// renumbered, so a save does not drop metadata the reader accepted.
+fn trailer_key_rewritten(key: &[u8]) -> bool {
+    matches!(
+        key,
+        b"Size" | b"Prev" | b"XRefStm" | b"Type" | b"W" | b"Index" | b"Filter" | b"DecodeParms" | b"Length" | b"Root" | b"Info" | b"Encrypt" | b"ID"
+    )
+}
+
 fn output_number(index: usize) -> Result<u32, CosError> {
     u32::try_from(index).ok().and_then(|n| n.checked_add(1)).ok_or_else(|| CosError::Syntax { offset: 0, detail: "too many objects to save".into() })
 }

crates/cos/tests/writer.rs

@@ -327,3 +327,68 @@ fn page_tree_loops_in_object_streams_do_not_crash_readers() {
     assert!(String::from_utf8_lossy(&packed).contains("/ObjStm"));
     assert_eq!(hayro_pages(&packed), 1);
 }
+
+/// Catalog, empty page tree, and trailer entries a reader must not invent: a direct name, a
+/// direct string, and a string object reachable only from the trailer.
+fn trailer_fixture() -> Vec<u8> {
+    let mut out = b"%PDF-1.4\n".to_vec();
+    let mut offsets = Vec::new();
+    for (i, body) in ["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "(only from the trailer)"].iter().enumerate() {
+        offsets.push(out.len());
+        out.extend_from_slice(format!("{} 0 obj\n{body}\nendobj\n", i + 1).as_bytes());
+    }
+    let xref = out.len();
+    out.extend_from_slice(b"xref\n0 4\n0000000000 65535 f \n");
+    for offset in offsets {
+        out.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes());
+    }
+    out.extend_from_slice(
+        format!("trailer\n<< /Size 4 /Root 1 0 R /Version /1.7 /Custom (keep me) /Note 3 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes(),
+    );
+    out
+}
+
+fn trailer_custom(doc: &Document) -> Option<Vec<u8>> {
+    doc.trailer().get(b"Custom").and_then(Object::as_string).map(|s| s.bytes.clone())
+}
+
+fn trailer_note(doc: &Document) -> Option<Vec<u8>> {
+    doc.trailer().get(b"Note").map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.bytes.clone()))
+}
+
+#[test]
+fn full_save_preserves_unknown_trailer_data() {
+    let input = trailer_fixture();
+    let doc = Document::open(Arc::new(input)).unwrap();
+    assert_eq!(trailer_custom(&doc), Some(b"keep me".to_vec()));
+    assert_eq!(trailer_note(&doc).as_deref(), Some(b"only from the trailer".as_slice()));
+    assert_eq!(doc.trailer().get(b"Version").and_then(Object::as_name), Some(&b"1.7"[..]));
+    assert!(doc.repair_log().is_empty());
+    assert!(!doc.full_save_required());
+
+    let inc = Document::open(Arc::new(write_incremental(&doc, &SaveOptions::default()).unwrap())).unwrap();
+    assert_eq!(trailer_custom(&inc), trailer_custom(&doc));
+    assert_eq!(trailer_note(&inc), trailer_note(&doc));
+    assert_eq!(inc.trailer().get(b"Version"), doc.trailer().get(b"Version"));
+    assert!(inc.repair_log().is_empty());
+
+    for object_streams in [true, false] {
+        let opts = SaveOptions { object_streams, ..SaveOptions::default() };
+        let bytes = write_full(&doc, &opts).unwrap();
+        let back = Document::open(Arc::new(bytes)).unwrap();
+        assert!(back.root().is_some());
+        assert!(back.repair_log().is_empty(), "{:?}", back.repair_log());
+        assert_eq!(trailer_custom(&back), trailer_custom(&doc));
+        assert_eq!(trailer_note(&back), trailer_note(&doc));
+        assert_eq!(back.trailer().get(b"Version"), doc.trailer().get(b"Version"));
+    }
+
+    let mut required = doc.clone();
+    required.require_full_save();
+    let back = Document::open(Arc::new(write_incremental(&required, &SaveOptions::default()).unwrap())).unwrap();
+    assert!(back.root().is_some());
+    assert!(back.repair_log().is_empty(), "{:?}", back.repair_log());
+    assert_eq!(trailer_custom(&back), trailer_custom(&doc));
+    assert_eq!(trailer_note(&back), trailer_note(&doc));
+    assert_eq!(back.trailer().get(b"Version"), doc.trailer().get(b"Version"));
+}

docs/roadmap.md

@@ -95,7 +95,7 @@ total matches the parity estimate.
 | M | Milestone | Plan size (h) | Done | Remaining (h, 2026-10-10) | Notes |
 |---|---|---|---|---|---|
 | M0 | Skeleton: workspace, xtask gates, CI | 15–30 | 98% | 1–2 | GitHub workflow, `deny.toml` (licence audit of every dependency), parity checklist (826 features, `xtask parity`) done. Missing: remaining crate stubs, testkit/oracle crates |
-| M1 | COS: filters, crypt, parser, xref, writer | 120–200 | 80% | 20–40 | Done:<br>- filters and crypt: every standard-security revision R2–R6 (RC4, AES-128/256), SASLprep, permissions, creating encryption;<br>- cos: parse and repair (including invalid xref object-number ranges), decoded-stream limits including unfiltered data, decrypt on load, re-encrypt on save, incremental and full writing.<br>Corpus: open/edit/save passes on 958 files, and all 7 password-protected files open.<br>Full saves now pack objects into compressed object streams. An unchanged incremental save of a classic table writes a zero-length cross-reference subsection instead of a section with no subsections (#1339). Fuzzing runs nightly (`xtask fuzz`). Missing: ≥ 250 tests, own image codecs |
+| M1 | COS: filters, crypt, parser, xref, writer | 120–200 | 80% | 20–40 | Done:<br>- filters and crypt: every standard-security revision R2–R6 (RC4, AES-128/256), SASLprep, permissions, creating encryption;<br>- cos: parse and repair (including invalid xref object-number ranges), decoded-stream limits including unfiltered data, decrypt on load, re-encrypt on save, incremental and full writing.<br>Corpus: open/edit/save passes on 958 files, and all 7 password-protected files open.<br>Full saves now pack objects into compressed object streams. An unchanged incremental save of a classic table writes a zero-length cross-reference subsection instead of a section with no subsections (#1339). A full rewrite keeps unknown trailer entries, including objects reachable only from them (#1338). Fuzzing runs nightly (`xtask fuzz`). Missing: ≥ 250 tests, own image codecs |
 | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. Page marks, added content and print output retain small page-unit transform coefficients when serialized. |
 | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. The renderer keeps its one-point crop-size floor in physical points after UserUnit scaling, with regression coverage for crop origins, rotations and f32 reciprocal rounding. Existing unit policy and nearly-zero fallback are retained. |
 | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings |
@@ -177,6 +177,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-10-12 | minor | M1 #1338: a full rewrite keeps unknown trailer entries and objects reachable only from them. Estimates unchanged. |
 | 2026-10-14 | minor | M14: Italian catalog completed (2,528 entries) and dropped from the ≈ 2,000–2,140-entry group; estimates unchanged |
 | 2026-10-12 | minor | M4 #1310 #1311 #1312: undo back to the opened file is clean, Revert returns to the last saved file, and the next save keeps that revision. Estimates unchanged. |
 | 2026-10-14 | minor | M5: reconciled PR #1457 with upstream main, preserving comment submission shortcuts and upstream roadmap progress. Estimates unchanged. |