Merge remote-tracking branch 'origin/main' into tmp351

# Conflicts:
#	ROADMAP.md

ROADMAP.md

@@ -150,6 +150,8 @@ M0 → M1 → M2 → M3 → M4 must happen in order. After M4, M5–M12 can run
 
 - **2026-10-09 (M7, grouped artwork):** Edit text/images selects page-drawn Form XObjects as whole figures, including nested raster/vector/text content. Their BBox and optional Matrix determine the selection; move/resize/rotate/flip/delete rewrite only the selected page placement, preserving shared Forms and other placements. Engine, automation and UI core regressions cover geometry, rendered movement, save/reopen, undo and selection/drag/Delete. Bitmap replacement/extraction remain raster-only; inline images and individual elements inside a Form are not editable through this tool. Overall estimate unchanged (about 30–35%).
 
+- **2026-10-09 (M7, grouped artwork):** Edit text/images selects page-drawn Form XObjects as whole figures, including nested raster/vector/text content. Their BBox and optional Matrix determine the selection; move/resize/rotate/flip/delete rewrite only the selected page placement, preserving shared Forms and other placements. Engine, automation and UI core regressions cover geometry, rendered movement, save/reopen, undo and selection/drag/Delete. Bitmap replacement/extraction remain raster-only; inline images and individual elements inside a Form are not editable through this tool. Overall estimate unchanged (about 30–35%).
+
 - **2026-10-08 (M5.7, signature resize proportions):** Image signature corner handles preserve the embedded image's original aspect ratio and anchor the opposite corner; edge midpoint handles stretch width or height independently. The live preview and committed edit share the constrained geometry, including page/view rotation and reopened PDFs whose rectangles were stretched previously. Synthetic UI tests cover all corners, edge resizing, undo and redo. Milestone and effort estimates unchanged.
 - **2026-10-08 (M5.7, live signature adjustment):** Embedded image signatures resize and move with every pointer frame. Movement hides the frame and handles, which return on release; Escape cancels without committing. A cached background preserves the page beneath the image, and only the completed gesture enters undo history. The read-only engine preview and `comment_image_preview` tool share bounded image decoding and render layers. Synthetic pixel tests cover live geometry, alpha, coloured page content, rotation, cancellation and encrypted save/reopen. Milestone and effort estimates unchanged.
 - **2026-10-08 (M10/M4, create from multiple files):** Create a PDF ▸ Multiple files is live. The picked PDFs, images and `.txt` files open as one unsaved document in the Organize page grid, where pages are removed and reordered and Save pages writes what is shown. The grid gained a "+" at every gap that inserts files right there, and on desktop files dropped on the grid go to the gap under the pointer; Insert from file now takes several files in order, converts images and text, and selects what it inserted. File-type detection moved from the shell into `pdfcraft-create` (`source_kind`), with one engine entry point (`Session::convert_to_pdf`) shared by Open, Insert, Create and the new `doc_create_multiple` tool (which can also write one PDF per file). Not yet: Office/HTML/Markdown inputs, PDF Portfolios, progress for big batches, dropping onto the grid in the web build. Overall estimate unchanged (about 30–35%).

crates/automation/tests/hostile_args.rs

