Add a test for remote tool use by the agent (#30289)

- Adds a new smoke test for the use of the read_file tool by the agent
in an SSH project
- Fixes the SSH shutdown sequence to use a timer from the app's executor
instead of always using a real timer
- Changes the main executor loop for tests to advance the clock
automatically instead of panicking with `parked with nothing left to
run` when there is a delayed task

Release Notes:

- N/A
This commit is contained in:
Cole Miller 2025-05-08 16:53:04 -04:00 committed by GitHub
parent 660b4cee76
commit 8b764a5477
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 104 additions and 13 deletions

3
Cargo.lock generated
View file

@ -11952,6 +11952,8 @@ version = "0.1.0"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"askpass", "askpass",
"assistant_tool",
"assistant_tools",
"async-watch", "async-watch",
"backtrace", "backtrace",
"cargo_toml", "cargo_toml",
@ -11974,6 +11976,7 @@ dependencies = [
"http_client", "http_client",
"language", "language",
"language_extension", "language_extension",
"language_model",
"languages", "languages",
"libc", "libc",
"log", "log",

View file

@ -40,13 +40,12 @@ use crate::find_path_tool::FindPathTool;
use crate::grep_tool::GrepTool; use crate::grep_tool::GrepTool;
use crate::list_directory_tool::ListDirectoryTool; use crate::list_directory_tool::ListDirectoryTool;
use crate::now_tool::NowTool; use crate::now_tool::NowTool;
use crate::read_file_tool::ReadFileTool;
use crate::thinking_tool::ThinkingTool; use crate::thinking_tool::ThinkingTool;
pub use edit_file_tool::EditFileToolInput; pub use edit_file_tool::EditFileToolInput;
pub use find_path_tool::FindPathToolInput; pub use find_path_tool::FindPathToolInput;
pub use open_tool::OpenTool; pub use open_tool::OpenTool;
pub use read_file_tool::ReadFileToolInput; pub use read_file_tool::{ReadFileTool, ReadFileToolInput};
pub use terminal_tool::TerminalTool; pub use terminal_tool::TerminalTool;
pub fn init(http_client: Arc<HttpClientWithUrl>, cx: &mut App) { pub fn init(http_client: Arc<HttpClientWithUrl>, cx: &mut App) {

View file

@ -581,7 +581,11 @@ async fn test_ssh_collaboration_formatting_with_prettier(
} }
#[gpui::test] #[gpui::test]
async fn test_remote_server_debugger(cx_a: &mut TestAppContext, server_cx: &mut TestAppContext) { async fn test_remote_server_debugger(
cx_a: &mut TestAppContext,
server_cx: &mut TestAppContext,
executor: BackgroundExecutor,
) {
cx_a.update(|cx| { cx_a.update(|cx| {
release_channel::init(SemanticVersion::default(), cx); release_channel::init(SemanticVersion::default(), cx);
command_palette_hooks::init(cx); command_palette_hooks::init(cx);
@ -679,7 +683,7 @@ async fn test_remote_server_debugger(cx_a: &mut TestAppContext, server_cx: &mut
}); });
client_ssh.update(cx_a, |a, _| { client_ssh.update(cx_a, |a, _| {
a.shutdown_processes(Some(proto::ShutdownRemoteServer {})) a.shutdown_processes(Some(proto::ShutdownRemoteServer {}), executor)
}); });
shutdown_session.await.unwrap(); shutdown_session.await.unwrap();

View file

@ -281,6 +281,9 @@ impl BackgroundExecutor {
} }
if !dispatcher.parking_allowed() { if !dispatcher.parking_allowed() {
if dispatcher.advance_clock_to_next_delayed() {
continue;
}
let mut backtrace_message = String::new(); let mut backtrace_message = String::new();
let mut waiting_message = String::new(); let mut waiting_message = String::new();
if let Some(backtrace) = dispatcher.waiting_backtrace() { if let Some(backtrace) = dispatcher.waiting_backtrace() {

View file

@ -89,6 +89,15 @@ impl TestDispatcher {
self.state.lock().time = new_now; self.state.lock().time = new_now;
} }
pub fn advance_clock_to_next_delayed(&self) -> bool {
let next_due_time = self.state.lock().delayed.first().map(|(time, _)| *time);
if let Some(next_due_time) = next_due_time {
self.state.lock().time = next_due_time;
return true;
}
false
}
pub fn simulate_random_delay(&self) -> impl 'static + Send + Future<Output = ()> + use<> { pub fn simulate_random_delay(&self) -> impl 'static + Send + Future<Output = ()> + use<> {
struct YieldNow { struct YieldNow {
pub(crate) count: usize, pub(crate) count: usize,

View file

@ -1130,9 +1130,10 @@ impl Project {
cx.on_release(Self::release), cx.on_release(Self::release),
cx.on_app_quit(|this, cx| { cx.on_app_quit(|this, cx| {
let shutdown = this.ssh_client.take().and_then(|client| { let shutdown = this.ssh_client.take().and_then(|client| {
client client.read(cx).shutdown_processes(
.read(cx) Some(proto::ShutdownRemoteServer {}),
.shutdown_processes(Some(proto::ShutdownRemoteServer {})) cx.background_executor().clone(),
)
}); });
cx.background_executor().spawn(async move { cx.background_executor().spawn(async move {
@ -1472,9 +1473,10 @@ impl Project {
fn release(&mut self, cx: &mut App) { fn release(&mut self, cx: &mut App) {
if let Some(client) = self.ssh_client.take() { if let Some(client) = self.ssh_client.take() {
let shutdown = client let shutdown = client.read(cx).shutdown_processes(
.read(cx) Some(proto::ShutdownRemoteServer {}),
.shutdown_processes(Some(proto::ShutdownRemoteServer {})); cx.background_executor().clone(),
);
cx.background_spawn(async move { cx.background_spawn(async move {
if let Some(shutdown) = shutdown { if let Some(shutdown) = shutdown {

View file

@ -18,8 +18,8 @@ use futures::{
select, select_biased, select, select_biased,
}; };
use gpui::{ use gpui::{
App, AppContext as _, AsyncApp, BorrowAppContext, Context, Entity, EventEmitter, Global, App, AppContext as _, AsyncApp, BackgroundExecutor, BorrowAppContext, Context, Entity,
SemanticVersion, Task, WeakEntity, EventEmitter, Global, SemanticVersion, Task, WeakEntity,
}; };
use itertools::Itertools; use itertools::Itertools;
use parking_lot::Mutex; use parking_lot::Mutex;
@ -683,6 +683,7 @@ impl SshRemoteClient {
pub fn shutdown_processes<T: RequestMessage>( pub fn shutdown_processes<T: RequestMessage>(
&self, &self,
shutdown_request: Option<T>, shutdown_request: Option<T>,
executor: BackgroundExecutor,
) -> Option<impl Future<Output = ()> + use<T>> { ) -> Option<impl Future<Output = ()> + use<T>> {
let state = self.state.lock().take()?; let state = self.state.lock().take()?;
log::info!("shutting down ssh processes"); log::info!("shutting down ssh processes");
@ -705,7 +706,7 @@ impl SshRemoteClient {
// We wait 50ms instead of waiting for a response, because // We wait 50ms instead of waiting for a response, because
// waiting for a response would require us to wait on the main thread // waiting for a response would require us to wait on the main thread
// which we want to avoid in an `on_app_quit` callback. // which we want to avoid in an `on_app_quit` callback.
smol::Timer::after(Duration::from_millis(50)).await; executor.timer(Duration::from_millis(50)).await;
} }
// Drop `multiplex_task` because it owns our ssh_proxy_process, which is a // Drop `multiplex_task` because it owns our ssh_proxy_process, which is a

View file

@ -69,6 +69,8 @@ fork.workspace = true
libc.workspace = true libc.workspace = true
[dev-dependencies] [dev-dependencies]
assistant_tool.workspace = true
assistant_tools.workspace = true
client = { workspace = true, features = ["test-support"] } client = { workspace = true, features = ["test-support"] }
clock = { workspace = true, features = ["test-support"] } clock = { workspace = true, features = ["test-support"] }
dap = { workspace = true, features = ["test-support"] } dap = { workspace = true, features = ["test-support"] }
@ -79,6 +81,7 @@ language = { workspace = true, features = ["test-support"] }
node_runtime = { workspace = true, features = ["test-support"] } node_runtime = { workspace = true, features = ["test-support"] }
project = { workspace = true, features = ["test-support"] } project = { workspace = true, features = ["test-support"] }
remote = { workspace = true, features = ["test-support"] } remote = { workspace = true, features = ["test-support"] }
language_model = { workspace = true, features = ["test-support"] }
lsp = { workspace = true, features=["test-support"] } lsp = { workspace = true, features=["test-support"] }
unindent.workspace = true unindent.workspace = true
serde_json.workspace = true serde_json.workspace = true

View file

@ -2,8 +2,11 @@
/// The tests in this file assume that server_cx is running on Windows too. /// The tests in this file assume that server_cx is running on Windows too.
/// We neead to find a way to test Windows-Non-Windows interactions. /// We neead to find a way to test Windows-Non-Windows interactions.
use crate::headless_project::HeadlessProject; use crate::headless_project::HeadlessProject;
use assistant_tool::Tool as _;
use assistant_tools::{ReadFileTool, ReadFileToolInput};
use client::{Client, UserStore}; use client::{Client, UserStore};
use clock::FakeSystemClock; use clock::FakeSystemClock;
use language_model::{LanguageModelRequest, fake_provider::FakeLanguageModel};
use extension::ExtensionHostProxy; use extension::ExtensionHostProxy;
use fs::{FakeFs, Fs}; use fs::{FakeFs, Fs};
@ -1548,6 +1551,70 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA
assert_eq!(server_branch.name(), "totally-new-branch"); assert_eq!(server_branch.name(), "totally-new-branch");
} }
#[gpui::test]
async fn test_remote_agent_fs_tool_calls(cx: &mut TestAppContext, server_cx: &mut TestAppContext) {
let fs = FakeFs::new(server_cx.executor());
fs.insert_tree(
path!("/project"),
json!({
"a.txt": "A",
"b.txt": "B",
}),
)
.await;
let (project, _headless_project) = init_test(&fs, cx, server_cx).await;
project
.update(cx, |project, cx| {
project.find_or_create_worktree(path!("/project"), true, cx)
})
.await
.unwrap();
let action_log = cx.new(|_| assistant_tool::ActionLog::new(project.clone()));
let model = Arc::new(FakeLanguageModel::default());
let request = Arc::new(LanguageModelRequest::default());
let input = ReadFileToolInput {
path: "project/b.txt".into(),
start_line: None,
end_line: None,
};
let exists_result = cx.update(|cx| {
ReadFileTool::run(
Arc::new(ReadFileTool),
serde_json::to_value(input).unwrap(),
request.clone(),
project.clone(),
action_log.clone(),
model.clone(),
None,
cx,
)
});
let output = exists_result.output.await.unwrap().content;
assert_eq!(output, "B");
let input = ReadFileToolInput {
path: "project/c.txt".into(),
start_line: None,
end_line: None,
};
let does_not_exist_result = cx.update(|cx| {
ReadFileTool::run(
Arc::new(ReadFileTool),
serde_json::to_value(input).unwrap(),
request.clone(),
project.clone(),
action_log.clone(),
model.clone(),
None,
cx,
)
});
does_not_exist_result.output.await.unwrap_err();
}
pub async fn init_test( pub async fn init_test(
server_fs: &Arc<FakeFs>, server_fs: &Arc<FakeFs>,
cx: &mut TestAppContext, cx: &mut TestAppContext,