agent: Improve UX when agent tries to edit unsaved buffer (#55655)
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>
This commit is contained in:
1 parent
4f73a61cab
commit
a6b56268bf
18 files changed
+713
-1665
No files matched your search
@@ -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 (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 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 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
|
||||
text.text.contains("unsaved changes")
|
||||
&& text.text.contains("save")
|
||||
&& text.text.contains("discard"),
|
||||
"unexpected message body: {:?}",
|
||||
text.text,
|
||||
);
|
||||
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
|
||||
);
|
||||
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()
|
||||
|
||||
Reference in new issue
Block a user