diff --git a/crates/worker/src/feature/builtin/manage_worker.rs b/crates/worker/src/feature/builtin/manage_worker.rs index 1ea1950de..1f9ced49b 100644 --- a/crates/worker/src/feature/builtin/manage_worker.rs +++ b/crates/worker/src/feature/builtin/manage_worker.rs @@ -5,10 +5,14 @@ use std::sync::Arc; use agen::tool::{Tool, ToolDefinition, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; use async_trait::async_trait; use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use uuid::Uuid; use protocol::Segment; +use server_api::{ + BrowserWorkerWorkingDirectorySelection, CreateWorkspaceWorkerRequest, + CreateWorkspaceWorkerTicketAssignmentRequest, +}; use crate::feature::{ FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureModule, @@ -27,6 +31,7 @@ const FEATURE_DESCRIPTION: &str = pub const WORKER_LIFECYCLE_SERVICE_ID: &str = "worker.lifecycle"; pub const WORKER_CONTROL_SERVICE_ID: &str = "worker.control"; const WORKER_LIFECYCLE_SERVICE_VERSION: &str = "1"; +const INITIAL_WORKDIR_ALIAS: &str = "workdir"; #[derive(Debug, Clone, PartialEq, Eq)] pub enum WorkerControlSubject { @@ -457,40 +462,49 @@ struct WorkspaceWorkerLifecycleService { workspace_id: String, } +fn workspace_worker_create_request( + request: WorkerLifecycleSpawnRequest, +) -> Result { + let ticket_assignment = match (request.ticket_id, request.operation_id) { + (Some(ticket_id), Some(operation_id)) => { + Some(CreateWorkspaceWorkerTicketAssignmentRequest { + ticket_id, + operation_id, + }) + } + (None, None) => None, + _ => { + return Err(WorkspaceClientError::Request( + "ticket_id and operation_id must be provided together".to_string(), + )); + } + }; + let control_operation_id = ticket_assignment + .as_ref() + .map(|assignment| assignment.operation_id.clone()) + .unwrap_or_else(|| format!("worker-spawn-{}", Uuid::now_v7())); + Ok(CreateWorkspaceWorkerRequest { + runtime_id: request.runtime_id, + display_name: request.display_name, + profile: Some(request.profile), + ticket_assignment, + initial_submit: request.initial_submit, + workdir_attachments: vec![BrowserWorkerWorkingDirectorySelection { + alias: INITIAL_WORKDIR_ALIAS.to_string(), + working_directory_id: request.working_directory_id, + relative_cwd: request.relative_cwd, + }], + control_operation_id: Some(control_operation_id), + }) +} + #[async_trait] impl WorkerLifecycleService for WorkspaceWorkerLifecycleService { async fn spawn( &self, request: WorkerLifecycleSpawnRequest, ) -> Result { - let ticket_assignment = match (request.ticket_id, request.operation_id) { - (Some(ticket_id), Some(operation_id)) => Some(WorkerSpawnTicketAssignmentRequest { - ticket_id, - operation_id, - }), - (None, None) => None, - _ => { - return Err(WorkspaceClientError::Request( - "ticket_id and operation_id must be provided together".to_string(), - )); - } - }; - let control_operation_id = ticket_assignment - .as_ref() - .map(|assignment| assignment.operation_id.clone()) - .unwrap_or_else(|| format!("worker-spawn-{}", Uuid::now_v7())); - let body = WorkerSpawnRequest { - runtime_id: request.runtime_id, - display_name: request.display_name, - profile: request.profile, - control_operation_id, - ticket_assignment, - initial_submit: request.initial_submit, - working_directory: WorkerWorkingDirectorySelection { - working_directory_id: request.working_directory_id, - relative_cwd: request.relative_cwd, - }, - }; + let body = workspace_worker_create_request(request)?; self.client.execute(WorkspaceRequest::json( WorkspaceRequestMethod::Post, format!("/api/w/{}/worker-control/workers", self.workspace_id), @@ -694,31 +708,6 @@ struct WorkerSpawnInput { relative_cwd: Option, } -#[derive(Debug, Serialize)] -struct WorkerSpawnTicketAssignmentRequest { - ticket_id: String, - operation_id: String, -} - -#[derive(Debug, Serialize)] -struct WorkerSpawnRequest { - runtime_id: String, - display_name: String, - profile: String, - control_operation_id: String, - #[serde(skip_serializing_if = "Option::is_none")] - ticket_assignment: Option, - initial_submit: Vec, - working_directory: WorkerWorkingDirectorySelection, -} - -#[derive(Debug, Serialize)] -struct WorkerWorkingDirectorySelection { - working_directory_id: String, - #[serde(skip_serializing_if = "Option::is_none")] - relative_cwd: Option, -} - #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] enum WorkerSubjectInput { @@ -1445,7 +1434,23 @@ mod tests { "operation_id": "worker-spawn:00001KZ9E0DBS:call-1" }) ); + assert_eq!(body["workdir_attachments"].as_array().unwrap().len(), 1); + assert_eq!( + body["workdir_attachments"][0]["alias"], + INITIAL_WORKDIR_ALIAS + ); + assert_eq!( + body["workdir_attachments"][0]["working_directory_id"], + "workdir-1" + ); + assert_eq!( + body["workdir_attachments"][0]["relative_cwd"], + serde_json::Value::Null + ); + assert!(body.get("working_directory").is_none()); assert!(body.get("initial_text").is_none()); + serde_json::from_value::(body) + .expect("guarded spawn body must satisfy the strict Workspace create DTO"); } #[tokio::test] @@ -1714,30 +1719,38 @@ mod tests { } #[test] - fn worker_spawn_request_uses_authority_ids_without_runtime_paths() { - let request = WorkerSpawnRequest { + fn worker_spawn_request_uses_canonical_initial_attachment_contract() { + let request = workspace_worker_create_request(WorkerLifecycleSpawnRequest { runtime_id: "runtime-1".to_string(), - display_name: "Coder".to_string(), + working_directory_id: "wd-1".to_string(), + relative_cwd: Some("repo".to_string()), profile: "builtin:coder".to_string(), - control_operation_id: "spawn-operation-1".to_string(), - ticket_assignment: None, + ticket_id: None, + operation_id: None, + display_name: "Coder".to_string(), initial_submit: vec![ Segment::Flow { selector: "builtin:coder-review".to_string(), }, Segment::text("Implement the Ticket"), ], - working_directory: WorkerWorkingDirectorySelection { - working_directory_id: "wd-1".to_string(), - relative_cwd: Some("repo".to_string()), - }, - }; + }) + .unwrap(); let value = serde_json::to_value(request).unwrap(); assert_eq!(value["runtime_id"], "runtime-1"); - assert_eq!(value["working_directory"]["working_directory_id"], "wd-1"); + assert_eq!(value["workdir_attachments"].as_array().unwrap().len(), 1); + assert_eq!( + value["workdir_attachments"][0]["alias"], + INITIAL_WORKDIR_ALIAS + ); + assert_eq!( + value["workdir_attachments"][0]["working_directory_id"], + "wd-1" + ); + assert_eq!(value["workdir_attachments"][0]["relative_cwd"], "repo"); + assert!(value.get("working_directory").is_none()); assert!(value.get("cwd").is_none()); assert!(value.get("runtime_url").is_none()); - assert!(value["working_directory"].get("mode").is_none()); assert_eq!(value["initial_submit"][0]["kind"], "flow"); assert_eq!( value["initial_submit"][0]["selector"], diff --git a/crates/worker/src/feature/builtin/orchestration.rs b/crates/worker/src/feature/builtin/orchestration.rs index 0ad3cdb99..a16858705 100644 --- a/crates/worker/src/feature/builtin/orchestration.rs +++ b/crates/worker/src/feature/builtin/orchestration.rs @@ -260,7 +260,8 @@ mod tests { &serde_json::json!({ "ticket_id": "T-482", "runtime_id": "runtime-1", - "working_directory_id": "workdir-1" + "working_directory_id": "workdir-1", + "relative_cwd": "crates/yoi" }) .to_string(), ToolExecutionContext::new("call-7", "batch-1", 0), @@ -277,6 +278,8 @@ mod tests { Some("spawn-ticket-coder:00001KZXN51C7:call-7") ); assert_eq!(request.display_name, "Coder ยท T-482"); + assert_eq!(request.working_directory_id, "workdir-1"); + assert_eq!(request.relative_cwd.as_deref(), Some("crates/yoi")); assert_eq!( request.initial_submit, vec![ diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index 7dba7d0b3..4f4dc7cc9 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -2635,21 +2635,13 @@ impl WorkspaceWorkerRuntime for EmbeddedWorkerRuntime { }; let workspace_id = workspace_api.workspace_id.clone(); let config_bundle = spawn_config_bundle_ref(&request); - let create_request = CreateWorkerRequest { - worker_id: binding.worker_id, - create_fingerprint: binding.create_fingerprint, - profile, - display_name: request.requested_worker_name.clone(), - config_bundle, + let create_request = runtime_create_worker_request( + binding, + &request, profile_source, - initial_input: initial_worker_input(&request.initial_submit), - workdir_attachment_requests: request.resolved_workdir_attachment_requests.clone(), - workdir_attachments: request.resolved_workdir_attachments.clone(), - worker_observation_enabled: request.resolved_worker_observation_enabled, - worker_observation_grants: request.resolved_worker_observation_grants.clone(), - workspace_api: Some(workspace_api), - memory_settings: request.resolved_memory_settings.clone(), - }; + workspace_api, + config_bundle, + ); let workspace_scope = RuntimeWorkspaceScope::new(workspace_id, "embedded-backend"); match self .runtime @@ -4674,21 +4666,13 @@ impl WorkspaceWorkerRuntime for RemoteWorkerRuntime { } }; let config_bundle = spawn_config_bundle_ref(&request); - let create = CreateWorkerRequest { - worker_id: binding.worker_id, - create_fingerprint: binding.create_fingerprint, - profile, - display_name: request.requested_worker_name.clone(), - config_bundle, + let create = runtime_create_worker_request( + binding, + &request, profile_source, - initial_input: initial_worker_input(&request.initial_submit), - workdir_attachment_requests: request.resolved_workdir_attachment_requests.clone(), - workdir_attachments: request.resolved_workdir_attachments.clone(), - worker_observation_enabled: request.resolved_worker_observation_enabled, - worker_observation_grants: request.resolved_worker_observation_grants.clone(), - workspace_api: Some(workspace_api), - memory_settings: request.resolved_memory_settings.clone(), - }; + workspace_api, + config_bundle, + ); let create = match runtime_contract_convert(create) { Ok(create) => create, Err(diagnostic) => { @@ -5116,6 +5100,30 @@ fn remote_worker_projection_diagnostics( diagnostics } +fn runtime_create_worker_request( + binding: WorkerCreateBinding, + request: &WorkerSpawnRequest, + profile_source: ProfileSourceArchiveSource, + workspace_api: WorkspaceApiRef, + config_bundle: Option, +) -> CreateWorkerRequest { + CreateWorkerRequest { + worker_id: binding.worker_id, + create_fingerprint: binding.create_fingerprint, + profile: request.profile.clone(), + display_name: request.requested_worker_name.clone(), + config_bundle, + profile_source, + initial_input: initial_worker_input(&request.initial_submit), + workdir_attachment_requests: request.resolved_workdir_attachment_requests.clone(), + workdir_attachments: request.resolved_workdir_attachments.clone(), + worker_observation_enabled: request.resolved_worker_observation_enabled, + worker_observation_grants: request.resolved_worker_observation_grants.clone(), + workspace_api: Some(workspace_api), + memory_settings: request.resolved_memory_settings.clone(), + } +} + fn spawn_config_bundle_ref(request: &WorkerSpawnRequest) -> Option { request .resolved_config_bundle @@ -6813,6 +6821,41 @@ mod tests { assert_eq!(bundle_ref.digest, expected_digest); } + #[test] + fn runtime_create_request_serializes_only_canonical_workdir_attachments() { + let mut request = embedded_spawn_request(); + request.resolved_workdir_attachments = vec![WorkingDirectoryAttachmentClaim { + alias: workdir::WorkdirAttachmentAlias::new("workdir").unwrap(), + working_directory_id: "workdir-1".to_string(), + relative_cwd: Some("crates/yoi".to_string()), + }]; + let profile_source = + profile_source_archive_source(&request, &request.profile).expect("profile source"); + let create = runtime_create_worker_request( + test_create_binding(), + &request, + profile_source, + test_workspace_api(), + spawn_config_bundle_ref(&request), + ); + + let value = serde_json::to_value(create).unwrap(); + assert!(value.get("working_directory").is_none()); + assert!(value.get("working_directory_request").is_none()); + assert_eq!(value["workdir_attachments"].as_array().unwrap().len(), 1); + assert_eq!(value["workdir_attachments"][0]["alias"], "workdir"); + assert_eq!( + value["workdir_attachments"][0]["working_directory_id"], + "workdir-1" + ); + assert_eq!( + value["workdir_attachments"][0]["relative_cwd"], + "crates/yoi" + ); + serde_json::from_value::(value) + .expect("remote Runtime create payload must satisfy the strict DTO"); + } + #[test] fn registry_syncs_bundle_before_embedded_spawn() { let runtime = EmbeddedWorkerRuntime::new_memory_with_execution_backend(