From 816fa96e0784e22ee26581a79aa5c227051b0f65 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 1 Sep 2026 20:41:37 +0900 Subject: [PATCH] fix: restore workers with remote workdir attachments --- crates/client/src/backend_api.rs | 71 +++++++++++++++++++++++++++ crates/client/src/backend_runtime.rs | 2 +- crates/workspace-server/src/server.rs | 49 +++++++++++++++--- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/crates/client/src/backend_api.rs b/crates/client/src/backend_api.rs index be4c7519..7e96d189 100644 --- a/crates/client/src/backend_api.rs +++ b/crates/client/src/backend_api.rs @@ -192,6 +192,32 @@ impl BackendApiClient { format!("Bearer {}", self.access_token.0) } + pub async fn require_success( + &self, + response: reqwest::Response, + ) -> Result { + let status = response.status(); + match status { + StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => { + self.check_status(status)?; + } + status if !status.is_success() => { + let detail = response + .bytes() + .await + .ok() + .and_then(|body| backend_error_detail(&body)); + return Err(BackendApiClientError::BackendResponse { + origin: self.origin.clone(), + status: status.as_u16(), + detail, + }); + } + _ => {} + } + Ok(response) + } + pub fn check_status(&self, status: StatusCode) -> Result<(), BackendApiClientError> { match status { StatusCode::UNAUTHORIZED => Err(BackendApiClientError::Unauthorized { @@ -235,6 +261,18 @@ fn redirect_policy(origin: BackendOrigin) -> redirect::Policy { }) } +#[derive(Deserialize)] +struct BackendErrorBody { + message: String, +} + +fn backend_error_detail(body: &[u8]) -> Option { + serde_json::from_slice::(body) + .ok() + .map(|body| body.message) + .filter(|message| !message.trim().is_empty()) +} + #[derive(Debug)] pub enum BackendApiClientError { InvalidBackendOrigin(String), @@ -266,6 +304,11 @@ pub enum BackendApiClientError { origin: BackendOrigin, status: u16, }, + BackendResponse { + origin: BackendOrigin, + status: u16, + detail: Option, + }, Io { path: PathBuf, source: std::io::Error, @@ -312,6 +355,17 @@ impl fmt::Display for BackendApiClientError { Self::BackendStatus { origin, status } => { write!(f, "Backend {origin} returned HTTP {status}") } + Self::BackendResponse { + origin, + status, + detail, + } => { + write!(f, "Backend {origin} returned HTTP {status}")?; + if let Some(detail) = detail { + write!(f, ": {detail}")?; + } + Ok(()) + } Self::Io { path, source } => { write!(f, "failed to access {}: {source}", path.display()) } @@ -584,6 +638,23 @@ mod tests { ); } + #[test] + fn backend_error_detail_preserves_public_server_message() { + let detail = backend_error_detail( + br#"{"error":"Bad Request","message":"working_directory_runtime_mismatch: Working directory is owned by a different Runtime","diagnostics":[{"code":"working_directory_runtime_mismatch"}]}"#, + ); + let error = BackendApiClientError::BackendResponse { + origin: BackendOrigin::parse("http://127.0.0.1:8787").unwrap(), + status: 400, + detail, + }; + + assert_eq!( + error.to_string(), + "Backend http://127.0.0.1:8787 returned HTTP 400: working_directory_runtime_mismatch: Working directory is owned by a different Runtime" + ); + } + #[test] fn backend_origin_rejects_unsafe_authority_changes() { for invalid in [ diff --git a/crates/client/src/backend_runtime.rs b/crates/client/src/backend_runtime.rs index 01776566..4f118ff4 100644 --- a/crates/client/src/backend_runtime.rs +++ b/crates/client/src/backend_runtime.rs @@ -269,7 +269,7 @@ pub async fn restore_backend_worker( .json(&serde_json::json!({})) .send() .await?; - api.check_status(response.status())?; + let response = api.require_success(response).await?; Ok(response.json::().await?) } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index e043a97d..12158e15 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1909,16 +1909,18 @@ impl WorkspaceApi { .list_worker_workdir_links(&self.config.workspace_id, worker)? .into_iter() .find(|link| link.unlinked_at.is_none()) - && let Some(access) = repository_access_request_for_workdir( + { + let workdir_runtime_id = registered_workdir_runtime_id(self, &link.workdir_id)?; + if let Some(access) = repository_access_request_for_workdir( self, - &worker.runtime_id, + &workdir_runtime_id, &link.workdir_id, &format!("worker-restore:{}", WorkerId::now_v7()), - )? - { - self.runtime - .authorize_working_directory_repository_access(&worker.runtime_id, access) - .map_err(RuntimeRegistryError::into_error)?; + )? { + self.runtime + .authorize_working_directory_repository_access(&workdir_runtime_id, access) + .map_err(RuntimeRegistryError::into_error)?; + } } let binding = self .runtime @@ -25010,7 +25012,7 @@ mod tests { ) .await .unwrap(); - let app = build_inner_router(api); + let app = build_inner_router(api.clone()); let runtimes = get_json(app.clone(), "/api/runtimes").await; let embedded_summary = runtimes["items"] @@ -25065,6 +25067,37 @@ mod tests { "embedded_worker_runtime" ); + let workdir_id = "external-workdir"; + api.store + .upsert_workdir_registry(&WorkdirRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + workdir_id: workdir_id.to_string(), + runtime_id: "external-workdir-runtime".to_string(), + repository_id: "main".to_string(), + creation_selector: None, + creation_ref: None, + creation_tree: None, + current_selector: None, + current_ref: None, + current_tree: None, + observed_at_epoch_seconds: None, + materialization_status: "present".to_string(), + cleanliness: "clean".to_string(), + created_at: "1".to_string(), + updated_at: "1".to_string(), + }) + .unwrap(); + api.store + .attach_worker_workdir(&WorkerWorkdirLinkRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: RuntimeWorkerRef::new("embedded-worker-runtime", &worker_id), + workdir_id: workdir_id.to_string(), + role: "attachment".to_string(), + linked_at: "2".to_string(), + unlinked_at: None, + }) + .unwrap(); + let worker = get_json( app.clone(), &format!("/api/runtimes/embedded-worker-runtime/workers/{worker_id}"),