From aed3afdc2094a06fa715a95b8d22579b8965280e Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 15 Oct 2026 14:28:46 +0800 Subject: [PATCH] fix(automation): a missing path under --root says where it was looked for (#1890) (#1907) A relative path that doesn't exist below the root was refused with the path as typed and the system's localised message, so a script written for another working directory ("sub/a.pdf" with --root sub) gave nothing to correct from. The refusal now names the resolved path and the root; paths outside the root keep their uniform refusal. Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz Co-authored-by: Claude Fable 5.1 --- crates/automation/src/lib.rs | 13 +++++++++++-- crates/automation/tests/automation.rs | 25 ++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index d313ee835..7b8e166d8 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -1938,8 +1938,17 @@ impl Automation { return Err(outside()); } if !rest.is_empty() && !for_write { - // Missing, below a folder inside the root: say why, as the system reports it. - real = joined.canonicalize().map_err(|e| failed(format!("{path}: {e}")))?; + // Missing, below a folder inside the root: say where it was looked for, so a path + // written for another working directory can be corrected (#1890), and otherwise + // why, as the system reports it. `joined` is under the root, so naming it reveals + // nothing outside. + real = joined.canonicalize().map_err(|e| { + let (looked, root) = (joined.display(), root.display()); + match e.kind() { + std::io::ErrorKind::NotFound => failed(format!("{path} not found (looked for {looked}, --root {root})")), + _ => failed(format!("{path}: {e} (looked for {looked}, --root {root})")), + } + })?; if !real.starts_with(root) { return Err(outside()); // it appeared, as a link out, since the check above } diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index cdb11b685..e3eaf4ce5 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -857,7 +857,10 @@ fn root_refusals_do_not_reveal_what_exists_outside() { // Inside the root nothing changes: missing files say so, and existing ones open and save, // however the path is spelled. let missing = a.call("doc_open", &json!({ "path": "missing-inside.pdf" })).unwrap_err(); - assert!(matches!(&missing, ToolError::Failed(m) if m.starts_with("missing-inside.pdf: ") && !m.contains("outside")), "{missing:?}"); + assert!( + matches!(&missing, ToolError::Failed(m) if m.starts_with("missing-inside.pdf not found (looked for ") && !m.contains("outside")), + "{missing:?}" + ); let not_dir = a.call("doc_open", &json!({ "path": "inside.pdf/x" })).unwrap_err(); assert!(matches!(¬_dir, ToolError::Failed(m) if !m.contains("outside")), "{not_dir:?}"); #[cfg_attr(not(windows), allow(unused_mut))] @@ -5939,3 +5942,23 @@ fn absolute_gotor_consent_does_not_bypass_validation_or_passwords() { assert_eq!(opened["document"]["encrypted"], true); assert_eq!(opened["destination_applied"], true); } + +#[test] +fn a_missing_path_under_the_root_names_where_it_was_looked_for() { + // #1890: a script written from another working directory repeats the root's folder in its + // paths. The refusal names the resolved path and the root, so there is something to correct. + let (base, root) = sandbox("resolved-path"); + let mut a = auto(&root); + let looked = |rel: &str| root.join(rel).display().to_string(); + for (path, rel) in + [("root/inside.pdf", "root/inside.pdf"), ("missing/deeper/x.pdf", "missing/deeper/x.pdf"), ("./sub/../root/inside.pdf", "root/inside.pdf")] + { + let ToolError::Failed(m) = err(&mut a, "doc_open", json!({ "path": path })) else { panic!("{path}: not a failure") }; + assert_eq!(m, format!("{path} not found (looked for {}, --root {})", looked(rel), root.display())); + } + // The same mistake with a file that does exist outside still gets the plain refusal (#136). + let ToolError::Failed(m) = err(&mut a, "doc_open", json!({ "path": "../outside/secret.pdf" })) else { panic!("not a failure") }; + assert!(!m.contains("looked for"), "{m}"); + ok(&mut a, "doc_open", json!({ "path": "inside.pdf" })); + let _ = std::fs::remove_dir_all(base); +}