Release 0.2.0 #4
@@ -183,9 +183,10 @@ impl Tool for EditFile {
|
|||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str()
|
let user_path = args["path"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let content = read_to_string(user_path)?;
|
let content = read_to_string(user_path)?;
|
||||||
let updated = apply_edit(&content, &args, user_path)?;
|
let updated = apply_edit(&content, &args, display)?;
|
||||||
write_string(user_path, &updated)?;
|
write_string(user_path, &updated)?;
|
||||||
Ok(format!("Edited {user_path}."))
|
Ok(format!("Edited {display}."))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -108,6 +108,7 @@ impl Tool for GrepFiles {
|
|||||||
|
|
||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str().ok_or_else(|| anyhow::anyhow!("Missing: path"))?;
|
let user_path = args["path"].as_str().ok_or_else(|| anyhow::anyhow!("Missing: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let pattern = args["pattern"].as_str().ok_or_else(|| anyhow::anyhow!("Missing: pattern"))?;
|
let pattern = args["pattern"].as_str().ok_or_else(|| anyhow::anyhow!("Missing: pattern"))?;
|
||||||
let case_sensitive = args["case_sensitive"].as_bool().unwrap_or(false);
|
let case_sensitive = args["case_sensitive"].as_bool().unwrap_or(false);
|
||||||
let include_glob = args["include_glob"].as_str();
|
let include_glob = args["include_glob"].as_str();
|
||||||
@@ -123,7 +124,7 @@ impl Tool for GrepFiles {
|
|||||||
let glob_pattern = include_glob.and_then(|g| glob::Pattern::new(g).ok());
|
let glob_pattern = include_glob.and_then(|g| glob::Pattern::new(g).ok());
|
||||||
let root = resolve(user_path)?;
|
let root = resolve(user_path)?;
|
||||||
if !root.exists() {
|
if !root.exists() {
|
||||||
anyhow::bail!("Path not found: {user_path}");
|
anyhow::bail!("Path not found: {display}");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Walkers emit absolute paths (the `path` arg is resolved to an absolute working
|
// Walkers emit absolute paths (the `path` arg is resolved to an absolute working
|
||||||
@@ -138,7 +139,7 @@ impl Tool for GrepFiles {
|
|||||||
collect_matching_files(&root, &re, &glob_pattern, max_results + offset, &mut files)?;
|
collect_matching_files(&root, &re, &glob_pattern, max_results + offset, &mut files)?;
|
||||||
let files: Vec<String> = files.into_iter().skip(offset).take(max_results).map(rel).collect();
|
let files: Vec<String> = files.into_iter().skip(offset).take(max_results).map(rel).collect();
|
||||||
if files.is_empty() {
|
if files.is_empty() {
|
||||||
return Ok(format!("No files match {:?} in {user_path}.", pattern));
|
return Ok(format!("No files match {:?} in {display}.", pattern));
|
||||||
}
|
}
|
||||||
Ok(format!("{} file(s):\n{}", files.len(), files.join("\n")))
|
Ok(format!("{} file(s):\n{}", files.len(), files.join("\n")))
|
||||||
}
|
}
|
||||||
@@ -147,7 +148,7 @@ impl Tool for GrepFiles {
|
|||||||
collect_match_counts(&root, &re, &glob_pattern, max_results + offset, &mut counts)?;
|
collect_match_counts(&root, &re, &glob_pattern, max_results + offset, &mut counts)?;
|
||||||
let counts: Vec<(String, usize)> = counts.into_iter().skip(offset).take(max_results).collect();
|
let counts: Vec<(String, usize)> = counts.into_iter().skip(offset).take(max_results).collect();
|
||||||
if counts.is_empty() {
|
if counts.is_empty() {
|
||||||
return Ok(format!("No matches for {:?} in {user_path}.", pattern));
|
return Ok(format!("No matches for {:?} in {display}.", pattern));
|
||||||
}
|
}
|
||||||
let lines: Vec<String> = counts.into_iter().map(|(f, n)| format!("{}: {n}", rel(f))).collect();
|
let lines: Vec<String> = counts.into_iter().map(|(f, n)| format!("{}: {n}", rel(f))).collect();
|
||||||
Ok(format!("{} file(s):\n{}", lines.len(), lines.join("\n")))
|
Ok(format!("{} file(s):\n{}", lines.len(), lines.join("\n")))
|
||||||
@@ -160,7 +161,7 @@ impl Tool for GrepFiles {
|
|||||||
|
|
||||||
let matches: Vec<String> = matches.into_iter().skip(offset).take(max_results).map(rel).collect();
|
let matches: Vec<String> = matches.into_iter().skip(offset).take(max_results).map(rel).collect();
|
||||||
if matches.is_empty() {
|
if matches.is_empty() {
|
||||||
return Ok(format!("No matches for {:?} in {user_path}.", pattern));
|
return Ok(format!("No matches for {:?} in {display}.", pattern));
|
||||||
}
|
}
|
||||||
let mut out = format!("{} match(es):\n", matches.len());
|
let mut out = format!("{} match(es):\n", matches.len());
|
||||||
out.push_str(&matches.join("\n"));
|
out.push_str(&matches.join("\n"));
|
||||||
|
|||||||
@@ -120,8 +120,9 @@ impl Tool for InsertAtLine {
|
|||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str()
|
let user_path = args["path"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let text = read_to_string(user_path)?;
|
let text = read_to_string(user_path)?;
|
||||||
let (updated, msg) = apply_insert(&text, &args, user_path)?;
|
let (updated, msg) = apply_insert(&text, &args, display)?;
|
||||||
write_string(user_path, &updated)?;
|
write_string(user_path, &updated)?;
|
||||||
Ok(msg)
|
Ok(msg)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -238,12 +238,32 @@ pub fn resolve_view_path(fs: &UserFs, input: &str) -> Result<(PathBuf, String)>
|
|||||||
/// Rewrites the `path` argument of a physical fs-tool call to the resolved absolute
|
/// Rewrites the `path` argument of a physical fs-tool call to the resolved absolute
|
||||||
/// host path, so the on-disk `execute` (which takes absolute paths as-is) acts on
|
/// host path, so the on-disk `execute` (which takes absolute paths as-is) acts on
|
||||||
/// the caller's per-user workspace rather than the process working directory.
|
/// the caller's per-user workspace rather than the process working directory.
|
||||||
|
///
|
||||||
|
/// The caller's agent-visible path is stashed under [`DISPLAY_PATH_KEY`] so `execute`
|
||||||
|
/// can show it in its messages — the model must never see the host path. This key is
|
||||||
|
/// never persisted: tool args are logged from `call.arguments` *before* `run_with`
|
||||||
|
/// rewrites them, and tool results are plain strings.
|
||||||
pub(crate) fn rewrite_to_host(fs: &UserFs, agent_path: &str, mut args: Value) -> Result<Value> {
|
pub(crate) fn rewrite_to_host(fs: &UserFs, agent_path: &str, mut args: Value) -> Result<Value> {
|
||||||
let host = resolve_host_path(fs, agent_path)?;
|
let host = resolve_host_path(fs, agent_path)?;
|
||||||
|
args[DISPLAY_PATH_KEY] = Value::String(agent_path.to_string());
|
||||||
args["path"] = Value::String(host.to_string_lossy().into_owned());
|
args["path"] = Value::String(host.to_string_lossy().into_owned());
|
||||||
Ok(args)
|
Ok(args)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Private stash key for the agent-visible path, set by [`rewrite_to_host`] alongside
|
||||||
|
/// the host path in `path`.
|
||||||
|
const DISPLAY_PATH_KEY: &str = "__display_path";
|
||||||
|
|
||||||
|
/// The path to show in user-facing messages: the agent-visible path stashed by
|
||||||
|
/// [`rewrite_to_host`] when present, falling back to `path` itself for the
|
||||||
|
/// context-free legacy path (where `path` was never rewritten and is already the
|
||||||
|
/// agent path).
|
||||||
|
pub(crate) fn display_path_arg(args: &Value) -> &str {
|
||||||
|
args.get(DISPLAY_PATH_KEY).and_then(Value::as_str)
|
||||||
|
.or_else(|| args.get("path").and_then(Value::as_str))
|
||||||
|
.unwrap_or("")
|
||||||
|
}
|
||||||
|
|
||||||
/// A tool execution that fails immediately — surfaces a containment / access error
|
/// A tool execution that fails immediately — surfaces a containment / access error
|
||||||
/// from `run_with` without attempting a disk op.
|
/// from `run_with` without attempting a disk op.
|
||||||
pub(crate) fn error_exec<'a>(msg: String) -> Box<dyn ToolExecution + 'a> {
|
pub(crate) fn error_exec<'a>(msg: String) -> Box<dyn ToolExecution + 'a> {
|
||||||
@@ -597,4 +617,53 @@ mod tests {
|
|||||||
let _ = std::fs::remove_dir_all(&udir);
|
let _ = std::fs::remove_dir_all(&udir);
|
||||||
let _ = std::fs::remove_dir_all(&sdir);
|
let _ = std::fs::remove_dir_all(&sdir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Physical-path fs tools must report the **agent-visible** path in every
|
||||||
|
/// message they return — never the resolved host path. The agent's virtual
|
||||||
|
/// namespace (`~/…`, `shared/…`, `projects/…`) is all it should ever see;
|
||||||
|
/// the host workspace location is an internal detail. Regression for the
|
||||||
|
/// host-path leak that `rewrite_to_host` introduced into `execute`'s output.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn physical_fs_tools_show_agent_path_not_host() {
|
||||||
|
let (shared, sdir) = store("phys-shared").await;
|
||||||
|
let (user, udir) = store("phys-user").await;
|
||||||
|
|
||||||
|
let root = std::env::temp_dir().join(format!("skald-phys-{}", uuid::Uuid::new_v4()));
|
||||||
|
let home = root.join("homes").join("u1");
|
||||||
|
std::fs::create_dir_all(&home).unwrap();
|
||||||
|
|
||||||
|
let fs = Arc::new(UserFs::new(
|
||||||
|
"u1", home.clone(), "skald-u1", PathBuf::from("/root"), vec![], vec![], None,
|
||||||
|
));
|
||||||
|
let ctx = ToolContext { session_id: 1, user_id: "u1".into(), pool: Arc::clone(&user), fs };
|
||||||
|
let write = WriteFile::new(Arc::clone(&shared));
|
||||||
|
let edit = EditFile::new(Arc::clone(&shared));
|
||||||
|
let grep = GrepFiles::new();
|
||||||
|
|
||||||
|
// `/homes/u1` only ever appears in the resolved host path, never in the
|
||||||
|
// agent namespace — so it is a robust, OS-independent leak detector.
|
||||||
|
let leak_marker = "/homes/u1";
|
||||||
|
|
||||||
|
// write_file success → "Created ~/notes.md", never the host home.
|
||||||
|
let out = drive(&write, &ctx, json!({"path":"~/notes.md","content":"hello\nworld"}))
|
||||||
|
.await.unwrap();
|
||||||
|
assert!(out.contains("~/notes.md"), "agent path missing: {out}");
|
||||||
|
assert!(!out.contains(leak_marker), "host path leaked into write_file result: {out}");
|
||||||
|
|
||||||
|
// edit_file failure → the error names the agent path, never the host path.
|
||||||
|
let err = drive(&edit, &ctx, json!({"path":"~/notes.md","old":"nope","new":"x"}))
|
||||||
|
.await.unwrap_err();
|
||||||
|
assert!(err.contains("~/notes.md"), "agent path missing from error: {err}");
|
||||||
|
assert!(!err.contains(leak_marker), "host path leaked into edit_file error: {err}");
|
||||||
|
|
||||||
|
// grep_files no-match → "in ~/notes.md", never the host path.
|
||||||
|
let out = drive(&grep, &ctx, json!({"path":"~/notes.md","pattern":"zzz"}))
|
||||||
|
.await.unwrap();
|
||||||
|
assert!(out.contains("~/notes.md"), "agent path missing from grep: {out}");
|
||||||
|
assert!(!out.contains(leak_marker), "host path leaked into grep result: {out}");
|
||||||
|
|
||||||
|
let _ = std::fs::remove_dir_all(&root);
|
||||||
|
let _ = std::fs::remove_dir_all(&udir);
|
||||||
|
let _ = std::fs::remove_dir_all(&sdir);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -125,8 +125,9 @@ impl Tool for ReplaceLines {
|
|||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str()
|
let user_path = args["path"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let content = read_to_string(user_path)?;
|
let content = read_to_string(user_path)?;
|
||||||
let (updated, msg) = apply_replace(&content, &args, user_path)?;
|
let (updated, msg) = apply_replace(&content, &args, display)?;
|
||||||
write_string(user_path, &updated)?;
|
write_string(user_path, &updated)?;
|
||||||
Ok(msg)
|
Ok(msg)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -136,7 +136,8 @@ impl Tool for SearchFile {
|
|||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str()
|
let user_path = args["path"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let text = read_to_string(user_path)?;
|
let text = read_to_string(user_path)?;
|
||||||
render_search(&text, &args, user_path)
|
render_search(&text, &args, display)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -92,6 +92,7 @@ impl Tool for WriteFile {
|
|||||||
fn execute(&self, args: Value) -> Result<String> {
|
fn execute(&self, args: Value) -> Result<String> {
|
||||||
let user_path = args["path"].as_str()
|
let user_path = args["path"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: path"))?;
|
||||||
|
let display = super::display_path_arg(&args);
|
||||||
let content = args["content"].as_str()
|
let content = args["content"].as_str()
|
||||||
.ok_or_else(|| anyhow::anyhow!("Missing required argument: content"))?;
|
.ok_or_else(|| anyhow::anyhow!("Missing required argument: content"))?;
|
||||||
|
|
||||||
@@ -100,9 +101,9 @@ impl Tool for WriteFile {
|
|||||||
write_string(user_path, content)?;
|
write_string(user_path, content)?;
|
||||||
|
|
||||||
if existed {
|
if existed {
|
||||||
Ok(format!("Overwrote {user_path} ({} bytes).", content.len()))
|
Ok(format!("Overwrote {display} ({} bytes).", content.len()))
|
||||||
} else {
|
} else {
|
||||||
Ok(format!("Created {user_path} ({} bytes).", content.len()))
|
Ok(format!("Created {display} ({} bytes).", content.len()))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user