@sigitcodereview / sigit / commits / 643b7d1

Send file edits to ACP clients as diff content

create_file, edit_file and multi_edit used to report only a line of result text. They now return the file's text before and after, and handle_prompt sends it as ACP diff content ahead of that text, so Zed renders a real diff and can build its review UI from it. The permission request for these tools carries the same diff, so the user approves an edit by looking at the change rather than the raw JSON arguments. preview_file_change works it out without writing anything (create_file's parser makes parent directories, so the preview skips it) and runs before the workspace is released, because a relative path resolves against the session's cwd. execute_tool still returns a plain string, so the TUI, headless runs, subagents and hooks are unchanged. Closes #145 Co-Authored-By: siGit Code v1.6.4-acp <noreply@sigit.si> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Seto Elkahfi committed Oct 8, 2026 at 06:54 UTC 643b7d1b5bb1eecd14467f626592431c2bbeb7af

3 files changed +347 -53

.agents/AGENTS.md

+5 -1

@@ -330,7 +330,11 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

330 `execute_tool_impl` asks `route_for` before it dispatches `read_file`, `create_file`,

331 `edit_file` or `multi_edit`. Each of those tools is split into a parse step and a pure

332 render/apply step (`ReadFileCall`, `CreateFileCall`, `EditCall`) that the disk path and the

333 - client path share, so the model gets the same result text either way. The two capabilities are

333 + client path share, so the model gets the same result text either way. The writing tools also

334 + hand back the file before and after (`tools::FileChange`, via `execute_tool_with_change`), which

335 + `handle_prompt` sends as ACP `diff` content ahead of the result text. A permission request for

336 + one of them carries the same diff, worked out by `preview_file_change` without writing anything

337 + and before the workspace is released, since a relative path resolves against the session's cwd. The two capabilities are

338 independent: a client that only reads still gets its edits written to disk. The disk is the

339 fallback throughout: nothing is registered in the TUI or headless modes, a path outside the

340 session's roots is not routed, and a request the client fails or leaves unanswered for 30

src/main.rs

+81 -5

