From 5f651755d8f759ad8866fdaea01cc8bae3dfb767 Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 18 Jul 2026 21:49:19 +0900 Subject: [PATCH] runtime: preserve workdirs across worker deletion --- .yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep | 0 .yoi/tickets/00001KXTKS0VG/item.md | 29 +++ .yoi/tickets/00001KXTKS0VG/resolution.md | 1 + .yoi/tickets/00001KXTKS0VG/thread.md | 89 +++++++++ crates/worker-runtime/src/worker_backend.rs | 174 +++++++++++++++--- 5 files changed, 272 insertions(+), 21 deletions(-) create mode 100644 .yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep create mode 100644 .yoi/tickets/00001KXTKS0VG/item.md create mode 100644 .yoi/tickets/00001KXTKS0VG/resolution.md create mode 100644 .yoi/tickets/00001KXTKS0VG/thread.md diff --git a/.yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep b/.yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/.yoi/tickets/00001KXTKS0VG/item.md b/.yoi/tickets/00001KXTKS0VG/item.md new file mode 100644 index 00000000..9e089389 --- /dev/null +++ b/.yoi/tickets/00001KXTKS0VG/item.md @@ -0,0 +1,29 @@ +--- +title: 'Preserve Workdirs when Workers stop or delete' +state: 'closed' +created_at: '2026-07-18T12:38:47Z' +updated_at: '2026-07-18T12:49:12Z' +assignee: null +queued_by: 'yoi ticket' +queued_at: '2026-07-18T12:39:18Z' +--- + +## 背景 + +Backend-managed Workdir は Worker とは独立した再利用可能 resource として扱う。現在は Worker stop/delete 時に runtime が Worker の working directory binding を `materializer.cleanup` してしまい、workspace-server 側の Workdir record だけが残って `corrupted` と表示される。 + +Worker lifecycle と Workdir lifecycle を分離し、Worker の削除は Workdir の占有を解放するだけにする。Workdir 実体の削除は明示的な Workdir cleanup/delete API に限定する。 + +## 要件 + +- Worker stop/delete では Workdir 実体を削除しない。 +- Worker spawn failure rollback では、その spawn request で新規 materialize した Workdir だけ cleanup する。 +- 既存 Workdir に bind した spawn failure では Workdir を cleanup しない。 +- Workdir cleanup API 経由の明示 cleanup は維持する。 + +## 受け入れ条件 + +- `stop_worker` 後も Worker に bind されていた Workdir 実体が残る。 +- Worker spawn failure で既存 Workdir bind が使われた場合、Workdir 実体は残る。 +- Worker spawn failure で新規 materialize した場合だけ rollback cleanup される。 +- focused worker-runtime tests が通る。 diff --git a/.yoi/tickets/00001KXTKS0VG/resolution.md b/.yoi/tickets/00001KXTKS0VG/resolution.md new file mode 100644 index 00000000..6ba2d2da --- /dev/null +++ b/.yoi/tickets/00001KXTKS0VG/resolution.md @@ -0,0 +1 @@ +Worker stop/delete no longer cleans up Workdir materialization. Spawn failure rollback is limited to newly materialized Workdirs and uses Workdir cleanup by id so rollback does not leave corrupted records. Existing Workdir binds survive spawn failures. diff --git a/.yoi/tickets/00001KXTKS0VG/thread.md b/.yoi/tickets/00001KXTKS0VG/thread.md new file mode 100644 index 00000000..79f36a76 --- /dev/null +++ b/.yoi/tickets/00001KXTKS0VG/thread.md @@ -0,0 +1,89 @@ + + +## 作成 + +LocalTicketBackend によって作成されました。 + +--- + + + +## Intake summary + +Marked ready by `yoi ticket state`. + +--- + + + +## State changed + +Marked ready by `yoi ticket state`. + + +--- + + + +## State changed + +Ticket を `yoi ticket` が queued にしました。 + + +--- + + + +## State changed + +State changed to `inprogress`. + + +--- + + + +## Implementation report + +Worker lifecycle と Workdir lifecycle を分離した。 + +変更点: +- `stop_worker` から `materializer.cleanup(binding)` を削除し、Worker stop/delete で Workdir 実体を削除しないようにした。 +- live execution map の `working_directory` field は stop cleanup 用だけだったため削除した。 +- `spawn_worker` の failure rollback 対象を「この spawn request で新規 materialize した binding」だけに限定した。 +- rollback は `cleanup(binding)` ではなく `cleanup_working_directory(id)` を使い、worktree 実体だけでなく record root も消して corrupted 残骸を残さないようにした。 +- 既存 Workdir bind の spawn failure では Workdir を cleanup しない。 + +追加テスト: +- `stopping_and_deleting_worker_preserves_bound_working_directory` +- `spawn_failure_with_existing_working_directory_preserves_workdir` +- `spawn_failure_with_new_materialization_rolls_back_workdir_record` + +検証: +- `cargo test -p worker-runtime --features ws-server,fs-store stopping_and_deleting_worker_preserves_bound_working_directory --quiet` +- `cargo test -p worker-runtime --features ws-server,fs-store spawn_failure_with_existing_working_directory_preserves_workdir --quiet` +- `cargo test -p worker-runtime --features ws-server,fs-store spawn_failure_with_new_materialization_rolls_back_workdir_record --quiet` +- `cargo test -p worker-runtime --features ws-server,fs-store worker_backend --quiet` +- `cargo fmt --check` +- `git diff --check` + + +--- + + + +## State changed + +Ticket を closed にしました。 + + +--- + + + +## 完了 + +Worker stop/delete no longer cleans up Workdir materialization. Spawn failure rollback is limited to newly materialized Workdirs and uses Workdir cleanup by id so rollback does not leave corrupted records. Existing Workdir binds survive spawn failures. + + +--- diff --git a/crates/worker-runtime/src/worker_backend.rs b/crates/worker-runtime/src/worker_backend.rs index 4772ee03..18071c9f 100644 --- a/crates/worker-runtime/src/worker_backend.rs +++ b/crates/worker-runtime/src/worker_backend.rs @@ -543,7 +543,6 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { struct RuntimeWorkerExecution { handle: WorkerHandle, busy: Arc, - working_directory: Option, } /// `worker-runtime` execution backend backed by real `worker` crate Workers. @@ -721,14 +720,7 @@ where )); } }; - workers.insert( - worker_ref.clone(), - RuntimeWorkerExecution { - handle, - busy, - working_directory: working_directory.clone(), - }, - ); + workers.insert(worker_ref.clone(), RuntimeWorkerExecution { handle, busy }); WorkerExecutionSpawnResult::Connected { handle: WorkerExecutionHandle::new(worker_ref, self.backend_id()), @@ -816,6 +808,7 @@ where } let mut request = request; + let mut rollback_working_directory = None; let working_directory = match ( request.request.working_directory_request.as_ref(), request.request.working_directory.as_ref(), @@ -836,6 +829,7 @@ where match materializer.materialize(&request.worker_ref, working_directory_request) { Ok(binding) => { request.working_directory = Some(binding.clone()); + rollback_working_directory = Some(binding.clone()); Some(binding) } Err(error) => { @@ -887,9 +881,9 @@ where Err(message) => { if let (Some(materializer), Some(binding)) = ( self.working_directory_materializer.as_ref(), - working_directory.as_ref(), + rollback_working_directory.as_ref(), ) { - let _ = materializer.cleanup(binding); + let _ = materializer.cleanup_working_directory(&binding.working_directory.id); } return WorkerExecutionSpawnResult::Errored(WorkerExecutionResult::errored( WorkerExecutionOperation::Spawn, @@ -1089,19 +1083,12 @@ where "execution handle does not reference a live Worker", ); }; - let result = self.send_method( + self.send_method( WorkerExecutionOperation::Stop, execution.handle, Method::Shutdown, WorkerExecutionRunState::Stopped, - ); - if let (Some(materializer), Some(binding)) = ( - self.working_directory_materializer.as_ref(), - execution.working_directory.as_ref(), - ) { - let _ = materializer.cleanup(binding); - } - result + ) } fn cancel_worker(&self, handle: &WorkerExecutionHandle) -> WorkerExecutionResult { @@ -1162,7 +1149,8 @@ mod tests { use crate::Runtime as EmbeddedRuntime; use crate::catalog::{ ConfigBundleRef, CreateWorkerRequest, MaterializerKind, ProfileSelector, - RepositorySelector, WorkingDirectoryRepository, WorkingDirectoryRequest, + RepositorySelector, WorkingDirectoryClaim, WorkingDirectoryRepository, + WorkingDirectoryRequest, }; use crate::execution::WorkerExecutionContext; use crate::management::RuntimeOptions; @@ -1404,6 +1392,26 @@ mod tests { assert!(status.success(), "git {:?} failed", args); } + #[derive(Clone)] + struct FailingFactory; + + #[async_trait] + impl RuntimeWorkerFactory for FailingFactory { + async fn spawn_controller( + &self, + _request: WorkerExecutionSpawnRequest, + ) -> Result { + Err("spawn failed".to_string()) + } + + async fn restore_controller( + &self, + _request: WorkerExecutionRestoreRequest, + ) -> Result { + Err("restore failed".to_string()) + } + } + fn create_clean_repo() -> tempfile::TempDir { let dir = tempfile::tempdir().unwrap(); git(dir.path(), &["init"]); @@ -1432,6 +1440,17 @@ mod tests { } } + fn materialized_worktree_root( + runtime_base: &std::path::Path, + working_directory_id: &str, + ) -> PathBuf { + runtime_base + .join("working-directories") + .join(working_directory_id) + .join("root") + .join("repo-main") + } + #[test] fn runtime_worker_name_is_runtime_local() { let worker_ref = crate::identity::WorkerRef::new(crate::identity::WorkerId::new(1)); @@ -1617,4 +1636,117 @@ mod tests { assert!(!cwd.starts_with(repo.path())); assert!(cwd.join("README.md").exists()); } + + #[test] + fn stopping_and_deleting_worker_preserves_bound_working_directory() { + let client = MockClient::new(simple_text_events()); + let runtime_base = tempfile::tempdir().unwrap(); + let repo = create_clean_repo(); + let store = tempfile::tempdir().unwrap(); + let factory = MockFactory { + client, + runtime_base: runtime_base.path().to_path_buf(), + cwd: repo.path().to_path_buf(), + store_dir: store.path().join("sessions"), + worker_metadata_dir: store.path().join("workers"), + observed_cwds: Arc::new(Mutex::new(Vec::new())), + observed_workspace_clients: Arc::new(Mutex::new(Vec::new())), + }; + let backend = WorkerRuntimeExecutionBackend::new(factory) + .unwrap() + .with_working_directory_materializer(LocalGitWorktreeMaterializer::new( + runtime_base.path(), + )); + let runtime = + EmbeddedRuntime::with_execution_backend(RuntimeOptions::default(), Arc::new(backend)) + .unwrap(); + runtime.store_config_bundle(test_bundle()).unwrap(); + let mut request = create_request("chat"); + request.working_directory_request = Some(working_directory_request(repo.path())); + let detail = runtime.create_worker(request).unwrap(); + let workdir_id = detail + .execution + .working_directory + .as_ref() + .unwrap() + .summary + .working_directory_id + .clone(); + let worktree_root = materialized_worktree_root(runtime_base.path(), &workdir_id); + assert!(worktree_root.join("README.md").exists()); + + runtime.stop_worker(&detail.worker_ref, None).unwrap(); + runtime.delete_worker(&detail.worker_ref).unwrap(); + + assert!(worktree_root.join("README.md").exists()); + let status = runtime.working_directory(&workdir_id).unwrap(); + assert_eq!( + status.summary.status, + crate::catalog::WorkingDirectoryStatusKind::Active + ); + assert_eq!(status.summary.cleanliness.as_deref(), Some("clean")); + assert_eq!(status.summary.primary_worker_id, None); + } + + #[test] + fn spawn_failure_with_existing_working_directory_preserves_workdir() { + let runtime_base = tempfile::tempdir().unwrap(); + let repo = create_clean_repo(); + let backend = WorkerRuntimeExecutionBackend::new(FailingFactory) + .unwrap() + .with_working_directory_materializer(LocalGitWorktreeMaterializer::new( + runtime_base.path(), + )); + let runtime = + EmbeddedRuntime::with_execution_backend(RuntimeOptions::default(), Arc::new(backend)) + .unwrap(); + runtime.store_config_bundle(test_bundle()).unwrap(); + let status = runtime + .create_working_directory(working_directory_request(repo.path())) + .unwrap(); + let workdir_id = status.summary.working_directory_id.clone(); + let worktree_root = materialized_worktree_root(runtime_base.path(), &workdir_id); + assert!(worktree_root.join("README.md").exists()); + let mut request = create_request("chat"); + request.working_directory = Some(WorkingDirectoryClaim { + working_directory_id: workdir_id.clone(), + relative_cwd: None, + }); + + let error = runtime.create_worker(request).unwrap_err(); + + assert!(format!("{error:?}").contains("spawn failed")); + assert!(worktree_root.join("README.md").exists()); + let status = runtime.working_directory(&workdir_id).unwrap(); + assert_eq!( + status.summary.status, + crate::catalog::WorkingDirectoryStatusKind::Active + ); + } + + #[test] + fn spawn_failure_with_new_materialization_rolls_back_workdir_record() { + let runtime_base = tempfile::tempdir().unwrap(); + let repo = create_clean_repo(); + let backend = WorkerRuntimeExecutionBackend::new(FailingFactory) + .unwrap() + .with_working_directory_materializer(LocalGitWorktreeMaterializer::new( + runtime_base.path(), + )); + let runtime = + EmbeddedRuntime::with_execution_backend(RuntimeOptions::default(), Arc::new(backend)) + .unwrap(); + runtime.store_config_bundle(test_bundle()).unwrap(); + let mut request = create_request("chat"); + request.working_directory_request = Some(working_directory_request(repo.path())); + + let error = runtime.create_worker(request).unwrap_err(); + + assert!(format!("{error:?}").contains("spawn failed")); + let working_directories_root = runtime_base.path().join("working-directories"); + let remaining_entries = fs::read_dir(working_directories_root) + .map(|entries| entries.count()) + .unwrap_or(0); + assert_eq!(remaining_entries, 0); + } }