diff --git a/Cargo.lock b/Cargo.lock index a9bce3786..f0265c3be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4340,6 +4340,7 @@ dependencies = [ "parking_lot", "serde", "serde_json", + "tempfile", "thiserror 2.0.21", "tokio", "tokio-util", diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index 0b30157c4..e1da00d19 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -1111,11 +1111,15 @@ async fn idle_agents_stop_and_resume_their_session_on_the_next_message() { } /// Deleting a task whose agent still runs in the worktree removes both and leaves no -/// process behind. On Windows the running agent would lock the folder (os error 32); -/// that the wait comes first is checked by `ide-jsonrpc`'s `exited` tests. -#[cfg(unix)] +/// process behind, whether it is waiting for permission or idle without session/load. #[tokio::test(flavor = "multi_thread")] async fn deleting_a_task_waits_for_its_agent_to_exit_before_removing_the_worktree() { + for no_load in [false, true] { + delete_running_agent(no_load).await; + } +} + +async fn delete_running_agent(no_load: bool) { let tmp = tempfile::tempdir().unwrap(); let root = dunce::canonicalize(tmp.path()).unwrap(); let git = |args: &[&str]| { @@ -1132,13 +1136,26 @@ async fn deleting_a_task_waits_for_its_agent_to_exit_before_removing_the_worktre git(&["add", "."]); git(&["commit", "-qm", "initial"]); - let agent = Arc::new(FakeAgent::default()); + let agent = Arc::new(FakeAgent { no_load, ..FakeAgent::default() }); let pids: Arc>> = Arc::default(); let (fake, started) = (agent.clone(), pids.clone()); // The fake agent, served on behalf of a real process that exits only when killed. let connector: Connector = Arc::new(move |_spec, cwd, client, _stderr| { - let mut cmd = tokio::process::Command::new("sleep"); - cmd.arg("30").current_dir(cwd).kill_on_drop(true); + #[cfg(unix)] + let mut cmd = { + let mut cmd = tokio::process::Command::new("sleep"); + cmd.arg("30"); + cmd + }; + #[cfg(windows)] + let mut cmd = { + let mut cmd = tokio::process::Command::new("cmd"); + cmd.args(["/D", "/C", "echo ready>agent-ready & ping -n 30 127.0.0.1 >nul"]); + cmd + }; + ide_core::process::prepare(cmd.as_std_mut()); + ide_core::process::own_group(cmd.as_std_mut()); + cmd.current_dir(cwd).kill_on_drop(true); let child = cmd.spawn()?; started.lock().push(child.id().unwrap()); let (a, b) = duplex(1 << 20); @@ -1163,7 +1180,7 @@ async fn deleting_a_task_waits_for_its_agent_to_exit_before_removing_the_worktre let task = mgr .create(NewAgentTask { project_root: None, - prompt: text("Fix the lexer"), + prompt: text(if no_load { "again" } else { "Fix the lexer" }), provider: "claude".into(), config: vec![], isolation: Isolation::Worktree, @@ -1171,12 +1188,35 @@ async fn deleting_a_task_waits_for_its_agent_to_exit_before_removing_the_worktre }) .await .unwrap(); - wait_for(&mgr, &task.id, AgentStatus::NeedsInput).await; + wait_for(&mgr, &task.id, if no_load { AgentStatus::Ready } else { AgentStatus::NeedsInput }).await; let pid = *pids.lock().last().expect("the agent was started"); + #[cfg(windows)] + { + tokio::time::timeout(Duration::from_secs(5), async { + while !task.cwd.0.join("agent-ready").exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert_eq!( + std::fs::rename(&task.cwd.0, task.cwd.0.with_extension("moved")).unwrap_err().raw_os_error(), + Some(32), + "the running agent really locks its worktree" + ); + } + mgr.delete(&task.id, true).await.unwrap(); assert!(!task.cwd.0.exists()); // Killed and reaped: not even a zombie is left. + #[cfg(unix)] let alive = std::process::Command::new("kill").args(["-0", &pid.to_string()]).status().unwrap().success(); + #[cfg(windows)] + let alive = { + let mut cmd = std::process::Command::new("tasklist"); + ide_core::process::prepare(&mut cmd).args(["/FI", &format!("PID eq {pid}"), "/FO", "CSV", "/NH"]); + String::from_utf8_lossy(&cmd.output().unwrap().stdout).contains(&format!("\"{pid}\"")) + }; assert!(!alive, "the agent (pid {pid}) is still running"); } diff --git a/crates/ide-agent/src/worktree.rs b/crates/ide-agent/src/worktree.rs index 6956cb90b..ddd45c414 100644 --- a/crates/ide-agent/src/worktree.rs +++ b/crates/ide-agent/src/worktree.rs @@ -192,6 +192,31 @@ mod tests { assert_eq!(calls, 2); } + #[cfg(windows)] + #[test] + fn removing_waits_for_a_windows_file_lock_to_be_released() { + use std::os::windows::fs::OpenOptionsExt; + let tmp = tempfile::tempdir().unwrap(); + let repo = tmp.path().join("repo"); + std::fs::create_dir(&repo).unwrap(); + git(&repo, &["init", "-q", "-b", "main"]); + std::fs::write(repo.join("a.txt"), "one\n").unwrap(); + git(&repo, &["add", "."]); + git(&repo, &["commit", "-q", "-m", "init"]); + let worktrees = tmp.path().join("wt"); + let created = create(&repo, &worktrees, "Locked file", None).unwrap(); + let file = std::fs::OpenOptions::new().read(true).share_mode(0).open(created.path.join("a.txt")).unwrap(); + assert_eq!(std::fs::remove_file(created.path.join("a.txt")).unwrap_err().raw_os_error(), Some(32)); + let release = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(800)); + drop(file); + }); + let result = remove(&repo, &worktrees, &created.path); + release.join().unwrap(); + result.unwrap(); + assert!(!created.path.exists()); + } + #[test] fn slugs_are_branch_safe() { assert_eq!(slugify("Fix lexer UTF-16 columns!"), "fix-lexer-utf-16-columns"); diff --git a/crates/ide-jsonrpc/Cargo.toml b/crates/ide-jsonrpc/Cargo.toml index 615d1f815..4161989af 100644 --- a/crates/ide-jsonrpc/Cargo.toml +++ b/crates/ide-jsonrpc/Cargo.toml @@ -16,3 +16,6 @@ parking_lot.workspace = true thiserror.workspace = true tracing.workspace = true encoding_rs.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/ide-jsonrpc/src/tests.rs b/crates/ide-jsonrpc/src/tests.rs index 033f1405d..0eb57031f 100644 --- a/crates/ide-jsonrpc/src/tests.rs +++ b/crates/ide-jsonrpc/src/tests.rs @@ -434,6 +434,44 @@ async fn exited_resolves_at_once_without_a_process() { tokio::time::timeout(Duration::from_secs(1), client.exited()).await.unwrap(); } +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn exited_waits_for_a_windows_descendant_to_release_its_file() { + use std::os::windows::fs::OpenOptionsExt; + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("hold.ps1"), + "$f=[IO.File]::Open('locked.txt','Create','ReadWrite','None'); [IO.File]::WriteAllText('ready.txt','ready'); Start-Sleep 30", + ) + .unwrap(); + let peer = Peer::spawn( + "cmd", + &[ + "/D".into(), + "/C".into(), + "powershell -NoProfile -NonInteractive -ExecutionPolicy Bypass -File hold.ps1".into(), + ], + tmp.path(), + &[], + Arc::new(NoopHandler), + Box::new(|_| {}), + ) + .unwrap(); + tokio::time::timeout(Duration::from_secs(10), async { + while !tmp.path().join("ready.txt").exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + let open = || std::fs::OpenOptions::new().read(true).share_mode(0).open(tmp.path().join("locked.txt")); + assert_eq!(open().unwrap_err().raw_os_error(), Some(32)); + assert!(tokio::time::timeout(Duration::from_millis(200), peer.exited()).await.is_err()); + peer.kill(); + tokio::time::timeout(Duration::from_secs(5), peer.exited()).await.unwrap(); + drop(open().expect("exited must wait for the descendant's file handle to close")); +} + /// A launcher that prints in the system code page and exits before speaking the /// protocol: `jdtls.bat` on a Chinese Windows when its Python step fails, then /// `pause` prints `请按任意键继续. . .` in GBK on stdout.