@@ -74,8 +74,8 @@ use agent_client_protocol::schema::v1::{

74 AgentAuthCapabilities, AgentCapabilities, AuthMethod, AuthMethodAgent, AuthenticateRequest,

75 AuthenticateResponse, AvailableCommand, AvailableCommandInput, AvailableCommandsUpdate,

76 CancelNotification, CloseSessionRequest, CloseSessionResponse, ConfigOptionUpdate,

77 - ContentBlock, ContentChunk, CurrentModeUpdate, EmbeddedResourceResource, ForkSessionRequest,

78 - ForkSessionResponse, Implementation, InitializeRequest, InitializeResponse,

77 + ContentBlock, ContentChunk, CurrentModeUpdate, Diff, EmbeddedResourceResource,

78 + ForkSessionRequest, ForkSessionResponse, Implementation, InitializeRequest, InitializeResponse,

79 ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse,

80 LogoutCapabilities, LogoutRequest, LogoutResponse, McpCapabilities, McpServer, Meta,

81 NewSessionRequest, NewSessionResponse, PermissionOption, PermissionOptionKind, Plan, PlanEntry,

@@ -513,6 +513,25 @@ fn tool_output_content(output: &str) -> ToolCallContent {

513 fenced_code_block("", &chat::cap_output_preview(output)).into()

514 }

515

516 +/// A file a tool wrote, as ACP `diff` content, which clients render as a real

517 +/// diff (Zed builds its review UI from it) instead of a line of result text

518 +/// (issue #145).

519 +fn file_change_content(change: &tools::FileChange) -> ToolCallContent {

520 + Diff::new(change.path.clone(), change.new_text.clone())

521 + .old_text(change.old_text.clone())

522 + .into()

523 +}

524 +

525 +/// The content of a finished tool call's card: the diff of the file it wrote,

526 +/// if it wrote one, then its result text.

527 +fn tool_result_content(output: &str, change: Option<&tools::FileChange>) -> Vec<ToolCallContent> {

528 + change

529 + .map(file_change_content)

530 + .into_iter()

531 + .chain(std::iter::once(tool_output_content(output)))

532 + .collect()

533 +}

534 +

535 /// A single-file location for the path-bearing tools, so ACP clients can

536 /// follow the agent through a file as it works (issue #78's "follow-along").

537 fn tool_call_locations(name: &str, arguments_json: &str) -> Vec<ToolCallLocation> {

@@ -3203,6 +3222,7 @@ impl SiGitAgent {

3222

3223 // Permission gate: read-only tools pass straight through; a

3224 // mutating tool consults policy and may ask the client.

3225 + let mut change = None;

3226 let output = match decision {

3227 None => format!(

3228 "The tool `{}` was not executed again because the model repeated \

@@ -3211,13 +3231,20 @@ impl SiGitAgent {

3231 ),

3232 Some(decision) => match decision {

3233 permissions::Decision::Allow => {

3214 - tools::execute_tool(&tc.name, &tc.arguments).await

3234 + let outcome =

3235 + tools::execute_tool_with_change(&tc.name, &tc.arguments).await;

3236 + change = outcome.change;

3237 + outcome.output

3238 }

3239 permissions::Decision::Deny(reason) => {

3240 log::info!(" ✗ {} denied by policy", tc.name);

3241 reason

3242 }

3243 permissions::Decision::Ask => {

3244 + // Worked out while the session still holds the

3245 + // workspace: a relative path resolves against

3246 + // its cwd.

3247 + let preview = tools::preview_file_change(&tc.name, &tc.arguments).await;

3248 // The answer can take as long as the user likes.

3249 if release_while_waiting {

3250 workspace.release();

@@ -3232,6 +3259,7 @@ impl SiGitAgent {

3259 &tc.id,

3260 &tc.name,

3261 &tc.arguments,

3262 + preview.as_ref(),

3263 ) => verdict,

3264 _ = cancellation.cancelled() => PermissionVerdict::TurnCancelled,

3265 };

@@ -3266,7 +3294,11 @@ impl SiGitAgent {

3294 )

3295 .ok();

3296 }

3269 - tools::execute_tool(&tc.name, &tc.arguments).await

3297 + let outcome =

3298 + tools::execute_tool_with_change(&tc.name, &tc.arguments)

3299 + .await;

3300 + change = outcome.change;

3301 + outcome.output

3302 }

3303 PermissionVerdict::Denied(reason) => {

3304 log::info!(" ✗ {} denied by user", tc.name);

@@ -3329,7 +3361,7 @@ impl SiGitAgent {

3361 tc.id.clone(),

3362 ToolCallUpdateFields::new()

3363 .status(ToolCallStatus::Completed)

3332 - .content(vec![tool_output_content(&output)])

3364 + .content(tool_result_content(&output, change.as_ref()))

3365 .raw_output(serde_json::Value::String(output.clone())),

3366 )),

3367 )

@@ -3519,6 +3551,10 @@ impl SiGitAgent {

3551 /// recorded via [`permissions::grant_for_session`]. Only safe to call from

3552 /// a spawned task (see the handler registration in `run_acp_server`): the

3553 /// dispatch loop must be free to route the client's answer back to us.

3554 + ///

3555 + /// `preview` is the file change the call would make, from

3556 + /// [`tools::preview_file_change`]; when there is one the card shows it as

3557 + /// a diff, so the user approves an edit by looking at the change.

3558 async fn request_tool_permission(

3559 &self,

3560 cx: &ConnectionTo<Client>,

@@ -3526,6 +3562,7 @@ impl SiGitAgent {

3562 tool_call_id: &str,

3563 tool_name: &str,

3564 arguments: &str,

3565 + preview: Option<&tools::FileChange>,

3566 ) -> PermissionVerdict {

3567 // The user decides from this dialog, so show the arguments with any

3568 // truncation flagged (a silently clipped command could hide its tail

@@ -3550,6 +3587,7 @@ impl SiGitAgent {

3587 .title(title)

3588 .kind(tool_kind_for(tool_name))

3589 .status(ToolCallStatus::Pending)

3590 + .content(preview.map(|change| vec![file_change_content(change)]))

3591 .raw_input(raw_input),

3592 ),

3593 vec![

@@ -6985,6 +7023,44 @@ mod tests {

7023 assert!(rendered.contains(" \\n\\t "));

7024 }

7025

7026 + #[test]

7027 + fn a_file_change_is_sent_as_diff_content_before_the_result() {

7028 + let change = tools::FileChange {

7029 + path: PathBuf::from("/repo/src/lib.rs"),

7030 + old_text: Some("old\n".to_string()),

7031 + new_text: "new\n".to_string(),

7032 + };

7033 + let content = serde_json::to_value(tool_result_content(

7034 + "Edited file: /repo/src/lib.rs (4 bytes written)",

7035 + Some(&change),

7036 + ))

7037 + .unwrap();

7038 + assert_eq!(

7039 + content[0],

7040 + serde_json::json!({

7041 + "type": "diff",

7042 + "path": "/repo/src/lib.rs",

7043 + "oldText": "old\n",

7044 + "newText": "new\n",

7045 + })

7046 + );

7047 + assert_eq!(content[1]["type"], "content");

7048 + assert_eq!(content.as_array().unwrap().len(), 2);

7049 +

7050 + // A created file has no old text; ACP leaves `oldText` out.

7051 + let created = tools::FileChange {

7052 + old_text: None,

7053 + ..change

7054 + };

7055 + let diff = serde_json::to_value(file_change_content(&created)).unwrap();

7056 + assert!(diff.get("oldText").is_none_or(serde_json::Value::is_null));

7057 +

7058 + // Every other tool keeps the result text alone.

7059 + let plain = serde_json::to_value(tool_result_content("ok", None)).unwrap();

7060 + assert_eq!(plain.as_array().unwrap().len(), 1);

7061 + assert_eq!(plain[0]["type"], "content");

7062 + }

7063 +

7064 #[test]

7065 fn tool_arguments_content_only_labels_valid_json() {

7066 let json = format!("{:?}", tool_arguments_content(r#"{"path":"src/main.rs"}"#));

src/tools.rs

+261 -47

@@ -437,7 +437,40 @@ pub fn all_tools() -> Vec<AgentTool> {

437

438 // ── Tool execution ───────────────────────────────────────────────────────────

439

440 +/// A file a tool call wrote, before and after, so an ACP client can show the

441 +/// change as a diff instead of the result text (`ToolCallContent::Diff`).

442 +#[derive(Debug, Clone, PartialEq)]

443 +pub struct FileChange {

444 + /// Absolute, as ACP requires.

445 + pub path: PathBuf,

446 + /// `None` for a file the call created.

447 + pub old_text: Option<String>,

448 + pub new_text: String,

449 +}

450 +

451 +/// What a tool call produced: the result text the model sees and, for the

452 +/// file-writing tools, the change it made.

453 +#[derive(Debug)]

454 +pub struct ToolOutcome {

455 + pub output: String,

456 + pub change: Option<FileChange>,

457 +}

458 +

459 +impl From<String> for ToolOutcome {

460 + fn from(output: String) -> Self {

461 + Self {

462 + output,

463 + change: None,

464 + }

465 + }

466 +}

467 +

468 pub async fn execute_tool(name: &str, arguments: &str) -> String {

469 + execute_tool_with_change(name, arguments).await.output

470 +}

471 +

472 +/// [`execute_tool`], keeping the file change for the surfaces that render it.

473 +pub async fn execute_tool_with_change(name: &str, arguments: &str) -> ToolOutcome {

474 // Settings are read once per call (not once per hook phase) so a

475 // deployment without any [hooks] configured pays one cheap TOML parse

476 // per tool call rather than two.

@@ -455,13 +488,13 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {

488 if let Some(cwd) = &cwd

489 && !hook_settings.post_tool_use.is_empty()

490 {

458 - crate::hooks::run_post_tool_use_hooks(&hook_settings, name, &result, cwd);

491 + crate::hooks::run_post_tool_use_hooks(&hook_settings, name, &result.output, cwd);

492 }

493

494 result

495 }

496

464 -async fn execute_tool_impl(name: &str, arguments: &str) -> String {

497 +async fn execute_tool_impl(name: &str, arguments: &str) -> ToolOutcome {

498 // An editor that serves files from its buffers gets the file tools' reads

499 // and writes for paths in the session's roots (see `client_fs`).

500 if let Some(route) = client_file_route(name, arguments) {

@@ -469,12 +502,14 @@ async fn execute_tool_impl(name: &str, arguments: &str) -> String {

502 }

503

504 match name {

472 - TASK_TOOL_NAME => exec_task(arguments).await,

473 - WEB_SEARCH_TOOL_NAME => exec_web_search(arguments).await,

474 - "command_output" => exec_command_output_wait(arguments, active_session()).await,

505 + TASK_TOOL_NAME => exec_task(arguments).await.into(),

506 + WEB_SEARCH_TOOL_NAME => exec_web_search(arguments).await.into(),

507 + "command_output" => exec_command_output_wait(arguments, active_session())

508 + .await

509 + .into(),

510 // Tools discovered from MCP servers are namespaced `mcp__<server>__<tool>`

511 // and forwarded to the owning server.

477 - _ if crate::mcp::is_mcp_tool(name) => crate::mcp::call_tool(name, arguments).await,

512 + _ if crate::mcp::is_mcp_tool(name) => crate::mcp::call_tool(name, arguments).await.into(),

513 // Everything else is synchronous and may run for a long time: a shell

514 // command up to COMMAND_TIMEOUT, a glob over a big tree, an HTTP fetch

515 // on reqwest::blocking (which panics if polled on a runtime thread).

@@ -500,7 +535,7 @@ async fn execute_tool_impl(name: &str, arguments: &str) -> String {

535 // tool result, not the whole turn. Note this is strictly safer

536 // than running the tool inline, where the panic would unwind

537 // through the turn and take the ACP connection with it.

503 - .unwrap_or_else(|err| format!("Error: {tool} task failed: {err}"))

538 + .unwrap_or_else(|err| format!("Error: {tool} task failed: {err}").into())

539 }

540 }

541 }

@@ -514,16 +549,15 @@ async fn execute_tool_impl(name: &str, arguments: &str) -> String {

549 /// dispatch exists for: not blocking the caller's task for its whole run.

550 ///

551 /// `owner` is the session the call runs for; background tasks are scoped to it.

517 -fn execute_sync_tool(name: &str, arguments: &str, owner: Option<&str>) -> String {

518 - match name {

552 +fn execute_sync_tool(name: &str, arguments: &str, owner: Option<&str>) -> ToolOutcome {

553 + let output = match name {

554 + "create_file" => return create_file(arguments),

555 + "edit_file" | "multi_edit" => return edit(name, arguments),

556 "read_file" => exec_read_file(arguments),

557 "list_directory" => exec_list_directory(arguments),

558 "search_files" => exec_search_files(arguments),

559 "read_website" => exec_read_website(arguments),

560 "create_directory" => exec_create_directory(arguments),

524 - "create_file" => exec_create_file(arguments),

525 - "edit_file" => exec_edit_file(arguments),

526 - "multi_edit" => exec_multi_edit(arguments),

561 "glob" => exec_glob(arguments),

562 "write_todos" => exec_write_todos(arguments),

563 "remember" => exec_remember(arguments),

@@ -532,7 +566,8 @@ fn execute_sync_tool(name: &str, arguments: &str, owner: Option<&str>) -> String

566 "kill_command" => exec_kill_command(arguments, owner),

567 "skill" => crate::skills::activate_skill(arguments),

568 _ => format!("Unknown tool: {name}"),

535 - }

569 + };

570 + output.into()

571 }

572

573 // ── web_search ───────────────────────────────────────────────────────────────

@@ -1475,27 +1510,39 @@ impl CreateFileCall {

1510 })

1511 }

1512

1478 - fn written(&self) -> String {

1479 - format!(

1480 - "Created file: {} ({} bytes)",

1481 - self.path.display(),

1482 - self.content.len()

1483 - )

1513 + fn written(self) -> ToolOutcome {

1514 + ToolOutcome {

1515 + output: format!(

1516 + "Created file: {} ({} bytes)",

1517 + self.path.display(),

1518 + self.content.len()

1519 + ),

1520 + change: Some(FileChange {

1521 + path: self.path,

1522 + old_text: None,

1523 + new_text: self.content,

1524 + }),

1525 + }

1526 }

1527 }

1528

1529 /// fails if file exists so the LLM is forced to use `edit_file` for modifications.

1488 -fn exec_create_file(arguments: &str) -> String {

1530 +fn create_file(arguments: &str) -> ToolOutcome {

1531 let call = match CreateFileCall::parse(arguments) {

1532 Ok(call) => call,

1491 - Err(error) => return error,

1533 + Err(error) => return error.into(),

1534 };

1535 match fs::write(&call.path, &call.content) {

1536 Ok(()) => call.written(),

1495 - Err(err) => format!("Error: could not write file: {err}"),

1537 + Err(err) => format!("Error: could not write file: {err}").into(),

1538 }

1539 }

1540

1541 +#[cfg(test)]

1542 +fn exec_create_file(arguments: &str) -> String {

1543 + create_file(arguments).output

1544 +}

1545 +

1546 // ── edit_file / multi_edit ─────────────────────────────────────────────────

1547

1548 /// Apply one exact-substring replacement to `contents`. Returns the updated

@@ -1674,9 +1721,9 @@ impl EditCall {

1721 }

1722 }

1723

1677 - fn written(&self, updated: &str) -> String {

1724 + fn written(self, contents: String, updated: String) -> ToolOutcome {

1725 let absolute_path_str = self.path.display().to_string();

1679 - match &self.ops {

1726 + let output = match &self.ops {

1727 EditOps::Single { .. } => format!(

1728 "Edited file: {absolute_path_str} ({} bytes written)",

1729 updated.len()

@@ -1686,42 +1733,90 @@ impl EditCall {

1733 edits.len(),

1734 updated.len()

1735 ),

1736 + };

1737 + ToolOutcome {

1738 + output,

1739 + change: Some(FileChange {

1740 + path: self.path,

1741 + old_text: Some(contents),

1742 + new_text: updated,

1743 + }),

1744 }

1745 }

1746 }

1747

1693 -/// `edit_file` and `multi_edit` against the disk.

1694 -fn exec_edit(tool: &str, arguments: &str) -> String {

1748 +/// `edit_file` and `multi_edit` against the disk. A `multi_edit` batch is

1749 +/// atomic: each edit is applied to the result of the previous one, and the

1750 +/// file is only written if *every* edit matches.

1751 +fn edit(tool: &str, arguments: &str) -> ToolOutcome {

1752 let call = match EditCall::parse(tool, arguments) {

1753 Ok(call) => call,

1697 - Err(error) => return error,

1754 + Err(error) => return error.into(),

1755 };

1756

1757 let contents = match fs::read_to_string(&call.path) {

1758 Ok(c) => c,

1702 - Err(err) => return format!("Error: could not read file: {err}"),

1759 + Err(err) => return format!("Error: could not read file: {err}").into(),

1760 };

1761

1762 let updated = match call.apply(&contents) {

1763 Ok(updated) => updated,

1707 - Err(error) => return error,

1764 + Err(error) => return error.into(),

1765 };

1766

1767 match fs::write(&call.path, &updated) {

1711 - Ok(()) => call.written(&updated),

1712 - Err(err) => format!("Error: could not write file: {err}"),

1768 + Ok(()) => call.written(contents, updated),

1769 + Err(err) => format!("Error: could not write file: {err}").into(),

1770 }

1771 }

1772

1773 +#[cfg(test)]

1774 fn exec_edit_file(arguments: &str) -> String {

1717 - exec_edit("edit_file", arguments)

1775 + edit("edit_file", arguments).output

1776 }

1777

1720 -/// Apply a batch of edits to one file atomically: each edit is applied to the

1721 -/// result of the previous one, and the file is only written if *every* edit

1722 -/// matches. A failure leaves the file untouched.

1778 +#[cfg(test)]

1779 fn exec_multi_edit(arguments: &str) -> String {

1724 - exec_edit("multi_edit", arguments)

1780 + edit("multi_edit", arguments).output

1781 +}

1782 +

1783 +/// The change a `create_file`, `edit_file` or `multi_edit` call would make,

1784 +/// worked out without writing anything so the permission request can show

1785 +/// the user the diff they are approving. `None` for any other tool, and for

1786 +/// a call that would fail (its error is reported when it runs).

1787 +///

1788 +/// Reads the file the same way the call will, through the client when the

1789 +/// path is routed there, so the preview matches an unsaved buffer. Resolves

1790 +/// relative paths against the cwd, so call it while the session is installed.

1791 +pub async fn preview_file_change(name: &str, arguments: &str) -> Option<FileChange> {

1792 + match name {

1793 + "create_file" => {

1794 + // Not `CreateFileCall::parse`: it creates the parent directories,

1795 + // and nothing may change on disk before the user approves.

1796 + let args: Value = serde_json::from_str(arguments).ok()?;

1797 + let path = absolute_path(Path::new(args.get("path")?.as_str()?));

1798 + let content = args.get("content")?.as_str()?;

1799 + (!path.exists()).then(|| FileChange {

1800 + path,

1801 + old_text: None,

1802 + new_text: content.to_string(),

1803 + })

1804 + }

1805 + "edit_file" | "multi_edit" => {

1806 + let call = EditCall::parse(name, arguments).ok()?;

1807 + let contents = match client_file_route(name, arguments) {

1808 + Some(route) => route.read(&call.path).await.ok()?,

1809 + None => fs::read_to_string(&call.path).ok()?,

1810 + };

1811 + let updated = call.apply(&contents).ok()?;

1812 + Some(FileChange {

1813 + path: call.path,

1814 + old_text: Some(contents),

1815 + new_text: updated,

1816 + })

1817 + }

1818 + _ => None,

1819 + }

1820 }

1821

1822 // ── File tools through the ACP client ────────────────────────────────────────

@@ -1747,44 +1842,44 @@ async fn exec_file_tool_via_client(

1842 name: &str,

1843 arguments: &str,

1844 route: &crate::client_fs::Route,

1750 -) -> String {

1845 +) -> ToolOutcome {

1846 match name {

1847 "read_file" => {

1848 let call = match ReadFileCall::parse(arguments) {

1849 Ok(call) => call,

1755 - Err(error) => return error,

1850 + Err(error) => return error.into(),

1851 };

1852 match route.read(&call.path).await {

1758 - Ok(contents) => call.render(contents),

1759 - Err(err) => format!("Error: could not read file: {err}"),

1853 + Ok(contents) => call.render(contents).into(),

1854 + Err(err) => format!("Error: could not read file: {err}").into(),

1855 }

1856 }

1857 "create_file" => {

1858 let call = match CreateFileCall::parse(arguments) {

1859 Ok(call) => call,

1765 - Err(error) => return error,

1860 + Err(error) => return error.into(),

1861 };

1862 match route.write(&call.path, &call.content).await {

1863 Ok(()) => call.written(),

1769 - Err(err) => format!("Error: could not write file: {err}"),

1864 + Err(err) => format!("Error: could not write file: {err}").into(),

1865 }

1866 }

1867 _ => {

1868 let call = match EditCall::parse(name, arguments) {

1869 Ok(call) => call,

1775 - Err(error) => return error,

1870 + Err(error) => return error.into(),

1871 };

1872 let contents = match route.read(&call.path).await {

1873 Ok(c) => c,

1779 - Err(err) => return format!("Error: could not read file: {err}"),

1874 + Err(err) => return format!("Error: could not read file: {err}").into(),

1875 };

1876 let updated = match call.apply(&contents) {

1877 Ok(updated) => updated,

1783 - Err(error) => return error,

1878 + Err(error) => return error.into(),

1879 };

1880 match route.write(&call.path, &updated).await {

1786 - Ok(()) => call.written(&updated),

1787 - Err(err) => format!("Error: could not write file: {err}"),

1881 + Ok(()) => call.written(contents, updated),

1882 + Err(err) => format!("Error: could not write file: {err}").into(),

1883 }

1884 }

1885 }

@@ -3409,6 +3504,125 @@ mod tests {

3504 let _ = fs::remove_dir_all(&dir);

3505 }

3506

3507 + #[test]

3508 + fn written_files_report_their_change() {

3509 + let dir = std::env::temp_dir().join("sigit_test_file_change");

3510 + let _ = fs::remove_dir_all(&dir);

3511 + fs::create_dir_all(&dir).unwrap();

3512 + let file_path = dir.join("notes.txt");

3513 +

3514 + let created = create_file(

3515 + &serde_json::json!({ "path": file_path, "content": "one\ntwo\n" }).to_string(),

3516 + );

3517 + assert_eq!(

3518 + created.change,

3519 + Some(FileChange {

3520 + path: file_path.clone(),

3521 + old_text: None,

3522 + new_text: "one\ntwo\n".to_string(),

3523 + })

3524 + );

3525 +

3526 + let edited = edit(

3527 + "edit_file",

3528 + &serde_json::json!({ "path": file_path, "old_text": "two", "new_text": "2" })

3529 + .to_string(),

3530 + );

3531 + assert_eq!(

3532 + edited.change,

3533 + Some(FileChange {

3534 + path: file_path.clone(),

3535 + old_text: Some("one\ntwo\n".to_string()),

3536 + new_text: "one\n2\n".to_string(),

3537 + })

3538 + );

3539 +

3540 + let batch = edit(

3541 + "multi_edit",

3542 + &serde_json::json!({

3543 + "path": file_path,

3544 + "edits": [

3545 + { "old_text": "one", "new_text": "1" },

3546 + { "old_text": "2", "new_text": "two" },

3547 + ],

3548 + })

3549 + .to_string(),

3550 + );

3551 + let change = batch.change.expect("multi_edit reports its change");

3552 + assert_eq!(change.old_text.as_deref(), Some("one\n2\n"));

3553 + assert_eq!(change.new_text, "1\ntwo\n");

3554 +

3555 + // A call that writes nothing has nothing to show.

3556 + let failed = edit(

3557 + "edit_file",

3558 + &serde_json::json!({ "path": file_path, "old_text": "absent", "new_text": "x" })

3559 + .to_string(),

3560 + );

3561 + assert!(

3562 + failed.output.starts_with("Error:"),

3563 + "got: {}",

3564 + failed.output

3565 + );

3566 + assert_eq!(failed.change, None);

3567 +

3568 + let _ = fs::remove_dir_all(&dir);

3569 + }

3570 +

3571 + #[tokio::test]

3572 + async fn previewing_a_change_writes_nothing() {

3573 + let dir = std::env::temp_dir().join("sigit_test_preview_change");

3574 + let _ = fs::remove_dir_all(&dir);

3575 + fs::create_dir_all(&dir).unwrap();

3576 + let existing = dir.join("existing.txt");

3577 + fs::write(&existing, "alpha\nbeta\n").unwrap();

3578 +

3579 + let edit_args =

3580 + serde_json::json!({ "path": existing, "old_text": "beta", "new_text": "gamma" })

3581 + .to_string();

3582 + let preview = preview_file_change("edit_file", &edit_args).await;

3583 + assert_eq!(

3584 + preview,

3585 + Some(FileChange {

3586 + path: existing.clone(),

3587 + old_text: Some("alpha\nbeta\n".to_string()),

3588 + new_text: "alpha\ngamma\n".to_string(),

3589 + })

3590 + );

3591 + assert_eq!(fs::read_to_string(&existing).unwrap(), "alpha\nbeta\n");

3592 +

3593 + // `create_file` would make the parent directories; the preview must not.

3594 + let new_file = dir.join("missing").join("new.txt");

3595 + let create_args = serde_json::json!({ "path": new_file, "content": "fresh" }).to_string();

3596 + let preview = preview_file_change("create_file", &create_args).await;

3597 + assert_eq!(

3598 + preview,

3599 + Some(FileChange {

3600 + path: new_file.clone(),

3601 + old_text: None,

3602 + new_text: "fresh".to_string(),

3603 + })

3604 + );

3605 + assert!(!dir.join("missing").exists());

3606 +

3607 + // Calls that would fail, and tools that write no file, preview nothing.

3608 + let unmatched =

3609 + serde_json::json!({ "path": existing, "old_text": "absent", "new_text": "x" })

3610 + .to_string();

3611 + assert_eq!(preview_file_change("edit_file", &unmatched).await, None);

3612 + let clobber = serde_json::json!({ "path": existing, "content": "x" }).to_string();

3613 + assert_eq!(preview_file_change("create_file", &clobber).await, None);

3614 + assert_eq!(

3615 + preview_file_change(

3616 + "delete_file",

3617 + &serde_json::json!({ "path": existing }).to_string()

3618 + )

3619 + .await,

3620 + None

3621 + );

3622 +

3623 + let _ = fs::remove_dir_all(&dir);

3624 + }

3625 +

3626 #[test]

3627 fn test_create_file_already_exists() {

3628 let dir = std::env::temp_dir().join("sigit_test_create_exists");