automation: bookmark_import refuses a node key it does not know

node_of read only the keys it knew and ignored the rest, so a misspelt
key such as "colour" was dropped and the bookmark imported without its
colour and without an error. A node may now carry only the eight keys
bookmark_export writes (title, page, view, color, bold, italic, open,
children); any other key refuses the whole import, naming the node's
path and the key, before anything is written. The tool description
says so.

Test: a_misspelt_bookmark_key_is_refused_not_dropped puts "colour" on
a nested node and checks the import is refused and the outline left
empty; it fails with the check reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9
This commit is contained in:
KenM76andClaude Opus 5.5 committed 2026-11-04 07:10:55 -05:00
1 parent 5bbcb7a218
commit e79b559455
3 files changed
+21 -1

No files matched your search

+6
View File
@@ -12,6 +12,9 @@ use serde_json::{Value, json};
/// Deepest nesting a bookmark tree may have (the listing reaches 33 levels).
const MAX_DEPTH: usize = 33;
/// The keys a node may have: the ones [`node_json`] writes.
const NODE_KEYS: [&str; 8] = ["title", "page", "view", "color", "bold", "italic", "open", "children"];
impl Automation {
pub(crate) fn bookmark_export(&self, a: &Args) -> Result<Value> {
let id = self.doc(a)?.id;
@@ -134,6 +137,9 @@ fn list_of(list: &Value, at: &[usize], count: &mut usize) -> Result<Vec<OutlineN
/// One node and, through [`list_of`], its children (recursion bounded by `MAX_DEPTH`).
fn node_of(v: &Value, at: &[usize], count: &mut usize) -> Result<OutlineNode> {
let o = v.as_object().ok_or_else(|| bad(at, "must be an object"))?;
if let Some(k) = o.keys().find(|k| !NODE_KEYS.contains(&k.as_str())) {
return Err(bad(at, &format!("unknown key {k:?} (a bookmark has {})", NODE_KEYS.join(", "))));
}
let field = |k: &str| o.get(k).filter(|x| !x.is_null());
// An empty title is kept: files have them, and export writes them as "".
let title = field("title").and_then(Value::as_str).ok_or_else(|| bad(at, "needs a title"))?.to_string();
+1 -1
View File
@@ -336,7 +336,7 @@ pub fn tools() -> Vec<ToolDef> {
t(
"bookmark_import",
"Import bookmarks",
"Add a tree of bookmarks, as bookmark_export gives it (bookmarks) or as a JSON file it wrote (file), under a parent bookmark (or at the top level) at a position, or with replace in place of the whole outline. A bookmark whose page this document lacks is kept without a destination and listed in unmapped (1-based paths into the given tree). A view takes exactly its type's values (XYZ 3, FitR 4, FitH/FitV/FitBH/FitBV 1, Fit/FitB none). Give exactly one of bookmarks and file. A title may be empty but not missing. Every node is checked before anything is written: at most 100000 bookmarks, nested at most 33 levels counting the parent's. truncated is true when the file says its export left bookmarks out. Undoable.",
"Add a tree of bookmarks, as bookmark_export gives it (bookmarks) or as a JSON file it wrote (file), under a parent bookmark (or at the top level) at a position, or with replace in place of the whole outline. A bookmark whose page this document lacks is kept without a destination and listed in unmapped (1-based paths into the given tree). A view takes exactly its type's values (XYZ 3, FitR 4, FitH/FitV/FitBH/FitBV 1, Fit/FitB none). Give exactly one of bookmarks and file. A title may be empty but not missing; a key a node does not have is refused. Every node is checked before anything is written: at most 100000 bookmarks, nested at most 33 levels counting the parent's. truncated is true when the file says its export left bookmarks out. Undoable.",
)
.destructive()
.with(schema(
+14
View File
@@ -10742,6 +10742,20 @@ fn odd_bookmarks_round_trip_and_a_cut_tree_says_so() {
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn a_misspelt_bookmark_key_is_refused_not_dropped() {
let dir = workdir("bookmark-io-key");
let mut a = auto(&dir);
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
let tree = json!([{ "title": "Parent", "children": [{ "title": "Red", "page": 1, "colour": [1, 0, 0] }] }]);
match a.call("bookmark_import", &json!({ "doc": doc, "bookmarks": tree })) {
Err(ToolError::InvalidArgs(m)) => assert!(m.contains("[1, 1]") && m.contains("\"colour\""), "{m}"),
other => panic!("expected InvalidArgs, got {other:?}"),
}
assert_eq!(ok(&mut a, "bookmark_list", json!({ "doc": doc }))["bookmarks"], json!([]));
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn a_bad_bookmark_tree_is_refused_whole_and_names_the_node() {
let dir = workdir("bookmark-io-bad");