@@ -0,0 +1,314 @@
+//! Every tool, called with hostile arguments (D2) and with paths that try to leave `--root` (D3).
+//!
+//! AGENTS.md §3 and §4 treat tool arguments as untrusted — they are the MCP attack surface, and an
+//! agent can send any JSON at all. But `tools::check_args` only checks the *shape* of an argument
+//! object: that it is an object, that it has no unknown keys, and that the required keys are
+//! present and not null. **Types and ranges are not validated there**, so a string where an
+//! integer belongs, `i64::MIN` where a page number belongs, or a 100,000-character path all reach
+//! the handler. Each handler has to cope on its own, and nothing checked that they all did.
+//!
+//! These tests call every tool in the table with a zoo of hostile values and require two things:
+//!
+//! 1. **Nothing panics.** A panic in a tool is a crashed MCP server or CLI (AGENTS.md §4), so
+//!    every call must come back as `Ok` or as a `ToolError` the agent can read.
+//! 2. **Nothing escapes `--root`.** Every path argument is tried with `..` traversal, absolute
+//!    paths, Windows drive and UNC paths, and a planted symbolic link. The call must fail, and
+//!    afterwards nothing outside the root may have been created.
+//!
+//! The second is the mechanized form of the D3 audit. Reading the code says every handler resolves
+//! its own paths and that no tool dispatches another internally; this keeps that true as tools are
+//! added, which a written finding would not.
+
+use std::panic::{AssertUnwindSafe, catch_unwind};
+use std::path::{Path, PathBuf};
+
+use pdfcraft_automation::{Automation, ToolError, tools};
+use serde_json::{Value, json};
+
+/// A fresh empty directory for one test.
+fn tmp(name: &str) -> PathBuf {
+    let d = std::env::temp_dir().join(format!("pdfcraft-hostile-{}-{name}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&d);
+    std::fs::create_dir_all(&d).expect("temp dir");
+    d
+}
+
+/// Values chosen to break assumptions: wrong types, the ends of the numeric ranges, strings that
+/// are empty, enormous, or full of separators and control characters, and containers that are
+/// empty, mistyped or deeply nested.
+fn zoo() -> Vec<Value> {
+    let deep = {
+        let mut v = json!(0);
+        for _ in 0..64 {
+            v = json!([v]);
+        }
+        v
+    };
+    vec![
+        Value::Null,
+        json!(true),
+        json!(false),
+        json!(0),
+        json!(-1),
+        json!(1),
+        json!(i64::MAX),
+        json!(i64::MIN),
+        json!(u64::MAX),
+        json!(0.5),
+        json!(-0.0),
+        json!(1e308),
+        json!(-1e308),
+        json!(""),
+        json!(" "),
+        json!("0"),
+        json!("-1"),
+        json!("null"),
+        json!("\0"),
+        json!("\u{202E}gnp.txe"), // right-to-left override: a name that displays reversed
+        json!("💥🙈"),
+        json!("%n%s%p"),
+        json!("a".repeat(100_000)),
+        json!("../".repeat(64)),
+        json!([]),
+        json!([null]),
+        json!([i64::MIN, i64::MAX]),
+        json!(["", "../.."]),
+        json!({}),
+        json!({ "unexpected": 1 }),
+        deep,
+    ]
+}
+
+/// Paths that must never resolve inside the root. Absolute ones point into the test's own temp
+/// `base` (the root's parent), never at real system files or network shares, so a confinement
+/// bug can only touch what the test made.
+fn escapes(base: &std::path::Path) -> Vec<String> {
+    let mut out: Vec<String> = vec![
+        "../outside.pdf".into(),
+        "../../outside.pdf".into(),
+        "../".repeat(40) + "outside.pdf",
+        "sub/../../outside.pdf".into(),
+        "./././../outside.pdf".into(),
+        base.join("outside.pdf").to_string_lossy().into_owned(),
+        base.join("new-outside.pdf").to_string_lossy().into_owned(),
+        base.join("sub").join("new-outside.pdf").to_string_lossy().into_owned(),
+        "link-out/outside.pdf".into(), // through a planted symbolic link, when the system allows one
+    ];
+    // Backslashes separate only on Windows; elsewhere this is an ordinary file name in the root.
+    if cfg!(windows) {
+        out.push(r"..\..\outside.pdf".into());
+    }
+    out
+}
+
+/// The properties of `def` whose values are paths, by name and whether they are a list.
+fn path_props(def: &pdfcraft_automation::ToolDef) -> Vec<(String, bool)> {
+    let Some(props) = def.input_schema["properties"].as_object() else { return Vec::new() };
+    props
+        .iter()
+        .filter(|(name, spec)| {
+            // `out` (an output file) and `image` (a picture to place) are paths when they are
+            // strings; `image` is an image number elsewhere, which the integer check skips.
+            let is_pathish = matches!(name.as_str(), "path" | "paths" | "folder" | "out_dir" | "file" | "out" | "image")
+                || spec["description"].as_str().is_some_and(|d| d.starts_with("A file path"));
+            // `file_page` is a page number inside a file, not a path.
+            is_pathish && spec["type"] != "integer"
+        })
+        .map(|(name, spec)| (name.clone(), spec["type"] == "array"))
+        .collect()
+}
+
+/// An arguments object giving every required property a plausible value, so a hostile value for
+/// one property is not masked by a "missing argument" error for another.
+///
+/// `doc` gets a **real** open document when one is given. Without that, almost every tool fails at
+/// `self.doc()` before it ever looks at a path, and a confinement test would pass vacuously — as
+/// an earlier version of this file did, even with `resolve` disabled entirely.
+fn plausible_with(def: &pdfcraft_automation::ToolDef, doc: Option<u64>) -> serde_json::Map<String, Value> {
+    let mut obj = plausible(def);
+    if let Some(id) = doc
+        && def.input_schema["properties"].as_object().is_some_and(|p| p.contains_key("doc"))
+    {
+        obj.insert("doc".into(), json!(id));
+    }
+    obj
+}
+
+fn plausible(def: &pdfcraft_automation::ToolDef) -> serde_json::Map<String, Value> {
+    let mut obj = serde_json::Map::new();
+    let props = def.input_schema["properties"].as_object().cloned().unwrap_or_default();
+    for req in def.input_schema["required"].as_array().into_iter().flatten().filter_map(Value::as_str) {
+        let spec = props.get(req).cloned().unwrap_or(Value::Null);
+        let value = match spec["type"].as_str() {
+            Some("integer") => json!(1),
+            Some("number") => json!(1.0),
+            Some("boolean") => json!(false),
+            Some("array") => json!([]),
+            Some("object") => json!({}),
+            _ => json!("in.pdf"),
+        };
+        obj.insert(req.to_string(), value);
+    }
+    obj
+}
+
+/// Run `call`, turning a panic into a message instead of unwinding out of the test.
+fn guarded(a: &mut Automation, tool: &str, args: &Value) -> std::result::Result<(), String> {
+    let result = catch_unwind(AssertUnwindSafe(|| a.call(tool, args)));
+    match result {
+        Ok(Ok(_)) => Ok(()),
+        // Any ToolError is a good outcome: the agent is told what was wrong.
+        Ok(Err(ToolError::UnknownTool(_) | ToolError::InvalidArgs(_) | ToolError::Failed(_))) => Ok(()),
+        Err(panic) => {
+            let what = panic
+                .downcast_ref::<&str>()
+                .map(|s| (*s).to_string())
+                .or_else(|| panic.downcast_ref::<String>().cloned())
+                .unwrap_or_else(|| "panic".into());
+            Err(format!("{tool} panicked on {args}: {what}"))
+        }
+    }
+}
+
+#[test]
+fn no_tool_panics_on_hostile_arguments() {
+    let root = tmp("args");
+    let mut a = Automation::new().with_root(&root).expect("root");
+    let values = zoo();
+    let mut failures = Vec::new();
+    let mut calls = 0usize;
+
+    for def in tools() {
+        // The whole arguments object, not just its properties.
+        for bad in [json!(null), json!([]), json!("string"), json!(7), json!(true), json!({})] {
+            calls += 1;
+            if let Err(e) = guarded(&mut a, def.name, &bad) {
+                failures.push(e);
+            }
+        }
+        let props: Vec<String> = def.input_schema["properties"].as_object().map(|p| p.keys().cloned().collect()).unwrap_or_default();
+        for prop in &props {
+            for value in &values {
+                let mut obj = plausible(&def);
+                obj.insert(prop.clone(), value.clone());
+                calls += 1;
+                if let Err(e) = guarded(&mut a, def.name, &Value::Object(obj)) {
+                    failures.push(e);
+                }
+            }
+        }
+    }
+
+    eprintln!("hostile arguments: {calls} calls over {} tools, {} panics", tools().len(), failures.len());
+    for f in failures.iter().take(20) {
+        eprintln!("  {f}");
+    }
+    let _ = std::fs::remove_dir_all(&root);
+    assert!(failures.is_empty(), "{} tool call(s) panicked; the first is: {}", failures.len(), failures.first().map_or("", String::as_str));
+}
+
+#[test]
+fn no_tool_escapes_the_root() {
+    let base = tmp("escape");
+    let root = base.join("root");
+    std::fs::create_dir_all(&root).expect("root");
+    // What a traversal would reach, and the state we require to be unchanged afterwards.
+    let outside = base.join("outside.pdf");
+    std::fs::write(
+        &outside,
+        b"%PDF-1.7
+% not yours
+",
+    )
+    .expect("outside file");
+    let before = listing(&base);
+
+    // A link inside the root pointing out of it, where the system allows one to be made.
+    #[cfg(unix)]
+    let _ = std::os::unix::fs::symlink(&base, root.join("link-out"));
+    #[cfg(windows)]
+    let _ = std::os::windows::fs::symlink_dir(&base, root.join("link-out"));
+
+    let mut a = Automation::new().with_root(&root).expect("root");
+
+    // A real document, so tools reach their path arguments instead of failing on `doc` first.
+    let created = a.call("doc_create", &json!({ "from": "blank", "pages": 1 })).expect("create a blank document");
+    let doc = created
+        .into_iter()
+        .find_map(|c| match c {
+            pdfcraft_automation::Content::Json(v) => v["doc"].as_u64(),
+            pdfcraft_automation::Content::Png { .. } => None,
+        })
+        .expect("the new document has an id");
+
+    let mut leaked = Vec::new();
+    let mut tried = 0usize;
+    let mut refused = 0usize;
+
+    for def in tools() {
+        for (prop, is_list) in path_props(&def) {
+            for escape in escapes(&base) {
+                let mut obj = plausible_with(&def, Some(doc));
+                obj.insert(prop.clone(), if is_list { json!([escape]) } else { json!(escape) });
+                let args = Value::Object(obj);
+                tried += 1;
+                match catch_unwind(AssertUnwindSafe(|| a.call(def.name, &args))) {
+                    // A path that leaves the root must be refused. Succeeding means the tool
+                    // either used it or ignored an argument it declares — both are faults.
+                    Ok(Ok(_)) => leaked.push(format!("{} accepted {prop}={escape:?}", def.name)),
+                    Ok(Err(_)) => refused += 1,
+                    Err(_) => leaked.push(format!("{} panicked on {prop}={escape:?}", def.name)),
+                }
+            }
+        }
+    }
+
+    // Second net: nothing outside the root may have appeared, vanished or changed.
+    let after = listing(&base);
+    for (path, size) in &after {
+        if path.starts_with(&root) {
+            continue;
+        }
+        match before.iter().find(|(p, _)| p == path) {
+            Some((_, was)) if was == size => {}
+            Some(_) => leaked.push(format!("{} was modified outside the root", path.display())),
+            None => leaked.push(format!("{} was created outside the root", path.display())),
+        }
+    }
+    for (path, _) in &before {
+        if !path.starts_with(&root) && !after.iter().any(|(p, _)| p == path) {
+            leaked.push(format!("{} was deleted outside the root", path.display()));
+        }
+    }
+
+    eprintln!("root confinement: {tried} escape attempts over {} tools, {refused} refused, {} leaks", tools().len(), leaked.len());
+    for l in leaked.iter().take(20) {
+        eprintln!("  {l}");
+    }
+    let _ = std::fs::remove_dir_all(&base);
+    assert!(tried > 100, "only {tried} escape attempts: the path properties are not being found");
+    assert!(leaked.is_empty(), "{} confinement failure(s); the first is: {}", leaked.len(), leaked.first().map_or("", String::as_str));
+}
+
+/// Every regular file under `dir` with its size, not following symbolic links.
+fn listing(dir: &Path) -> Vec<(PathBuf, u64)> {
+    let mut out = Vec::new();
+    let mut stack = vec![dir.to_path_buf()];
+    while let Some(d) = stack.pop() {
+        let Ok(entries) = std::fs::read_dir(&d) else { continue };
+        for entry in entries.flatten() {
+            let path = entry.path();
+            let Ok(meta) = std::fs::symlink_metadata(&path) else { continue };
+            if meta.file_type().is_symlink() {
+                continue; // do not walk through a link we planted
+            }
+            if meta.is_dir() {
+                stack.push(path);
+            } else if meta.is_file() {
+                out.push((path, meta.len()));
+            }
+        }
+    }
+    out.sort();
+    out
+}

crates/forms/src/tests.rs

@@ -1886,3 +1886,209 @@ fn check_box_redraw_keeps_down_states_it_still_draws() {
     assert!(d.contains(on.as_bytes()) && d.contains(b"Off"));
     assert!(!d.contains(b"Gone"), "a state the box no longer draws");
 }
+
+#[test]
+fn filling_and_authoring_keep_shared_appearances_and_drop_stale_alternates() {
+    let alternates: [(&[u8], &[u8]); 2] = [(b"R", b"0 0 20 10 re f\n"), (b"D", b"1 1 18 8 re S\n")];
+    let cases = [
+        (NewField::Text { multiline: false }, true),
+        (NewField::Text { multiline: false }, false),
+        (NewField::CheckBox, false),
+        (NewField::Button { caption: "Button".into() }, false),
+        (NewField::Signature, false),
+    ];
+    for (kind, fill) in cases {
+        for indirect in [false, true] {
+            let mut doc = one_page();
+            let first = add_field(&mut doc, 0, [50.0, 600.0, 250.0, 630.0], &kind, Some("First")).unwrap();
+            let other = add_field(&mut doc, 0, [50.0, 550.0, 250.0, 580.0], &kind, Some("Other")).unwrap();
+            let first_widget = field(&fields(&doc), &first).widgets[0].obj;
+            let other_widget = field(&fields(&doc), &other).widgets[0].obj;
+            let mut entries = doc.get(first_widget).as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().clone();
+            for (key, bytes) in alternates {
+                let mut d = Dict::new();
+                d.set(b"Type".to_vec(), Object::name("XObject"));
+                d.set(b"Subtype".to_vec(), Object::name("Form"));
+                d.set(b"BBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 20.into(), 10.into()]));
+                let r = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(d, bytes.to_vec())));
+                entries.set(key.to_vec(), Object::Ref(r));
+            }
+            entries.set(b"VendorState".to_vec(), Object::name("Retained"));
+            let shared = if indirect { Object::Ref(doc.add(Object::Dict(entries))) } else { Object::Dict(entries) };
+            for widget in [first_widget, other_widget] {
+                doc.update_dict(widget, |d| d.set(b"AP".to_vec(), shared.clone())).unwrap();
+            }
+            let original = reopen(&doc);
+            for full in [false, true] {
+                let mut doc = original.clone();
+                let other_widget = field(&fields(&doc), &other).widgets[0].obj;
+                let source = doc.get(other_widget).as_dict().unwrap().get(b"AP").unwrap().clone();
+                let before = doc.resolve(&source);
+                if fill {
+                    set_value(&mut doc, &first, &FieldValue::Text("Updated value".into())).unwrap();
+                } else {
+                    set_props(&mut doc, &first, &FieldProps { tooltip: Some("Updated help".into()), ..FieldProps::default() }).unwrap();
+                }
+                assert_eq!(doc.resolve(&source), before, "a shared AP must not change");
+                let bytes = if full {
+                    pdfcraft_cos::write_full(&doc, &SaveOptions::default()).unwrap()
+                } else {
+                    assert!(!doc.revisions().is_empty(), "incremental save needs an existing revision");
+                    assert!(!doc.encryption_changed() && !doc.full_save_required(), "this edit must not require a full rewrite");
+                    let original = doc.bytes();
+                    let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap();
+                    assert!(bytes.len() > original.len(), "incremental save appends the changed appearance");
+                    assert!(bytes.starts_with(original.as_slice()), "incremental save preserves the complete original prefix");
+                    bytes
+                };
+                hayro_syntax::Pdf::new(bytes.clone()).unwrap();
+                let doc = Document::open(Arc::new(bytes)).unwrap();
+                let all = fields(&doc);
+                let first = field(&all, &first);
+                let other = field(&all, &other);
+                let first_ap = doc.resolve(doc.get(first.widgets[0].obj).as_dict().unwrap().get(b"AP").unwrap());
+                let other_ap = doc.resolve(doc.get(other.widgets[0].obj).as_dict().unwrap().get(b"AP").unwrap());
+                let first_ap = first_ap.as_dict().unwrap();
+                let other_ap = other_ap.as_dict().unwrap();
+                assert_ne!(first_ap.get(b"N"), other_ap.get(b"N"), "normal appearance was regenerated");
+                assert_eq!(other_ap.len(), 4);
+                // The redrawn widget keeps unknown entries, but not down/rollover looks that
+                // would show its old value or caption on press or hover.
+                assert_eq!(
+                    first_ap.len(),
+                    2,
+                    "{kind:?} fill={fill} indirect={indirect} full={full}: {:?}",
+                    first_ap.iter().map(|(k, _)| String::from_utf8_lossy(k).into_owned()).collect::<Vec<_>>()
+                );
+                for (key, bytes) in alternates {
+                    assert!(first_ap.get(key).is_none(), "stale /{}", String::from_utf8_lossy(key));
+                    let object = doc.resolve(other_ap.get(key).unwrap());
+                    let Object::Stream(stream) = &*object else { panic!("alternate appearance") };
+                    assert_eq!(stream.decoded().unwrap(), bytes);
+                }
+                assert_eq!(first_ap.name(b"VendorState"), Some(&b"Retained"[..]));
+                assert_eq!(other_ap.name(b"VendorState"), Some(&b"Retained"[..]));
+                if fill {
+                    assert_eq!(first.value, ["Updated value"]);
+                    assert!(ap(&doc, &first.widgets[0]).contains("(Updated value) Tj"));
+                    assert!(other.value.is_empty());
+                } else {
+                    assert_eq!(first.tooltip.as_deref(), Some("Updated help"));
+                    assert_eq!(other.tooltip, None);
+                }
+            }
+        }
+    }
+}
+
+#[test]
+fn pushbutton_redraw_resolves_caption_and_layout_after_save_and_reopen() {
+    fn saved(doc: &Document, full: bool) -> Document {
+        let bytes = if full {
+            pdfcraft_cos::write_full(doc, &SaveOptions::default()).unwrap()
+        } else {
+            assert!(!doc.revisions().is_empty(), "incremental save needs an existing revision");
+            assert!(!doc.encryption_changed() && !doc.full_save_required(), "this edit must not require a full rewrite");
+            let original = doc.bytes();
+            let bytes = write_incremental(doc, &SaveOptions::default()).unwrap();
+            assert!(bytes.len() > original.len(), "incremental save appends the changed appearance");
+            assert!(bytes.starts_with(original.as_slice()), "incremental save preserves the complete original prefix");
+            bytes
+        };
+        hayro_syntax::Pdf::new(bytes.clone()).unwrap();
+        Document::open(Arc::new(bytes)).unwrap()
+    }
+
+    for indirect_caption in [false, true] {
+        for indirect_layout in [false, true] {
+            for icon_only in [false, true] {
+                let mut doc = one_page();
+                let name =
+                    add_field(&mut doc, 0, [50.0, 600.0, 250.0, 650.0], &NewField::Button { caption: "Original caption".into() }, Some("Picture"))
+                        .unwrap();
+                let widget = field(&fields(&doc), &name).widgets[0].obj;
+                let caption = Object::String(pdfcraft_cos::PdfString::literal(b"Original caption".to_vec()));
+                let caption = if indirect_caption { Object::Ref(doc.add(caption)) } else { caption };
+                let layout = Object::Int(i64::from(icon_only));
+                let layout = if indirect_layout { Object::Ref(doc.add(layout)) } else { layout };
+                let mut icon_dict = Dict::new();
+                icon_dict.set(b"Type".to_vec(), Object::name("XObject"));
+                icon_dict.set(b"Subtype".to_vec(), Object::name("Form"));
+                icon_dict.set(b"BBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 4.into(), 2.into()]));
+                let icon_bytes = b"0 0 4 2 re f\n";
+                let icon = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(icon_dict, icon_bytes.to_vec())));
+                let mut mk = Dict::new();
+                mk.set(b"CA".to_vec(), caption);
+                mk.set(b"TP".to_vec(), layout);
+                mk.set(b"I".to_vec(), Object::Ref(icon));
+                mk.set(b"VendorNote".to_vec(), Object::name("Retained"));
+                let mk = doc.add(Object::Dict(mk));
+                doc.update_dict(widget, |d| d.set(b"MK".to_vec(), Object::Ref(mk))).unwrap();
+
+                // Import the operands from a real saved file before an unrelated public edit.
+                let original = saved(&doc, true);
+                for full in [false, true] {
+                    let mut doc = original.clone();
+                    let widget = field(&fields(&doc), &name).widgets[0].obj;
+                    let mk_before = doc.get(widget).as_dict().unwrap().get(b"MK").unwrap().clone();
+                    let original_mk = doc.resolve(&mk_before);
+                    set_props(&mut doc, &name, &FieldProps { tooltip: Some("Picture help".into()), ..FieldProps::default() }).unwrap();
+                    assert_eq!(doc.get(widget).as_dict().unwrap().get(b"MK"), Some(&mk_before));
+                    assert_eq!(doc.resolve(&mk_before), original_mk, "redraw must not rewrite the source MK");
+                    let doc = saved(&doc, full);
+                    let f = field(&fields(&doc), &name).clone();
+                    assert_eq!(f.tooltip.as_deref(), Some("Picture help"));
+                    let w = &f.widgets[0];
+                    let wd = doc.get(w.obj);
+                    let mk_object = doc.resolve(wd.as_dict().unwrap().get(b"MK").unwrap());
+                    let mk = mk_object.as_dict().unwrap();
+                    assert_eq!(mk.get(b"CA").unwrap().as_ref().is_some(), indirect_caption);
+                    assert_eq!(doc.resolve(mk.get(b"CA").unwrap()).as_string().unwrap().bytes, b"Original caption");
+                    assert_eq!(mk.get(b"TP").unwrap().as_ref().is_some(), indirect_layout);
+                    assert_eq!(doc.resolve(mk.get(b"TP").unwrap()).as_int(), Some(i64::from(icon_only)));
+                    assert_eq!(mk.name(b"VendorNote"), Some(&b"Retained"[..]));
+                    let appearance = ap(&doc, w);
+                    assert_eq!(appearance.contains("(Original caption) Tj"), !icon_only, "{appearance}");
+                    assert_eq!(appearance.matches("/Icon Do").count(), 1, "{appearance}");
+                    assert!(appearance.contains("24.000000 0 0 24.000000 52.000 1.000 cm"), "{appearance}");
+                    let n = wd.as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().reference(b"N").unwrap();
+                    let appearance_object = doc.get(n);
+                    let Object::Stream(stream) = &*appearance_object else { panic!("appearance") };
+                    let resource_icon =
+                        stream.dict.get(b"Resources").unwrap().as_dict().unwrap().get(b"XObject").unwrap().as_dict().unwrap().get(b"Icon").unwrap();
+                    assert_eq!(resource_icon, mk.get(b"I").unwrap());
+                    let icon_object = doc.resolve(resource_icon);
+                    let Object::Stream(icon) = &*icon_object else { panic!("icon") };
+                    assert_eq!(icon.decoded().unwrap(), icon_bytes);
+                }
+            }
+        }
+    }
+}
+
+/// A check box's down appearances survive a redraw for the states it still draws (same names),
+/// and a down appearance for a state it no longer has is dropped rather than left stale.
+#[test]
+fn check_box_redraw_keeps_down_states_it_still_draws() {
+    let mut doc = one_page();
+    let name = add_field(&mut doc, 0, [50.0, 600.0, 70.0, 620.0], &NewField::CheckBox, Some("agree")).unwrap();
+    let widget = field(&fields(&doc), &name).widgets[0].obj;
+    let on = field(&fields(&doc), &name).widgets[0].on_state.clone().unwrap_or_else(|| "Yes".into());
+    let mut ap = doc.get(widget).as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().clone();
+    let mut down = Dict::new();
+    for state in [on.as_bytes(), b"Off".as_slice(), b"Gone".as_slice()] {
+        let mut d = Dict::new();
+        d.set(b"Subtype".to_vec(), Object::name("Form"));
+        d.set(b"BBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 20.into(), 20.into()]));
+        let r = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(d, b"0 0 20 20 re f\n".to_vec())));
+        down.set(state.to_vec(), Object::Ref(r));
+    }
+    ap.set(b"D".to_vec(), Object::Dict(down));
+    doc.update_dict(widget, |d| d.set(b"AP".to_vec(), Object::Dict(ap))).unwrap();
+    set_props(&mut doc, &name, &FieldProps { tooltip: Some("Tick to agree".into()), ..FieldProps::default() }).unwrap();
+    let ap = doc.get(widget).as_dict().unwrap().get(b"AP").map(|a| doc.resolve(a)).unwrap();
+    let d = ap.as_dict().unwrap().get(b"D").map(|d| doc.resolve(d)).expect("down states kept");
+    let d = d.as_dict().unwrap();
+    assert!(d.contains(on.as_bytes()) && d.contains(b"Off"));
+    assert!(!d.contains(b"Gone"), "a state the box no longer draws");
+}