From 74fbadb1cd84159fc0b1b856cc510588378267fc Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 20 Sep 2026 22:07:59 +0900 Subject: [PATCH] fix: complete multi-workdir isolation and launch UI --- crates/tools/src/edit.rs | 4 +- crates/tools/src/read.rs | 8 +- crates/tools/src/tracker.rs | 13 +- crates/tools/src/write.rs | 4 +- crates/tools/tests/integration.rs | 60 +++ crates/workspace-server/src/server.rs | 177 ++++++- .../console/worker-console.ui.test.ts | 6 +- .../workspace/sidebar/worker-launch.test.ts | 446 +++++++++++------- .../lib/workspace/sidebar/worker-launch.ts | 107 ++++- .../src/lib/workspace/styles/workers.css | 46 +- .../w/[workspaceId]/workers/new/+page.svelte | 210 +++++++-- 11 files changed, 804 insertions(+), 277 deletions(-) diff --git a/crates/tools/src/edit.rs b/crates/tools/src/edit.rs index f8fd9c2e5..4a22cb218 100644 --- a/crates/tools/src/edit.rs +++ b/crates/tools/src/edit.rs @@ -52,7 +52,9 @@ impl Tool for EditTool { params.target_workdir.as_deref(), workdir::WorkdirSessionCapability::Edit, )?; - let tracker = self.tracker.scoped(selected.alias.as_str()); + let tracker = self + .tracker + .scoped_attachment(&selected.alias, selected.generation); let path = WorkdirPath::new(¶ms.file_path).map_err(ToolsError::from)?; tracing::debug!(path = %path, replace_all = params.replace_all, "Edit"); diff --git a/crates/tools/src/read.rs b/crates/tools/src/read.rs index 2769d36be..5a6800b02 100644 --- a/crates/tools/src/read.rs +++ b/crates/tools/src/read.rs @@ -53,7 +53,9 @@ impl Tool for ReadTool { params.target_workdir.as_deref(), workdir::WorkdirSessionCapability::Read, )?; - let tracker = self.tracker.scoped(selected.alias.as_str()); + let tracker = self + .tracker + .scoped_attachment(&selected.alias, selected.generation); let offset = params.offset.unwrap_or(0); let limit = params.limit.unwrap_or(DEFAULT_LIMIT).max(1); @@ -216,7 +218,7 @@ mod tests { // History recorded assert!( tracker - .scoped("workdir") + .scoped_attachment(&workdir::WorkdirAttachmentAlias::new("workdir").unwrap(), 0,) .expected_workdir_hash(&WorkdirPath::new("a.txt").unwrap()) .is_ok() ); @@ -247,7 +249,7 @@ mod tests { #[tokio::test] async fn read_tool_missing_file() { - let (dir, fs, tracker) = setup(); + let (_dir, fs, tracker) = setup(); let def = read_tool(fs, tracker); let (_, tool) = def(); let input = serde_json::json!({ diff --git a/crates/tools/src/tracker.rs b/crates/tools/src/tracker.rs index 14be471c2..feaaf30ff 100644 --- a/crates/tools/src/tracker.rs +++ b/crates/tools/src/tracker.rs @@ -161,8 +161,9 @@ impl Tracker { } /// Return a view whose read-before-mutation state is isolated to one - /// Worker-local attachment alias while aggregate recency/change statistics - /// remain shared with the Worker tracker. + /// attachment incarnation (the Worker-local alias plus router generation), + /// while aggregate recency/change statistics remain shared with the Worker + /// tracker. pub fn scoped(&self, namespace: impl Into) -> Self { Self { inner: self.inner.clone(), @@ -171,6 +172,14 @@ impl Tracker { } } + pub(crate) fn scoped_attachment( + &self, + alias: &workdir::WorkdirAttachmentAlias, + generation: u64, + ) -> Self { + self.scoped(format!("{}#{generation}", alias.as_str())) + } + fn key(&self, path: PathBuf) -> TrackedPath { TrackedPath { namespace: self.namespace.clone(), diff --git a/crates/tools/src/write.rs b/crates/tools/src/write.rs index 5b8e33da6..8479de806 100644 --- a/crates/tools/src/write.rs +++ b/crates/tools/src/write.rs @@ -49,7 +49,9 @@ impl Tool for WriteTool { params.target_workdir.as_deref(), workdir::WorkdirSessionCapability::Write, )?; - let tracker = self.tracker.scoped(selected.alias.as_str()); + let tracker = self + .tracker + .scoped_attachment(&selected.alias, selected.generation); let path = WorkdirPath::new(¶ms.file_path).map_err(ToolsError::from)?; tracing::debug!(path = %path, bytes = params.content.len(), "Write"); diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index f79b89b13..f1d346e60 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -339,6 +339,66 @@ async fn routed_tools_observe_detach_after_registration() { router.close_all().await.unwrap(); } +#[tokio::test] +async fn detach_and_reattach_same_alias_requires_a_fresh_read() { + let first = TempDir::new().unwrap(); + let second = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + std::fs::write(first.path().join("same.txt"), "same bytes\n").unwrap(); + std::fs::write(second.path().join("same.txt"), "same bytes\n").unwrap(); + + let router = Arc::new(WorkdirSessionRouter::new()); + let alias = WorkdirAttachmentAlias::new("checkout").unwrap(); + router + .attach( + alias.clone(), + Arc::new(LocalWorkdirSession::new( + scope_with_spill(first.path(), spill.path()), + first.path().to_path_buf(), + )), + ) + .unwrap(); + let reg = Registry::new(routed_builtin_tools( + router.clone(), + Tracker::new(), + spill.path().to_path_buf(), + )); + call( + ®.get("Read"), + json!({ "target_workdir": "checkout", "file_path": "same.txt" }), + ) + .await; + + router.detach(&alias).await.unwrap(); + router + .attach( + alias.clone(), + Arc::new(LocalWorkdirSession::new( + scope_with_spill(second.path(), spill.path()), + second.path().to_path_buf(), + )), + ) + .unwrap(); + + let error = call_err( + ®.get("Edit"), + json!({ + "target_workdir": "checkout", + "file_path": "same.txt", + "old_string": "same", + "new_string": "changed" + }), + ) + .await; + assert!(error.to_string().contains("has not been read"), "{error}"); + assert_eq!( + std::fs::read_to_string(second.path().join("same.txt")).unwrap(), + "same bytes\n" + ); + + router.close_all().await.unwrap(); +} + #[tokio::test] async fn view_image_reads_scoped_bytes_into_durable_tool_detail() { let dir = TempDir::new().unwrap(); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index e050a3b77..7c996b441 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -383,6 +383,7 @@ static EMBEDDED_RUNTIME_REQUEST_IDENTITY: std::sync::LazyLock< #[derive(Clone)] struct WorkdirCommandSession { + attachment_alias: String, source: WorkdirSessionHandle, provider_handle: CommandHandle, } @@ -421,6 +422,7 @@ impl WorkdirSessionRegistry { fn register_command( &mut self, worker: RuntimeWorkerRef, + attachment_alias: String, source: WorkdirSessionHandle, provider_handle: CommandHandle, ) -> CommandHandle { @@ -436,6 +438,7 @@ impl WorkdirSessionRegistry { self.commands.insert( (worker, external_handle.clone()), WorkdirCommandSession { + attachment_alias, source, provider_handle, }, @@ -446,13 +449,42 @@ impl WorkdirSessionRegistry { fn command( &self, worker: &RuntimeWorkerRef, + attachment_alias: &str, external_handle: &CommandHandle, ) -> Option { self.commands .get(&(worker.clone(), external_handle.clone())) + .filter(|session| session.attachment_alias == attachment_alias) .cloned() } + fn take_worker_commands_for_alias( + &mut self, + worker: &RuntimeWorkerRef, + attachment_alias: &str, + ) -> Vec { + let command_handles = self + .commands + .iter() + .filter(|((owner, _), session)| { + owner == worker && session.attachment_alias == attachment_alias + }) + .map(|((_, handle), _)| handle.clone()) + .collect::>(); + command_handles + .into_iter() + .filter_map(|external_handle| { + self.commands + .remove(&(worker.clone(), external_handle.clone())) + .map(|session| RegisteredWorkdirSession::Command { + worker: worker.clone(), + external_handle, + session, + }) + }) + .collect() + } + fn take_worker(&mut self, worker: &RuntimeWorkerRef) -> Vec { let mut sessions = Vec::new(); if let Some(session) = self.attachments.remove(worker) { @@ -498,14 +530,10 @@ impl WorkdirSessionRegistry { } } -async fn close_worker_workdir_sessions( +async fn close_registered_workdir_sessions( registry: &Arc>, - worker: &RuntimeWorkerRef, + registered: Vec, ) -> std::result::Result<(), String> { - let registered = registry - .lock() - .map_err(|_| "Workdir session registry was poisoned".to_string())? - .take_worker(worker); let mut failed = Vec::new(); let mut errors = Vec::new(); for item in registered { @@ -533,6 +561,29 @@ async fn close_worker_workdir_sessions( } } +async fn close_worker_workdir_sessions( + registry: &Arc>, + worker: &RuntimeWorkerRef, +) -> std::result::Result<(), String> { + let registered = registry + .lock() + .map_err(|_| "Workdir session registry was poisoned".to_string())? + .take_worker(worker); + close_registered_workdir_sessions(registry, registered).await +} + +async fn close_worker_workdir_command_sessions_for_alias( + registry: &Arc>, + worker: &RuntimeWorkerRef, + attachment_alias: &str, +) -> std::result::Result<(), String> { + let registered = registry + .lock() + .map_err(|_| "Workdir session registry was poisoned".to_string())? + .take_worker_commands_for_alias(worker, attachment_alias); + close_registered_workdir_sessions(registry, registered).await +} + const ATTACHMENT_UPLOAD_GRANT_TTL_MS: u64 = 5 * 60 * 1_000; #[derive(Clone)] @@ -8529,6 +8580,20 @@ async fn close_current_worker_attachment_session_locked( Ok(()) } +async fn close_current_worker_command_sessions_for_alias_locked( + api: &WorkspaceApi, + worker: &RuntimeWorkerRef, + attachment_alias: &str, +) -> Result<()> { + close_worker_workdir_command_sessions_for_alias(&api.workdir_sessions, worker, attachment_alias) + .await + .map_err(|message| Error::RuntimeOperationFailed { + runtime_id: worker.runtime_id.clone(), + code: "workdir_session_close_failed".to_string(), + message, + }) +} + async fn close_current_worker_session_locked( api: &WorkspaceApi, worker: &RuntimeWorkerRef, @@ -8583,7 +8648,7 @@ async fn refresh_current_worker_session_locked( api: &WorkspaceApi, worker: &RuntimeWorkerRef, ) -> Result<()> { - close_current_worker_session_locked(api, worker).await?; + close_current_worker_attachment_session_locked(api, worker).await?; let remaining = api .store .list_worker_workdir_links(&api.config.workspace_id, worker)? @@ -8653,7 +8718,7 @@ async fn scoped_attach_current_worker_workdir( attached: true, })); } - close_current_worker_session_locked(&api, &worker).await?; + close_current_worker_attachment_session_locked(&api, &worker).await?; let link = api.store.attach_worker_workdir(&WorkerWorkdirLinkRecord { workspace_id: api.config.workspace_id.clone(), worker: worker.clone(), @@ -8734,7 +8799,8 @@ async fn scoped_detach_current_worker_workdir( worker.runtime_id, worker.worker_id )) })?; - close_current_worker_session_locked(&api, &worker).await?; + close_current_worker_attachment_session_locked(&api, &worker).await?; + close_current_worker_command_sessions_for_alias_locked(&api, &worker, alias.as_str()).await?; api.store.detach_worker_workdir( &api.config.workspace_id, &worker, @@ -8903,7 +8969,12 @@ async fn scoped_execute_current_worker_workdir_operation( .workdir_sessions .lock() .expect("Workdir session registry lock poisoned") - .register_command(worker.clone(), registered_source, provider_handle); + .register_command( + worker.clone(), + target_workdir.clone(), + registered_source, + provider_handle, + ); WorkdirSessionOperationResult::CommandStart(external_handle) } WorkdirSessionOperation::CommandStatus(external_handle) => { @@ -8966,7 +9037,7 @@ fn current_worker_command_session( .workdir_sessions .lock() .expect("Workdir session registry lock poisoned") - .command(worker, external_handle) + .command(worker, target_workdir, external_handle) .ok_or_else(|| { WorkdirOperationApiError::Provider(current_worker_workdir_operation_error( worker, @@ -20100,8 +20171,12 @@ mod tests { let mut registry = WorkdirSessionRegistry::default(); registry.insert_attachment(worker.clone(), source.clone()); let registered_source = registry.remove_attachment(&worker).unwrap(); - let external_handle = - registry.register_command(worker.clone(), registered_source, provider_handle.clone()); + let external_handle = registry.register_command( + worker.clone(), + "checkout".to_string(), + registered_source, + provider_handle.clone(), + ); assert_ne!(external_handle, provider_handle); let refreshed: WorkdirSessionHandle = Arc::new(workdir::LocalWorkdirSession::new( @@ -20116,7 +20191,14 @@ mod tests { .await .unwrap(); - let command = registry.command(&worker, &external_handle).unwrap(); + let command = registry + .command(&worker, "checkout", &external_handle) + .unwrap(); + assert!( + registry + .command(&worker, "other", &external_handle) + .is_none() + ); let output = command .source .command_output(workdir::CommandOutputRequest { @@ -20176,6 +20258,73 @@ mod tests { ); } + #[tokio::test] + async fn attachment_scoped_command_cleanup_preserves_other_aliases() { + let left_directory = tempfile::tempdir().unwrap(); + let right_directory = tempfile::tempdir().unwrap(); + let worker = RuntimeWorkerRef::new("runtime-command-alias", "worker-command-alias"); + let make_session = |path: &std::path::Path| -> WorkdirSessionHandle { + Arc::new(workdir::LocalWorkdirSession::new( + manifest::Scope::writable(path).unwrap(), + path.to_path_buf(), + )) + }; + let left = make_session(left_directory.path()); + let right = make_session(right_directory.path()); + let request = || workdir::CommandRequest { + command: "sleep 30".to_string(), + timeout_secs: 60, + output_limit: 4096, + cwd: None, + spill_dir: None, + tool_call_id: None, + }; + let left_provider = left.start_command(request()).await.unwrap(); + let right_provider = right.start_command(request()).await.unwrap(); + + let registry = Arc::new(Mutex::new(WorkdirSessionRegistry::default())); + let (left_external, right_external) = { + let mut sessions = registry.lock().unwrap(); + let left_external = + sessions.register_command(worker.clone(), "left".to_string(), left, left_provider); + let right_external = sessions.register_command( + worker.clone(), + "right".to_string(), + right, + right_provider, + ); + assert!(sessions.command(&worker, "right", &left_external).is_none()); + assert!(sessions.command(&worker, "left", &right_external).is_none()); + (left_external, right_external) + }; + + close_worker_workdir_command_sessions_for_alias(®istry, &worker, "right") + .await + .unwrap(); + + let left_command = { + let sessions = registry.lock().unwrap(); + assert!( + sessions + .command(&worker, "right", &right_external) + .is_none() + ); + sessions.command(&worker, "left", &left_external).unwrap() + }; + assert_eq!( + left_command + .source + .command_status(left_command.provider_handle.clone()) + .await + .unwrap(), + workdir::CommandStatus::Running + ); + + close_worker_workdir_sessions(®istry, &worker) + .await + .unwrap(); + } + fn seed_test_api_token(store: &dyn ControlPlaneStore, suffix: &str) -> String { let account_id = format!("account-{suffix}"); let user_id = format!("user-{suffix}"); diff --git a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts index c4f21edaa..774df4ab1 100644 --- a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts +++ b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts @@ -256,8 +256,10 @@ Deno.test("workspace Worker list lives on the dedicated Workers page", async () workersNav.includes("worker.display_name || worker.label") && workersNav.includes("worker-status-dot") && workersNav.includes("worker-status-spinner") && - workersNav.includes("worker.repository_key ?? '—'") && - workersNav.includes("worker.working_directory_id ?? '—'") && + workersNav.includes("workdirMeta(worker)") && + workersNav.includes("attachment.alias") && + workersNav.includes("attachment.repository_key ?? '—'") && + workersNav.includes("attachment.working_directory_id") && !workersNav.includes('aria-disabled="true"'), "Workers sidebar should link to the Worker list page and show state indicators with repository/workdir metadata", ); diff --git a/web/workspace/src/lib/workspace/sidebar/worker-launch.test.ts b/web/workspace/src/lib/workspace/sidebar/worker-launch.test.ts index 12df7eca1..b1abd3d67 100644 --- a/web/workspace/src/lib/workspace/sidebar/worker-launch.test.ts +++ b/web/workspace/src/lib/workspace/sidebar/worker-launch.test.ts @@ -1,6 +1,7 @@ import { buildCreateWorkspaceWorkerRequest, defaultWorkerLaunchForm, + workerLaunchAttachmentError, } from "./worker-launch.ts"; import type { WorkerLaunchOptionsResponse } from "./types.ts"; @@ -8,6 +9,262 @@ declare const Deno: { test(name: string, fn: () => Promise | void): void; }; +Deno.test("defaultWorkerLaunchForm uses the Backend-published defaults and initial Workdir", () => { + const form = defaultWorkerLaunchForm( + options, + emptyForm({ initial_text: "hello" }), + ); + + assertEquals(form.runtime_id, "remote"); + assertEquals(form.display_name, "Worker"); + assertEquals(form.profile, "builtin:coder"); + assertEquals(form.initial_text, "hello"); + assertEquals(form.workdir_attachments, [{ + alias: "workdir", + working_directory_id: "wd-1-repo", + relative_cwd: "", + }]); + assertEquals(form.working_directory_repository_key, "repo"); + assertEquals(form.working_directory_selector, "HEAD"); +}); + +Deno.test("defaultWorkerLaunchForm preserves an available Ticket role profile", () => { + const reviewerOptions = { + ...options, + profiles: [ + ...options.profiles, + { id: "builtin:reviewer", label: "Reviewer", description: "review" }, + ], + }; + const form = defaultWorkerLaunchForm( + reviewerOptions, + emptyForm({ + display_name: "Review worker", + profile: "builtin:reviewer", + initial_text: "Review the ticket.", + working_directory_repository_key: "repo", + working_directory_selector: "HEAD", + }), + ); + + assertEquals(form.profile, "builtin:reviewer"); +}); + +Deno.test("defaultWorkerLaunchForm skips occupied Workdirs and leaves an editable attachment", () => { + const form = defaultWorkerLaunchForm( + { + ...options, + working_directories: [{ + ...options.working_directories[0], + occupied_by: { + runtime_id: "embedded", + worker_id: "0198f82e-6d90-7f15-a121-174a02e10e77", + display_name: "Worker 12", + linked_at: "2026-07-24T00:00:00Z", + }, + }], + }, + emptyForm({ initial_text: "hello" }), + ); + + assertEquals(form.workdir_attachments, [{ + alias: "workdir", + working_directory_id: "", + relative_cwd: "", + }]); +}); + +Deno.test("defaultWorkerLaunchForm preserves a Ticket repository target", () => { + const form = defaultWorkerLaunchForm( + { + ...options, + repositories: [ + ...options.repositories, + { repository_key: "ticket-repo", default_selector: "main" }, + ], + working_directories: [ + options.working_directories[0], + { + ...options.working_directories[0], + working_directory_id: "ticket-workdir", + repository_key: "ticket-repo", + creation_selector: "work/ticket", + }, + ], + }, + emptyForm({ + display_name: "Ticket worker", + profile: "builtin:coder", + initial_text: "Work on a ticket.", + working_directory_repository_key: "ticket-repo", + working_directory_selector: "work/ticket", + }), + ); + + assertEquals(form.workdir_attachments, [{ + alias: "workdir", + working_directory_id: "ticket-workdir", + relative_cwd: "", + }]); + assertEquals(form.working_directory_repository_key, "ticket-repo"); + assertEquals(form.working_directory_selector, "work/ticket"); +}); + +Deno.test("defaultWorkerLaunchForm clears attachments for a Workdir-less Runtime", () => { + const form = defaultWorkerLaunchForm( + options, + emptyForm({ + runtime_id: "embedded", + workdir_attachments: [{ + alias: "checkout", + working_directory_id: "wd-1-repo", + relative_cwd: "src", + }], + }), + ); + + assertEquals(form.workdir_attachments, []); +}); + +Deno.test("buildCreateWorkspaceWorkerRequest emits multiple aliased attachments", () => { + const request = buildCreateWorkspaceWorkerRequest(emptyForm({ + runtime_id: "remote", + display_name: "Worker", + profile: "builtin:coder", + initial_text: "go", + workdir_attachments: [ + { + alias: " checkout ", + working_directory_id: "wd-1-repo", + relative_cwd: "crates/yoi", + }, + { + alias: "docs", + working_directory_id: "wd-2-docs", + relative_cwd: " ", + }, + ], + })); + + assertEquals(request, { + runtime_id: "remote", + display_name: "Worker", + profile: "builtin:coder", + ticket_assignment: null, + initial_submit: [{ kind: "text", content: "go" }], + workdir_attachments: [ + { + alias: "checkout", + working_directory_id: "wd-1-repo", + relative_cwd: "crates/yoi", + }, + { + alias: "docs", + working_directory_id: "wd-2-docs", + relative_cwd: null, + }, + ], + control_operation_id: null, + }); +}); + +Deno.test("buildCreateWorkspaceWorkerRequest validates attachment aliases and selections", () => { + assertEquals( + workerLaunchAttachmentError([attachment("invalid/alias", "wd-1-repo")]), + "Attachment 1 alias must be 1–64 ASCII letters, digits, dots, underscores, or hyphens, and start with a letter or digit.", + ); + assertThrows( + () => + buildCreateWorkspaceWorkerRequest(emptyForm({ + workdir_attachments: [ + attachment("checkout", "wd-1-repo"), + attachment("checkout", "wd-2-docs"), + ], + })), + "Attachment alias “checkout” is used more than once.", + ); + assertThrows( + () => + buildCreateWorkspaceWorkerRequest(emptyForm({ + workdir_attachments: [ + attachment("checkout", "wd-1-repo"), + attachment("docs", "wd-1-repo"), + ], + })), + "A Workdir cannot be attached more than once to one Worker.", + ); + assertThrows( + () => + buildCreateWorkspaceWorkerRequest(emptyForm({ + workdir_attachments: [attachment("checkout", "")], + })), + "Attachment 1 must select a Workdir.", + ); + assertThrows( + () => + buildCreateWorkspaceWorkerRequest(emptyForm({ + workdir_attachments: [{ + ...attachment("checkout", "wd-1-repo"), + relative_cwd: "../outside", + }], + })), + "Attachment 1 relative cwd must stay inside the selected Workdir.", + ); +}); + +Deno.test("buildCreateWorkspaceWorkerRequest sends no initial segments for an empty draft", () => { + const request = buildCreateWorkspaceWorkerRequest(emptyForm({ + runtime_id: "embedded", + profile: "builtin:companion", + initial_text: " ", + })); + + assertEquals(request.initial_submit, []); +}); + +Deno.test("buildCreateWorkspaceWorkerRequest emits an empty attachment list for Workdir-less launches", () => { + const request = buildCreateWorkspaceWorkerRequest(emptyForm({ + runtime_id: "embedded", + display_name: "Worker", + profile: "builtin:companion", + initial_text: "chat", + })); + + assertEquals(request, { + runtime_id: "embedded", + display_name: "Worker", + profile: "builtin:companion", + ticket_assignment: null, + initial_submit: [{ kind: "text", content: "chat" }], + workdir_attachments: [], + control_operation_id: null, + }); +}); + +function emptyForm( + overrides: Partial[0]> = + {}, +): Parameters[0] { + return { + runtime_id: "", + display_name: "", + profile: "", + initial_text: "", + workdir_attachments: [], + working_directory_repository_key: "", + working_directory_selector: "", + ...overrides, + }; +} + +function attachment(alias: string, workingDirectoryId: string) { + return { + alias, + working_directory_id: workingDirectoryId, + relative_cwd: "", + }; +} + function assertEquals(actual: T, expected: T): void { const actualJson = JSON.stringify(actual); const expectedJson = JSON.stringify(expected); @@ -16,6 +273,18 @@ function assertEquals(actual: T, expected: T): void { } } +function assertThrows(fn: () => unknown, expectedMessage: string): void { + try { + fn(); + } catch (error) { + if (error instanceof Error && error.message === expectedMessage) return; + throw new Error( + `Expected error ${JSON.stringify(expectedMessage)}, got ${String(error)}`, + ); + } + throw new Error(`Expected error ${JSON.stringify(expectedMessage)}`); +} + const options: WorkerLaunchOptionsResponse = { workspace_id: "workspace", runtimes: [ @@ -66,180 +335,3 @@ const options: WorkerLaunchOptionsResponse = { ], diagnostics: [], }; - -Deno.test("defaultWorkerLaunchForm uses the Backend-published Workspace default profile", () => { - const form = defaultWorkerLaunchForm(options, { - runtime_id: "", - display_name: "", - profile: "", - initial_text: "hello", - working_directory_id: "", - working_directory_repository_key: "", - working_directory_selector: "", - relative_cwd: "", - }); - - assertEquals(form.runtime_id, "remote"); - assertEquals(form.display_name, "Worker"); - assertEquals(form.profile, "builtin:coder"); - assertEquals(form.initial_text, "hello"); - assertEquals(form.working_directory_id, "wd-1-repo"); - assertEquals(form.working_directory_repository_key, "repo"); - assertEquals(form.working_directory_selector, "HEAD"); -}); - -Deno.test("defaultWorkerLaunchForm preserves an available Ticket role profile", () => { - const reviewerOptions = { - ...options, - profiles: [ - ...options.profiles, - { id: "builtin:reviewer", label: "Reviewer", description: "review" }, - ], - }; - const form = defaultWorkerLaunchForm(reviewerOptions, { - runtime_id: "", - display_name: "Review worker", - profile: "builtin:reviewer", - initial_text: "Review the ticket.", - working_directory_id: "", - working_directory_repository_key: "repo", - working_directory_selector: "HEAD", - relative_cwd: "", - }); - - assertEquals(form.profile, "builtin:reviewer"); -}); - -Deno.test("defaultWorkerLaunchForm skips occupied working directories", () => { - const form = defaultWorkerLaunchForm( - { - ...options, - working_directories: [ - { - ...options.working_directories[0], - occupied_by: { - runtime_id: "embedded", - worker_id: "0198f82e-6d90-7f15-a121-174a02e10e77", - display_name: "Worker 12", - linked_at: "2026-07-24T00:00:00Z", - }, - }, - ], - }, - { - runtime_id: "", - display_name: "", - profile: "", - initial_text: "hello", - working_directory_id: "", - working_directory_repository_key: "", - working_directory_selector: "", - relative_cwd: "", - }, - ); - - assertEquals(form.working_directory_id, ""); -}); - -Deno.test("defaultWorkerLaunchForm preserves a Ticket repository target", () => { - const form = defaultWorkerLaunchForm( - { - ...options, - repositories: [ - ...options.repositories, - { - repository_key: "ticket-repo", - default_selector: "main", - }, - ], - working_directories: [ - options.working_directories[0], - { - ...options.working_directories[0], - working_directory_id: "ticket-workdir", - repository_key: "ticket-repo", - creation_selector: "work/ticket", - }, - ], - }, - { - runtime_id: "", - display_name: "Ticket worker", - profile: "builtin:coder", - initial_text: "Work on a ticket.", - working_directory_id: "", - working_directory_repository_key: "ticket-repo", - working_directory_selector: "work/ticket", - relative_cwd: "", - }, - ); - - assertEquals(form.working_directory_id, "ticket-workdir"); - assertEquals(form.working_directory_repository_key, "ticket-repo"); - assertEquals(form.working_directory_selector, "work/ticket"); -}); - -Deno.test("buildCreateWorkspaceWorkerRequest sends one aliased attachment", () => { - const request = buildCreateWorkspaceWorkerRequest({ - runtime_id: "embedded", - display_name: "Worker", - profile: "builtin:coder", - initial_text: "go", - working_directory_id: "wd-1-repo", - working_directory_repository_key: "repo", - working_directory_selector: "main", - relative_cwd: "crates/yoi", - }); - - assertEquals(request, { - runtime_id: "embedded", - display_name: "Worker", - profile: "builtin:coder", - ticket_assignment: null, - initial_submit: [{ kind: "text", content: "go" }], - workdir_attachments: [{ - alias: "workdir", - working_directory_id: "wd-1-repo", - relative_cwd: "crates/yoi", - }], - control_operation_id: null, - }); -}); - -Deno.test("buildCreateWorkspaceWorkerRequest sends no initial segments for an empty draft", () => { - const request = buildCreateWorkspaceWorkerRequest({ - runtime_id: "embedded", - display_name: "Worker", - profile: "builtin:companion", - initial_text: " ", - working_directory_id: "", - working_directory_repository_key: "", - working_directory_selector: "", - relative_cwd: "", - }); - - assertEquals(request.initial_submit, []); -}); - -Deno.test("buildCreateWorkspaceWorkerRequest emits an empty attachment list for workdirless launches", () => { - const request = buildCreateWorkspaceWorkerRequest({ - runtime_id: "embedded", - display_name: "Worker", - profile: "builtin:companion", - initial_text: "chat", - working_directory_id: "", - working_directory_repository_key: "", - working_directory_selector: "", - relative_cwd: "", - }); - - assertEquals(request, { - runtime_id: "embedded", - display_name: "Worker", - profile: "builtin:companion", - ticket_assignment: null, - initial_submit: [{ kind: "text", content: "chat" }], - workdir_attachments: [], - control_operation_id: null, - }); -}); diff --git a/web/workspace/src/lib/workspace/sidebar/worker-launch.ts b/web/workspace/src/lib/workspace/sidebar/worker-launch.ts index 16227371b..834651337 100644 --- a/web/workspace/src/lib/workspace/sidebar/worker-launch.ts +++ b/web/workspace/src/lib/workspace/sidebar/worker-launch.ts @@ -1,19 +1,29 @@ -import type { CreateWorkspaceWorkerRequest } from "$lib/generated/worker-launch-api"; +import type { + BrowserWorkerWorkingDirectorySelection, + CreateWorkspaceWorkerRequest, +} from "$lib/generated/worker-launch-api"; import { parseCreateWorkspaceWorkerRequest } from "$lib/workspace/api/workers"; import type { WorkerLaunchOptionsResponse } from "./types"; +export type WorkerLaunchAttachmentFormState = { + alias: string; + working_directory_id: string; + relative_cwd: string; +}; + export type WorkerLaunchFormState = { runtime_id: string; display_name: string; profile: string; initial_text: string; - working_directory_id: string; + workdir_attachments: WorkerLaunchAttachmentFormState[]; working_directory_repository_key: string; working_directory_selector: string; - relative_cwd: string; }; +const WORKDIR_ALIAS_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; + export function defaultWorkerLaunchForm( options: WorkerLaunchOptionsResponse | null, current: WorkerLaunchFormState, @@ -59,6 +69,27 @@ export function defaultWorkerLaunchForm( repository.repository_key === current.working_directory_repository_key ) ?? options?.repositories[0]; + const availableWorkdirIds = new Set( + availableWorkingDirectories.map((directory) => + directory.working_directory_id + ), + ); + const currentAttachments = workdirlessRuntime + ? [] + : current.workdir_attachments.filter((attachment) => + availableWorkdirIds.has(attachment.working_directory_id) + ); + const workdirAttachments = currentAttachments.length > 0 + ? currentAttachments + : preferredWorkingDirectory + ? [{ + alias: "workdir", + working_directory_id: preferredWorkingDirectory.working_directory_id, + relative_cwd: "", + }] + : selectedRuntime?.working_directory_required === true + ? [{ alias: "workdir", working_directory_id: "", relative_cwd: "" }] + : []; return { runtime_id: current.runtime_id || preferredRuntime?.runtime_id || "", @@ -68,22 +99,68 @@ export function defaultWorkerLaunchForm( ? current.profile : preferredProfile?.id || "", initial_text: current.initial_text, - working_directory_id: - !workdirlessRuntime && availableWorkingDirectories.some( - (directory) => - directory.working_directory_id === current.working_directory_id, - ) - ? current.working_directory_id - : preferredWorkingDirectory?.working_directory_id || "", + workdir_attachments: workdirAttachments, working_directory_repository_key: current.working_directory_repository_key || preferredRepository?.repository_key || "", working_directory_selector: current.working_directory_selector || preferredRepository?.default_selector || "HEAD", - relative_cwd: current.relative_cwd, }; } +export function workerLaunchAttachmentError( + attachments: WorkerLaunchAttachmentFormState[], +): string | null { + const aliases = new Set(); + const workdirIds = new Set(); + + for (const [index, attachment] of attachments.entries()) { + const position = index + 1; + const alias = attachment.alias.trim(); + const workdirId = attachment.working_directory_id.trim(); + const relativeCwd = attachment.relative_cwd.trim(); + + if (!WORKDIR_ALIAS_PATTERN.test(alias)) { + return `Attachment ${position} alias must be 1–64 ASCII letters, digits, dots, underscores, or hyphens, and start with a letter or digit.`; + } + if (aliases.has(alias)) { + return `Attachment alias “${alias}” is used more than once.`; + } + aliases.add(alias); + + if (!workdirId) { + return `Attachment ${position} must select a Workdir.`; + } + if (workdirIds.has(workdirId)) { + return "A Workdir cannot be attached more than once to one Worker."; + } + workdirIds.add(workdirId); + + if ( + relativeCwd.startsWith("/") || + relativeCwd.split("/").some((component) => component === "..") + ) { + return `Attachment ${position} relative cwd must stay inside the selected Workdir.`; + } + } + + return null; +} + +function validatedAttachments( + attachments: WorkerLaunchAttachmentFormState[], +): BrowserWorkerWorkingDirectorySelection[] { + const error = workerLaunchAttachmentError(attachments); + if (error) { + throw new Error(error); + } + return attachments.map((attachment) => ({ + alias: attachment.alias.trim(), + working_directory_id: attachment.working_directory_id.trim(), + relative_cwd: attachment.relative_cwd.trim() || null, + })); +} + export function buildCreateWorkspaceWorkerRequest( form: WorkerLaunchFormState, ): CreateWorkspaceWorkerRequest { @@ -96,13 +173,7 @@ export function buildCreateWorkspaceWorkerRequest( initial_submit: initialMessage ? [{ kind: "text", content: form.initial_text }] : [], - workdir_attachments: form.working_directory_id - ? [{ - alias: "workdir", - working_directory_id: form.working_directory_id, - relative_cwd: form.relative_cwd.trim() || null, - }] - : [], + workdir_attachments: validatedAttachments(form.workdir_attachments), control_operation_id: null, }); } diff --git a/web/workspace/src/lib/workspace/styles/workers.css b/web/workspace/src/lib/workspace/styles/workers.css index b9146d2da..7fbeb1b28 100644 --- a/web/workspace/src/lib/workspace/styles/workers.css +++ b/web/workspace/src/lib/workspace/styles/workers.css @@ -143,19 +143,41 @@ min-width: 0; max-width: 100%; } - .worker-inline-select.wd-select { - min-width: 20rem; + .worker-attachment-heading { + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-3); + } + .worker-attachment-heading h3, + .new-working-directory-panel h3 { + margin: 0; + color: var(--text-strong); + font-size: var(--font-size-body); + } + .worker-attachment-list { + display: grid; + } + .worker-attachment-row { + display: grid; + grid-template-columns: + minmax(7rem, 0.7fr) minmax(13rem, 1.5fr) minmax(10rem, 1fr) auto; + align-items: end; + gap: var(--space-2); + border-top: 1px solid var(--line); + padding: var(--space-3) 0; + } + .worker-attachment-row:last-child { + border-bottom: 1px solid var(--line); + } + .worker-attachment-remove { + margin-bottom: 1px; } .new-working-directory-panel { display: grid; gap: var(--space-3); padding-left: var(--space-3); } - .new-working-directory-panel h3 { - margin: 0; - color: var(--text-strong); - font-size: var(--font-size-body); - } .new-working-directory-fields, .worker-detail-grid { display: grid; @@ -182,15 +204,19 @@ flex-direction: column; white-space: normal; } - .worker-inline-select, - .worker-inline-select.wd-select { + .worker-inline-select { width: 100%; min-width: 0; } .new-working-directory-fields, - .worker-detail-grid { + .worker-detail-grid, + .worker-attachment-row { grid-template-columns: 1fr; } + .worker-attachment-remove { + justify-self: start; + margin-bottom: 0; + } } .worker-submit-error { display: grid; diff --git a/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte index dc98160a5..fba5c5c15 100644 --- a/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte @@ -11,7 +11,12 @@ parseWorkerLaunchOptionsResponse, } from '$lib/workspace/api/workers'; import { formatCurrentWorkdirRevision } from '$lib/workspace/settings/workdir-revision'; - import { buildCreateWorkspaceWorkerRequest, defaultWorkerLaunchForm } from '$lib/workspace/sidebar/worker-launch'; + import { + buildCreateWorkspaceWorkerRequest, + defaultWorkerLaunchForm, + workerLaunchAttachmentError, + } from '$lib/workspace/sidebar/worker-launch'; + import type { WorkerLaunchAttachmentFormState } from '$lib/workspace/sidebar/worker-launch'; import type { Diagnostic, WorkerLaunchOptionsResponse, @@ -62,18 +67,17 @@ : '', ); let initialText = $state(ticketContext?.initialInput ?? ''); - let workingDirectoryId = $state(''); + let workdirAttachments = $state([]); let workingDirectoryDisplayName = $state(''); let workingDirectoryRepositoryKey = $state(ticketContext?.repositoryKey ?? ''); let workingDirectorySelector = $state(ticketContext?.refSelector ?? 'HEAD'); - let relativeCwd = $state(''); let creatingWorkingDirectory = $state(false); - let isNewWorkingDirectorySelected = $derived(workingDirectoryId === NEW_WORKING_DIRECTORY_VALUE); + let newWorkingDirectoryAttachmentIndex = $derived( + workdirAttachments.findIndex((attachment) => attachment.working_directory_id === NEW_WORKING_DIRECTORY_VALUE), + ); + let isNewWorkingDirectorySelected = $derived(newWorkingDirectoryAttachmentIndex >= 0); let selectedRuntime = $derived(options?.runtimes.find((runtime) => runtime.runtime_id === runtimeId)); let selectedRuntimeAllowsNoWorkdir = $derived(selectedRuntime?.working_directory_required === false); - let hasSelectedExistingWorkdir = $derived(Boolean( - workingDirectoryId && !isNewWorkingDirectorySelected && !selectedRuntimeAllowsNoWorkdir, - )); let availableWorkingDirectories = $derived( selectedRuntimeAllowsNoWorkdir ? [] @@ -83,10 +87,17 @@ directory.occupied_by == null ), ); + let attachmentValidationError = $derived( + isNewWorkingDirectorySelected + ? 'Create the new Workdir before starting the Worker.' + : workerLaunchAttachmentError(workdirAttachments), + ); let canStartWorker = $derived(Boolean( runtimeId && profile && - (hasSelectedExistingWorkdir || (selectedRuntimeAllowsNoWorkdir && !isNewWorkingDirectorySelected)), + (selectedRuntimeAllowsNoWorkdir + ? workdirAttachments.length === 0 + : workdirAttachments.length > 0 && !attachmentValidationError), )); function workerApiPath(path: string): string { @@ -105,12 +116,49 @@ }); $effect(() => { - if (selectedRuntimeAllowsNoWorkdir && workingDirectoryId) { - workingDirectoryId = ''; - relativeCwd = ''; + if (selectedRuntimeAllowsNoWorkdir && workdirAttachments.length > 0) { + workdirAttachments = []; + } else if (selectedRuntime?.working_directory_required === true && workdirAttachments.length === 0) { + workdirAttachments = [{ + alias: 'workdir', + working_directory_id: availableWorkingDirectories[0]?.working_directory_id ?? '', + relative_cwd: '', + }]; } }); + function nextAttachmentAlias(): string { + const aliases = new Set(workdirAttachments.map((attachment) => attachment.alias.trim())); + if (!aliases.has('workdir')) return 'workdir'; + let suffix = 2; + while (aliases.has(`workdir${suffix}`)) suffix += 1; + return `workdir${suffix}`; + } + + function addAttachment(): void { + const selectedWorkdirs = new Set( + workdirAttachments.map((attachment) => attachment.working_directory_id), + ); + const available = availableWorkingDirectories.find((directory) => + !selectedWorkdirs.has(directory.working_directory_id) + ); + workdirAttachments = [...workdirAttachments, { + alias: nextAttachmentAlias(), + working_directory_id: available?.working_directory_id ?? '', + relative_cwd: '', + }]; + } + + function removeAttachment(index: number): void { + workdirAttachments = workdirAttachments.filter((_, candidateIndex) => candidateIndex !== index); + } + + function workdirSelectedByAnotherAttachment(workdirId: string, index: number): boolean { + return workdirAttachments.some((attachment, candidateIndex) => + candidateIndex !== index && attachment.working_directory_id === workdirId + ); + } + async function loadLaunchOptions(signal?: AbortSignal) { loading = true; optionsError = null; @@ -126,19 +174,30 @@ display_name: displayName, profile, initial_text: initialText, - working_directory_id: workingDirectoryId, + workdir_attachments: workdirAttachments, working_directory_repository_key: workingDirectoryRepositoryKey, working_directory_selector: workingDirectorySelector, - relative_cwd: relativeCwd, }); runtimeId = form.runtime_id; displayName = form.display_name; profile = form.profile; - workingDirectoryId = form.working_directory_id || - (ticketContext?.repositoryKey ? NEW_WORKING_DIRECTORY_VALUE : ''); + const runtimeRequiresWorkdir = payload.runtimes.find((runtime) => + runtime.runtime_id === form.runtime_id + )?.working_directory_required !== false; + workdirAttachments = form.workdir_attachments; + if ( + runtimeRequiresWorkdir && + ticketContext?.repositoryKey && + workdirAttachments.every((attachment) => !attachment.working_directory_id) + ) { + workdirAttachments = [{ + alias: 'workdir', + working_directory_id: NEW_WORKING_DIRECTORY_VALUE, + relative_cwd: '', + }]; + } workingDirectoryRepositoryKey = form.working_directory_repository_key; workingDirectorySelector = form.working_directory_selector; - relativeCwd = form.relative_cwd; } catch (err) { if (err instanceof DOMException && err.name === 'AbortError') { return; @@ -152,6 +211,11 @@ } async function createWorkingDirectory() { + const attachmentIndex = newWorkingDirectoryAttachmentIndex; + if (attachmentIndex < 0) { + submitError = { message: 'select New workdir on an attachment first', diagnostics: [] }; + return; + } if (!runtimeId) { submitError = { message: 'select a runtime before creating a workdir', diagnostics: [] }; return; @@ -195,7 +259,11 @@ ], } : options; - workingDirectoryId = payload.item.working_directory_id; + workdirAttachments = workdirAttachments.map((attachment, index) => + index === attachmentIndex && attachment.working_directory_id === NEW_WORKING_DIRECTORY_VALUE + ? { ...attachment, working_directory_id: payload.item.working_directory_id } + : attachment + ); } catch (err) { submitError = exceptionDisplayError(err, 'workdir create failed'); } finally { @@ -208,8 +276,16 @@ submitError = { message: 'workspace id is unavailable', diagnostics: [] }; return; } - if (isNewWorkingDirectorySelected || (!workingDirectoryId && !selectedRuntimeAllowsNoWorkdir)) { - submitError = { message: 'select or create a workdir before starting a Worker; only embedded Runtime can start without one', diagnostics: [] }; + if (selectedRuntimeAllowsNoWorkdir && workdirAttachments.length > 0) { + submitError = { message: 'the selected Runtime must start without Workdir attachments', diagnostics: [] }; + return; + } + if (!selectedRuntimeAllowsNoWorkdir && workdirAttachments.length === 0) { + submitError = { message: 'add at least one Workdir attachment before starting a Worker; only embedded Runtime can start without one', diagnostics: [] }; + return; + } + if (attachmentValidationError) { + submitError = { message: attachmentValidationError, diagnostics: [] }; return; } @@ -224,10 +300,9 @@ display_name: displayName, profile, initial_text: initialText, - working_directory_id: workingDirectoryId, + workdir_attachments: workdirAttachments, working_directory_repository_key: workingDirectoryRepositoryKey, working_directory_selector: workingDirectorySelector, - relative_cwd: relativeCwd, })), }); if (!response.ok) { @@ -304,21 +379,7 @@

Location

- Run at - - in + Run in
- {#if !selectedRuntimeAllowsNoWorkdir && !workingDirectoryId && !isNewWorkingDirectorySelected} -

This Runtime requires a selected workdir before starting a Worker.

- {:else if selectedRuntimeAllowsNoWorkdir && !workingDirectoryId} -

No filesystem tools or Bash will be available without a workdir.

+ {#if selectedRuntimeAllowsNoWorkdir} +

No filesystem tools or Bash will be available without a Workdir.

+ {:else} +
+

Initial Workdir attachments

+ +
+ +
+ {#each workdirAttachments as attachment, index} +
+ + + + +
+ {/each} +
+ + {#if attachmentValidationError} +

{attachmentValidationError}

+ {/if} {/if} {#if isNewWorkingDirectorySelected}
-

New workdir

+

New Workdir for “{workdirAttachments[newWorkingDirectoryAttachmentIndex]?.alias || `attachment ${newWorkingDirectoryAttachmentIndex + 1}`}”

{/if} - - {#if hasSelectedExistingWorkdir || isNewWorkingDirectorySelected} - - {/if}