From 5564425488ed0b34c3c872d2d09043abadc95a5d Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 12 Sep 2026 13:30:25 +0900 Subject: [PATCH] fix: retry retained workdir removal --- crates/workspace-server/src/server.rs | 27 ++++++++++++++++++- .../workspace-server/src/workdir_removal.rs | 18 ++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 528293ad..b3f26088 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -11352,7 +11352,9 @@ fn execute_reserved_workdir_removal_with_provider( recovery: bool, provider: &dyn WorkdirRemovalRuntimeProvider, ) -> Result { - if operation.state == WorkdirRemovalOperationState::Completed { + if operation.state == WorkdirRemovalOperationState::Completed + && operation.disposition == Some(WorkdirRemovalDisposition::Removed) + { return Ok(operation); } let operation = if recovery && operation.state == WorkdirRemovalOperationState::Pending { @@ -26667,6 +26669,7 @@ mod tests { assert_eq!(unknown_provider.cleanup_calls(), 0); let (dirty_operation, mut dirty_summary) = reserve_removal_fixture(&api, "provider-dirty"); + let clean_retry_summary = dirty_summary.clone(); dirty_summary.cleanliness = Some("dirty".to_string()); let dirty_provider = FakeWorkdirRemovalProvider::new( workdir_removal_result( @@ -26686,6 +26689,28 @@ mod tests { assert_eq!(dirty.disposition, Some(WorkdirRemovalDisposition::Retained)); assert_eq!(dirty_provider.cleanup_calls(), 0); + let clean_retry_provider = FakeWorkdirRemovalProvider::new( + workdir_removal_result( + WorkerOperationState::Accepted, + Some(clean_retry_summary), + Vec::new(), + ), + workdir_removal_result(WorkerOperationState::Accepted, None, Vec::new()), + ); + let removed_after_retry = execute_reserved_workdir_removal_with_provider( + &api, + dirty, + false, + &clean_retry_provider, + ) + .unwrap(); + assert_eq!( + removed_after_retry.disposition, + Some(WorkdirRemovalDisposition::Removed) + ); + assert_eq!(removed_after_retry.attempt_count, 2); + assert_eq!(clean_retry_provider.cleanup_calls(), 1); + let (corrupted_operation, mut corrupted_summary) = reserve_removal_fixture(&api, "provider-corrupted"); corrupted_summary.status = WorkingDirectoryStatusKind::Corrupted; diff --git a/crates/workspace-server/src/workdir_removal.rs b/crates/workspace-server/src/workdir_removal.rs index 3f6cbc79..5edea488 100644 --- a/crates/workspace-server/src/workdir_removal.rs +++ b/crates/workspace-server/src/workdir_removal.rs @@ -264,7 +264,9 @@ impl SqliteWorkspaceStore { let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; let operation = require_operation(&tx, workspace_id, operation_id, request_fingerprint)?; - if operation.state == WorkdirRemovalOperationState::Completed { + if operation.state == WorkdirRemovalOperationState::Completed + && operation.disposition == Some(WorkdirRemovalDisposition::Removed) + { tx.commit()?; return Ok(operation); } @@ -1197,5 +1199,19 @@ mod tests { .unwrap() .is_some() ); + + let retry = store + .begin_workdir_removal_attempt( + &retained.workspace_id, + &retained.operation_id, + &retained.request_fingerprint, + attempt_owner(), + ) + .unwrap(); + assert_eq!(retry.state, WorkdirRemovalOperationState::Pending); + assert_eq!(retry.attempt_count, 1); + assert_eq!(retry.disposition, None); + assert_eq!(retry.failure_category, None); + assert!(retry.retryable); } }