diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 5bc654a2a..4e1f1b0cd 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -17775,9 +17775,15 @@ async fn scoped_attach_current_worker_workdir( code: "working_directory_not_found".to_string(), message: format!("unknown Workdir `{workdir_id}`"), })?; - if workdir.materialization_status != "present" { + // A failed removal must not prevent its owner from attaching a still usable + // repository checkout to resolve current blockers. Runtime binding validates + // the actual checkout; durable pending-removal reservations still fence this + // mutation. Other sources and missing/corrupted materializations stay denied. + let repository_retry = workdir.materialization_status == "pending" + && matches!(workdir.source, WorkdirRegistrySource::Repository { .. }); + if workdir.materialization_status != "present" && !repository_retry { return Err(Error::WorkdirAttachmentConflict(format!( - "Workdir `{workdir_id}` is not active ({})", + "Workdir `{workdir_id}` is not available for attachment ({})", workdir.materialization_status )) .into()); @@ -53649,6 +53655,91 @@ mod tests { assert_eq!(response.status(), StatusCode::FORBIDDEN); } + #[tokio::test] + async fn current_worker_can_reattach_usable_pending_checkout_after_failed_removal_but_not_during_attempt() + { + let mut fixture = manual_worker_assignment_fixture().await; + let identity = + worker_runtime::auth::RuntimeIdentityMaterial::generate(&fixture.worker.runtime_id) + .unwrap(); + configure_runtime_request_auth(&mut fixture.api, &identity, &fixture.worker.runtime_id); + let api = &fixture.api; + api.store + .detach_worker_workdir( + TEST_WORKSPACE_ID, + &fixture.worker, + Some(&fixture.docs_workdir_id), + "release for removal", + ) + .unwrap(); + let mut record = api + .store + .get_workdir_registry(TEST_WORKSPACE_ID, &fixture.docs_workdir_id) + .unwrap() + .unwrap(); + record.materialization_status = "pending".into(); + api.store.upsert_workdir_registry(&record).unwrap(); + let intent = + workdir_removal_intent(&record, "account:owner", "remove docs checkout").unwrap(); + let pending = api + .config_store + .reserve_workdir_removal_operation(&intent) + .unwrap(); + let path = format!("/api/w/{TEST_WORKSPACE_ID}/workers/self/workdir-attachments"); + let request = || { + runtime_source_request( + &identity, + Some(&fixture.worker.worker_id), + "POST", + &path, + serde_json::to_vec( + &json!({ "alias": "docs", "working_directory_id": fixture.docs_workdir_id }), + ) + .unwrap(), + ) + }; + let blocked = build_router(api.clone()).oneshot(request()).await.unwrap(); + let blocked_status = blocked.status(); + let blocked_body = to_bytes(blocked.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + blocked_status, + StatusCode::CONFLICT, + "{}", + String::from_utf8_lossy(&blocked_body) + ); + api.config_store + .fail_workdir_removal_operation(&pending, "provider_unavailable", true) + .unwrap(); + let attached = build_router(api.clone()).oneshot(request()).await.unwrap(); + let attached_status = attached.status(); + let attached_body = to_bytes(attached.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + attached_status, + StatusCode::OK, + "{}", + String::from_utf8_lossy(&attached_body) + ); + let attached: server_api::CurrentWorkerWorkdirAttachmentResponse = + serde_json::from_slice(&attached_body).unwrap(); + assert!(attached.attached); + assert_eq!(attached.working_directory_id, fixture.docs_workdir_id); + assert_eq!( + api.store + .get_workdir_registry(TEST_WORKSPACE_ID, &fixture.docs_workdir_id) + .unwrap() + .unwrap() + .materialization_status, + "pending" + ); + assert!( + api.config_store + .workdir_removal_guards(&pending) + .unwrap() + .iter() + .any(|guard| guard.category == "active_attachment") + ); + } + #[tokio::test] async fn current_worker_attachment_list_and_stale_detach_use_signed_identity() { let mut fixture = manual_worker_assignment_fixture().await; diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 9d78665a7..2388611c3 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -10254,6 +10254,19 @@ impl ControlPlaneStore for SqliteWorkspaceStore { record.workdir_id, active.worker.runtime_id, active.worker.worker_id ))); } + // Direct attachments (including ordinary recovery/reconnection) + // must share the same atomic removal fence as reserved attachments. + let removal_pending: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM workdir_removal_operations WHERE workspace_id=?1 AND workdir_id=?2 AND state='pending')", + params![record.workspace_id, record.workdir_id], + |row| row.get(0), + )?; + if removal_pending { + return Err(Error::WorkdirAttachmentConflict(format!( + "Workdir {} has a pending durable removal operation", + record.workdir_id + ))); + } let write = tx.execute( r#"INSERT INTO worker_workdir_links ( workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, linked_at, unlinked_at, connection_id diff --git a/docs/development/workdir-removal.md b/docs/development/workdir-removal.md index 097efb0a2..1adf5cdf0 100644 --- a/docs/development/workdir-removal.md +++ b/docs/development/workdir-removal.md @@ -10,6 +10,11 @@ reported cause. Each attempt rechecks current attachments, attachment reservations, retention holds, materialization identity and current file changes. A previous clean observation is not permission to discard new changes. A dirty Workdir must be preserved or its changes resolved by its owner before deletion. +When the same repository checkout is still usable, an authorized Worker may +reattach it through normal `WorkdirAttach` to preserve or resolve changes, then +detach and retry removal. An executing removal fences new attachments; a failed +attempt does not permanently prevent reconnection. Missing or replaced checkout +identity is not made usable by this permission. ## Interpreting a retained Workdir