| 1 | //! Canonical action-based wrapper for file system tools. |
| 2 | //! |
| 3 | //! The model sees one tool: `File` with an `action` parameter |
| 4 | //! (read | list | search_name | search_content | write | edit | patch). |
| 5 | //! The per-action legacy execution aliases were removed in v0.9.3. |
| 6 | |
| 7 | use async_trait::async_trait; |
| 8 | use serde_json::{Value, json}; |
| 9 | |
| 10 | use super::apply_patch::ApplyPatchTool; |
| 11 | use super::canonical_action::required_action; |
| 12 | use super::file::{EditFileTool, ListDirTool, ReadFileTool, WriteFileTool}; |
| 13 | use super::file_search::FileSearchTool; |
| 14 | use super::search::GrepFilesTool; |
| 15 | use super::spec::{ |
| 16 | ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, |
| 17 | }; |
| 18 | |
| 19 | /// Lift a parameter description out of the tool that implements the action. |
| 20 | /// |
| 21 | /// Returns an empty string when the key is absent, which the |
| 22 | /// `wrapper_borrows_every_inner_description` test turns into a build-time |
| 23 | /// failure — a renamed inner parameter must not silently blank the only |
| 24 | /// description the model gets to read. |
| 25 | fn borrowed(schema: &Value, key: &str) -> String { |
| 26 | schema |
| 27 | .get("properties") |
| 28 | .and_then(|properties| properties.get(key)) |
| 29 | .and_then(|property| property.get("description")) |
| 30 | .and_then(Value::as_str) |
| 31 | .unwrap_or_default() |
| 32 | .to_string() |
| 33 | } |
| 34 | |
| 35 | /// `borrowed`, tagged with the action the parameter belongs to. |
| 36 | fn describe(schema: &Value, key: &str, action: &str) -> String { |
| 37 | let base = borrowed(schema, key); |
| 38 | if base.is_empty() { |
| 39 | return String::new(); |
| 40 | } |
| 41 | format!("{} — {action}.", base.trim_end_matches('.')) |
| 42 | } |
| 43 | |
| 44 | pub struct FileTool { |
| 45 | name: &'static str, |
| 46 | forced_action: Option<&'static str>, |
| 47 | allow_writes: bool, |
| 48 | allow_patch: bool, |
| 49 | } |
| 50 | |
| 51 | impl FileTool { |
| 52 | pub const fn new(name: &'static str) -> Self { |
| 53 | Self { |
| 54 | name, |
| 55 | forced_action: None, |
| 56 | allow_writes: true, |
| 57 | allow_patch: false, |
| 58 | } |
| 59 | } |
| 60 | |
| 61 | pub const fn with_patch(name: &'static str) -> Self { |
| 62 | Self { |
| 63 | name, |
| 64 | forced_action: None, |
| 65 | allow_writes: true, |
| 66 | allow_patch: true, |
| 67 | } |
| 68 | } |
| 69 | |
| 70 | pub const fn read_only(name: &'static str) -> Self { |
| 71 | Self { |
| 72 | name, |
| 73 | forced_action: None, |
| 74 | allow_writes: false, |
| 75 | allow_patch: false, |
| 76 | } |
| 77 | } |
| 78 | |
| 79 | pub const fn alias(name: &'static str, action: &'static str) -> Self { |
| 80 | Self { |
| 81 | name, |
| 82 | forced_action: Some(action), |
| 83 | allow_writes: true, |
| 84 | allow_patch: true, |
| 85 | } |
| 86 | } |
| 87 | |
| 88 | /// The action set this instance actually dispatches, in schema order. |
| 89 | /// |
| 90 | /// The `enum` the model reads and the name list its errors quote are both |
| 91 | /// built from this, so a mode that hides `write` can never advertise it or |
| 92 | /// suggest it in a refusal. |
| 93 | fn available_actions(&self) -> Vec<&'static str> { |
| 94 | if let Some(forced) = self.forced_action { |
| 95 | return vec![forced]; |
| 96 | } |
| 97 | let mut actions = vec!["read", "list", "search_name", "search_content"]; |
| 98 | if self.allow_writes { |
| 99 | actions.extend(["write", "edit"]); |
| 100 | } |
| 101 | if self.allow_patch { |
| 102 | actions.push("patch"); |
| 103 | } |
| 104 | actions |
| 105 | } |
| 106 | |
| 107 | /// Policy-side action resolution. |
| 108 | /// |
| 109 | /// Approval, read-only, and parallel-safety predicates cannot fail, so a |
| 110 | /// missing action resolves to the most restrictive answer they can give. |
| 111 | /// `execute` does **not** share this fallback — see `required_action`. |
| 112 | fn resolve_action<'a>(&self, input: &'a Value) -> &'a str { |
| 113 | self.forced_action.unwrap_or_else(|| { |
| 114 | input |
| 115 | .get("action") |
| 116 | .and_then(Value::as_str) |
| 117 | .unwrap_or("read") |
| 118 | }) |
| 119 | } |
| 120 | |
| 121 | /// Execution-side action resolution: `action` is required, as the schema |
| 122 | /// has always said it is. |
| 123 | fn required_action(&self, input: &Value) -> Result<String, ToolError> { |
| 124 | if let Some(forced) = self.forced_action { |
| 125 | return Ok(forced.to_string()); |
| 126 | } |
| 127 | required_action(input, self.name, &self.available_actions()) |
| 128 | } |
| 129 | |
| 130 | fn strip_action(&self, input: Value) -> Result<Value, ToolError> { |
| 131 | let mut input = input; |
| 132 | if let Some(obj) = input.as_object_mut() { |
| 133 | obj.remove("action"); |
| 134 | Ok(input) |
| 135 | } else { |
| 136 | Err(ToolError::invalid_input( |
| 137 | "File tool input must be an object", |
| 138 | )) |
| 139 | } |
| 140 | } |
| 141 | } |
| 142 | |
| 143 | #[async_trait] |
| 144 | impl ToolSpec for FileTool { |
| 145 | fn name(&self) -> &'static str { |
| 146 | self.name |
| 147 | } |
| 148 | |
| 149 | fn model_visible(&self) -> bool { |
| 150 | self.name == "File" |
| 151 | } |
| 152 | |
| 153 | fn description(&self) -> &'static str { |
| 154 | "Read, list, search, write, edit, or patch workspace files. Use read before edit; edit performs one exact replacement, while patch is best for multi-hunk or multi-file changes. Read/list/search actions are parallel-safe and do not require approval. Available actions depend on the active mode and feature policy." |
| 155 | } |
| 156 | |
| 157 | fn input_schema(&self) -> Value { |
| 158 | let actions = self.available_actions(); |
| 159 | // Every per-action parameter description is borrowed from the tool |
| 160 | // that implements the action rather than restated here. The wrapper is |
| 161 | // the only schema the model ever reads, and the restated copy had gone |
| 162 | // stale: it advertised `max_lines` "default 200" long after the real |
| 163 | // default became 500-with-a-16KB-budget, and a `blame` action `File` |
| 164 | // has never had. Borrowing makes that class of drift impossible. |
| 165 | let read = ReadFileTool.input_schema(); |
| 166 | let name_search = FileSearchTool.input_schema(); |
| 167 | let content_search = GrepFilesTool.input_schema(); |
| 168 | let write = WriteFileTool.input_schema(); |
| 169 | let edit = EditFileTool.input_schema(); |
| 170 | let patch = ApplyPatchTool.input_schema(); |
| 171 | json!({ |
| 172 | "type": "object", |
| 173 | "properties": { |
| 174 | "action": { |
| 175 | "type": "string", |
| 176 | "enum": actions, |
| 177 | "description": "Action to perform" |
| 178 | }, |
| 179 | "path": { |
| 180 | "type": "string", |
| 181 | "description": format!( |
| 182 | "{} Optional for list and search (default: .).", |
| 183 | borrowed(&read, "path"), |
| 184 | ) |
| 185 | }, |
| 186 | "start_line": { |
| 187 | "type": "integer", |
| 188 | "description": describe(&read, "start_line", "action=read") |
| 189 | }, |
| 190 | "max_lines": { |
| 191 | "type": "integer", |
| 192 | "description": describe(&read, "max_lines", "action=read") |
| 193 | }, |
| 194 | "pages": { |
| 195 | "type": "string", |
| 196 | "description": describe(&read, "pages", "action=read") |
| 197 | }, |
| 198 | "content": { |
| 199 | "type": "string", |
| 200 | "description": describe(&write, "content", "action=write") |
| 201 | }, |
| 202 | "search": { |
| 203 | "type": "string", |
| 204 | "description": describe(&edit, "search", "action=edit") |
| 205 | }, |
| 206 | "replace": { |
| 207 | "oneOf": [ |
| 208 | { "type": "string", "description": describe(&edit, "replace", "action=edit") }, |
| 209 | { "type": "array", "items": { "type": "object", "properties": { "path": { "type": "string" }, "content": { "type": "string" } }, "required": ["path", "content"] }, "description": "Full-file replacements — action=patch." } |
| 210 | ] |
| 211 | }, |
| 212 | "fuzz": { |
| 213 | "type": "integer", |
| 214 | "description": describe(&patch, "fuzz", "action=patch") |
| 215 | }, |
| 216 | "query": { |
| 217 | "type": "string", |
| 218 | "description": describe(&name_search, "query", "action=search_name") |
| 219 | }, |
| 220 | "pattern": { |
| 221 | "type": "string", |
| 222 | "description": describe(&content_search, "pattern", "action=search_content") |
| 223 | }, |
| 224 | "limit": { |
| 225 | "type": "integer", |
| 226 | "description": describe(&name_search, "limit", "action=search_name") |
| 227 | }, |
| 228 | "max_results": { |
| 229 | "type": "integer", |
| 230 | "description": format!( |
| 231 | "{} search_name: alias for `limit`.", |
| 232 | describe(&content_search, "max_results", "action=search_content"), |
| 233 | ) |
| 234 | }, |
| 235 | "extensions": { |
| 236 | "type": "array", |
| 237 | "items": { "type": "string" }, |
| 238 | "description": describe(&name_search, "extensions", "action=search_name") |
| 239 | }, |
| 240 | "include": { |
| 241 | "type": "array", |
| 242 | "items": { "type": "string" }, |
| 243 | "description": describe(&content_search, "include", "action=search_content") |
| 244 | }, |
| 245 | "exclude": { |
| 246 | "type": "array", |
| 247 | "items": { "type": "string" }, |
| 248 | "description": format!( |
| 249 | "{} Also action=search_name.", |
| 250 | describe(&content_search, "exclude", "action=search_content"), |
| 251 | ) |
| 252 | }, |
| 253 | "context_lines": { |
| 254 | "type": "integer", |
| 255 | "description": describe(&content_search, "context_lines", "action=search_content") |
| 256 | }, |
| 257 | "case_insensitive": { |
| 258 | "type": "boolean", |
| 259 | "description": describe(&content_search, "case_insensitive", "action=search_content") |
| 260 | }, |
| 261 | "patch": { |
| 262 | "type": "string", |
| 263 | "description": "Unified diff patch content for action=patch" |
| 264 | }, |
| 265 | "changes": { |
| 266 | "type": "array", |
| 267 | "items": { "type": "object", "properties": { "path": { "type": "string" }, "content": { "type": "string" } }, "required": ["path", "content"] }, |
| 268 | "description": "Deprecated alias for replace in action=patch" |
| 269 | }, |
| 270 | "create_if_missing": { |
| 271 | "type": "boolean", |
| 272 | "description": "Create files if missing for action=patch" |
| 273 | } |
| 274 | }, |
| 275 | "required": ["action"] |
| 276 | }) |
| 277 | } |
| 278 | |
| 279 | fn capabilities(&self) -> Vec<ToolCapability> { |
| 280 | let mut capabilities = vec![ToolCapability::ReadOnly, ToolCapability::Sandboxable]; |
| 281 | let can_mutate = match self.forced_action { |
| 282 | Some("write" | "edit" | "patch") => true, |
| 283 | Some(_) => false, |
| 284 | None => self.allow_writes || self.allow_patch, |
| 285 | }; |
| 286 | if can_mutate { |
| 287 | capabilities.extend([ |
| 288 | ToolCapability::WritesFiles, |
| 289 | ToolCapability::RequiresApproval, |
| 290 | ]); |
| 291 | } |
| 292 | capabilities |
| 293 | } |
| 294 | |
| 295 | fn approval_requirement_for(&self, input: &Value) -> ApprovalRequirement { |
| 296 | match self.resolve_action(input) { |
| 297 | "read" | "list" | "search_name" | "search_content" => ApprovalRequirement::Auto, |
| 298 | "write" | "edit" | "patch" => ApprovalRequirement::Suggest, |
| 299 | _ => ApprovalRequirement::Auto, |
| 300 | } |
| 301 | } |
| 302 | |
| 303 | fn is_read_only_for(&self, input: &Value) -> bool { |
| 304 | matches!( |
| 305 | self.resolve_action(input), |
| 306 | "read" | "list" | "search_name" | "search_content" |
| 307 | ) |
| 308 | } |
| 309 | |
| 310 | fn supports_parallel_for(&self, input: &Value) -> bool { |
| 311 | matches!( |
| 312 | self.resolve_action(input), |
| 313 | "read" | "list" | "search_name" | "search_content" |
| 314 | ) |
| 315 | } |
| 316 | |
| 317 | fn starts_detached_for(&self, _input: &Value) -> bool { |
| 318 | false |
| 319 | } |
| 320 | |
| 321 | async fn execute(&self, input: Value, context: &ToolContext) -> Result<ToolResult, ToolError> { |
| 322 | let action = self.required_action(&input)?; |
| 323 | if matches!(action.as_str(), "write" | "edit") && !self.allow_writes { |
| 324 | return Err(ToolError::not_available(format!( |
| 325 | "File action=\"{action}\" is unavailable in the current mode; nothing was written. Available actions here: {}. Switch to Act mode (`/mode act`) for write-capable file work.", |
| 326 | self.available_actions().join(", ") |
| 327 | ))); |
| 328 | } |
| 329 | if action == "patch" && !self.allow_patch { |
| 330 | return Err(ToolError::not_available(format!( |
| 331 | "File action=\"patch\" is unavailable because the patch feature is disabled; nothing was written. Available actions here: {}. When writes are available in this mode, action=\"edit\" replaces a single exact match and action=\"write\" replaces the whole file.", |
| 332 | self.available_actions().join(", ") |
| 333 | ))); |
| 334 | } |
| 335 | let input = self.strip_action(input)?; |
| 336 | |
| 337 | match action.as_str() { |
| 338 | "read" => ReadFileTool.execute(input, context).await, |
| 339 | "list" => ListDirTool.execute(input, context).await, |
| 340 | // The cross-action spellings the wrapper advertises |
| 341 | // (`max_results` on search_name, `query`/`limit` on |
| 342 | // search_content) used to be copied here. They are alias-table |
| 343 | // entries on the implementing tools now — one mechanism, applied |
| 344 | // before the same unknown-parameter check every other action runs, |
| 345 | // and a direct call to the inner tool behaves identically. |
| 346 | "search_name" => FileSearchTool.execute(input, context).await, |
| 347 | "search_content" => GrepFilesTool.execute(input, context).await, |
| 348 | "write" => WriteFileTool.execute(input, context).await, |
| 349 | "edit" => EditFileTool.execute(input, context).await, |
| 350 | "patch" => ApplyPatchTool.execute(input, context).await, |
| 351 | other => Err(ToolError::invalid_input(format!( |
| 352 | "Unknown File action \"{other}\"; nothing was run. Pass one of: {}.", |
| 353 | self.available_actions().join(", ") |
| 354 | ))), |
| 355 | } |
| 356 | } |
| 357 | } |
| 358 | |
| 359 | #[cfg(test)] |
| 360 | mod tests { |
| 361 | use super::*; |
| 362 | use serde_json::json; |
| 363 | use tempfile::tempdir; |
| 364 | |
| 365 | fn tool() -> FileTool { |
| 366 | FileTool::with_patch("File") |
| 367 | } |
| 368 | |
| 369 | async fn err(tool: &FileTool, input: Value) -> String { |
| 370 | let tmp = tempdir().expect("tempdir"); |
| 371 | let ctx = ToolContext::new(tmp.path().to_path_buf()); |
| 372 | tool.execute(input, &ctx) |
| 373 | .await |
| 374 | .expect_err("call must be refused") |
| 375 | .to_string() |
| 376 | } |
| 377 | |
| 378 | /// #5209's shape one level up: `File{path, content}` used to answer an |
| 379 | /// intended write with the file's contents under a success receipt. |
| 380 | #[tokio::test] |
| 381 | async fn missing_action_is_refused_instead_of_silently_reading() { |
| 382 | let message = err(&tool(), json!({"path": "a.rs", "content": "fn main() {}"})).await; |
| 383 | assert!( |
| 384 | message.contains("requires an `action`"), |
| 385 | "must say what is missing: {message}" |
| 386 | ); |
| 387 | assert!( |
| 388 | message.contains("nothing was run"), |
| 389 | "must deny having done work: {message}" |
| 390 | ); |
| 391 | for action in [ |
| 392 | "read", |
| 393 | "list", |
| 394 | "search_name", |
| 395 | "search_content", |
| 396 | "write", |
| 397 | "edit", |
| 398 | "patch", |
| 399 | ] { |
| 400 | assert!(message.contains(action), "must name `{action}`: {message}"); |
| 401 | } |
| 402 | } |
| 403 | |
| 404 | #[tokio::test] |
| 405 | async fn non_string_action_is_refused_with_the_valid_values() { |
| 406 | let message = err(&tool(), json!({"action": 3, "path": "a.rs"})).await; |
| 407 | assert!(message.contains("to be a string"), "{message}"); |
| 408 | assert!(message.contains("read"), "{message}"); |
| 409 | } |
| 410 | |
| 411 | #[tokio::test] |
| 412 | async fn unknown_action_names_the_actions_that_dispatch() { |
| 413 | let message = err(&tool(), json!({"action": "str_replace", "path": "a.rs"})).await; |
| 414 | assert!( |
| 415 | message.contains("str_replace"), |
| 416 | "must quote the bad value: {message}" |
| 417 | ); |
| 418 | assert!(message.contains("nothing was run"), "{message}"); |
| 419 | assert!( |
| 420 | message.contains("edit"), |
| 421 | "must name the real action: {message}" |
| 422 | ); |
| 423 | } |
| 424 | |
| 425 | /// A refusal must never advertise an action this instance cannot run. |
| 426 | #[tokio::test] |
| 427 | async fn read_only_instances_do_not_suggest_write_actions() { |
| 428 | let message = err(&FileTool::read_only("File"), json!({"path": "a.rs"})).await; |
| 429 | assert!(message.contains("read"), "{message}"); |
| 430 | assert!( |
| 431 | !message.contains("write"), |
| 432 | "read-only File must not offer write: {message}" |
| 433 | ); |
| 434 | assert!( |
| 435 | !message.contains("edit"), |
| 436 | "read-only File must not offer edit: {message}" |
| 437 | ); |
| 438 | } |
| 439 | |
| 440 | #[tokio::test] |
| 441 | async fn disabled_write_refusal_states_what_is_available() { |
| 442 | let message = err( |
| 443 | &FileTool::read_only("File"), |
| 444 | json!({"action": "write", "path": "a.rs", "content": "x"}), |
| 445 | ) |
| 446 | .await; |
| 447 | assert!(message.contains("nothing was written"), "{message}"); |
| 448 | assert!(message.contains("Available actions here"), "{message}"); |
| 449 | } |
| 450 | |
| 451 | /// The wrapper is the only schema the model reads. Every per-action |
| 452 | /// description must come from the tool that implements the action, so the |
| 453 | /// stale-copy drift that produced "default 200" cannot recur. |
| 454 | #[test] |
| 455 | fn wrapper_borrows_every_inner_description() { |
| 456 | let schema = tool().input_schema(); |
| 457 | let properties = schema["properties"].as_object().expect("properties"); |
| 458 | for (name, property) in properties { |
| 459 | let text = if name == "replace" { |
| 460 | property.to_string() |
| 461 | } else { |
| 462 | property["description"] |
| 463 | .as_str() |
| 464 | .unwrap_or_default() |
| 465 | .to_string() |
| 466 | }; |
| 467 | assert!( |
| 468 | !text.trim().is_empty(), |
| 469 | "`{name}` lost its description — an inner parameter was probably renamed" |
| 470 | ); |
| 471 | } |
| 472 | } |
| 473 | |
| 474 | #[test] |
| 475 | fn read_parameters_state_the_defaults_the_code_actually_uses() { |
| 476 | let schema = tool().input_schema(); |
| 477 | let max_lines = schema["properties"]["max_lines"]["description"] |
| 478 | .as_str() |
| 479 | .expect("max_lines description"); |
| 480 | let inner = ReadFileTool.input_schema()["properties"]["max_lines"]["description"] |
| 481 | .as_str() |
| 482 | .expect("inner max_lines description") |
| 483 | .to_string(); |
| 484 | assert!( |
| 485 | max_lines.contains(inner.trim_end_matches('.')), |
| 486 | "wrapper must quote the implementing tool verbatim: {max_lines}" |
| 487 | ); |
| 488 | assert!( |
| 489 | !max_lines.contains("200"), |
| 490 | "the retired 200-line default must not be advertised: {max_lines}" |
| 491 | ); |
| 492 | assert!( |
| 493 | !max_lines.contains("blame"), |
| 494 | "`File` has no blame action; blame lives on `Git`: {max_lines}" |
| 495 | ); |
| 496 | } |
| 497 | |
| 498 | /// `fuzz` belongs to `patch` alone. `edit` read it into a discarded |
| 499 | /// binding while the schema kept advertising it, and a live model read |
| 500 | /// that as "an optional fuzzy-matching flag for the search" — a |
| 501 | /// capability claim no code honored. The advertisement is gone; what |
| 502 | /// remains must describe only the integer `patch` really uses. |
| 503 | #[test] |
| 504 | fn fuzz_is_advertised_only_for_patch() { |
| 505 | let schema = tool().input_schema(); |
| 506 | assert_eq!(schema["properties"]["fuzz"]["type"], "integer"); |
| 507 | let fuzz = schema["properties"]["fuzz"]["description"] |
| 508 | .as_str() |
| 509 | .expect("fuzz description"); |
| 510 | assert!(fuzz.contains("action=patch"), "{fuzz}"); |
| 511 | assert!( |
| 512 | !fuzz.contains("action=edit"), |
| 513 | "edit no longer accepts fuzz: {fuzz}" |
| 514 | ); |
| 515 | assert!( |
| 516 | EditFileTool.input_schema()["properties"] |
| 517 | .get("fuzz") |
| 518 | .is_none(), |
| 519 | "edit must not advertise a parameter it does not implement" |
| 520 | ); |
| 521 | } |
| 522 | |
| 523 | /// `File` is in the active catalog in every mode, so its schema is re-sent |
| 524 | /// on every turn of every session. Borrowing the implementing tools' |
| 525 | /// descriptions buys accuracy with bytes; this bounds what that costs and |
| 526 | /// prints the per-parameter breakdown when it trips, so the next increase |
| 527 | /// is a decision rather than a drift. |
| 528 | #[test] |
| 529 | fn schema_stays_within_its_catalog_byte_budget() { |
| 530 | const BUDGET_BYTES: usize = 3_000; |
| 531 | |
| 532 | let schema = FileTool::with_patch("File").input_schema(); |
| 533 | let mut rows: Vec<(usize, String)> = schema["properties"] |
| 534 | .as_object() |
| 535 | .expect("properties") |
| 536 | .iter() |
| 537 | .map(|(name, property)| (property.to_string().len(), name.clone())) |
| 538 | .collect(); |
| 539 | rows.sort_by_key(|row| std::cmp::Reverse(row.0)); |
| 540 | let total: usize = rows.iter().map(|(bytes, _)| bytes).sum(); |
| 541 | |
| 542 | assert!( |
| 543 | total <= BUDGET_BYTES, |
| 544 | "File schema is {total} bytes against a {BUDGET_BYTES} budget; \ |
| 545 | trim explanation, keep instruction. Breakdown: {rows:?}" |
| 546 | ); |
| 547 | } |
| 548 | |
| 549 | // === Per-action parameter validation === |
| 550 | // |
| 551 | // #5209 taught `edit` to refuse a parameter it does not implement. Only |
| 552 | // `edit` learned it, so every other action still dropped unknown keys and |
| 553 | // answered anyway: a misspelled `start_line` on `read` returned the head |
| 554 | // of the file with nothing in the response admitting the requested window |
| 555 | // was never honored. These cover the refusal on every action, and — the |
| 556 | // half that keeps a refusal honest — that each action still accepts its |
| 557 | // full legitimate parameter set, optional names and aliases included. |
| 558 | |
| 559 | /// A workspace with one file, already read, so `edit` and `patch` are |
| 560 | /// past their freshness precondition and reach parameter validation. |
| 561 | async fn workspace() -> (tempfile::TempDir, ToolContext) { |
| 562 | let tmp = tempdir().expect("tempdir"); |
| 563 | let ctx = ToolContext::new(tmp.path().to_path_buf()); |
| 564 | std::fs::write(tmp.path().join("doc.txt"), "alpha\nbeta\ngamma\n").expect("write"); |
| 565 | tool() |
| 566 | .execute(json!({"action": "read", "path": "doc.txt"}), &ctx) |
| 567 | .await |
| 568 | .expect("seed read"); |
| 569 | (tmp, ctx) |
| 570 | } |
| 571 | |
| 572 | /// The minimal call that dispatches for each action, in schema order. |
| 573 | fn minimal_calls() -> Vec<(&'static str, Value)> { |
| 574 | vec![ |
| 575 | ("read", json!({"action": "read", "path": "doc.txt"})), |
| 576 | ("list", json!({"action": "list"})), |
| 577 | ( |
| 578 | "search_name", |
| 579 | json!({"action": "search_name", "query": "doc"}), |
| 580 | ), |
| 581 | ( |
| 582 | "search_content", |
| 583 | json!({"action": "search_content", "pattern": "alpha"}), |
| 584 | ), |
| 585 | ( |
| 586 | "write", |
| 587 | json!({"action": "write", "path": "new.txt", "content": "x\n"}), |
| 588 | ), |
| 589 | ( |
| 590 | "edit", |
| 591 | json!({"action": "edit", "path": "doc.txt", "search": "alpha", "replace": "delta"}), |
| 592 | ), |
| 593 | ( |
| 594 | "patch", |
| 595 | json!({"action": "patch", "path": "doc.txt", "patch": "@@ -1,1 +1,1 @@\n-alpha\n+delta\n"}), |
| 596 | ), |
| 597 | ] |
| 598 | } |
| 599 | |
| 600 | fn with_key(mut input: Value, key: &str, value: Value) -> Value { |
| 601 | input |
| 602 | .as_object_mut() |
| 603 | .expect("object") |
| 604 | .insert(key.to_string(), value); |
| 605 | input |
| 606 | } |
| 607 | |
| 608 | /// The gap this closes. A parameter with no known meaning is refused by |
| 609 | /// every action, not just `edit`, and the refusal carries the same four |
| 610 | /// facts everywhere: what was wrong, what is allowed, what is required, |
| 611 | /// and that nothing was done. |
| 612 | #[tokio::test] |
| 613 | async fn every_action_refuses_an_unknown_parameter() { |
| 614 | for (action, call) in minimal_calls() { |
| 615 | let (_tmp, ctx) = workspace().await; |
| 616 | let message = tool() |
| 617 | .execute(with_key(call, "bogus_param", json!(true)), &ctx) |
| 618 | .await |
| 619 | .expect_err("an unknown parameter must be refused") |
| 620 | .to_string(); |
| 621 | assert!( |
| 622 | message.contains("bogus_param"), |
| 623 | "{action} must name the offending parameter: {message}" |
| 624 | ); |
| 625 | assert!( |
| 626 | message.contains(&format!("unexpected File {action} parameter")), |
| 627 | "{action} must name the action it refused: {message}" |
| 628 | ); |
| 629 | assert!( |
| 630 | message.contains("Allowed parameters are"), |
| 631 | "{action} must name the allowed set: {message}" |
| 632 | ); |
| 633 | assert!( |
| 634 | message.contains("Required:"), |
| 635 | "{action} must name the required set: {message}" |
| 636 | ); |
| 637 | assert!( |
| 638 | message.contains(&format!("The {action} was not performed")), |
| 639 | "{action} must deny having done the work: {message}" |
| 640 | ); |
| 641 | } |
| 642 | } |
| 643 | |
| 644 | /// The specific silent wrong answer that motivated this: a misspelled |
| 645 | /// read window used to be dropped, and the head of the file came back |
| 646 | /// under a success receipt as if it were the requested range. |
| 647 | #[tokio::test] |
| 648 | async fn a_misspelled_read_window_is_refused_rather_than_answered_with_the_head() { |
| 649 | let (_tmp, ctx) = workspace().await; |
| 650 | let message = tool() |
| 651 | .execute( |
| 652 | json!({"action": "read", "path": "doc.txt", "start_lien": 2}), |
| 653 | &ctx, |
| 654 | ) |
| 655 | .await |
| 656 | .expect_err("a misspelled window must not silently return the head") |
| 657 | .to_string(); |
| 658 | assert!(message.contains("start_lien"), "{message}"); |
| 659 | assert!(message.contains("`start_line`"), "{message}"); |
| 660 | } |
| 661 | |
| 662 | /// A refusal is only worth having if the legitimate call still lands. |
| 663 | /// Every action's full parameter set — every optional name included — |
| 664 | /// must survive validation. |
| 665 | #[tokio::test] |
| 666 | async fn every_action_accepts_its_full_legitimate_parameter_set() { |
| 667 | let full: Vec<(&str, Value)> = vec![ |
| 668 | ( |
| 669 | "read", |
| 670 | json!({"action": "read", "path": "doc.txt", "start_line": 1, "max_lines": 2, "pages": "1"}), |
| 671 | ), |
| 672 | ("list", json!({"action": "list", "path": "."})), |
| 673 | ( |
| 674 | "search_name", |
| 675 | json!({"action": "search_name", "query": "doc", "path": ".", "limit": 5, |
| 676 | "extensions": ["txt"], "exclude": ["target/**"]}), |
| 677 | ), |
| 678 | ( |
| 679 | "search_content", |
| 680 | json!({"action": "search_content", "pattern": "alpha", "path": ".", |
| 681 | "include": ["*.txt"], "exclude": ["target/**"], "context_lines": 1, |
| 682 | "case_insensitive": true, "max_results": 5}), |
| 683 | ), |
| 684 | ( |
| 685 | "write", |
| 686 | json!({"action": "write", "path": "new.txt", "content": "x\n"}), |
| 687 | ), |
| 688 | ( |
| 689 | "edit", |
| 690 | json!({"action": "edit", "path": "doc.txt", "search": "alpha", "replace": "delta"}), |
| 691 | ), |
| 692 | ( |
| 693 | "patch", |
| 694 | json!({"action": "patch", "path": "doc.txt", |
| 695 | "patch": "@@ -1,1 +1,1 @@\n-alpha\n+delta\n", |
| 696 | "fuzz": 3, "create_if_missing": false}), |
| 697 | ), |
| 698 | ]; |
| 699 | |
| 700 | for (action, call) in full { |
| 701 | let (_tmp, ctx) = workspace().await; |
| 702 | let result = tool() |
| 703 | .execute(call, &ctx) |
| 704 | .await |
| 705 | .unwrap_or_else(|error| panic!("{action} must accept its own parameters: {error}")); |
| 706 | assert!(result.success, "{action}: {}", result.content); |
| 707 | } |
| 708 | } |
| 709 | |
| 710 | /// Validation runs *after* alias translation, so every cross-harness |
| 711 | /// spelling the alias lane folds must still reach the action it names. |
| 712 | /// A refusal that fired first would undo #5209's fix. |
| 713 | #[tokio::test] |
| 714 | async fn every_alias_survives_validation() { |
| 715 | let aliased: Vec<(&str, Value)> = vec![ |
| 716 | // Path spellings, on every action that takes a path. |
| 717 | ("read", json!({"action": "read", "file_path": "doc.txt"})), |
| 718 | ("read", json!({"action": "read", "filePath": "doc.txt"})), |
| 719 | ("list", json!({"action": "list", "file_path": "."})), |
| 720 | ( |
| 721 | "search_name", |
| 722 | json!({"action": "search_name", "query": "doc", "file_path": "."}), |
| 723 | ), |
| 724 | ( |
| 725 | "search_content", |
| 726 | json!({"action": "search_content", "pattern": "alpha", "file_path": "."}), |
| 727 | ), |
| 728 | ( |
| 729 | "write", |
| 730 | json!({"action": "write", "file_path": "new.txt", "content": "x\n"}), |
| 731 | ), |
| 732 | // Read-window spellings. |
| 733 | ( |
| 734 | "read", |
| 735 | json!({"action": "read", "path": "doc.txt", "offset": 2, "limit": 1}), |
| 736 | ), |
| 737 | ( |
| 738 | "read", |
| 739 | json!({"action": "read", "path": "doc.txt", "line_offset": 2, "n_lines": 1}), |
| 740 | ), |
| 741 | ( |
| 742 | "read", |
| 743 | json!({"action": "read", "path": "doc.txt", "num_lines": 1}), |
| 744 | ), |
| 745 | // Search spellings the wrapper advertises across both actions. |
| 746 | ( |
| 747 | "search_name", |
| 748 | json!({"action": "search_name", "query": "doc", "max_results": 5}), |
| 749 | ), |
| 750 | ( |
| 751 | "search_content", |
| 752 | json!({"action": "search_content", "query": "alpha", "limit": 5}), |
| 753 | ), |
| 754 | ]; |
| 755 | |
| 756 | for (action, call) in aliased { |
| 757 | let (_tmp, ctx) = workspace().await; |
| 758 | let result = tool() |
| 759 | .execute(call.clone(), &ctx) |
| 760 | .await |
| 761 | .unwrap_or_else(|error| panic!("{action} must accept {call}: {error}")); |
| 762 | assert!(result.success, "{action} / {call}: {}", result.content); |
| 763 | } |
| 764 | |
| 765 | // Edit spellings need their own loop: each one mutates the file. |
| 766 | for (search, replace) in [ |
| 767 | ("old_string", "new_string"), |
| 768 | ("old_str", "new_str"), |
| 769 | ("oldText", "newText"), |
| 770 | ("old_text", "new_text"), |
| 771 | ] { |
| 772 | let (_tmp, ctx) = workspace().await; |
| 773 | let result = tool() |
| 774 | .execute( |
| 775 | json!({"action": "edit", "path": "doc.txt", |
| 776 | search: "alpha", replace: "delta"}), |
| 777 | &ctx, |
| 778 | ) |
| 779 | .await |
| 780 | .unwrap_or_else(|error| panic!("edit must accept {search}/{replace}: {error}")); |
| 781 | assert!(result.success, "{search}/{replace}: {}", result.content); |
| 782 | } |
| 783 | let (_tmp, ctx) = workspace().await; |
| 784 | let result = tool() |
| 785 | .execute( |
| 786 | json!({"action": "edit", "path": "doc.txt", "search": "alpha", "replacement": "delta"}), |
| 787 | &ctx, |
| 788 | ) |
| 789 | .await |
| 790 | .expect("edit must accept `replacement`"); |
| 791 | assert!(result.success, "{}", result.content); |
| 792 | } |
| 793 | |
| 794 | /// A parameter that belongs to a *different* action is still unknown to |
| 795 | /// this one. Silently dropping it is how a model learns a call worked |
| 796 | /// when the argument it cared about was discarded. |
| 797 | #[tokio::test] |
| 798 | async fn parameters_do_not_leak_between_actions() { |
| 799 | for (action, call) in [ |
| 800 | ( |
| 801 | "read", |
| 802 | json!({"action": "read", "path": "doc.txt", "case_insensitive": true}), |
| 803 | ), |
| 804 | ( |
| 805 | "write", |
| 806 | json!({"action": "write", "path": "new.txt", "content": "x\n", "start_line": 2}), |
| 807 | ), |
| 808 | ( |
| 809 | "list", |
| 810 | json!({"action": "list", "path": ".", "context_lines": 3}), |
| 811 | ), |
| 812 | ( |
| 813 | "search_name", |
| 814 | json!({"action": "search_name", "query": "doc", "context_lines": 3}), |
| 815 | ), |
| 816 | ( |
| 817 | "search_content", |
| 818 | json!({"action": "search_content", "pattern": "alpha", "extensions": ["txt"]}), |
| 819 | ), |
| 820 | ] { |
| 821 | let (_tmp, ctx) = workspace().await; |
| 822 | let message = tool() |
| 823 | .execute(call, &ctx) |
| 824 | .await |
| 825 | .expect_err("another action's parameter must be refused") |
| 826 | .to_string(); |
| 827 | assert!( |
| 828 | message.contains(&format!("The {action} was not performed")), |
| 829 | "{action}: {message}" |
| 830 | ); |
| 831 | } |
| 832 | } |
| 833 | |
| 834 | /// Every action's required names must be names it also allows, or a |
| 835 | /// refusal would tell the model to pass something the action rejects. |
| 836 | #[test] |
| 837 | fn every_required_parameter_is_also_an_allowed_one() { |
| 838 | use crate::tools::file::{ |
| 839 | EDIT_PARAMS, LIST_PARAMS, PATCH_PARAMS, READ_PARAMS, SEARCH_CONTENT_PARAMS, |
| 840 | SEARCH_NAME_PARAMS, WRITE_PARAMS, |
| 841 | }; |
| 842 | |
| 843 | for params in [ |
| 844 | READ_PARAMS, |
| 845 | WRITE_PARAMS, |
| 846 | EDIT_PARAMS, |
| 847 | LIST_PARAMS, |
| 848 | SEARCH_NAME_PARAMS, |
| 849 | SEARCH_CONTENT_PARAMS, |
| 850 | PATCH_PARAMS, |
| 851 | ] { |
| 852 | params.assert_required_is_allowed(); |
| 853 | } |
| 854 | } |
| 855 | |
| 856 | #[test] |
| 857 | fn advertised_actions_match_the_actions_that_dispatch() { |
| 858 | for (tool, expected) in [ |
| 859 | (FileTool::with_patch("File"), 7), |
| 860 | (FileTool::new("File"), 6), |
| 861 | (FileTool::read_only("File"), 4), |
| 862 | ] { |
| 863 | let schema = tool.input_schema(); |
| 864 | let advertised = schema["properties"]["action"]["enum"] |
| 865 | .as_array() |
| 866 | .expect("action enum") |
| 867 | .len(); |
| 868 | assert_eq!(advertised, expected); |
| 869 | assert_eq!(advertised, tool.available_actions().len()); |
| 870 | } |
| 871 | } |
| 872 | } |
| 873 |