From d7c4396cd31b5f0227e07b49cb0a632363ac6873 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 26 Jun 2026 04:45:22 +0900 Subject: [PATCH] fix: scope workspace worker lookup by runtime --- crates/workspace-server/src/hosts.rs | 229 ++++++++++++++++++++++++-- crates/workspace-server/src/lib.rs | 9 +- crates/workspace-server/src/server.rs | 3 +- 3 files changed, 226 insertions(+), 15 deletions(-) diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index f576cda2..d09ed546 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -305,9 +305,16 @@ pub struct WorkerProxyConnectPoint { #[derive(Debug, Clone, PartialEq, Eq)] pub enum RuntimeRegistryError { - InvalidIdentifier { kind: &'static str, value: String }, + InvalidIdentifier { + kind: &'static str, + value: String, + }, + UnknownRuntime(String), UnknownHost(String), - UnknownWorker(String), + UnknownWorker { + runtime_id: String, + worker_id: String, + }, } impl RuntimeRegistryError { @@ -317,8 +324,15 @@ impl RuntimeRegistryError { kind: kind.to_string(), value, }, + Self::UnknownRuntime(runtime_id) => Error::UnknownRuntime(runtime_id), Self::UnknownHost(host_id) => Error::UnknownHost(host_id), - Self::UnknownWorker(worker_id) => Error::UnknownWorker(worker_id), + Self::UnknownWorker { + runtime_id, + worker_id, + } => Error::UnknownWorker { + runtime_id, + worker_id, + }, } } } @@ -474,15 +488,25 @@ impl RuntimeRegistry { } } - pub fn worker(&self, worker_id: &str) -> Result { + pub fn worker( + &self, + runtime_id: &str, + worker_id: &str, + ) -> Result { + validate_backend_identifier("runtime_id", runtime_id)?; validate_backend_identifier("worker_id", worker_id)?; - for runtime in &self.runtimes { - let lookup = runtime.worker(worker_id); - if let Some(worker) = lookup.worker { - return Ok(worker); - } - } - Err(RuntimeRegistryError::UnknownWorker(worker_id.to_string())) + let runtime = self + .runtimes + .iter() + .find(|runtime| runtime.runtime_id() == runtime_id) + .ok_or_else(|| RuntimeRegistryError::UnknownRuntime(runtime_id.to_string()))?; + let lookup = runtime.worker(worker_id); + lookup + .worker + .ok_or_else(|| RuntimeRegistryError::UnknownWorker { + runtime_id: runtime_id.to_string(), + worker_id: worker_id.to_string(), + }) } } @@ -1025,6 +1049,7 @@ mod tests { use super::*; use serde_json::json; use std::fs; + use std::sync::Arc; use tempfile::TempDir; fn write_metadata(dir: &Path, worker_name: &str, metadata: &WorkerMetadata) { @@ -1056,6 +1081,184 @@ mod tests { host_id_for_workspace("local:test") } + #[derive(Clone)] + struct FixtureRuntime { + runtime_id: String, + host_id: String, + workers: Vec, + } + + impl FixtureRuntime { + fn with_worker(runtime_id: &str, host_id: &str, worker_id: &str, label: &str) -> Self { + Self { + runtime_id: runtime_id.to_string(), + host_id: host_id.to_string(), + workers: vec![WorkerSummary { + runtime_id: runtime_id.to_string(), + worker_id: worker_id.to_string(), + host_id: host_id.to_string(), + label: label.to_string(), + role: None, + profile: None, + workspace: WorkerWorkspaceSummary { + visibility: "opaque".to_string(), + identity: host_id.to_string(), + }, + state: "running".to_string(), + status: "available".to_string(), + last_seen_at: None, + implementation: WorkerImplementationSummary { + kind: "fixture".to_string(), + display_hint: "test fixture".to_string(), + }, + capabilities: WorkerCapabilitySummary { + can_accept_input: false, + can_stream_events: false, + can_stop: false, + can_spawn_followup: false, + can_read_bounded_transcript: false, + }, + diagnostics: Vec::new(), + }], + } + } + } + + impl WorkspaceWorkerRuntime for FixtureRuntime { + fn runtime_id(&self) -> &str { + &self.runtime_id + } + + fn runtime_summary(&self, _limit: usize) -> RuntimeSummary { + RuntimeSummary { + runtime_id: self.runtime_id.clone(), + label: self.runtime_id.clone(), + kind: "fixture".to_string(), + status: "available".to_string(), + source: RuntimeSourceSummary::embedded_worker_runtime_reserved(), + host_ids: vec![self.host_id.clone()], + capabilities: RuntimeCapabilitySummary { + can_list_hosts: true, + can_list_workers: true, + can_get_worker: true, + can_spawn_worker: false, + can_stop_worker: false, + can_accept_input: false, + can_stream_events: false, + can_read_bounded_transcript: false, + has_workspace_fs: false, + has_shell: false, + has_git: false, + supports_worktrees: false, + supports_backend_internal_tools: false, + local_pod_inspection: "none".to_string(), + workspace_scope: "none".to_string(), + max_workers: self.workers.len(), + os: "test".to_string(), + arch: "test".to_string(), + }, + diagnostics: Vec::new(), + } + } + + fn list_hosts(&self, _limit: usize) -> RuntimeList { + RuntimeList::new( + vec![HostSummary { + runtime_id: self.runtime_id.clone(), + host_id: self.host_id.clone(), + label: "fixture host".to_string(), + kind: "fixture".to_string(), + status: "available".to_string(), + observed_at: "unknown".to_string(), + last_seen_at: None, + capabilities: self.runtime_summary(1).capabilities, + diagnostics: Vec::new(), + }], + Vec::new(), + ) + } + + fn list_workers(&self, limit: usize) -> RuntimeList { + RuntimeList::new( + self.workers.iter().take(limit).cloned().collect(), + Vec::new(), + ) + } + + fn worker(&self, worker_id: &str) -> WorkerLookupResult { + WorkerLookupResult { + worker: self + .workers + .iter() + .find(|worker| worker.worker_id == worker_id) + .cloned(), + diagnostics: Vec::new(), + } + } + } + + #[test] + fn registry_worker_lookup_is_scoped_by_runtime_id() { + let registry = RuntimeRegistry::new(vec![ + Arc::new(FixtureRuntime::with_worker( + "runtime-a", + "host-a", + "shared-worker", + "worker from runtime a", + )), + Arc::new(FixtureRuntime::with_worker( + "runtime-b", + "host-b", + "shared-worker", + "worker from runtime b", + )), + ]); + + let from_runtime_b = registry.worker("runtime-b", "shared-worker").unwrap(); + assert_eq!(from_runtime_b.runtime_id, "runtime-b"); + assert_eq!(from_runtime_b.host_id, "host-b"); + assert_eq!(from_runtime_b.label, "worker from runtime b"); + + let from_runtime_a = registry.worker("runtime-a", "shared-worker").unwrap(); + assert_eq!(from_runtime_a.runtime_id, "runtime-a"); + assert_eq!(from_runtime_a.host_id, "host-a"); + assert_eq!(from_runtime_a.label, "worker from runtime a"); + } + + #[test] + fn registry_worker_lookup_reports_unknown_runtime_and_worker_separately() { + let registry = RuntimeRegistry::new(vec![Arc::new(FixtureRuntime::with_worker( + "runtime-a", + "host-a", + "worker-a", + "worker from runtime a", + ))]); + + let unknown_runtime = registry.worker("runtime-missing", "worker-a").unwrap_err(); + assert_eq!( + unknown_runtime, + RuntimeRegistryError::UnknownRuntime("runtime-missing".to_string()) + ); + assert!(matches!( + unknown_runtime.into_error(), + Error::UnknownRuntime(runtime_id) if runtime_id == "runtime-missing" + )); + + let unknown_worker = registry.worker("runtime-a", "worker-missing").unwrap_err(); + assert_eq!( + unknown_worker, + RuntimeRegistryError::UnknownWorker { + runtime_id: "runtime-a".to_string(), + worker_id: "worker-missing".to_string(), + } + ); + assert!(matches!( + unknown_worker.into_error(), + Error::UnknownWorker { runtime_id, worker_id } + if runtime_id == "runtime-a" && worker_id == "worker-missing" + )); + } + #[test] fn local_runtime_reports_host_without_private_paths() { let bridge = LocalWorkerRuntime::new("local:test", "/workspace/project", None); @@ -1222,7 +1425,9 @@ mod tests { assert!(!worker.worker_id.contains('@')); assert!(!worker.worker_id.contains('#')); - let from_registry = registry.worker(&worker.worker_id).unwrap(); + let from_registry = registry + .worker(&worker.runtime_id, &worker.worker_id) + .unwrap(); assert_eq!(from_registry.worker_id, worker.worker_id); let from_runtime = bridge.worker(&worker.worker_id).worker.unwrap(); assert_eq!(from_runtime.worker_id, worker.worker_id); diff --git a/crates/workspace-server/src/lib.rs b/crates/workspace-server/src/lib.rs index 7a804518..817c6f1e 100644 --- a/crates/workspace-server/src/lib.rs +++ b/crates/workspace-server/src/lib.rs @@ -40,8 +40,13 @@ pub enum Error { MissingFrontmatter(String), #[error("unknown local host `{0}`")] UnknownHost(String), - #[error("unknown local worker `{0}`")] - UnknownWorker(String), + #[error("unknown runtime `{0}`")] + UnknownRuntime(String), + #[error("unknown worker `{worker_id}` in runtime `{runtime_id}`")] + UnknownWorker { + runtime_id: String, + worker_id: String, + }, #[error("invalid runtime {kind} `{value}`")] InvalidRuntimeIdentifier { kind: String, value: String }, #[error("runtime `{runtime_id}` does not support `{capability}`")] diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 32e74bbf..983f8f9d 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -611,7 +611,8 @@ impl IntoResponse for ApiError { Error::InvalidRecordId(_) | Error::MissingFrontmatter(_) | Error::UnknownHost(_) - | Error::UnknownWorker(_) + | Error::UnknownRuntime(_) + | Error::UnknownWorker { .. } | Error::UnknownRepository(_) => StatusCode::NOT_FOUND, Error::Ticket(_) => StatusCode::NOT_FOUND, Error::RuntimeCapabilityUnsupported { .. } => StatusCode::NOT_IMPLEMENTED,