M3: doc_open reads open parameters and link_open applies named destinations (storytold#3425)

Everything is reachable headlessly (AGENTS.md §3), so the open-parameter
parser moves from the app to pdfcraft_engine::open_parameters. doc_open
now splits `manual.pdf#page=2` the way the app does, only when the whole
path names no file, resolving the file part under --root. It reports the
destination as link_open does.

link_open and doc_open share destination_report, which looks a named
destination up in the target's /Names /Dests or catalog /Dests, instead
of reporting it as not applied.

Raised by CodeRabbit in review of storytold#5049.

Author: hazeliscoding

crates/automation/src/documents.rs

@@ -100,12 +100,27 @@ impl Automation {
     }
 
     pub(crate) fn doc_open(&mut self, a: &Args) -> Result<Value> {
-        let path = self.resolve(a.str("path")?, false)?;
+        let requested = a.str("path")?;
+        // `manual.pdf#nameddest=Intro`, as a help system starts a PDF reader (#3425): the file under
+        // --root, and where it opens. A path that names a file is never split.
+        let parameters = pdfcraft_engine::open_parameters::split(requested, |p| self.resolve(p, false).is_ok_and(|p| p.is_file()));
+        let (requested, destination) = match &parameters {
+            Some((file, destination)) => (file.as_str(), Some(destination.clone())),
+            None => (requested, None),
+        };
+        let path = self.resolve(requested, false)?;
         let bytes = read_input(&path)?;
         let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
         let id = self.session.open(name, Some(path.to_string_lossy().into_owned()), Arc::new(bytes), a.opt_str("password")?).map_err(failed)?;
         let doc = self.session.get(id).ok_or_else(|| failed("the document vanished"))?;
-        Ok(summary(doc))
+        let mut value = summary(doc);
+        if let Some(destination) = destination {
+            let (applied, destination, warning) = crate::links::destination_report(doc, destination);
+            value["destination_applied"] = json!(applied);
+            value["destination"] = destination;
+            value["warning"] = json!(warning);
+        }
+        Ok(value)
     }
 
     pub(crate) fn doc_close(&mut self, a: &Args) -> Result<Value> {

crates/automation/src/links.rs

@@ -11,6 +11,26 @@ use std::path::Path;
 use crate::comments::parse_color;
 use crate::{Args, Automation, Result, ToolError, failed};
 
+/// Where `destination` puts `doc`, a document just opened from a link or with open parameters:
+/// whether it applies, the destination as reported, and a warning when it doesn't. A named
+/// destination is looked up in the document, as the app does.
+pub(crate) fn destination_report(doc: &pdfcraft_engine::Document, destination: RemoteDestination) -> (bool, Value, Option<&'static str>) {
+    let (name, page, view) = match destination {
+        RemoteDestination::Default => return (true, json!({ "kind": "default" }), None),
+        RemoteDestination::Page { page, view } => (None, page, view),
+        RemoteDestination::Named(name) => match pdfcraft_render::named_destination(doc.bytes.clone(), doc.password.as_deref(), &name) {
+            Some((page, view)) => (Some(name), page, view),
+            None => return (false, json!({ "kind": "named", "name": name }), Some("The PDF opened, but the named destination was not applied.")),
+        },
+    };
+    let mut value = json!({ "kind": "page", "page": page.saturating_add(1), "view": dest_view_value(view) });
+    if let Some(name) = name {
+        value["kind"] = json!("named");
+        value["name"] = json!(name);
+    }
+    if page < doc.info.pages.len() { (true, value, None) } else { (false, value, Some("The PDF opened, but the page destination is out of range.")) }
+}
+
 fn bad(m: impl Into<String>) -> ToolError {
     ToolError::InvalidArgs(m.into())
 }
@@ -135,20 +155,7 @@ impl Automation {
         let id =
             self.session.open(name, Some(path.to_string_lossy().into_owned()), std::sync::Arc::new(bytes), a.opt_str("password")?).map_err(failed)?;
         let opened = self.session.get(id).ok_or_else(|| failed("the linked document vanished"))?;
-        let (destination_applied, destination_value, warning) = match destination {
-            RemoteDestination::Default => (true, json!({ "kind": "default" }), None),
-            RemoteDestination::Named(name) => {
-                (false, json!({ "kind": "named", "name": name }), Some("The PDF opened, but the named destination was not applied."))
-            }
-            RemoteDestination::Page { page, view } if page < opened.info.pages.len() => {
-                (true, json!({ "kind": "page", "page": page.saturating_add(1), "view": dest_view_value(view) }), None)
-            }
-            RemoteDestination::Page { page, view } => (
-                false,
-                json!({ "kind": "page", "page": page.saturating_add(1), "view": dest_view_value(view) }),
-                Some("The PDF opened, but the link's page destination is out of range."),
-            ),
-        };
+        let (destination_applied, destination_value, warning) = destination_report(opened, destination);
         let mut display_path = path.to_string_lossy().into_owned();
         if let Some(root) = &self.root
             && let Ok(relative) = path.strip_prefix(root)

crates/automation/src/tools.rs

@@ -138,7 +138,7 @@ pub fn tools() -> Vec<ToolDef> {
         t("attachment_delete", "Delete an attachment", "Remove the document-level attachment with this name (as doc_info lists it). Undoable.")
             .destructive()
             .with(schema(json!({"doc": doc(), "name": {"type": "string"}}), &["doc", "name"])),
-        t("doc_open", "Open a PDF", "Open a PDF file and return its document id, page count and whether it can be edited.")
+        t("doc_open", "Open a PDF", "Open a PDF file and return its document id, page count and whether it can be edited. A path that ends in open parameters, as help systems pass them (`manual.pdf#page=12`, `#nameddest=Intro` or `#Intro`), opens the file and reports its destination as link_open does; a path that names a file opens as it is.")
             .cmd("file.open")
             .with(schema(json!({ "path": { "type": "string" }, "password": { "type": "string", "description": "User or owner password for encrypted files." } }), &["path"])),
         t("doc_list", "List open documents", "List the open documents with their ids, page counts and unsaved state.").ro().with(schema(json!({}), &[])),
@@ -652,7 +652,7 @@ pub fn tools() -> Vec<ToolDef> {
         t("link_list", "List links", "Every link: page, 1-based index (for link_edit/link_delete), rect (points from the top-left of the page), and where it goes (url or to_page).")
             .ro()
             .with(schema(json!({ "doc": doc() }), &["doc"])),
-        t("link_open", "Open a linked PDF", "Follow a local GoToR link in PdfCraft: open the referenced PDF as a new document, respecting --root. page and index use link_list numbering. Named destinations are reported but not applied. Absolute file specifications require allow_absolute: true after reviewing the target; relative links need no opt-in.")
+        t("link_open", "Open a linked PDF", "Follow a local GoToR link in PdfCraft: open the referenced PDF as a new document, respecting --root. page and index use link_list numbering. A named destination is looked up in the target, as the app does; one the target doesn't define is reported with a warning. Absolute file specifications require allow_absolute: true after reviewing the target; relative links need no opt-in.")
             .with(schema(
                 json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "index": { "type": "integer", "minimum": 1 }, "password": { "type": "string", "description": "Optional password for the linked PDF." }, "allow_absolute": { "type": "boolean", "description": "Explicitly allow an absolute local GoToR target (default false). Existing validation and --root confinement still apply." } }),
                 &["doc", "page", "index"],

crates/automation/tests/it/automation.rs

@@ -9,7 +9,16 @@ use serde_json::{Value, json};
 
 /// A PDF with `n` 200×300 pt pages reading "Page 1", "Page 2", …
 fn fixture(n: usize) -> Vec<u8> {
-    let mut objs: Vec<Vec<u8>> = vec![b"<< /Type /Catalog /Pages 2 0 R >>".to_vec()];
+    fixture_with(n, "")
+}
+
+/// Named destinations for [`fixture_with`]: `chapter` (page 2) in the catalog's /Dests and
+/// `Load Flow` (page 3) in the /Names /Dests tree.
+const NAMED_DESTS: &str = "/Dests << /chapter [6 0 R /Fit] >> /Names << /Dests << /Names [(Load Flow) [8 0 R /XYZ 0 300 0]] >> >>";
+
+/// [`fixture`] with `catalog` added to the catalog dictionary. Page `i` (0-based) is object `4 + 2i`.
+fn fixture_with(n: usize, catalog: &str) -> Vec<u8> {
+    let mut objs: Vec<Vec<u8>> = vec![format!("<< /Type /Catalog /Pages 2 0 R {catalog} >>").into_bytes()];
     let kids: Vec<String> = (0..n).map(|i| format!("{} 0 R", 4 + 2 * i)).collect();
     objs.push(format!("<< /Type /Pages /Kids [{}] /Count {n} /MediaBox [0 0 200 300] >>", kids.join(" ")).into_bytes());
     objs.push(b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_vec());
@@ -5258,6 +5267,36 @@ fn local_gotor_link_open_refuses_directories_devices_and_non_pdf_targets() {
     }
 }
 
+#[test]
+fn doc_open_reads_open_parameters_after_the_file_name() {
+    // #3425: help systems start a PDF reader with `manual.pdf#nameddest=…` or `#page=…`.
+    let (_base, root) = sandbox("open-parameters");
+    std::fs::write(root.join("manual.pdf"), fixture_with(3, NAMED_DESTS)).unwrap();
+    std::fs::write(root.join("odd#page=2.pdf"), fixture(1)).unwrap();
+    let mut a = auto(&root);
+    for (path, kind, page) in [("manual.pdf#page=3", "page", 3), ("manual.pdf#nameddest=Load Flow", "named", 3), ("manual.pdf#chapter", "named", 2)] {
+        let opened = ok(&mut a, "doc_open", json!({ "path": path }));
+        assert_eq!((opened["name"].as_str(), opened["pages"].as_u64()), (Some("manual.pdf"), Some(3)), "{path}");
+        assert_eq!(opened["destination_applied"], true, "{path}: {opened}");
+        assert_eq!((opened["destination"]["kind"].as_str(), opened["destination"]["page"].as_u64()), (Some(kind), Some(page)), "{path}");
+    }
+    for (path, warning) in [("manual.pdf#nameddest=Nowhere", "named destination"), ("manual.pdf#page=9", "out of range")] {
+        let opened = ok(&mut a, "doc_open", json!({ "path": path }));
+        assert_eq!(opened["destination_applied"], false, "{path}");
+        assert!(opened["warning"].as_str().is_some_and(|w| w.contains(warning)), "{path}: {opened}");
+    }
+    // A file whose own name has a `#` opens as itself, and a plain path reports no destination.
+    let opened = ok(&mut a, "doc_open", json!({ "path": "odd#page=2.pdf" }));
+    assert_eq!((opened["name"].as_str(), opened["pages"].as_u64()), (Some("odd#page=2.pdf"), Some(1)));
+    assert!(opened.get("destination").is_none(), "{opened}");
+    // The file before the `#` must exist, inside --root.
+    let before = a.session().docs().len();
+    for path in ["missing.pdf#page=2", "../outside/secret.pdf#page=1"] {
+        err(&mut a, "doc_open", json!({ "path": path }));
+        assert_eq!(a.session().docs().len(), before, "{path}");
+    }
+}
+
 #[test]
 fn local_gotor_link_open_applies_destinations_and_honors_root() {
     let (base, root) = sandbox("gotor-open");
@@ -5307,6 +5346,13 @@ fn local_gotor_link_open_applies_destinations_and_honors_root() {
         assert_eq!(opened["destination_applied"], false);
         assert!(opened["warning"].as_str().unwrap().contains(warning), "{opened}");
     }
+    // A name the target defines is looked up there, as the app does.
+    std::fs::write(root.join("named-target.pdf"), fixture_with(3, NAMED_DESTS)).unwrap();
+    std::fs::write(root.join("named-found.pdf"), remote_link_pdf("named-target.pdf", Some("(chapter)"))).unwrap();
+    let source = ok(&mut a, "doc_open", json!({ "path": "named-found.pdf" }))["doc"].as_u64().unwrap();
+    let opened = ok(&mut a, "link_open", json!({ "doc": source, "page": 1, "index": 1 }));
+    assert_eq!(opened["destination_applied"], true, "{opened}");
+    assert_eq!(opened["destination"], json!({ "kind": "named", "name": "chapter", "page": 2, "view": { "kind": "fit" } }));
 
     let password_source = ok(&mut a, "doc_open", json!({ "path": "password-source.pdf" }))["doc"].as_u64().unwrap();
     let documents_before = a.session().docs().len();

crates/engine/README.md

@@ -78,6 +78,7 @@ family (`sign`, `sign_with_timestamp`, `timestamp_document`, `embed_ltv`).
 | `catalog` | Acrobat's "All tools" information architecture as data — groups, sections, icons, hues, and an `Availability` of `Ready`, `Planned(milestone)` or `Provider`. The UI renders what is *not* built yet, honestly |
 | `actions` | PDF action dispatch (go-to, URI, named, set-layer-state, JavaScript) |
 | `links` | Link creation, properties, and finding URLs in page text |
+| `open_parameters` | A path's PDF open parameters (`manual.pdf#nameddest=Intro`, `#page=12`): the file to open and where to show it. The app and `doc_open` both read them |
 | `js` | The bridge to `pdfcraft-js`. `forms` never depends on `js` directly; the engine injects it, so a build without JavaScript is possible |
 | `xfa`, `ocr`, `compare`, `export` | Façades over the L3/L4 crates of the same names |
 

crates/engine/src/lib.rs

@@ -37,6 +37,7 @@ mod layers;
 pub mod links;
 pub mod ocr;
 mod open;
+pub mod open_parameters;
 pub mod optimizer;
 mod page_filter;
 mod pictures;

…/ui-egui/src/platform/open_parameters.rs crates/engine/src/open_parameters.rscrates/ui-egui/src/platform/open_parameters.rs renamed to crates/engine/src/open_parameters.rs crates/ui-egui/src/platform/open_parameters.rs renamed to crates/engine/src/open_parameters.rs

@@ -13,7 +13,7 @@ const MAX_CANDIDATES: usize = 32;
 /// The file `path` names and the destination its open parameters ask for, when `path` is not a
 /// file itself but what comes before one of its `#` is. The first such `#` counts, so a folder or
 /// file name may contain `#` too. `None`: open `path` as it is.
-pub(crate) fn split(path: &str, is_file: impl Fn(&str) -> bool) -> Option<(String, RemoteDestination)> {
+pub fn split(path: &str, is_file: impl Fn(&str) -> bool) -> Option<(String, RemoteDestination)> {
     if is_file(path) {
         return None;
     }

crates/ui-egui/src/app/files.rs

@@ -535,7 +535,7 @@ impl PdfCraftApp {
     pub(crate) fn open_path_then(&mut self, path: &str, then: Option<&'static str>) -> OpenOutcome {
         // `manual.pdf#nameddest=Intro`, as a help system starts a PDF reader (#3425): the file,
         // shown at that destination once open. A path that names a file is never split.
-        let parameters = open_parameters::split(path, |p| std::path::Path::new(p).is_file());
+        let parameters = pdfcraft_engine::open_parameters::split(path, |p| std::path::Path::new(p).is_file());
         let (path, after_open) = match &parameters {
             Some((file, destination)) => (file.as_str(), Some(destination.clone())),
             None => (path, None),

crates/ui-egui/src/lib.rs

@@ -113,8 +113,6 @@ pub use platform::file_watch;
 use platform::macos_association;
 pub use platform::motion;
 #[cfg(not(target_arch = "wasm32"))]
-use platform::open_parameters;
-#[cfg(not(target_arch = "wasm32"))]
 use platform::open_worker;
 #[cfg(not(target_arch = "wasm32"))]
 use platform::pickers;

crates/ui-egui/src/platform.rs

@@ -5,8 +5,6 @@ pub mod file_watch;
 pub(super) mod macos_association;
 pub mod motion;
 #[cfg(not(target_arch = "wasm32"))]
-pub(super) mod open_parameters;
-#[cfg(not(target_arch = "wasm32"))]
 pub(super) mod open_worker;
 #[cfg(not(target_arch = "wasm32"))]
 pub(super) mod pickers;