fix(automation): validate doc_close discard_changes (M13) (#306)

Validate the optional flag for clean documents before closing them. Add
regressions for invalid types, document preservation, and valid defaults.

AI-generated by Codex (GPT-6), with GPT-6.1 Sol and GPT-6 Astra agents.

Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
sanyamsingh001
2026-10-08 13:05:43 -04:00
committed by GitHub
co-authored by Codex Brandon Thomas
parent edce549a32
commit d770f649b4
2 changed files with 106 additions and 1 deletions
+2 -1
View File
@@ -621,7 +621,8 @@ impl Automation {
fn doc_close(&mut self, a: &Args) -> Result<Value> {
let doc = self.doc(a)?;
let id = doc.id;
if doc.dirty && !a.opt_bool("discard_changes")?.unwrap_or(false) {
let discard_changes = a.opt_bool("discard_changes")?.unwrap_or(false);
if doc.dirty && !discard_changes {
return Err(failed("the document has unsaved changes: save it with doc_save, or pass discard_changes: true"));
}
self.session.close(id);
+104
View File
@@ -2209,3 +2209,107 @@ fn doc_info_describes_set_layer_links() {
})
);
}
mod close_argument_tests {
use super::*;
use std::sync::atomic::{AtomicU64, Ordering};
// Create exclusively and remove only this test's directory, never a pre-existing one.
struct CloseDir(PathBuf);
impl CloseDir {
fn new() -> Self {
static NEXT: AtomicU64 = AtomicU64::new(0);
let parent = std::env::temp_dir();
for _ in 0..128 {
let serial = NEXT.fetch_add(1, Ordering::Relaxed);
let path = parent.join(format!("pdfcraft-close-{}-{serial}", std::process::id()));
match std::fs::create_dir(&path) {
Ok(()) => {
let dir = Self(path);
std::fs::write(dir.0.join("a.pdf"), fixture(3)).unwrap();
std::fs::write(dir.0.join("b.pdf"), fixture(2)).unwrap();
return dir;
}
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(error) => panic!("creating close test directory: {error}"),
}
}
panic!("no unused close test directory after 128 attempts");
}
}
impl Drop for CloseDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
#[test]
fn close_rejects_invalid_discard_types_without_changing_clean_or_dirty_documents() {
let dir = CloseDir::new();
let source = std::fs::read(dir.0.join("a.pdf")).unwrap();
let other_source = std::fs::read(dir.0.join("b.pdf")).unwrap();
let mut a = auto(&dir.0);
let other = ok(&mut a, "doc_open", json!({ "path": "b.pdf" }))["doc"].as_u64().unwrap();
for dirty in [false, true] {
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
if dirty {
ok(&mut a, "doc_set_info", json!({ "doc": doc, "key": "Title", "value": "Unsaved title" }));
}
let list = ok(&mut a, "doc_list", json!({}));
let info = ok(&mut a, "doc_info", json!({ "doc": doc }));
let text = ok(&mut a, "text_extract", json!({ "doc": doc }));
let bytes = a.session().get(pdfcraft_engine::DocId(doc)).unwrap().bytes.clone();
for invalid in [json!("false"), json!(0), json!([]), json!({})] {
let error = a.call("doc_close", &json!({ "doc": doc, "discard_changes": invalid })).unwrap_err();
assert!(matches!(error, ToolError::InvalidArgs(ref message) if message == "discard_changes must be true or false"));
assert_eq!(ok(&mut a, "doc_list", json!({})), list, "all document identities, order, paths and history stay unchanged");
assert_eq!(ok(&mut a, "doc_info", json!({ "doc": doc })), info);
assert_eq!(ok(&mut a, "text_extract", json!({ "doc": doc })), text);
let current = a.session().get(pdfcraft_engine::DocId(doc)).unwrap();
assert_eq!(current.bytes, bytes);
assert_eq!(current.dirty, dirty);
assert_eq!(std::fs::read(dir.0.join("a.pdf")).unwrap(), source);
assert_eq!(std::fs::read(dir.0.join("b.pdf")).unwrap(), other_source);
}
assert_eq!(ok(&mut a, "doc_close", json!({ "doc": doc, "discard_changes": true }))["closed"], doc);
assert!(a.session().get(pdfcraft_engine::DocId(doc)).is_none());
assert!(a.session().get(pdfcraft_engine::DocId(other)).is_some());
}
}
#[test]
fn close_keeps_default_and_boolean_discard_controls() {
let dir = CloseDir::new();
let source = std::fs::read(dir.0.join("a.pdf")).unwrap();
let mut a = auto(&dir.0);
let other = ok(&mut a, "doc_open", json!({ "path": "b.pdf" }))["doc"].as_u64().unwrap();
for discard in [None, Some(Value::Null), Some(json!(false)), Some(json!(true))] {
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
let mut args = json!({ "doc": doc });
if let Some(discard) = discard {
args["discard_changes"] = discard;
}
assert_eq!(ok(&mut a, "doc_close", args)["closed"], doc);
assert!(a.session().get(pdfcraft_engine::DocId(doc)).is_none());
assert!(a.session().get(pdfcraft_engine::DocId(other)).is_some());
}
let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
ok(&mut a, "doc_set_info", json!({ "doc": doc, "key": "Title", "value": "Unsaved title" }));
let before = ok(&mut a, "doc_list", json!({}));
let info = ok(&mut a, "doc_info", json!({ "doc": doc }));
let bytes = a.session().get(pdfcraft_engine::DocId(doc)).unwrap().bytes.clone();
for args in [json!({ "doc": doc }), json!({ "doc": doc, "discard_changes": null }), json!({ "doc": doc, "discard_changes": false })] {
let error = a.call("doc_close", &args).unwrap_err();
assert!(matches!(error, ToolError::Failed(ref message) if message.contains("unsaved changes")));
assert_eq!(ok(&mut a, "doc_list", json!({})), before);
assert_eq!(ok(&mut a, "doc_info", json!({ "doc": doc })), info);
assert_eq!(a.session().get(pdfcraft_engine::DocId(doc)).unwrap().bytes, bytes);
}
assert_eq!(ok(&mut a, "doc_close", json!({ "doc": doc, "discard_changes": true }))["closed"], doc);
assert!(a.session().get(pdfcraft_engine::DocId(doc)).is_none());
assert!(a.session().get(pdfcraft_engine::DocId(other)).is_some());
assert_eq!(std::fs::read(dir.0.join("a.pdf")).unwrap(), source);
}
}