From bc27d324edeac862569bfc90ea8a600999d9db27 Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Sun, 23 Aug 2026 19:25:10 +0800 Subject: [PATCH 1/4] Consolidate Runner execution lifecycle tests --- .../src/webcodex_runner/detached_job/tests.rs | 160 +++++------ .../src/webcodex_runner/job_manager_tests.rs | 177 ++++-------- .../src/webcodex_runner/transport_tests.rs | 266 +++++++++--------- 3 files changed, 262 insertions(+), 341 deletions(-) diff --git a/crates/webcodex-runner/src/webcodex_runner/detached_job/tests.rs b/crates/webcodex-runner/src/webcodex_runner/detached_job/tests.rs index 3813d4a0..bef386b8 100644 --- a/crates/webcodex-runner/src/webcodex_runner/detached_job/tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/detached_job/tests.rs @@ -261,18 +261,7 @@ fn accepted_active_record_is_never_reclaimed() { let temp = tempfile::tempdir().unwrap(); let store = DetachedJobStore::new(temp.path().join("state")); let request = make_payload_request("linger", Vec::new()); - let outcome = handoff_detached_job(&store, request.clone()).unwrap(); - assert!(matches!(outcome, DetachedHandoffOutcome::Accepted { .. })); - assert!( - wait_until(Duration::from_secs(5), || store - .read(&request.job_id) - .is_ok_and(|record| { - record.phase == DetachedJobPhase::Running - && record.ownership_accepted_at_unix_ms.is_some() - })), - "accepted detached execution never reached a live Running state" - ); - let running = store.read(&request.job_id).unwrap(); + let running = handoff_and_wait_running(&store, &request); assert!(running.ownership_accepted_at_unix_ms.is_some()); assert_eq!(running.phase, DetachedJobPhase::Running); @@ -635,6 +624,70 @@ fn make_payload_request(scenario: &str, env: Vec<(String, String)>) -> DetachedS request } +#[cfg(unix)] +fn wait_for_running_record(store: &DetachedJobStore, job_id: &str) -> DetachedJobRecord { + assert!( + wait_until(Duration::from_secs(5), || store.read(job_id).is_ok_and( + |record| { + record.phase == DetachedJobPhase::Running + && record.ownership_accepted_at_unix_ms.is_some() + } + )), + "detached execution never reached an accepted Running state: {job_id}" + ); + store.read(job_id).unwrap() +} + +#[cfg(unix)] +fn handoff_and_wait_running( + store: &DetachedJobStore, + request: &DetachedStartRequest, +) -> DetachedJobRecord { + let outcome = handoff_detached_job(store, request.clone()).unwrap(); + assert!(matches!(outcome, DetachedHandoffOutcome::Accepted { .. })); + wait_for_running_record(store, &request.job_id) +} + +#[cfg(unix)] +fn run_accept_then_exit_owner(temp: &Path, state_root: &Path, request: &DetachedStartRequest) { + let instruction = temp.join("accept-exit-owner.json"); + fs::write( + &instruction, + serde_json::to_vec(&(state_root.to_path_buf(), request.clone())).unwrap(), + ) + .unwrap(); + let mut owner = Command::new(std::env::current_exe().unwrap()); + owner + .arg("--exact") + .arg("webcodex_runner::detached_job::tests::accept_then_exit_owner_subprocess_entrypoint") + .arg("--nocapture") + .env_clear() + .env("WEBCODEX_DETACHED_ACCEPT_EXIT_INSTRUCTION", &instruction) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + assert!(owner.spawn().unwrap().wait().unwrap().success()); +} + +#[cfg(any(target_os = "linux", target_os = "macos"))] +fn tree_payload_request(temp: &Path) -> (DetachedStartRequest, PathBuf, PathBuf) { + let parent_marker = temp.join("parent.pid"); + let child_marker = temp.join("child.pid"); + let request = make_payload_request( + "tree", + vec![ + ( + "PARENT_PID_MARKER".to_string(), + parent_marker.to_string_lossy().into_owned(), + ), + ( + "CHILD_PID_MARKER".to_string(), + child_marker.to_string_lossy().into_owned(), + ), + ], + ); + (request, parent_marker, child_marker) +} + #[cfg(unix)] #[test] fn accepted_handoff_keeps_payload_alive_after_owner_process_exits() { @@ -701,23 +754,7 @@ fn accepted_handoff_survives_owner_exit_before_ack() { marker.to_string_lossy().into_owned(), )], ); - let instruction = temp.path().join("accept-exit-owner.json"); - fs::write( - &instruction, - serde_json::to_vec(&(state_root.clone(), request.clone())).unwrap(), - ) - .unwrap(); - - let mut owner = Command::new(std::env::current_exe().unwrap()); - owner - .arg("--exact") - .arg("webcodex_runner::detached_job::tests::accept_then_exit_owner_subprocess_entrypoint") - .arg("--nocapture") - .env_clear() - .env("WEBCODEX_DETACHED_ACCEPT_EXIT_INSTRUCTION", &instruction) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - assert!(owner.spawn().unwrap().wait().unwrap().success()); + run_accept_then_exit_owner(temp.path(), &state_root, &request); let store = DetachedJobStore::new(state_root); let terminal = wait_for_terminal(&store, &request.job_id); @@ -842,27 +879,10 @@ fn restart_scan_reconciles_live_detached_execution_without_respawn() { marker.to_string_lossy().into_owned(), )], ); - let instruction = temp.path().join("restart-owner.json"); - fs::write( - &instruction, - serde_json::to_vec(&(state_root.clone(), request.clone())).unwrap(), - ) - .unwrap(); - let mut owner = Command::new(std::env::current_exe().unwrap()); - owner - .arg("--exact") - .arg("webcodex_runner::detached_job::tests::accept_then_exit_owner_subprocess_entrypoint") - .arg("--nocapture") - .env_clear() - .env("WEBCODEX_DETACHED_ACCEPT_EXIT_INSTRUCTION", &instruction) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - assert!(owner.spawn().unwrap().wait().unwrap().success()); + run_accept_then_exit_owner(temp.path(), &state_root, &request); let store = DetachedJobStore::new(state_root); - assert!(wait_until(Duration::from_secs(5), || store - .read(&request.job_id) - .is_ok_and(|record| record.phase == DetachedJobPhase::Running))); + let _running = wait_for_running_record(&store, &request.job_id); let records = store.scan_for_client(&request.client_id).unwrap(); assert_eq!(records.len(), 1); let recovered = store @@ -885,28 +905,13 @@ fn durable_stop_request_terminates_exact_supervisor_owned_tree() { let _guard = test_env_lock(); let temp = tempfile::tempdir().unwrap(); let store = DetachedJobStore::new(temp.path().join("state")); - let parent_marker = temp.path().join("parent.pid"); - let child_marker = temp.path().join("child.pid"); - let request = make_payload_request( - "tree", - vec![ - ( - "PARENT_PID_MARKER".to_string(), - parent_marker.to_string_lossy().into_owned(), - ), - ( - "CHILD_PID_MARKER".to_string(), - child_marker.to_string_lossy().into_owned(), - ), - ], - ); - let _ = handoff_detached_job(&store, request.clone()).unwrap(); + let (request, parent_marker, child_marker) = tree_payload_request(temp.path()); + let running = handoff_and_wait_running(&store, &request); assert!(wait_until(Duration::from_secs(5), || { parent_marker.exists() && child_marker.exists() })); let parent_pid: u32 = fs::read_to_string(&parent_marker).unwrap().parse().unwrap(); let child_pid: u32 = fs::read_to_string(&child_marker).unwrap().parse().unwrap(); - let running = store.read(&request.job_id).unwrap(); let stopped = store .request_stop(&request.job_id, &running.execution_id) .unwrap(); @@ -956,11 +961,7 @@ fn stale_native_supervisor_identity_reconciles_to_lost_without_respawn() { let temp = tempfile::tempdir().unwrap(); let store = DetachedJobStore::new(temp.path().join("state")); let request = make_payload_request("linger", Vec::new()); - let _ = handoff_detached_job(&store, request.clone()).unwrap(); - assert!(wait_until(Duration::from_secs(5), || store - .read(&request.job_id) - .is_ok_and(|record| record.phase == DetachedJobPhase::Running))); - let mut running = store.read(&request.job_id).unwrap(); + let mut running = handoff_and_wait_running(&store, &request); let real_supervisor_pid = running.supervisor.as_ref().unwrap().pid; running.supervisor.as_mut().unwrap().native_start_id = stale_native_start_identity().to_string(); @@ -1194,28 +1195,13 @@ fn supervisor_death_terminates_payload_process_tree() { let _guard = test_env_lock(); let temp = tempfile::tempdir().unwrap(); let store = DetachedJobStore::new(temp.path().join("state")); - let parent_marker = temp.path().join("parent.pid"); - let child_marker = temp.path().join("child.pid"); - let request = make_payload_request( - "tree", - vec![ - ( - "PARENT_PID_MARKER".to_string(), - parent_marker.to_string_lossy().into_owned(), - ), - ( - "CHILD_PID_MARKER".to_string(), - child_marker.to_string_lossy().into_owned(), - ), - ], - ); - let _ = handoff_detached_job(&store, request.clone()).unwrap(); + let (request, parent_marker, child_marker) = tree_payload_request(temp.path()); + let running = handoff_and_wait_running(&store, &request); assert!(wait_until(Duration::from_secs(5), || { parent_marker.exists() && child_marker.exists() })); let parent_pid: u32 = fs::read_to_string(&parent_marker).unwrap().parse().unwrap(); let child_pid: u32 = fs::read_to_string(&child_marker).unwrap().parse().unwrap(); - let running = store.read(&request.job_id).unwrap(); assert_eq!(running.phase, DetachedJobPhase::Running); let supervisor_pid = running.supervisor.as_ref().unwrap().pid; assert!(process_alive(supervisor_pid)); diff --git a/crates/webcodex-runner/src/webcodex_runner/job_manager_tests.rs b/crates/webcodex-runner/src/webcodex_runner/job_manager_tests.rs index 40509d94..da32b82b 100644 --- a/crates/webcodex-runner/src/webcodex_runner/job_manager_tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/job_manager_tests.rs @@ -4012,6 +4012,52 @@ fn insert_running_job( stop_requested } +struct RunningJobTreeFixture { + parent_pid: u32, + grandchild_pid: u32, + output: mpsc::Receiver, +} + +impl RunningJobTreeFixture { + fn assert_terminated(&self, timeout: Duration, tag: &str) { + assert!( + wait_for_process_exit(self.parent_pid, timeout, &format!("{tag}-parent")), + "{tag} parent survived tree termination" + ); + assert!( + wait_for_process_exit(self.grandchild_pid, timeout, &format!("{tag}-grandchild")), + "{tag} descendant survived tree termination" + ); + assert!( + wait_for_stdout_eof(&self.output, timeout, &format!("{tag}-eof")), + "{tag} stdout did not reach EOF after tree termination" + ); + } +} + +fn seed_running_job_tree( + manager: &JobManager, + job_id: &str, + marker: &Path, +) -> (RunningJobTreeFixture, Arc) { + let (managed, output) = spawn_helper_raw( + "spawn-grandchild-keepalive", + &[marker.to_str().unwrap(), "3", "60", "60"], + ); + let parent_pid = managed.id(); + let grandchild_pid = read_grandchild_pid(&output); + let child = Arc::new(Mutex::new(managed)); + let stop_requested = insert_running_job(manager, job_id, Some(child)); + ( + RunningJobTreeFixture { + parent_pid, + grandchild_pid, + output, + }, + stop_requested, + ) +} + /// An explicit stop terminates the whole job process tree, including a /// descendant that inherited the stdout pipe, and the stdout reader reaches /// EOF instead of blocking forever. @@ -4019,43 +4065,20 @@ fn insert_running_job( fn job_stop_terminates_whole_tree_including_descendant() { let temp = tempfile::tempdir().unwrap(); let marker = temp.path().join("stop-grandchild.marker"); - let (managed, rx) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker.to_str().unwrap(), "3", "60", "60"], - ); - let parent_pid = managed.id(); - let grandchild_pid = read_grandchild_pid(&rx); + let manager = JobManager::new(1); + let (tree, stop_requested) = seed_running_job_tree(&manager, "stop-tree-job", &marker); assert!( - process_running(parent_pid), + process_running(tree.parent_pid), "job parent should be running before stop" ); assert!( - process_running(grandchild_pid), + process_running(tree.grandchild_pid), "job descendant should be running before stop" ); - let child = Arc::new(Mutex::new(managed)); - - let manager = JobManager::new(1); - let stop_requested = insert_running_job(&manager, "stop-tree-job", Some(child.clone())); manager.stop("stop-tree-job").expect("stop job"); assert!(stop_requested.load(Ordering::SeqCst)); - assert!( - wait_for_process_exit(parent_pid, Duration::from_secs(5), "parent-after-stop"), - "job parent survived stop" - ); - assert!( - wait_for_process_exit( - grandchild_pid, - Duration::from_secs(5), - "grandchild-after-stop" - ), - "job descendant survived stop; stop must terminate the whole tree" - ); - assert!( - wait_for_stdout_eof(&rx, Duration::from_secs(5), "stop-eof"), - "stdout must reach EOF after the whole tree is terminated" - ); + tree.assert_terminated(Duration::from_secs(5), "explicit-stop"); assert!( !marker.exists(), "delayed grandchild marker must never appear after stop" @@ -4169,20 +4192,8 @@ fn job_stop_all_terminates_all_trees_and_preserves_completed_jobs() { let temp = tempfile::tempdir().unwrap(); let marker_a = temp.path().join("stop-all-a.marker"); let marker_b = temp.path().join("stop-all-b.marker"); - let (managed_a, rx_a) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker_a.to_str().unwrap(), "3", "60", "60"], - ); - let (managed_b, rx_b) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker_b.to_str().unwrap(), "3", "60", "60"], - ); - let a_parent = managed_a.id(); - let a_grandchild = read_grandchild_pid(&rx_a); - let b_parent = managed_b.id(); - let b_grandchild = read_grandchild_pid(&rx_b); - insert_running_job(&manager, "running-a", Some(Arc::new(Mutex::new(managed_a)))); - insert_running_job(&manager, "running-b", Some(Arc::new(Mutex::new(managed_b)))); + let (tree_a, _) = seed_running_job_tree(&manager, "running-a", &marker_a); + let (tree_b, _) = seed_running_job_tree(&manager, "running-b", &marker_b); manager.stop_all(); @@ -4195,19 +4206,8 @@ fn job_stop_all_terminates_all_trees_and_preserves_completed_jobs() { !completed_stop.load(Ordering::SeqCst), "a terminal job must not be signalled during shutdown" ); - for (pid, tag) in [ - (a_parent, "a-parent"), - (a_grandchild, "a-grandchild"), - (b_parent, "b-parent"), - (b_grandchild, "b-grandchild"), - ] { - assert!( - wait_for_process_exit(pid, Duration::from_secs(5), tag), - "{tag} survived stop_all" - ); - } - assert!(wait_for_stdout_eof(&rx_a, Duration::from_secs(5), "a-eof")); - assert!(wait_for_stdout_eof(&rx_b, Duration::from_secs(5), "b-eof")); + tree_a.assert_terminated(Duration::from_secs(5), "stop-all-a"); + tree_b.assert_terminated(Duration::from_secs(5), "stop-all-b"); } /// Repeated stops are idempotent: the second stop must not panic and must not @@ -4216,36 +4216,15 @@ fn job_stop_all_terminates_all_trees_and_preserves_completed_jobs() { fn job_stop_twice_is_idempotent() { let temp = tempfile::tempdir().unwrap(); let marker = temp.path().join("twice.marker"); - let (managed, rx) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker.to_str().unwrap(), "3", "60", "60"], - ); - let parent_pid = managed.id(); - let grandchild_pid = read_grandchild_pid(&rx); - let child = Arc::new(Mutex::new(managed)); let manager = JobManager::new(1); - insert_running_job(&manager, "twice-job", Some(child)); + let (tree, _) = seed_running_job_tree(&manager, "twice-job", &marker); manager.stop("twice-job").expect("first stop"); manager .stop("twice-job") .expect("second stop must be idempotent"); - assert!(wait_for_process_exit( - parent_pid, - Duration::from_secs(5), - "twice-parent" - )); - assert!(wait_for_process_exit( - grandchild_pid, - Duration::from_secs(5), - "twice-grandchild" - )); - assert!(wait_for_stdout_eof( - &rx, - Duration::from_secs(5), - "twice-eof" - )); + tree.assert_terminated(Duration::from_secs(5), "stop-twice"); } /// Stopping a job whose tree already exited naturally must not panic and must @@ -4278,34 +4257,19 @@ fn job_stop_after_natural_exit_does_not_panic() { fn last_job_manager_owner_drop_terminates_running_tree_with_worker_clone_alive() { let temp = tempfile::tempdir().unwrap(); let marker = temp.path().join("drop.marker"); - let (managed, rx) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker.to_str().unwrap(), "3", "60", "60"], - ); - let parent_pid = managed.id(); - let grandchild_pid = read_grandchild_pid(&rx); let manager = JobManager::new(1); let second_owner = manager.clone(); let worker_manager = manager.clone_for_worker(); - let stop_requested = - insert_running_job(&manager, "drop-job", Some(Arc::new(Mutex::new(managed)))); + let (tree, stop_requested) = seed_running_job_tree(&manager, "drop-job", &marker); drop(manager); - assert!(process_running(parent_pid) && process_running(grandchild_pid)); + assert!(process_running(tree.parent_pid) && process_running(tree.grandchild_pid)); assert!(!stop_requested.load(Ordering::SeqCst)); drop(second_owner); assert!(worker_manager.shutting_down.load(Ordering::SeqCst)); assert!(stop_requested.load(Ordering::SeqCst)); - assert!( - wait_for_process_exit(parent_pid, Duration::from_secs(5), "drop-parent"), - "running job parent survived last owner drop" - ); - assert!( - wait_for_process_exit(grandchild_pid, Duration::from_secs(5), "drop-grandchild"), - "running job descendant survived last owner drop" - ); - assert!(wait_for_stdout_eof(&rx, Duration::from_secs(5), "drop-eof")); + tree.assert_terminated(Duration::from_secs(5), "last-owner-drop"); drop(worker_manager); } @@ -4333,15 +4297,8 @@ fn cleanup_managed_tree_on_exited_tree_does_not_panic() { fn job_stop_racing_shutdown_does_not_panic() { let temp = tempfile::tempdir().unwrap(); let marker = temp.path().join("race.marker"); - let (managed, rx) = spawn_helper_raw( - "spawn-grandchild-keepalive", - &[marker.to_str().unwrap(), "3", "60", "60"], - ); - let parent_pid = managed.id(); - let grandchild_pid = read_grandchild_pid(&rx); - let child = Arc::new(Mutex::new(managed)); let manager = JobManager::new(1); - insert_running_job(&manager, "race-job", Some(child.clone())); + let (tree, _) = seed_running_job_tree(&manager, "race-job", &marker); let stop_manager = manager.clone(); let stopper = std::thread::spawn(move || { @@ -4353,17 +4310,7 @@ fn job_stop_racing_shutdown_does_not_panic() { stopper.join().expect("stop thread must not panic"); assert!(outcome.resources >= 1); - assert!(wait_for_process_exit( - parent_pid, - Duration::from_secs(5), - "race-parent" - )); - assert!(wait_for_process_exit( - grandchild_pid, - Duration::from_secs(5), - "race-grandchild" - )); - assert!(wait_for_stdout_eof(&rx, Duration::from_secs(5), "race-eof")); + tree.assert_terminated(Duration::from_secs(5), "stop-shutdown-race"); } /// A job timeout must terminate the whole tree (parent shell, helper, and the diff --git a/crates/webcodex-runner/src/webcodex_runner/transport_tests.rs b/crates/webcodex-runner/src/webcodex_runner/transport_tests.rs index c7751b3a..2dd02bb2 100644 --- a/crates/webcodex-runner/src/webcodex_runner/transport_tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/transport_tests.rs @@ -571,6 +571,59 @@ impl ConcurrentPollingServer { } } +struct PollingRunnerHandle { + result_rx: std::sync::mpsc::Receiver>, + handle: thread::JoinHandle<()>, +} + +impl PollingRunnerHandle { + fn assert_pending(&self, context: &str) { + match self.result_rx.try_recv() { + Err(std::sync::mpsc::TryRecvError::Empty) => {} + Err(std::sync::mpsc::TryRecvError::Disconnected) => { + panic!("{context}: polling runner disconnected before reporting a result") + } + Ok(result) => panic!("{context}: polling runner returned early: {result:?}"), + } + } + + fn finish(self, timeout: Duration, context: &str) -> Result<(), String> { + match self.result_rx.recv_timeout(timeout) { + Ok(result) => { + if self.handle.join().is_err() { + panic!("{context}: polling runner thread panicked after reporting its result"); + } + result + } + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + if self.handle.join().is_err() { + panic!("{context}: polling runner thread panicked before reporting its result"); + } + panic!("{context}: polling runner exited without reporting a result"); + } + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => { + panic!("{context}: polling runner exceeded hard timeout {timeout:?}"); + } + } + } +} + +fn spawn_polling_runner( + cfg: AgentConfig, + runtime: AgentRuntimeState, + once: bool, + instance_id: &str, + shutdown: Arc, +) -> PollingRunnerHandle { + let instance_id = instance_id.to_string(); + let (result_tx, result_rx) = std::sync::mpsc::sync_channel(1); + let handle = thread::spawn(move || { + let result = run_polling_agent_with_shutdown(cfg, once, &instance_id, shutdown, &runtime); + let _ = result_tx.send(result); + }); + PollingRunnerHandle { result_rx, handle } +} + fn start_concurrent_polling_server( handler: Arc ConcurrentHttpResponse + Send + Sync>, ) -> ConcurrentPollingServer { @@ -1095,19 +1148,13 @@ fn polling_long_ordinary_dispatch_does_not_pin_and_results_stay_correlated_exact let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let runner_shutdown_for_thread = Arc::clone(&runner_shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-correlation", - runner_shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-correlation", + Arc::clone(&runner_shutdown), + ); assert_eq!( event_rx.recv_timeout(Duration::from_secs(5)).unwrap(), @@ -1131,11 +1178,9 @@ fn polling_long_ordinary_dispatch_does_not_pin_and_results_stay_correlated_exact "result-req-slow-a" ); - runner_rx - .recv_timeout(Duration::from_secs(10)) - .expect("polling runner completion") + runner + .finish(Duration::from_secs(10), "polling correlation runner") .expect("polling runner should shut down cleanly"); - runner.join().unwrap(); server.finish(); let results = results.lock().unwrap(); @@ -1211,19 +1256,13 @@ fn polling_dispatch_bound_backpressures_without_a_local_pending_queue() { let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let runner_shutdown_for_thread = Arc::clone(&runner_shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-bound", - runner_shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-bound", + Arc::clone(&runner_shutdown), + ); let deadline = Instant::now() + Duration::from_secs(5); for path in &started[..2] { @@ -1254,11 +1293,9 @@ fn polling_dispatch_bound_backpressures_without_a_local_pending_queue() { std::fs::write(&releases[1], "release\n").unwrap(); std::fs::write(&releases[2], "release\n").unwrap(); - runner_rx - .recv_timeout(Duration::from_secs(10)) - .expect("bounded polling runner completion") + runner + .finish(Duration::from_secs(10), "bounded polling runner") .expect("bounded polling runner should shut down cleanly"); - runner.join().unwrap(); server.finish(); assert_eq!(result_count.load(Ordering::SeqCst), 3); for marker in markers { @@ -1336,19 +1373,13 @@ fn polling_job_start_dispatches_behind_one_long_ordinary_request() { let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let runner_shutdown_for_thread = Arc::clone(&runner_shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-job", - runner_shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-job", + Arc::clone(&runner_shutdown), + ); job_rx .recv_timeout(Duration::from_secs(5)) @@ -1358,11 +1389,9 @@ fn polling_job_start_dispatches_behind_one_long_ordinary_request() { assert_eq!(std::fs::read_to_string(&job_marker).unwrap(), "job-ran\n"); std::fs::write(&release_a, "release\n").unwrap(); - runner_rx - .recv_timeout(Duration::from_secs(10)) - .expect("Job-behind-ordinary runner completion") + runner + .finish(Duration::from_secs(10), "Job-behind-ordinary runner") .expect("Job-behind-ordinary runner should shut down cleanly"); - runner.join().unwrap(); server.finish(); assert_eq!( std::fs::read_to_string(&marker_a).unwrap().lines().count(), @@ -1406,30 +1435,24 @@ fn polling_once_waits_for_its_tracked_ordinary_dispatch() { let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let shutdown = Arc::new(AtomicBool::new(false)); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = - run_polling_agent_with_shutdown(cfg, true, "inst-e1-once", shutdown, &runner_runtime); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + true, + "inst-e1-once", + Arc::new(AtomicBool::new(false)), + ); wait_for_path( &started, Instant::now() + Duration::from_secs(5), "--once request to start", ); - assert!( - runner_rx.try_recv().is_err(), - "--once returned while its ordinary dispatch was still active" - ); + runner.assert_pending("--once returned while its ordinary dispatch was still active"); std::fs::write(&release, "release\n").unwrap(); - runner_rx - .recv_timeout(Duration::from_secs(5)) - .expect("--once runner completion") + runner + .finish(Duration::from_secs(5), "--once polling runner") .expect("--once runner should complete successfully"); - runner.join().unwrap(); server.finish(); assert_eq!(poll_count.load(Ordering::SeqCst), 1); @@ -1483,38 +1506,27 @@ fn polling_once_preserves_job_manager_drain_before_exit() { let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let shutdown = Arc::new(AtomicBool::new(false)); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - true, - "inst-e1-once-job", - shutdown, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + true, + "inst-e1-once-job", + Arc::new(AtomicBool::new(false)), + ); wait_for_path( &started, Instant::now() + Duration::from_secs(5), "--once Job to start", ); - assert!( - runner_rx.try_recv().is_err(), - "--once returned before JobManager drained its active Job" - ); + runner.assert_pending("--once returned before JobManager drained its active Job"); std::fs::write(&release, "release\n").unwrap(); terminal_rx .recv_timeout(Duration::from_secs(5)) .expect("--once Job terminal update"); - runner_rx - .recv_timeout(Duration::from_secs(5)) - .expect("--once Job runner completion") + runner + .finish(Duration::from_secs(5), "--once Job polling runner") .expect("--once Job runner should complete successfully"); - runner.join().unwrap(); server.finish(); assert_eq!(poll_count.load(Ordering::SeqCst), 1); @@ -1561,20 +1573,14 @@ fn polling_shutdown_with_active_background_dispatch_is_bounded_and_non_replaying let cfg = polling_agent_config(server.server_url.clone(), temp.path().join("projects.d")); let runtime = AgentRuntimeState::with_shutdown_budget(&cfg, PathBuf::new(), Duration::from_secs(2)); - let runner_runtime = runtime.clone(); let shutdown = Arc::new(AtomicBool::new(false)); - let shutdown_for_thread = Arc::clone(&shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-shutdown", - shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-shutdown", + Arc::clone(&shutdown), + ); wait_for_path( &started, @@ -1583,11 +1589,9 @@ fn polling_shutdown_with_active_background_dispatch_is_bounded_and_non_replaying ); let shutdown_started = Instant::now(); shutdown.store(true, Ordering::SeqCst); - runner_rx - .recv_timeout(Duration::from_secs(5)) - .expect("active-shutdown runner completion") + runner + .finish(Duration::from_secs(5), "active-shutdown polling runner") .expect("active-shutdown runner should exit cleanly"); - runner.join().unwrap(); assert!( shutdown_started.elapsed() < Duration::from_secs(3), "shutdown exceeded its bounded cleanup budget" @@ -1682,19 +1686,13 @@ fn polling_background_project_operation_invalidates_the_project_cache() { let mut cfg = polling_agent_config(server.server_url.clone(), projects_dir.clone()); cfg.policy.allowed_roots = vec![temp.path().to_path_buf()]; let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let runner_shutdown_for_thread = Arc::clone(&runner_shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-project-cache", - runner_shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-project-cache", + Arc::clone(&runner_shutdown), + ); // The register_project round trip is an actual project operation (it may // spawn git); on a loaded runner the poll that observes the refreshed @@ -1702,11 +1700,9 @@ fn polling_background_project_operation_invalidates_the_project_cache() { refreshed_rx .recv_timeout(Duration::from_secs(30)) .expect("a later poll must carry refreshed project metadata"); - runner_rx - .recv_timeout(Duration::from_secs(30)) - .expect("project-cache runner completion") + runner + .finish(Duration::from_secs(30), "project-cache polling runner") .expect("project-cache runner should shut down cleanly"); - runner.join().unwrap(); server.finish(); assert!(projects_dir.join("e1-project.toml").exists()); assert!(poll_count.load(Ordering::SeqCst) >= 2); @@ -1804,19 +1800,13 @@ fn polling_persistent_shell_exec_remains_responsive_to_close() { let server = start_concurrent_polling_server(handler); let cfg = polling_agent_config(server.server_url.clone(), projects_dir); let runtime = test_runtime(&cfg); - let runner_runtime = runtime.clone(); - let runner_shutdown_for_thread = Arc::clone(&runner_shutdown); - let (runner_tx, runner_rx) = std::sync::mpsc::sync_channel(1); - let runner = thread::spawn(move || { - let result = run_polling_agent_with_shutdown( - cfg, - false, - "inst-e1-persistent", - runner_shutdown_for_thread, - &runner_runtime, - ); - let _ = runner_tx.send(result); - }); + let runner = spawn_polling_runner( + cfg, + runtime.clone(), + false, + "inst-e1-persistent", + Arc::clone(&runner_shutdown), + ); wait_for_path( &started, @@ -1824,11 +1814,9 @@ fn polling_persistent_shell_exec_remains_responsive_to_close() { "persistent-shell exec to start", ); allow_close.store(true, Ordering::SeqCst); - runner_rx - .recv_timeout(Duration::from_secs(10)) - .expect("persistent-shell polling runner completion") + runner + .finish(Duration::from_secs(10), "persistent-shell polling runner") .expect("persistent-shell polling runner should shut down cleanly"); - runner.join().unwrap(); server.finish(); let mut result_ids = state.lock().unwrap().result_ids.clone(); From 6fb9a11bb60f5b303ee0caee0f5d07094cb60bf5 Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Sun, 23 Aug 2026 20:06:30 +0800 Subject: [PATCH 2/4] Consolidate Runner LSP synchronization tests --- .../computer_windows_uia_tests.rs | 140 ++++++------------ .../src/webcodex_runner/lsp/fake_server.rs | 13 +- .../src/webcodex_runner/lsp/mod.rs | 3 + .../webcodex_runner/lsp/navigation_tests.rs | 83 ++--------- .../src/webcodex_runner/lsp/test_support.rs | 66 +++++++++ .../src/webcodex_runner/lsp/tests.rs | 89 ++--------- 6 files changed, 152 insertions(+), 242 deletions(-) create mode 100644 crates/webcodex-runner/src/webcodex_runner/lsp/test_support.rs diff --git a/crates/webcodex-runner/src/webcodex_runner/computer_windows_uia_tests.rs b/crates/webcodex-runner/src/webcodex_runner/computer_windows_uia_tests.rs index 9623d208..db8426a4 100644 --- a/crates/webcodex-runner/src/webcodex_runner/computer_windows_uia_tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/computer_windows_uia_tests.rs @@ -411,6 +411,33 @@ $form.Add_Shown({ $form.Activate(); $button.Focus() }) .expect("launch private WinForms foreground probe"); Self { child } } + + fn wait_for_window(&mut self, title: &str, context: &str) -> PlatformWindow { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if let Some(status) = self + .child + .try_wait() + .unwrap_or_else(|error| panic!("query {context} process: {error}")) + { + panic!("{context} exited before discovery: {status}"); + } + if let Some(candidate) = platform::list_windows(4096) + .unwrap_or_else(|error| panic!("list Windows windows for {context}: {error}")) + .into_iter() + .find(|candidate| candidate.title == title) + { + return candidate; + } + let now = Instant::now(); + assert!(now < deadline, "timed out discovering {context}"); + thread::sleep( + deadline + .saturating_duration_since(now) + .min(Duration::from_millis(20)), + ); + } + } } impl Drop for WindowsControlFixture { @@ -664,25 +691,10 @@ fn computer_windows_window_activation_live_smoke() { #[ignore = "requires an interactive Windows desktop; creates and closes a private WinForms control fixture"] fn computer_windows_control_fixture_live_smoke() { let mut fixture = WindowsControlFixture::start(); - let candidate = (0..500) - .find_map(|_| { - if let Some(status) = fixture - .child - .try_wait() - .expect("query private WinForms fixture process") - { - panic!("private WinForms fixture exited before discovery: {status}"); - } - let candidate = platform::list_windows(4096) - .expect("list Windows windows for control fixture") - .into_iter() - .find(|candidate| candidate.title == WINDOWS_CONTROL_FIXTURE_TITLE); - if candidate.is_none() { - thread::sleep(Duration::from_millis(20)); - } - candidate - }) - .expect("discover private WinForms control fixture"); + let candidate = fixture.wait_for_window( + WINDOWS_CONTROL_FIXTURE_TITLE, + "private WinForms control fixture", + ); let record = surface_record(candidate); let activation = platform::activate_window("surface_windows_control_fixture_activate", &record) .expect("activate private WinForms control fixture"); @@ -812,25 +824,10 @@ fn computer_windows_control_fixture_live_smoke() { #[ignore = "requires an interactive Windows desktop; creates and closes a private scrollable WinForms fixture"] fn computer_windows_scroll_to_element_fixture_live_smoke() { let mut fixture = WindowsControlFixture::start(); - let candidate = (0..500) - .find_map(|_| { - if let Some(status) = fixture - .child - .try_wait() - .expect("query private WinForms scroll fixture process") - { - panic!("private WinForms scroll fixture exited before discovery: {status}"); - } - let candidate = platform::list_windows(4096) - .expect("list Windows windows for scroll fixture") - .into_iter() - .find(|candidate| candidate.title == WINDOWS_CONTROL_FIXTURE_TITLE); - if candidate.is_none() { - thread::sleep(Duration::from_millis(20)); - } - candidate - }) - .expect("discover private WinForms scroll fixture"); + let candidate = fixture.wait_for_window( + WINDOWS_CONTROL_FIXTURE_TITLE, + "private WinForms scroll fixture", + ); let record = surface_record(candidate); platform::activate_window("surface_windows_scroll_fixture_activate", &record) .expect("activate private WinForms scroll fixture"); @@ -904,25 +901,10 @@ fn computer_windows_scroll_to_element_fixture_live_smoke() { #[ignore = "requires an interactive Windows desktop; creates and closes only private WinForms key-input fixtures"] fn computer_windows_key_input_fixture_live_smoke() { let mut fixture = WindowsControlFixture::start(); - let candidate = (0..500) - .find_map(|_| { - if let Some(status) = fixture - .child - .try_wait() - .expect("query private WinForms key-input fixture process") - { - panic!("private WinForms key-input fixture exited before discovery: {status}"); - } - let candidate = platform::list_windows(4096) - .expect("list Windows windows for key-input fixture") - .into_iter() - .find(|candidate| candidate.title == WINDOWS_CONTROL_FIXTURE_TITLE); - if candidate.is_none() { - thread::sleep(Duration::from_millis(20)); - } - candidate - }) - .expect("discover private WinForms key-input fixture"); + let candidate = fixture.wait_for_window( + WINDOWS_CONTROL_FIXTURE_TITLE, + "private WinForms key-input fixture", + ); let record = surface_record(candidate); let hwnd = platform::win_hwnd(record.native_id).expect("resolve key fixture HWND"); let foreground_deadline = Instant::now() + Duration::from_secs(2); @@ -1073,25 +1055,8 @@ fn computer_windows_key_input_fixture_live_smoke() { assert!(protected.starts_with("permission_denied:"), "{protected}"); let mut foreground_probe = WindowsControlFixture::start_foreground_probe(); - let probe_candidate = (0..500) - .find_map(|_| { - if let Some(status) = foreground_probe - .child - .try_wait() - .expect("query private foreground probe process") - { - panic!("private foreground probe exited before discovery: {status}"); - } - let candidate = platform::list_windows(4096) - .expect("list Windows windows for foreground probe") - .into_iter() - .find(|candidate| candidate.title == WINDOWS_FOREGROUND_PROBE_TITLE); - if candidate.is_none() { - thread::sleep(Duration::from_millis(20)); - } - candidate - }) - .expect("discover private foreground probe"); + let probe_candidate = foreground_probe + .wait_for_window(WINDOWS_FOREGROUND_PROBE_TITLE, "private foreground probe"); let probe_record = surface_record(probe_candidate); let probe_hwnd = platform::win_hwnd(probe_record.native_id).expect("resolve foreground probe HWND"); @@ -1118,25 +1083,10 @@ fn computer_windows_key_input_fixture_live_smoke() { #[ignore = "requires an interactive Windows desktop; creates and replaces indistinguishable private WinForms controls"] fn computer_windows_uia_stale_identity_rejects_indistinguishable_replacement_live() { let mut fixture = WindowsControlFixture::start(); - let candidate = (0..500) - .find_map(|_| { - if let Some(status) = fixture - .child - .try_wait() - .expect("query private WinForms fixture process") - { - panic!("private WinForms fixture exited before discovery: {status}"); - } - let candidate = platform::list_windows(4096) - .expect("list Windows windows for identity fixture") - .into_iter() - .find(|candidate| candidate.title == WINDOWS_CONTROL_FIXTURE_TITLE); - if candidate.is_none() { - thread::sleep(Duration::from_millis(20)); - } - candidate - }) - .expect("discover private WinForms identity fixture"); + let candidate = fixture.wait_for_window( + WINDOWS_CONTROL_FIXTURE_TITLE, + "private WinForms identity fixture", + ); let record = surface_record(candidate); platform::activate_window("surface_windows_identity_fixture_activate", &record) .expect("activate private WinForms identity fixture"); diff --git a/crates/webcodex-runner/src/webcodex_runner/lsp/fake_server.rs b/crates/webcodex-runner/src/webcodex_runner/lsp/fake_server.rs index b174414b..8005c822 100644 --- a/crates/webcodex-runner/src/webcodex_runner/lsp/fake_server.rs +++ b/crates/webcodex-runner/src/webcodex_runner/lsp/fake_server.rs @@ -1,6 +1,7 @@ -// Standalone Rust fake LSP server used by `tests.rs`. The test suite compiles -// this file directly with rustc so it never depends on rust-analyzer or a -// scripting runtime and never becomes a production binary target. +// Standalone Rust fake LSP server used by the lifecycle and navigation tests. +// The test suite compiles this file directly with rustc so it never depends on +// a real language server or scripting runtime and never becomes a production +// binary target. use std::env; use std::fs::{self, OpenOptions}; @@ -248,6 +249,12 @@ fn run() -> io::Result<()> { "call_hierarchy_shared_deadline" if method == "callHierarchy/incomingCalls" => { thread::sleep(Duration::from_millis(1600)); write_result(&mut writer, id, method, &body)?; + if let Some(marker) = &marker { + append_marker( + marker, + "hierarchy-complete:callHierarchy/incomingCalls\n", + )?; + } } "call_hierarchy_method_unsupported" if method == "textDocument/prepareCallHierarchy" diff --git a/crates/webcodex-runner/src/webcodex_runner/lsp/mod.rs b/crates/webcodex-runner/src/webcodex_runner/lsp/mod.rs index 0e27308f..0677c9d4 100644 --- a/crates/webcodex-runner/src/webcodex_runner/lsp/mod.rs +++ b/crates/webcodex-runner/src/webcodex_runner/lsp/mod.rs @@ -29,6 +29,9 @@ fn serialize_fake_lsp_test() -> FakeLspTestSerialGuard { FakeLspTestSerialGuard } +#[cfg(test)] +mod test_support; + #[cfg(test)] #[path = "navigation_tests.rs"] mod navigation_tests; diff --git a/crates/webcodex-runner/src/webcodex_runner/lsp/navigation_tests.rs b/crates/webcodex-runner/src/webcodex_runner/lsp/navigation_tests.rs index 00f60400..17d1b7b4 100644 --- a/crates/webcodex-runner/src/webcodex_runner/lsp/navigation_tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/lsp/navigation_tests.rs @@ -1,8 +1,7 @@ use super::navigation::{handle_lsp_request, is_lsp_request_kind}; -use super::position::{lsp_to_public, public_to_lsp, MAX_LSP_DOCUMENT_BYTES}; -use super::supervisor::{ - LspCommand, LspServerKind, LspSupervisor, LspSupervisorConfig, PositionEncoding, -}; +use super::position::MAX_LSP_DOCUMENT_BYTES; +use super::supervisor::{LspCommand, LspServerKind, LspSupervisor, LspSupervisorConfig}; +use super::test_support::{fake_server_path, wait_until}; use crate::lsp_bridge::{ parse_agent_lsp_result_envelope, AgentLspPayload, AgentLspRequest, CallHierarchyDirection, AGENT_LSP_REQUEST_KIND, MAX_CALL_HIERARCHY_CALL_ENTRIES_INSPECTED_PER_RPC, @@ -15,51 +14,10 @@ use serde_json::Value; use std::collections::HashMap; use std::fs; use std::path::{Path, PathBuf}; +#[cfg(windows)] use std::process::Command; -use std::sync::OnceLock; use std::time::{Duration, Instant}; -struct FakeServerBinary { - path: PathBuf, - _dir: tempfile::TempDir, -} - -fn fake_server_binary() -> &'static FakeServerBinary { - static BINARY: OnceLock = OnceLock::new(); - BINARY.get_or_init(|| { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("fake-lsp-server"); - let src = - Path::new(env!("CARGO_MANIFEST_DIR")).join("src/webcodex_runner/lsp/fake_server.rs"); - // The spawned rustc competes with every other test process when the - // whole suite runs in parallel and can fail transiently; retry once - // and keep any real failure diagnosable by capturing stderr. - let mut attempts = 0; - loop { - attempts += 1; - let output = Command::new("rustc") - .arg("--edition=2021") - .arg("--crate-name=webcodex_lsp_fake") - .arg("-o") - .arg(&path) - .arg(&src) - .output() - .expect("rustc fake server"); - if output.status.success() { - break; - } - if attempts >= 2 { - panic!( - "fake LSP server failed to compile after {attempts} attempts ({}):\n{}", - output.status, - String::from_utf8_lossy(&output.stderr) - ); - } - } - FakeServerBinary { path, _dir: dir } - }) -} - /// Minimal agent shell request carrying a typed LSP payload. fn shell_lsp_request(payload: AgentLspPayload) -> ShellAgentShellRequest { ShellAgentShellRequest { @@ -154,13 +112,12 @@ impl NavFixture { ) .unwrap(); - let fake = fake_server_binary(); let marker = temp.path().join("marker"); let exit_marker = temp.path().join("exit"); let supervisor = LspSupervisor::new(LspSupervisorConfig { commands: HashMap::from([( kind, - LspCommand::new(fake.path.as_os_str()) + LspCommand::new(fake_server_path().as_os_str().to_owned()) .arg(scenario) .arg(marker.as_os_str()) .arg(exit_marker.as_os_str()), @@ -405,9 +362,17 @@ fn call_hierarchy_uses_one_shared_operation_deadline() { "hierarchy re-armed per-RPC timeouts instead of sharing its request budget: {elapsed:?}" ); - // Let the fake server finish its stalled handler. A timed-out traversal must - // not resume and issue the next direction after the caller has returned. - std::thread::sleep(Duration::from_millis(800)); + // Wait for the fake server to finish its deliberately stalled incoming-call + // reply. A timed-out traversal must not resume and issue the next direction + // after the caller has returned. + assert!( + wait_until(Duration::from_secs(2), || { + fs::read_to_string(&fixture.marker) + .unwrap_or_default() + .contains("hierarchy-complete:callHierarchy/incomingCalls") + }), + "fake server never completed the stalled incoming-call reply" + ); let marker = fs::read_to_string(&fixture.marker).unwrap(); assert_eq!( marker @@ -1549,22 +1514,6 @@ fn project_relative_normalization_and_no_absolute_in_result() { assert_eq!(envelope["result"]["locations"][0]["path"], "src/main.rs"); } -#[test] -fn utf_encoding_public_conversions() { - let _serial = super::serialize_fake_lsp_test(); - let text = "a😀b\n"; - for encoding in [ - PositionEncoding::Utf8, - PositionEncoding::Utf16, - PositionEncoding::Utf32, - ] { - let (line, character) = public_to_lsp(text, 1, 3, encoding).unwrap(); - assert_eq!(line, 0); - let back = lsp_to_public(text, line, character, encoding).unwrap(); - assert_eq!(back, (1, 3)); - } -} - #[test] fn missing_lsp_payload_returns_structured_error() { let _serial = super::serialize_fake_lsp_test(); diff --git a/crates/webcodex-runner/src/webcodex_runner/lsp/test_support.rs b/crates/webcodex-runner/src/webcodex_runner/lsp/test_support.rs new file mode 100644 index 00000000..15cb3bc0 --- /dev/null +++ b/crates/webcodex-runner/src/webcodex_runner/lsp/test_support.rs @@ -0,0 +1,66 @@ +use std::env; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::OnceLock; +use std::time::{Duration, Instant}; + +struct FakeServerBinary { + _temp: tempfile::TempDir, + path: PathBuf, +} + +pub(super) fn fake_server_path() -> &'static Path { + static BINARY: OnceLock = OnceLock::new(); + &BINARY + .get_or_init(|| { + let manifest = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let source = manifest.join("src/webcodex_runner/lsp/fake_server.rs"); + let temp = tempfile::tempdir().unwrap(); + let path = temp + .path() + .join(format!("webcodex-lsp-fake{}", env::consts::EXE_SUFFIX)); + let rustc = env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc")); + + for attempt in 1..=2 { + let output = Command::new(&rustc) + .arg("--edition=2021") + .arg("--crate-name=webcodex_lsp_fake") + .arg(&source) + .arg("-o") + .arg(&path) + .output() + .expect("run rustc for fake LSP server"); + if output.status.success() { + return FakeServerBinary { _temp: temp, path }; + } + if attempt == 2 { + panic!( + "fake LSP server compilation failed after {attempt} attempts ({}):\n{}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + } + } + unreachable!() + }) + .path +} + +pub(super) fn wait_until(timeout: Duration, mut condition: impl FnMut() -> bool) -> bool { + let deadline = Instant::now() + timeout; + loop { + if condition() { + return true; + } + let now = Instant::now(); + if now >= deadline { + return false; + } + std::thread::sleep( + deadline + .saturating_duration_since(now) + .min(Duration::from_millis(5)), + ); + } +} diff --git a/crates/webcodex-runner/src/webcodex_runner/lsp/tests.rs b/crates/webcodex-runner/src/webcodex_runner/lsp/tests.rs index bef6a9f8..2d642114 100644 --- a/crates/webcodex-runner/src/webcodex_runner/lsp/tests.rs +++ b/crates/webcodex-runner/src/webcodex_runner/lsp/tests.rs @@ -1,61 +1,19 @@ +use super::super::test_support::{fake_server_path, wait_until}; use super::*; use serde_json::{json, Value}; use std::fs; #[cfg(target_os = "linux")] use std::path::Path; use std::path::PathBuf; -use std::process::Command; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{mpsc, Arc, Barrier, OnceLock, Weak}; +use std::sync::{mpsc, Arc, Barrier}; use std::time::{Duration, Instant}; use tempfile::TempDir; -static FAKE_SERVER: OnceLock>> = OnceLock::new(); - -struct FakeServerBinary { - _temp: TempDir, - path: PathBuf, -} - -fn fake_server_binary() -> Arc { - let cache = FAKE_SERVER.get_or_init(|| Mutex::new(Weak::new())); - let mut cached = cache.lock().unwrap(); - if let Some(binary) = cached.upgrade() { - return binary; - } - let manifest = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let source = manifest.join("src/webcodex_runner/lsp/fake_server.rs"); - let temp = tempfile::tempdir().unwrap(); - let output = temp - .path() - .join(format!("webcodex-lsp-fake{}", env::consts::EXE_SUFFIX)); - let rustc = env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc")); - let result = Command::new(rustc) - .arg("--edition=2021") - .arg("--crate-name=webcodex_lsp_fake") - .arg(&source) - .arg("-o") - .arg(&output) - .output() - .expect("run rustc for fake LSP server"); - assert!( - result.status.success(), - "fake LSP server compilation failed: {}", - String::from_utf8_lossy(&result.stderr) - ); - let binary = Arc::new(FakeServerBinary { - _temp: temp, - path: output, - }); - *cached = Arc::downgrade(&binary); - binary -} - struct Fixture { // Drop the supervisor before the temporary directory so the fake server // can persist its graceful-exit marker during supervisor Drop. supervisor: LspSupervisor, - _fake: Arc, _temp: TempDir, root: PathBuf, marker: PathBuf, @@ -101,8 +59,7 @@ impl Fixture { fs::create_dir(&root).unwrap(); let marker = temp.path().join("starts.marker"); let exit_marker = temp.path().join("exit.marker"); - let fake = fake_server_binary(); - let command = LspCommand::new(fake.path.clone()) + let command = LspCommand::new(fake_server_path().as_os_str().to_owned()) .arg(scenario) .arg(marker.as_os_str()) .arg(exit_marker.as_os_str()) @@ -119,7 +76,6 @@ impl Fixture { }); Self { supervisor, - _fake: fake, _temp: temp, root, marker, @@ -155,17 +111,6 @@ impl Fixture { } } -fn wait_until(timeout: Duration, condition: impl Fn() -> bool) -> bool { - let deadline = Instant::now() + timeout; - while Instant::now() < deadline { - if condition() { - return true; - } - std::thread::sleep(Duration::from_millis(5)); - } - condition() -} - #[test] fn lsp_supervisor_is_lazy_and_reuses_one_process_for_concurrent_project_calls() { let _serial = super::super::serialize_fake_lsp_test(); @@ -1031,8 +976,7 @@ fn captured_initialize_options(kind: LspServerKind) -> Value { fs::create_dir(&root).unwrap(); let marker = temp.path().join("starts.marker"); let exit_marker = temp.path().join("exit.marker"); - let fake = fake_server_binary(); - let command = LspCommand::new(fake.path.clone()) + let command = LspCommand::new(fake_server_path().as_os_str().to_owned()) .arg("normal") .arg(marker.as_os_str()) .arg(exit_marker.as_os_str()); @@ -1163,8 +1107,7 @@ fn gopls_process_environment_overrides_ambient_network_settings() { fs::create_dir(&root).unwrap(); let marker = temp.path().join("starts.marker"); let exit_marker = temp.path().join("exit.marker"); - let fake = fake_server_binary(); - let command = LspCommand::new(fake.path.clone()) + let command = LspCommand::new(fake_server_path().as_os_str().to_owned()) .arg("capture_safety_env") .arg(marker.as_os_str()) .arg(exit_marker.as_os_str()) @@ -1389,7 +1332,6 @@ fn lsp_shutdown_and_drop_reap_the_child_process() { let pid = server.process_id(); let Fixture { supervisor, - _fake, _temp, root: _, marker: _, @@ -1399,7 +1341,6 @@ fn lsp_shutdown_and_drop_reap_the_child_process() { drop(supervisor); assert!(wait_until(Duration::from_secs(1), || exit_marker.exists())); assert!(wait_until(Duration::from_secs(1), || !process_exists(pid))); - drop(_fake); drop(_temp); } @@ -1513,7 +1454,6 @@ fn lsp_multiple_hanging_servers_share_one_supervisor_deadline() { ); let Fixture { supervisor, - _fake, _temp, root: _, marker: _, @@ -1525,7 +1465,6 @@ fn lsp_multiple_hanging_servers_share_one_supervisor_deadline() { supervisor_drop_started.elapsed() < Duration::from_millis(100), "supervisor Drop re-armed the configured shutdown timeout" ); - drop(_fake); drop(_temp); } @@ -1563,7 +1502,6 @@ fn lsp_reaper_timeout_does_not_rearm_supervisor_drop_budget() { let Fixture { supervisor, - _fake, _temp, root: _, marker: _, @@ -1579,7 +1517,6 @@ fn lsp_reaper_timeout_does_not_rearm_supervisor_drop_budget() { release_tx.send(()).unwrap(); assert!(wait_until(Duration::from_secs(1), || exited.load(Ordering::SeqCst))); - drop(_fake); drop(_temp); } @@ -1592,8 +1529,7 @@ fn lsp_initialize_timeout_cleanup_uses_configured_shutdown_budget() { fs::create_dir(&root).unwrap(); let marker = temp.path().join("starts.marker"); let exit_marker = temp.path().join("exit.marker"); - let fake = fake_server_binary(); - let command = LspCommand::new(fake.path.clone()) + let command = LspCommand::new(fake_server_path().as_os_str().to_owned()) .arg("initialize_hang") .arg(marker.as_os_str()) .arg(exit_marker.as_os_str()); @@ -1644,7 +1580,6 @@ fn lsp_initialize_timeout_cleanup_uses_configured_shutdown_budget() { } } drop(supervisor); - drop(fake); drop(temp); } @@ -1844,11 +1779,11 @@ fn lsp_rejects_missing_or_non_directory_project_roots_before_spawn() { #[test] fn lsp_command_resolution_uses_explicit_env_then_path_without_shell() { let _serial = super::super::serialize_fake_lsp_test(); - let fake = fake_server_binary(); + let fake = fake_server_path(); let explicit = LspSupervisor::new(LspSupervisorConfig { commands: HashMap::from([( LspServerKind::RustAnalyzer, - LspCommand::new(fake.path.as_os_str()), + LspCommand::new(fake.as_os_str().to_owned()), )]), ..LspSupervisorConfig::default() }); @@ -1865,16 +1800,16 @@ fn lsp_command_resolution_uses_explicit_env_then_path_without_shell() { let (from_env, env_source) = supervisor .resolve_command_from_sources( LspServerKind::RustAnalyzer, - Some(fake.path.as_os_str().to_owned()), + Some(fake.as_os_str().to_owned()), Some(OsStr::new("")), ) .unwrap(); - assert_eq!(from_env.program, fake.path.as_os_str()); + assert_eq!(from_env.program, fake.as_os_str()); assert_eq!(env_source, crate::lsp_bridge::LspCommandSource::Environment); let path_dir = tempfile::tempdir().unwrap(); let analyzer = path_dir.path().join("rust-analyzer"); - fs::copy(&fake.path, &analyzer).unwrap(); + fs::copy(fake, &analyzer).unwrap(); let path = env::join_paths([path_dir.path()]).unwrap(); let (from_path, path_source) = supervisor .resolve_command_from_sources(LspServerKind::RustAnalyzer, None, Some(&path)) @@ -1892,7 +1827,7 @@ fn lsp_command_resolution_uses_explicit_env_then_path_without_shell() { let spaced = tempfile::tempdir().unwrap(); let program = spaced.path().join("fake server with spaces"); - fs::hard_link(&fake.path, &program).unwrap(); + fs::hard_link(fake, &program).unwrap(); let project = tempfile::tempdir().unwrap(); let marker = spaced.path().join("marker"); let exit_marker = spaced.path().join("exit"); From b09a6bfbf73e5c4e4ae8239a25e491a158aac28f Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Sun, 23 Aug 2026 21:51:14 +0800 Subject: [PATCH 3/4] Complete test governance cleanup --- .github/workflows/ci.yml | 5 +- crates/webcodex-admin/src/tests.rs | 69 +++++-- docs/TESTING.md | 7 +- src/agent_quic.rs | 78 ++++--- src/agent_ws.rs | 125 +++++------- src/config.rs | 191 +++++++++--------- .../connector_runtime_tests.rs | 56 +++-- src/lib.rs | 20 +- src/mcp_tests/http_transport.rs | 4 +- src/mcp_tests/tools.rs | 97 +++++---- src/model_surface.rs | 28 ++- src/project_entry_tests.rs | 8 +- src/runtime_http_tests.rs | 17 +- src/shell_client/mod_tests/raw_shell.rs | 8 +- src/test_support.rs | 49 +++++ src/tool_request_trace.rs | 18 +- src/tool_runtime/tests/coding_task.rs | 169 ++++++---------- .../tests/coding_task_semantic_navigation.rs | 42 ++-- .../tests/continuation_feedback.rs | 4 +- src/tool_runtime/tests/dispatch.rs | 169 +++------------- src/tool_runtime/tests/execution_context.rs | 42 ++-- src/tool_runtime/tests/explicit_resume.rs | 14 +- src/tool_runtime/tests/files.rs | 48 ++--- src/tool_runtime/tests/files_helpers.rs | 40 +--- src/tool_runtime/tests/git.rs | 34 ++-- src/tool_runtime/tests/handoff.rs | 6 +- src/tool_runtime/tests/handoff_brief.rs | 6 +- src/tool_runtime/tests/hygiene.rs | 2 +- src/tool_runtime/tests/jobs.rs | 8 +- src/tool_runtime/tests/lsp.rs | 22 +- src/tool_runtime/tests/metadata.rs | 24 +-- src/tool_runtime/tests/observe_jobs.rs | 14 +- src/tool_runtime/tests/permission_gate.rs | 14 +- src/tool_runtime/tests/process.rs | 38 ++-- src/tool_runtime/tests/reconnect.rs | 13 +- src/tool_runtime/tests/script.rs | 60 ++---- .../tests/search_project_texts.rs | 80 +++----- src/tool_runtime/tests/session_shells.rs | 11 +- src/tool_runtime/tests/sessions.rs | 16 +- src/tool_runtime/tests/sessions_current.rs | 23 +-- src/tool_runtime/tests/sessions_git.rs | 16 +- src/tool_runtime/tests/sessions_guards.rs | 51 ++--- .../tests/sessions_instructions.rs | 24 +-- src/tool_runtime/tests/sessions_resolver.rs | 12 +- src/tool_runtime/tests/startup_brief.rs | 8 +- src/tool_runtime/tests/support/agent.rs | 19 +- src/tool_runtime/tests/sync_timeout.rs | 12 +- src/tool_runtime/tests/trusted_smoke.rs | 8 +- src/tool_runtime/tests/validation_events.rs | 24 +-- src/tool_runtime/tests/validation_handoff.rs | 32 ++- src/tool_runtime/tests/validation_summary.rs | 2 +- src/tool_runtime/tests/work_on_project.rs | 18 +- 52 files changed, 831 insertions(+), 1074 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd917cbc..3d042181 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,10 +63,9 @@ jobs: cargo test --locked -p webcodex --lib -- \ openapi_operation_ids_are_minimal \ openapi_all_local_refs_resolve \ - explicit_resume_openapi_metadata_is_distinct_from_session_recording \ + openapi_tool_call_request_does_not_advertise_hidden_start_bootstrap \ mcp_tools_list_returns_same_names_as_runtime \ - mcp_tools_list_parity_with_rest_tools_list \ - explicit_resume_mcp_schema_and_metadata_are_exposed \ + explicit_resume_advanced_compatibility_schema_and_metadata_are_retained \ http_project_connector_lists_and_dispatches_only_canonical_capabilities test-linux-rust: diff --git a/crates/webcodex-admin/src/tests.rs b/crates/webcodex-admin/src/tests.rs index 9491ca83..52d193d3 100644 --- a/crates/webcodex-admin/src/tests.rs +++ b/crates/webcodex-admin/src/tests.rs @@ -14,6 +14,47 @@ fn request(values: &[&str]) -> AdminCliRequest { build_admin_request(&cmd).unwrap() } +struct EnvGuard { + _lock: std::sync::MutexGuard<'static, ()>, + previous: std::collections::BTreeMap>, +} + +impl EnvGuard { + fn new() -> Self { + Self { + _lock: TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()), + previous: std::collections::BTreeMap::new(), + } + } + + fn set(&mut self, name: &str, value: &str) { + self.previous + .entry(name.to_string()) + .or_insert_with(|| std::env::var_os(name)); + std::env::set_var(name, value); + } + + fn remove(&mut self, name: &str) { + self.previous + .entry(name.to_string()) + .or_insert_with(|| std::env::var_os(name)); + std::env::remove_var(name); + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + for (name, value) in &self.previous { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + } +} + #[test] fn admin_usage_keeps_rest_registration_commands_but_not_create_local() { let stdout = usage(); @@ -162,8 +203,8 @@ fn agent_tokens_register_hash_builds_hash_registration_request() { #[test] fn agent_tokens_register_hash_defaults_agent_scopes_and_prefers_admin_token() { - let _guard = TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_ACCOUNT_CREDENTIAL", "wc_acct_default"); + let mut env = EnvGuard::new(); + env.set("WEBCODEX_ACCOUNT_CREDENTIAL", "wc_acct_default"); let req = request(&[ "agent-tokens", "register-hash", @@ -190,13 +231,13 @@ fn agent_tokens_register_hash_defaults_agent_scopes_and_prefers_admin_token() { "agent:job_update" ]) ); - std::env::remove_var("WEBCODEX_ACCOUNT_CREDENTIAL"); + env.remove("WEBCODEX_ACCOUNT_CREDENTIAL"); } #[test] fn agent_tokens_register_hash_uses_credential_env_and_default_account_credential() { - let _guard = TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("CUSTOM_ACCT", "wc_acct_custom"); + let mut env = EnvGuard::new(); + env.set("CUSTOM_ACCT", "wc_acct_custom"); let req = request(&[ "agent-tokens", "register-hash", @@ -214,9 +255,9 @@ fn agent_tokens_register_hash_uses_credential_env_and_default_account_credential "wc_agent_aaaaaaa", ]); assert_eq!(req.token, "wc_acct_custom"); - std::env::remove_var("CUSTOM_ACCT"); + env.remove("CUSTOM_ACCT"); - std::env::set_var("WEBCODEX_ACCOUNT_CREDENTIAL", "wc_acct_default"); + env.set("WEBCODEX_ACCOUNT_CREDENTIAL", "wc_acct_default"); let req = request(&[ "agent-tokens", "register-hash", @@ -232,7 +273,7 @@ fn agent_tokens_register_hash_uses_credential_env_and_default_account_credential "wc_agent_bbbbbbb", ]); assert_eq!(req.token, "wc_acct_default"); - std::env::remove_var("WEBCODEX_ACCOUNT_CREDENTIAL"); + env.remove("WEBCODEX_ACCOUNT_CREDENTIAL"); } #[test] @@ -289,8 +330,8 @@ fn token_file_is_read() { #[test] fn env_token_fallback_is_used() { - let _guard = TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_TOKEN", "fake-env-token"); + let mut env = EnvGuard::new(); + env.set("WEBCODEX_TOKEN", "fake-env-token"); let cmd = parse_admin_cli(&args(&[ "users", "list", @@ -300,13 +341,13 @@ fn env_token_fallback_is_used() { .unwrap(); let req = build_admin_request(&cmd).unwrap(); assert_eq!(req.token, "fake-env-token"); - std::env::remove_var("WEBCODEX_TOKEN"); + env.remove("WEBCODEX_TOKEN"); } #[test] fn explicit_admin_token_wins_over_default_account_credential_env() { - let _guard = TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_ACCOUNT_CREDENTIAL", "fake-account-credential"); + let mut env = EnvGuard::new(); + env.set("WEBCODEX_ACCOUNT_CREDENTIAL", "fake-account-credential"); let cmd = parse_admin_cli(&args(&[ "tokens", "register-hash", @@ -324,7 +365,7 @@ fn explicit_admin_token_wins_over_default_account_credential_env() { .unwrap(); let req = build_admin_request(&cmd).unwrap(); assert_eq!(req.token, "fake-admin"); - std::env::remove_var("WEBCODEX_ACCOUNT_CREDENTIAL"); + env.remove("WEBCODEX_ACCOUNT_CREDENTIAL"); } #[test] diff --git a/docs/TESTING.md b/docs/TESTING.md index 1ed356c3..7967f38b 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -92,8 +92,11 @@ The lanes above define test semantics; workflows decide when to run them. Quick-start shared-key mode intentionally accepts an unknown non-`wc_` Bearer as a lightweight shared-key principal, but invalid `wc_` managed-token prefixes and empty or whitespace Bearer values must still be rejected. -- Sleep, timeout, and polling tests must have bounded timeouts. Prefer channels, - notifications, direct state inspection, or bounded retry loops over raw sleeps. +- Sleep, timeout, and polling tests must be bounded. Positive readiness uses one + absolute deadline created once for the whole wait and never reset after partial + progress; prefer channels, notifications, or direct state inspection. Short + negative probes, semantic grace windows, and exact count/protocol iterations + may remain when they are the contract. - Ignored tests are not dead tests. Each ignored test should have a reason and a documented lane for running it intentionally. diff --git a/src/agent_quic.rs b/src/agent_quic.rs index 776c9a2f..c30a630f 100644 --- a/src/agent_quic.rs +++ b/src/agent_quic.rs @@ -510,6 +510,47 @@ mod tests { /// ALPN used by the QUIC integration tests. const TEST_ALPN: &str = AGENT_QUIC_ALPN_V1; + async fn wait_for_quic_client_connected( + registry: &ShellClientRegistry, + client_id: &str, + expected: bool, + ) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(3); + loop { + let view = registry.get_client_view(client_id).await.unwrap(); + if view.connected == expected { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "client {client_id} connected={expected} was not observed before the 3-second deadline; last status={} transport={}", + view.status, + view.transport + ); + tokio::time::sleep(Duration::from_millis(25)).await; + } + } + + async fn wait_for_quic_job_status( + registry: &ShellClientRegistry, + job_id: &str, + expected: &str, + ) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(3); + loop { + let job = registry.get_job(job_id).await.unwrap(); + if job.status == expected { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "job {job_id} did not reach {expected} before the 3-second deadline; last status={}", + job.status + ); + tokio::time::sleep(Duration::from_millis(25)).await; + } + } + /// Generate a self-signed cert/key for `localhost` using rcgen, returned as /// DER types directly consumable by rustls. Avoids PEM parsing in tests. fn self_signed_cert() -> (CertificateDer<'static>, PrivateKeyDer<'static>) { @@ -824,15 +865,9 @@ mod tests { .last_seen; assert!(after > before, "ping must refresh last_seen"); - // Close the stream; the server reconciles. + // Close the stream; the server reconciles the retained client offline. send.finish().unwrap(); - // Give the server a moment to observe the stream end. - for _ in 0..20 { - tokio::time::sleep(Duration::from_millis(25)).await; - if registry.get_client_view("quic-rt").await.is_some() { - break; - } - } + wait_for_quic_client_connected(®istry, "quic-rt", false).await; client_endpoint.close(quinn::VarInt::from_u32(0), b""); conn.close(quinn::VarInt::from_u32(0), b"done"); } @@ -1046,13 +1081,7 @@ mod tests { .await .unwrap(); - for _ in 0..20 { - tokio::time::sleep(Duration::from_millis(25)).await; - let updated = registry.get_job(&job.job_id).await.unwrap(); - if updated.status == "running" { - break; - } - } + wait_for_quic_job_status(®istry, &job.job_id, "running").await; let updated = registry.get_job(&job.job_id).await.unwrap(); assert_eq!(updated.status, "running"); let (_job, stdout, _stderr, _next_stdout, _next_stderr) = registry @@ -1124,12 +1153,7 @@ mod tests { client_endpoint.close(quinn::VarInt::from_u32(0), b""); conn.close(quinn::VarInt::from_u32(0), b"done"); - for _ in 0..40 { - tokio::time::sleep(Duration::from_millis(25)).await; - if registry.get_job(&job.job_id).await.unwrap().status == "lost" { - break; - } - } + wait_for_quic_job_status(®istry, &job.job_id, "lost").await; let lost = registry.get_job(&job.job_id).await.unwrap(); assert_eq!(lost.status, "lost"); assert!(lost.error.unwrap().contains("disconnected")); @@ -1225,17 +1249,7 @@ mod tests { ) .await .unwrap(); - for _ in 0..40 { - if !registry - .get_client_view("quic-goodbye") - .await - .unwrap() - .connected - { - break; - } - tokio::time::sleep(Duration::from_millis(25)).await; - } + wait_for_quic_client_connected(®istry, "quic-goodbye", false).await; assert!( !registry .get_client_view("quic-goodbye") diff --git a/src/agent_ws.rs b/src/agent_ws.rs index b9b94b81..32459d8c 100644 --- a/src/agent_ws.rs +++ b/src/agent_ws.rs @@ -350,6 +350,43 @@ mod tests { use tokio_tungstenite::tungstenite::http::HeaderValue; use tokio_tungstenite::tungstenite::Message as TungsteniteMessage; + async fn wait_for_ws_client_connected( + registry: &ShellClientRegistry, + client_id: &str, + expected: bool, + ) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(3); + loop { + let view = registry.get_client_view(client_id).await.unwrap(); + if view.connected == expected { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "client {client_id} connected={expected} was not observed before the 3-second deadline; last status={} transport={}", + view.status, + view.transport + ); + tokio::time::sleep(Duration::from_millis(25)).await; + } + } + + async fn wait_for_ws_job_status(registry: &ShellClientRegistry, job_id: &str, expected: &str) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(3); + loop { + let job = registry.get_job(job_id).await.unwrap(); + if job.status == expected { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "job {job_id} did not reach {expected} before the 3-second deadline; last status={}", + job.status + ); + tokio::time::sleep(Duration::from_millis(25)).await; + } + } + fn register_envelope(client_id: &str) -> AgentEnvelope { register_envelope_with_instance(client_id, "ws-inst") } @@ -806,7 +843,8 @@ mod tests { )) .await .unwrap(); - for _ in 0..20 { + let metadata_deadline = tokio::time::Instant::now() + Duration::from_secs(3); + loop { let view = registry.get_client_view("ws-roundtrip").await.unwrap(); if view .policy @@ -817,6 +855,10 @@ mod tests { { break; } + assert!( + tokio::time::Instant::now() < metadata_deadline, + "runtime metadata was not projected before the 3-second deadline" + ); tokio::time::sleep(Duration::from_millis(25)).await; } let view = registry.get_client_view("ws-roundtrip").await.unwrap(); @@ -935,13 +977,7 @@ mod tests { .await .unwrap(); - // Give the server a moment to process the frame. - for _ in 0..20 { - tokio::time::sleep(Duration::from_millis(25)).await; - if registry.get_client_view("ws-pong").await.unwrap().connected { - break; - } - } + wait_for_ws_client_connected(®istry, "ws-pong", true).await; let fresh = registry.get_client_view("ws-pong").await.unwrap(); assert!(fresh.connected, "pong must refresh liveness"); assert_eq!(fresh.status, "online"); @@ -982,16 +1018,9 @@ mod tests { assert_eq!(view1.transport, "websocket"); assert!(view1.connected); - // Disconnect: server reconciles (retains the client record). + // Disconnect: server reconciles and retains the client record offline. drop(ws1); - for _ in 0..40 { - tokio::time::sleep(Duration::from_millis(25)).await; - // Reconcile happens in the background; we just wait for it to - // settle by observing the record is still present. - if registry.get_client_view("ws-recon").await.is_some() { - break; - } - } + wait_for_ws_client_connected(®istry, "ws-recon", false).await; // Reconnect with the same client_id. let (mut ws2, _resp) = connect_async(url).await.unwrap(); @@ -1023,7 +1052,7 @@ mod tests { } #[tokio::test] - async fn ws_disconnect_marks_notifier_removed() { + async fn ws_disconnect_marks_client_offline_and_retains_record() { let registry = Arc::new(ShellClientRegistry::default()); let addr = start_server(registry.clone()).await; @@ -1040,24 +1069,10 @@ mod tests { let view = registry.get_client_view("ws-disc").await.unwrap(); assert_eq!(view.transport, "websocket"); - // Drop the socket. drop(ws); - // Give the server a moment to observe the disconnect and clean up. - for _ in 0..20 { - tokio::time::sleep(Duration::from_millis(50)).await; - // After disconnect the notifier is gone; the client decays to - // stale once last_seen ages past the online window. We only - // assert the notifier was removed by re-registering a notifier - // successfully (which would fail if still present? it replaces, so - // instead assert transport label is unchanged but the client is - // still known). - let _ = registry.get_client_view("ws-disc").await; - } - // The client record is retained (so jobs/results can still resolve) - // but its transport label persists; the key guarantee is that the - // server did not crash and the pump was torn down. - let view = registry.get_client_view("ws-disc").await; - assert!(view.is_some()); + wait_for_ws_client_connected(®istry, "ws-disc", false).await; + let view = registry.get_client_view("ws-disc").await.unwrap(); + assert_eq!(view.transport, "websocket"); } #[tokio::test] @@ -1211,14 +1226,8 @@ mod tests { // Drop the socket; the server must reconcile running jobs to "lost" // instead of leaving them running forever. drop(ws); - let mut lost = registry.get_job(&job.job_id).await.unwrap(); - for _ in 0..40 { - if lost.status == "lost" { - break; - } - tokio::time::sleep(Duration::from_millis(50)).await; - lost = registry.get_job(&job.job_id).await.unwrap(); - } + wait_for_ws_job_status(®istry, &job.job_id, "lost").await; + let lost = registry.get_job(&job.job_id).await.unwrap(); assert_eq!(lost.status, "lost"); assert!(lost.error.unwrap().contains("disconnected")); } @@ -1312,13 +1321,7 @@ mod tests { AgentEnvelope::Registered { success: true, .. } )); drop(ws1); - // Let the server observe the disconnect and reconcile. - for _ in 0..40 { - tokio::time::sleep(Duration::from_millis(25)).await; - if registry.get_client_view("ws-same").await.is_some() { - break; - } - } + wait_for_ws_client_connected(®istry, "ws-same", false).await; // Reconnect with the SAME instance id. let (mut ws2, _resp) = connect_async(url).await.unwrap(); @@ -1370,17 +1373,7 @@ mod tests { )) .await .unwrap(); - for _ in 0..40 { - if !registry - .get_client_view("ws-goodbye") - .await - .unwrap() - .connected - { - break; - } - tokio::time::sleep(Duration::from_millis(25)).await; - } + wait_for_ws_client_connected(®istry, "ws-goodbye", false).await; let offline = registry.get_client_view("ws-goodbye").await.unwrap(); assert!(!offline.connected); @@ -1485,14 +1478,8 @@ mod tests { // B's own disconnect does reconcile the job. drop(ws_b); - let mut lost = registry.get_job(&job.job_id).await.unwrap(); - for _ in 0..40 { - if lost.status == "lost" { - break; - } - tokio::time::sleep(Duration::from_millis(50)).await; - lost = registry.get_job(&job.job_id).await.unwrap(); - } + wait_for_ws_job_status(®istry, &job.job_id, "lost").await; + let lost = registry.get_job(&job.job_id).await.unwrap(); assert_eq!(lost.status, "lost"); } diff --git a/src/config.rs b/src/config.rs index c067da7b..3437b1c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -659,12 +659,12 @@ mod tests { #[test] fn quic_server_config_from_env_disabled_by_default() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_QUIC_ENABLED"); - std::env::remove_var("WEBCODEX_QUIC_LISTEN"); - std::env::remove_var("WEBCODEX_QUIC_CERT"); - std::env::remove_var("WEBCODEX_QUIC_KEY"); - std::env::remove_var("WEBCODEX_QUIC_ALPN"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_QUIC_ENABLED"); + env.remove("WEBCODEX_QUIC_LISTEN"); + env.remove("WEBCODEX_QUIC_CERT"); + env.remove("WEBCODEX_QUIC_KEY"); + env.remove("WEBCODEX_QUIC_ALPN"); let cfg = QuicServerConfig::from_env(); assert!(!cfg.enabled); assert_eq!(cfg.listen, "0.0.0.0:8443"); @@ -692,13 +692,13 @@ mod tests { #[test] fn codex_config_from_env_uses_defaults_when_unset() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); + let mut env = crate::test_support::TestEnvGuard::new(); // Clear CODEX_* env vars so we get deterministic defaults. - std::env::remove_var("CODEX_BIN"); - std::env::remove_var("CODEX_APPROVAL_MODE"); - std::env::remove_var("CODEX_DEFAULT_TIMEOUT_SECS"); - std::env::remove_var("CODEX_MAX_PROMPT_BYTES"); - std::env::remove_var("CODEX_ALLOWED_EXTRA_ARGS"); + env.remove("CODEX_BIN"); + env.remove("CODEX_APPROVAL_MODE"); + env.remove("CODEX_DEFAULT_TIMEOUT_SECS"); + env.remove("CODEX_MAX_PROMPT_BYTES"); + env.remove("CODEX_ALLOWED_EXTRA_ARGS"); let cfg = CodexConfig::from_env(); assert_eq!(cfg.bin, "codex"); @@ -711,12 +711,12 @@ mod tests { #[test] fn codex_config_from_env_parses_overrides() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("CODEX_BIN", "/usr/local/bin/codex"); - std::env::set_var("CODEX_APPROVAL_MODE", "suggest"); - std::env::set_var("CODEX_DEFAULT_TIMEOUT_SECS", "600"); - std::env::set_var("CODEX_MAX_PROMPT_BYTES", "2048"); - std::env::set_var("CODEX_ALLOWED_EXTRA_ARGS", "--verbose, --json, --no-color"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("CODEX_BIN", "/usr/local/bin/codex"); + env.set("CODEX_APPROVAL_MODE", "suggest"); + env.set("CODEX_DEFAULT_TIMEOUT_SECS", "600"); + env.set("CODEX_MAX_PROMPT_BYTES", "2048"); + env.set("CODEX_ALLOWED_EXTRA_ARGS", "--verbose, --json, --no-color"); let cfg = CodexConfig::from_env(); assert_eq!(cfg.bin, "/usr/local/bin/codex"); @@ -732,56 +732,59 @@ mod tests { assert!(!cfg.is_extra_arg_allowed("--danger")); // Restore defaults. - std::env::remove_var("CODEX_BIN"); - std::env::remove_var("CODEX_APPROVAL_MODE"); - std::env::remove_var("CODEX_DEFAULT_TIMEOUT_SECS"); - std::env::remove_var("CODEX_MAX_PROMPT_BYTES"); - std::env::remove_var("CODEX_ALLOWED_EXTRA_ARGS"); + env.remove("CODEX_BIN"); + env.remove("CODEX_APPROVAL_MODE"); + env.remove("CODEX_DEFAULT_TIMEOUT_SECS"); + env.remove("CODEX_MAX_PROMPT_BYTES"); + env.remove("CODEX_ALLOWED_EXTRA_ARGS"); } #[test] fn codex_config_from_env_trims_approval_mode_whitespace() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("CODEX_APPROVAL_MODE", " suggest "); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("CODEX_APPROVAL_MODE", " suggest "); let cfg = CodexConfig::from_env(); assert_eq!(cfg.approval_mode, "suggest"); // An unset/blank value normalizes to empty (disabled). The disabled // sentinels (none/off/disabled) are recognized later by // build_codex_command, so the config keeps the trimmed token. - std::env::set_var("CODEX_APPROVAL_MODE", " "); + env.set("CODEX_APPROVAL_MODE", " "); let cfg = CodexConfig::from_env(); assert_eq!(cfg.approval_mode, ""); - std::env::remove_var("CODEX_APPROVAL_MODE"); + env.remove("CODEX_APPROVAL_MODE"); } #[test] fn codex_config_from_env_ignores_invalid_numeric_values() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("CODEX_DEFAULT_TIMEOUT_SECS", "not-a-number"); - std::env::set_var("CODEX_MAX_PROMPT_BYTES", "-5"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("CODEX_DEFAULT_TIMEOUT_SECS", "not-a-number"); + env.set("CODEX_MAX_PROMPT_BYTES", "-5"); let cfg = CodexConfig::from_env(); assert_eq!(cfg.default_timeout_secs, 3600); assert_eq!(cfg.max_prompt_bytes, 100_000); - std::env::remove_var("CODEX_DEFAULT_TIMEOUT_SECS"); - std::env::remove_var("CODEX_MAX_PROMPT_BYTES"); + env.remove("CODEX_DEFAULT_TIMEOUT_SECS"); + env.remove("CODEX_MAX_PROMPT_BYTES"); } #[test] fn oauth2_config_defaults_to_disabled() { - let _env = crate::auth::AuthEnvGuard::auth_required(); - std::env::remove_var("WEBCODEX_OAUTH2_ENABLED"); - std::env::remove_var("WEBCODEX_PUBLIC_URL"); - std::env::remove_var("WEBCODEX_OAUTH2_ISSUER"); - std::env::remove_var("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_REQUIRE_PKCE"); - std::env::remove_var("WEBCODEX_OAUTH2_SHARED_KEY_BRIDGE"); - std::env::remove_var("WEBCODEX_OAUTH2_TRUSTED_MCP_FILE_CLIENT_IDS"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_SHARED_KEY_ENABLED"); + env.remove("WEBCODEX_ALLOW_ANONYMOUS"); + env.remove("WEBCODEX_OAUTH2_SHARED_KEY_BRIDGE"); + env.remove("WEBCODEX_OAUTH2_ENABLED"); + env.remove("WEBCODEX_PUBLIC_URL"); + env.remove("WEBCODEX_OAUTH2_ISSUER"); + env.remove("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_REQUIRE_PKCE"); + env.remove("WEBCODEX_OAUTH2_SHARED_KEY_BRIDGE"); + env.remove("WEBCODEX_OAUTH2_TRUSTED_MCP_FILE_CLIENT_IDS"); let cfg = OAuth2Config::from_env(); assert!(!cfg.enabled); @@ -796,17 +799,17 @@ mod tests { #[test] fn oauth2_config_from_env_parses_overrides() { - let env = crate::auth::AuthEnvGuard::new(); - std::env::set_var("WEBCODEX_OAUTH2_ENABLED", "true"); - std::env::set_var("WEBCODEX_OAUTH2_ISSUER", "https://example.com"); - std::env::set_var("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS", "1800"); - std::env::set_var("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS", "86400"); - std::env::set_var("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS", "600"); - std::env::set_var("WEBCODEX_OAUTH2_REQUIRE_PKCE", "false"); - env.enable_oauth2_shared_key_bridge(); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_OAUTH2_ENABLED", "true"); + env.set("WEBCODEX_OAUTH2_ISSUER", "https://example.com"); + env.set("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS", "1800"); + env.set("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS", "86400"); + env.set("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS", "600"); + env.set("WEBCODEX_OAUTH2_REQUIRE_PKCE", "false"); + env.set("WEBCODEX_OAUTH2_SHARED_KEY_BRIDGE", "true"); let trusted_a = format!("wc_client_{}", "a".repeat(64)); let trusted_b = format!("wc_client_{}", "b".repeat(64)); - std::env::set_var( + env.set( "WEBCODEX_OAUTH2_TRUSTED_MCP_FILE_CLIENT_IDS", format!( " {trusted_a} , invalid-client, {trusted_a}, {trusted_b}, wc_client_{} ", @@ -824,119 +827,119 @@ mod tests { assert!(cfg.shared_key_bridge_enabled); assert_eq!(cfg.trusted_mcp_file_client_ids, vec![trusted_a, trusted_b]); - std::env::remove_var("WEBCODEX_OAUTH2_ENABLED"); - std::env::remove_var("WEBCODEX_OAUTH2_ISSUER"); - std::env::remove_var("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS"); - std::env::remove_var("WEBCODEX_OAUTH2_REQUIRE_PKCE"); - std::env::remove_var("WEBCODEX_OAUTH2_TRUSTED_MCP_FILE_CLIENT_IDS"); + env.remove("WEBCODEX_OAUTH2_ENABLED"); + env.remove("WEBCODEX_OAUTH2_ISSUER"); + env.remove("WEBCODEX_OAUTH2_ACCESS_TOKEN_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_REFRESH_TOKEN_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_AUTH_CODE_TTL_SECS"); + env.remove("WEBCODEX_OAUTH2_REQUIRE_PKCE"); + env.remove("WEBCODEX_OAUTH2_TRUSTED_MCP_FILE_CLIENT_IDS"); } #[test] fn oauth2_config_issuer_prefers_oauth2_issuer_over_public_url() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_PUBLIC_URL", "https://pub.example.com"); - std::env::set_var("WEBCODEX_OAUTH2_ISSUER", "https://issuer.example.com"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_PUBLIC_URL", "https://pub.example.com"); + env.set("WEBCODEX_OAUTH2_ISSUER", "https://issuer.example.com"); let cfg = OAuth2Config::from_env(); assert_eq!(cfg.issuer.as_deref(), Some("https://issuer.example.com")); - std::env::remove_var("WEBCODEX_OAUTH2_ISSUER"); + env.remove("WEBCODEX_OAUTH2_ISSUER"); // Falls back to WEBCODEX_PUBLIC_URL when OAUTH2_ISSUER is absent. let cfg = OAuth2Config::from_env(); assert_eq!(cfg.issuer.as_deref(), Some("https://pub.example.com")); - std::env::remove_var("WEBCODEX_PUBLIC_URL"); + env.remove("WEBCODEX_PUBLIC_URL"); } #[test] fn codex_config_allowed_extra_args_ignores_empty_entries() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("CODEX_ALLOWED_EXTRA_ARGS", " --verbose , , --json "); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("CODEX_ALLOWED_EXTRA_ARGS", " --verbose , , --json "); let cfg = CodexConfig::from_env(); assert_eq!(cfg.allowed_extra_args, vec!["--verbose", "--json"]); - std::env::remove_var("CODEX_ALLOWED_EXTRA_ARGS"); + env.remove("CODEX_ALLOWED_EXTRA_ARGS"); } #[test] fn load_startup_env_files_explicit_path_loads_webcodex_env() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); + let mut env = crate::test_support::TestEnvGuard::new(); let dir = tempfile::tempdir().unwrap(); let new_file = dir.path().join("webcodex.env"); std::fs::write(&new_file, "WEBCODEX_TOKEN=new\n").unwrap(); - std::env::set_var("WEBCODEX_ENV_FILE", &new_file); - std::env::remove_var("WEBCODEX_TOKEN"); + env.set("WEBCODEX_ENV_FILE", &new_file); + env.remove("WEBCODEX_TOKEN"); let loads = load_startup_env_files().unwrap(); assert_eq!(loads.len(), 1); assert_eq!(loads[0].path, new_file); assert_eq!(std::env::var("WEBCODEX_TOKEN").unwrap(), "new"); - std::env::remove_var("WEBCODEX_ENV_FILE"); + env.remove("WEBCODEX_ENV_FILE"); } #[test] fn mcp_compact_schemas_defaults_off() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_MCP_COMPACT_SCHEMAS"); assert!(!mcp_compact_schemas_enabled()); } #[test] fn mcp_compact_schemas_true_enables() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); assert!(mcp_compact_schemas_enabled()); - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "1"); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "1"); assert!(mcp_compact_schemas_enabled()); - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "false"); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "false"); assert!(!mcp_compact_schemas_enabled()); - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "maybe"); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "maybe"); // Invalid values are treated as unset by env_flag -> default false. assert!(!mcp_compact_schemas_enabled()); - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); + env.remove("WEBCODEX_MCP_COMPACT_SCHEMAS"); } #[test] fn action_compact_responses_defaults_off() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_ACTION_COMPACT_RESPONSES"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_ACTION_COMPACT_RESPONSES"); assert!(!action_compact_responses_enabled()); } #[test] fn action_compact_responses_true_enables() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_ACTION_COMPACT_RESPONSES", "true"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_ACTION_COMPACT_RESPONSES", "true"); assert!(action_compact_responses_enabled()); - std::env::set_var("WEBCODEX_ACTION_COMPACT_RESPONSES", "1"); + env.set("WEBCODEX_ACTION_COMPACT_RESPONSES", "1"); assert!(action_compact_responses_enabled()); - std::env::set_var("WEBCODEX_ACTION_COMPACT_RESPONSES", "yes"); + env.set("WEBCODEX_ACTION_COMPACT_RESPONSES", "yes"); assert!(action_compact_responses_enabled()); - std::env::set_var("WEBCODEX_ACTION_COMPACT_RESPONSES", "false"); + env.set("WEBCODEX_ACTION_COMPACT_RESPONSES", "false"); assert!(!action_compact_responses_enabled()); - std::env::set_var("WEBCODEX_ACTION_COMPACT_RESPONSES", "maybe"); + env.set("WEBCODEX_ACTION_COMPACT_RESPONSES", "maybe"); assert!(!action_compact_responses_enabled()); - std::env::remove_var("WEBCODEX_ACTION_COMPACT_RESPONSES"); + env.remove("WEBCODEX_ACTION_COMPACT_RESPONSES"); } #[test] fn tool_request_trace_defaults_off() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); assert!(!tool_request_trace_enabled()); } #[test] fn tool_request_trace_true_enables() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_TOOL_REQUEST_TRACE", "true"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_TOOL_REQUEST_TRACE", "true"); assert!(tool_request_trace_enabled()); - std::env::set_var("WEBCODEX_TOOL_REQUEST_TRACE", "false"); + env.set("WEBCODEX_TOOL_REQUEST_TRACE", "false"); assert!(!tool_request_trace_enabled()); - std::env::set_var("WEBCODEX_TOOL_REQUEST_TRACE", "maybe"); + env.set("WEBCODEX_TOOL_REQUEST_TRACE", "maybe"); assert!(!tool_request_trace_enabled()); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); } } diff --git a/src/connector_runtime/connector_runtime_tests.rs b/src/connector_runtime/connector_runtime_tests.rs index 8917b07d..03be0bd5 100644 --- a/src/connector_runtime/connector_runtime_tests.rs +++ b/src/connector_runtime/connector_runtime_tests.rs @@ -134,7 +134,8 @@ async fn register_agent_with_lsp_capabilities( } async fn next_lsp_request(registry: &ShellClientRegistry) -> ShellAgentShellRequest { - for _ in 0..200 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -148,9 +149,12 @@ async fn next_lsp_request(registry: &ShellClientRegistry) -> ShellAgentShellRequ assert!(request.command.is_empty()); return request; } + assert!( + tokio::time::Instant::now() < deadline, + "connector did not dispatch an LSP request within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("connector did not dispatch an LSP request"); } async fn complete_lsp_request( @@ -897,7 +901,8 @@ async fn inspect_to_write_keeps_task_and_rechecks_write_authority() { let responder_registry = registry.clone(); let responder = tokio::spawn(async move { - for _ in 0..1_000 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = responder_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -934,9 +939,12 @@ async fn inspect_to_write_keeps_task_and_rechecks_write_authority() { .unwrap(); return; } + assert!( + tokio::time::Instant::now() < deadline, + "workspace upgrade did not register the isolated project within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("workspace upgrade did not register the isolated project"); }); let upgraded = connector .call_for_window( @@ -1117,7 +1125,8 @@ async fn writable_start_registers_and_releases_a_reusable_git_worktree() { .unwrap(); let agent_registry = registry.clone(); let responder = tokio::spawn(async move { - for _ in 0..1_000 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = agent_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -1155,9 +1164,12 @@ async fn writable_start_registers_and_releases_a_reusable_git_worktree() { .unwrap(); return; } + assert!( + tokio::time::Instant::now() < deadline, + "connector did not register its isolated execution project within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("connector did not register its isolated execution project"); }); let owner = auth("u1"); let outcome = connector @@ -1192,7 +1204,8 @@ async fn writable_start_registers_and_releases_a_reusable_git_worktree() { ); let check_registry = registry.clone(); let check_responder = tokio::spawn(async move { - for _ in 0..1_000 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = check_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -1233,9 +1246,12 @@ async fn writable_start_registers_and_releases_a_reusable_git_worktree() { .unwrap(); return; } + assert!( + tokio::time::Instant::now() < deadline, + "connector did not dispatch structured validation within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("connector did not dispatch structured validation"); }); let checked = connector .call( @@ -1396,7 +1412,8 @@ async fn failed_task_binding_releases_prepared_workspace_for_retry() { let responder_registry = registry.clone(); let responder = tokio::spawn(async move { let mut registrations = 0; - for _ in 0..2_000 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = responder_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -1436,9 +1453,12 @@ async fn failed_task_binding_releases_prepared_workspace_for_retry() { return; } } + assert!( + tokio::time::Instant::now() < deadline, + "connector did not issue both workspace registrations within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("connector did not issue both workspace registrations"); }); let owner = auth("u1"); @@ -1535,7 +1555,8 @@ async fn canonical_read_reaches_bound_executor_and_advances_event_cursor() { let agent_registry = registry.clone(); let responder = tokio::spawn(async move { - for _ in 0..100 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = agent_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -1574,9 +1595,12 @@ async fn canonical_read_reaches_bound_executor_and_advances_event_cursor() { .unwrap(); return; } + assert!( + tokio::time::Instant::now() < deadline, + "connector did not dispatch the read to its bound executor within 10 seconds" + ); tokio::task::yield_now().await; } - panic!("connector did not dispatch the read to its bound executor"); }); let outcome = connector .call( @@ -2159,7 +2183,8 @@ async fn code_impact_is_available_in_normal_inspect_and_read_only_tasks() { let registration = if mode == "normal" { let registration_registry = registry.clone(); Some(tokio::spawn(async move { - for _ in 0..1_000 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = registration_registry .poll(ShellAgentPollRequest { client_id: "hosted".to_string(), @@ -2195,9 +2220,12 @@ async fn code_impact_is_available_in_normal_inspect_and_read_only_tasks() { .unwrap(); return; } + assert!( + tokio::time::Instant::now() < deadline, + "normal task did not register its isolated execution project within 10 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(1)).await; } - panic!("normal task did not register its isolated execution project"); })) } else { None diff --git a/src/lib.rs b/src/lib.rs index 738e0f50..a97be925 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -580,16 +580,16 @@ mod tests { #[test] fn test_config_from_env_defaults() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - // Clear env vars to test defaults - std::env::remove_var("WEBCODEX_ADDR"); - std::env::remove_var("WEBCODEX_DATA"); - std::env::remove_var("WEBCODEX_TOKEN"); - std::env::remove_var("CODEX_BIN"); - std::env::remove_var("CODEX_APPROVAL_MODE"); - std::env::remove_var("CODEX_DEFAULT_TIMEOUT_SECS"); - std::env::remove_var("CODEX_MAX_PROMPT_BYTES"); - std::env::remove_var("CODEX_ALLOWED_EXTRA_ARGS"); + let mut env = crate::test_support::TestEnvGuard::new(); + // Clear env vars to test defaults; Drop restores the process environment. + env.remove("WEBCODEX_ADDR"); + env.remove("WEBCODEX_DATA"); + env.remove("WEBCODEX_TOKEN"); + env.remove("CODEX_BIN"); + env.remove("CODEX_APPROVAL_MODE"); + env.remove("CODEX_DEFAULT_TIMEOUT_SECS"); + env.remove("CODEX_MAX_PROMPT_BYTES"); + env.remove("CODEX_ALLOWED_EXTRA_ARGS"); let config = Config::from_env(); assert_eq!(config.addr, "0.0.0.0:8080"); diff --git a/src/mcp_tests/http_transport.rs b/src/mcp_tests/http_transport.rs index ebcbf825..e95a82b6 100644 --- a/src/mcp_tests/http_transport.rs +++ b/src/mcp_tests/http_transport.rs @@ -517,8 +517,8 @@ async fn http_mcp_initialize_success() { async fn http_mcp_tools_list_success() { // Default (non-compact) HTTP tools/list: full schema fields present. // Compact-mode shape is covered by mcp_tools_list_compact_*. - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_MCP_COMPACT_SCHEMAS"); let config = test_config(Some("secret")); let (_tmp, db) = test_db(); let runtime = Arc::new(test_runtime()); diff --git a/src/mcp_tests/tools.rs b/src/mcp_tests/tools.rs index 30902c38..72bb36e9 100644 --- a/src/mcp_tests/tools.rs +++ b/src/mcp_tests/tools.rs @@ -1,5 +1,32 @@ use super::*; +async fn wait_for_mcp_agent_request( + registry: &crate::shell_client::ShellClientRegistry, + client_id: &str, + agent_instance_id: &str, + label: &str, +) -> crate::shell_protocol::ShellAgentShellRequest { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { + if let Some(request) = registry + .poll(ShellAgentPollRequest { + client_id: client_id.to_string(), + agent_instance_id: agent_instance_id.to_string(), + projects: None, + }) + .await + .unwrap() + { + return request; + } + assert!( + tokio::time::Instant::now() < deadline, + "{label} did not dispatch within 10 seconds" + ); + tokio::task::yield_now().await; + } +} + // The compact switch is read per tools/list request, so `WEBCODEX_MCP_COMPACT_SCHEMAS` // must stay stable (and serialized against other env-mutating tests) for the whole // async body below. The full-operator surface is passed explicitly instead of via env. @@ -12,7 +39,7 @@ async fn mcp_tools_list_returns_same_names_as_runtime() { // covered by dedicated tests: // `mcp_tools_list_default_retains_output_schema` and // `mcp_tools_list_compact_omits_output_schema_only`. - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); + let mut env = crate::test_support::TestEnvGuard::new(); let runtime = test_runtime_with_surface(ModelSurface::FullOperatorRuntime); let runtime_names: Vec = registered_tool_specs() .iter() @@ -26,9 +53,9 @@ async fn mcp_tools_list_returns_same_names_as_runtime() { for compact in [false, true] { if compact { - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); } else { - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); + env.remove("WEBCODEX_MCP_COMPACT_SCHEMAS"); } let outcome = handle_mcp_request( &runtime, @@ -115,7 +142,6 @@ async fn mcp_tools_list_returns_same_names_as_runtime() { } } } - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); } #[test] @@ -473,29 +499,13 @@ async fn mcp_image_call_returns_native_image_for_remote_agent_project() { } }); - let mut request = None; - for _ in 0..200 { - request = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: client_id.to_string(), - agent_instance_id: agent_instance_id.to_string(), - projects: None, - }) - .await - .unwrap(); - if request.is_some() || call.is_finished() { - break; - } - tokio::task::yield_now().await; - } - let request = match request { - Some(request) => request, - None => { - let outcome = call.await.unwrap(); - panic!("MCP image call should enqueue a remote artifact read, got {outcome:?}"); - } - }; + let request = wait_for_mcp_agent_request( + &runtime.shell_clients, + client_id, + agent_instance_id, + "MCP image call", + ) + .await; assert_eq!(request.kind, "file_read_project_artifact"); assert_eq!(request.cwd.as_deref(), Some("/remote/session-atlas")); let payload: Value = serde_json::from_str(request.content.as_deref().unwrap()).unwrap(); @@ -743,8 +753,8 @@ fn mcp_tools_list_compact_is_smaller_than_full_serialized() { #[allow(clippy::await_holding_lock)] #[tokio::test] async fn mcp_tools_call_still_returns_structured_content_under_compact_flag() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_MCP_COMPACT_SCHEMAS", "true"); let runtime = test_runtime(); let outcome = handle_mcp_request( &runtime, @@ -756,7 +766,6 @@ async fn mcp_tools_call_still_returns_structured_content_under_compact_flag() { None, ) .await; - std::env::remove_var("WEBCODEX_MCP_COMPACT_SCHEMAS"); let McpOutcome::Ok(value) = outcome else { panic!("expected Ok, got {outcome:?}"); }; @@ -1112,8 +1121,8 @@ async fn mcp_tools_list_hides_testing_metadata_while_raw_call_records_it() { #[tokio::test] async fn mcp_show_changes_distinguishes_reserved_session_id_from_query_session_id() { use crate::shell_protocol::{ - ShellAgentPollRequest, ShellAgentProjectSummary, ShellAgentResultRequest, - ShellClientCapabilities, ShellClientRegisterRequest, + ShellAgentProjectSummary, ShellAgentResultRequest, ShellClientCapabilities, + ShellClientRegisterRequest, }; let runtime = test_runtime(); @@ -1199,23 +1208,13 @@ async fn mcp_show_changes_distinguishes_reserved_session_id_from_query_session_i Some(&auth), ); let complete = async { - let mut req = None; - for _ in 0..50 { - req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "mcp-client".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if req.is_some() { - break; - } - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } - let req = req.expect("show_changes should enqueue an agent shell request"); + let req = wait_for_mcp_agent_request( + &runtime.shell_clients, + "mcp-client", + "inst", + "show_changes", + ) + .await; let stdout = "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nabc123\0abc123\0test head\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n"; runtime .shell_clients diff --git a/src/model_surface.rs b/src/model_surface.rs index 6499a8b3..363780e2 100644 --- a/src/model_surface.rs +++ b/src/model_surface.rs @@ -92,10 +92,6 @@ pub(crate) fn local_coding_tool_specs() -> Vec { mod tests { use super::*; - fn env_guard() -> std::sync::MutexGuard<'static, ()> { - crate::admin_cli::TEST_ENV_LOCK.lock().unwrap() - } - #[test] fn local_coding_tool_names_are_ordered_and_unique() { let mut seen = std::collections::HashSet::new(); @@ -186,28 +182,28 @@ mod tests { #[test] fn default_surface_is_local_coding_without_connector_or_env() { - let _guard = env_guard(); - std::env::remove_var(MCP_MODEL_SURFACE_ENV); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove(MCP_MODEL_SURFACE_ENV); assert_eq!(resolve_model_surface(None), Ok(ModelSurface::LocalCoding)); } #[test] fn explicit_local_coding_and_full_operator_values() { - let _guard = env_guard(); - std::env::set_var(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_LOCAL_CODING_V1); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_LOCAL_CODING_V1); assert_eq!(resolve_model_surface(None), Ok(ModelSurface::LocalCoding)); - std::env::set_var(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_FULL_OPERATOR_V1); + env.set(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_FULL_OPERATOR_V1); assert_eq!( resolve_model_surface(None), Ok(ModelSurface::FullOperatorRuntime) ); - std::env::remove_var(MCP_MODEL_SURFACE_ENV); + env.remove(MCP_MODEL_SURFACE_ENV); } #[test] fn connector_configured_selects_canonical_connector() { - let _guard = env_guard(); - std::env::remove_var(MCP_MODEL_SURFACE_ENV); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove(MCP_MODEL_SURFACE_ENV); let context = connector_context(); assert_eq!( resolve_model_surface(Some(&context)), @@ -217,17 +213,17 @@ mod tests { #[test] fn invalid_and_conflicting_values_fail_resolution() { - let _guard = env_guard(); - std::env::set_var(MCP_MODEL_SURFACE_ENV, "bogus-surface"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set(MCP_MODEL_SURFACE_ENV, "bogus-surface"); let error = resolve_model_surface(None).expect_err("invalid value must fail"); assert!(error.contains("unsupported"), "error: {error}"); assert!(error.contains("bogus-surface"), "error: {error}"); - std::env::set_var(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_LOCAL_CODING_V1); + env.set(MCP_MODEL_SURFACE_ENV, MCP_MODEL_SURFACE_LOCAL_CODING_V1); let context = connector_context(); let error = resolve_model_surface(Some(&context)).expect_err("conflict must fail"); assert!(error.contains("cannot be combined"), "error: {error}"); assert!(error.contains(MCP_MODEL_SURFACE_ENV), "error: {error}"); - std::env::remove_var(MCP_MODEL_SURFACE_ENV); + env.remove(MCP_MODEL_SURFACE_ENV); } } diff --git a/src/project_entry_tests.rs b/src/project_entry_tests.rs index cdf99f39..eb37dc62 100644 --- a/src/project_entry_tests.rs +++ b/src/project_entry_tests.rs @@ -456,7 +456,8 @@ async fn next_project_agent_request( registry: &ShellClientRegistry, client_id: &str, ) -> ShellAgentShellRequest { - for _ in 0..2_000 { + let deadline = tokio::time::Instant::now() + Duration::from_secs(10); + loop { if let Some(request) = registry .poll(ShellAgentPollRequest { client_id: client_id.to_string(), @@ -468,9 +469,12 @@ async fn next_project_agent_request( { return request; } + assert!( + tokio::time::Instant::now() < deadline, + "configured project Agent did not receive a request within 10 seconds" + ); tokio::time::sleep(Duration::from_millis(1)).await; } - panic!("configured project Agent did not receive a request"); } async fn complete_project_agent_request( diff --git a/src/runtime_http_tests.rs b/src/runtime_http_tests.rs index 183e664a..9117fee6 100644 --- a/src/runtime_http_tests.rs +++ b/src/runtime_http_tests.rs @@ -1799,9 +1799,9 @@ async fn api_show_changes_with_session_id() { .await }; let complete = async { - let mut req = None; - for _ in 0..20 { - req = registry + let deadline = tokio::time::Instant::now() + Duration::from_secs(10); + let req = loop { + let req = registry .poll(ShellAgentPollRequest { client_id: "importer".to_string(), agent_instance_id: "inst-import".to_string(), @@ -1809,12 +1809,15 @@ async fn api_show_changes_with_session_id() { }) .await .unwrap(); - if req.is_some() { - break; + if let Some(req) = req { + break req; } + assert!( + tokio::time::Instant::now() < deadline, + "show_changes did not enqueue an agent request within 10 seconds" + ); tokio::task::yield_now().await; - } - let req = req.expect("show_changes should enqueue an agent shell request"); + }; let stdout = "## main\n?? README.md\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nstatus_exit=0\nrepository_probe=inside_worktree\nrepository_probe_exit=0\nfiles_total=1\nfiles_returned=1\nfiles_truncated=0\nfiles_limit=200\nmodified=0\nadded=0\ndeleted=0\nrenamed=0\ncopied=0\nuntracked=1\nconflicted=0\nstaged=0\nunstaged=0\nstatus_trunc_count=0\nstatus_trunc_bytes=0\nstatus_trunc_path=0\nstatus_bytes=20\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ncommit=abc123\nshort=abc123\nsummary=test head\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nhead_exit=0\nhead_truncated=0\nhead_bytes=44\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ndiff_stat_exit=0\ndiff_stat_truncated=0\ndiff_stat_bytes=0\n"; registry .complete(ShellAgentResultRequest { diff --git a/src/shell_client/mod_tests/raw_shell.rs b/src/shell_client/mod_tests/raw_shell.rs index 5a42072b..614b2cd8 100644 --- a/src/shell_client/mod_tests/raw_shell.rs +++ b/src/shell_client/mod_tests/raw_shell.rs @@ -35,7 +35,8 @@ async fn raw_shell_run_wait_timeout_preserves_known_dispatch_evidence() { })) .send(&service); let poll = async { - for _ in 0..200 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); + loop { if let Some(request) = registry .poll(ShellAgentPollRequest { client_id: client_id.to_string(), @@ -47,9 +48,12 @@ async fn raw_shell_run_wait_timeout_preserves_known_dispatch_evidence() { { return request; } + assert!( + tokio::time::Instant::now() < deadline, + "raw shell request was not dispatched within 2 seconds" + ); tokio::time::sleep(std::time::Duration::from_millis(5)).await; } - panic!("raw shell request was not dispatched"); }; let (mut response, request) = tokio::join!(response, poll); assert_eq!(request.kind, "run_shell"); diff --git a/src/test_support.rs b/src/test_support.rs index a2fc77dd..1aabd210 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -8,6 +8,55 @@ use std::path::PathBuf; use std::sync::Arc; +/// Panic-safe process-global environment mutation guard for server tests. +/// +/// The guard holds the canonical server test env lock for its full lifetime, +/// snapshots each variable only before the first mutation, and restores the +/// original process environment during unwinding as well as ordinary drop. +pub(crate) struct TestEnvGuard { + _lock: std::sync::MutexGuard<'static, ()>, + previous: std::collections::BTreeMap>, +} + +impl TestEnvGuard { + pub(crate) fn new() -> Self { + Self { + _lock: crate::admin_cli::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()), + previous: std::collections::BTreeMap::new(), + } + } + + fn remember(&mut self, name: &str) { + if !self.previous.contains_key(name) { + self.previous + .insert(name.to_string(), std::env::var_os(name)); + } + } + + pub(crate) fn set(&mut self, name: &str, value: impl AsRef) { + self.remember(name); + std::env::set_var(name, value); + } + + pub(crate) fn remove(&mut self, name: &str) { + self.remember(name); + std::env::remove_var(name); + } +} + +impl Drop for TestEnvGuard { + fn drop(&mut self) { + for (name, value) in &self.previous { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + } +} + /// Minimal `Config` for tests (token sets whether auth is enabled). pub(crate) fn test_config(token: Option<&str>) -> Arc { Arc::new(crate::Config { diff --git a/src/tool_request_trace.rs b/src/tool_request_trace.rs index b921a9d9..901d985e 100644 --- a/src/tool_request_trace.rs +++ b/src/tool_request_trace.rs @@ -305,19 +305,19 @@ mod tests { #[test] fn estimate_json_bytes_is_none_when_trace_disabled() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); assert!(estimate_json_bytes(&json!({"a": 1})).is_none()); - std::env::set_var("WEBCODEX_TOOL_REQUEST_TRACE", "true"); + env.set("WEBCODEX_TOOL_REQUEST_TRACE", "true"); let n = estimate_json_bytes(&json!({"a": 1})).expect("size when enabled"); assert!(n > 0); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); } #[test] fn incomplete_drop_is_safe_when_disabled() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); let guard = ToolRequestLifecycle::new( "mcp", "trace-test".into(), @@ -332,12 +332,12 @@ mod tests { #[test] fn completed_drop_is_silent() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); - std::env::set_var("WEBCODEX_TOOL_REQUEST_TRACE", "true"); + let mut env = crate::test_support::TestEnvGuard::new(); + env.set("WEBCODEX_TOOL_REQUEST_TRACE", "true"); let guard = ToolRequestLifecycle::new("api", "trace-ok".into(), "-", "POST /api/tools/call", None); guard.handler_returned(200, Some(12), Some(true), Some(true), "ok"); drop(guard); - std::env::remove_var("WEBCODEX_TOOL_REQUEST_TRACE"); + env.remove("WEBCODEX_TOOL_REQUEST_TRACE"); } } diff --git a/src/tool_runtime/tests/coding_task.rs b/src/tool_runtime/tests/coding_task.rs index 8d9836a5..732a2cd3 100644 --- a/src/tool_runtime/tests/coding_task.rs +++ b/src/tool_runtime/tests/coding_task.rs @@ -11,6 +11,27 @@ use crate::tool_runtime::{ use serde_json::{json, Value}; use std::fs; use std::path::PathBuf; +use std::time::{Duration, Instant}; + +async fn service_agent_task_until_finished( + runtime: &ToolRuntime, + client_id: &str, + task: &tokio::task::JoinHandle, + label: &str, +) { + let deadline = Instant::now() + Duration::from_secs(10); + while !task.is_finished() { + assert!( + Instant::now() < deadline, + "{label} did not finish within the 10-second test deadline" + ); + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { + complete_agent_request_by_running_locally(runtime, client_id, request).await; + } else { + tokio::time::sleep(Duration::from_millis(5)).await; + } + } +} #[test] fn coding_task_tools_are_registered_in_metadata_and_openapi() { @@ -356,12 +377,14 @@ async fn start_coding_task_creates_managed_temporary_project_then_restores_it_as }); let mut create_seen = false; - for _ in 0..300 { - if task.is_finished() { - break; - } - let Some(request) = next_patch_agent_request(&runtime1, client_id).await else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; + let startup_deadline = Instant::now() + Duration::from_secs(10); + while !task.is_finished() { + assert!( + Instant::now() < startup_deadline, + "managed temporary startup did not finish within the 10-second test deadline" + ); + let Some(request) = probe_patch_agent_request(&runtime1, client_id).await else { + tokio::time::sleep(Duration::from_millis(5)).await; continue; }; if request.kind == "create_project" { @@ -524,17 +547,7 @@ async fn start_coding_task_can_explicitly_disable_current_binding() { } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(request) = next_patch_agent_request(&runtime, "coding-start").await { - complete_agent_request_by_running_locally(&runtime, "coding-start", request).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!(task.is_finished(), "start_coding_task did not finish"); + service_agent_task_until_finished(&runtime, "coding-start", &task, "start_coding_task").await; let result = task.await.unwrap(); @@ -906,20 +919,7 @@ async fn start_coding_task_with_git_inspection( .await } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(runtime, client_id).await { - complete_agent_request_by_running_locally(runtime, client_id, req).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "start_coding_task did not finish after servicing startup agent requests" - ); + service_agent_task_until_finished(runtime, client_id, &task, "start_coding_task").await; task.await.unwrap() } @@ -943,20 +943,7 @@ async fn start_coding_task_serviced( .await } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(runtime, client_id).await { - complete_agent_request_by_running_locally(runtime, client_id, req).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "start_coding_task did not finish after servicing startup agent requests" - ); + service_agent_task_until_finished(runtime, client_id, &task, "start_coding_task").await; task.await.unwrap() } @@ -1414,9 +1401,7 @@ async fn finish_coding_task_requires_explicit_session_and_returns_structured_fie .await } }); - let req = next_patch_agent_request(&runtime, "coding-finish") - .await - .expect("finish_coding_task should inspect changes through the agent"); + let req = wait_for_patch_agent_request(&runtime, "coding-finish").await; assert_internal_posix_script_contains(&req, "git status --porcelain=v1 -b"); let show_changes_stdout = "## main\n M README.md\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nstatus_exit=0\nrepository_probe=inside_worktree\nrepository_probe_exit=0\nfiles_total=1\nfiles_returned=1\nfiles_truncated=0\nfiles_limit=200\nmodified=1\nadded=0\ndeleted=0\nrenamed=0\ncopied=0\nuntracked=0\nconflicted=0\nstaged=0\nunstaged=1\nstatus_trunc_count=0\nstatus_trunc_bytes=0\nstatus_trunc_path=0\nstatus_bytes=20\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ncommit=abc123\nshort=abc123\nsummary=add readme\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nhead_exit=0\nhead_truncated=0\nhead_bytes=45\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n README.md | 1 +\n 1 file changed, 1 insertion(+)\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ndiff_stat_exit=0\ndiff_stat_truncated=0\ndiff_stat_bytes=52\n"; complete_patch_agent_request( @@ -1531,20 +1516,13 @@ async fn finish_coding_task_summary_only_is_compact_for_clean_project() { } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(&runtime, "coding-finish-compact").await { - complete_agent_request_by_running_locally(&runtime, "coding-finish-compact", req).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "finish_coding_task summary_only did not finish after read-only agent requests" - ); + service_agent_task_until_finished( + &runtime, + "coding-finish-compact", + &task, + "finish_coding_task summary_only", + ) + .await; let result = task.await.unwrap(); assert!(result.success, "{:?}", result.error); @@ -1701,20 +1679,13 @@ async fn finish_coding_task_summary_only_includes_review_evidence_for_docs_only_ .await } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(&runtime, "coding-finish-docs").await { - complete_agent_request_by_running_locally(&runtime, "coding-finish-docs", req).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "finish_coding_task summary_only did not finish after read-only agent requests" - ); + service_agent_task_until_finished( + &runtime, + "coding-finish-docs", + &task, + "finish_coding_task summary_only", + ) + .await; let result = task.await.unwrap(); assert!(result.success, "{:?}", result.error); @@ -1799,9 +1770,7 @@ async fn finish_coding_task_summary_only_treats_dirty_workspace_as_advisory() { .await } }); - let req = next_patch_agent_request(&runtime, "coding-finish-dirty") - .await - .expect("finish_coding_task should inspect changes"); + let req = wait_for_patch_agent_request(&runtime, "coding-finish-dirty").await; complete_agent_request_by_running_locally(&runtime, "coding-finish-dirty", req).await; let result = task.await.unwrap(); @@ -1882,21 +1851,13 @@ async fn finish_coding_task_does_not_resolve_a_different_validation_identity() { .await } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(&runtime, "coding-finish-resolved").await { - complete_agent_request_by_running_locally(&runtime, "coding-finish-resolved", req) - .await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "finish_coding_task summary_only did not finish after read-only agent requests" - ); + service_agent_task_until_finished( + &runtime, + "coding-finish-resolved", + &task, + "finish_coding_task summary_only", + ) + .await; let result = task.await.unwrap(); assert!(result.success, "{:?}", result.error); @@ -3223,20 +3184,8 @@ async fn finish_coding_task_with_agent( .await } }); - for _ in 0..200 { - if task.is_finished() { - break; - } - if let Some(req) = next_patch_agent_request(runtime, client_id).await { - complete_agent_request_by_running_locally(runtime, client_id, req).await; - } else { - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - } - assert!( - task.is_finished(), - "finish_coding_task summary_only did not finish after read-only agent requests" - ); + service_agent_task_until_finished(runtime, client_id, &task, "finish_coding_task summary_only") + .await; task.await.unwrap() } @@ -3323,9 +3272,7 @@ async fn start_coding_task_standard_omits_repeated_manifest_and_recommended_flow .await } }); - let request = next_patch_agent_request(&runtime, "coding-flow-full") - .await - .expect("standard startup should inspect workspace state"); + let request = wait_for_patch_agent_request(&runtime, "coding-flow-full").await; complete_agent_request_by_running_locally(&runtime, "coding-flow-full", request).await; let result = task.await.unwrap(); assert!(result.success, "{:?}", result.error); diff --git a/src/tool_runtime/tests/coding_task_semantic_navigation.rs b/src/tool_runtime/tests/coding_task_semantic_navigation.rs index d26501a8..705dba13 100644 --- a/src/tool_runtime/tests/coding_task_semantic_navigation.rs +++ b/src/tool_runtime/tests/coding_task_semantic_navigation.rs @@ -78,15 +78,7 @@ async fn next_semantic_status_request( runtime: &ToolRuntime, client_id: &str, ) -> crate::shell_protocol::ShellAgentShellRequest { - let mut request = None; - for _ in 0..200 { - request = next_patch_agent_request(runtime, client_id).await; - if request.is_some() { - break; - } - tokio::time::sleep(Duration::from_millis(2)).await; - } - let request = request.expect("semantic navigation status request"); + let request = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(request.kind, AGENT_LSP_REQUEST_KIND); assert!(request.command.is_empty()); assert!(request.stdin.is_none()); @@ -171,20 +163,18 @@ async fn finish_start_servicing_locally( client_id: &str, task: tokio::task::JoinHandle, ) -> ToolResult { - for _ in 0..400 { - if task.is_finished() { - break; - } - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + let deadline = Instant::now() + Duration::from_secs(10); + while !task.is_finished() { + assert!( + Instant::now() < deadline, + "start_coding_task did not finish within the 10-second test deadline" + ); + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { complete_agent_request_by_running_locally(runtime, client_id, request).await; } else { tokio::time::sleep(Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "start_coding_task did not finish after servicing startup agent requests" - ); task.await.unwrap() } @@ -308,11 +298,13 @@ async fn coding_task_semantic_navigation_legacy_agent_is_not_enqueued() { let project = register_semantic_agent(&runtime, "legacy-agent", "demo", temp.path(), false).await; let task = spawn_start(&runtime, project, SessionMode::Normal); - for _ in 0..400 { - if task.is_finished() { - break; - } - if let Some(request) = next_patch_agent_request(&runtime, "legacy-agent").await { + let deadline = Instant::now() + Duration::from_secs(10); + while !task.is_finished() { + assert!( + Instant::now() < deadline, + "legacy-agent startup did not finish within the 10-second test deadline" + ); + if let Some(request) = probe_patch_agent_request(&runtime, "legacy-agent").await { assert_ne!( request.kind, AGENT_LSP_REQUEST_KIND, "legacy agent must not receive an LSP probe" @@ -329,7 +321,7 @@ async fn coding_task_semantic_navigation_legacy_agent_is_not_enqueued() { assert_eq!(semantic["reason_code"], "lsp_capability_not_advertised"); assert_eq!(semantic["available"], false); assert_eq!(semantic["capability"], Value::Null); - assert!(next_patch_agent_request(&runtime, "legacy-agent") + assert!(probe_patch_agent_request(&runtime, "legacy-agent") .await .is_none()); } @@ -374,7 +366,7 @@ async fn coding_task_semantic_navigation_disconnected_agent_is_nonblocking() { warning_kinds.contains(&"git_unavailable"), "{warning_kinds:?}" ); - assert!(next_patch_agent_request(&runtime, "offline-agent") + assert!(probe_patch_agent_request(&runtime, "offline-agent") .await .is_none()); } diff --git a/src/tool_runtime/tests/continuation_feedback.rs b/src/tool_runtime/tests/continuation_feedback.rs index 131150f2..e4b05089 100644 --- a/src/tool_runtime/tests/continuation_feedback.rs +++ b/src/tool_runtime/tests/continuation_feedback.rs @@ -2116,7 +2116,7 @@ async fn validation_summary_surfaces_validation_delta_without_shell_or_new_event "validation_summary appended events" ); assert!( - next_patch_agent_request(&runtime, "vsummary-agent") + probe_patch_agent_request(&runtime, "vsummary-agent") .await .is_none(), "validation_summary enqueued an agent request" @@ -2223,7 +2223,7 @@ async fn finish_coding_task_continuation_matches_handoff_attempt_without_rerunni "finish_coding_task must not re-run validation" ); assert!( - next_patch_agent_request(&runtime, "finish-agent") + probe_patch_agent_request(&runtime, "finish-agent") .await .is_none(), "finish_coding_task enqueued an agent request" diff --git a/src/tool_runtime/tests/dispatch.rs b/src/tool_runtime/tests/dispatch.rs index a5725fda..d7fc1e81 100644 --- a/src/tool_runtime/tests/dispatch.rs +++ b/src/tool_runtime/tests/dispatch.rs @@ -5,8 +5,7 @@ use super::super::patch::*; use super::super::*; use super::support::*; use crate::shell_protocol::{ - ShellAgentPollRequest, ShellAgentResultRequest, ShellClientCapabilities, - ShellClientRegisterRequest, + ShellAgentResultRequest, ShellClientCapabilities, ShellClientRegisterRequest, }; use serde_json::json; @@ -144,9 +143,7 @@ async fn agent_run_shell_resolves_relative_cwd_from_registered_project_root() { .await } }); - let request = next_patch_agent_request(&runtime, "cwd-agent") - .await - .expect("run_shell should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "cwd-agent").await; assert_eq!( request.cwd.as_deref(), Some(expected.to_string_lossy().as_ref()) @@ -166,7 +163,7 @@ async fn agent_run_shell_resolves_relative_cwd_from_registered_project_root() { assert_eq!(result.output["failure_kind"], "permission_denied"); } assert!( - next_patch_agent_request(&runtime, "cwd-agent") + probe_patch_agent_request(&runtime, "cwd-agent") .await .is_none(), "unsafe cwd must be rejected before Agent enqueue" @@ -188,9 +185,7 @@ async fn cargo_check_failure_includes_stderr_tail_or_guidance() { .cargo_check(project, None, None, None, None, None, None, Some(60)) .await }); - let req = next_patch_agent_request(&runtime, "cargo-checker") - .await - .expect("cargo_check should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-checker").await; assert_eq!(req.command, "cargo check --all-targets"); complete_patch_agent_request( &runtime, @@ -244,9 +239,7 @@ async fn cargo_test_failure_includes_stderr_tail_or_guidance() { ) .await }); - let req = next_patch_agent_request(&runtime, "cargo-tester") - .await - .expect("cargo_test should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-tester").await; assert_eq!(req.command, "cargo test 'failing'"); complete_patch_agent_request( &runtime, @@ -297,9 +290,7 @@ async fn cargo_test_output_includes_bounded_failed_test_diagnostics() { ) .await }); - let req = next_patch_agent_request(&runtime, "cargo-diag") - .await - .expect("cargo_test should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-diag").await; assert_eq!(req.command, "cargo test 'multi_fail'"); complete_patch_agent_request( &runtime, @@ -399,9 +390,7 @@ async fn cargo_test_passing_output_includes_empty_failed_test_details_diagnostic ) .await }); - let req = next_patch_agent_request(&runtime, "cargo-pass-diag") - .await - .expect("cargo_test should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-pass-diag").await; complete_patch_agent_request( &runtime, "cargo-pass-diag", @@ -456,9 +445,7 @@ async fn cargo_test_multi_harness_counts_match_diagnostics_summary() { ) .await }); - let req = next_patch_agent_request(&runtime, "cargo-multi-harness") - .await - .expect("cargo_test should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-multi-harness").await; complete_patch_agent_request( &runtime, "cargo-multi-harness", @@ -520,9 +507,7 @@ async fn cargo_test_agent_timeout_is_not_validation_failed() { ) .await }); - let req = next_patch_agent_request(&runtime, "cargo-timeout") - .await - .expect("cargo_test should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-timeout").await; assert_eq!(req.command, "cargo test 'slow'"); runtime .shell_clients @@ -562,9 +547,7 @@ async fn cargo_fmt_failure_includes_stderr_tail_or_guidance() { .cargo_fmt(project, None, Some(true), Some(60)) .await }); - let req = next_patch_agent_request(&runtime, "cargo-formatter") - .await - .expect("cargo_fmt should enqueue a cargo command"); + let req = wait_for_patch_agent_request(&runtime, "cargo-formatter").await; assert_eq!(req.command, "cargo fmt -- --check"); complete_patch_agent_request( &runtime, @@ -630,23 +613,7 @@ new file mode 100644\n\ let apply_task = tokio::spawn(async move { runtime_for_task.apply_patch(project, patch).await }); - let mut check_req = None; - for _ in 0..10 { - check_req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "patcher".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if check_req.is_some() { - break; - } - tokio::task::yield_now().await; - } - let check_req = check_req.expect("apply_patch should enqueue git apply --check for the agent"); + let check_req = wait_for_agent_request_for_instance(&runtime, "patcher", "inst").await; assert_eq!(check_req.command, "git apply --check -"); assert!(check_req .stdin @@ -668,23 +635,7 @@ new file mode 100644\n\ .await .unwrap(); - let mut apply_req = None; - for _ in 0..10 { - apply_req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "patcher".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if apply_req.is_some() { - break; - } - tokio::task::yield_now().await; - } - let apply_req = apply_req.expect("apply_patch should enqueue git apply for the agent"); + let apply_req = wait_for_agent_request_for_instance(&runtime, "patcher", "inst").await; assert_eq!(apply_req.command, "git apply -"); assert!(apply_req .stdin @@ -734,9 +685,7 @@ async fn apply_patch_agent_command_excludes_patch_content_and_uses_stdin_and_cwd tokio::spawn(async move { runtime_for_task.apply_patch(project, patch_for_apply).await }); // 1) preflight check: `git apply --check -` - let check_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("apply_patch should enqueue a git apply --check request"); + let check_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&check_req.command, marker); assert_eq!(check_req.command, "git apply --check -"); assert_eq!(check_req.stdin.as_deref(), Some(patch.as_str())); @@ -744,9 +693,7 @@ async fn apply_patch_agent_command_excludes_patch_content_and_uses_stdin_and_cwd complete_patch_agent_request(&runtime, "patcher", &check_req.request_id, 0, "OK\n", "").await; // 2) apply: `git apply -` - let apply_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("apply_patch should enqueue a git apply request"); + let apply_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&apply_req.command, marker); assert_eq!(apply_req.command, "git apply -"); assert_eq!(apply_req.stdin.as_deref(), Some(patch.as_str())); @@ -794,24 +741,20 @@ async fn apply_patch_checked_does_not_apply_when_check_fails() { }); // 1) validate preflight check: fails (exit 1) -> can_apply=false. - let check_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("apply_patch_checked should enqueue a validate check request"); + let check_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&check_req.command, marker); assert_eq!(check_req.command, "git apply --check -"); assert_eq!(check_req.stdin.as_deref(), Some(patch.as_str())); complete_patch_agent_request(&runtime, "patcher", &check_req.request_id, 1, "", "bad").await; // 2) validate stat summary still runs (read-only, regardless of can_apply). - let stat_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("validate_patch should enqueue a git apply --stat request"); + let stat_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&stat_req.command, marker); assert_eq!(stat_req.command, "git apply --stat -"); complete_patch_agent_request(&runtime, "patcher", &stat_req.request_id, 0, "stat", "").await; // 3) No apply step must be enqueued because the preflight failed. - let leaked_apply = next_patch_agent_request(&runtime, "patcher").await; + let leaked_apply = probe_patch_agent_request(&runtime, "patcher").await; assert!( leaked_apply.is_none(), "apply_patch_checked must not apply when the check fails (got: {:?})", @@ -853,24 +796,18 @@ async fn apply_patch_checked_applies_large_patch_over_command_limit_via_stdin() }); // 1) validate check. - let check_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("validate check request"); + let check_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&check_req.command, marker); assert_eq!(check_req.stdin.as_deref(), Some(patch.as_str())); complete_patch_agent_request(&runtime, "patcher", &check_req.request_id, 0, "", "").await; // 2) validate stat. - let stat_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("validate stat request"); + let stat_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&stat_req.command, marker); complete_patch_agent_request(&runtime, "patcher", &stat_req.request_id, 0, "stat", "").await; // 3) apply preflight check. - let apply_check_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("apply check request"); + let apply_check_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&apply_check_req.command, marker); assert_eq!(apply_check_req.command, "git apply --check -"); assert_eq!(apply_check_req.stdin.as_deref(), Some(patch.as_str())); @@ -885,16 +822,14 @@ async fn apply_patch_checked_applies_large_patch_over_command_limit_via_stdin() .await; // 4) apply. - let apply_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("apply request"); + let apply_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&apply_req.command, marker); assert_eq!(apply_req.command, "git apply -"); assert_eq!(apply_req.stdin.as_deref(), Some(patch.as_str())); complete_patch_agent_request(&runtime, "patcher", &apply_req.request_id, 0, "", "").await; // 5) post-apply git_diff_summary (drain + complete generically). - if let Some(diff_req) = next_patch_agent_request(&runtime, "patcher").await { + if let Some(diff_req) = probe_patch_agent_request(&runtime, "patcher").await { complete_patch_agent_request(&runtime, "patcher", &diff_req.request_id, 0, "", "").await; } @@ -1054,25 +989,9 @@ async fn register_project_crosses_historical_64_threshold_and_is_immediately_res ) .await }); - let mut request = None; - for _ in 0..20 { - request = next_agent_request_for_client(&runtime, client_id).await; - if request.is_some() { - break; - } - tokio::task::yield_now().await; - } - let request = match request { - Some(request) => request, - None => { - assert!( - task.is_finished(), - "register_project task neither enqueued nor completed" - ); - let early = task.await.unwrap(); - panic!("register_project completed before enqueue: {early:?}"); - } - }; + let request = + wait_for_agent_request_for_instance(&runtime, client_id, &format!("inst-{client_id}")) + .await; assert_eq!(request.kind, "register_project"); let authoritative = json!({ "outcome": "registered", @@ -1115,7 +1034,7 @@ async fn register_project_crosses_historical_64_threshold_and_is_immediately_res authoritative["revision"].as_str().map(str::to_string) ); assert!( - next_agent_request_for_client(&runtime, client_id) + probe_agent_request_for_client(&runtime, client_id) .await .is_none(), "successful projection must not replay the mutation" @@ -1152,15 +1071,9 @@ async fn register_project_projection_failure_returns_reconcile_required_without_ ) .await }); - let mut request = None; - for _ in 0..20 { - request = next_agent_request_for_client(&runtime, client_id).await; - if request.is_some() { - break; - } - tokio::task::yield_now().await; - } - let request = request.expect("register_project should enqueue one mutation"); + let request = + wait_for_agent_request_for_instance(&runtime, client_id, &format!("inst-{client_id}")) + .await; assert_eq!(request.kind, "register_project"); let authoritative = json!({ "outcome": "registered", @@ -1216,7 +1129,7 @@ async fn register_project_projection_failure_returns_reconcile_required_without_ "failed Server projection must not falsely advertise routing" ); assert!( - next_agent_request_for_client(&runtime, client_id) + probe_agent_request_for_client(&runtime, client_id) .await .is_none(), "uncertain post-persist state must not blind-retry register_project" @@ -1305,19 +1218,7 @@ async fn dispatch_unregister_project_removes_server_inventory_after_terminal_run .await } }); - let request = loop { - if let Some(request) = next_agent_request_for_client(&runtime, client_id).await { - break request; - } - if task.is_finished() { - let result = task.await.unwrap(); - panic!( - "unregister returned before dispatching a lifecycle request: success={} error={:?} output={}", - result.success, result.error, result.output - ); - } - tokio::task::yield_now().await; - }; + let request = wait_for_agent_request_for_client(&runtime, client_id).await; assert_eq!(request.kind, "project_lifecycle_unregister"); let payload: serde_json::Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); @@ -1500,9 +1401,7 @@ async fn mutating_dispatch_feeds_the_activity_recorder() { .await } }); - let request = next_patch_agent_request(&runtime, "activity-shell") - .await - .expect("run_shell should enqueue a request"); + let request = wait_for_patch_agent_request(&runtime, "activity-shell").await; complete_patch_agent_request(&runtime, "activity-shell", &request.request_id, 0, "ok", "") .await; let shell = shell_task.await.unwrap(); @@ -1531,9 +1430,7 @@ async fn mutating_dispatch_feeds_the_activity_recorder() { .await } }); - let request = next_patch_agent_request(&runtime, "activity-shell") - .await - .expect("short project id should enqueue on the resolved client"); + let request = wait_for_patch_agent_request(&runtime, "activity-shell").await; complete_patch_agent_request(&runtime, "activity-shell", &request.request_id, 0, "ok", "") .await; let alias = alias_task.await.unwrap(); diff --git a/src/tool_runtime/tests/execution_context.rs b/src/tool_runtime/tests/execution_context.rs index 9d043822..55fa3a8a 100644 --- a/src/tool_runtime/tests/execution_context.rs +++ b/src/tool_runtime/tests/execution_context.rs @@ -70,9 +70,7 @@ async fn run_shell_inherits_session_context_and_explicit_arguments_override_it() .await } }); - let inherited_request = next_patch_agent_request(&runtime, "context-shell") - .await - .expect("inherited run_shell should enqueue"); + let inherited_request = wait_for_patch_agent_request(&runtime, "context-shell").await; assert_eq!( inherited_request.cwd.as_deref(), Some(frontend.to_string_lossy().as_ref()) @@ -114,9 +112,7 @@ async fn run_shell_inherits_session_context_and_explicit_arguments_override_it() .await } }); - let override_request = next_patch_agent_request(&runtime, "context-shell") - .await - .expect("overridden run_shell should enqueue"); + let override_request = wait_for_patch_agent_request(&runtime, "context-shell").await; assert_eq!( override_request.cwd.as_deref(), Some(override_dir.to_string_lossy().as_ref()) @@ -157,9 +153,7 @@ async fn run_shell_inherits_session_context_and_explicit_arguments_override_it() .await } }); - let no_session_request = next_patch_agent_request(&runtime, "context-shell") - .await - .expect("sessionless run_shell should enqueue"); + let no_session_request = wait_for_patch_agent_request(&runtime, "context-shell").await; assert_eq!( no_session_request.cwd.as_deref(), Some(root.to_string_lossy().as_ref()) @@ -231,9 +225,7 @@ async fn run_job_inherits_session_cwd_and_shell() { assert!(result.success, "{:?}", result.error); assert_eq!(result.output["cwd"], "frontend"); assert_eq!(result.output["shell"], "bash"); - let request = next_agent_request_for_client(&runtime, "context-job") - .await - .expect("run_job should enqueue a start_job request"); + let request = wait_for_agent_request_for_client(&runtime, "context-job").await; assert_eq!(request.kind, "start_job"); assert_eq!( request.cwd.as_deref(), @@ -297,9 +289,7 @@ async fn session_ssh_resource_uses_remote_cwd_and_safe_agent_context_for_shell_a .await } }); - let shell_request = next_agent_request_for_client(&runtime, "context-ssh") - .await - .expect("SSH shell should enqueue"); + let shell_request = wait_for_agent_request_for_client(&runtime, "context-ssh").await; assert_eq!(shell_request.cwd.as_deref(), Some("/remote/override")); let shell_context = shell_request .job_context @@ -344,9 +334,7 @@ async fn session_ssh_resource_uses_remote_cwd_and_safe_agent_context_for_shell_a assert_eq!(job.output["ssh_resource"], "tmp"); assert_eq!(job.output["cwd"], "/remote/default"); let job_id = job.output["job_id"].as_str().unwrap().to_string(); - let job_request = next_agent_request_for_client(&runtime, "context-ssh") - .await - .expect("SSH job should enqueue"); + let job_request = wait_for_agent_request_for_client(&runtime, "context-ssh").await; assert_eq!(job_request.kind, "start_job"); assert_eq!(job_request.cwd.as_deref(), Some("/remote/default")); assert_eq!( @@ -433,7 +421,7 @@ async fn session_ssh_resource_rejects_structured_cargo_before_legacy_sync_start( .as_deref() .is_some_and(|error| error.contains("ssh_resource_unsupported_for_request"))); assert!( - next_agent_request_for_client(&runtime, "context-ssh-cargo") + probe_agent_request_for_client(&runtime, "context-ssh-cargo") .await .is_none(), "structured Cargo rejection must happen before the legacy sync command starts" @@ -496,7 +484,7 @@ async fn session_ssh_resource_rejects_mutating_cargo_fmt_before_start() { .as_deref() .is_some_and(|error| error.contains("ssh_resource_unsupported_for_request"))); assert!( - next_agent_request_for_client(&runtime, "context-ssh-cargo-fmt") + probe_agent_request_for_client(&runtime, "context-ssh-cargo-fmt") .await .is_none(), "mutating cargo fmt rejection must happen before an Agent shell request starts" @@ -552,7 +540,7 @@ async fn session_ssh_resource_requires_runner_ssh_shell_capability() { .as_deref() .is_some_and(|error| error.contains("agent_capability_unavailable"))); assert!( - next_agent_request_for_client(&runtime, "context-ssh-legacy") + probe_agent_request_for_client(&runtime, "context-ssh-legacy") .await .is_none(), "an old Runner must not receive an SSH resource request" @@ -609,9 +597,7 @@ async fn session_ssh_transport_failure_marks_remote_delivery_uncertain() { .await } }); - let request = next_agent_request_for_client(&runtime, "context-ssh-transport") - .await - .expect("SSH shell should enqueue"); + let request = wait_for_agent_request_for_client(&runtime, "context-ssh-transport").await; runtime .shell_clients .complete(ShellAgentResultPayload { @@ -688,7 +674,7 @@ async fn mismatch_and_invalid_context_fail_closed_without_root_fallback() { assert!(!mismatch.success); assert_eq!(mismatch.output["failure_kind"], "session_project_mismatch"); assert!( - next_patch_agent_request(&runtime, "context-second") + probe_patch_agent_request(&runtime, "context-second") .await .is_none(), "mismatched Session must not enqueue with inherited context" @@ -752,9 +738,7 @@ async fn nonexistent_inherited_cwd_is_not_retried_at_project_root() { .await } }); - let request = next_patch_agent_request(&runtime, "context-missing") - .await - .expect("run_shell should dispatch the inherited cwd once"); + let request = wait_for_patch_agent_request(&runtime, "context-missing").await; assert_eq!( request.cwd.as_deref(), Some(root.join("missing").to_string_lossy().as_ref()) @@ -781,7 +765,7 @@ async fn nonexistent_inherited_cwd_is_not_retried_at_project_root() { .unwrap_or_default() .contains("cwd does not exist")); assert!( - next_patch_agent_request(&runtime, "context-missing") + probe_patch_agent_request(&runtime, "context-missing") .await .is_none(), "invalid inherited cwd must not fall back to the project root" diff --git a/src/tool_runtime/tests/explicit_resume.rs b/src/tool_runtime/tests/explicit_resume.rs index dca5a3b0..e82e0bd7 100644 --- a/src/tool_runtime/tests/explicit_resume.rs +++ b/src/tool_runtime/tests/explicit_resume.rs @@ -7,6 +7,7 @@ use crate::tool_runtime::sessions::{SessionEvent, SessionGuards, SessionTranspor use crate::tool_runtime::{SessionMode, StartupDetail, ToolCall, ToolResult, ToolRuntime}; use serde_json::{json, Value}; use std::path::Path; +use std::time::{Duration, Instant}; fn coding_call( project: &str, @@ -75,10 +76,12 @@ async fn dispatch_start_coding_task_without_window( } }); - for _ in 0..5_000 { - if task.is_finished() { - break; - } + let deadline = Instant::now() + Duration::from_secs(10); + while !task.is_finished() { + assert!( + Instant::now() < deadline, + "start_coding_task did not finish within the 10-second test deadline" + ); if let Some(request) = runtime .shell_clients .poll(crate::shell_protocol::ShellAgentPollRequest { @@ -100,10 +103,9 @@ async fn dispatch_start_coding_task_without_window( ) .await; } else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(Duration::from_millis(2)).await; } } - assert!(task.is_finished(), "start_coding_task did not finish"); task.await.unwrap() } diff --git a/src/tool_runtime/tests/files.rs b/src/tool_runtime/tests/files.rs index 22ce1278..028081f6 100644 --- a/src/tool_runtime/tests/files.rs +++ b/src/tool_runtime/tests/files.rs @@ -293,9 +293,8 @@ async fn delete_project_files_replaced_agent_keeps_legacy_shell_fallback() { } }); - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-replaced", "inst-b") - .await - .expect("replaced agent should receive the legacy shell request"); + let req = + wait_for_agent_request_for_instance(&runtime, "cleanup-delete-replaced", "inst-b").await; assert_eq!(req.kind, "run_shell"); assert!(req.command.contains("rm -f --")); complete_patch_agent_request_for_instance( @@ -310,7 +309,7 @@ async fn delete_project_files_replaced_agent_keeps_legacy_shell_fallback() { .await; assert!(task.await.unwrap().success); let extra = - next_agent_request_for_instance(&runtime, "cleanup-delete-replaced", "inst-b").await; + probe_agent_request_for_instance(&runtime, "cleanup-delete-replaced", "inst-b").await; assert!( extra.is_none(), "exactly one legacy request may be emitted: {extra:?}" @@ -372,9 +371,7 @@ async fn delete_project_files_capability_revoked_before_enqueue_falls_back_to_le // The only request the Runner receives is the legacy shell fallback: no // structured request may be queued for a client that no longer advertises // the capability, and no duplicate structured + legacy pair may appear. - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-fence", "inst-b") - .await - .expect("replaced agent should receive exactly one legacy shell request"); + let req = wait_for_agent_request_for_instance(&runtime, "cleanup-delete-fence", "inst-b").await; assert_eq!(req.kind, "run_shell"); assert!(req.command.contains("rm -f --")); complete_patch_agent_request_for_instance( @@ -388,7 +385,7 @@ async fn delete_project_files_capability_revoked_before_enqueue_falls_back_to_le ) .await; assert!(task.await.unwrap().success); - let extra = next_agent_request_for_instance(&runtime, "cleanup-delete-fence", "inst-b").await; + let extra = probe_agent_request_for_instance(&runtime, "cleanup-delete-fence", "inst-b").await; assert!( extra.is_none(), "no duplicate structured + legacy request may be emitted: {extra:?}" @@ -477,7 +474,7 @@ async fn delete_project_files_replacement_before_poll_reports_not_started() { // No legacy fallback and no inherited structured request for the // replacement Runner. let extra = - next_agent_request_for_instance(&runtime, "cleanup-delete-replace-early", "inst-b").await; + probe_agent_request_for_instance(&runtime, "cleanup-delete-replace-early", "inst-b").await; assert!( extra.is_none(), "replacement Runner must receive no request: {extra:?}" @@ -510,9 +507,8 @@ async fn delete_project_files_replacement_after_poll_reports_outcome_unknown() { wait_for_pending_requests(&runtime, "cleanup-delete-replace-late", 1).await; // Dispatch the structured request to the original instance. - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-replace-late", "inst") - .await - .expect("structured delete should be polled by the original instance"); + let req = + wait_for_agent_request_for_instance(&runtime, "cleanup-delete-replace-late", "inst").await; assert_eq!(req.kind, "file_delete_project_files"); // Replace the Runner before it returns its result. runtime @@ -570,7 +566,7 @@ async fn delete_project_files_replacement_after_poll_reports_outcome_unknown() { "replacement must not complete the replaced request: {err}" ); let extra = - next_agent_request_for_instance(&runtime, "cleanup-delete-replace-late", "inst-b").await; + probe_agent_request_for_instance(&runtime, "cleanup-delete-replace-late", "inst-b").await; assert!( extra.is_none(), "replacement Runner must receive no inherited request: {extra:?}" @@ -661,9 +657,8 @@ async fn delete_project_files_timeout_after_dispatch_reports_outcome_unknown() { wait_for_pending_requests(&runtime, "cleanup-delete-timeout-late", 1).await; // Dispatch the structured request; the Runner never returns a result, so // the wait timeout fires after dispatch may have started deleting. - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-timeout-late", "inst") - .await - .expect("structured delete should be polled"); + let req = + wait_for_agent_request_for_instance(&runtime, "cleanup-delete-timeout-late", "inst").await; assert_eq!(req.kind, "file_delete_project_files"); let result = task.await.unwrap(); @@ -722,9 +717,7 @@ async fn delete_project_files_waiter_dropped_without_undispatch_proof_reports_ou // cancellation API: remove the pending record (dropping the oneshot // sender) without resolving it, so the tool's receiver observes the // channel close. The registry returns the preserved dispatch truth. - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-waiter", "inst") - .await - .expect("structured delete should be polled"); + let req = wait_for_agent_request_for_instance(&runtime, "cleanup-delete-waiter", "inst").await; let dispatch = runtime .shell_clients .cancel_request_dispatch_state(&req.request_id) @@ -783,9 +776,8 @@ async fn delete_project_files_terminal_failure_reports_outcome_unknown() { // The Runner returns a definitive terminal failure after dispatch // (non-zero exit). The mutation may already have deleted files, so the // failure must never collapse into an ordinary retry-safe error. - let req = next_agent_request_for_instance(&runtime, "cleanup-delete-terminal", "inst") - .await - .expect("structured delete should be polled"); + let req = + wait_for_agent_request_for_instance(&runtime, "cleanup-delete-terminal", "inst").await; complete_patch_agent_request_for_instance( &runtime, "cleanup-delete-terminal", @@ -808,7 +800,7 @@ async fn delete_project_files_terminal_failure_reports_outcome_unknown() { "error was: {error}" ); // No automatic legacy fallback follows the uncertain mutation. - let extra = next_agent_request_for_instance(&runtime, "cleanup-delete-terminal", "inst").await; + let extra = probe_agent_request_for_instance(&runtime, "cleanup-delete-terminal", "inst").await; assert!( extra.is_none(), "no legacy fallback may follow an uncertain structured delete: {extra:?}" @@ -1105,7 +1097,7 @@ async fn artifact_upload_begin_policy_rejection_is_classified() { assert!(error.contains(".artifact"), "{error}"); assert!(error.contains("artifacts/smoke/.artifact"), "{error}"); assert!( - next_patch_agent_request(&runtime, "artifact-policy-session") + probe_patch_agent_request(&runtime, "artifact-policy-session") .await .is_none(), "policy rejection must happen before enqueue" @@ -1157,7 +1149,7 @@ async fn validate_patch_never_enqueues_mutating_apply_command() { complete_patch_agent_request(&runtime, "patcher", &stat_req.request_id, 0, "stat", "").await; // 3) No mutating apply must be enqueued — validate_patch is dry-run only. - let leaked_apply = next_patch_agent_request(&runtime, "patcher").await; + let leaked_apply = probe_patch_agent_request(&runtime, "patcher").await; assert!( leaked_apply.is_none(), "validate_patch enqueued a mutating command (got: {:?})", @@ -3515,7 +3507,7 @@ async fn file_read_project_tools_require_file_read_capability() { result.error ); } - assert!(next_patch_agent_request(&runtime, "file-read-capability") + assert!(probe_patch_agent_request(&runtime, "file-read-capability") .await .is_none()); } @@ -3755,7 +3747,7 @@ async fn project_read_adapters_reject_out_of_project_paths_before_agent_dispatch assert_eq!(result.output["field"], field, "{case}"); } assert!( - next_patch_agent_request(&runtime, "path-boundary") + probe_patch_agent_request(&runtime, "path-boundary") .await .is_none(), "{case} must reject before Agent dispatch" @@ -4452,7 +4444,7 @@ async fn read_file_refuses_secret_paths_before_reaching_agent() { } assert!( - next_patch_agent_request(&runtime, "secret-read") + probe_patch_agent_request(&runtime, "secret-read") .await .is_none(), "a refused secret path still reached the agent" diff --git a/src/tool_runtime/tests/files_helpers.rs b/src/tool_runtime/tests/files_helpers.rs index 8e4c6da6..caeabba8 100644 --- a/src/tool_runtime/tests/files_helpers.rs +++ b/src/tool_runtime/tests/files_helpers.rs @@ -43,9 +43,7 @@ async fn save_project_artifact_routes_to_agent_file_op() { } }); - let req = next_patch_agent_request(&runtime, "artifact-save") - .await - .expect("save_project_artifact should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-save").await; assert_eq!(req.kind, "file_save_project_artifact"); assert!(req.command.is_empty()); assert!(req.stdin.is_none()); @@ -97,9 +95,7 @@ async fn read_project_artifact_metadata_routes_to_agent_file_op() { } }); - let req = next_patch_agent_request(&runtime, "artifact-meta") - .await - .expect("read_project_artifact_metadata should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-meta").await; assert_eq!(req.kind, "file_read_project_artifact_metadata"); assert!(req.command.is_empty()); assert!(req.stdin.is_none()); @@ -154,9 +150,7 @@ async fn read_project_artifact_metadata_allow_missing_routes_to_agent_file_op() } }); - let req = next_patch_agent_request(&runtime, "artifact-meta-missing") - .await - .expect("read_project_artifact_metadata should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-meta-missing").await; let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); assert_eq!(payload["allow_missing"], true); @@ -209,9 +203,7 @@ async fn read_project_artifact_routes_to_agent_file_op() { } }); - let req = next_patch_agent_request(&runtime, "artifact-read") - .await - .expect("read_project_artifact should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-read").await; assert_eq!(req.kind, "file_read_project_artifact"); assert!(req.command.is_empty()); assert!(req.stdin.is_none()); @@ -275,9 +267,7 @@ async fn read_project_artifact_mcp_image_routes_complete_bounded_remote_read() { } }); - let req = next_patch_agent_request(&runtime, "artifact-image") - .await - .expect("MCP image read should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-image").await; assert_eq!(req.kind, "file_read_project_artifact"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); @@ -345,9 +335,7 @@ async fn read_project_artifact_mcp_image_rejects_untrusted_remote_mime() { .await } }); - let req = next_patch_agent_request(&runtime, "artifact-image-mime") - .await - .expect("MCP image read should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-image-mime").await; let pdf = b"%PDF-1.7\n"; let stdout = json!({ "path": "docs/images/spoofed.png", @@ -446,9 +434,7 @@ async fn artifact_upload_tools_route_to_agent_file_ops() { .await } }); - let req = next_patch_agent_request(&runtime, "artifact-upload") - .await - .expect("artifact_upload_begin should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-upload").await; assert_eq!(req.kind, "file_artifact_upload_begin"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); @@ -490,9 +476,7 @@ async fn artifact_upload_tools_route_to_agent_file_ops() { .await } }); - let req = next_patch_agent_request(&runtime, "artifact-upload") - .await - .expect("artifact_upload_chunk should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-upload").await; assert_eq!(req.kind, "file_artifact_upload_chunk"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); @@ -527,9 +511,7 @@ async fn artifact_upload_tools_route_to_agent_file_ops() { .await } }); - let req = next_patch_agent_request(&runtime, "artifact-upload") - .await - .expect("artifact_upload_finish should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-upload").await; assert_eq!(req.kind, "file_artifact_upload_finish"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); @@ -560,9 +542,7 @@ async fn artifact_upload_tools_route_to_agent_file_ops() { .await } }); - let req = next_patch_agent_request(&runtime, "artifact-upload") - .await - .expect("artifact_upload_abort should enqueue an artifact file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-upload").await; assert_eq!(req.kind, "file_artifact_upload_abort"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("artifact payload")).unwrap(); diff --git a/src/tool_runtime/tests/git.rs b/src/tool_runtime/tests/git.rs index 3eba70f3..0a675f9f 100644 --- a/src/tool_runtime/tests/git.rs +++ b/src/tool_runtime/tests/git.rs @@ -252,7 +252,7 @@ async fn git_path_mutation_capability_preflight_matches_structured_process_runti assert!(error.contains("structured_process_argv"), "{error}"); assert!(error.contains("no shell fallback"), "{error}"); assert!( - next_patch_agent_request(&runtime, "discard-shell-only") + probe_patch_agent_request(&runtime, "discard-shell-only") .await .is_none(), "capability preflight must reject before enqueue" @@ -284,7 +284,7 @@ async fn git_restore_missing_structured_process_capability_is_definite_not_start assert!(!result.success); assert_eq!(result.output["execution_state"], "not_started"); assert_eq!(result.output["failure_kind"], "capability_unavailable"); - assert!(next_patch_agent_request(&runtime, "restore-no-argv") + assert!(probe_patch_agent_request(&runtime, "restore-no-argv") .await .is_none()); } @@ -341,7 +341,7 @@ async fn git_restore_stays_sync_on_structured_job_capable_runner() { assert!(result.success, "{:?}", result.error); assert_eq!(result.output["restored_paths"], json!(["safe.txt"])); assert!( - next_patch_agent_request(&runtime, "restore-sync-job-capable") + probe_patch_agent_request(&runtime, "restore-sync-job-capable") .await .is_none() ); @@ -396,7 +396,7 @@ async fn git_restore_replacement_after_dispatch_reports_outcome_unknown_without_ assert!(!result.success); assert_eq!(result.output["execution_state"], "outcome_unknown"); assert_eq!(result.output["failure_kind"], "outcome_unknown"); - let retry = next_agent_request_for_instance(&runtime, "restore-uncertain", "inst-b").await; + let retry = probe_agent_request_for_instance(&runtime, "restore-uncertain", "inst-b").await; assert!( retry.is_none(), "uncertain mutation must not be retried: {retry:?}" @@ -771,7 +771,7 @@ async fn run_agent_git_diff_hunks_committed_page( if task.is_finished() { break; } - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { assert_eq!(request.kind, "run_internal_posix_script"); assert_eq!( request.cwd.as_deref(), @@ -1121,7 +1121,7 @@ async fn git_diff_hunks_never_returns_secret_path_content_in_any_mode() { assert!(!explicit.success); assert_eq!(explicit.output["reason_code"], "sensitive_path"); assert!( - next_patch_agent_request(&runtime, "diff-secret-boundary") + probe_patch_agent_request(&runtime, "diff-secret-boundary") .await .is_none(), "explicit protected path must fail before Runner dispatch" @@ -1176,7 +1176,7 @@ async fn git_diff_hunks_committed_range_validation_and_merge_base_fail_closed() assert!(!result.success); assert_eq!(result.output["reason_code"], reason); assert!( - next_patch_agent_request(&runtime, "committed-validation") + probe_patch_agent_request(&runtime, "committed-validation") .await .is_none(), "invalid committed input must fail before Runner dispatch" @@ -2105,7 +2105,7 @@ async fn git_diff_hunks_scoped_fence_ignores_outside_change_and_rejects_scope_mi project_mismatch.output["reason_code"], "continuation_mismatch" ); - assert!(next_patch_agent_request(&runtime, other_client_id) + assert!(probe_patch_agent_request(&runtime, other_client_id) .await .is_none()); @@ -2121,7 +2121,7 @@ async fn git_diff_hunks_scoped_fence_ignores_outside_change_and_rejects_scope_mi .await; assert!(!mismatch.success); assert_eq!(mismatch.output["reason_code"], "continuation_mismatch"); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); @@ -2133,7 +2133,7 @@ async fn git_diff_hunks_scoped_fence_ignores_outside_change_and_rejects_scope_mi cached_mismatch.output["reason_code"], "continuation_mismatch" ); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -2276,7 +2276,7 @@ async fn git_diff_hunks_malformed_continuation_fails_before_runner_dispatch() { assert_eq!(result.output["reason_code"], "invalid_continuation"); assert_eq!(result.output["files"], json!([])); assert_eq!(result.output["next_continuation"], Value::Null); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -3600,9 +3600,7 @@ async fn show_changes_with_session_id_returns_session_block_and_records_call() { .await } }); - let req = next_agent_request_for_instance(&runtime, "telemetry-show", "inst") - .await - .expect("read_file should enqueue before show_changes"); + let req = wait_for_agent_request_for_instance(&runtime, "telemetry-show", "inst").await; complete_patch_agent_request( &runtime, "telemetry-show", @@ -3683,9 +3681,7 @@ async fn show_changes_accepts_unique_short_id() { .await } }); - let req = next_agent_request_for_client(&runtime, "workstation") - .await - .expect("show_changes should enqueue an agent shell request"); + let req = wait_for_agent_request_for_client(&runtime, "workstation").await; assert_eq!(req.cwd.as_deref(), Some("/root/git/workstation-other-repo")); let stdout = "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nstatus_exit=0\nrepository_probe=inside_worktree\nrepository_probe_exit=0\nfiles_total=0\nfiles_returned=0\nfiles_truncated=0\nfiles_limit=200\nmodified=0\nadded=0\ndeleted=0\nrenamed=0\ncopied=0\nuntracked=0\nconflicted=0\nstaged=0\nunstaged=0\nstatus_trunc_count=0\nstatus_trunc_bytes=0\nstatus_trunc_path=0\nstatus_bytes=7\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ncommit=abc123\nshort=abc123\nsummary=head\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nhead_exit=0\nhead_truncated=0\nhead_bytes=39\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ndiff_stat_exit=0\ndiff_stat_truncated=0\ndiff_stat_bytes=0\n"; runtime @@ -3871,7 +3867,7 @@ async fn run_git_review_summary_via_agent( tokio::time::Instant::now() < deadline, "git_review_summary did not finish within 10 seconds for client {client_id}" ); - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { assert_eq!(request.kind, "run_internal_posix_script"); assert!(request.command.is_empty()); let payload = request @@ -4824,7 +4820,7 @@ async fn run_show_changes_via_agent( tokio::time::Instant::now() < deadline, "show_changes did not finish within 10 seconds for client {client_id}" ); - if let Some(req) = next_patch_agent_request(runtime, client_id).await { + if let Some(req) = probe_patch_agent_request(runtime, client_id).await { assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); let payload = req diff --git a/src/tool_runtime/tests/handoff.rs b/src/tool_runtime/tests/handoff.rs index 195b0db8..9ae10d54 100644 --- a/src/tool_runtime/tests/handoff.rs +++ b/src/tool_runtime/tests/handoff.rs @@ -871,7 +871,7 @@ async fn real_cargo_nonzero_failures_match_validation_failed_expectations() { }); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let req = loop { - if let Some(req) = next_patch_agent_request(&runtime, "cargo-expected-kind").await { + if let Some(req) = probe_patch_agent_request(&runtime, "cargo-expected-kind").await { break req; } if task.is_finished() { @@ -985,7 +985,7 @@ async fn cargo_test_zero_tests_success_is_detected_and_warns_in_handoff() { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let req = loop { - if let Some(req) = next_patch_agent_request(&runtime, "cargo-zero-tests").await { + if let Some(req) = probe_patch_agent_request(&runtime, "cargo-zero-tests").await { break req; } assert!( @@ -3197,7 +3197,7 @@ async fn complete_agent_shell_requests_until_finished( std::time::Instant::now() < deadline, "tool did not finish within 10 seconds after Agent requests for {client_id}" ); - if let Some(req) = next_patch_agent_request(runtime, client_id).await { + if let Some(req) = probe_patch_agent_request(runtime, client_id).await { complete_agent_request_by_running_locally(runtime, client_id, req).await; } else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; diff --git a/src/tool_runtime/tests/handoff_brief.rs b/src/tool_runtime/tests/handoff_brief.rs index d89a606e..e49a5e78 100644 --- a/src/tool_runtime/tests/handoff_brief.rs +++ b/src/tool_runtime/tests/handoff_brief.rs @@ -1104,7 +1104,7 @@ async fn internal_handoff_projection_does_not_append_events_or_enqueue_agent_req result.output["handoff_brief"]["validation"]["status"], "not_requested" ); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); let after = runtime @@ -1236,7 +1236,7 @@ async fn public_handoff_dispatch_records_only_standard_telemetry_and_preserves_g ) .unwrap(); assert_eq!(before_discussion, after_discussion); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert_eq!( @@ -1294,7 +1294,7 @@ async fn finish_and_handoff_surfaces_return_the_same_brief_for_the_same_snapshot std::time::Instant::now() < deadline, "finish_coding_task did not finish within 10 seconds while proving shared handoff brief" ); - if let Some(request) = next_patch_agent_request(&runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(&runtime, client_id).await { complete_agent_request_by_running_locally(&runtime, client_id, request).await; } else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; diff --git a/src/tool_runtime/tests/hygiene.rs b/src/tool_runtime/tests/hygiene.rs index 69525d0c..a34156d1 100644 --- a/src/tool_runtime/tests/hygiene.rs +++ b/src/tool_runtime/tests/hygiene.rs @@ -43,7 +43,7 @@ async fn dispatch_hygiene_with_agent( tokio::time::Instant::now() < deadline, "hygiene check did not finish within 10 seconds for client {client_id}" ); - if let Some(req) = next_patch_agent_request(runtime, client_id).await { + if let Some(req) = probe_patch_agent_request(runtime, client_id).await { assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); let payload = req diff --git a/src/tool_runtime/tests/jobs.rs b/src/tool_runtime/tests/jobs.rs index 888f8f03..ada085e4 100644 --- a/src/tool_runtime/tests/jobs.rs +++ b/src/tool_runtime/tests/jobs.rs @@ -660,7 +660,7 @@ async fn long_run_shell_hands_off_same_job_once_and_status_log_stop_observe_it() assert!(result.output["observation_token"].is_string()); assert_run_shell_result_matches_schema(&result); assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "handoff must not redispatch the shell command" @@ -914,7 +914,7 @@ async fn long_run_shell_async_job_capability_does_not_bypass_shell_authority() { let error = result.error.as_deref().unwrap_or_default(); assert!(error.contains("does not support shell"), "{error}"); assert!(error.contains(client_id), "{error}"); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); @@ -957,7 +957,7 @@ async fn long_run_shell_job_start_rejection_is_not_started_and_enqueues_nothing( assert_eq!(result.output["promoted_to_job"], false); assert_eq!(result.output["async_handoff_available"], true); assert_run_shell_result_matches_schema(&result); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); @@ -1040,7 +1040,7 @@ async fn long_run_shell_job_timeout_is_terminal_and_never_becomes_fake_outcome_u .as_str() .unwrap_or_default() .contains("runner deadline reached")); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert!(runtime.shell_clients.remove_job_record(&job_id).await); diff --git a/src/tool_runtime/tests/lsp.rs b/src/tool_runtime/tests/lsp.rs index 0f5fbac6..d2e6adf1 100644 --- a/src/tool_runtime/tests/lsp.rs +++ b/src/tool_runtime/tests/lsp.rs @@ -369,15 +369,7 @@ async fn complete_lsp_agent_request( client_id: &str, result: impl serde::Serialize, ) { - let mut req = None; - for _ in 0..200 { - req = next_patch_agent_request(runtime, client_id).await; - if req.is_some() { - break; - } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - let req = req.expect("expected LSP agent request"); + let req = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(req.kind, AGENT_LSP_REQUEST_KIND); assert!(req.lsp.is_some()); assert!(req.command.is_empty()); @@ -653,7 +645,7 @@ async fn call_hierarchy_requires_its_distinct_runner_capability_before_dispatch( "{error}" ); assert!( - next_patch_agent_request(&runtime, "navigation-only") + probe_patch_agent_request(&runtime, "navigation-only") .await .is_none(), "capability failure must happen before agent dispatch" @@ -732,15 +724,7 @@ async fn call_hierarchy_dispatch_uses_typed_bridge_and_validates_bounds() { .await } }); - let mut request = None; - for _ in 0..200 { - request = next_patch_agent_request(&runtime, "hierarchy-agent").await; - if request.is_some() { - break; - } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - let request = request.expect("typed call hierarchy request"); + let request = wait_for_patch_agent_request(&runtime, "hierarchy-agent").await; assert_eq!( request.lsp.as_ref().map(|payload| &payload.request), Some(&AgentLspRequest::CallHierarchy { diff --git a/src/tool_runtime/tests/metadata.rs b/src/tool_runtime/tests/metadata.rs index b2cd8a77..ae953682 100644 --- a/src/tool_runtime/tests/metadata.rs +++ b/src/tool_runtime/tests/metadata.rs @@ -540,9 +540,7 @@ async fn shared_key_list_projects_and_dispatch_are_filtered_by_auth_group() { .await } }); - let req = next_agent_request_for_client(&runtime, "client-a") - .await - .expect("bridge read_file should enqueue for shared-key group A"); + let req = wait_for_agent_request_for_client(&runtime, "client-a").await; complete_patch_agent_request_for_instance( &runtime, "client-a", @@ -634,9 +632,7 @@ async fn shared_key_list_projects_and_dispatch_are_filtered_by_auth_group() { .await } }); - let req = next_agent_request_for_client(&runtime, "client-open") - .await - .expect("open read_file should enqueue for the open agent"); + let req = wait_for_agent_request_for_client(&runtime, "client-open").await; complete_patch_agent_request_for_instance( &runtime, "client-open", @@ -666,9 +662,7 @@ async fn shared_key_list_projects_and_dispatch_are_filtered_by_auth_group() { .await } }); - let req = next_agent_request_for_client(&runtime, "client-open") - .await - .expect("open git_status should enqueue for the open agent"); + let req = wait_for_agent_request_for_client(&runtime, "client-open").await; complete_patch_agent_request_for_instance( &runtime, "client-open", @@ -868,7 +862,7 @@ async fn replacement_runner_pending_inventory_has_zero_project_routing_authority assert_eq!(result.output["error_kind"], "unknown_project"); } assert!( - next_agent_request_for_instance(&runtime, client_id, new_instance) + probe_agent_request_for_instance(&runtime, client_id, new_instance) .await .is_none(), "pending replacement must receive zero project execution dispatches" @@ -918,9 +912,7 @@ async fn replacement_runner_pending_inventory_has_zero_project_routing_authority .await } }); - let request = next_agent_request_for_instance(&runtime, client_id, new_instance) - .await - .expect("completed replacement snapshot should restore routing"); + let request = wait_for_agent_request_for_instance(&runtime, client_id, new_instance).await; assert_eq!(request.cwd.as_deref(), Some(path_b.as_str())); complete_patch_agent_request_for_instance( &runtime, @@ -1006,7 +998,7 @@ async fn replacement_runner_removed_project_never_inherits_old_authority() { ); assert_eq!(result.output["error_kind"], "unknown_project"); assert!( - next_agent_request_for_instance(&runtime, client_id, new_instance) + probe_agent_request_for_instance(&runtime, client_id, new_instance) .await .is_none(), "removed project must receive zero dispatch during {phase}" @@ -1257,9 +1249,7 @@ async fn unique_short_agent_project_id_is_resolved_by_runtime_surface() { .await } }); - let req = next_agent_request_for_instance(&runtime, "oe", "inst") - .await - .expect("unique short id should resolve to the owning agent"); + let req = wait_for_agent_request_for_instance(&runtime, "oe", "inst").await; assert_eq!(req.cwd.as_deref(), Some("/tmp/agent-proj")); runtime .shell_clients diff --git a/src/tool_runtime/tests/observe_jobs.rs b/src/tool_runtime/tests/observe_jobs.rs index 85a545b1..3bb94267 100644 --- a/src/tool_runtime/tests/observe_jobs.rs +++ b/src/tool_runtime/tests/observe_jobs.rs @@ -106,9 +106,7 @@ async fn register_and_start_agent_job( .await; assert!(started.success, "{:?}", started.error); let job_id = started.output["job_id"].as_str().unwrap().to_string(); - let request = next_patch_agent_request(runtime, client_id) - .await - .expect("Agent Job start request"); + let request = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(request.job_id.as_deref(), Some(job_id.as_str())); (job_id, request, auth) } @@ -151,9 +149,7 @@ async fn start_owned_agent_job( .await; assert!(started.success, "{:?}", started.error); let job_id = started.output["job_id"].as_str().unwrap().to_string(); - let request = next_agent_request_for_client(runtime, client_id) - .await - .expect("owned Agent Job request"); + let request = wait_for_agent_request_for_client(runtime, client_id).await; assert_eq!(request.job_id.as_deref(), Some(job_id.as_str())); job_id } @@ -841,9 +837,7 @@ async fn observe_jobs_recovering_lost_and_stop_requested_match_job_log_semantics ) .await; let recovering_job = started.output["job_id"].as_str().unwrap().to_string(); - let recovering_request = next_patch_agent_request(&runtime, "observe-recovering") - .await - .unwrap(); + let recovering_request = wait_for_patch_agent_request(&runtime, "observe-recovering").await; runtime .shell_clients .update_job(ShellAgentJobUpdateRequest { @@ -1029,7 +1023,7 @@ async fn observe_jobs_mixed_success_result_matches_declared_output_schema_and_en assert!(result.success, "{:?}", result.error); assert_eq!(result.output["succeeded_count"], 1); assert_eq!(result.output["failed_count"], 1); - assert!(next_patch_agent_request(&runtime, "observe-no-enqueue") + assert!(probe_patch_agent_request(&runtime, "observe-no-enqueue") .await .is_none()); diff --git a/src/tool_runtime/tests/permission_gate.rs b/src/tool_runtime/tests/permission_gate.rs index 3abd66ed..941026e0 100644 --- a/src/tool_runtime/tests/permission_gate.rs +++ b/src/tool_runtime/tests/permission_gate.rs @@ -53,9 +53,7 @@ async fn register_write_agent(runtime: &ToolRuntime, client_id: &str) { /// Complete a successful agent write so the mutation path can finish. async fn complete_write_ok(runtime: &ToolRuntime, client_id: &str, path: &str) { - let req = next_patch_agent_request(runtime, client_id) - .await - .expect("mutation tool should enqueue agent request under allowing modes"); + let req = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(req.kind, "file_write_project_file"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("file-op payload")).unwrap(); @@ -160,7 +158,7 @@ async fn restricted_blocks_mutation_before_agent_enqueue() { ); assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "restricted authority must not enqueue mutation" @@ -213,7 +211,7 @@ async fn invalid_mode_blocks_mutation_and_does_not_auto_approve() { "{err}" ); assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "invalid mode must not enqueue mutation" @@ -255,7 +253,7 @@ async fn hard_policy_deny_still_suppresses_permission_attach() { result.output.get("permission") ); assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "policy rejection must happen before enqueue" @@ -304,9 +302,7 @@ async fn read_only_tool_skips_permission_decision() { .await } }); - let req = next_patch_agent_request(&runtime, client_id) - .await - .expect("read_file should still execute under restricted authority"); + let req = wait_for_patch_agent_request(&runtime, client_id).await; complete_patch_agent_request( &runtime, client_id, diff --git a/src/tool_runtime/tests/process.rs b/src/tool_runtime/tests/process.rs index dd95da3d..96861291 100644 --- a/src/tool_runtime/tests/process.rs +++ b/src/tool_runtime/tests/process.rs @@ -447,7 +447,7 @@ async fn detached_process_requires_job_run_and_detach_scopes_before_any_admissio "{label} should fail at scope gate" ); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); - assert!(next_patch_agent_request(&runtime, "detached-scope-gate") + assert!(probe_patch_agent_request(&runtime, "detached-scope-gate") .await .is_none()); } @@ -478,7 +478,7 @@ async fn detached_process_requires_job_run_and_detach_scopes_before_any_admissio assert!(!denied.success); assert!(denied.error_status.is_some()); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); - assert!(next_patch_agent_request(&runtime, "detached-scope-gate") + assert!(probe_patch_agent_request(&runtime, "detached-scope-gate") .await .is_none()); @@ -521,7 +521,7 @@ async fn detached_process_requires_job_run_and_detach_scopes_before_any_admissio .kind, "start_detached_process_job" ); - assert!(next_patch_agent_request(&runtime, "detached-scope-gate") + assert!(probe_patch_agent_request(&runtime, "detached-scope-gate") .await .is_none()); } @@ -602,7 +602,7 @@ async fn detached_process_idempotency_replays_same_intent_and_rejects_conflict() assert!(replay.success, "{:?}", replay.error); assert_eq!(replay.output["job_id"], job_id); assert!( - next_patch_agent_request(&runtime, "detached-idempotency") + probe_patch_agent_request(&runtime, "detached-idempotency") .await .is_none(), "same-key same-intent replay must not redispatch" @@ -625,7 +625,7 @@ async fn detached_process_idempotency_replays_same_intent_and_rejects_conflict() assert_eq!(conflict.output["failure_kind"], "idempotency_conflict"); assert_eq!(conflict.output["execution_state"], "not_started"); assert!( - next_patch_agent_request(&runtime, "detached-idempotency") + probe_patch_agent_request(&runtime, "detached-idempotency") .await .is_none(), "conflicting key must fail before redispatch" @@ -748,7 +748,7 @@ async fn detached_process_lost_initiation_after_server_restart_recovers_same_job assert_eq!(retry.output["job_id"], job_id); assert_eq!(retry.output["redispatched"], false); assert!( - next_patch_agent_request(&restarted, "detached-restart-recovery") + probe_patch_agent_request(&restarted, "detached-restart-recovery") .await .is_none(), "lost-response recovery must not enqueue a second payload" @@ -778,7 +778,7 @@ async fn detached_process_requires_explicit_runner_authority_before_admission() assert!(!result.success); assert_eq!(result.output["execution_state"], "not_started"); assert_eq!(result.output["failure_kind"], "capability_unavailable"); - assert!(next_patch_agent_request(&runtime, "detached-no-authority") + assert!(probe_patch_agent_request(&runtime, "detached-no-authority") .await .is_none()); } @@ -803,7 +803,7 @@ async fn detached_process_uses_existing_job_identity_and_typed_runner_request() assert_eq!(request.job_id.as_deref(), Some(job_id.as_str())); assert!(request.process.is_some()); assert!(request.script.is_none()); - assert!(next_patch_agent_request(&runtime, "detached-product-path") + assert!(probe_patch_agent_request(&runtime, "detached-product-path") .await .is_none()); } @@ -1217,7 +1217,7 @@ async fn run_process_slow_handoff_is_queryable_once_and_keeps_the_original_budge listed.iter().filter(|job| job["job_id"] == job_id).count(), 1 ); - assert!(next_patch_agent_request(&runtime, "process-slow-job") + assert!(probe_patch_agent_request(&runtime, "process-slow-job") .await .is_none()); @@ -1249,7 +1249,7 @@ async fn run_process_slow_handoff_is_queryable_once_and_keeps_the_original_budge terminal.output["structured_execution"]["execution_source"], "run_process" ); - assert!(next_patch_agent_request(&runtime, "process-slow-job") + assert!(probe_patch_agent_request(&runtime, "process-slow-job") .await .is_none()); } @@ -1317,7 +1317,7 @@ async fn stop_job_stops_the_promoted_process_without_starting_a_replacement() { Some(ShellCommandExecutionState::Completed) ); assert!( - next_patch_agent_request(&runtime, "process-stop-job") + probe_patch_agent_request(&runtime, "process-stop-job") .await .is_none(), "stopping the Job must never enqueue a replacement execution" @@ -1496,7 +1496,7 @@ async fn b2_process_runner_uses_direct_sync_and_rejects_durable_only_timeout() { assert_eq!(rejected.output["execution_state"], "not_started"); assert_eq!(rejected.output["command_started"], false); assert_eq!(rejected.output["failure_kind"], "capability_unavailable"); - assert!(next_patch_agent_request(&runtime, "process-b2") + assert!(probe_patch_agent_request(&runtime, "process-b2") .await .is_none()); } @@ -1570,7 +1570,7 @@ async fn run_process_capability_absence_fails_prestart_without_shell_fallback() .unwrap_or_default() .contains("no shell fallback")); assert!( - next_patch_agent_request(&runtime, "legacy-process-agent") + probe_patch_agent_request(&runtime, "legacy-process-agent") .await .is_none(), "capability failure must not enqueue run_process or run_shell" @@ -1637,7 +1637,7 @@ async fn authority_denied_run_process_has_prestart_lifecycle() { assert_eq!(result.output["command_started"], false); assert_eq!(result.output["command_completed"], false); assert_eq!(result.output["failure_kind"], "permission_denied"); - assert!(next_patch_agent_request(&runtime, "process-authority") + assert!(probe_patch_agent_request(&runtime, "process-authority") .await .is_none()); } @@ -1838,7 +1838,7 @@ async fn run_process_named_ssh_resource_fails_before_enqueue() { assert_eq!(result.output["error_kind"], "unsupported_resource"); assert_eq!(result.output["recovery_kind"], "fix_input"); assert!(result.output.get("recovery_tool").is_none()); - assert!(next_patch_agent_request(&runtime, "process-ssh") + assert!(probe_patch_agent_request(&runtime, "process-ssh") .await .is_none()); } @@ -1966,7 +1966,7 @@ async fn run_process_validation_and_inspect_permission_boundaries_fail_closed() ); assert_eq!(durable_only.output["async_handoff_available"], false); assert!(!root.join("marker").exists()); - assert!(next_patch_agent_request(&runtime, "process-guards") + assert!(probe_patch_agent_request(&runtime, "process-guards") .await .is_none()); @@ -2025,7 +2025,7 @@ async fn run_process_validation_and_inspect_permission_boundaries_fail_closed() assert_eq!(denied.output["command_started"], false); assert_eq!(denied.output["command_completed"], false); assert_eq!(denied.output["failure_kind"], "session_guard_denied"); - assert!(next_patch_agent_request(&runtime, "process-guards") + assert!(probe_patch_agent_request(&runtime, "process-guards") .await .is_none()); } @@ -2057,7 +2057,7 @@ async fn closed_session_run_process_has_prestart_lifecycle() { assert_eq!(result.output["command_started"], false); assert_eq!(result.output["command_completed"], false); assert_eq!(result.output["failure_kind"], "session_closed"); - assert!(next_patch_agent_request(&runtime, "process-closed") + assert!(probe_patch_agent_request(&runtime, "process-closed") .await .is_none()); } @@ -2116,7 +2116,7 @@ async fn model_facing_session_denials_keep_run_process_prestart_lifecycle() { assert_eq!(result.output["failure_kind"], failure_kind); } assert!( - next_patch_agent_request(&runtime, "process-kernel-guards") + probe_patch_agent_request(&runtime, "process-kernel-guards") .await .is_none(), "model-facing Session denials must happen before Runner enqueue" diff --git a/src/tool_runtime/tests/reconnect.rs b/src/tool_runtime/tests/reconnect.rs index 11c050c3..d018937d 100644 --- a/src/tool_runtime/tests/reconnect.rs +++ b/src/tool_runtime/tests/reconnect.rs @@ -241,9 +241,7 @@ async fn runner_disconnect_and_reconnect_change_layers_independently() { .await } }); - let req = next_agent_request_for_instance(&runtime, "rc-agent", "inst-b") - .await - .expect("new instance receives work after reconnect"); + let req = wait_for_agent_request_for_instance(&runtime, "rc-agent", "inst-b").await; complete_patch_agent_request_for_instance( &runtime, "rc-agent", @@ -1517,9 +1515,7 @@ async fn agent_job_lost_on_disconnect_stays_terminal_after_reconnect() { ) .await .unwrap(); - let _req = next_agent_request_for_instance(&runtime, "job-agent", "inst-a") - .await - .expect("job request dispatched"); + let _req = wait_for_agent_request_for_instance(&runtime, "job-agent", "inst-a").await; // Transport drops mid-job: job authority is not silently completed. runtime @@ -1824,7 +1820,12 @@ async fn dispatch_start_coding_task_in_window_with_transport( .await } }); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); while !task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "start_coding_task did not finish within the 10-second test deadline" + ); if let Some(req) = runtime .shell_clients .poll(crate::shell_protocol::ShellAgentPollRequest { diff --git a/src/tool_runtime/tests/script.rs b/src/tool_runtime/tests/script.rs index 7d6d9184..50935e73 100644 --- a/src/tool_runtime/tests/script.rs +++ b/src/tool_runtime/tests/script.rs @@ -216,9 +216,7 @@ async fn run_script_wire_is_typed_body_free_command_and_supports_more_than_32_ki .await } }); - let request = next_patch_agent_request(&runtime, "script-wire") - .await - .expect("run_script should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "script-wire").await; assert_eq!(request.kind, "run_script"); assert_eq!(request.command, ""); assert!(request.process.is_none()); @@ -280,9 +278,7 @@ async fn run_script_fast_success_projects_back_and_removes_the_hidden_job() { .await } }); - let request = next_patch_agent_request(&runtime, "script-fast-job") - .await - .expect("hidden script Job should dispatch"); + let request = wait_for_patch_agent_request(&runtime, "script-fast-job").await; assert_eq!(request.kind, "start_script_job"); assert_eq!(request.command, ""); assert!(request.process.is_none()); @@ -387,9 +383,7 @@ async fn run_script_fast_missing_interpreter_retains_not_started_through_the_hid .await } }); - let request = next_patch_agent_request(&runtime, "script-prestart-job") - .await - .expect("hidden script Job should dispatch"); + let request = wait_for_patch_agent_request(&runtime, "script-prestart-job").await; let queued = runtime .shell_clients .get_hidden_job_for_auth(Some(&auth), request.job_id.as_deref().unwrap()) @@ -471,9 +465,7 @@ async fn run_script_slow_handoff_keeps_typed_payload_ephemeral_and_safe_metadata .await } }); - let request = next_patch_agent_request(&runtime, "script-slow-job") - .await - .expect("typed script Job should dispatch"); + let request = wait_for_patch_agent_request(&runtime, "script-slow-job").await; assert_eq!(request.kind, "start_script_job"); assert_eq!(request.command, ""); assert!(request.process.is_none()); @@ -612,7 +604,7 @@ async fn run_script_slow_handoff_keeps_typed_payload_ephemeral_and_safe_metadata terminal.command_execution_state, Some(ShellCommandExecutionState::Completed) ); - assert!(next_patch_agent_request(&runtime, "script-slow-job") + assert!(probe_patch_agent_request(&runtime, "script-slow-job") .await .is_none()); } @@ -662,9 +654,7 @@ async fn b2_script_runner_uses_direct_sync_and_rejects_durable_only_timeout() { .await } }); - let request = next_patch_agent_request(&runtime, "script-b2") - .await - .expect("B2 direct script request"); + let request = wait_for_patch_agent_request(&runtime, "script-b2").await; assert_eq!(request.kind, "run_script"); complete_script_lifecycle( &runtime, @@ -710,7 +700,7 @@ async fn b2_script_runner_uses_direct_sync_and_rejects_durable_only_timeout() { assert_eq!(rejected.output["execution_state"], "not_started"); assert_eq!(rejected.output["command_started"], false); assert_eq!(rejected.output["failure_kind"], "capability_unavailable"); - assert!(next_patch_agent_request(&runtime, "script-b2") + assert!(probe_patch_agent_request(&runtime, "script-b2") .await .is_none()); } @@ -734,9 +724,7 @@ async fn run_script_nonzero_timeout_uncertainty_and_interpreter_absence_are_trut .await } }); - let request = next_patch_agent_request(&runtime, "script-lifecycle") - .await - .unwrap(); + let request = wait_for_patch_agent_request(&runtime, "script-lifecycle").await; complete_script_lifecycle( &runtime, "script-lifecycle", @@ -768,9 +756,7 @@ async fn run_script_nonzero_timeout_uncertainty_and_interpreter_absence_are_trut .await } }); - let request = next_patch_agent_request(&runtime, "script-lifecycle") - .await - .unwrap(); + let request = wait_for_patch_agent_request(&runtime, "script-lifecycle").await; complete_script_lifecycle( &runtime, "script-lifecycle", @@ -804,9 +790,7 @@ async fn run_script_nonzero_timeout_uncertainty_and_interpreter_absence_are_trut .await } }); - next_patch_agent_request(&runtime, "script-lifecycle") - .await - .unwrap(); + wait_for_patch_agent_request(&runtime, "script-lifecycle").await; runtime .shell_clients .reconcile_disconnect("script-lifecycle", "inst") @@ -846,9 +830,7 @@ async fn run_script_nonzero_timeout_uncertainty_and_interpreter_absence_are_trut .await } }); - let request = next_patch_agent_request(&missing_runtime, "script-interpreter") - .await - .unwrap(); + let request = wait_for_patch_agent_request(&missing_runtime, "script-interpreter").await; complete_script_lifecycle( &missing_runtime, "script-interpreter", @@ -889,7 +871,7 @@ async fn run_script_capability_and_authority_fail_before_enqueue_without_shell_f .as_deref() .unwrap_or_default() .contains("no shell fallback")); - assert!(next_patch_agent_request(&runtime, "legacy-script") + assert!(probe_patch_agent_request(&runtime, "legacy-script") .await .is_none()); @@ -913,7 +895,7 @@ async fn run_script_capability_and_authority_fail_before_enqueue_without_shell_f assert_eq!(denied.output["command_started"], false); assert_eq!(denied.output["command_completed"], false); assert_eq!(denied.output["failure_kind"], "permission_denied"); - assert!(next_patch_agent_request(&denied_runtime, "denied-script") + assert!(probe_patch_agent_request(&denied_runtime, "denied-script") .await .is_none()); } @@ -976,9 +958,7 @@ async fn run_script_session_defaults_and_evidence_are_body_and_stdin_free() { .await } }); - let request = next_patch_agent_request(&runtime, "script-context") - .await - .expect("Session run_script should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "script-context").await; assert_eq!( request.cwd.as_deref(), Some(frontend.to_string_lossy().as_ref()) @@ -1081,7 +1061,7 @@ async fn run_script_ssh_read_only_closed_and_inspect_session_boundaries_fail_clo assert_eq!(unsupported.output["error_kind"], "unsupported_resource"); assert_eq!(unsupported.output["recovery_kind"], "fix_input"); assert!(unsupported.output.get("recovery_tool").is_none()); - assert!(next_patch_agent_request(&runtime, "script-guards") + assert!(probe_patch_agent_request(&runtime, "script-guards") .await .is_none()); @@ -1106,7 +1086,7 @@ async fn run_script_ssh_read_only_closed_and_inspect_session_boundaries_fail_clo assert_eq!(mismatched.output["execution_state"], "not_started"); assert_eq!(mismatched.output["command_started"], false); assert_eq!(mismatched.output["command_completed"], false); - assert!(next_patch_agent_request(&runtime, "script-guards") + assert!(probe_patch_agent_request(&runtime, "script-guards") .await .is_none()); @@ -1175,9 +1155,7 @@ async fn run_script_ssh_read_only_closed_and_inspect_session_boundaries_fail_clo .await } }); - let request = next_patch_agent_request(&runtime, "script-guards") - .await - .expect("inspect script should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "script-guards").await; assert_eq!( request.sandbox.as_deref(), Some(crate::command_sandbox::INSPECT_SANDBOX_MODE) @@ -1313,7 +1291,7 @@ async fn run_script_shared_bounds_reject_before_enqueue_with_full_prestart_tuple "capability_unavailable" ); assert_eq!(durable_only.output["async_handoff_available"], false); - assert!(next_patch_agent_request(&runtime, "script-bounds") + assert!(probe_patch_agent_request(&runtime, "script-bounds") .await .is_none()); } @@ -1355,7 +1333,7 @@ async fn model_facing_run_script_session_denials_keep_phase_a_tuple() { assert_eq!(result.output["command_started"], false); assert_eq!(result.output["command_completed"], false); assert_eq!(result.output["failure_kind"], "session_guard_denied"); - assert!(next_patch_agent_request(&runtime, "script-kernel") + assert!(probe_patch_agent_request(&runtime, "script-kernel") .await .is_none()); } diff --git a/src/tool_runtime/tests/search_project_texts.rs b/src/tool_runtime/tests/search_project_texts.rs index ca67acde..ee2c6ebe 100644 --- a/src/tool_runtime/tests/search_project_texts.rs +++ b/src/tool_runtime/tests/search_project_texts.rs @@ -105,9 +105,7 @@ async fn run_single_agent_batch_response( .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("single batch search request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; complete_patch_agent_request( &runtime, client_id, @@ -265,9 +263,7 @@ async fn search_project_text_default_success_is_sparse_after_session_recording() .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("single sparse search request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; complete_search_success(&runtime, client_id, &request, "src/a.rs").await; let result = task.await.unwrap(); @@ -362,9 +358,7 @@ async fn search_project_text_nondefault_success_keeps_effective_metadata() { .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("nondefault search request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; complete_search_success(&runtime, client_id, &request, "src/a.rs").await; let result = task.await.unwrap(); @@ -413,9 +407,7 @@ async fn search_project_text_grep_fallback_keeps_backend_metadata() { .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("grep fallback search request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; let stdout = concat!( "{\"webcodex_search\":{\"backend\":\"grep\",\"feature_unavailable\":false}}\n", "src/a.rs:1:needle\n" @@ -458,8 +450,8 @@ async fn search_project_texts_default_matches_items_are_sparse_and_schema_valid( } }); let requests = [ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; for request in &requests { let pattern = request_pattern(request); @@ -548,9 +540,7 @@ async fn search_project_texts_dispatch_mixed_batch_only_sparsifies_success_item( .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("mixed batch successful query request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&request), "steady"); complete_search_success(&runtime, client_id, &request, "src/steady.rs").await; assert_no_agent_request(&runtime, client_id).await; @@ -625,8 +615,8 @@ async fn search_project_texts_retries_one_dropped_agent_request_and_restores_ord }); let first_two = vec![ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; let dropped = first_two .iter() @@ -642,9 +632,7 @@ async fn search_project_texts_retries_one_dropped_agent_request_and_restores_ord .await; complete_search_success(&runtime, client_id, steady, "src/steady.rs").await; - let retry = next_patch_agent_request(&runtime, client_id) - .await - .expect("dropped query retry"); + let retry = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&retry), "retry-me"); assert_ne!(retry.request_id, dropped.request_id); complete_search_success(&runtime, client_id, &retry, "src/retried.rs").await; @@ -677,15 +665,13 @@ async fn search_project_texts_stops_after_two_dropped_agent_attempts() { } }); - let first = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let first = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&first), "drop-twice"); runtime .shell_clients .cancel_request(&first.request_id) .await; - let second = next_patch_agent_request(&runtime, client_id) - .await - .expect("one retry after first drop"); + let second = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&second), "drop-twice"); assert_ne!(second.request_id, first.request_id); runtime @@ -725,8 +711,8 @@ async fn search_project_texts_retry_stays_inside_existing_concurrency_slot() { }); let first_two = vec![ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; let retry_slot = first_two .iter() @@ -741,9 +727,7 @@ async fn search_project_texts_retry_stays_inside_existing_concurrency_slot() { .cancel_request(&retry_slot.request_id) .await; - let retry = next_patch_agent_request(&runtime, client_id) - .await - .expect("retry must replace work inside the occupied query slot"); + let retry = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&retry), "retry-slot"); assert!( poll_agent_request(&runtime, client_id).await.is_none(), @@ -751,9 +735,7 @@ async fn search_project_texts_retry_stays_inside_existing_concurrency_slot() { ); complete_search_success(&runtime, client_id, &retry, "src/retry.rs").await; - let third = next_patch_agent_request(&runtime, client_id) - .await - .expect("third query after retry slot completes"); + let third = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&third), "third"); complete_search_success(&runtime, client_id, blocker, "src/blocker.rs").await; complete_search_success(&runtime, client_id, &third, "src/third.rs").await; @@ -779,15 +761,13 @@ async fn search_project_texts_retry_uses_only_remaining_absolute_deadline() { } }); - let first = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let first = wait_for_patch_agent_request(&runtime, client_id).await; tokio::time::sleep(Duration::from_millis(1100)).await; runtime .shell_clients .cancel_request(&first.request_id) .await; - let retry = next_patch_agent_request(&runtime, client_id) - .await - .expect("retry before batch deadline"); + let retry = wait_for_patch_agent_request(&runtime, client_id).await; assert!( retry.timeout_secs < first.timeout_secs, "retry reset the command timeout instead of using remaining batch budget: first={} retry={}", @@ -812,7 +792,7 @@ async fn search_project_texts_retry_uses_only_remaining_absolute_deadline() { .await } }); - let request = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let request = wait_for_patch_agent_request(&runtime, client_id).await; let result = tokio::time::timeout(Duration::from_secs(2), task) .await .expect("absolute batch deadline should end the query") @@ -947,15 +927,15 @@ async fn search_project_texts_restores_input_order_after_out_of_order_runner_com }); let first_two = [ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; let second = first_two .iter() .find(|request| request_pattern(request) == "second") .unwrap(); complete_search_success(&runtime, client_id, second, "src/second.rs").await; - let third = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let third = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&third), "third"); complete_search_success(&runtime, client_id, &third, "src/third.rs").await; let first = first_two @@ -1025,7 +1005,7 @@ async fn search_project_texts_isolates_validation_no_match_and_protected_path_re }); for _ in 0..2 { - let request = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let request = wait_for_patch_agent_request(&runtime, client_id).await; match request_pattern(&request).as_str() { "found" => complete_search_success(&runtime, client_id, &request, "src/found.rs").await, "absent" => { @@ -1085,8 +1065,8 @@ async fn search_project_texts_runner_in_flight_is_concurrent_and_never_exceeds_t }); let mut active = vec![ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; let third_before_completion = runtime .shell_clients @@ -1107,7 +1087,7 @@ async fn search_project_texts_runner_in_flight_is_concurrent_and_never_exceeds_t while dispatched < 8 { let finished = active.remove(0); complete_search_success(&runtime, client_id, &finished, "src/result.rs").await; - active.push(next_patch_agent_request(&runtime, client_id).await.unwrap()); + active.push(wait_for_patch_agent_request(&runtime, client_id).await); dispatched += 1; max_in_flight = max_in_flight.max(active.len()); assert!(active.len() <= 2); @@ -1145,15 +1125,15 @@ async fn search_project_texts_deadline_preserves_fast_result_and_cancels_unfinis } }); let first_two = vec![ - next_patch_agent_request(&runtime, client_id).await.unwrap(), - next_patch_agent_request(&runtime, client_id).await.unwrap(), + wait_for_patch_agent_request(&runtime, client_id).await, + wait_for_patch_agent_request(&runtime, client_id).await, ]; let fast = first_two .iter() .find(|request| request_pattern(request) == "fast") .unwrap(); complete_search_success(&runtime, client_id, fast, "src/fast.rs").await; - let third = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let third = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request_pattern(&third), "slow-b"); let unfinished = first_two .into_iter() @@ -1225,7 +1205,7 @@ async fn search_project_texts_records_one_event_without_patterns_and_aggregates_ } }); for _ in 0..2 { - let request = next_patch_agent_request(&runtime, client_id).await.unwrap(); + let request = wait_for_patch_agent_request(&runtime, client_id).await; let stdout = match request_pattern(&request).as_str() { "RAW_BATCH_PATTERN_ALPHA" => format!( "{}{}", diff --git a/src/tool_runtime/tests/session_shells.rs b/src/tool_runtime/tests/session_shells.rs index a06047a9..86dc7e0d 100644 --- a/src/tool_runtime/tests/session_shells.rs +++ b/src/tool_runtime/tests/session_shells.rs @@ -62,14 +62,9 @@ async fn setup( async fn next_persistent_request( runtime: &ToolRuntime, ) -> crate::shell_protocol::ShellAgentShellRequest { - for _ in 0..100 { - if let Some(request) = next_patch_agent_request(runtime, CLIENT).await { - assert_eq!(request.kind, "persistent_shell"); - return request; - } - tokio::time::sleep(std::time::Duration::from_millis(2)).await; - } - panic!("persistent shell request was not enqueued") + let request = wait_for_patch_agent_request(runtime, CLIENT).await; + assert_eq!(request.kind, "persistent_shell"); + request } async fn complete( diff --git a/src/tool_runtime/tests/sessions.rs b/src/tool_runtime/tests/sessions.rs index 92d8d5aa..b5a31359 100644 --- a/src/tool_runtime/tests/sessions.rs +++ b/src/tool_runtime/tests/sessions.rs @@ -53,9 +53,7 @@ async fn read_agent_file_for_session( .await } }); - let req = next_agent_request_for_instance(runtime, client_id, "inst") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_agent_request_for_instance(runtime, client_id, "inst").await; complete_patch_agent_request( runtime, client_id, @@ -106,9 +104,7 @@ async fn read_file_with_session_id_records_event_without_content() { .await } }); - let req = next_agent_request_for_instance(&runtime, "telemetry-read", "inst") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_agent_request_for_instance(&runtime, "telemetry-read", "inst").await; assert_eq!(req.kind, "file_read"); complete_patch_agent_request( &runtime, @@ -177,9 +173,7 @@ async fn read_file_without_session_id_omits_session_telemetry() { .await } }); - let req = next_agent_request_for_instance(&runtime, "telemetry-nosession", "inst") - .await - .expect("read_file should enqueue without session_id"); + let req = wait_for_agent_request_for_instance(&runtime, "telemetry-nosession", "inst").await; complete_patch_agent_request( &runtime, "telemetry-nosession", @@ -685,9 +679,7 @@ async fn finish_coding_task_does_not_auto_close_session() { .await } }); - let req = next_patch_agent_request(&runtime, "coding-finish-no-close") - .await - .expect("finish_coding_task should inspect workspace through the agent"); + let req = wait_for_patch_agent_request(&runtime, "coding-finish-no-close").await; complete_patch_agent_request( &runtime, "coding-finish-no-close", diff --git a/src/tool_runtime/tests/sessions_current.rs b/src/tool_runtime/tests/sessions_current.rs index 68997561..2c3970b4 100644 --- a/src/tool_runtime/tests/sessions_current.rs +++ b/src/tool_runtime/tests/sessions_current.rs @@ -298,9 +298,7 @@ async fn bound_current_session_records_project_tool_without_session_id() { .await } }); - let req = next_agent_request_for_instance(&runtime, "current-read", "inst") - .await - .expect("read_file should enqueue with current session"); + let req = wait_for_agent_request_for_instance(&runtime, "current-read", "inst").await; complete_patch_agent_request( &runtime, "current-read", @@ -417,9 +415,8 @@ async fn open_anonymous_can_bind_current_session_and_record_project_read() { .await } }); - let req = next_agent_request_for_instance(&runtime, "open-current", "inst-open-current") - .await - .expect("open read_file should enqueue with current session"); + let req = + wait_for_agent_request_for_instance(&runtime, "open-current", "inst-open-current").await; complete_patch_agent_request_for_instance( &runtime, "open-current", @@ -494,9 +491,7 @@ async fn generic_tool_call_uses_bound_current_session_without_session_id() { .await } }); - let req = next_agent_request_for_instance(&runtime, "current-generic", "inst") - .await - .expect("generic read_file should enqueue with current session"); + let req = wait_for_agent_request_for_instance(&runtime, "current-generic", "inst").await; complete_patch_agent_request( &runtime, "current-generic", @@ -590,9 +585,7 @@ async fn explicit_session_id_wins_over_current_session() { .await } }); - let req = next_agent_request_for_instance(&runtime, "current-explicit", "inst") - .await - .expect("read_file should enqueue with explicit session"); + let req = wait_for_agent_request_for_instance(&runtime, "current-explicit", "inst").await; complete_patch_agent_request( &runtime, "current-explicit", @@ -693,7 +686,7 @@ async fn unknown_explicit_session_id_does_not_fallback_to_current_session() { 0 ); assert!( - next_agent_request_for_instance(&runtime, "current-missing-explicit", "inst") + probe_agent_request_for_instance(&runtime, "current-missing-explicit", "inst") .await .is_none(), "unknown explicit session_id must not enqueue via current-session fallback" @@ -765,9 +758,7 @@ async fn stale_current_session_is_cleared_and_project_tool_runs_without_session( .await } }); - let req = next_agent_request_for_instance(&runtime, "current-stale", "inst") - .await - .expect("stale current session should not block no-session call"); + let req = wait_for_agent_request_for_instance(&runtime, "current-stale", "inst").await; complete_patch_agent_request( &runtime, "current-stale", diff --git a/src/tool_runtime/tests/sessions_git.rs b/src/tool_runtime/tests/sessions_git.rs index 2ea89ebe..a3954572 100644 --- a/src/tool_runtime/tests/sessions_git.rs +++ b/src/tool_runtime/tests/sessions_git.rs @@ -33,9 +33,7 @@ async fn git_status_with_session_id_records_git_read_event() { .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-git") - .await - .expect("git_status should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-git").await; complete_patch_agent_request(&runtime, "telemetry-git", &req.request_id, 0, "", "").await; let result = task.await.unwrap(); @@ -87,9 +85,7 @@ async fn git_log_parses_commits() { .await } }); - let req = next_patch_agent_request(&runtime, "git-log-parse") - .await - .expect("git_log should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "git-log-parse").await; assert!(req.command.contains("git log")); assert!(req.command.contains("-n 21")); complete_patch_agent_request(&runtime, "git-log-parse", &req.request_id, 0, &stdout, "").await; @@ -149,9 +145,7 @@ async fn git_log_limit_and_skip_returns_second_recent_and_truncated() { .await } }); - let req = next_patch_agent_request(&runtime, "git-log-page") - .await - .expect("git_log should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "git-log-page").await; assert!(req.command.contains("-n 2")); assert!(req.command.contains("--skip 1")); complete_patch_agent_request(&runtime, "git-log-page", &req.request_id, 0, &stdout, "").await; @@ -249,9 +243,7 @@ async fn git_log_read_only_session_allowed_and_recorded() { .await } }); - let req = next_patch_agent_request(&runtime, "git-log-readonly") - .await - .expect("git_log should be allowed in read_only session"); + let req = wait_for_patch_agent_request(&runtime, "git-log-readonly").await; complete_patch_agent_request( &runtime, "git-log-readonly", diff --git a/src/tool_runtime/tests/sessions_guards.rs b/src/tool_runtime/tests/sessions_guards.rs index 04dae176..7124cf89 100644 --- a/src/tool_runtime/tests/sessions_guards.rs +++ b/src/tool_runtime/tests/sessions_guards.rs @@ -109,9 +109,7 @@ async fn same_project_session_records_without_project_mismatch_warning() { .await } }); - let req = next_patch_agent_request(&runtime, "alpha-client") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_patch_agent_request(&runtime, "alpha-client").await; complete_patch_agent_request( &runtime, "alpha-client", @@ -166,9 +164,7 @@ async fn read_only_cross_project_session_succeeds_with_structured_warning() { .await } }); - let req = next_patch_agent_request(&runtime, "bravo-client") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_patch_agent_request(&runtime, "bravo-client").await; complete_patch_agent_request( &runtime, "bravo-client", @@ -313,9 +309,7 @@ async fn allow_cross_project_session_allows_mutation_and_records_warning() { .await } }); - let req = next_patch_agent_request(&runtime, "bravo-client") - .await - .expect("write_project_file should enqueue a native file-op request"); + let req = wait_for_patch_agent_request(&runtime, "bravo-client").await; assert_eq!(req.kind, "file_write_project_file"); let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("file-op payload")).unwrap(); @@ -449,9 +443,7 @@ async fn current_session_binding_cannot_cross_project_boundary() { .await } }); - let req = next_patch_agent_request(&runtime, "bravo-client") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_patch_agent_request(&runtime, "bravo-client").await; complete_patch_agent_request( &runtime, "bravo-client", @@ -526,7 +518,7 @@ async fn read_only_current_session_guard_blocks_write_before_enqueue() { assert_eq!(result.output["session_id"], session.session_id); assert_eq!(result.output["session_recorded"], true); assert!( - next_agent_request_for_instance(&runtime, "current-guard", "inst") + probe_agent_request_for_instance(&runtime, "current-guard", "inst") .await .is_none(), "guard denial must happen before an agent request is enqueued" @@ -652,9 +644,7 @@ async fn inspect_session_blocks_structured_write_and_landlocks_run_shell() { .await } }); - let request = next_patch_agent_request(&runtime, "guard-inspect") - .await - .expect("inspect run_shell should be enqueued"); + let request = wait_for_patch_agent_request(&runtime, "guard-inspect").await; assert_eq!( request.sandbox.as_deref(), Some(crate::command_sandbox::INSPECT_SANDBOX_MODE) @@ -714,9 +704,7 @@ async fn inspect_session_landlocks_cargo_and_async_job_entry_points() { } }); let cargo_request = - next_agent_request_for_instance(&runtime, "guard-inspect-all-shell", "inst") - .await - .expect("inspect cargo_check should enqueue"); + wait_for_agent_request_for_instance(&runtime, "guard-inspect-all-shell", "inst").await; assert_eq!(cargo_request.kind, "run_shell"); assert_eq!( cargo_request.sandbox.as_deref(), @@ -748,9 +736,8 @@ async fn inspect_session_landlocks_cargo_and_async_job_entry_points() { ) .await; assert!(job.success, "{:?}", job.error); - let job_request = next_agent_request_for_instance(&runtime, "guard-inspect-all-shell", "inst") - .await - .expect("inspect run_job should enqueue"); + let job_request = + wait_for_agent_request_for_instance(&runtime, "guard-inspect-all-shell", "inst").await; assert_eq!(job_request.kind, "start_job"); assert_eq!( job_request.sandbox.as_deref(), @@ -800,9 +787,7 @@ async fn read_only_session_allows_read_file_and_records_success() { .await } }); - let req = next_agent_request_for_instance(&runtime, "guard-read", "inst") - .await - .expect("read_file should be allowed in read_only session"); + let req = wait_for_agent_request_for_instance(&runtime, "guard-read", "inst").await; assert_eq!(req.kind, "file_read"); complete_patch_agent_request( &runtime, @@ -977,7 +962,7 @@ async fn read_only_session_rejects_all_artifact_upload_tools_without_base64_leak assert_eq!(result.output["session_recorded"], true); } assert!( - next_patch_agent_request(&runtime, "guard-artifact-upload") + probe_patch_agent_request(&runtime, "guard-artifact-upload") .await .is_none(), "artifact upload guard denial must not enqueue an agent request" @@ -1070,7 +1055,7 @@ async fn read_only_session_rejects_run_shell_before_agent_enqueue() { assert!(result.output.get("permission").is_none()); assert_eq!(result.output["session_recorded"], true); assert!( - next_patch_agent_request(&runtime, "guard-shell") + probe_patch_agent_request(&runtime, "guard-shell") .await .is_none(), "run_shell guard denial must not enqueue an agent request" @@ -1150,9 +1135,7 @@ async fn deny_write_only_allows_read_and_shell_tools() { .await } }); - let req = next_agent_request_for_instance(&runtime, "guard-write-only", "inst") - .await - .expect("read_file should be allowed with deny_write_tools only"); + let req = wait_for_agent_request_for_instance(&runtime, "guard-write-only", "inst").await; complete_patch_agent_request( &runtime, "guard-write-only", @@ -1186,9 +1169,7 @@ async fn deny_write_only_allows_read_and_shell_tools() { .await } }); - let req = next_patch_agent_request(&runtime, "guard-write-only") - .await - .expect("run_shell should be allowed when deny_shell_tools=false"); + let req = wait_for_patch_agent_request(&runtime, "guard-write-only").await; complete_patch_agent_request(&runtime, "guard-write-only", &req.request_id, 0, "", "").await; assert!(shell_task.await.unwrap().success); } @@ -1258,9 +1239,7 @@ async fn deny_shell_only_allows_write_tools() { .await } }); - let req = next_patch_agent_request(&runtime, "guard-shell-only") - .await - .expect("write_project_file should be allowed when deny_write_tools=false"); + let req = wait_for_patch_agent_request(&runtime, "guard-shell-only").await; assert_eq!(req.kind, "file_write_project_file"); complete_patch_agent_request( &runtime, diff --git a/src/tool_runtime/tests/sessions_instructions.rs b/src/tool_runtime/tests/sessions_instructions.rs index cc25f97c..fde8adda 100644 --- a/src/tool_runtime/tests/sessions_instructions.rs +++ b/src/tool_runtime/tests/sessions_instructions.rs @@ -45,9 +45,7 @@ async fn start_session_without_project_instructions_when_no_candidate_exists() { }); // Drive every candidate file_read in order; each fails with not-found. for expected_path in project_instructions::INSTRUCTION_CANDIDATE_PATHS { - let req = next_agent_request_for_instance(&runtime, "instr-empty", "inst") - .await - .expect("each candidate should enqueue an agent file_read"); + let req = wait_for_agent_request_for_instance(&runtime, "instr-empty", "inst").await; assert_eq!(req.kind, "file_read"); assert_eq!( req.path.as_deref(), @@ -115,9 +113,7 @@ async fn start_session_loads_agents_md_from_agent_project() { } }); // The loader tries AGENTS.md first; drive that single file_read. - let req = next_agent_request_for_instance(&runtime, "instr-loader", "inst") - .await - .expect("instruction load should enqueue an agent file_read"); + let req = wait_for_agent_request_for_instance(&runtime, "instr-loader", "inst").await; assert_eq!(req.kind, "file_read"); assert_eq!(req.path.as_deref(), Some("AGENTS.md")); complete_patch_agent_request( @@ -186,9 +182,7 @@ async fn start_session_truncates_large_instruction_file() { .await } }); - let req = next_agent_request_for_instance(&runtime, "instr-trunc", "inst") - .await - .expect("instruction load should enqueue an agent file_read"); + let req = wait_for_agent_request_for_instance(&runtime, "instr-trunc", "inst").await; assert_eq!(req.kind, "file_read"); assert_eq!(req.path.as_deref(), Some("AGENTS.md")); // Simulate the agent returning MAX_LINES_PER_FILE + 1 lines for a file @@ -261,9 +255,7 @@ async fn session_summary_returns_project_instructions_without_content() { .await } }); - let req = next_agent_request_for_instance(&runtime, "instr-summary", "inst") - .await - .expect("instruction load should enqueue an agent file_read"); + let req = wait_for_agent_request_for_instance(&runtime, "instr-summary", "inst").await; complete_patch_agent_request( &runtime, "instr-summary", @@ -340,9 +332,7 @@ async fn load_project_instructions_first_match_wins_from_agent_project() { // CLAUDE.md is present (3rd candidate). First match wins => agents.md. let load = runtime.load_project_instructions(&config); let drive_agent = async { - let missing = next_agent_request_for_instance(&runtime, "instr-order", "inst") - .await - .expect("AGENTS.md candidate should enqueue an agent file_read"); + let missing = wait_for_agent_request_for_instance(&runtime, "instr-order", "inst").await; assert_eq!(missing.kind, "file_read"); assert_eq!(missing.path.as_deref(), Some("AGENTS.md")); complete_patch_agent_request( @@ -355,9 +345,7 @@ async fn load_project_instructions_first_match_wins_from_agent_project() { ) .await; - let present = next_agent_request_for_instance(&runtime, "instr-order", "inst") - .await - .expect("agents.md candidate should enqueue an agent file_read"); + let present = wait_for_agent_request_for_instance(&runtime, "instr-order", "inst").await; assert_eq!(present.kind, "file_read"); assert_eq!(present.path.as_deref(), Some("agents.md")); complete_patch_agent_request( diff --git a/src/tool_runtime/tests/sessions_resolver.rs b/src/tool_runtime/tests/sessions_resolver.rs index a06099ec..32d2b358 100644 --- a/src/tool_runtime/tests/sessions_resolver.rs +++ b/src/tool_runtime/tests/sessions_resolver.rs @@ -91,9 +91,7 @@ async fn read_file_accepts_unique_short_id() { .await } }); - let req = next_agent_request_for_client(&runtime, "workstation") - .await - .expect("read_file should enqueue an agent file_read request"); + let req = wait_for_agent_request_for_client(&runtime, "workstation").await; assert_eq!(req.cwd.as_deref(), Some("/root/git/workstation-other-repo")); runtime .shell_clients @@ -131,9 +129,7 @@ async fn git_status_accepts_unique_short_id() { .await } }); - let req = next_agent_request_for_client(&runtime, "workstation") - .await - .expect("git_status should enqueue an agent shell request"); + let req = wait_for_agent_request_for_client(&runtime, "workstation").await; assert_eq!(req.cwd.as_deref(), Some("/root/git/workstation-other-repo")); runtime .shell_clients @@ -198,9 +194,7 @@ async fn full_id_remains_compatible_for_project_tools() { .await } }); - let req = next_agent_request_for_client(&runtime, "workstation") - .await - .expect("full id should still enqueue an agent request"); + let req = wait_for_agent_request_for_client(&runtime, "workstation").await; runtime .shell_clients .complete(ShellAgentResultRequest { diff --git a/src/tool_runtime/tests/startup_brief.rs b/src/tool_runtime/tests/startup_brief.rs index f9281848..81ebf96d 100644 --- a/src/tool_runtime/tests/startup_brief.rs +++ b/src/tool_runtime/tests/startup_brief.rs @@ -880,9 +880,7 @@ async fn startup_uses_project_scoped_lifecycle_aware_job_summary() { ) .await .unwrap(); - let start_request = next_agent_request_for_instance(&runtime, "startup-jobs", "inst") - .await - .expect("runner should receive the queued start_job request"); + let start_request = wait_for_agent_request_for_instance(&runtime, "startup-jobs", "inst").await; assert_eq!(start_request.kind, "start_job"); runtime .shell_clients @@ -987,9 +985,7 @@ async fn startup_uses_project_scoped_lifecycle_aware_job_summary() { .await .unwrap(); assert_eq!(stopped.status, "stop_requested"); - let stop_request = next_agent_request_for_instance(&runtime, "startup-jobs", "inst") - .await - .expect("runner should receive the stop_job request"); + let stop_request = wait_for_agent_request_for_instance(&runtime, "startup-jobs", "inst").await; assert_eq!(stop_request.kind, "stop_job"); assert_eq!(stop_request.job_id.as_deref(), Some(job.job_id.as_str())); diff --git a/src/tool_runtime/tests/support/agent.rs b/src/tool_runtime/tests/support/agent.rs index 4b71e191..6ec0af7d 100644 --- a/src/tool_runtime/tests/support/agent.rs +++ b/src/tool_runtime/tests/support/agent.rs @@ -378,7 +378,7 @@ pub(in crate::tool_runtime::tests) async fn dispatch_checkpoint_with_local_agent }); let deadline = Instant::now() + Duration::from_secs(10); let req = loop { - let request = next_patch_agent_request(runtime, client_id).await; + let request = probe_patch_agent_request(runtime, client_id).await; if request.is_some() || task.is_finished() { break request; } @@ -615,14 +615,14 @@ pub(in crate::tool_runtime::tests) async fn register_agent_projects_for_auth( .unwrap(); } -pub(in crate::tool_runtime::tests) async fn next_agent_request_for_client( +pub(in crate::tool_runtime::tests) async fn probe_agent_request_for_client( runtime: &ToolRuntime, client_id: &str, ) -> Option { - next_agent_request_for_instance(runtime, client_id, &format!("inst-{}", client_id)).await + probe_agent_request_for_instance(runtime, client_id, &format!("inst-{}", client_id)).await } -pub(in crate::tool_runtime::tests) async fn next_agent_request_for_instance( +pub(in crate::tool_runtime::tests) async fn probe_agent_request_for_instance( runtime: &ToolRuntime, client_id: &str, agent_instance_id: &str, @@ -673,6 +673,13 @@ pub(in crate::tool_runtime::tests) async fn wait_for_agent_request_for_instance( } } +pub(in crate::tool_runtime::tests) async fn wait_for_agent_request_for_client( + runtime: &ToolRuntime, + client_id: &str, +) -> ShellAgentShellRequest { + wait_for_agent_request_for_instance(runtime, client_id, &format!("inst-{client_id}")).await +} + pub(in crate::tool_runtime::tests) async fn runtime_with_resolver_projects() -> ToolRuntime { let runtime = test_runtime(); let file_caps = ShellClientCapabilities { @@ -731,7 +738,7 @@ pub(in crate::tool_runtime::tests) async fn runtime_with_resolver_projects() -> // * server-configured (non-agent) projects are rejected by every patch // tool, so the server never touches the filesystem directly. -pub(in crate::tool_runtime::tests) async fn next_patch_agent_request( +pub(in crate::tool_runtime::tests) async fn probe_patch_agent_request( runtime: &ToolRuntime, client_id: &str, ) -> Option { @@ -754,7 +761,7 @@ pub(in crate::tool_runtime::tests) async fn next_patch_agent_request( } /// Wait for a request that the test requires to be dispatched. Unlike -/// `next_patch_agent_request`, which is intentionally a short probe for +/// `probe_patch_agent_request`, which is intentionally a short probe for /// negative/no-dispatch assertions, this positive readiness wait uses one /// absolute wall-clock deadline so scheduler contention cannot turn a fixed /// yield count into a flaky failure. diff --git a/src/tool_runtime/tests/sync_timeout.rs b/src/tool_runtime/tests/sync_timeout.rs index fd6e6d04..e62fcef3 100644 --- a/src/tool_runtime/tests/sync_timeout.rs +++ b/src/tool_runtime/tests/sync_timeout.rs @@ -265,9 +265,7 @@ async fn dispatched_shared_capture_wait_timeout_reports_outcome_unknown_without_ .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("full cargo_test should be dispatched to the Agent"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request.command, "cargo test"); let active_summary = runtime .sessions @@ -377,9 +375,7 @@ async fn shared_capture_missing_pending_record_reports_outcome_unknown() { .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("capture request should be dispatched"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!( runtime .shell_clients @@ -487,9 +483,7 @@ async fn timeout_rejection_does_not_pollute_validation_summary() { .await } }); - let req = next_patch_agent_request(&runtime, "sync-timeout-ledger") - .await - .expect("valid cargo_check should enqueue"); + let req = wait_for_patch_agent_request(&runtime, "sync-timeout-ledger").await; assert!(req.command.contains("cargo check")); complete_patch_agent_request( &runtime, diff --git a/src/tool_runtime/tests/trusted_smoke.rs b/src/tool_runtime/tests/trusted_smoke.rs index f2ef3405..9f7cc9cf 100644 --- a/src/tool_runtime/tests/trusted_smoke.rs +++ b/src/tool_runtime/tests/trusted_smoke.rs @@ -12,6 +12,7 @@ use serde_json::Value; use sha2::Digest; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; +use std::time::{Duration, Instant}; const CLIENT: &str = "smoke-agent"; @@ -29,7 +30,12 @@ async fn dispatch_with_local_agent( runtime.dispatch_with_auth(call, Some(&bootstrap)).await } }); + let deadline = Instant::now() + Duration::from_secs(10); while !task.is_finished() { + assert!( + Instant::now() < deadline, + "trusted smoke dispatch did not finish within the 10-second test deadline" + ); poll_calls.fetch_add(1, Ordering::SeqCst); let request = runtime .shell_clients @@ -41,7 +47,7 @@ async fn dispatch_with_local_agent( .await .unwrap(); let Some(req) = request else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(Duration::from_millis(2)).await; continue; }; let (exit_code, stdout, stderr) = if req.kind == "file_write_project_file" { diff --git a/src/tool_runtime/tests/validation_events.rs b/src/tool_runtime/tests/validation_events.rs index e0f98cd2..b0d3645a 100644 --- a/src/tool_runtime/tests/validation_events.rs +++ b/src/tool_runtime/tests/validation_events.rs @@ -1053,9 +1053,7 @@ async fn run_shell_declared_validation_enters_unified_summary_with_shell_and_roo .await } }); - let request = next_patch_agent_request(&runtime, "validation-shell") - .await - .expect("run_shell should reach the Agent"); + let request = wait_for_patch_agent_request(&runtime, "validation-shell").await; assert_eq!(request.kind, "run_shell"); assert!(request.command.starts_with("exec bash -c ")); complete_patch_agent_request( @@ -1133,9 +1131,7 @@ async fn completed_run_job_validation_enters_handoff_from_job_authority() { assert_eq!(execution.output["cwd"], "."); assert_eq!(execution.output["shell"], "bash"); let job_id = execution.output["job_id"].as_str().unwrap().to_string(); - let request = next_agent_request_for_client(&runtime, "validation-job") - .await - .expect("run_job should enqueue a start_job request"); + let request = wait_for_agent_request_for_client(&runtime, "validation-job").await; assert_eq!(request.kind, "start_job"); runtime .shell_clients @@ -1294,9 +1290,7 @@ async fn finish_coding_task_validation_available_when_ledger_has_validation_even .await } }); - let req = next_patch_agent_request(&runtime, "validation-finish") - .await - .expect("cargo_check should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "validation-finish").await; assert!(req.command.contains("cargo check --all-targets")); complete_patch_agent_request(&runtime, "validation-finish", &req.request_id, 0, "", "").await; let check = check_task.await.unwrap(); @@ -1331,9 +1325,7 @@ async fn finish_coding_task_validation_available_when_ledger_has_validation_even .await } }); - let req = next_patch_agent_request(&runtime, "validation-finish") - .await - .expect("cargo_test should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "validation-finish").await; assert!(req.command.contains("cargo test")); complete_patch_agent_request( &runtime, @@ -1370,9 +1362,7 @@ async fn finish_coding_task_validation_available_when_ledger_has_validation_even .await } }); - let req = next_patch_agent_request(&runtime, "validation-finish") - .await - .expect("finish_coding_task should inspect changes through the agent"); + let req = wait_for_patch_agent_request(&runtime, "validation-finish").await; assert_internal_posix_script_contains(&req, "git status --porcelain=v1 -b"); complete_patch_agent_request( &runtime, @@ -1479,9 +1469,7 @@ async fn finish_coding_task_validation_available_when_ledger_has_validation_even .await } }); - let req = next_patch_agent_request(&runtime, "validation-finish") - .await - .expect("summary-only finish should inspect changes through the agent"); + let req = wait_for_patch_agent_request(&runtime, "validation-finish").await; complete_patch_agent_request( &runtime, "validation-finish", diff --git a/src/tool_runtime/tests/validation_handoff.rs b/src/tool_runtime/tests/validation_handoff.rs index a95b49fb..c41b6c6e 100644 --- a/src/tool_runtime/tests/validation_handoff.rs +++ b/src/tool_runtime/tests/validation_handoff.rs @@ -27,9 +27,7 @@ async fn poll_start_validation_job( runtime: &ToolRuntime, client_id: &str, ) -> (crate::shell_protocol::ShellAgentShellRequest, String) { - let request = next_patch_agent_request(runtime, client_id) - .await - .expect("structured validation should enqueue a start_validation_job request"); + let request = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(request.kind, "start_validation_job", "{:?}", request.kind); let job_id = request.job_id.clone().expect("start_validation_job job_id"); (request, job_id) @@ -41,7 +39,7 @@ async fn wait_for_agent_request( ) -> crate::shell_protocol::ShellAgentShellRequest { let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); loop { - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { return request; } if tokio::time::Instant::now() >= deadline { @@ -261,7 +259,7 @@ async fn go_test_fails_closed_without_structured_go_capability() { .as_deref() .unwrap_or_default() .contains("structured_go_test_json")); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -310,7 +308,7 @@ async fn go_test_requires_first_class_tool_capability_before_job_reservation() { .unwrap_or_default() .contains("structured_go_test_tool")); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); let summary = runtime @@ -415,7 +413,7 @@ async fn focused_go_test_packages_require_explicit_runner_capability_before_disp .unwrap_or_default() .contains("structured_go_test_packages")); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -463,7 +461,7 @@ async fn go_test_rejects_empty_or_oversized_package_lists_before_dispatch() { assert_eq!(result.output["command_started"], false); } assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -1652,9 +1650,7 @@ async fn explicit_short_timeout_never_creates_a_job() { }); // The short path enqueues a plain `run_shell`-style request (the existing // sync capture), not a structured validation Job. - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("short validation should enqueue a shell request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_ne!(request.kind, "start_validation_job"); complete_sync_shell_lifecycle( &runtime, @@ -1863,7 +1859,7 @@ async fn invalid_cargo_args_fail_before_command_or_agent_request() { ); // No agent request may have been enqueued for the rejected call. assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "{label}: no agent request may be enqueued" @@ -1947,7 +1943,7 @@ async fn cancel_queued_before_handoff_removes_start_request_and_hidden_record() .get_hidden_job_for_auth(None, &job_id) .await .is_err()); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert!(runtime.shell_clients.list_jobs(Some(10)).await.is_empty()); @@ -2022,9 +2018,7 @@ async fn cancel_running_before_handoff_retains_record_until_runner_stops() { if intent_registered { crate::shell_client::recovery_timeout_sweep(&runtime.shell_clients).await; } - let stop = next_patch_agent_request(&runtime, client_id) - .await - .expect("cancellation should enqueue stop_job"); + let stop = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(stop.kind, "stop_job"); assert_eq!(stop.job_id.as_deref(), Some(job_id.as_str())); let hidden = runtime @@ -2381,7 +2375,7 @@ async fn legacy_agent_explicit_120_runs_but_121_rejects_before_start() { assert_eq!(rejected.output["failure_kind"], "capability_unavailable"); assert_eq!(rejected.output["command_started"], false); assert_eq!(rejected.output["command_completed"], false); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); assert_cargo_result_matches_schema("cargo_check", &rejected); @@ -2920,9 +2914,7 @@ async fn cargo_fmt_mutating_never_auto_promotes() { .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("cargo fmt (mutating) should enqueue a shell request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_ne!(request.kind, "start_validation_job"); runtime .shell_clients diff --git a/src/tool_runtime/tests/validation_summary.rs b/src/tool_runtime/tests/validation_summary.rs index e8cfe538..1ac63c82 100644 --- a/src/tool_runtime/tests/validation_summary.rs +++ b/src/tool_runtime/tests/validation_summary.rs @@ -157,7 +157,7 @@ async fn validation_summary_is_guard_safe_read_only_and_does_not_pollute_ledger( .events .is_empty()); assert!( - next_patch_agent_request(&runtime, "validation-summary-safe") + probe_patch_agent_request(&runtime, "validation-summary-safe") .await .is_none() ); diff --git a/src/tool_runtime/tests/work_on_project.rs b/src/tool_runtime/tests/work_on_project.rs index 2cf7d272..ce417127 100644 --- a/src/tool_runtime/tests/work_on_project.rs +++ b/src/tool_runtime/tests/work_on_project.rs @@ -163,7 +163,7 @@ async fn dispatch_recording_startup_requests( std::time::Instant::now() < deadline, "coding startup did not finish within 10 seconds; serviced requests: {request_kinds:?}" ); - let Some(request) = next_patch_agent_request(runtime, client_id).await else { + let Some(request) = probe_patch_agent_request(runtime, client_id).await else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; @@ -223,7 +223,7 @@ async fn dispatch_startup_without_window( std::time::Instant::now() < deadline, "coding startup without window did not finish within 10 seconds for client {client_id}" ); - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { complete_agent_request_by_running_locally(runtime, client_id, request).await; } else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; @@ -255,7 +255,7 @@ async fn dispatch_with_path_runner( std::time::Instant::now() < deadline, "path-based coding call did not finish within 10 seconds for client {client_id}" ); - if let Some(request) = next_patch_agent_request(runtime, client_id).await { + if let Some(request) = probe_patch_agent_request(runtime, client_id).await { if request.kind == "resolve_or_register_project" { let payload: Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); @@ -1107,7 +1107,7 @@ async fn legacy_031_runner_path_source_fails_before_queue_or_session_state() { assert_eq!(result.output["permission"]["status"], "auto_approved"); assert_eq!(result.output["permission"]["tool_name"], "register_project"); assert!( - next_patch_agent_request(&runtime, client_id) + probe_patch_agent_request(&runtime, client_id) .await .is_none(), "legacy Runner received a path-registration request" @@ -1506,7 +1506,7 @@ async fn path_source_cross_project_recording_session_reports_resolved_mismatch() std::time::Instant::now() < deadline, "kernel path bootstrap did not finish within 10 seconds for client {target_client}" ); - if let Some(request) = next_patch_agent_request(&runtime, target_client).await { + if let Some(request) = probe_patch_agent_request(&runtime, target_client).await { if request.kind == "resolve_or_register_project" { let payload: Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); @@ -1651,7 +1651,7 @@ async fn path_source_respects_restricted_authority_before_runner_enqueue() { assert_eq!(result.output["error_kind"], "permission_denied"); assert_eq!(result.output["permission"]["status"], "denied"); assert_eq!(result.output["permission"]["tool_name"], "register_project"); - assert!(next_patch_agent_request(&runtime, client_id) + assert!(probe_patch_agent_request(&runtime, client_id) .await .is_none()); } @@ -2721,7 +2721,7 @@ async fn start_coding_task_standard_repository_unavailable_keeps_session_and_war // No fallback to an arbitrary shell scan: no extra agent request enqueued. assert!( - next_patch_agent_request(&runtime, "wop-nocap") + probe_patch_agent_request(&runtime, "wop-nocap") .await .is_none(), "unavailable overview must not fall back to a shell scan" @@ -2772,7 +2772,7 @@ async fn start_coding_task_standard_repository_overview_timeout_is_nonblocking() std::time::Instant::now() < deadline, "overview-timeout startup did not finish within 10 seconds" ); - let Some(request) = next_patch_agent_request(&runtime, "wop-timeout").await else { + let Some(request) = probe_patch_agent_request(&runtime, "wop-timeout").await else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; @@ -2878,7 +2878,7 @@ async fn dispatch_start_coding_task_with_overview_stdout( std::time::Instant::now() < deadline, "overview startup did not finish within 10 seconds for client {client_id}" ); - let Some(request) = next_patch_agent_request(runtime, client_id).await else { + let Some(request) = probe_patch_agent_request(runtime, client_id).await else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; From 8eb21911dfdad404b8bef9ca8a1a3dbcade1843d Mon Sep 17 00:00:00 2001 From: yyjeqhc <1772413353@qq.com> Date: Sun, 23 Aug 2026 21:51:59 +0800 Subject: [PATCH 4/4] Codify test governance rules --- AGENTS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5120bad7..af1f44da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,9 @@ Product direction: [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md). - Implementation ownership and independent adversarial review are separate passes. In an implementation-owner pass, prioritize a complete authoritative vertical slice and strong first delivery within the current contract; resolve known correctness issues, including concrete authority/identity boundaries created by the feature, but do not fragment the implementation around speculative reviewer concerns. A later review pass independently challenges the resulting design and implementation. - Keep only the interfaces actually affected by the change consistent. Do not touch or revalidate unrelated projections merely because they exist. - Add focused tests for changed behavior when practical. Update documentation when public behavior or operations change. +- When a subsystem already has a dedicated `tests/` module tree, put ordinary new tests there instead of growing production facade files. Keep inline `#[cfg(test)]` blocks small and tightly coupled to private implementation helpers; process, network, and integration fixtures belong in dedicated test modules. +- Do not grow one test file into a multi-domain catch-all. When an already-large test module needs coverage for a distinct lifecycle or contract domain, create or reuse a domain-specific test module and split by canonical ownership, not arbitrary line-count chunks. +- In async tests, required readiness must use a `wait_*` path with one absolute deadline that partial progress never resets. `probe_*` helpers are only for observations where immediate absence is valid or for one iteration inside an already-owned outer deadline; never use a probe when absence means test failure. - Ask only when required information cannot be discovered, instructions materially conflict, or proceeding could destroy work. Otherwise continue and report any material deviation. ## 4. Validate only changed behavior