fix: confirm remote Workdir close before releasing session ownership

crates/workdir/src/http.rs

@@ -556,6 +556,7 @@ mod client {
         capabilities: WorkdirSessionCapabilities,
         environment: crate::WorkdirSessionEnvironment,
         closed: AtomicBool,
+        close_lock: tokio::sync::Mutex<()>,
     }
 
     impl RemoteWorkdirSession {
@@ -616,6 +617,7 @@ mod client {
                 capabilities: opened.capabilities,
                 environment: opened.environment,
                 closed: AtomicBool::new(false),
+                close_lock: tokio::sync::Mutex::new(()),
             })
         }
 
@@ -1014,7 +1016,11 @@ mod client {
         }
 
         async fn close(&self) -> Result<(), WorkdirError> {
-            if self.closed.swap(true, Ordering::AcqRel) {
+            // In-flight is not closed. Dropping this guard on error or cancellation
+            // leaves confirmation unset, so the next caller can retry Runtime's
+            // idempotent DELETE even when the previous response was lost.
+            let _close = self.close_lock.lock().await;
+            if self.closed.load(Ordering::Acquire) {
                 return Ok(());
             }
             let url = endpoint(
@@ -1033,6 +1039,7 @@ mod client {
                 .await
                 .map_err(http_unavailable)?;
             if response.status() == StatusCode::NO_CONTENT || response.status().is_success() {
+                self.closed.store(true, Ordering::Release);
                 Ok(())
             } else {
                 Err(decode_error(response).await)
@@ -1211,7 +1218,7 @@ mod client {
 
             // Seed an already-open session to exercise the public operation and
             // close boundaries independently of the rejected open request.
-            let remote = RemoteWorkdirSession {
+            let mut remote = RemoteWorkdirSession {
                 client,
                 base_url,
                 authorization: authorization.clone(),
@@ -1222,6 +1229,7 @@ mod client {
                     WorkdirSessionCapabilities::EMPTY,
                 ),
                 closed: AtomicBool::new(false),
+                close_lock: tokio::sync::Mutex::new(()),
             };
             let stat = StatRequest {
                 path: crate::WorkdirPath::new("file").unwrap(),
@@ -1263,6 +1271,15 @@ mod client {
                 listener.accept().unwrap_err().kind(),
                 std::io::ErrorKind::WouldBlock
             );
+            // An invalid endpoint cannot be produced by a successful open, but
+            // must not turn a pre-dispatch error into a confirmed close either.
+            remote.base_url = Url::parse("data:invalid-base").unwrap();
+            for _ in 0..2 {
+                assert!(matches!(
+                    remote.close().await,
+                    Err(WorkdirError::InvalidArgument(_))
+                ));
+            }
         }
     }
 

crates/workdir/tests/remote_close.rs

@@ -0,0 +1,287 @@
+#![cfg(feature = "http-client")]
+//! Remote close protocol tests with a loopback HTTP boundary. Requests and responses
+//! are explicitly gated; Runtime lease integration belongs to workspace-server.
+use std::{
+    future::Future,
+    sync::{
+        Arc,
+        atomic::{AtomicBool, Ordering},
+    },
+    task::Poll,
+    time::Duration,
+};
+use tokio::{
+    io::{AsyncReadExt, AsyncWriteExt},
+    net::{TcpListener, TcpStream},
+    sync::mpsc,
+};
+use workdir::{
+    Workdir, WorkdirError, WorkdirSession,
+    http::{
+        OpenWorkdirSessionRequest, OpenWorkdirSessionResponse, RemoteWorkdirSession,
+        WorkdirHttpAuthorization, WorkdirHttpAuthorizationFuture, WorkdirSessionId,
+    },
+};
+
+struct Provider {
+    url: reqwest::Url,
+    requests: mpsc::UnboundedReceiver<TcpStream>,
+    task: tokio::task::JoinHandle<()>,
+}
+impl Drop for Provider {
+    fn drop(&mut self) {
+        self.task.abort();
+    }
+}
+impl Provider {
+    async fn new() -> Self {
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let url = format!("http://{}", listener.local_addr().unwrap())
+            .parse()
+            .unwrap();
+        let (tx, requests) = mpsc::unbounded_channel();
+        let task = tokio::spawn(async move {
+            loop {
+                let (mut stream, _) = listener.accept().await.unwrap();
+                let mut bytes = Vec::new();
+                while !bytes.ends_with(b"\r\n\r\n") {
+                    bytes.push(stream.read_u8().await.unwrap());
+                    assert!(bytes.len() < 16384);
+                }
+                let headers = String::from_utf8(bytes).unwrap();
+                let length = headers
+                    .lines()
+                    .find_map(|line| {
+                        let (name, value) = line.split_once(':')?;
+                        name.eq_ignore_ascii_case("content-length")
+                            .then(|| value.trim().parse::<usize>().unwrap())
+                    })
+                    .unwrap_or(0);
+                assert!(length < 16384);
+                stream.read_exact(&mut vec![0; length]).await.unwrap();
+                assert!(
+                    headers.starts_with("POST /v1/working-directories/w/sessions ")
+                        || headers.starts_with("DELETE /v1/workdir-sessions/s ")
+                );
+                if tx.send(stream).is_err() {
+                    break;
+                }
+            }
+        });
+        Self {
+            url,
+            requests,
+            task,
+        }
+    }
+    async fn request_while<F: Future>(&mut self, future: std::pin::Pin<&mut F>) -> TcpStream {
+        tokio::time::timeout(Duration::from_secs(5), async {
+            tokio::select! {
+                request = self.requests.recv() => request.expect("provider stopped"),
+                _ = future => panic!("close/open completed without the expected HTTP request"),
+            }
+        })
+        .await
+        .expect("request timed out")
+    }
+    async fn open(&mut self, auth: Arc<dyn WorkdirHttpAuthorization>) -> RemoteWorkdirSession {
+        let future = RemoteWorkdirSession::open_with_authorization(
+            reqwest::Client::builder()
+                .no_proxy()
+                .timeout(Duration::from_secs(5))
+                .build()
+                .unwrap(),
+            self.url.clone(),
+            auth,
+            Workdir::new("w").id().clone(),
+            OpenWorkdirSessionRequest::default(),
+        );
+        tokio::pin!(future);
+        let stream = self.request_while(future.as_mut()).await;
+        let body = serde_json::to_string(&OpenWorkdirSessionResponse {
+            session_id: WorkdirSessionId::new("s").unwrap(),
+            workdir_id: Workdir::new("w").id().clone(),
+            capabilities: workdir::WorkdirSessionCapabilities::ALL,
+            environment: workdir::WorkdirSessionEnvironment::root(
+                workdir::WorkdirSessionCapabilities::ALL,
+            ),
+        })
+        .unwrap();
+        reply(stream, "200 OK", &body).await;
+        future.await.unwrap()
+    }
+}
+async fn reply(mut stream: TcpStream, status: &str, body: &str) {
+    stream.write_all(format!("HTTP/1.1 {status}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).as_bytes()).await.unwrap();
+}
+#[derive(Debug, Default)]
+struct Authorization {
+    reject_delete: AtomicBool,
+}
+impl WorkdirHttpAuthorization for Authorization {
+    fn bearer_token(
+        &self,
+        method: String,
+        _: String,
+        _: Vec<u8>,
+    ) -> WorkdirHttpAuthorizationFuture {
+        let reject = method == "DELETE" && self.reject_delete.swap(false, Ordering::SeqCst);
+        Box::pin(async move {
+            if reject {
+                Err(WorkdirError::Unavailable(
+                    "fixture authorization rejected".into(),
+                ))
+            } else {
+                Ok("fixture-token".into())
+            }
+        })
+    }
+}
+
+#[tokio::test]
+async fn close_authorization_failure_retries_same_session_and_confirmed_close_skips_http() {
+    let mut provider = Provider::new().await;
+    let auth = Arc::new(Authorization::default());
+    let remote = provider.open(auth.clone()).await;
+    auth.reject_delete.store(true, Ordering::SeqCst);
+    assert!(remote.close().await.is_err());
+    assert!(provider.requests.try_recv().is_err());
+    let retry = remote.close();
+    tokio::pin!(retry);
+    let request = provider.request_while(retry.as_mut()).await;
+    reply(request, "204 No Content", "").await;
+    retry.await.unwrap();
+    remote.close().await.unwrap();
+    assert!(provider.requests.try_recv().is_err());
+}
+
+#[derive(Debug)]
+struct GatedAuthorization {
+    entered: mpsc::UnboundedSender<()>,
+    release: std::sync::Mutex<Option<tokio::sync::oneshot::Receiver<()>>>,
+}
+impl WorkdirHttpAuthorization for GatedAuthorization {
+    fn bearer_token(
+        &self,
+        method: String,
+        _: String,
+        _: Vec<u8>,
+    ) -> WorkdirHttpAuthorizationFuture {
+        let gate = if method == "DELETE" {
+            self.release.lock().unwrap().take()
+        } else {
+            None
+        };
+        let entered = self.entered.clone();
+        Box::pin(async move {
+            if let Some(gate) = gate {
+                entered.send(()).unwrap();
+                gate.await.unwrap();
+            }
+            Ok("fixture-token".into())
+        })
+    }
+}
+
+#[tokio::test]
+async fn close_cancel_during_authorization_allows_an_actual_retry() {
+    let mut provider = Provider::new().await;
+    let (entered, mut events) = mpsc::unbounded_channel();
+    let (_release, gate) = tokio::sync::oneshot::channel();
+    let remote = provider
+        .open(Arc::new(GatedAuthorization {
+            entered,
+            release: std::sync::Mutex::new(Some(gate)),
+        }))
+        .await;
+    let mut first = Box::pin(remote.close());
+    tokio::time::timeout(Duration::from_secs(5), async {
+        tokio::select! {
+            _ = events.recv() => (),
+            _ = &mut first => panic!("close completed while authorization was gated"),
+        }
+    })
+    .await
+    .unwrap();
+    drop(first);
+    assert!(provider.requests.try_recv().is_err());
+    let retry = remote.close();
+    tokio::pin!(retry);
+    let request = provider.request_while(retry.as_mut()).await;
+    reply(request, "204 No Content", "").await;
+    retry.await.unwrap();
+}
+
+#[tokio::test]
+async fn close_http_rejection_or_lost_response_requires_another_delete() {
+    for response in [
+        Some((
+            "503 Service Unavailable",
+            r#"{"code":"unavailable","message":"fixture"}"#,
+        )),
+        None,
+    ] {
+        let mut provider = Provider::new().await;
+        let remote = provider.open(Arc::new(Authorization::default())).await;
+        let first = remote.close();
+        tokio::pin!(first);
+        let request = provider.request_while(first.as_mut()).await;
+        if let Some((status, body)) = response {
+            reply(request, status, body).await;
+        } else {
+            drop(request);
+        } // Runtime may have deleted; no response confirms it.
+        assert!(first.await.is_err());
+        let retry = remote.close();
+        tokio::pin!(retry);
+        let request = provider.request_while(retry.as_mut()).await;
+        reply(request, "204 No Content", "").await;
+        retry.await.unwrap();
+    }
+}
+
+#[tokio::test]
+async fn close_cancel_releases_serialization_and_waiter_must_confirm_delete() {
+    let mut provider = Provider::new().await;
+    let remote = provider.open(Arc::new(Authorization::default())).await;
+    let mut first = Box::pin(remote.close());
+    let request = provider.request_while(first.as_mut()).await;
+    let mut waiter = Box::pin(remote.close());
+    assert!(
+        std::future::poll_fn(|cx| Poll::Ready(waiter.as_mut().poll(cx)))
+            .await
+            .is_pending()
+    );
+    drop(first);
+    drop(request);
+    let request = provider.request_while(waiter.as_mut()).await;
+    reply(request, "204 No Content", "").await;
+    waiter.await.unwrap();
+}
+
+#[tokio::test]
+async fn concurrent_close_waits_for_confirmation_and_retries_after_failure() {
+    for success in [false, true] {
+        let mut provider = Provider::new().await;
+        let remote = provider.open(Arc::new(Authorization::default())).await;
+        let mut first = Box::pin(remote.close());
+        let request = provider.request_while(first.as_mut()).await;
+        let mut waiter = Box::pin(remote.close());
+        assert!(
+            std::future::poll_fn(|cx| Poll::Ready(waiter.as_mut().poll(cx)))
+                .await
+                .is_pending()
+        );
+        if success {
+            reply(request, "204 No Content", "").await;
+            first.await.unwrap();
+        } else {
+            drop(request);
+            assert!(first.await.is_err());
+            let request = provider.request_while(waiter.as_mut()).await;
+            reply(request, "204 No Content", "").await;
+        }
+        waiter.await.unwrap();
+        assert!(provider.requests.try_recv().is_err());
+    }
+}