Bennet Bo Fenner and Ben Brandt created
Before:
1. Agent tries to edit unsaved file
2. Tool call fails with error telling the agent to ask the user to save
or discard edits
3. User types save/restore
4. Agent uses save/restore tool
https://github.com/user-attachments/assets/c94dd361-e8e0-48ee-be31-da8afe594419
After:
1. Agent tries to edit unsaved file
2. User is prompted to save/restore file
3. User accepts/rejects or saves/discards file manually
https://github.com/user-attachments/assets/1d98a0c4-4420-4426-94f2-42355de230be
Release Notes:
- agent: Improved UX when agent tries to edit unsaved buffer
---------
Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>assets/settings/default.json | 2
crates/acp_thread/src/acp_thread.rs | 43
crates/acp_thread/src/connection.rs | 3
crates/agent/src/agent.rs | 5
crates/agent/src/tests/mod.rs | 106 -
crates/agent/src/thread.rs | 60
crates/agent/src/tools.rs | 6
crates/agent/src/tools/edit_file_tool.rs | 256 +++
crates/agent/src/tools/edit_session.rs | 149 +
crates/agent/src/tools/restore_file_from_disk_tool.rs | 673 ----------
crates/agent/src/tools/save_file_tool.rs | 756 ------------
crates/agent/src/tools/tool_permissions.rs | 86 +
crates/agent/src/tools/write_file_tool.rs | 206 +++
crates/agent_servers/src/acp.rs | 1
crates/agent_ui/src/conversation_view.rs | 1
crates/settings_ui/src/pages.rs | 3
crates/settings_ui/src/pages/tool_permissions_setup.rs | 19
crates/sidebar/src/sidebar_tests.rs | 1
18 files changed, 712 insertions(+), 1,664 deletions(-)
@@ -1122,8 +1122,6 @@
"now": true,
"rename_symbol": true,
"read_file": true,
- "restore_file_from_disk": true,
- "save_file": true,
"open": true,
"grep": true,
"spawn_agent": true,
@@ -570,6 +570,22 @@ impl From<RequestPermissionOutcome> for acp::RequestPermissionOutcome {
}
}
+/// What a `WaitingForConfirmation` prompt represents semantically.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum AuthorizationKind {
+ /// The user is granting or denying permission for the tool call to
+ /// proceed. The selected `PermissionOptionKind` determines whether the
+ /// tool call transitions to `InProgress` (allow) or `Rejected` (reject).
+ /// This is the default for tool authorization prompts.
+ PermissionGrant,
+ /// The user is choosing between actions for the tool to take next
+ /// (for example, "Save" vs "Discard" before editing a dirty buffer).
+ /// The tool call always transitions to `InProgress` regardless of the
+ /// selected `PermissionOptionKind`; the caller interprets the chosen
+ /// `option_id` to decide what to do.
+ ActionChoice,
+}
+
#[derive(Debug)]
pub enum ToolCallStatus {
/// The tool call hasn't started running yet, but we start showing it to
@@ -579,6 +595,7 @@ pub enum ToolCallStatus {
WaitingForConfirmation {
options: PermissionOptions,
respond_tx: oneshot::Sender<SelectedPermissionOutcome>,
+ kind: AuthorizationKind,
},
/// The tool call is currently running.
InProgress,
@@ -2080,6 +2097,7 @@ impl AcpThread {
&mut self,
tool_call: acp::ToolCallUpdate,
options: PermissionOptions,
+ kind: AuthorizationKind,
cx: &mut Context<Self>,
) -> Result<Task<RequestPermissionOutcome>> {
let (tx, rx) = oneshot::channel();
@@ -2087,6 +2105,7 @@ impl AcpThread {
let status = ToolCallStatus::WaitingForConfirmation {
options,
respond_tx: tx,
+ kind,
};
let tool_call_id = tool_call.tool_call_id.clone();
@@ -2118,15 +2137,25 @@ impl AcpThread {
return;
};
- let new_status = match outcome.option_kind {
- acp::PermissionOptionKind::RejectOnce | acp::PermissionOptionKind::RejectAlways => {
- ToolCallStatus::Rejected
+ let is_action_choice = matches!(
+ call.status,
+ ToolCallStatus::WaitingForConfirmation {
+ kind: AuthorizationKind::ActionChoice,
+ ..
}
- acp::PermissionOptionKind::AllowOnce | acp::PermissionOptionKind::AllowAlways => {
+ );
+ let new_status =
+ if is_action_choice {
ToolCallStatus::InProgress
- }
- _ => ToolCallStatus::InProgress,
- };
+ } else {
+ match outcome.option_kind {
+ acp::PermissionOptionKind::RejectOnce
+ | acp::PermissionOptionKind::RejectAlways => ToolCallStatus::Rejected,
+ acp::PermissionOptionKind::AllowOnce
+ | acp::PermissionOptionKind::AllowAlways => ToolCallStatus::InProgress,
+ _ => ToolCallStatus::InProgress,
+ }
+ };
let curr_status = mem::replace(&mut call.status, new_status);
@@ -641,6 +641,8 @@ mod test_support {
use gpui::{AppContext as _, WeakEntity};
use parking_lot::Mutex;
+ use crate::AuthorizationKind;
+
use super::*;
/// Creates a PNG image encoded as base64 for testing.
@@ -915,6 +917,7 @@ mod test_support {
thread.request_tool_call_authorization(
tool_call.clone().into(),
options.clone(),
+ AuthorizationKind::PermissionGrant,
cx,
)
})??
@@ -1298,9 +1298,12 @@ impl NativeAgentConnection {
options,
response,
context: _,
+ kind,
}) => {
let outcome_task = acp_thread.update(cx, |thread, cx| {
- thread.request_tool_call_authorization(tool_call, options, cx)
+ thread.request_tool_call_authorization(
+ tool_call, options, kind, cx,
+ )
})??;
cx.background_spawn(async move {
if let acp_thread::RequestPermissionOutcome::Selected(outcome) =
@@ -6391,112 +6391,6 @@ async fn test_copy_path_tool_deny_rule_blocks_copy(cx: &mut TestAppContext) {
);
}
-#[gpui::test]
-async fn test_save_file_tool_denies_if_any_path_denied(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- "/root",
- json!({
- "normal.txt": "normal content",
- "readonly": {
- "config.txt": "readonly content"
- }
- }),
- )
- .await;
- let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await;
-
- cx.update(|cx| {
- let mut settings = agent_settings::AgentSettings::get_global(cx).clone();
- settings.tool_permissions.tools.insert(
- SaveFileTool::NAME.into(),
- agent_settings::ToolRules {
- default: Some(settings::ToolPermissionMode::Allow),
- always_allow: vec![],
- always_deny: vec![agent_settings::CompiledRegex::new(r"readonly", false).unwrap()],
- always_confirm: vec![],
- invalid_patterns: vec![],
- },
- );
- agent_settings::AgentSettings::override_global(settings, cx);
- });
-
- #[allow(clippy::arc_with_non_send_sync)]
- let tool = Arc::new(crate::SaveFileTool::new(project));
- let (event_stream, _rx) = crate::ToolCallEventStream::test();
-
- let task = cx.update(|cx| {
- tool.run(
- ToolInput::resolved(crate::SaveFileToolInput {
- paths: vec![
- std::path::PathBuf::from("root/normal.txt"),
- std::path::PathBuf::from("root/readonly/config.txt"),
- ],
- }),
- event_stream,
- cx,
- )
- });
-
- let result = task.await;
- assert!(
- result.is_err(),
- "expected save to be blocked due to denied path"
- );
- assert!(
- result.unwrap_err().contains("blocked"),
- "error should mention the save was blocked"
- );
-}
-
-#[gpui::test]
-async fn test_save_file_tool_respects_deny_rules(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree("/root", json!({"config.secret": "secret config"}))
- .await;
- let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await;
-
- cx.update(|cx| {
- let mut settings = agent_settings::AgentSettings::get_global(cx).clone();
- settings.tool_permissions.tools.insert(
- SaveFileTool::NAME.into(),
- agent_settings::ToolRules {
- default: Some(settings::ToolPermissionMode::Allow),
- always_allow: vec![],
- always_deny: vec![agent_settings::CompiledRegex::new(r"\.secret$", false).unwrap()],
- always_confirm: vec![],
- invalid_patterns: vec![],
- },
- );
- agent_settings::AgentSettings::override_global(settings, cx);
- });
-
- #[allow(clippy::arc_with_non_send_sync)]
- let tool = Arc::new(crate::SaveFileTool::new(project));
- let (event_stream, _rx) = crate::ToolCallEventStream::test();
-
- let task = cx.update(|cx| {
- tool.run(
- ToolInput::resolved(crate::SaveFileToolInput {
- paths: vec![std::path::PathBuf::from("root/config.secret")],
- }),
- event_stream,
- cx,
- )
- });
-
- let result = task.await;
- assert!(result.is_err(), "expected save to be blocked");
- assert!(
- result.unwrap_err().contains("blocked"),
- "error should mention the save was blocked"
- );
-}
-
#[gpui::test]
async fn test_web_search_tool_deny_rule_blocks_search(cx: &mut TestAppContext) {
init_test(cx);
@@ -3,8 +3,8 @@ use crate::{
DbLanguageModel, DbThread, DeletePathTool, DiagnosticsTool, EditFileTool, FetchTool,
FindPathTool, FindReferencesTool, GetCodeActionsTool, GoToDefinitionTool, GrepTool,
ListDirectoryTool, MovePathTool, NowTool, OpenTool, ProjectSnapshot, ReadFileTool, RenameTool,
- RestoreFileFromDiskTool, SaveFileTool, SpawnAgentTool, SystemPromptTemplate, Template,
- Templates, TerminalTool, ToolPermissionDecision, UpdatePlanTool, WebSearchTool, WriteFileTool,
+ SpawnAgentTool, SystemPromptTemplate, Template, Templates, TerminalTool,
+ ToolPermissionDecision, UpdatePlanTool, WebSearchTool, WriteFileTool,
decide_permission_from_settings,
};
use acp_thread::{MentionUri, UserMessageId};
@@ -825,7 +825,6 @@ impl ToolPermissionContext {
|| tool_name == WriteFileTool::NAME
|| tool_name == DeletePathTool::NAME
|| tool_name == CreateDirectoryTool::NAME
- || tool_name == SaveFileTool::NAME
{
(
extract_path_pattern(value),
@@ -925,6 +924,7 @@ pub struct ToolCallAuthorization {
pub options: acp_thread::PermissionOptions,
pub response: oneshot::Sender<acp_thread::SelectedPermissionOutcome>,
pub context: Option<ToolPermissionContext>,
+ pub kind: acp_thread::AuthorizationKind,
}
#[derive(Debug, thiserror::Error)]
@@ -1571,8 +1571,6 @@ impl Thread {
self.action_log.clone(),
update_agent_location,
));
- self.add_tool(SaveFileTool::new(self.project.clone()));
- self.add_tool(RestoreFileFromDiskTool::new(self.project.clone()));
self.add_tool(TerminalTool::new(self.project.clone(), environment.clone()));
self.add_tool(WebSearchTool);
@@ -3878,6 +3876,57 @@ impl ToolCallEventStream {
self.run_authorization_loop(title, options, Some(context), None, cx)
}
+ /// Prompts the user to choose between an explicit set of actions and
+ /// returns the chosen `option_id`.
+ ///
+ /// Unlike [`Self::authorize`] / [`Self::authorize_always_prompt`], this
+ /// does not interpret the user's choice as a permission grant — callers
+ /// are responsible for handling each `option_id` explicitly. Use this
+ /// when a tool needs the user to pick between several side-effecting
+ /// actions (for example, "Save" vs "Discard" for a dirty buffer).
+ pub fn prompt_for_decision(
+ &self,
+ title: Option<String>,
+ message: Option<String>,
+ options: Vec<acp::PermissionOption>,
+ cx: &mut App,
+ ) -> Task<Result<acp::PermissionOptionId>> {
+ let options = acp_thread::PermissionOptions::Flat(options);
+ let stream = self.stream.clone();
+ let tool_use_id = self.tool_use_id.clone();
+ cx.spawn(async move |_cx| {
+ let mut fields = acp::ToolCallUpdateFields::new();
+ if let Some(title) = title {
+ fields = fields.title(title);
+ }
+ if let Some(message) = message {
+ fields = fields.content(vec![acp::ToolCallContent::from(message)]);
+ }
+
+ let (response_tx, response_rx) = oneshot::channel();
+ if let Err(error) = stream
+ .0
+ .unbounded_send(Ok(ThreadEvent::ToolCallAuthorization(
+ ToolCallAuthorization {
+ tool_call: acp::ToolCallUpdate::new(tool_use_id.to_string(), fields),
+ options,
+ response: response_tx,
+ context: None,
+ kind: acp_thread::AuthorizationKind::ActionChoice,
+ },
+ )))
+ {
+ log::error!("Failed to send tool call decision prompt: {error}");
+ return Err(anyhow!("Failed to send tool call decision prompt: {error}"));
+ }
+
+ let outcome = response_rx
+ .await
+ .map_err(|_| anyhow!("authorization channel closed"))?;
+ Ok(outcome.option_id)
+ })
+ }
+
/// Prompts the user for authorization.
///
/// When `check_settings` is `Some`, this gate is settings-driven: the
@@ -3925,6 +3974,7 @@ impl ToolCallEventStream {
options,
response: response_tx,
context,
+ kind: acp_thread::AuthorizationKind::PermissionGrant,
},
)))
{
@@ -20,8 +20,6 @@ mod now_tool;
mod open_tool;
mod read_file_tool;
mod rename_tool;
-mod restore_file_from_disk_tool;
-mod save_file_tool;
mod spawn_agent_tool;
mod symbol_locator;
mod terminal_tool;
@@ -79,8 +77,6 @@ pub use now_tool::*;
pub use open_tool::*;
pub use read_file_tool::*;
pub use rename_tool::*;
-pub use restore_file_from_disk_tool::*;
-pub use save_file_tool::*;
pub use spawn_agent_tool::*;
pub use symbol_locator::*;
pub use terminal_tool::*;
@@ -176,8 +172,6 @@ tools! {
OpenTool,
ReadFileTool,
RenameTool,
- RestoreFileFromDiskTool,
- SaveFileTool,
SpawnAgentTool,
TerminalTool,
UpdatePlanTool,
@@ -1871,9 +1871,12 @@ mod tests {
assert_eq!(input_path, Some(PathBuf::from("root/test.txt")));
}
+ /// When the buffer has unsaved changes and the user picks "Save", the
+ /// pending edits are flushed to disk and the agent's edit then proceeds
+ /// against the just-saved content.
#[gpui::test]
- async fn test_streaming_dirty_buffer_detected(cx: &mut TestAppContext) {
- let (edit_tool, project, action_log, _fs, _thread) =
+ async fn test_streaming_dirty_buffer_save(cx: &mut TestAppContext) {
+ let (edit_tool, project, action_log, fs, _thread) =
setup_test(cx, json!({"test.txt": "original content"})).await;
let read_tool = Arc::new(crate::ReadFileTool::new(
project.clone(),
@@ -1881,7 +1884,6 @@ mod tests {
true,
));
- // Read the file first
cx.update(|cx| {
read_tool.clone().run(
ToolInput::resolved(crate::ReadFileToolInput {
@@ -1896,7 +1898,6 @@ mod tests {
.await
.unwrap();
- // Open the buffer and make it dirty
let project_path = project
.read_with(cx, |project, cx| {
project.find_project_path("root/test.txt", cx)
@@ -1909,54 +1910,219 @@ mod tests {
buffer.update(cx, |buffer, cx| {
let end_point = buffer.max_point();
- buffer.edit([(end_point..end_point, " added text")], None, cx);
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
});
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
- let is_dirty = buffer.read_with(cx, |buffer, _| buffer.is_dirty());
- assert!(is_dirty, "Buffer should be dirty after in-memory edit");
-
- // Try to edit - should fail because buffer has unsaved changes
- let result = cx
- .update(|cx| {
- edit_tool.clone().run(
- ToolInput::resolved(EditFileToolInput {
- path: "root/test.txt".into(),
- edits: vec![Edit {
- old_text: "original content".into(),
- new_text: "new content".into(),
- }],
- }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await;
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ edit_tool.clone().run(
+ ToolInput::resolved(EditFileToolInput {
+ path: "root/test.txt".into(),
+ edits: vec![Edit {
+ old_text: "original content plus user edit".into(),
+ new_text: "replaced content".into(),
+ }],
+ }),
+ stream_tx,
+ cx,
+ )
+ });
- let EditFileToolOutput::Error {
- error,
- diff,
- input_path,
- } = result.unwrap_err()
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+ let content = auth.tool_call.fields.content.as_deref().unwrap_or(&[]);
+ let acp::ToolCallContent::Content(text) = content.first().expect("expected message body")
else {
- panic!("expected error");
+ panic!("expected text body, got: {:?}", content.first());
+ };
+ let acp::ContentBlock::Text(text) = &text.content else {
+ panic!("expected text body, got: {:?}", text.content);
};
assert!(
- error.contains("This file has unsaved changes."),
- "Error should mention unsaved changes, got: {}",
- error
- );
- assert!(
- error.contains("keep or discard"),
- "Error should ask whether to keep or discard changes, got: {}",
- error
- );
- assert!(
- error.contains("save or revert the file manually"),
- "Error should ask user to manually save or revert when tools aren't available, got: {}",
- error
+ text.text.contains("unsaved changes")
+ && text.text.contains("save")
+ && text.text.contains("discard"),
+ "unexpected message body: {:?}",
+ text.text,
);
- assert!(diff.is_empty());
- assert!(input_path.is_none());
+ auth.response
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("save"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
+ .unwrap();
+
+ let EditFileToolOutput::Success { new_text, .. } = task.await.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "replaced content");
+ assert!(!buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let on_disk = fs.load(path!("/root/test.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "replaced content");
+ }
+
+ /// When the buffer has unsaved changes and the user picks "Discard", the
+ /// pending edits are reverted to match disk and the agent's edit then
+ /// proceeds against the on-disk content.
+ #[gpui::test]
+ async fn test_streaming_dirty_buffer_discard(cx: &mut TestAppContext) {
+ let (edit_tool, project, action_log, fs, _thread) =
+ setup_test(cx, json!({"test.txt": "original content"})).await;
+ let read_tool = Arc::new(crate::ReadFileTool::new(
+ project.clone(),
+ action_log.clone(),
+ true,
+ ));
+
+ cx.update(|cx| {
+ read_tool.clone().run(
+ ToolInput::resolved(crate::ReadFileToolInput {
+ path: "root/test.txt".to_string(),
+ start_line: None,
+ end_line: None,
+ }),
+ ToolCallEventStream::test().0,
+ cx,
+ )
+ })
+ .await
+ .unwrap();
+
+ let project_path = project
+ .read_with(cx, |project, cx| {
+ project.find_project_path("root/test.txt", cx)
+ })
+ .expect("Should find project path");
+ let buffer = project
+ .update(cx, |project, cx| project.open_buffer(project_path, cx))
+ .await
+ .unwrap();
+
+ buffer.update(cx, |buffer, cx| {
+ let end_point = buffer.max_point();
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
+ });
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ edit_tool.clone().run(
+ ToolInput::resolved(EditFileToolInput {
+ path: "root/test.txt".into(),
+ // Match the on-disk content, not the dirty in-memory content.
+ edits: vec![Edit {
+ old_text: "original content".into(),
+ new_text: "replaced content".into(),
+ }],
+ }),
+ stream_tx,
+ cx,
+ )
+ });
+
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+ auth.response
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("discard"),
+ acp::PermissionOptionKind::RejectOnce,
+ ))
+ .unwrap();
+
+ let EditFileToolOutput::Success { new_text, .. } = task.await.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "replaced content");
+ assert!(!buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let on_disk = fs.load(path!("/root/test.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "replaced content");
+ }
+
+ /// When the buffer is dirty and the user resolves it manually — e.g.
+ /// pressing `cmd-s` while the prompt is visible — the prompt is
+ /// dismissed automatically and the edit proceeds against the saved
+ /// content. The user shouldn't have to also click a button.
+ #[gpui::test]
+ async fn test_streaming_dirty_buffer_resolved_externally(cx: &mut TestAppContext) {
+ let (edit_tool, project, action_log, fs, _thread) =
+ setup_test(cx, json!({"test.txt": "original content"})).await;
+ let read_tool = Arc::new(crate::ReadFileTool::new(
+ project.clone(),
+ action_log.clone(),
+ true,
+ ));
+
+ cx.update(|cx| {
+ read_tool.clone().run(
+ ToolInput::resolved(crate::ReadFileToolInput {
+ path: "root/test.txt".to_string(),
+ start_line: None,
+ end_line: None,
+ }),
+ ToolCallEventStream::test().0,
+ cx,
+ )
+ })
+ .await
+ .unwrap();
+
+ let project_path = project
+ .read_with(cx, |project, cx| {
+ project.find_project_path("root/test.txt", cx)
+ })
+ .expect("Should find project path");
+ let buffer = project
+ .update(cx, |project, cx| project.open_buffer(project_path, cx))
+ .await
+ .unwrap();
+
+ buffer.update(cx, |buffer, cx| {
+ let end_point = buffer.max_point();
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
+ });
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ edit_tool.clone().run(
+ ToolInput::resolved(EditFileToolInput {
+ path: "root/test.txt".into(),
+ edits: vec![Edit {
+ old_text: "original content plus user edit".into(),
+ new_text: "replaced content".into(),
+ }],
+ }),
+ stream_tx,
+ cx,
+ )
+ });
+
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+
+ // Simulate the user saving the buffer manually (e.g. cmd-s) while
+ // the prompt is visible. The tool should detect the buffer became
+ // clean and proceed without the user clicking anything.
+ project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+ .await
+ .unwrap();
+
+ // The prompt's response channel should drop without a click; the
+ // tool dismisses the prompt by transitioning the tool call status
+ // to `InProgress`.
+ let dismiss = stream_rx.expect_update_fields().await;
+ assert_eq!(dismiss.status, Some(acp::ToolCallStatus::InProgress));
+ drop(auth);
+
+ let EditFileToolOutput::Success { new_text, .. } = task.await.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "replaced content");
+ assert!(!buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let on_disk = fs.load(path!("/root/test.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "replaced content");
}
#[gpui::test]
@@ -2,17 +2,16 @@ mod reindent;
mod streaming_fuzzy_matcher;
mod streaming_parser;
-use super::restore_file_from_disk_tool::RestoreFileFromDiskTool;
-use super::save_file_tool::SaveFileTool;
-use crate::{AgentTool, Thread, ToolCallEventStream};
+use crate::{Thread, ToolCallEventStream};
use acp_thread::Diff;
use action_log::ActionLog;
-use agent_client_protocol::schema::{ToolCallLocation, ToolCallUpdateFields};
+use agent_client_protocol::schema::{self as acp, ToolCallLocation, ToolCallUpdateFields};
use anyhow::Result;
use collections::HashSet;
+use futures::{FutureExt, channel::oneshot};
use gpui::{App, AppContext, AsyncApp, Entity, Task, WeakEntity};
use language::language_settings::{self, FormatOnSave};
-use language::{Buffer, LanguageRegistry};
+use language::{Buffer, BufferEvent, LanguageRegistry};
use language_model::LanguageModelToolResultContent;
use project::lsp_store::{FormatTrigger, LspFormatTarget};
use project::{AgentLocation, Project, ProjectPath};
@@ -665,7 +664,8 @@ impl EditSession {
.await
.map_err(|e| e.to_string())?;
- let file_changed_since_last_read = ensure_buffer_saved(&buffer, &abs_path, &context, cx)?;
+ let file_changed_since_last_read =
+ ensure_buffer_saved(&buffer, &abs_path, mode, &context, event_stream, cx).await?;
let diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
event_stream.update_diff(diff.clone());
@@ -932,53 +932,25 @@ fn agent_edit_buffer<I, S, T>(
});
}
-fn ensure_buffer_saved(
+async fn ensure_buffer_saved(
buffer: &Entity<Buffer>,
abs_path: &PathBuf,
+ mode: EditSessionMode,
context: &EditSessionContext,
+ event_stream: &ToolCallEventStream,
cx: &mut AsyncApp,
) -> Result<bool, String> {
let last_read_mtime = context
.action_log
.read_with(cx, |log, _| log.file_read_time(abs_path));
- let check_result = context.thread.read_with(cx, |thread, cx| {
- let current = buffer
- .read(cx)
- .file()
- .and_then(|file| file.disk_state().mtime());
- let dirty = buffer.read(cx).is_dirty();
- let has_save = thread.has_tool(SaveFileTool::NAME);
- let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME);
- (current, dirty, has_save, has_restore)
+ let (current_mtime, is_dirty) = buffer.read_with(cx, |buffer, _cx| {
+ let current = buffer.file().and_then(|file| file.disk_state().mtime());
+ let dirty = buffer.is_dirty();
+ (current, dirty)
});
- let Ok((current_mtime, is_dirty, has_save_tool, has_restore_tool)) = check_result else {
- return Ok(false);
- };
-
if is_dirty {
- let message = match (has_save_tool, has_restore_tool) {
- (true, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (true, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask the user to manually revert the file, then inform you when it's ok to proceed."
- }
- (false, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask the user to manually save the file, then inform you when it's ok to proceed. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (false, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes, \
- then ask them to save or revert the file manually and inform you when it's ok to proceed."
- }
- };
- return Err(message.to_string());
+ resolve_dirty_buffer(buffer, mode, context, event_stream, cx).await?;
}
if let (Some(last_read), Some(current)) = (last_read_mtime, current_mtime)
@@ -990,6 +962,99 @@ fn ensure_buffer_saved(
Ok(false)
}
+/// Prompts the user about how to handle a dirty buffer that the agent
+/// wants to edit (`EditSessionMode::Edit`) or overwrite
+/// (`EditSessionMode::Write`), and performs the chosen action so the
+/// edit session can proceed (or returns `Err` to cancel).
+///
+/// If the user resolves the dirty state externally (e.g. cmd-s or
+/// reload) while the prompt is visible, the prompt is dismissed
+/// automatically.
+async fn resolve_dirty_buffer(
+ buffer: &Entity<Buffer>,
+ mode: EditSessionMode,
+ context: &EditSessionContext,
+ event_stream: &ToolCallEventStream,
+ cx: &mut AsyncApp,
+) -> Result<(), String> {
+ let (manual_resolve_tx, manual_resolve_rx) = oneshot::channel::<()>();
+ let _buffer_subscription = cx.update(|cx| {
+ let mut tx = Some(manual_resolve_tx);
+ cx.subscribe(buffer, move |buffer, event: &BufferEvent, cx| {
+ if matches!(
+ event,
+ BufferEvent::Saved | BufferEvent::Reloaded | BufferEvent::DirtyChanged
+ ) && !buffer.read(cx).is_dirty()
+ && let Some(tx) = tx.take()
+ {
+ tx.send(()).ok();
+ }
+ })
+ });
+
+ let prompt_kind = match mode {
+ EditSessionMode::Edit => super::tool_permissions::DirtyBufferPromptKind::Edit,
+ EditSessionMode::Write => super::tool_permissions::DirtyBufferPromptKind::Overwrite,
+ };
+ let prompt = cx.update(|cx| {
+ super::tool_permissions::authorize_dirty_buffer(prompt_kind, event_stream, cx)
+ });
+
+ let decision = futures::select_biased! {
+ _ = manual_resolve_rx.fuse() => {
+ None
+ }
+ decision = prompt.fuse() => {
+ Some(decision.map_err(|e| e.to_string())?)
+ }
+ };
+
+ let Some(decision) = decision else {
+ event_stream.update_fields(
+ acp::ToolCallUpdateFields::new().status(acp::ToolCallStatus::InProgress),
+ );
+ return match mode {
+ EditSessionMode::Edit => Ok(()),
+ EditSessionMode::Write => Err(
+ "The user saved their unsaved changes while the prompt was visible; \
+ the file overwrite was cancelled to preserve them. Ask the user how \
+ they'd like to proceed before retrying."
+ .to_string(),
+ ),
+ };
+ };
+
+ match decision {
+ super::tool_permissions::DirtyBufferDecision::Save => {
+ context
+ .project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+ .await
+ .map_err(|e| format!("Failed to save buffer: {e}"))?;
+ }
+ super::tool_permissions::DirtyBufferDecision::Discard => {
+ context
+ .project
+ .update(cx, |project, cx| {
+ project.reload_buffers(HashSet::from_iter([buffer.clone()]), false, cx)
+ })
+ .await
+ .map_err(|e| format!("Failed to discard unsaved changes: {e}"))?;
+ }
+ super::tool_permissions::DirtyBufferDecision::Keep => {
+ let error = "The user chose to keep their unsaved changes; the file overwrite \
+ was cancelled. Ask the user how they'd like to proceed before \
+ retrying."
+ .to_string();
+ event_stream.update_fields(
+ acp::ToolCallUpdateFields::new().content(vec![error.clone().into()]),
+ );
+ return Err(error);
+ }
+ }
+ Ok(())
+}
+
fn resolve_path(
mode: EditSessionMode,
path: &PathBuf,
@@ -1,673 +0,0 @@
-use super::tool_permissions::{
- ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
- path_has_symlink_escape, resolve_project_path, sensitive_settings_kind,
-};
-use agent_client_protocol::schema as acp;
-use agent_settings::AgentSettings;
-use collections::FxHashSet;
-use futures::FutureExt as _;
-use gpui::{App, Entity, SharedString, Task};
-use language::Buffer;
-use project::Project;
-use schemars::JsonSchema;
-use serde::{Deserialize, Serialize};
-use settings::Settings;
-use std::path::{Path, PathBuf};
-use std::sync::Arc;
-use util::markdown::MarkdownInlineCode;
-
-use crate::{
- AgentTool, ToolCallEventStream, ToolInput, ToolPermissionDecision,
- authorize_with_sensitive_settings, decide_permission_for_path,
-};
-
-/// Discards unsaved changes in open buffers by reloading file contents from disk.
-///
-/// Use this tool when:
-/// - You attempted to edit files but they have unsaved changes the user does not want to keep.
-/// - You want to reset files to the on-disk state before retrying an edit.
-///
-/// Only use this tool after asking the user for permission, because it will discard unsaved changes.
-#[derive(Debug, Serialize, Deserialize, JsonSchema)]
-pub struct RestoreFileFromDiskToolInput {
- /// The paths of the files to restore from disk.
- pub paths: Vec<PathBuf>,
-}
-
-pub struct RestoreFileFromDiskTool {
- project: Entity<Project>,
-}
-
-impl RestoreFileFromDiskTool {
- pub fn new(project: Entity<Project>) -> Self {
- Self { project }
- }
-}
-
-impl AgentTool for RestoreFileFromDiskTool {
- type Input = RestoreFileFromDiskToolInput;
- type Output = String;
-
- const NAME: &'static str = "restore_file_from_disk";
-
- fn kind() -> acp::ToolKind {
- acp::ToolKind::Other
- }
-
- fn initial_title(
- &self,
- input: Result<Self::Input, serde_json::Value>,
- _cx: &mut App,
- ) -> SharedString {
- match input {
- Ok(input) if input.paths.len() == 1 => "Restore file from disk".into(),
- Ok(input) => format!("Restore {} files from disk", input.paths.len()).into(),
- Err(_) => "Restore files from disk".into(),
- }
- }
-
- fn run(
- self: Arc<Self>,
- input: ToolInput<Self::Input>,
- event_stream: ToolCallEventStream,
- cx: &mut App,
- ) -> Task<Result<String, String>> {
- let project = self.project.clone();
-
- cx.spawn(async move |cx| {
- let input = input.recv().await.map_err(|e| e.to_string())?;
-
- // Check for any immediate deny before doing async work.
- for path in &input.paths {
- let path_str = path.to_string_lossy();
- let decision = cx.update(|cx| {
- decide_permission_for_path(Self::NAME, &path_str, AgentSettings::get_global(cx))
- });
- if let ToolPermissionDecision::Deny(reason) = decision {
- return Err(reason);
- }
- }
-
- let input_paths = input.paths;
-
- let fs = project.read_with(cx, |project, _cx| project.fs().clone());
- let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
-
- let mut confirmation_paths: Vec<String> = Vec::new();
-
- for path in &input_paths {
- let path_str = path.to_string_lossy();
- let decision = cx.update(|cx| {
- decide_permission_for_path(Self::NAME, &path_str, AgentSettings::get_global(cx))
- });
- let symlink_escape = project.read_with(cx, |project, cx| {
- path_has_symlink_escape(project, path, &canonical_roots, cx)
- });
-
- match decision {
- ToolPermissionDecision::Allow => {
- if !symlink_escape {
- let is_sensitive = super::tool_permissions::is_sensitive_settings_path(
- Path::new(&*path_str),
- fs.as_ref(),
- )
- .await;
- if is_sensitive {
- confirmation_paths.push(path_str.to_string());
- }
- }
- }
- ToolPermissionDecision::Deny(reason) => {
- return Err(reason);
- }
- ToolPermissionDecision::Confirm => {
- if !symlink_escape {
- confirmation_paths.push(path_str.to_string());
- }
- }
- }
- }
-
- if !confirmation_paths.is_empty() {
- let title = if confirmation_paths.len() == 1 {
- format!(
- "Restore {} from disk",
- MarkdownInlineCode(&confirmation_paths[0])
- )
- } else {
- let paths: Vec<_> = confirmation_paths
- .iter()
- .take(3)
- .map(|p| p.as_str())
- .collect();
- if confirmation_paths.len() > 3 {
- format!(
- "Restore {}, and {} more from disk",
- paths.join(", "),
- confirmation_paths.len() - 3
- )
- } else {
- format!("Restore {} from disk", paths.join(", "))
- }
- };
-
- let mut settings_kind = None;
- for p in &confirmation_paths {
- if let Some(kind) = sensitive_settings_kind(Path::new(p), fs.as_ref()).await {
- settings_kind = Some(kind);
- break;
- }
- }
- let context = crate::ToolPermissionContext::new(Self::NAME, confirmation_paths);
- let authorize = cx.update(|cx| {
- authorize_with_sensitive_settings(
- settings_kind,
- context,
- &title,
- &event_stream,
- cx,
- )
- });
- authorize.await.map_err(|e| e.to_string())?;
- }
- let mut buffers_to_reload: FxHashSet<Entity<Buffer>> = FxHashSet::default();
-
- let mut restored_paths: Vec<PathBuf> = Vec::new();
- let mut clean_paths: Vec<PathBuf> = Vec::new();
- let mut not_found_paths: Vec<PathBuf> = Vec::new();
- let mut open_errors: Vec<(PathBuf, String)> = Vec::new();
- let dirty_check_errors: Vec<(PathBuf, String)> = Vec::new();
- let mut reload_errors: Vec<String> = Vec::new();
-
- for path in input_paths {
- let project_path = match project.read_with(cx, |project, cx| {
- resolve_project_path(project, &path, &canonical_roots, cx)
- }) {
- Ok(resolved) => {
- let (project_path, symlink_canonical_target) = match resolved {
- ResolvedProjectPath::Safe(path) => (path, None),
- ResolvedProjectPath::SymlinkEscape {
- project_path,
- canonical_target,
- } => (project_path, Some(canonical_target)),
- };
- if let Some(canonical_target) = &symlink_canonical_target {
- let path_str = path.to_string_lossy();
- let authorize_task = cx.update(|cx| {
- authorize_symlink_access(
- Self::NAME,
- &path_str,
- canonical_target,
- &event_stream,
- cx,
- )
- });
- let result = authorize_task.await;
- if let Err(err) = result {
- reload_errors.push(format!("{}: {}", path.to_string_lossy(), err));
- continue;
- }
- }
- project_path
- }
- Err(_) => {
- not_found_paths.push(path);
- continue;
- }
- };
-
- let open_buffer_task =
- project.update(cx, |project, cx| project.open_buffer(project_path, cx));
-
- let buffer = futures::select! {
- result = open_buffer_task.fuse() => {
- match result {
- Ok(buffer) => buffer,
- Err(error) => {
- open_errors.push((path, error.to_string()));
- continue;
- }
- }
- }
- _ = event_stream.cancelled_by_user().fuse() => {
- return Err("Restore cancelled by user".to_string());
- }
- };
-
- let is_dirty = buffer.read_with(cx, |buffer, _| buffer.is_dirty());
-
- if is_dirty {
- buffers_to_reload.insert(buffer);
- restored_paths.push(path);
- } else {
- clean_paths.push(path);
- }
- }
-
- if !buffers_to_reload.is_empty() {
- let reload_task = project.update(cx, |project, cx| {
- project.reload_buffers(buffers_to_reload, true, cx)
- });
-
- let result = futures::select! {
- result = reload_task.fuse() => result,
- _ = event_stream.cancelled_by_user().fuse() => {
- return Err("Restore cancelled by user".to_string());
- }
- };
- if let Err(error) = result {
- reload_errors.push(error.to_string());
- }
- }
-
- let mut lines: Vec<String> = Vec::new();
-
- if !restored_paths.is_empty() {
- lines.push(format!("Restored {} file(s).", restored_paths.len()));
- }
- if !clean_paths.is_empty() {
- lines.push(format!("{} clean.", clean_paths.len()));
- }
-
- if !not_found_paths.is_empty() {
- lines.push(format!("Not found ({}):", not_found_paths.len()));
- for path in ¬_found_paths {
- lines.push(format!("- {}", path.display()));
- }
- }
- if !open_errors.is_empty() {
- lines.push(format!("Open failed ({}):", open_errors.len()));
- for (path, error) in &open_errors {
- lines.push(format!("- {}: {}", path.display(), error));
- }
- }
- if !dirty_check_errors.is_empty() {
- lines.push(format!(
- "Dirty check failed ({}):",
- dirty_check_errors.len()
- ));
- for (path, error) in &dirty_check_errors {
- lines.push(format!("- {}: {}", path.display(), error));
- }
- }
- if !reload_errors.is_empty() {
- lines.push(format!("Reload failed ({}):", reload_errors.len()));
- for error in &reload_errors {
- lines.push(format!("- {}", error));
- }
- }
-
- if lines.is_empty() {
- Ok("No paths provided.".to_string())
- } else {
- Ok(lines.join("\n"))
- }
- })
- }
-}
-
-#[cfg(test)]
-mod tests {
- use super::*;
- use fs::Fs as _;
- use gpui::TestAppContext;
- use language::LineEnding;
- use project::FakeFs;
- use serde_json::json;
- use settings::SettingsStore;
- use util::path;
-
- fn init_test(cx: &mut TestAppContext) {
- cx.update(|cx| {
- let settings_store = SettingsStore::test(cx);
- cx.set_global(settings_store);
- });
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.default = settings::ToolPermissionMode::Allow;
- AgentSettings::override_global(settings, cx);
- });
- }
-
- #[gpui::test]
- async fn test_restore_file_from_disk_output_and_effects(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- "/root",
- json!({
- "dirty.txt": "on disk: dirty\n",
- "clean.txt": "on disk: clean\n",
- }),
- )
- .await;
-
- let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
- let tool = Arc::new(RestoreFileFromDiskTool::new(project.clone()));
-
- // Make dirty.txt dirty in-memory by saving different content into the buffer without saving to disk.
- let dirty_project_path = project.read_with(cx, |project, cx| {
- project
- .find_project_path("root/dirty.txt", cx)
- .expect("dirty.txt should exist in project")
- });
-
- let dirty_buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(dirty_project_path, cx)
- })
- .await
- .unwrap();
- dirty_buffer.update(cx, |buffer, cx| {
- buffer.edit([(0..buffer.len(), "in memory: dirty\n")], None, cx);
- });
- assert!(
- dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "dirty.txt buffer should be dirty before restore"
- );
-
- // Ensure clean.txt is opened but remains clean.
- let clean_project_path = project.read_with(cx, |project, cx| {
- project
- .find_project_path("root/clean.txt", cx)
- .expect("clean.txt should exist in project")
- });
-
- let clean_buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(clean_project_path, cx)
- })
- .await
- .unwrap();
- assert!(
- !clean_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "clean.txt buffer should start clean"
- );
-
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput {
- paths: vec![
- PathBuf::from("root/dirty.txt"),
- PathBuf::from("root/clean.txt"),
- ],
- }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
-
- // Output should mention restored + clean.
- assert!(
- output.contains("Restored 1 file(s)."),
- "expected restored count line, got:\n{output}"
- );
- assert!(
- output.contains("1 clean."),
- "expected clean count line, got:\n{output}"
- );
-
- // Effect: dirty buffer should be restored back to disk content and become clean.
- let dirty_text = dirty_buffer.read_with(cx, |buffer, _| buffer.text());
- assert_eq!(
- dirty_text, "on disk: dirty\n",
- "dirty.txt buffer should be restored to disk contents"
- );
- assert!(
- !dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "dirty.txt buffer should not be dirty after restore"
- );
-
- // Disk contents should be unchanged (restore-from-disk should not write).
- let disk_dirty = fs.load(path!("/root/dirty.txt").as_ref()).await.unwrap();
- assert_eq!(disk_dirty, "on disk: dirty\n");
-
- // Sanity: clean buffer should remain clean and unchanged.
- let clean_text = clean_buffer.read_with(cx, |buffer, _| buffer.text());
- assert_eq!(clean_text, "on disk: clean\n");
- assert!(
- !clean_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "clean.txt buffer should remain clean"
- );
-
- // Test empty paths case.
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput { paths: vec![] }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
- assert_eq!(output, "No paths provided.");
-
- // Test not-found path case (path outside the project root).
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput {
- paths: vec![PathBuf::from("nonexistent/path.txt")],
- }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
- assert!(
- output.contains("Not found (1):"),
- "expected not-found header line, got:\n{output}"
- );
- assert!(
- output.contains("- nonexistent/path.txt"),
- "expected not-found path bullet, got:\n{output}"
- );
-
- let _ = LineEnding::Unix; // keep import used if the buffer edit API changes
- }
-
- #[gpui::test]
- async fn test_restore_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(RestoreFileFromDiskTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let task = cx.update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- });
-
- cx.run_until_parked();
-
- let auth = event_rx.expect_authorization().await;
- let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
- assert!(
- title.contains("points outside the project"),
- "Expected symlink escape authorization, got: {title}",
- );
-
- auth.response
- .send(acp_thread::SelectedPermissionOutcome::new(
- acp::PermissionOptionId::new("allow"),
- acp::PermissionOptionKind::AllowOnce,
- ))
- .unwrap();
-
- let _result = task.await;
- }
-
- #[gpui::test]
- async fn test_restore_file_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
- init_test(cx);
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.tools.insert(
- "restore_file_from_disk".into(),
- agent_settings::ToolRules {
- default: Some(settings::ToolPermissionMode::Deny),
- ..Default::default()
- },
- );
- AgentSettings::override_global(settings, cx);
- });
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(RestoreFileFromDiskTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let result = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- })
- .await;
-
- assert!(result.is_err(), "Tool should fail when policy denies");
- assert!(
- !matches!(
- event_rx.try_recv(),
- Ok(Ok(crate::ThreadEvent::ToolCallAuthorization(_)))
- ),
- "Deny policy should not emit symlink authorization prompt",
- );
- }
-
- #[gpui::test]
- async fn test_restore_file_symlink_escape_confirm_requires_single_approval(
- cx: &mut TestAppContext,
- ) {
- init_test(cx);
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
- AgentSettings::override_global(settings, cx);
- });
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(RestoreFileFromDiskTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let task = cx.update(|cx| {
- tool.clone().run(
- ToolInput::resolved(RestoreFileFromDiskToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- });
-
- cx.run_until_parked();
-
- let auth = event_rx.expect_authorization().await;
- let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
- assert!(
- title.contains("points outside the project"),
- "Expected symlink escape authorization, got: {title}",
- );
-
- auth.response
- .send(acp_thread::SelectedPermissionOutcome::new(
- acp::PermissionOptionId::new("allow"),
- acp::PermissionOptionKind::AllowOnce,
- ))
- .unwrap();
-
- assert!(
- !matches!(
- event_rx.try_recv(),
- Ok(Ok(crate::ThreadEvent::ToolCallAuthorization(_)))
- ),
- "Expected a single authorization prompt",
- );
-
- let _result = task.await;
- }
-}
@@ -1,756 +0,0 @@
-use agent_client_protocol::schema as acp;
-use agent_settings::AgentSettings;
-use collections::FxHashSet;
-use futures::FutureExt as _;
-use gpui::{App, Entity, SharedString, Task};
-use language::Buffer;
-use project::Project;
-use schemars::JsonSchema;
-use serde::{Deserialize, Serialize};
-use settings::Settings;
-use std::path::{Path, PathBuf};
-use std::sync::Arc;
-use util::markdown::MarkdownInlineCode;
-
-use super::tool_permissions::{
- ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
- path_has_symlink_escape, resolve_project_path, sensitive_settings_kind,
-};
-use crate::{
- AgentTool, ToolCallEventStream, ToolInput, ToolPermissionDecision,
- authorize_with_sensitive_settings, decide_permission_for_path,
-};
-
-/// Saves files that have unsaved changes.
-///
-/// Use this tool when you need to edit files but they have unsaved changes that must be saved first.
-/// Only use this tool after asking the user for permission to save their unsaved changes.
-#[derive(Debug, Serialize, Deserialize, JsonSchema)]
-pub struct SaveFileToolInput {
- /// The paths of the files to save.
- pub paths: Vec<PathBuf>,
-}
-
-pub struct SaveFileTool {
- project: Entity<Project>,
-}
-
-impl SaveFileTool {
- pub fn new(project: Entity<Project>) -> Self {
- Self { project }
- }
-}
-
-impl AgentTool for SaveFileTool {
- type Input = SaveFileToolInput;
- type Output = String;
-
- const NAME: &'static str = "save_file";
-
- fn kind() -> acp::ToolKind {
- acp::ToolKind::Other
- }
-
- fn initial_title(
- &self,
- input: Result<Self::Input, serde_json::Value>,
- _cx: &mut App,
- ) -> SharedString {
- match input {
- Ok(input) if input.paths.len() == 1 => "Save file".into(),
- Ok(input) => format!("Save {} files", input.paths.len()).into(),
- Err(_) => "Save files".into(),
- }
- }
-
- fn run(
- self: Arc<Self>,
- input: ToolInput<Self::Input>,
- event_stream: ToolCallEventStream,
- cx: &mut App,
- ) -> Task<Result<String, String>> {
- let project = self.project.clone();
-
- cx.spawn(async move |cx| {
- let input = input.recv().await.map_err(|e| e.to_string())?;
-
- // Check for any immediate deny before doing async work.
- for path in &input.paths {
- let path_str = path.to_string_lossy();
- let decision = cx.update(|cx| {
- decide_permission_for_path(Self::NAME, &path_str, AgentSettings::get_global(cx))
- });
- if let ToolPermissionDecision::Deny(reason) = decision {
- return Err(reason);
- }
- }
-
- let input_paths = input.paths;
-
- let fs = project.read_with(cx, |project, _cx| project.fs().clone());
- let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await;
-
- let mut confirmation_paths: Vec<String> = Vec::new();
-
- for path in &input_paths {
- let path_str = path.to_string_lossy();
- let decision = cx.update(|cx| {
- decide_permission_for_path(Self::NAME, &path_str, AgentSettings::get_global(cx))
- });
- let symlink_escape = project.read_with(cx, |project, cx| {
- path_has_symlink_escape(project, path, &canonical_roots, cx)
- });
-
- match decision {
- ToolPermissionDecision::Allow => {
- if !symlink_escape {
- let is_sensitive = super::tool_permissions::is_sensitive_settings_path(
- Path::new(&*path_str),
- fs.as_ref(),
- )
- .await;
- if is_sensitive {
- confirmation_paths.push(path_str.to_string());
- }
- }
- }
- ToolPermissionDecision::Deny(reason) => {
- return Err(reason);
- }
- ToolPermissionDecision::Confirm => {
- if !symlink_escape {
- confirmation_paths.push(path_str.to_string());
- }
- }
- }
- }
-
- if !confirmation_paths.is_empty() {
- let title = if confirmation_paths.len() == 1 {
- format!("Save {}", MarkdownInlineCode(&confirmation_paths[0]))
- } else {
- let paths: Vec<_> = confirmation_paths
- .iter()
- .take(3)
- .map(|p| p.as_str())
- .collect();
- if confirmation_paths.len() > 3 {
- format!(
- "Save {}, and {} more",
- paths.join(", "),
- confirmation_paths.len() - 3
- )
- } else {
- format!("Save {}", paths.join(", "))
- }
- };
-
- let mut settings_kind = None;
- for p in &confirmation_paths {
- if let Some(kind) = sensitive_settings_kind(Path::new(p), fs.as_ref()).await {
- settings_kind = Some(kind);
- break;
- }
- }
- let context =
- crate::ToolPermissionContext::new(Self::NAME, confirmation_paths.clone());
- let authorize = cx.update(|cx| {
- authorize_with_sensitive_settings(
- settings_kind,
- context,
- &title,
- &event_stream,
- cx,
- )
- });
- authorize.await.map_err(|e| e.to_string())?;
- }
-
- let mut buffers_to_save: FxHashSet<Entity<Buffer>> = FxHashSet::default();
-
- let mut dirty_count: usize = 0;
- let mut clean_paths: Vec<PathBuf> = Vec::new();
- let mut not_found_paths: Vec<PathBuf> = Vec::new();
- let mut open_errors: Vec<(PathBuf, String)> = Vec::new();
- let mut authorization_errors: Vec<(PathBuf, String)> = Vec::new();
- let mut save_errors: Vec<(String, String)> = Vec::new();
-
- for path in input_paths {
- let project_path = match project.read_with(cx, |project, cx| {
- resolve_project_path(project, &path, &canonical_roots, cx)
- }) {
- Ok(resolved) => {
- let (project_path, symlink_canonical_target) = match resolved {
- ResolvedProjectPath::Safe(path) => (path, None),
- ResolvedProjectPath::SymlinkEscape {
- project_path,
- canonical_target,
- } => (project_path, Some(canonical_target)),
- };
- if let Some(canonical_target) = &symlink_canonical_target {
- let path_str = path.to_string_lossy();
- let authorize_task = cx.update(|cx| {
- authorize_symlink_access(
- Self::NAME,
- &path_str,
- canonical_target,
- &event_stream,
- cx,
- )
- });
- let result = authorize_task.await;
- if let Err(err) = result {
- authorization_errors.push((path.clone(), err.to_string()));
- continue;
- }
- }
- project_path
- }
- Err(_) => {
- not_found_paths.push(path);
- continue;
- }
- };
-
- let open_buffer_task =
- project.update(cx, |project, cx| project.open_buffer(project_path, cx));
-
- let buffer = futures::select! {
- result = open_buffer_task.fuse() => {
- match result {
- Ok(buffer) => buffer,
- Err(error) => {
- open_errors.push((path, error.to_string()));
- continue;
- }
- }
- }
- _ = event_stream.cancelled_by_user().fuse() => {
- return Err("Save cancelled by user".to_string());
- }
- };
-
- let is_dirty = buffer.read_with(cx, |buffer, _| buffer.is_dirty());
-
- if is_dirty {
- buffers_to_save.insert(buffer);
- dirty_count += 1;
- } else {
- clean_paths.push(path);
- }
- }
-
- // Save each buffer individually since there's no batch save API.
- for buffer in buffers_to_save {
- let path_for_buffer = buffer
- .read_with(cx, |buffer, _| {
- buffer
- .file()
- .map(|file| file.path().to_rel_path_buf())
- .map(|path| path.as_rel_path().as_unix_str().to_owned())
- })
- .unwrap_or_else(|| "<unknown>".to_string());
-
- let save_task = project.update(cx, |project, cx| project.save_buffer(buffer, cx));
-
- let save_result = futures::select! {
- result = save_task.fuse() => result,
- _ = event_stream.cancelled_by_user().fuse() => {
- return Err("Save cancelled by user".to_string());
- }
- };
- if let Err(error) = save_result {
- save_errors.push((path_for_buffer, error.to_string()));
- }
- }
-
- let mut lines: Vec<String> = Vec::new();
-
- let successful_saves = dirty_count.saturating_sub(save_errors.len());
- if successful_saves > 0 {
- lines.push(format!("Saved {} file(s).", successful_saves));
- }
- if !clean_paths.is_empty() {
- lines.push(format!("{} clean.", clean_paths.len()));
- }
-
- if !not_found_paths.is_empty() {
- lines.push(format!("Not found ({}):", not_found_paths.len()));
- for path in ¬_found_paths {
- lines.push(format!("- {}", path.display()));
- }
- }
- if !open_errors.is_empty() {
- lines.push(format!("Open failed ({}):", open_errors.len()));
- for (path, error) in &open_errors {
- lines.push(format!("- {}: {}", path.display(), error));
- }
- }
- if !authorization_errors.is_empty() {
- lines.push(format!(
- "Authorization failed ({}):",
- authorization_errors.len()
- ));
- for (path, error) in &authorization_errors {
- lines.push(format!("- {}: {}", path.display(), error));
- }
- }
- if !save_errors.is_empty() {
- lines.push(format!("Save failed ({}):", save_errors.len()));
- for (path, error) in &save_errors {
- lines.push(format!("- {}: {}", path, error));
- }
- }
-
- if lines.is_empty() {
- Ok("No paths provided.".to_string())
- } else {
- Ok(lines.join("\n"))
- }
- })
- }
-}
-
-#[cfg(test)]
-mod tests {
- use super::*;
- use fs::Fs as _;
- use gpui::TestAppContext;
- use project::FakeFs;
- use serde_json::json;
- use settings::SettingsStore;
- use util::path;
-
- fn init_test(cx: &mut TestAppContext) {
- cx.update(|cx| {
- let settings_store = SettingsStore::test(cx);
- cx.set_global(settings_store);
- });
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.default = settings::ToolPermissionMode::Allow;
- AgentSettings::override_global(settings, cx);
- });
- }
-
- #[gpui::test]
- async fn test_save_file_output_and_effects(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- "/root",
- json!({
- "dirty.txt": "on disk: dirty\n",
- "clean.txt": "on disk: clean\n",
- }),
- )
- .await;
-
- let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
- let tool = Arc::new(SaveFileTool::new(project.clone()));
-
- // Make dirty.txt dirty in-memory.
- let dirty_project_path = project.read_with(cx, |project, cx| {
- project
- .find_project_path("root/dirty.txt", cx)
- .expect("dirty.txt should exist in project")
- });
-
- let dirty_buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(dirty_project_path, cx)
- })
- .await
- .unwrap();
- dirty_buffer.update(cx, |buffer, cx| {
- buffer.edit([(0..buffer.len(), "in memory: dirty\n")], None, cx);
- });
- assert!(
- dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "dirty.txt buffer should be dirty before save"
- );
-
- // Ensure clean.txt is opened but remains clean.
- let clean_project_path = project.read_with(cx, |project, cx| {
- project
- .find_project_path("root/clean.txt", cx)
- .expect("clean.txt should exist in project")
- });
-
- let clean_buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(clean_project_path, cx)
- })
- .await
- .unwrap();
- assert!(
- !clean_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "clean.txt buffer should start clean"
- );
-
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![
- PathBuf::from("root/dirty.txt"),
- PathBuf::from("root/clean.txt"),
- ],
- }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
-
- // Output should mention saved + clean.
- assert!(
- output.contains("Saved 1 file(s)."),
- "expected saved count line, got:\n{output}"
- );
- assert!(
- output.contains("1 clean."),
- "expected clean count line, got:\n{output}"
- );
-
- // Effect: dirty buffer should now be clean and disk should have new content.
- assert!(
- !dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "dirty.txt buffer should not be dirty after save"
- );
-
- let disk_dirty = fs.load(path!("/root/dirty.txt").as_ref()).await.unwrap();
- assert_eq!(
- disk_dirty, "in memory: dirty\n",
- "dirty.txt disk content should be updated"
- );
-
- // Sanity: clean buffer should remain clean and disk unchanged.
- let disk_clean = fs.load(path!("/root/clean.txt").as_ref()).await.unwrap();
- assert_eq!(disk_clean, "on disk: clean\n");
-
- // Test empty paths case.
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput { paths: vec![] }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
- assert_eq!(output, "No paths provided.");
-
- // Test not-found path case.
- let output = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![PathBuf::from("nonexistent/path.txt")],
- }),
- ToolCallEventStream::test().0,
- cx,
- )
- })
- .await
- .unwrap();
- assert!(
- output.contains("Not found (1):"),
- "expected not-found header line, got:\n{output}"
- );
- assert!(
- output.contains("- nonexistent/path.txt"),
- "expected not-found path bullet, got:\n{output}"
- );
- }
-
- #[gpui::test]
- async fn test_save_file_symlink_escape_requests_authorization(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(SaveFileTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let task = cx.update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- });
-
- cx.run_until_parked();
-
- let auth = event_rx.expect_authorization().await;
- let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
- assert!(
- title.contains("points outside the project"),
- "Expected symlink escape authorization, got: {title}",
- );
-
- auth.response
- .send(acp_thread::SelectedPermissionOutcome::new(
- acp::PermissionOptionId::new("allow"),
- acp::PermissionOptionKind::AllowOnce,
- ))
- .unwrap();
-
- let _result = task.await;
- }
-
- #[gpui::test]
- async fn test_save_file_symlink_escape_honors_deny_policy(cx: &mut TestAppContext) {
- init_test(cx);
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.tools.insert(
- "save_file".into(),
- agent_settings::ToolRules {
- default: Some(settings::ToolPermissionMode::Deny),
- ..Default::default()
- },
- );
- AgentSettings::override_global(settings, cx);
- });
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(SaveFileTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let result = cx
- .update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- })
- .await;
-
- assert!(result.is_err(), "Tool should fail when policy denies");
- assert!(
- !matches!(
- event_rx.try_recv(),
- Ok(Ok(crate::ThreadEvent::ToolCallAuthorization(_)))
- ),
- "Deny policy should not emit symlink authorization prompt",
- );
- }
-
- #[gpui::test]
- async fn test_save_file_symlink_escape_confirm_requires_single_approval(
- cx: &mut TestAppContext,
- ) {
- init_test(cx);
- cx.update(|cx| {
- let mut settings = AgentSettings::get_global(cx).clone();
- settings.tool_permissions.default = settings::ToolPermissionMode::Confirm;
- AgentSettings::override_global(settings, cx);
- });
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "src": {}
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let tool = Arc::new(SaveFileTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let task = cx.update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![PathBuf::from("project/link.txt")],
- }),
- event_stream,
- cx,
- )
- });
-
- cx.run_until_parked();
-
- let auth = event_rx.expect_authorization().await;
- let title = auth.tool_call.fields.title.as_deref().unwrap_or("");
- assert!(
- title.contains("points outside the project"),
- "Expected symlink escape authorization, got: {title}",
- );
-
- auth.response
- .send(acp_thread::SelectedPermissionOutcome::new(
- acp::PermissionOptionId::new("allow"),
- acp::PermissionOptionKind::AllowOnce,
- ))
- .unwrap();
-
- assert!(
- !matches!(
- event_rx.try_recv(),
- Ok(Ok(crate::ThreadEvent::ToolCallAuthorization(_)))
- ),
- "Expected a single authorization prompt",
- );
-
- let _result = task.await;
- }
-
- #[gpui::test]
- async fn test_save_file_symlink_denial_does_not_reduce_success_count(cx: &mut TestAppContext) {
- init_test(cx);
-
- let fs = FakeFs::new(cx.executor());
- fs.insert_tree(
- path!("/root"),
- json!({
- "project": {
- "dirty.txt": "on disk value\n",
- },
- "external": {
- "secret.txt": "secret content"
- }
- }),
- )
- .await;
-
- fs.create_symlink(
- path!("/root/project/link.txt").as_ref(),
- PathBuf::from("../external/secret.txt"),
- )
- .await
- .unwrap();
-
- let project = Project::test(fs.clone(), [path!("/root/project").as_ref()], cx).await;
- cx.executor().run_until_parked();
-
- let dirty_project_path = project.read_with(cx, |project, cx| {
- project
- .find_project_path("project/dirty.txt", cx)
- .expect("dirty.txt should exist in project")
- });
- let dirty_buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(dirty_project_path, cx)
- })
- .await
- .unwrap();
- dirty_buffer.update(cx, |buffer, cx| {
- buffer.edit([(0..buffer.len(), "in memory value\n")], None, cx);
- });
- assert!(
- dirty_buffer.read_with(cx, |buffer, _| buffer.is_dirty()),
- "dirty.txt should be dirty before save"
- );
-
- let tool = Arc::new(SaveFileTool::new(project));
-
- let (event_stream, mut event_rx) = ToolCallEventStream::test();
- let task = cx.update(|cx| {
- tool.clone().run(
- ToolInput::resolved(SaveFileToolInput {
- paths: vec![
- PathBuf::from("project/dirty.txt"),
- PathBuf::from("project/link.txt"),
- ],
- }),
- event_stream,
- cx,
- )
- });
-
- cx.run_until_parked();
-
- let auth = event_rx.expect_authorization().await;
- auth.response
- .send(acp_thread::SelectedPermissionOutcome::new(
- acp::PermissionOptionId::new("deny"),
- acp::PermissionOptionKind::RejectOnce,
- ))
- .unwrap();
-
- let output = task.await.unwrap();
- assert!(
- output.contains("Saved 1 file(s)."),
- "Expected successful save count to remain accurate, got:\n{output}",
- );
- assert!(
- output.contains("Authorization failed (1):"),
- "Expected authorization failure section, got:\n{output}",
- );
- assert!(
- !output.contains("Save failed"),
- "Authorization denials should not be counted as save failures, got:\n{output}",
- );
- }
-}
@@ -2,6 +2,7 @@ use crate::{
Thread, ToolCallEventStream, ToolPermissionContext, ToolPermissionDecision,
decide_permission_for_path,
};
+use agent_client_protocol::schema as acp;
use anyhow::{Result, anyhow};
use fs::Fs;
use gpui::{App, Entity, Task, WeakEntity};
@@ -521,6 +522,91 @@ pub fn authorize_file_edit(
})
}
+/// The user's choice when prompted about how to handle unsaved changes
+/// in a buffer that the agent wants to edit or overwrite.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DirtyBufferDecision {
+ /// Save the buffer's pending edits to disk, then proceed.
+ /// (Edit-mode prompt only.)
+ Save,
+ /// Discard the buffer's pending edits (reload from disk), then proceed.
+ Discard,
+ /// Keep the buffer's pending edits and cancel the agent's operation.
+ /// (Overwrite-mode prompt only.)
+ Keep,
+}
+
+/// Which prompt to show when the agent encounters a dirty buffer.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DirtyBufferPromptKind {
+ /// The agent wants to apply targeted edits on top of the current
+ /// content. Offers Save (persist edits, then edit on top) vs Discard
+ /// (revert to disk, then edit).
+ Edit,
+ /// The agent wants to overwrite the file's entire contents. Offers
+ /// Keep (cancel the overwrite to preserve the user's work) vs
+ /// Discard (reload from disk and let the agent overwrite).
+ Overwrite,
+}
+
+/// Prompts the user about how to handle a dirty buffer that the agent
+/// wants to edit or overwrite. Returns the chosen action; the caller is
+/// responsible for actually performing the corresponding side effect
+/// (save / reload / cancel) before continuing.
+pub fn authorize_dirty_buffer(
+ kind: DirtyBufferPromptKind,
+ event_stream: &ToolCallEventStream,
+ cx: &mut App,
+) -> Task<Result<DirtyBufferDecision>> {
+ let (message, options) = match kind {
+ DirtyBufferPromptKind::Edit => (
+ "This file has unsaved changes. Do you want to save or discard them \
+ before the agent continues editing?"
+ .to_string(),
+ vec![
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("save"),
+ "Save",
+ acp::PermissionOptionKind::AllowOnce,
+ ),
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("discard"),
+ "Discard",
+ acp::PermissionOptionKind::RejectOnce,
+ ),
+ ],
+ ),
+ DirtyBufferPromptKind::Overwrite => (
+ "This file has unsaved changes and the agent wants to overwrite it.".to_string(),
+ vec![
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("discard"),
+ "Overwrite",
+ acp::PermissionOptionKind::AllowOnce,
+ ),
+ acp::PermissionOption::new(
+ acp::PermissionOptionId::new("keep"),
+ "Cancel",
+ acp::PermissionOptionKind::RejectOnce,
+ ),
+ ],
+ ),
+ };
+
+ let prompt = event_stream.prompt_for_decision(None, Some(message), options, cx);
+ cx.spawn(async move |_cx| {
+ let option_id = prompt.await?;
+ match option_id.0.as_ref() {
+ "save" => Ok(DirtyBufferDecision::Save),
+ "discard" => Ok(DirtyBufferDecision::Discard),
+ "keep" => Ok(DirtyBufferDecision::Keep),
+ other => Err(anyhow!(
+ "Unexpected dirty-buffer decision option_id: {other}"
+ )),
+ }
+ })
+}
+
#[cfg(test)]
mod tests {
use super::*;
@@ -1091,6 +1091,212 @@ mod tests {
);
}
+ /// When the buffer has unsaved user edits and the user picks
+ /// "Discard my edits", the pending edits are reverted to match disk
+ /// and the agent's overwrite proceeds.
+ #[gpui::test]
+ async fn test_streaming_write_dirty_buffer_discard(cx: &mut TestAppContext) {
+ let (write_tool, project, _action_log, fs, _thread) =
+ setup_test(cx, json!({"file.txt": "on disk content"})).await;
+
+ let project_path = project
+ .read_with(cx, |project, cx| {
+ project.find_project_path("root/file.txt", cx)
+ })
+ .expect("Should find project path");
+ let buffer = project
+ .update(cx, |project, cx| project.open_buffer(project_path, cx))
+ .await
+ .unwrap();
+ buffer.update(cx, |buffer, cx| {
+ let end_point = buffer.max_point();
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
+ });
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ write_tool.clone().run(
+ ToolInput::resolved(WriteFileToolInput {
+ path: "root/file.txt".into(),
+ content: "agent overwrote it".into(),
+ }),
+ stream_tx,
+ cx,
+ )
+ });
+
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+
+ // Verify the prompt is the overwrite-mode prompt.
+ let content = auth.tool_call.fields.content.as_deref().unwrap_or(&[]);
+ let acp::ToolCallContent::Content(text) = content.first().expect("expected message body")
+ else {
+ panic!("expected text body, got: {:?}", content.first());
+ };
+ let acp::ContentBlock::Text(text) = &text.content else {
+ panic!("expected text body, got: {:?}", text.content);
+ };
+ assert!(
+ text.text.contains("overwrite"),
+ "expected overwrite-mode prompt, got: {:?}",
+ text.text,
+ );
+
+ // Verify both option ids are present (option_id is the stable contract).
+ let option_ids: Vec<&str> = match &auth.options {
+ acp_thread::PermissionOptions::Flat(opts) => {
+ opts.iter().map(|o| o.option_id.0.as_ref()).collect()
+ }
+ other => panic!("expected flat options, got: {other:?}"),
+ };
+ assert!(option_ids.contains(&"keep"), "options: {option_ids:?}");
+ assert!(option_ids.contains(&"discard"), "options: {option_ids:?}");
+
+ auth.response
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("discard"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
+ .unwrap();
+
+ let EditSessionOutput::Success { new_text, .. } = task.await.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "agent overwrote it");
+ assert!(!buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let on_disk = fs.load(path!("/root/file.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "agent overwrote it");
+ }
+
+ /// When the buffer has unsaved user edits and the user picks
+ /// "Keep my edits", the overwrite is cancelled with an error and the
+ /// user's pending edits are preserved.
+ #[gpui::test]
+ async fn test_streaming_write_dirty_buffer_keep(cx: &mut TestAppContext) {
+ let (write_tool, project, _action_log, fs, _thread) =
+ setup_test(cx, json!({"file.txt": "on disk content"})).await;
+
+ let project_path = project
+ .read_with(cx, |project, cx| {
+ project.find_project_path("root/file.txt", cx)
+ })
+ .expect("Should find project path");
+ let buffer = project
+ .update(cx, |project, cx| project.open_buffer(project_path, cx))
+ .await
+ .unwrap();
+ buffer.update(cx, |buffer, cx| {
+ let end_point = buffer.max_point();
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
+ });
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ write_tool.clone().run(
+ ToolInput::resolved(WriteFileToolInput {
+ path: "root/file.txt".into(),
+ content: "agent overwrote it".into(),
+ }),
+ stream_tx,
+ cx,
+ )
+ });
+
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+ auth.response
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("keep"),
+ acp::PermissionOptionKind::RejectOnce,
+ ))
+ .unwrap();
+
+ let EditSessionOutput::Error { error, .. } = task.await.unwrap_err() else {
+ panic!("expected error");
+ };
+ assert!(
+ error.contains("keep") || error.contains("cancelled"),
+ "expected cancel-style error message, got: {error:?}",
+ );
+
+ // The user's in-memory edits are preserved.
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let buffer_text = buffer.read_with(cx, |buffer, _| buffer.text());
+ assert_eq!(buffer_text, "on disk content plus user edit");
+
+ // The on-disk content is untouched.
+ let on_disk = fs.load(path!("/root/file.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "on disk content");
+ }
+
+ /// When the user manually saves the buffer (e.g. cmd-s) while the
+ /// overwrite prompt is visible, that's treated as "Keep my edits":
+ /// the user just deliberately persisted their work, so we cancel the
+ /// agent's overwrite to avoid clobbering it.
+ #[gpui::test]
+ async fn test_streaming_write_dirty_buffer_resolved_externally(cx: &mut TestAppContext) {
+ let (write_tool, project, _action_log, fs, _thread) =
+ setup_test(cx, json!({"file.txt": "on disk content"})).await;
+
+ let project_path = project
+ .read_with(cx, |project, cx| {
+ project.find_project_path("root/file.txt", cx)
+ })
+ .expect("Should find project path");
+ let buffer = project
+ .update(cx, |project, cx| project.open_buffer(project_path, cx))
+ .await
+ .unwrap();
+ buffer.update(cx, |buffer, cx| {
+ let end_point = buffer.max_point();
+ buffer.edit([(end_point..end_point, " plus user edit")], None, cx);
+ });
+ assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+
+ let (stream_tx, mut stream_rx) = ToolCallEventStream::test();
+ let task = cx.update(|cx| {
+ write_tool.clone().run(
+ ToolInput::resolved(WriteFileToolInput {
+ path: "root/file.txt".into(),
+ content: "agent overwrote it".into(),
+ }),
+ stream_tx,
+ cx,
+ )
+ });
+
+ let _update = stream_rx.expect_update_fields().await;
+ let auth = stream_rx.expect_authorization().await;
+
+ // User saves manually while the prompt is up.
+ project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+ .await
+ .unwrap();
+
+ // The prompt is dismissed by transitioning to InProgress.
+ let dismiss = stream_rx.expect_update_fields().await;
+ assert_eq!(dismiss.status, Some(acp::ToolCallStatus::InProgress));
+ drop(auth);
+
+ // The overwrite is cancelled with an error.
+ let EditSessionOutput::Error { error, .. } = task.await.unwrap_err() else {
+ panic!("expected error");
+ };
+ assert!(
+ error.contains("saved") || error.contains("cancelled"),
+ "expected cancel-on-manual-save error, got: {error:?}",
+ );
+
+ // The user's edits were saved to disk and not clobbered.
+ assert!(!buffer.read_with(cx, |buffer, _| buffer.is_dirty()));
+ let on_disk = fs.load(path!("/root/file.txt").as_ref()).await.unwrap();
+ assert_eq!(on_disk, "on disk content plus user edit");
+ }
+
async fn setup_test_with_fs(
cx: &mut TestAppContext,
fs: Arc<project::FakeFs>,
@@ -3359,6 +3359,7 @@ fn handle_request_permission(
thread.request_tool_call_authorization(
args.tool_call,
acp_thread::PermissionOptions::Flat(args.options),
+ acp_thread::AuthorizationKind::PermissionGrant,
cx,
)
})
@@ -7120,6 +7120,7 @@ pub(crate) mod tests {
"Allow",
acp::PermissionOptionKind::AllowOnce,
)]),
+ acp_thread::AuthorizationKind::PermissionGrant,
cx,
)
.unwrap()
@@ -15,7 +15,6 @@ pub(crate) use tool_permissions_setup::render_tool_permissions_setup_page;
pub use tool_permissions_setup::{
render_copy_path_tool_config, render_create_directory_tool_config,
render_delete_path_tool_config, render_edit_file_tool_config, render_fetch_tool_config,
- render_move_path_tool_config, render_restore_file_from_disk_tool_config,
- render_save_file_tool_config, render_terminal_tool_config, render_web_search_tool_config,
+ render_move_path_tool_config, render_terminal_tool_config, render_web_search_tool_config,
render_write_file_tool_config,
};
@@ -62,12 +62,6 @@ const TOOLS: &[ToolInfo] = &[
description: "Directory creation",
regex_explanation: "Patterns are matched against the directory path being created.",
},
- ToolInfo {
- id: "save_file",
- name: "Save File",
- description: "File saving operations",
- regex_explanation: "Patterns are matched against the file path being saved.",
- },
ToolInfo {
id: "fetch",
name: "Fetch",
@@ -80,12 +74,6 @@ const TOOLS: &[ToolInfo] = &[
description: "Web search queries",
regex_explanation: "Patterns are matched against the search query.",
},
- ToolInfo {
- id: "restore_file_from_disk",
- name: "Restore File from Disk",
- description: "Discards unsaved changes by reloading from disk",
- regex_explanation: "Patterns are matched against the file path being restored.",
- },
];
pub(crate) struct ToolInfo {
@@ -314,10 +302,8 @@ fn get_tool_render_fn(
"copy_path" => render_copy_path_tool_config,
"move_path" => render_move_path_tool_config,
"create_directory" => render_create_directory_tool_config,
- "save_file" => render_save_file_tool_config,
"fetch" => render_fetch_tool_config,
"search_web" => render_web_search_tool_config,
- "restore_file_from_disk" => render_restore_file_from_disk_tool_config,
_ => render_terminal_tool_config, // fallback
}
}
@@ -1395,13 +1381,8 @@ tool_config_page_fn!(render_delete_path_tool_config, "delete_path");
tool_config_page_fn!(render_copy_path_tool_config, "copy_path");
tool_config_page_fn!(render_move_path_tool_config, "move_path");
tool_config_page_fn!(render_create_directory_tool_config, "create_directory");
-tool_config_page_fn!(render_save_file_tool_config, "save_file");
tool_config_page_fn!(render_fetch_tool_config, "fetch");
tool_config_page_fn!(render_web_search_tool_config, "search_web");
-tool_config_page_fn!(
- render_restore_file_from_disk_tool_config,
- "restore_file_from_disk"
-);
#[cfg(test)]
mod tests {
@@ -444,6 +444,7 @@ fn request_test_tool_authorization(
"Allow",
acp::PermissionOptionKind::AllowOnce,
)]),
+ acp_thread::AuthorizationKind::PermissionGrant,
cx,
)
.unwrap()