diff --git a/crates/protocol/src/subscription.rs b/crates/protocol/src/subscription.rs index 5043db5ff..d53f5290f 100644 --- a/crates/protocol/src/subscription.rs +++ b/crates/protocol/src/subscription.rs @@ -599,7 +599,8 @@ pub struct SubscriptionWorkerWorkdirAttachment { /// Stable Worker-local routing alias. pub alias: String, /// Runtime-internal Repository id. Workspace-facing TypeScript contracts - /// omit this field and require `repository_key` from the Server projection. + /// omit this field and use `repository_key` for Repository Workdirs. Both + /// Repository fields are absent for non-Repository Workdirs. #[serde(default, skip_serializing_if = "Option::is_none")] #[cfg_attr(feature = "typescript", ts(skip))] #[cfg_attr(feature = "json-schema", schemars(skip))] @@ -622,7 +623,8 @@ impl SubscriptionWorkerWorkdirAttachment { validate_identifier("repository_id", repository_id, MAX_RESOURCE_ID_BYTES) } (None, Some(repository_key)) => validate_repository_key(repository_key), - _ => Err(SubscriptionValidationError::InvalidIdentifier { + (None, None) => Ok(()), + (Some(_), Some(_)) => Err(SubscriptionValidationError::InvalidIdentifier { field: "repository_authority", }), } @@ -1009,6 +1011,43 @@ mod tests { assert!(workdir_json.get("repository_id").is_none()); } + #[test] + fn external_worker_attachment_has_no_repository_authority() { + let mut external_worker = worker("worker-1"); + external_worker + .workdir_attachments + .push(SubscriptionWorkerWorkdirAttachment { + alias: "external".to_string(), + repository_id: None, + repository_key: None, + working_directory_id: SubscriptionWorkdirId::new("external-workdir-1").unwrap(), + }); + + external_worker.validate().unwrap(); + let json = serde_json::to_value(&external_worker).unwrap(); + let attachment = &json["workdir_attachments"][0]; + assert!(attachment.get("repository_id").is_none()); + assert!(attachment.get("repository_key").is_none()); + assert_eq!(attachment["working_directory_id"], "external-workdir-1"); + } + + #[test] + fn worker_attachment_rejects_ambiguous_repository_authority() { + let attachment = SubscriptionWorkerWorkdirAttachment { + alias: "checkout".to_string(), + repository_id: Some("repository-id".to_string()), + repository_key: Some("repository-key".to_string()), + working_directory_id: SubscriptionWorkdirId::new("workdir-1").unwrap(), + }; + + assert!(matches!( + attachment.validate(), + Err(SubscriptionValidationError::InvalidIdentifier { + field: "repository_authority" + }) + )); + } + #[test] fn subscribe_frame_has_stable_versioned_json_shape() { let frame = SubscriptionFrame::new(SubscriptionFramePayload::Request( diff --git a/crates/workspace-server/src/workspace_subscription.rs b/crates/workspace-server/src/workspace_subscription.rs index 5319e4a4d..86772e6ea 100644 --- a/crates/workspace-server/src/workspace_subscription.rs +++ b/crates/workspace-server/src/workspace_subscription.rs @@ -513,20 +513,29 @@ fn project_working_directory( } fn project_repository_keys(api: &WorkspaceApi, worker: &mut SubscriptionWorker) -> bool { + project_repository_keys_with(worker, |repository_id| { + api.store + .get_repository(&api.config.workspace_id, repository_id) + .ok() + .flatten() + .map(|repository| repository.repository_key) + }) +} + +fn project_repository_keys_with( + worker: &mut SubscriptionWorker, + mut resolve_repository_key: impl FnMut(&str) -> Option, +) -> bool { for attachment in &mut worker.workdir_attachments { let Some(repository_id) = attachment.repository_id.take() else { - if attachment.repository_key.is_none() { - return false; - } + // External Workdirs have no Repository identity. Their stable + // working_directory_id joins against the Workspace Workdir projection. continue; }; - let Ok(Some(repository)) = api - .store - .get_repository(&api.config.workspace_id, &repository_id) - else { + let Some(repository_key) = resolve_repository_key(&repository_id) else { return false; }; - attachment.repository_key = Some(repository.repository_key); + attachment.repository_key = Some(repository_key); } true } @@ -593,6 +602,44 @@ mod tests { protocol::subscription::SubscriptionWorkerId::new("worker-1").unwrap() } + #[test] + fn external_workdir_attachment_does_not_remove_workspace_worker() { + let mut worker = SubscriptionWorker { + worker_id: worker_id(), + runtime_id: Some("runtime-1".to_string()), + resource_key: Some("W-1".to_string()), + availability: protocol::subscription::SubscriptionWorkerAvailability::Observed, + subject_revision: 1, + worker_state: None, + state: protocol::subscription::SubscriptionWorkerState::Idle, + has_running_internal_workers: false, + workspace_id: Some("workspace-1".to_string()), + display_name: Some("Companion".to_string()), + profile: Some("builtin:companion".to_string()), + workdir_attachments: vec![ + protocol::subscription::SubscriptionWorkerWorkdirAttachment { + alias: "external".to_string(), + repository_id: None, + repository_key: None, + working_directory_id: protocol::subscription::SubscriptionWorkdirId::new( + "external-workdir-1", + ) + .unwrap(), + }, + ], + }; + + assert!(project_repository_keys_with(&mut worker, |_| { + panic!("External Workdir must not require Repository projection") + })); + worker.validate().unwrap(); + assert_eq!(worker.workdir_attachments.len(), 1); + assert_eq!( + worker.workdir_attachments[0].working_directory_id.as_str(), + "external-workdir-1" + ); + } + async fn run_with_events(events: mpsc::Receiver) -> SubscriptionResponse { let (control_outbound, mut control_receiver) = mpsc::channel(1); let (protocol_outbound, _protocol_receiver) = mpsc::channel(1);