From b4099ddda4a35a7a149df97feadea3872eebd24a Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 7 Nov 2026 09:25:42 +0900 Subject: [PATCH] fix: wait for remote workdir cleanup confirmation --- crates/workspace-server/src/hosts.rs | 36 ++++--- .../src/hosts/tests/workdir_cleanup_tests.rs | 95 +++++++++++++++++++ 2 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 crates/workspace-server/src/hosts/tests/workdir_cleanup_tests.rs diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index 7f7ac29d2..7b68eac3e 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -4194,6 +4194,7 @@ pub struct RemoteWorkerRuntime { host_id: String, resource_broker: BackendResourceBroker, http: BlockingHttpClient, + workdir_cleanup_http: BlockingHttpClient, async_http: AsyncHttpClient, request_timeout: Duration, } @@ -4346,15 +4347,20 @@ impl RemoteWorkerRuntime { }; let timeout = config.timeout; let blocking_resolution = pinned_endpoint.clone(); - let http = run_blocking_http(move || { - let mut builder = BlockingHttpClient::builder() - .timeout(timeout) - .redirect(reqwest::redirect::Policy::none()) - .no_proxy(); - if let Some((host, addresses)) = &blocking_resolution { - builder = builder.resolve_to_addrs(host, addresses); - } - builder.build() + let (http, workdir_cleanup_http) = run_blocking_http(move || { + let build_client = |timeout: Option| { + let mut builder = BlockingHttpClient::builder() + .timeout(timeout) + .redirect(reqwest::redirect::Policy::none()) + .no_proxy(); + if let Some((host, addresses)) = &blocking_resolution { + builder = builder.resolve_to_addrs(host, addresses); + } + builder.build() + }; + // Cleanup duration depends on the retained tree. A response deadline + // does not cancel Runtime deletion, so it must not bound this wait. + Ok::<_, reqwest::Error>((build_client(Some(timeout))?, build_client(None)?)) }) .map_err(|err| RuntimeRegistryError::RuntimeOperationFailed { runtime_id: config.runtime_id.clone(), @@ -4393,6 +4399,7 @@ impl RemoteWorkerRuntime { cached_status: config.cached_status, resource_broker: BackendResourceBroker::default(), http, + workdir_cleanup_http, async_http, request_timeout: timeout, }) @@ -5325,9 +5332,13 @@ impl WorkspaceWorkerRuntime for RemoteWorkerRuntime { &self, working_directory_id: &str, ) -> RuntimeWorkingDirectoryResult { - match self.delete_json::(&format!( - "/v1/working-directories/{working_directory_id}" - )) { + let path = format!("/v1/working-directories/{working_directory_id}"); + match self.send_json::( + &path, + "DELETE", + &[], + self.workdir_cleanup_http.delete(self.endpoint(&path)), + ) { Ok(response) => RuntimeWorkingDirectoryResult { state: InternalWorkerOperationState::Accepted, working_directory: Some(response.working_directory), @@ -6828,6 +6839,7 @@ pub fn placeholder_spawn_response(host_id: impl Into) -> WorkerSpawnResu #[cfg(test)] mod tests { mod restore_auth_tests; + mod workdir_cleanup_tests; mod workspace_auth_tests; use super::*; diff --git a/crates/workspace-server/src/hosts/tests/workdir_cleanup_tests.rs b/crates/workspace-server/src/hosts/tests/workdir_cleanup_tests.rs new file mode 100644 index 000000000..8320e00e6 --- /dev/null +++ b/crates/workspace-server/src/hosts/tests/workdir_cleanup_tests.rs @@ -0,0 +1,95 @@ +//! Exercises the production RemoteWorkerRuntime HTTP client against a gated loopback +//! provider. Only the provider is replaced; no live Runtime or Workdir is used. +//! Runs with the ordinary workspace-server crate tests. + +use super::*; +use axum::{Json, Router, routing::delete}; +use tokio::{sync::Notify, task::JoinSet, time::timeout}; + +#[tokio::test] +async fn workdir_cleanup_waits_past_ordinary_request_timeout_for_provider_confirmation() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let runtime = RemoteWorkerRuntime::new( + RemoteRuntimeConfig::new( + "runtime-test", + "Test Runtime", + format!("http://{}", listener.local_addr().unwrap()), + Some("secret-token".into()), + ) + .with_timeout(Duration::from_millis(100)), + "workspace-test".into(), + "http://127.0.0.1:1".into(), + ) + .unwrap(); + let entered = Arc::new(Notify::new()); + let release = Arc::new(Notify::new()); + let provider_entered = entered.clone(); + let provider_release = release.clone(); + let app = Router::new().route( + "/v1/working-directories/workdir-test", + delete(move |headers: axum::http::HeaderMap| async move { + assert_eq!(headers[AUTHORIZATION], "Bearer secret-token"); + assert_eq!(headers["x-yoi-workspace-id"], "workspace-test"); + provider_entered.notify_one(); + // The fixture's deadline only bounds test failure/teardown. Completion + // is controlled by the test after an ordinary request has timed out. + let _ = timeout(Duration::from_secs(5), provider_release.notified()).await; + Json(json!({ + "working_directory": { "summary": { + "working_directory_id": "workdir-test", + "repository_id": "main", + "materializer_kind": "runtime_git_clone", + "status": "not_found" + }} + })) + }) + .get(|| async { + let _ = timeout(Duration::from_secs(5), std::future::pending::<()>()).await; + Json(json!({})) + }), + ); + let mut server = JoinSet::new(); + server.spawn(async move { axum::serve(listener, app).await.unwrap() }); + let cleanup_runtime = runtime.clone(); + let cleanup = tokio::task::spawn_blocking(move || { + cleanup_runtime.cleanup_working_directory("workdir-test") + }); + timeout(Duration::from_secs(5), entered.notified()) + .await + .expect("provider received cleanup"); + + let ordinary = timeout( + Duration::from_secs(5), + tokio::task::spawn_blocking(move || runtime.working_directory("workdir-test")), + ) + .await + .expect("ordinary request remains bounded") + .unwrap(); + let cleanup_still_waiting = !cleanup.is_finished(); + release.notify_one(); + let removed = timeout(Duration::from_secs(5), cleanup) + .await + .expect("cleanup receives provider confirmation") + .unwrap(); + server.shutdown().await; + + assert_eq!(ordinary.state, InternalWorkerOperationState::Rejected); + assert!( + ordinary + .diagnostics + .iter() + .any(|d| d.code == "remote_runtime_timeout") + ); + assert!( + cleanup_still_waiting, + "cleanup must not inherit the ordinary timeout" + ); + assert_eq!(removed.state, InternalWorkerOperationState::Accepted); + assert!(removed.diagnostics.is_empty()); + let summary = removed.working_directory.unwrap().summary; + assert_eq!(summary.working_directory_id, "workdir-test"); + assert_eq!( + summary.status, + workdir::workspace::WorkingDirectoryStatusKind::NotFound + ); +}