From 509e250c4fb56dbcddf58ef4f3b61232c6541f15 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 28 Oct 2026 09:37:46 -0700 Subject: [PATCH] Scripts: leave a script out of Recent Script Files when its path can't be made absolute Services::full_path now returns None when the working directory can't be read, instead of the relative path. file.runScript still runs the script but does not add it to the list, so a later working directory can't make the entry open a different file. --- apps/effectcraft-web/src/files.rs | 4 ++-- crates/engine/src/cmd/file_more.rs | 6 +++-- crates/engine/src/cmd/scripts.rs | 35 +++++++++++++++++++++++++++--- crates/engine/src/services.rs | 7 +++--- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/apps/effectcraft-web/src/files.rs b/apps/effectcraft-web/src/files.rs index a6770de71..f6aac5aa4 100644 --- a/apps/effectcraft-web/src/files.rs +++ b/apps/effectcraft-web/src/files.rs @@ -152,8 +152,8 @@ impl Services for WebServices { false } /// The table's keys are the paths themselves. - fn full_path(&self, path: &str) -> String { - path.to_string() + fn full_path(&self, path: &str) -> Option { + Some(path.to_string()) } } diff --git a/crates/engine/src/cmd/file_more.rs b/crates/engine/src/cmd/file_more.rs index 5264ba7a8..c9d581581 100644 --- a/crates/engine/src/cmd/file_more.rs +++ b/crates/engine/src/cmd/file_more.rs @@ -465,7 +465,9 @@ fn run_script(s: &mut Session, p: &Value) -> Result { let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); let out = run_js(s, &code, path, false)?; let result = report_js(s, &name, out)?; - remember_script(s, crate::prefs::RecentScript::Path(full)); + if let Some(full) = full { + remember_script(s, crate::prefs::RecentScript::Path(full)); + } return Ok(result); } let mut script_path = None; @@ -473,7 +475,7 @@ fn run_script(s: &mut Session, p: &Value) -> Result { (Some(Value::Array(a)), _) => a.clone(), (_, Some(path)) => { let bytes = s.services.read_file(path).map_err(|e| EngineError::Other(format!("cannot read {path}: {e}")))?; - script_path = Some(s.services.full_path(path)); + script_path = s.services.full_path(path); let text = String::from_utf8_lossy(&bytes).to_string(); match serde_json::from_str::(&text) { Ok(Value::Array(a)) => a, diff --git a/crates/engine/src/cmd/scripts.rs b/crates/engine/src/cmd/scripts.rs index 0f1f377a5..635429d6b 100644 --- a/crates/engine/src/cmd/scripts.rs +++ b/crates/engine/src/cmd/scripts.rs @@ -661,11 +661,40 @@ mod tests { fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { std::fs::write(self.at(path), data) } - fn full_path(&self, path: &str) -> String { - self.at(path).to_string_lossy().into_owned() + fn full_path(&self, path: &str) -> Option { + Some(self.at(path).to_string_lossy().into_owned()) } } + /// Reads from a folder but can't make paths absolute. + struct NoFullPath(PathBuf); + + impl crate::Services for NoFullPath { + fn read_file(&self, path: &str) -> std::io::Result> { + std::fs::read(self.0.join(path)) + } + fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + std::fs::write(self.0.join(path), data) + } + fn full_path(&self, _path: &str) -> Option { + None + } + } + + /// A script whose path can't be made absolute still runs, and Recent Script Files does not + /// keep its relative path. + #[test] + fn unresolved_script_paths_are_not_remembered() { + let (mut s, root) = session_in_tmp("recent-unresolved"); + std::fs::write(root.join("Tool.jsx"), "a").unwrap(); + std::fs::write(root.join("steps.jsonl"), "[]").unwrap(); + s.services = Arc::new(NoFullPath(root.clone())); + assert_eq!(s.execute("file.runScript", json!({"path": "Tool.jsx"})).unwrap()["ran"], "a"); + s.execute("file.runScript", json!({"path": "steps.jsonl"})).unwrap(); + assert!(s.prefs.recent_scripts.is_empty(), "{:?}", s.prefs.recent_scripts); + let _ = std::fs::remove_dir_all(&root); + } + /// A script run from a relative path runs the same file again from Recent Script Files after /// the working directory changes, and two scripts with the same file name show their paths. #[test] @@ -694,7 +723,7 @@ mod tests { #[test] fn native_full_paths_are_absolute() { use crate::Services; - assert!(std::path::Path::new(&crate::FsServices.full_path("Tool.jsx")).is_absolute()); + assert!(crate::FsServices.full_path("Tool.jsx").is_some_and(|p| std::path::Path::new(&p).is_absolute())); } #[test] diff --git a/crates/engine/src/services.rs b/crates/engine/src/services.rs index a8ee48060..6fb390ef4 100644 --- a/crates/engine/src/services.rs +++ b/crates/engine/src/services.rs @@ -39,9 +39,10 @@ pub trait Services: Send + Sync { } /// `path` as a list of recent files keeps it (File ▸ Scripts ▸ Recent Script Files): a /// relative path is made absolute against the working directory, so it opens the same file - /// after the working directory changes. - fn full_path(&self, path: &str) -> String { - std::path::absolute(path).map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|_| path.to_string()) + /// after the working directory changes. `None` when the path can't be made absolute (the + /// working directory can't be read); the script is then not added to the list. + fn full_path(&self, path: &str) -> Option { + std::path::absolute(path).ok().map(|p| p.to_string_lossy().into_owned()) } }