runtime: preserve workdirs across worker deletion
This commit is contained in:
parent
de3416cf3b
commit
5f651755d8
0
.yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep
Normal file
0
.yoi/tickets/00001KXTKS0VG/artifacts/.gitkeep
Normal file
29
.yoi/tickets/00001KXTKS0VG/item.md
Normal file
29
.yoi/tickets/00001KXTKS0VG/item.md
Normal file
|
|
@ -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 が通る。
|
||||
1
.yoi/tickets/00001KXTKS0VG/resolution.md
Normal file
1
.yoi/tickets/00001KXTKS0VG/resolution.md
Normal file
|
|
@ -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.
|
||||
89
.yoi/tickets/00001KXTKS0VG/thread.md
Normal file
89
.yoi/tickets/00001KXTKS0VG/thread.md
Normal file
|
|
@ -0,0 +1,89 @@
|
|||
<!-- event: create author: "yoi ticket" at: 2026-07-18T12:38:47Z -->
|
||||
|
||||
## 作成
|
||||
|
||||
LocalTicketBackend によって作成されました。
|
||||
|
||||
---
|
||||
|
||||
<!-- event: intake_summary author: hare at: 2026-07-18T12:39:18Z -->
|
||||
|
||||
## Intake summary
|
||||
|
||||
Marked ready by `yoi ticket state`.
|
||||
|
||||
---
|
||||
|
||||
<!-- event: state_changed author: "yoi ticket" at: 2026-07-18T12:39:18Z from: planning to: ready reason: cli_state field: state -->
|
||||
|
||||
## State changed
|
||||
|
||||
Marked ready by `yoi ticket state`.
|
||||
|
||||
|
||||
---
|
||||
|
||||
<!-- event: state_changed author: "yoi ticket" at: 2026-07-18T12:39:18Z from: ready to: queued reason: queued field: state -->
|
||||
|
||||
## State changed
|
||||
|
||||
Ticket を `yoi ticket` が queued にしました。
|
||||
|
||||
|
||||
---
|
||||
|
||||
<!-- event: state_changed author: "yoi ticket" at: 2026-07-18T12:39:18Z from: queued to: inprogress reason: cli_state field: state -->
|
||||
|
||||
## State changed
|
||||
|
||||
State changed to `inprogress`.
|
||||
|
||||
|
||||
---
|
||||
|
||||
<!-- event: implementation_report author: hare at: 2026-07-18T12:49:12Z -->
|
||||
|
||||
## 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`
|
||||
|
||||
|
||||
---
|
||||
|
||||
<!-- event: state_changed author: hare at: 2026-07-18T12:49:12Z from: inprogress to: closed reason: closed field: state -->
|
||||
|
||||
## State changed
|
||||
|
||||
Ticket を closed にしました。
|
||||
|
||||
|
||||
---
|
||||
|
||||
<!-- event: close author: hare at: 2026-07-18T12:49:12Z status: 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.
|
||||
|
||||
|
||||
---
|
||||
|
|
@ -543,7 +543,6 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory {
|
|||
struct RuntimeWorkerExecution {
|
||||
handle: WorkerHandle,
|
||||
busy: Arc<AtomicBool>,
|
||||
working_directory: Option<WorkingDirectoryBinding>,
|
||||
}
|
||||
|
||||
/// `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<WorkerHandle, String> {
|
||||
Err("spawn failed".to_string())
|
||||
}
|
||||
|
||||
async fn restore_controller(
|
||||
&self,
|
||||
_request: WorkerExecutionRestoreRequest,
|
||||
) -> Result<WorkerHandle, String> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user