Merge pull request #21 from storytold/pr/never-crash-automation

Never crash: the automation (MCP/script) tools have no expect/unreachable

crates/automation/src/comments.rs

@@ -305,7 +305,9 @@ impl Automation {
         let author = a.opt_str("author")?.unwrap_or(DEFAULT_AUTHOR).to_string();
         // Replace Text: the struck-out text and a grouped caret holding `contents`.
         if kind == "replace" {
-            let Shape::TextMarkup { quads, .. } = shape else { unreachable!("replace marks text") };
+            let Shape::TextMarkup { quads, .. } = shape else {
+                return Err(ToolError::InvalidArgs("replace marks text: give it text to replace".into()));
+            };
             if contents.trim().is_empty() {
                 return Err(ToolError::InvalidArgs("replace needs `contents`: the replacement text".into()));
             }

crates/automation/src/forms.rs

@@ -196,7 +196,8 @@ impl Automation {
                     "read_only": f.read_only(),
                     "required": f.has(field_flags::REQUIRED),
                 });
-                let o = v.as_object_mut().expect("object");
+                let mut more = serde_json::Map::new();
+                let o = &mut more;
                 if let Some(t) = &f.tooltip {
                     o.insert("tooltip".into(), json!(t));
                 }
@@ -236,6 +237,9 @@ impl Automation {
                     }
                     _ => {}
                 }
+                if let Value::Object(m) = &mut v {
+                    m.extend(more);
+                }
                 v
             })
             .collect();

crates/automation/src/lib.rs

@@ -9,6 +9,8 @@
 //!   with the origin at the top-left of the displayed page.
 //! - An optional root directory confines every path a tool reads or writes.
 
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
+
 mod a11y;
 mod comments;
 mod content;
@@ -1204,12 +1206,15 @@ impl Automation {
 
     fn renderer(&mut self, id: DocId) -> Result<&mut PageRenderer> {
         let doc = self.session.get(id).ok_or_else(|| failed("no such document"))?;
-        let stale = self.renderers.get(&id).is_none_or(|(b, _)| !Arc::ptr_eq(b, &doc.bytes));
-        if stale {
+        let fresh = || {
             let config = RenderConfig { password: doc.password.as_deref().map(Arc::from), ..Default::default() };
-            self.renderers.insert(id, (doc.bytes.clone(), PageRenderer::new(doc.bytes.clone(), config)));
+            (doc.bytes.clone(), PageRenderer::new(doc.bytes.clone(), config))
+        };
+        let entry = self.renderers.entry(id).or_insert_with(fresh);
+        if !Arc::ptr_eq(&entry.0, &doc.bytes) {
+            *entry = fresh();
         }
-        Ok(&mut self.renderers.get_mut(&id).expect("inserted above").1)
+        Ok(&mut entry.1)
     }
 
     fn page_render(&mut self, a: &Args) -> Result<Content> {
@@ -1245,13 +1250,16 @@ impl Automation {
         if !Arc::ptr_eq(&entry.0, &bytes) || entry.1.len() != n {
             *entry = (bytes.clone(), vec![None; n]);
         }
-        let mut missing: Vec<usize> = pages.iter().copied().filter(|p| entry.1[*p].is_none()).collect();
+        let mut missing: Vec<usize> = pages.iter().copied().filter(|p| entry.1.get(*p).is_some_and(Option::is_none)).collect();
         missing.sort_unstable();
         missing.dedup();
         for (p, text) in extract_parallel(&bytes, password, &missing) {
-            entry.1[p] = Some(Arc::new(text.map_err(|e| failed(format!("page {}: {e}", p + 1)))?));
+            let text = Arc::new(text.map_err(|e| failed(format!("page {}: {e}", p + 1)))?);
+            if let Some(slot) = entry.1.get_mut(p) {
+                *slot = Some(text);
+            }
         }
-        Ok(pages.iter().map(|p| entry.1[*p].clone().expect("extracted above")).collect())
+        pages.iter().map(|p| entry.1.get(*p).cloned().flatten().ok_or_else(|| failed(format!("page {} has no text", p + 1)))).collect()
     }
 
     fn text_extract(&mut self, a: &Args) -> Result<Value> {

crates/automation/src/redact.rs

@@ -73,7 +73,10 @@ impl Automation {
                 Edit::AddAnnotation(NewAnnotation { page: *page, shape, style: style.clone(), contents: String::new(), author: author.clone() })
             })
             .collect();
-        let edit = if edits.len() == 1 { edits.into_iter().next().expect("one") } else { Edit::Batch { label: "Mark for redaction".into(), edits } };
+        let edit = match <[Edit; 1]>::try_from(edits) {
+            Ok([one]) => one,
+            Err(edits) => Edit::Batch { label: "Mark for redaction".into(), edits },
+        };
         let mut out = self.apply(a, edit)?;
         out["marked"] = json!(count);
         out["pages_marked"] = json!(marks.iter().map(|m| m.0 + 1).collect::<std::collections::BTreeSet<_>>());

crates/automation/src/tools.rs

@@ -48,11 +48,15 @@ fn color() -> Value {
 
 /// Properties that pick one comment: its `id` (from comment_list), or `page` + `index`.
 fn comment_ref(mut extra: Value) -> Value {
-    let props = extra.as_object_mut().expect("an object");
-    props.insert("doc".into(), doc());
-    props.insert("id".into(), json!({ "type": "string", "description": "The comment's id (from comment_list)." }));
-    props.insert("page".into(), json!({ "type": "integer", "minimum": 1, "description": "With `index`, instead of `id`." }));
-    props.insert("index".into(), json!({ "type": "integer", "minimum": 1, "description": "1-based position on the page, as comment_list reports." }));
+    if let Some(props) = extra.as_object_mut() {
+        props.insert("doc".into(), doc());
+        props.insert("id".into(), json!({ "type": "string", "description": "The comment's id (from comment_list)." }));
+        props.insert("page".into(), json!({ "type": "integer", "minimum": 1, "description": "With `index`, instead of `id`." }));
+        props.insert(
+            "index".into(),
+            json!({ "type": "integer", "minimum": 1, "description": "1-based position on the page, as comment_list reports." }),
+        );
+    }
     extra
 }