From b0733e53b22b51f5e2c6f730393553fa0e7e1b32 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 28 Oct 2026 12:15:35 +0900 Subject: [PATCH 1/6] fix: release removed Subject bodies while preserving journal history --- crates/subjektiv/src/journal/binding.rs | 23 + crates/subjektiv/src/journal/store.rs | 26 + crates/workspace-server/src/retention.rs | 24 +- crates/workspace-server/src/server.rs | 71 +- .../src/server/subjektiv_journal.rs | 419 +++++--- .../server/tests/subjektiv_journal_tests.rs | 974 +++++++++++++++++- crates/workspace-server/src/store.rs | 6 +- 7 files changed, 1370 insertions(+), 173 deletions(-) diff --git a/crates/subjektiv/src/journal/binding.rs b/crates/subjektiv/src/journal/binding.rs index 3430a72cb..9267cd50c 100644 --- a/crates/subjektiv/src/journal/binding.rs +++ b/crates/subjektiv/src/journal/binding.rs @@ -194,6 +194,29 @@ impl SubjektivStore { }) } + /// End a body after the Host has confirmed its Runtime removal. This is not a + /// completed Journal handoff: source coverage, positions and saved receipts stay + /// unchanged. Revocation and closing generations are atomic, so no retained Job + /// can write after deletion even while Backend Job cleanup is being retried. + pub fn release_removed_journal_body(&self, host: &JournalHostContext) -> JournalResult<()> { + self.database.try_transaction(|tx| { + super::store::check_session_host(self, tx, host)?; + let attr = host.attribution.as_ref().expect("checked Session authority"); + let binding: Option = tx.query_row( + "SELECT binding_id FROM journal_writer_bindings WHERE subject_id=?1 AND runtime_id=?2 AND worker_id=?3 ORDER BY rowid DESC LIMIT 1", + params![host.subject_id, attr.runtime_id, attr.worker_id], |r| r.get(0), + ).optional()?; + let binding = binding.unwrap_or_else(|| crate::issued_id("journal-removed-body")); + tx.execute("INSERT OR IGNORE INTO journal_writer_bindings(binding_id,subject_id,runtime_id,worker_id,active) VALUES(?1,?2,?3,?4,0)",params![binding,host.subject_id,attr.runtime_id,attr.worker_id])?; + tx.execute("UPDATE journal_writer_bindings SET active=0 WHERE subject_id=?1 AND runtime_id=?2 AND worker_id=?3",params![host.subject_id,attr.runtime_id,attr.worker_id])?; + // Bind legacy attributed Sessions to an inactive lifetime, never to the + // successor. Retained unfinished history is not an active writer. + tx.execute("INSERT OR IGNORE INTO journal_session_bindings(session_id,binding_id) SELECT session_id,?4 FROM subject_session_attributions WHERE subject_id=?1 AND runtime_id=?2 AND worker_id=?3",params![host.subject_id,attr.runtime_id,attr.worker_id,binding])?; + tx.execute("UPDATE journal_generations SET active=0 WHERE journal_id IN (SELECT j.id FROM journals j JOIN subject_session_attributions a ON a.session_id=j.source_session_id WHERE a.subject_id=?1 AND a.runtime_id=?2 AND a.worker_id=?3)",params![host.subject_id,attr.runtime_id,attr.worker_id])?; + Ok(()) + }) + } + /// The Host must hold its lifecycle fence, confirm Idle, reconcile all committed original /// Sessions (including the save/notification gap), and exclude queued/running/Unknown Jobs. /// This transaction rejects retained writers, pending source bytes and active generations. diff --git a/crates/subjektiv/src/journal/store.rs b/crates/subjektiv/src/journal/store.rs index 72008f1b1..a5e4bedf4 100644 --- a/crates/subjektiv/src/journal/store.rs +++ b/crates/subjektiv/src/journal/store.rs @@ -651,6 +651,32 @@ impl SubjektivStore { Ok(()) }) } + /// Read the exact retained Job/attempt snapshot without granting writer + /// authority or creating a generation. Used for outcome recovery after removal. + pub fn retained_journal_generation( + &self, + host: &JournalHostContext, + id: &str, + binding: &JobAttemptBinding, + ) -> JournalResult> { + self.database.try_with_connection(|tx| { + check_host(self, tx, host, false)?; + check_session_host(self, tx, host)?; + journal(tx, host, id, true)?; + let raw: Option = tx.query_row( + "SELECT snapshot_json FROM journal_generations WHERE journal_id=?1 AND job_id=?2 AND attempt_id=?3", + params![id, binding.job_id, binding.attempt_id], |r| r.get(0), + ).optional()?; + raw.map(|raw| { + let generation: JournalGeneration = serde_json::from_str(&raw)?; + if generation.journal_id != id || &generation.job != binding { + return Err(conflict("retained generation identity mismatch")); + } + Ok(generation) + }).transpose() + }) + } + pub fn read_journal_generation( &self, host: &JournalHostContext, diff --git a/crates/workspace-server/src/retention.rs b/crates/workspace-server/src/retention.rs index b8469122e..3b01d553e 100644 --- a/crates/workspace-server/src/retention.rs +++ b/crates/workspace-server/src/retention.rs @@ -325,7 +325,7 @@ impl SqliteWorkspaceStore { let now = Utc::now().to_rfc3339(); self.with_conn_mut(|conn| { let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; - let policy=load_removal_policy(&tx,&req.workspace_id,&req.worker,erased_journal)?.ok_or_else(|| StoreError::InvalidInput(format!("policy-missing:{}",req.workspace_id)))?; + let policy=load_removal_policy(&tx,&req.workspace_id,&req.worker,erased_journal,true)?.ok_or_else(|| StoreError::InvalidInput(format!("policy-missing:{}",req.workspace_id)))?; let worker=match load_worker(&tx,&req.workspace_id,&req.worker)? { Some(v)=>v, None=>{ @@ -371,7 +371,7 @@ impl SqliteWorkspaceStore { return Err(StoreError::InvalidInput(format!("stale:{plan_id}:plan state {} is not executable", state_s(plan.state)))); } if !plan.blockers.is_empty(){return Err(StoreError::InvalidInput(format!("blocked:{}",serde_json::to_string(&plan.blockers).unwrap())));} - let policy=load_removal_policy(&tx,workspace_id,&plan.worker,plan.policy_id==ERASED_JOURNAL_POLICY_ID)?.ok_or_else(||StoreError::InvalidInput(format!("policy-missing:{workspace_id}")))?; + let policy=load_removal_policy(&tx,workspace_id,&plan.worker,plan.policy_id==ERASED_JOURNAL_POLICY_ID,plan.policy_id=="backend-subject-removal-archive-v1")?.ok_or_else(||StoreError::InvalidInput(format!("policy-missing:{workspace_id}")))?; if policy.policy_id!=plan.policy_id||policy.policy_digest!=plan.policy_digest{ mark_stale(&tx,&plan,"policy content changed")?;tx.commit()?; return Err(stale_error(&plan,"policy content changed")); @@ -1020,6 +1020,7 @@ fn load_removal_policy( w: &str, worker: &RuntimeWorkerRef, erased_journal: bool, + preserve_subject_history: bool, ) -> crate::Result> { let Some(mut policy) = load_policy(c, w)? else { return Ok(None); @@ -1044,6 +1045,25 @@ fn load_removal_policy( policy.archive_retention = update.archive_retention; policy.diagnostics_disposition = update.diagnostics_disposition; policy.diagnostics_retention_seconds = None; + } else if preserve_subject_history && policy.session_disposition == SessionDisposition::Purge && c.query_row( + "SELECT EXISTS(SELECT 1 FROM worker_create_reservations WHERE workspace_id=?1 AND runtime_id=?2 AND worker_id=?3 AND substr(singleton_key,1,10)='subjektiv:')", + params![w, worker.runtime_id, worker.worker_id], + |r| r.get::<_, bool>(0), + )? { + // Removing a Subject body releases occupancy, not its original history. + // A configured Purge must not silently erase an unfinished Journal. Explicit + // Session erasure uses the separate, proof-bound branch above. + let update = WorkerRetentionPolicyUpdate { + policy_id: "backend-subject-removal-archive-v1".into(), + session_disposition: SessionDisposition::Archive, + metadata_disposition: policy.metadata_disposition, + archive_retention: policy.archive_retention, + diagnostics_disposition: policy.diagnostics_disposition, + diagnostics_retention_seconds: policy.diagnostics_retention_seconds, + }; + policy.policy_id = update.policy_id.clone(); + policy.policy_digest = worker_retention_policy_digest(&update); + policy.session_disposition = update.session_disposition; } Ok(Some(policy)) } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index cc505f91a..bca0a9580 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -2327,16 +2327,29 @@ impl WorkerRemovalService { )); } }; - match self.worker_projection.publish_ordered(|| { - self.store - .commit_worker_removal( - &self.workspace_id, - &prepared.plan.operation_id, - &prepared.plan.input_fingerprint, - &result, - ) - .map(|commit| ((), commit.catalog)) - }) { + let commit_result = + match subjektiv_journal::finish_worker_removal(self, target, &prepared, &result, || { + self.worker_projection.publish_ordered(|| { + self.store + .commit_worker_removal( + &self.workspace_id, + &prepared.plan.operation_id, + &prepared.plan.input_fingerprint, + &result, + ) + .map(|commit| ((), commit.catalog)) + }) + }) { + Ok(result) => result, + Err(error) => { + return Ok(worker_remove_error_response( + StatusCode::CONFLICT, + "subject_removal_completion_unconfirmed", + &error.to_string(), + )); + } + }; + match commit_result { Ok(()) => { subjektiv_journal::after_worker_removal(self, &result); Ok(worker_remove_success_response(target)) @@ -2722,16 +2735,34 @@ impl WorkerRemovalService { )); } }; - match self.worker_projection.publish_ordered(|| { - self.store - .commit_worker_removal( - &self.workspace_id, - &plan.operation_id, - &plan.input_fingerprint, - &retention_result, - ) - .map(|commit| ((), commit.catalog)) - }) { + let commit_result = match subjektiv_journal::finish_worker_removal( + self, + target, + &prepared, + &retention_result, + || { + self.worker_projection.publish_ordered(|| { + self.store + .commit_worker_removal( + &self.workspace_id, + &plan.operation_id, + &plan.input_fingerprint, + &retention_result, + ) + .map(|commit| ((), commit.catalog)) + }) + }, + ) { + Ok(result) => result, + Err(error) => { + return Ok(worker_remove_error_response( + StatusCode::CONFLICT, + "subject_removal_completion_unconfirmed", + &error.to_string(), + )); + } + }; + match commit_result { Ok(()) => {} Err(error) => { let _ = self.store.fail_worker_removal( diff --git a/crates/workspace-server/src/server/subjektiv_journal.rs b/crates/workspace-server/src/server/subjektiv_journal.rs index fccee17a6..fded1c5fb 100644 --- a/crates/workspace-server/src/server/subjektiv_journal.rs +++ b/crates/workspace-server/src/server/subjektiv_journal.rs @@ -92,6 +92,20 @@ pub(super) fn prepare(api: &WorkspaceApi, reservation: &BackendJobReservation) - }; let store = store(api)?; let host = host(api, &store, grant)?; + let attr = store + .session_attribution(&grant.session_id) + .map_err(|e| Error::Store(e.to_string()))? + .ok_or_else(|| { + Error::WorkspacePermissionDenied("Journal Session attribution missing".into()) + })?; + if store + .journal_worker_was_released(&attr.subject_id, &attr.runtime_id, &attr.worker_id) + .map_err(domain)? + { + return Err(Error::WorkspacePermissionDenied( + "released Journal body cannot dispatch another attempt".into(), + )); + } let input = saved_input(&reservation.job.request, grant)?; let g = store .prepare_journal_input( @@ -584,6 +598,18 @@ fn dispatch( .lock() .map_err(|_| Error::Store("Journal lifecycle lock poisoned".into()))?; let store = store(api)?; + if store + .journal_worker_was_released( + &attribution.subject_id, + &attribution.runtime_id, + &attribution.worker_id, + ) + .map_err(domain)? + { + // Deletion can retain an unfinished prefix. It must not mint attempts or + // transfer the removed body's write capability to its successor. + return Ok(()); + } let host = JournalHostContext::validated_session(&store, attribution.clone()).map_err(domain)?; if let Some(g) = store @@ -1740,8 +1766,8 @@ fn finish_erased_release( .release_subject_singleton(api.workspace_id(), &lease, binding_id, &operation) } -/// Normal deletion cannot substitute for Subject release, even for a stopped or disabled body. -/// Explicit erasure remains a separate monotonic source/diagnostics destruction path. +/// Ordinary removal owns writer revocation and occupancy release. Before execution +/// only preserve/erase source requirements are checked; an active writer is not a blocker. pub(super) fn prepare_worker_removal( service: &WorkerRemovalService, runtime: &RuntimeRegistry, @@ -1762,7 +1788,6 @@ pub(super) fn prepare_worker_removal( &service.journal_registration, ) .map_err(|e| Error::Store(e.to_string()))?; - let host = JournalHostContext::validated_session(&store, attr.clone()).map_err(domain)?; let source = runtime_api::SessionPublicSource::Retained { worker_id: worker_runtime::identity::WorkerId::parse(&target.worker_id) .ok_or_else(|| Error::Store("Worker identity invalid".into()))?, @@ -1779,25 +1804,9 @@ pub(super) fn prepare_worker_removal( finish_erased_release(&api, &attr)?; return Ok(Some(proof)); } - let Some(binding_id) = host.writer_binding_id() else { - return Err(Error::RepositoryConflict( - "Subject writer evidence is missing; ordinary deletion cannot bypass release".into(), - )); - }; - let writer = JournalWriterBinding { - subject_id: attr.subject_id.clone(), - runtime_id: attr.runtime_id.clone(), - worker_id: attr.worker_id.clone(), - binding_id: binding_id.into(), - }; - if store.journal_writer_active(&writer).map_err(domain)? { - return Err(Error::RepositoryConflict("Subject remains occupied: restore to actual Idle, finish Journal recording, then explicitly release before ordinary deletion".into())); - } - if store.journal_retention_required(&host).map_err(domain)? { - return Err(Error::RepositoryConflict( - "Released Subject has unresolved Journal material; retain occupancy and source".into(), - )); - } + // Do not require a live Controller, complete Journal, or prior release here. + // Runtime removal is the physical fence; finish_worker_removal revokes the + // exact old body only after its successful, matching retention receipt. Ok(None) } @@ -1886,20 +1895,6 @@ pub(super) fn validate_removal_retention( // its retained permanent fence, including recovery after the aggregate is gone. return Ok(()); } - let binding_id = host.writer_binding_id().ok_or_else(|| { - Error::RepositoryConflict("Subject writer evidence missing during removal recovery".into()) - })?; - let binding = JournalWriterBinding { - subject_id: attr.subject_id.clone(), - runtime_id: attr.runtime_id.clone(), - worker_id: attr.worker_id.clone(), - binding_id: binding_id.into(), - }; - if store.journal_writer_active(&binding).map_err(domain)? { - return Err(Error::RepositoryConflict( - "Prepared removal cannot bypass explicit Subject release".into(), - )); - } let source = runtime_api::SessionPublicSource::Retained { worker_id: worker_runtime::identity::WorkerId::parse(&target.worker_id) .ok_or_else(|| Error::Store("Worker identity invalid".into()))?, @@ -1907,37 +1902,140 @@ pub(super) fn validate_removal_retention( let runtime = service.runtime.upgrade().ok_or_else(|| { Error::Store("Runtime unavailable for source retention confirmation".into()) })?; - if store.journal_retention_required(&host).map_err(domain)? { - if prepared.runtime_request.session_disposition - != worker_runtime::retention::SessionDisposition::Archive - || prepared.runtime_request.archive_id.is_none() - { - return Err(Error::RepositoryConflict( - "unreflected Journal requires a retained original Session archive".into(), - )); + let retention_required = store.journal_retention_required(&host).map_err(domain)?; + if prepared.runtime_request.session_disposition + == worker_runtime::retention::SessionDisposition::Purge + && prepared.runtime_request.archive_id.is_none() + && !retention_required + && store + .journal_worker_was_released(&attr.subject_id, &attr.runtime_id, &attr.worker_id) + .map_err(domain)? + { + // Compatibility for an already issued, history-complete ordinary Purge + // plan. New Subject plans always Archive. Keep the old fingerprint and + // disposition; never turn this into an erasure or allow an active writer. + // A missing Runtime Worker is NOT success: execute_worker_retention still + // has to return the exact persisted receipt before anything can commit. + // A pending receipt may outlive the physical aggregate while Runtime's + // in-memory catalog still contains the Worker. Pin release is best effort + // in this historical branch; exact Runtime retention replay still enforces + // pins if the source exists, and requires a receipt if it is already gone. + if let Err(error) = retain_source(&runtime, &service.workspace_id, &attr, source, true) { + tracing::debug!(%error, "Historical Purge source unavailable; reconcile exact Runtime receipt"); } - if let Err(original) = retain_source(&runtime, &service.workspace_id, &attr, source, false) - { - // Recovery may already have transferred the physical source before its Backend - // catalog commit; authenticate the exact reserved archive, never another Session. - let archive = runtime_api::SessionPublicSource::Archived { - archive_id: prepared - .runtime_request - .archive_id - .clone() - .expect("checked"), - }; - retain_source(&runtime, &service.workspace_id, &attr, archive, false) - .map_err(|_| original)?; - } - } else { - // This stopped/removal-fenced Session has no admitted remainder; purge need not - // force a generation. A failed release conservatively blocks ordinary deletion. - retain_source(&runtime, &service.workspace_id, &attr, source, true)?; + return Ok(()); + } + if prepared.runtime_request.session_disposition + != worker_runtime::retention::SessionDisposition::Archive + || prepared.runtime_request.archive_id.is_none() + { + return Err(Error::RepositoryConflict( + "Subject removal retains its original Session; explicit erasure is required for Purge" + .into(), + )); + } + let release = !retention_required; + if let Err(original) = retain_source(&runtime, &service.workspace_id, &attr, source, release) { + // A lost reply may follow the physical transfer. Reconcile only the exact + // reserved archive and Session, whether or not an admitted remainder exists. + let archive = runtime_api::SessionPublicSource::Archived { + archive_id: prepared + .runtime_request + .archive_id + .clone() + .expect("checked"), + }; + retain_source(&runtime, &service.workspace_id, &attr, archive, release) + .map_err(|_| original)?; } Ok(()) } +/// A successful Runtime receipt, not catalog liveness, authorizes ending the old +/// writer. The ordinary removal commit releases occupancy atomically with catalog +/// deletion. Until then a crash leaves the singleton occupied and the same receipt +/// can safely repeat this step; no Controller restore or Journal generation is needed. +pub(super) fn finish_worker_removal( + service: &WorkerRemovalService, + target: &RuntimeWorkerRef, + prepared: &crate::retention::PreparedWorkerRemoval, + result: &worker_runtime::retention::WorkerRetentionExecutionResult, + commit: impl FnOnce() -> T, +) -> Result { + let Some(attr) = removal_attribution(service, target)? else { + return Ok(commit()); + }; + let request = &prepared.runtime_request; + if prepared.plan.worker != *target + || request.workspace_id != service.workspace_id + || request.source_runtime_id != target.runtime_id + || request.worker_id.to_string() != target.worker_id + || !result.source_removed + || result.worker_id != request.worker_id + || result.operation_id != request.operation_id + || result.input_fingerprint != request.input_fingerprint + || result.session_disposition != request.session_disposition + || result.diagnostics_disposition != request.diagnostics_disposition + || result.diagnostics_retained + != (request.diagnostics_disposition + == worker_runtime::retention::DiagnosticsDisposition::Retain) + { + return Err(Error::RepositoryConflict( + "Subject removal receipt does not match its plan".into(), + )); + } + if request.session_disposition == worker_runtime::retention::SessionDisposition::Archive { + let archive = result.archive.as_ref().ok_or_else(|| { + Error::RepositoryConflict("Subject removal archive receipt is missing".into()) + })?; + if Some(&archive.archive_id) != request.archive_id.as_ref() + || archive.workspace_id != request.workspace_id + || archive.source_runtime_id != request.source_runtime_id + || archive.source_worker_id != request.worker_id + || archive.source_session_id != attr.session_id + || archive.operation_id != request.operation_id + || archive.input_fingerprint != request.input_fingerprint + || archive.policy_id != request.policy_id + { + return Err(Error::RepositoryConflict( + "Subject removal archive identity mismatch".into(), + )); + } + } + let api = service.journal_dispatch_api.upgrade().ok_or_else(|| { + Error::Store("Workspace API unavailable for Subject removal completion".into()) + })?; + let gate = lifecycle_lock(&api, &attr.subject_id)?; + let _guard = gate + .lock() + .map_err(|_| Error::Store("Journal lifecycle lock poisoned".into()))?; + let store = store(&api)?; + let host = JournalHostContext::validated_session(&store, attr.clone()).map_err(domain)?; + if store.journal_session_erased(&host).map_err(domain)? { + if request.session_disposition == worker_runtime::retention::SessionDisposition::Archive { + // Erasure may win after Runtime archive transfer. Do not publish that + // archive or skip writer revocation: the same removal retry must take + // erased_archive_removal_proof and its physical-erasure commit path. + return Err(Error::RepositoryConflict( + "Session erased during removal; reconcile the same removal operation".into(), + )); + } + return Ok(commit()); + } + store.release_removed_journal_body(&host).map_err(domain)?; + for session in store + .subject_session_attributions(&attr.subject_id) + .map_err(|e| Error::Store(e.to_string()))? + .into_iter() + .filter(|a| a.runtime_id == target.runtime_id && a.worker_id == target.worker_id) + { + reconcile_session_jobs(&api, &session, false)?; + } + // Keep erasure excluded through the catalog commit: otherwise a tombstone + // could win between revocation and publication of the retained archive. + Ok(commit()) +} + #[derive(Deserialize)] #[serde(deny_unknown_fields)] pub(super) struct Empty {} @@ -2089,7 +2187,28 @@ pub(super) fn reconcile_unknown( .as_ref() .ok_or_else(|| Error::InvalidInput("Journal recovery requires a grant".into()))?; let store = store(api)?; - let host = host(api, &store, grant)?; + let attr = store + .session_attribution(&grant.session_id) + .map_err(|e| Error::Store(e.to_string()))? + .filter(|a| a.subject_id == grant.subject_id) + .ok_or_else(|| Error::WorkspacePermissionDenied("Journal attribution mismatch".into()))?; + let released = store + .journal_worker_was_released(&attr.subject_id, &attr.runtime_id, &attr.worker_id) + .map_err(domain)?; + let host = if released { + // Recovery reads immutable evidence, never renews a legacy writer grant. + let host = JournalHostContext::validated_session(&store, attr.clone()).map_err(domain)?; + if !grant.binding_id.is_empty() + && host.writer_binding_id() != Some(grant.binding_id.as_str()) + { + return Err(Error::WorkspacePermissionDenied( + "Journal recovery binding mismatch".into(), + )); + } + host + } else { + host(api, &store, grant)? + }; let erased = store.journal_session_erased(&host).map_err(domain)?; if !erased { job.load_executable_request()?; @@ -2097,70 +2216,93 @@ pub(super) fn reconcile_unknown( job_id: job.request.job_id.clone(), attempt_id: attempt.attempt_id.clone(), }; - let g = if legacy_descriptor(&job.request) { - // Only reconcile already-saved legacy receipts; never redispatch descriptors. + let generation = if released { store - .prepare_journal_generation( - &host, - &grant.journal_id, - binding, - grant.selected_ranges.clone(), - ) + .retained_journal_generation(&host, &grant.journal_id, &binding) .map_err(domain)? + } else if legacy_descriptor(&job.request) { + Some( + store + .prepare_journal_generation( + &host, + &grant.journal_id, + binding, + grant.selected_ranges.clone(), + ) + .map_err(domain)?, + ) } else { - let input = saved_input(&job.request, grant)?; - let generation = store - .prepare_journal_input(&host, &input, binding) - .map_err(domain)?; - input.validate_generation(&generation).map_err(domain)?; - generation + // Preserve active-body recovery: its saved intent can still establish + // the original generation fence. A released body must only read it. + Some( + store + .prepare_journal_input(&host, &saved_input(&job.request, grant)?, binding) + .map_err(domain)?, + ) }; - let ctx = JournalJobContext::validated_recovery(&store, &host, &g.id, g.job.clone()) - .map_err(domain)?; - match journal::service::finish(&store, &ctx) { - Ok(completion) => { - let submission = BackendJobResultSubmission { - job_id: job.request.job_id.clone(), - attempt_id: attempt.attempt_id.clone(), - input_digest: attempt.input_digest.clone(), - result: serde_json::to_value(completion) - .map_err(|e| Error::Store(e.to_string()))?, - }; - let current = api.store.reconcile_backend_journal_attempt( - api.workspace_id(), - attempt, - Some(&submission), - &Utc::now().to_rfc3339(), - )?; - let saved = api - .store - .get_backend_job(api.workspace_id(), &job.request.job_id)? - .ok_or_else(|| Error::Store("reconciled Job missing".into()))?; - notification_delivery::deliver_job_result( - api, - &BackendJobResultAcceptance { - job: saved, - attempt: current.clone(), - replayed: false, - }, - ); - if current.worker_cleanup_state == Some(BackendJobWorkerCleanupState::Completed) { - drain_after_completion(api, &job.request); - if job.input_disposition == crate::backend_job::JobInputDisposition::Available { - api.store.reclaim_backend_job_input( - api.workspace_id(), - &job.request.job_id, - &job.input_digest, - crate::backend_job::JobInputRetirementReason::Completed, - )?; + if let Some(g) = generation { + if g.selected_ranges != grant.selected_ranges { + return Err(Error::WorkspacePermissionDenied( + "Journal recovery selection mismatch".into(), + )); + } + if !legacy_descriptor(&job.request) { + saved_input(&job.request, grant)? + .validate_generation(&g) + .map_err(domain)?; + } + let ctx = JournalJobContext::validated_recovery(&store, &host, &g.id, g.job.clone()) + .map_err(domain)?; + match journal::service::finish(&store, &ctx) { + Ok(completion) => { + let submission = BackendJobResultSubmission { + job_id: job.request.job_id.clone(), + attempt_id: attempt.attempt_id.clone(), + input_digest: attempt.input_digest.clone(), + result: serde_json::to_value(completion) + .map_err(|e| Error::Store(e.to_string()))?, + }; + let current = api.store.reconcile_backend_journal_attempt( + api.workspace_id(), + attempt, + Some(&submission), + &Utc::now().to_rfc3339(), + )?; + let saved = api + .store + .get_backend_job(api.workspace_id(), &job.request.job_id)? + .ok_or_else(|| Error::Store("reconciled Job missing".into()))?; + notification_delivery::deliver_job_result( + api, + &BackendJobResultAcceptance { + job: saved, + attempt: current.clone(), + replayed: false, + }, + ); + if current.worker_cleanup_state == Some(BackendJobWorkerCleanupState::Completed) + { + if !released { + drain_after_completion(api, &job.request); + } + if job.input_disposition + == crate::backend_job::JobInputDisposition::Available + { + api.store.reclaim_backend_job_input( + api.workspace_id(), + &job.request.job_id, + &job.input_digest, + crate::backend_job::JobInputRetirementReason::Completed, + )?; + } + } else { + api.schedule_backend_job_worker_cleanup(¤t); } - } else { - api.schedule_backend_job_worker_cleanup(¤t); + return Ok(()); } - return Ok(()); + Err(JournalError::Conflict(_) | JournalError::PermissionDenied(_)) => {} + Err(error) => return Err(domain(error)), } - Err(JournalError::Conflict(_) | JournalError::PermissionDenied(_)) => {} - Err(error) => return Err(domain(error)), } } if attempt.worker_cleanup_state != Some(BackendJobWorkerCleanupState::Completed) { @@ -2179,11 +2321,17 @@ pub(super) fn reconcile_unknown( false }; if !confirmed_purge { + if released { + // Source deletion already fenced this generation. An unbound Unknown + // still lacks execution-cleanup proof: preserve Unknown, but it no + // longer owns the Subject or blocks removal of the original body. + return Ok(()); + } return Err(Error::RepositoryConflict( "Unknown Journal attempt has no committed stop/Purge fence; not retried".into(), )); } - if !erased { + if !erased && !released { close(api, &job.request, attempt)?; } api.store.reconcile_backend_journal_attempt( @@ -2192,7 +2340,7 @@ pub(super) fn reconcile_unknown( None, &Utc::now().to_rfc3339(), )?; - if !erased { + if !erased && !released { // Resume the same saved body input only after canonical execution/domain fences. // dispatch selects its retained intent before it can capture any newer source. let attr = store @@ -2209,6 +2357,10 @@ fn reconcile_session_jobs( attr: &crate::subjektiv::SubjectSessionAttribution, erased: bool, ) -> Result<()> { + let released = !erased + && store(api)? + .journal_worker_was_released(&attr.subject_id, &attr.runtime_id, &attr.worker_id) + .map_err(domain)?; let mut after: Option = None; loop { let jobs = api.store.list_backend_journal_jobs( @@ -2223,6 +2375,29 @@ fn reconcile_session_jobs( .store .get_backend_job_attempt(api.workspace_id(), &job.request.job_id, &id)? .ok_or_else(|| Error::Store("Journal current attempt missing".into()))?; + if released { + // Writer revocation has already fenced all Journal mutations. End + // pending/Unknown attempts without claiming their input was reflected; + // saved input, operations and original source remain retained. + if !attempt.state.terminal() { + attempt = api.store.finish_backend_job_attempt( + api.workspace_id(), + &job.request.job_id, + &id, + BackendJobAttemptState::Failed, + "journal_body_released", + "Source Worker was removed; unfinished Journal history retained", + &Utc::now().to_rfc3339(), + )?; + } + api.schedule_backend_job_worker_cleanup(&attempt); + if attempt.state == BackendJobAttemptState::Unknown { + // A saved checkpoint may settle success; otherwise retain Unknown + // until canonical Job-worker cleanup has supplied its Purge fence. + reconcile_unknown(api, job, &attempt)?; + } + continue; + } if !erased && legacy_descriptor(&job.request) && job.input_disposition == crate::backend_job::JobInputDisposition::Available diff --git a/crates/workspace-server/src/server/tests/subjektiv_journal_tests.rs b/crates/workspace-server/src/server/tests/subjektiv_journal_tests.rs index 364438db2..2d8ef5414 100644 --- a/crates/workspace-server/src/server/tests/subjektiv_journal_tests.rs +++ b/crates/workspace-server/src/server/tests/subjektiv_journal_tests.rs @@ -1134,6 +1134,7 @@ async fn unknown_partial_waits_for_durable_purge_fence_and_retains_sources_when_ "a stale cleanup snapshot must not overwrite newer evidence" ); subjektiv_journal::reconcile_unknown(&f.api, &dispatched.job, &unknown).unwrap(); + f.execution.take_inputs(); f.api .store .update_worker_retention( @@ -1151,8 +1152,9 @@ async fn unknown_partial_waits_for_durable_purge_fence_and_retains_sources_when_ BackendJobWorkerCleanupState::Completed, ) .await; - // Cleanup commits before domain reconciliation; wait for its authoritative outcome. - tokio::time::timeout(std::time::Duration::from_secs(10), async { + // Cleanup automatically retries the same immutable input after reconciliation. + // Failed is transient: a second manual retry races the production continuation. + let retry = tokio::time::timeout(std::time::Duration::from_secs(10), async { loop { let job = f .api @@ -1160,8 +1162,22 @@ async fn unknown_partial_waits_for_durable_purge_fence_and_retains_sources_when_ .get_backend_job(TEST_WORKSPACE_ID, &f.request.job_id) .unwrap() .unwrap(); - if job.state == BackendJobState::Failed { - break; + if job.current_attempt == 2 { + let id = crate::backend_job::attempt_id(&f.request.job_id, 2); + let attempt = f + .api + .store + .get_backend_job_attempt(TEST_WORKSPACE_ID, &f.request.job_id, &id) + .unwrap() + .unwrap(); + if attempt.state == BackendJobAttemptState::Dispatched { + break BackendJobReservation { + job, + attempt, + replayed: true, + resource_reused: false, + }; + } } tokio::task::yield_now().await; } @@ -1187,19 +1203,8 @@ async fn unknown_partial_waits_for_durable_purge_fence_and_retains_sources_when_ 1, "source pin survives failed/partial Job cleanup" ); - f.execution.take_inputs(); - let retry = f - .api - .store - .reserve_backend_job_retry( - TEST_WORKSPACE_ID, - &f.request.job_id, - &Utc::now().to_rfc3339(), - ) - .unwrap(); assert_eq!(retry.job.request.input, f.request.input); assert_eq!(retry.job.input_digest, f.request.input_digest().unwrap()); - let retry = f.api.dispatch_backend_job_reservation(retry).unwrap(); let resumed = retry.attempt.worker.as_ref().unwrap(); let delivered = f.execution.take_inputs(); assert_eq!(delivered.len(), 1); @@ -2018,6 +2023,114 @@ impl SubjectRemovalFixture { .release_subject_singleton(TEST_WORKSPACE_ID, &lease, &binding.binding_id, operation) .unwrap(); } + fn pending_journal(&self) -> BackendJobReservation { + self.pending_journal_input(false) + } + + fn pending_journal_input(&self, legacy: bool) -> BackendJobReservation { + let capture = + subjektiv_journal::capture_input(&self.api, &self.attr, 0, 0, 48 * 1024).unwrap(); + let ranges = subjektiv::journal::recording::capture_ranges(&capture).unwrap(); + let id = self + .store + .accept_journal_sources(&self.host, "unfinished history", &ranges) + .unwrap(); + let input = subjektiv::journal::recording::JournalInput::build( + &self.store, + &self.host, + &id, + |order, offset, budget| { + subjektiv_journal::capture_input(&self.api, &self.attr, order, offset, budget) + .map_err(|e| JournalError::Conflict(e.to_string())) + }, + ) + .unwrap(); + let mut request = + subjektiv_journal::recording_request(&self.host, self.attr.clone(), &id, input) + .unwrap(); + if legacy { + let grant = request.grants.subjektiv_journal.as_mut().unwrap(); + grant.binding_id.clear(); + request.input = + serde_json::json!({"journal_id":id, "selected_ranges":grant.selected_ranges}); + } + let reserved = self + .api + .store + .reserve_backend_job(TEST_WORKSPACE_ID, &request, &Utc::now().to_rfc3339()) + .unwrap(); + if legacy { + self.store + .prepare_journal_generation( + &self.host, + &id, + subjektiv::api::JobAttemptBinding { + job_id: request.job_id.clone(), + attempt_id: reserved.attempt.attempt_id.clone(), + }, + request + .grants + .subjektiv_journal + .as_ref() + .unwrap() + .selected_ranges + .clone(), + ) + .unwrap(); + } else { + subjektiv_journal::prepare(&self.api, &reserved).unwrap(); + } + reserved + } + + fn assert_removed_with_history(&self) -> String { + assert!( + self.api + .store + .get_worker_registry(TEST_WORKSPACE_ID, &self.target) + .unwrap() + .is_none() + ); + let key = crate::subjektiv::subject_worker_singleton_key(&self.attr.subject_id).unwrap(); + assert!( + self.api + .store + .current_worker_singleton_owner(TEST_WORKSPACE_ID, &key) + .unwrap() + .is_none() + ); + assert!( + self.store + .journal_worker_was_released( + &self.attr.subject_id, + &self.target.runtime_id, + &self.target.worker_id + ) + .unwrap() + ); + assert!(!self.store.journal_session_erased(&self.host).unwrap()); + assert!(!session_root(self.dir.path(), &self.target).exists()); + self.api + .config_store + .worker_session_archive(TEST_WORKSPACE_ID, &self.target, &self.attr.session_id) + .unwrap() + .expect("ordinary deletion retains original history") + .archive_id + } + + fn assert_occupied(&self) { + let key = crate::subjektiv::subject_worker_singleton_key(&self.attr.subject_id).unwrap(); + assert_eq!( + self.api + .store + .current_worker_singleton_owner(TEST_WORKSPACE_ID, &key) + .unwrap() + .unwrap() + .worker, + self.target + ); + } + fn remove(&self) -> worker::WorkspaceResponse { let key = self .api @@ -2067,6 +2180,811 @@ async fn subject_release_never_substitutes_catalog_idle_or_stopped_for_controlle assert!(f.store.journal_for_session(&f.host).unwrap().is_none()); } +#[tokio::test] +async fn stopped_subject_removal_releases_occupancy_and_preserves_unfinished_history() { + let f = SubjectRemovalFixture::new().await; + let reserved = f.pending_journal(); + let grant = reserved + .job + .request + .grants + .subjektiv_journal + .as_ref() + .unwrap(); + let before = f.store.read_journal(&f.host, &grant.journal_id).unwrap(); + // Even a Workspace Purge policy is not implicit authority to erase Subject history. + let policy = f + .api + .config_store + .worker_retention_policy(TEST_WORKSPACE_ID) + .unwrap() + .unwrap(); + let purge = f + .api + .config_store + .update_worker_retention_policy( + TEST_WORKSPACE_ID, + &policy.policy_digest, + &crate::retention::WorkerRetentionPolicyUpdate { + policy_id: "test-purge".into(), + session_disposition: worker_runtime::retention::SessionDisposition::Purge, + metadata_disposition: policy.metadata_disposition, + archive_retention: policy.archive_retention, + diagnostics_disposition: policy.diagnostics_disposition, + diagnostics_retention_seconds: policy.diagnostics_retention_seconds, + }, + ) + .unwrap(); + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + let archive = f.assert_removed_with_history(); + let runtime_api::SessionMaterialCatalogAvailability::Page { page } = f + .api + .runtime + .session_material_catalog( + &f.target.runtime_id, + runtime_api::SessionMaterialCatalogRequest { + workspace_id: TEST_WORKSPACE_ID.into(), + source: runtime_api::SessionPublicSource::Archived { + archive_id: archive, + }, + expected_session_id: f.attr.session_id.clone(), + expected_catalog_sha256: None, + offset: 0, + limit: 100, + }, + ) + .unwrap() + else { + panic!("original history must remain readable") + }; + assert_eq!(page.descriptors.len(), 2); + let after = f.store.read_journal(&f.host, &grant.journal_id).unwrap(); + assert_eq!(before.process_order, after.process_order); + assert_eq!(before.process_byte, after.process_byte); + assert!(f.store.journal_retention_required(&f.host).unwrap()); + assert!( + f.store + .active_journal_generation(&f.host, &grant.journal_id) + .is_err() + ); + assert!(subjektiv_journal::prepare(&f.api, &reserved).is_err()); + let job = f + .api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap(); + assert_eq!(job.state, BackendJobState::Failed); + assert_eq!( + job.load_executable_request().unwrap().input, + reserved.job.request.input + ); + assert_eq!( + f.api + .config_store + .worker_retention_policy(TEST_WORKSPACE_ID) + .unwrap() + .unwrap(), + purge + ); + + let Json(next) = scoped_create_workspace_worker( + State(f.api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: TEST_WORKSPACE_ID.into(), + }), + HeaderMap::new(), + Json(CreateWorkspaceWorkerRequest { + runtime_id: f.target.runtime_id.clone(), + display_name: "replacement".into(), + singleton_key: None, + profile: Some("builtin:companion".into()), + ticket_assignment: None, + initial_submit: vec![], + workdir_attachments: vec![], + feature_connections: server_api::WorkspaceWorkerFeatureConnectionsRequest { + subjektiv: Some(server_api::SubjektivWorkerConnectionRequest { + subject_id: f.attr.subject_id.clone(), + }), + }, + control_operation_id: None, + execution_grant: None, + }), + ) + .await + .unwrap(); + assert_ne!(next.worker_id, f.target.worker_id); + let key = crate::subjektiv::subject_worker_singleton_key(&f.attr.subject_id).unwrap(); + let successor = f + .api + .store + .current_worker_singleton_owner(TEST_WORKSPACE_ID, &key) + .unwrap() + .unwrap(); + assert_eq!(successor.worker.worker_id, next.worker_id); + let replay = WorkerRemovalService::new(&f.api) + .execute_target_removal_on_host( + f.api.runtime.as_ref(), + &f.target, + "repeat old deletion", + None, + ) + .await + .unwrap(); + assert_eq!(replay.status, StatusCode::OK.as_u16(), "{}", replay.body); + assert_eq!( + f.api + .store + .current_worker_singleton_owner(TEST_WORKSPACE_ID, &key) + .unwrap() + .unwrap(), + successor + ); + assert!(subjektiv_journal::bind_host_session(&f.api, f.attr.clone()).is_err()); + subjektiv_journal::drain_after_completion(&f.api, &reserved.job.request); + assert_eq!( + f.api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap() + .current_attempt, + 1 + ); +} + +#[tokio::test] +async fn subject_removal_archive_failure_does_not_release_writer_or_occupancy() { + let f = SubjectRemovalFixture::new().await; + let archive_parent = f + .dir + .path() + .join(".test-embedded-runtime-store/archives/workers"); + std::fs::create_dir_all(archive_parent.parent().unwrap()).unwrap(); + std::fs::write(&archive_parent, "injected archive failure").unwrap(); + assert_ne!(f.remove().status, StatusCode::OK.as_u16()); + f.assert_occupied(); + assert!( + !f.store + .journal_worker_was_released( + &f.attr.subject_id, + &f.target.runtime_id, + &f.target.worker_id + ) + .unwrap() + ); + assert!(session_root(f.dir.path(), &f.target).exists()); + let old = f + .api + .config_store + .recover_worker_removal_execution(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap(); + std::fs::remove_file(archive_parent).unwrap(); + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); + assert_eq!( + f.api + .config_store + .recover_worker_removal_execution(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap() + .plan + .operation_id, + old.plan.operation_id + ); +} + +#[tokio::test] +async fn subject_removal_reopens_after_writer_revocation_before_catalog_commit() { + let f = SubjectRemovalFixture::new().await; + let conn = rusqlite::Connection::open(&f.api.config.database_path).unwrap(); + conn.execute_batch("CREATE TRIGGER interrupt_subject_delete BEFORE DELETE ON worker_registry BEGIN SELECT RAISE(ABORT,'injected metadata failure'); END;").unwrap(); + assert_ne!(f.remove().status, StatusCode::OK.as_u16()); + assert!(!session_root(f.dir.path(), &f.target).exists()); + assert!( + f.store + .journal_worker_was_released( + &f.attr.subject_id, + &f.target.runtime_id, + &f.target.worker_id + ) + .unwrap() + ); + f.assert_occupied(); // Runtime deletion alone is not a successful Backend deletion. + conn.execute_batch("DROP TRIGGER interrupt_subject_delete;") + .unwrap(); + drop(conn); + let f = f.reopen().await; + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); +} + +#[tokio::test] +async fn subject_removal_rejects_mismatched_runtime_receipts_before_revocation() { + let f = SubjectRemovalFixture::new().await; + let inventory = f.api.runtime.worker_retention_inventory(&f.target).unwrap(); + let plan = f + .api + .config_store + .plan_worker_removal( + &crate::retention::WorkerRemovalPlanRequest { + workspace_id: TEST_WORKSPACE_ID.into(), + worker: f.target.clone(), + reason: "receipt recovery".into(), + journal_erasure: None, + }, + &inventory, + ) + .unwrap(); + let prepared = f + .api + .config_store + .prepare_worker_removal_execution(TEST_WORKSPACE_ID, &plan.plan_id, &plan.input_fingerprint) + .unwrap(); + let service = WorkerRemovalService::new(&f.api); + subjektiv_journal::validate_removal_retention(&service, &f.target, &prepared).unwrap(); + let result = f + .api + .runtime + .execute_worker_retention(&f.target, prepared.runtime_request.clone()) + .unwrap(); + for variant in 0..5 { + let mut wrong = result.clone(); + match variant { + 0 => wrong.source_removed = false, + 1 => wrong.operation_id.push_str("-wrong"), + 2 => wrong.input_fingerprint.push_str("-wrong"), + 3 => wrong + .archive + .as_mut() + .unwrap() + .source_session_id + .push_str("-wrong"), + _ => wrong.archive = None, + } + assert!( + subjektiv_journal::finish_worker_removal(&service, &f.target, &prepared, &wrong, || ()) + .is_err() + ); + f.assert_occupied(); + assert!( + !f.store + .journal_worker_was_released( + &f.attr.subject_id, + &f.target.runtime_id, + &f.target.worker_id + ) + .unwrap() + ); + } + // Simulate a lost Runtime reply: ordinary retry must recover its exact receipt. + let f = f.reopen().await; + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); +} + +#[tokio::test] +async fn subject_removal_unknown_job_settles_after_cleanup_without_restarting_old_writer() { + let f = SubjectRemovalFixture::new().await; + let reserved = f.pending_journal(); + let dispatched = f + .api + .dispatch_backend_job_reservation(reserved.clone()) + .unwrap(); + let worker = dispatched.attempt.worker.as_ref().unwrap(); + seed_session( + &session_root(f.dir.path(), worker), + session_store::new_session_id(), + message("delivered immutable Journal input"), + session_store::LoggedSessionHistoryOrigin::LocalHumanInput, + ); + f.api + .store + .update_worker_retention( + TEST_WORKSPACE_ID, + worker, + "pinned", + &Utc::now().to_rfc3339(), + ) + .unwrap(); + f.api + .finish_backend_job_attempt( + &reserved.job.request.job_id, + &dispatched.attempt.attempt_id, + BackendJobAttemptState::Unknown, + "response_lost", + "test lost result", + ) + .unwrap(); + wait_for_backend_job_worker_cleanup( + &f.api, + &reserved.job.request.job_id, + &dispatched.attempt.attempt_id, + BackendJobWorkerCleanupState::Failed, + ) + .await; + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); + assert_eq!( + f.api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap() + .state, + BackendJobState::Unknown + ); + f.api + .store + .update_worker_retention( + TEST_WORKSPACE_ID, + worker, + "normal", + &Utc::now().to_rfc3339(), + ) + .unwrap(); + let attempt = f + .api + .store + .get_backend_job_attempt( + TEST_WORKSPACE_ID, + &reserved.job.request.job_id, + &dispatched.attempt.attempt_id, + ) + .unwrap() + .unwrap(); + f.api.schedule_backend_job_worker_cleanup(&attempt); + wait_for_backend_job_worker_cleanup( + &f.api, + &reserved.job.request.job_id, + &dispatched.attempt.attempt_id, + BackendJobWorkerCleanupState::Completed, + ) + .await; + // Cleanup's own reconciliation owns the CAS; do not race it with a stale snapshot. + tokio::time::timeout(std::time::Duration::from_secs(10), async { + loop { + let job = f + .api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap(); + if job.state == BackendJobState::Failed { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + // Exercise restart recovery against the persisted state while the asynchronous + // cleanup task is still allowed to hold this fixture's Runtime owner lock. + subjektiv_journal::recover(&f.api).unwrap(); + let job = f + .api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap(); + assert_eq!(job.state, BackendJobState::Failed); + assert_eq!(job.current_attempt, 1); + assert_eq!( + job.load_executable_request().unwrap().input, + reserved.job.request.input + ); + assert!(f.store.journal_retention_required(&f.host).unwrap()); +} + +#[tokio::test] +async fn subject_removal_erasure_between_runtime_receipt_and_commit_requires_reconciliation() { + let f = SubjectRemovalFixture::new().await; + let inventory = f.api.runtime.worker_retention_inventory(&f.target).unwrap(); + let plan = f + .api + .config_store + .plan_worker_removal( + &crate::retention::WorkerRemovalPlanRequest { + workspace_id: TEST_WORKSPACE_ID.into(), + worker: f.target.clone(), + reason: "erase race".into(), + journal_erasure: None, + }, + &inventory, + ) + .unwrap(); + let prepared = f + .api + .config_store + .prepare_worker_removal_execution(TEST_WORKSPACE_ID, &plan.plan_id, &plan.input_fingerprint) + .unwrap(); + let service = WorkerRemovalService::new(&f.api); + subjektiv_journal::validate_removal_retention(&service, &f.target, &prepared).unwrap(); + let result = f + .api + .runtime + .execute_worker_retention(&f.target, prepared.runtime_request.clone()) + .unwrap(); + // The domain fence commits, but the original moved to the not-yet-published archive. + assert!( + f.api + .erase_subjektiv_journal_session(&f.attr.session_id, "erase-during-removal") + .is_err() + ); + assert!(f.store.journal_session_erased(&f.host).unwrap()); + let mut committed = false; + assert!( + subjektiv_journal::finish_worker_removal(&service, &f.target, &prepared, &result, || { + committed = true; + }) + .is_err() + ); + assert!(!committed); + f.assert_occupied(); + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_erased_removal(); + assert!( + f.store + .journal_worker_was_released( + &f.attr.subject_id, + &f.target.runtime_id, + &f.target.worker_id + ) + .unwrap() + ); +} + +#[tokio::test] +async fn subject_removal_prior_archive_policy_receipt_remains_recoverable() { + let f = SubjectRemovalFixture::new().await; + f.seed_completed_release_for_archive_recovery(); + let configured = f + .api + .config_store + .worker_retention_policy(TEST_WORKSPACE_ID) + .unwrap() + .unwrap(); + let conn = rusqlite::Connection::open(&f.api.config.database_path).unwrap(); + conn.execute_batch("CREATE TRIGGER interrupt_legacy_removal BEFORE DELETE ON worker_registry BEGIN SELECT RAISE(ABORT,'injected metadata failure'); END;").unwrap(); + assert_ne!(f.remove().status, StatusCode::OK.as_u16()); + let prepared = f + .api + .config_store + .recover_worker_removal_execution(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap(); + assert_eq!(prepared.plan.policy_id, configured.policy_id); + assert_eq!(prepared.plan.policy_digest, configured.policy_digest); + assert_eq!( + prepared.plan.state, + crate::retention::WorkerRemovalPlanState::Failed + ); + assert!(!session_root(f.dir.path(), &f.target).exists()); + conn.execute_batch("DROP TRIGGER interrupt_legacy_removal;") + .unwrap(); + drop(conn); + let f = f.reopen().await; + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); + let recovered = f + .api + .config_store + .recover_worker_removal_execution(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap(); + assert_eq!(prepared.plan.operation_id, recovered.plan.operation_id); + assert_eq!( + prepared.plan.input_fingerprint, + recovered.plan.input_fingerprint + ); + assert_eq!(prepared.plan.policy_digest, recovered.plan.policy_digest); +} + +async fn assert_subject_removal_recovers_unknown_snapshot(legacy: bool, completed: bool) { + let f = SubjectRemovalFixture::new().await; + let reserved = f.pending_journal_input(legacy); + let grant = reserved + .job + .request + .grants + .subjektiv_journal + .as_ref() + .unwrap(); + if completed { + // Fixture a committed domain checkpoint whose final Job response was lost. + // No Controller/model delivery is claimed by these direct domain writes. + let g = f + .store + .active_journal_generation(&f.host, &grant.journal_id) + .unwrap() + .unwrap(); + let ctx = JournalJobContext::validated(&f.store, &f.host, &g.id, g.job).unwrap(); + let record = f + .store + .execute_journal( + &ctx, + JournalOperation { + request_id: "saved-record".into(), + expected_state: g.initial_journal.state_token, + action: JournalAction::ConfirmUnchanged, + covered_ranges: grant.selected_ranges.clone(), + }, + ) + .unwrap(); + let position = f + .store + .execute_journal( + &ctx, + JournalOperation { + request_id: "saved-position".into(), + expected_state: record.state_token, + action: JournalAction::UpdatePosition { + current_position: "Saved before lost response".into(), + }, + covered_ranges: vec![], + }, + ) + .unwrap(); + f.store + .execute_journal( + &ctx, + JournalOperation { + request_id: "saved-checkpoint".into(), + expected_state: position.state_token, + action: JournalAction::Checkpoint { + record_operation_ids: vec!["saved-record".into()], + }, + covered_ranges: grant.selected_ranges.clone(), + }, + ) + .unwrap(); + subjektiv::journal::service::finish(&f.store, &ctx).unwrap(); + } + // With no bound execution Worker, the store records cleanup Completed. The + // unfinished case must remain Unknown (absence is not a Purge receipt). + f.api + .store + .finish_backend_job_attempt( + TEST_WORKSPACE_ID, + &reserved.job.request.job_id, + &reserved.attempt.attempt_id, + BackendJobAttemptState::Unknown, + "lost-response", + "fixture retained outcome", + &Utc::now().to_rfc3339(), + ) + .unwrap(); + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + f.assert_removed_with_history(); + let job = f + .api + .store + .get_backend_job(TEST_WORKSPACE_ID, &reserved.job.request.job_id) + .unwrap() + .unwrap(); + assert_eq!( + job.state, + if completed { + BackendJobState::Completed + } else { + BackendJobState::Unknown + } + ); + assert_eq!(job.current_attempt, 1); +} + +#[tokio::test] +async fn subject_removal_recovers_completed_unknown_without_recursive_subject_lock() { + assert_subject_removal_recovers_unknown_snapshot(false, true).await; +} + +#[tokio::test] +async fn subject_removal_recovers_legacy_completed_unknown_without_writer_authority() { + assert_subject_removal_recovers_unknown_snapshot(true, true).await; +} + +#[tokio::test] +async fn subject_removal_retains_legacy_unbound_unknown_without_blocking_deletion() { + assert_subject_removal_recovers_unknown_snapshot(true, false).await; +} + +#[tokio::test] +async fn subject_removal_prior_purge_receipt_recovers_without_inventing_erasure() { + assert_prior_purge_receipt(false).await; +} + +#[tokio::test] +async fn subject_removal_prior_pending_purge_receipt_recovers_with_stale_runtime_catalog() { + assert_prior_purge_receipt(true).await; +} + +async fn assert_prior_purge_receipt(stale_catalog: bool) { + let f = SubjectRemovalFixture::new().await; + f.seed_completed_release_for_archive_recovery(); + let configured = f + .api + .config_store + .worker_retention_policy(TEST_WORKSPACE_ID) + .unwrap() + .unwrap(); + f.api + .config_store + .update_worker_retention_policy( + TEST_WORKSPACE_ID, + &configured.policy_digest, + &crate::retention::WorkerRetentionPolicyUpdate { + policy_id: "old-ordinary-purge".into(), + session_disposition: worker_runtime::retention::SessionDisposition::Purge, + metadata_disposition: configured.metadata_disposition, + archive_retention: configured.archive_retention, + diagnostics_disposition: worker_runtime::retention::DiagnosticsDisposition::Purge, + diagnostics_retention_seconds: None, + }, + ) + .unwrap(); + let lease = f + .api + .config_store + .retained_worker_singleton_lease(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap(); + let conn = rusqlite::Connection::open(&f.api.config.database_path).unwrap(); + // Model the pre-upgrade planner's policy choice only; restore the exact Subject + // reservation before execution. All subsequent retention/receipt paths are real. + conn.execute("UPDATE worker_create_reservations SET singleton_key=NULL,singleton_generation=NULL WHERE workspace_id=?1 AND worker_id=?2", + rusqlite::params![TEST_WORKSPACE_ID, f.target.worker_id]).unwrap(); + let inventory = f.api.runtime.worker_retention_inventory(&f.target).unwrap(); + let plan = f + .api + .config_store + .plan_worker_removal( + &crate::retention::WorkerRemovalPlanRequest { + workspace_id: TEST_WORKSPACE_ID.into(), + worker: f.target.clone(), + reason: "pre-upgrade ordinary Purge".into(), + journal_erasure: None, + }, + &inventory, + ) + .unwrap(); + conn.execute("UPDATE worker_create_reservations SET singleton_key=?3,singleton_generation=?4 WHERE workspace_id=?1 AND worker_id=?2", + rusqlite::params![TEST_WORKSPACE_ID, f.target.worker_id, lease.key, lease.generation]).unwrap(); + drop(conn); + assert_eq!( + plan.session_disposition, + worker_runtime::retention::SessionDisposition::Purge + ); + let prepared = f + .api + .config_store + .prepare_worker_removal_execution(TEST_WORKSPACE_ID, &plan.plan_id, &plan.input_fingerprint) + .unwrap(); + if stale_catalog { + // Fixture the Runtime delete-to-final-receipt crash window. Its live + // catalog deliberately remains stopped while the aggregate is gone. + let request = &prepared.runtime_request; + let pending = worker_runtime::retention::WorkerRetentionExecutionResult { + operation_id: request.operation_id.clone(), + input_fingerprint: request.input_fingerprint.clone(), + worker_id: request.worker_id, + session_disposition: request.session_disposition, + diagnostics_disposition: request.diagnostics_disposition, + archive: None, + source_removed: false, + diagnostics_retained: false, + }; + let receipt = f + .dir + .path() + .join(".test-embedded-runtime-store/retention/operations") + .join(format!("{}.json", plan.operation_id)); + std::fs::create_dir_all(receipt.parent().unwrap()).unwrap(); + std::fs::write( + receipt, + serde_json::to_vec(&serde_json::json!({ + "schema_version":1, "request":request, "result":pending, + })) + .unwrap(), + ) + .unwrap(); + std::fs::remove_dir_all(session_root(f.dir.path(), &f.target).parent().unwrap()).unwrap(); + assert_eq!(f.api.runtime.worker(&f.target).unwrap().state, "stopped"); + } else { + let result = f + .api + .runtime + .execute_worker_retention(&f.target, prepared.runtime_request.clone()) + .unwrap(); + assert!(result.source_removed); + } + f.api + .config_store + .fail_worker_removal( + TEST_WORKSPACE_ID, + &plan.operation_id, + &plan.input_fingerprint, + "metadata_commit_failed", + ) + .unwrap(); + let f = if stale_catalog { f } else { f.reopen().await }; + let response = f.remove(); + assert_eq!( + response.status, + StatusCode::OK.as_u16(), + "{}", + response.body + ); + assert!(!f.store.journal_session_erased(&f.host).unwrap()); + assert!( + f.api + .store + .get_worker_registry(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .is_none() + ); + let recovered = f + .api + .config_store + .recover_worker_removal_execution(TEST_WORKSPACE_ID, &f.target) + .unwrap() + .unwrap(); + assert_eq!(recovered.plan.operation_id, plan.operation_id); + assert_eq!(recovered.plan.input_fingerprint, plan.input_fingerprint); + assert_eq!( + recovered.plan.state, + crate::retention::WorkerRemovalPlanState::Succeeded + ); +} + #[tokio::test] async fn disabled_subject_removal_does_not_admit_new_public_history_or_reserve_a_job() { let f = SubjectRemovalFixture::new().await; @@ -2074,7 +2992,7 @@ async fn disabled_subject_removal_does_not_admit_new_public_history_or_reserve_a let response = f.remove(); assert_eq!( response.status, - StatusCode::CONFLICT.as_u16(), + StatusCode::OK.as_u16(), "{}", response.body ); @@ -2083,7 +3001,7 @@ async fn disabled_subject_removal_does_not_admit_new_public_history_or_reserve_a .store .get_worker_registry(TEST_WORKSPACE_ID, &f.target) .unwrap() - .is_some() + .is_none() ); assert!(f.store.journal_for_session(&f.host).unwrap().is_none()); assert!( @@ -2093,6 +3011,7 @@ async fn disabled_subject_removal_does_not_admit_new_public_history_or_reserve_a .unwrap() .is_empty() ); + f.assert_removed_with_history(); } #[tokio::test] @@ -2148,7 +3067,7 @@ async fn disabled_subject_removal_retains_only_the_already_admitted_prefix() { let response = f.remove(); assert_eq!( response.status, - StatusCode::CONFLICT.as_u16(), + StatusCode::OK.as_u16(), "{}", response.body ); @@ -2157,7 +3076,7 @@ async fn disabled_subject_removal_retains_only_the_already_admitted_prefix() { .store .get_worker_registry(TEST_WORKSPACE_ID, &f.target) .unwrap() - .is_some() + .is_none() ); assert!( f.api @@ -2171,11 +3090,12 @@ async fn disabled_subject_removal_retains_only_the_already_admitted_prefix() { Some(id.as_str()) ); assert!(f.store.journal_retention_required(&f.host).unwrap()); - assert!(session_root(f.dir.path(), &f.target).exists()); + assert!(!session_root(f.dir.path(), &f.target).exists()); + f.assert_removed_with_history(); } #[tokio::test] -async fn attributed_unmaterialized_subject_still_requires_actual_idle_release() { +async fn attributed_unmaterialized_subject_removal_releases_without_restoring_controller() { let f = SubjectRemovalFixture::with_materialized_source(false).await; let retention = source_retention(f.dir.path()); let session = f.attr.session_id.parse().unwrap(); @@ -2200,19 +3120,20 @@ async fn attributed_unmaterialized_subject_still_requires_actual_idle_release() let response = f.remove(); assert_eq!( response.status, - StatusCode::CONFLICT.as_u16(), + StatusCode::OK.as_u16(), "{}", response.body ); - assert_eq!(retention.pins(session).unwrap().len(), 1); + assert!(retention.pins(session).unwrap().is_empty()); assert!( f.api .store .get_worker_registry(TEST_WORKSPACE_ID, &f.target) .unwrap() - .is_some() + .is_none() ); assert!(f.store.journal_for_session(&f.host).unwrap().is_none()); + f.assert_removed_with_history(); } #[tokio::test] @@ -2222,11 +3143,11 @@ async fn missing_materialized_subject_source_blocks_removal_without_marking_it_e let response = f.remove(); assert_eq!( response.status, - StatusCode::CONFLICT.as_u16(), + StatusCode::SERVICE_UNAVAILABLE.as_u16(), "{}", response.body ); - assert!(response.body.contains("journal_handoff_incomplete")); + assert!(response.body.contains("runtime_retention_failed")); assert!( f.api .store @@ -2242,6 +3163,7 @@ async fn missing_materialized_subject_source_blocks_removal_without_marking_it_e .unwrap() .is_empty() ); + f.assert_occupied(); } fn source_retention(root: &std::path::Path) -> session_store::retention::SessionRetentionStore { diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 702dfb5ac..85e1f24cd 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -3112,8 +3112,8 @@ impl SqliteWorkspaceStore { }, ) .optional()?; - // Subject occupancy survives removal until an explicit release - // proof. It must not resurrect the removed creation reservation: + // An unconfirmed legacy removal cannot release Subject occupancy. + // Completed removal or explicit release does; never resurrect a reservation: // Runtime callbacks require a live reservation or catalog member. if stored.as_ref().is_some_and(|row| row.6 == "removed") { return Err(Error::RepositoryConflict(format!( @@ -16223,7 +16223,7 @@ fn current_worker_singleton_owner( CASE WHEN substr(owner.singleton_key,1,10)='subjektiv:' AND reservation.state='removed' \ AND NOT EXISTS(SELECT 1 FROM subject_worker_releases r WHERE r.workspace_id=owner.workspace_id AND r.worker_id=owner.worker_id AND r.runtime_id=owner.runtime_id AND r.singleton_key=owner.singleton_key AND r.singleton_generation=owner.generation AND r.completed=1) \ AND NOT EXISTS(SELECT 1 FROM subject_create_recoveries r WHERE r.workspace_id=owner.workspace_id AND r.worker_id=owner.worker_id AND r.runtime_id=owner.runtime_id AND r.singleton_key=owner.singleton_key AND r.singleton_generation=owner.generation AND r.create_fingerprint=reservation.create_fingerprint AND r.completed=1) \ - THEN 'created' ELSE reservation.state END \ + AND NOT EXISTS(SELECT 1 FROM worker_removal_operations r WHERE r.workspace_id=owner.workspace_id AND r.worker_id=owner.worker_id AND r.runtime_id=owner.runtime_id AND r.state='succeeded') THEN 'created' ELSE reservation.state END \ FROM worker_singleton_owners owner \ LEFT JOIN worker_create_reservations reservation \ ON reservation.workspace_id = owner.workspace_id \ From 181b0d637ec21956dbbc9e28cec7c0540ac7ee7b Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 28 Oct 2026 12:15:35 +0900 Subject: [PATCH 2/6] docs: define Workspace Plugin API contracts --- resources/plugin/API.md | 344 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 344 insertions(+) create mode 100644 resources/plugin/API.md diff --git a/resources/plugin/API.md b/resources/plugin/API.md new file mode 100644 index 000000000..ecd371b36 --- /dev/null +++ b/resources/plugin/API.md @@ -0,0 +1,344 @@ +# Workspace Plugin APIの設計契約 + +本書はWorkspace管理のPluginに採用する設計を定める。実行形式はWebAssembly Component Model、engineはWasmtime、guest/Host境界はWITとする。実装済みAPIの利用ガイドではない。既存の[v1 Tool world](wit/yoi-plugin-tool-v1.wit)、[v1 instance world](wit/yoi-plugin-instance-v1.wit)、[v1 Host imports](wit/deps/yoi-host/yoi-host-v1.wit)、PDK、templateは旧v1のoffline authoring資産であり、本書のv2実行契約をまだ提供していない。 + +管理、guest実行、Server上の拡張データを横断する判断をWIT資産の近くで一か所に保持する。実装時には各wire型と境界testを契約の機械的な根拠にし、ここには所有境界・失敗保証・採用理由を残す。作業順と進捗の正本はO-4、T-469、T-465、T-533とする。 + +## 1. 三つのAPIと所有境界 + +Pluginを管理する権限とPluginコードに渡す権限を分離する。Serverに保存するからServer operatorしか導入できない、とはしない。 + +| 境界 | 呼び出し方向 | 責任 | +| --- | --- | --- | +| Workspace管理API | 認可された管理client → Server | 導入・更新・失効・削除、設定、検証結果、診断 | +| Component exports | Runtime Host → guest | Tool呼び出し、Service/event処理 | +| Host imports | guest → Runtime Host → 必要なdomain owner | 外部通信、拡張データ、診断等の限定された作用 | + +ServerはWorkspace所属、設定、installation、artifact、永続操作の正本を持つ。Runtimeはplanを適用し、guestと接続・一時handleを所有する。公開HTTP DTOは`server-api`、domain認可と永続化は`workspace-server`、guest ABIは`resources/plugin/wit`とPDKの責任とする。Rust traitやServer RESTを汎用proxyとしてguestへ渡さない。 + +Workspace管理者は自Workspaceに導入・更新できる。読取りcallerにはWorkspace読取り権限、mutationにはWorkspace管理権限を要求し、既存の認証済みactorから判断する。operator権限は通常導入の前提にしない。Worker/model/repository/packageの自己申告は管理権限にならない。 + +Hostが実行する全操作の上限は、packageの要求、Workspaceの付与、Profileの縮小、呼出元から委譲可能な範囲、現在の対象資源・運用制約の積集合とする。Serviceはlaunch時に明示されたService用grantを使い、管理者のsession credentialを借用しない。SubWorkerへのPlugin継承も再解決された委譲範囲内のみとし、親のhandleを移送しない。 + +## 2. identityと保存するもの + +installationはWorkspaceに属し、artifactはimmutable bytesで識別する。同じpackage名やversionは権限でも実体の一意性の証拠でもない。 + +- `installation_id`はServerが発行するWorkspace内の導入identity。package作者の`id`は名前であり、このidentityを置換しない。 +- 一つのinstallationに複数のimmutable artifactを登録できる。updateは新しいdigestの登録であり既存bytesを置換しない。同じinstallation内のpackage id/versionに異なる内容を登録する場合はversion衝突として拒否する。他Workspaceの同名versionは検査・存在開示の対象にしない。 +- 同一package idの別installationは許すが、設定aliasを一意にする。同じpackageを別credentialで使う用途をversion名の改変で代替しない。 +- Tool公開名はinstallationの不変ID由来namespaceと宣言Tool名からHostが構成する。表示aliasの変更で既存planのTool名を変えない。衝突・長さ超過はplan検証時に拒否し、登録時に黙ってrenameしない。 +- artifact blobの共有は物理dedupeに限定する。他Workspaceのcatalog、設定、導入履歴、data、grantは共有されない。hashを知っているだけでは取得できない。 + +### package digestと配送digest + +既存`manifest::plugin`のdigestは、path順のfile集合をhashするpackage identityであり、ZIP全体のSHA-256ではない。両者を同じfieldとして検証しない。 + +- `package_digest`は既存のfile集合digestを維持する。schema/manifest/componentを含む論理内容の同一性を検証する。 +- `artifact_digest`はServerが保存・配送するcanonical archive bytesのSHA-256とする。`artifact_size`と共に配送・cacheを検証する。 +- Serverは検証済みfile集合から既存canonical pack規則でarchiveを生成し、そのbytesを一度保存する。取得時に再packしない。upload元ZIPのbytesと保存bytesが異なり得るため、両digestと保存sizeを応答する。 +- planは両digest、package schema、exact Component world/API versionを固定する。Runtimeは受信bytesのhashに加え、package validatorで論理digestを検証する。 + +## 3. Workspace管理API + +管理操作は既存のtyped `ServerApi`へ追加する。以下は採用するoperationの意味であり、別のPlugin専用HTTP frameworkは作らない。HTTP prefixは既存の`/api/w/{workspace}`とし、route/DTO/OpenAPI/clientは同じ宣言から生成する。 + +### 操作と競合 + +| 操作 | 契約 | +| --- | --- | +| `POST /plugin-uploads` | bounded binary `.yoi-plugin`を受け、caller/Workspaceに束縛した一時upload identityと検証結果を返す。install・enableはしない | +| `POST /plugins` | upload identity、Workspace内alias、operation keyでinstallationを作成。結果はinstallation/artifact identityと確定operationの識別子 | +| `GET /plugins` / `GET /plugins/{installation}` | Workspace-scoped bounded一覧・詳細。opaque cursor、page sizeの上限あり。guest codeは実行しない | +| `POST /plugins/{installation}/artifacts` | 新しい検証済みuploadからartifact追加。観測した最終catalog変更operationと新しいoperation keyが必要。既存plan/configは変更しない | +| `POST /plugins/{installation}/revoke` | 指定artifact digestの導入許可を失効。観測した最終catalog変更operation、理由、新しいoperation key。新規実行と既存実行の操作権限を撤回 | +| `POST /plugins/{installation}/uninstall` | 観測した最終catalog変更operationと新しいoperation keyで除去を要求。参照があれば拒否し、安全な阻害情報を返す | +| `GET /plugin-operations/{operation}` | 認可された同Workspace callerが保存結果を照合。結果不明のmutationを新しいkeyでやり直さない | + +uploadはmanagement operationと分ける。巨大bytesをJSON/idempotency記録に埋めず、検証に失敗したuploadからcatalogが生じないようにするためである。upload identityは24時間で失効する一時資源で、同じcontentの再uploadは可能。失効後も既にcommitしたinstallationを消さない。usage/quotaは未確定uploadも含む。 + +catalog mutationのoperation keyはWorkspace・認証主体・操作kindに束縛し、canonical request fingerprintと保存結果をdomain更新と同じtransactionで確定する。fingerprintは対象、観測した最終catalog変更operation、内容digest、alias、理由など意味を変える入力を含み、生成された結果IDや秘密値を含まない。同じkey/入力には元の結果、異なる入力にはconflictを返す。replay時も現在の読取り権限を確認し、権限失効後に以前の結果を漏らさない。保存済みreplayは最新stateの観測ではない。 + +catalogの追加・失効・除去では、そのinstallationの最終変更を確定したoperation identityをtransaction内で比較してから更新する。時刻やstateだけでは、同じstateのままartifactが追加された後の古い削除判断を拒否できないためである。設定内容はconfig treeの実内容digestで比較し、別の汎用更新カウンタは導入しない。 + +configのenable/disable、artifact選択、接続binding、grant変更は既存Workspace config Preview/Commit/CASへ統合する。別のenable REST endpointを作って設定正本を二重化しない。Preview後のinstallation/schema変更はcommit時にも照合する。 + +### install・disable・revoke・uninstall + +- installは導入可能なartifactを登録するだけで、Workerの開始やTool登録を行わない。 +- updateとconfig disableは新規/replacementのplanに反映する。既存Workerのpinをhot reloadしない。即時停止が必要なら明示revokeを使う。 +- revokeはinstallationとartifact digestの組に対して不可逆とする。同じ組を再許可して古い実行を復活させない。管理者が同じbytesを再利用する場合は新しいinstallationを明示作成し、新しいconfig/planを生成する。古いinstallationの失効tombstoneとpinは保持する。 +- revokeは新規Host操作を拒否し、稼働中guestの取消・接続closeを要求する。既に外部へ送った効果の取り消しは保証しない。Remote Runtimeへ失効通知を送るだけでは認可が撤回された証拠にならない。保護操作のadmissionはownerでlive確認し、確認不能なら拒否する。stream接続はHostの失効監視で閉じ、監視を維持できない場合も継続しない。 +- uninstallはconfig、active/pending/restorable Worker pinが参照する間は拒否する。revokeしてもpinを勝手に消さない。Worker retention解除・config変更・uninstallを別操作として扱う。 +- installationの拡張データが残る場合もuninstallは拒否する。後述の明示purgeで消すか、データを保持したdisabled installationとして残す。 +- artifact GCは全Workspaceの参照を確認する。DB参照より先に永続blobをpublishし、crash時の未参照blobは後で回収する。未publish blobをcatalogから参照させない。 + +参照検査・新規pin/config参照の作成・削除fenceは同じdomain transaction境界で競合を解決する。削除直前の単なるlist結果では判断しない。 + +HTTP errorは既存共通error envelopeへ合わせる。invalid=400、未認証=401、権限不足=403、不可視対象=404、最終catalog変更operation不一致/operation-key/参照競合=409、size=413、quota=429、owner unavailable=503を使う。他Workspace対象の存在をerror詳細から漏らさない。errorの`retryable`だけでmutationの新規再送を許可しない。 + +## 4. package宣言とABI version + +新しい実行契約は`yoi:plugin@2.0.0`と`yoi:host@2.0.0`にする。JSON文字列だけのHost呼出し、数値handle、guest側WebSocket recvを変えるため、v1の意味を黙って差し替えない。 + +- 新しいmanifest schemaはv2とし、required worlds/interfaces、Tool schema、接続slot、拡張data schema、event subscriptionを静的に宣言する。schema contributionを得るためにguestを実行しない。 +- Workspace設定はDCDLのclosed typed schemaで検証する。Toolのmodel-facing入力はJSON Schemaで定義する。JSON SchemaとDCDL schemaを同じものと仮定しない。 +- Componentの実imports/exportsを静的に検査してmanifestと照合する。manifestの申告だけで未公開importをlinkしない。 +- 初期実装はexact world/interface versionだけを受け入れる。semver rangeでHost APIを黙って置換しない。互換な追加は別versionとして明示サポートし、既存pinには元の契約を提供するかunsupportedで止める。 +- 旧v1 packageはofflineで読めても新Runtimeでは実行対象にしない。v2向けrebuild/explicit installを要求し、汎用JSON adapterで旧権限経路を復活させない。 +- PDK/template/validator/WITを一緒にv2へ更新する。今回の設計文書だけを根拠に既存v1 templateが新Runtimeで動くとは扱わない。 + +## 5. Component exports + +Tool payloadだけはPluginごとにschemaが異なるため、schema検証済みUTF-8 JSONを運ぶ。それ以外の共通context、error、event envelope、Host request/resultはWIT record/variant/resourceで定義する。 + +以下は採用するinterfaceの形を説明する最小例で、現行WITへ追加済みという意味ではない。 + +```wit +package yoi:plugin@2.0.0; + +interface tool { + record invocation { + invocation-id: string, + tool-name: string, + input-json: string, + config-json: string, + remaining-ms: u32, + } + record output { + summary: string, + content: option, + structured-json: option, + } + record tool-error { + code: string, + message: string, + } + invoke: func(input: invocation) -> result; +} + +world tool-plugin { + export tool; +} +``` + +`invocation-id`はHost生成の追跡identityであり権限ではない。contextにServer token、host path、任意Workspace selectorを含めない。configはplanのnon-secret snapshotで、secret値は含めない。structured outputには宣言されたoutput schemaを要求し、Hostが検証する。 + +Tool worldは呼出しごとにfresh Store/instanceとし、compiled Componentは共有できる。Toolのglobal/static memoryを永続化や他呼出しとの通信に使えない。v2初期Toolは応答一回のbounded callとし、guestがmodelへ直接streamを注入するAPIは設けない。 + +Service/event worldには`on-start(config) -> result`、`on-event(envelope) -> event-result`、`on-stop(reason) -> result`を設ける。いずれもfresh Storeで呼び、HostがWorker/installation/planに束縛したService resourceを保持する。ToolとServiceで共有する状態はHost経由の明示dataとし、guest memoryの残存を契約にしない。`status()`を呼んで診断のためにguestを実行せず、Hostが実行・接続・失敗の観測を返す。 + +`on-start`は設定検証とローカル初期化だけを行い、logging以外のHost作用を認めない。GETも副作用を持ち得るため、汎用HTTPや付随data書込みを初期化中に許可しない。外部event sourceの接続は全required Pluginの検証・導入成功後にHostが開始する。initialization失敗時に外部効果のrollbackを装わない。`on-stop`もloggingだけのbest effort通知で、resource解放の前提にしない。 + +## 6. Host importsとcapability lifetime + +公開するinterfaceは機能ごとに分ける。汎用`invoke(method, json)`、汎用Server HTTP proxy、DB connection、host path、任意process実行APIは提供しない。 + +| interface | 操作の意味 | +| --- | --- | +| `connections` | 宣言slot名から、その呼出しに許可されたconnection resourceを取得 | +| `http` | connectionに束縛されたbounded request/response | +| `extension-data` | Plugin所有schemaのdataを対象object capability経由でread/CAS update | +| `logging` | bounded structured診断。model contextへの注入ではない | +| `workdir` | 後続で既存attachment-scoped operationを型付きにadapt。ambient filesystemは不可 | +| `websocket` | 後続のHost-owned driverへのsend/close。recvはevent配送で行う | + +`connections.open(slot)`はWIT resourceを返す。slotはmanifestの宣言名で、Workspace設定が具体的な接続先とsecret参照にbindする。guestは任意credential IDや別Workspace IDを指定しない。resourceはStore/呼出し内だけで有効で、serialize・persist・別Workerへtransferできない。Serviceの長寿命接続自体はHost所有とし、各event処理時にfresh handleを発行する。 + +Hostは各importでcurrent invocation、installation、plan、付与範囲、対象資源、deadline/cancellationを照合する。lookup済みhandleを後続操作の無条件許可にしない。guestのComponent start sectionからの外部I/Oも禁止し、export呼出し用contextのないimportを拒否する。 + +planはconfig treeの内容digest、Profile projection digest、installation/artifact identities、接続binding identityとpolicyの内容digest、宣言schemaと実効grant、実行主体と委譲identityを固定する。管理操作を行った人の資格情報はguestの実行主体ではない。 + +| 変更・状態 | 既存plan/new export/restoreへの作用 | +| --- | --- | +| artifact追加、config enable/disable、Profileの選択・grant設定変更 | 新規/replacement planだけに適用。既存pinの集合や付与snapshotは変更しない | +| Workspace/対象資源のlive grant上限の縮小、明示的grant revoke、実行主体の委譲失効 | 既存snapshotとの積集合を直ちに縮小。次のadmission/Host操作を拒否し、進行中処理へ取消要求 | +| live grant拡大 | 古いsnapshotを広げない。追加利用には新規planが必要 | +| 接続binding失効、installation+artifactのrevoke | 新しいexport・接続・restoreを拒否。既存guest/driverを取消対象にする | +| 管理者の管理権限喪失 | 以後の管理操作を拒否。独立したWorker実行権限まで導入者の権限と同一視しない | +| ownerへ確認できない | fresh export、restore、保護操作・保存結果読取りを拒否。cached planを認可にしない | + +各fresh exportの開始とrestoreにはServer ownerでinstallation/plan/実行主体のlive admissionを行う。pure Toolも例外にしない。既にadmit済みのローカル計算は取消・deadlineまで進み得るが、失効後の結果公開と追加Host作用は拒否する。初期Toolにoffline実行leaseは設けない。設定でのgrant変更とlive security grantの失効を混同しないよう、管理画面/APIの結果も適用対象を明示する。 + +WASIを一括linkしない。v2 worldに宣言したimportだけをallowlistし、ambient filesystem/network/environment/stdin/stdoutを与えない。clock/random等を追加する場合も個別のversioned interfaceとして明示する。PDKはこのimport集合だけでcompileできるようにする。 + +Wasmtimeのasync Host実装を使って待機中のexecutor threadを占有しない。ただしguestにexperimental async Component ABIを要求せず、初期WITは通常のcall/resultとする。Storeを並行利用せず、同一Service consumerへのevent処理を直列化する。 + +## 7. 外部HTTP接続 + +HTTPはURLを自由に受けるAPIではなく、connection resourceに対する相対pathのrequestとする。Server/Runtimeの資格情報を外部requestへ転用しない。 + +requestはmethod enum、absolute-path形式の相対path、query pair、許可されたheader pair、body bytes、要求timeoutを持つ。responseはstatus、許可されたheader、body bytesを持つ。4xx/5xxもHTTP応答として返し、transport errorと区別する。binaryはbyte listで運び、base64やJSON stringで二重に包まない。 + +初期Host interfaceの型は次の形にする。Tool worldにはこれらの必要なinterfaceを明示importし、未使用の機能を一括importするworldにはしない。 + +```wit +package yoi:host@2.0.0; + +interface errors { + enum code { + invalid-input, denied, not-found, conflict, unsupported, + quota-exceeded, deadline-exceeded, cancelled, unavailable, + indeterminate, receipt-expired, + } + enum effect-observation { not-dispatched, response-observed, indeterminate } + record host-error { + code: code, + message: string, + diagnostic-id: string, + effect: effect-observation, + } +} + +interface connections { + use errors.{host-error}; + resource connection; + open: func(slot: string) -> result; +} + +interface http { + use connections.{connection}; + use errors.{host-error}; + enum method { get, head, post, put, patch, delete, options } + record pair { name: string, value: string } + record request { + method: method, + path: string, + query: list, + headers: list, + body: list, + timeout-ms: u32, + effect-key: string, + } + record response { + status: u16, + headers: list, + body: list, + } + send: func(target: borrow, input: request) -> result; +} +``` + +queryはHostがpercent-encodeして送る。path中への`?`埋込みを拒否し、query/headerの重複・順序は許可されたprotocol semanticsを保持する。guest指定の接続/転送制御headerは拒否し、Hostが必要なものを生成する。effect-keyは最大128 UTF-8 bytesの非秘密のopaque文字列とし、Hostのinvocationに束縛する。 + +- Workspace bindingはexact origin（scheme/host/port）、許可method、segment境界のpath範囲、secret参照、資源上限を持つ。packageはslotと要求範囲を宣言できるが、bindingを自己発行できない。 +- 初期APIはHTTPSのみ。TLS検証を無効にできない。raw socket、任意proxy、Unix socket、CONNECT、upgrade、redirect自動追従は提供しない。 +- pathはHostが一度canonicalizeし検査・送信に同じ値を使う。scheme/authority変更、userinfo、fragment、backslash、encoded separator/dot traversalを拒否する。prefixの文字列一致で`/api`に`/apix`を含めない。 +- HostがDNS解決した全候補を検査し、許可したaddressへ接続を固定する。接続時の再解決で検査を迂回しない。TLSの名前検証とSNIは許可hostnameを使う。 +- loopback、link-local、metadata endpoint、Server/Runtime管理endpointを拒否する。private LANはServer運用policyが許す範囲内でWorkspaceが明示bindingできる。Workspace管理権限だけで共有Hostの内部network全体を開かない。 +- `Authorization`、Cookie、Host、proxy/hop-by-hop header等はguestが指定できない。接続policyで許可したnon-secret headerだけを受け付ける。credentialはHostが指定origin/pathへだけ注入し、responseのSet-Cookie等を返さない。 +- credentialの値をguestへ返すAPIはない。ただし相手serverがsecretをechoする可能性まで完全に除去できるとは主張しない。credentialの配送先はWorkspaceが管理する信頼境界として扱う。 +- planはsecret参照と接続policyの内容digestを固定し、secret値は保存しない。通常のsecret rotationは同じbindingのHost解決で反映できるが、宛先/権限変更は新規planにする。live失効・policy縮小は古いplanにも効く。 +- request/responseはbuffered boundedとし、上限到達時は切断してlimit errorを返す。切断されたresponseを成功に見せない。圧縮展開後の上限も検査する。 + +### 外部副作用と再試行 + +HostはHTTP requestを自動再送しない。GETであってもprovider固有semanticsがあるため、transport層が勝手に再試行しない。redirectもguestにresponseとして返すだけで、別originへの追跡は新しい認可済みbindingが必要になる。 + +全HTTP requestに`(Workspace, installation, Worker, consumer-or-tool-call, logical-invocation, effect-key)`のscopeを使い、request fingerprint、送信admission、保存結果を記録する。consumerはServiceの場合の固定identity、Toolの場合は永続Tool-call identityであり、別consumerで同じevent-idを処理してもreceiptを共有しない。GETも例外にせず、methodだけから副作用なしを推定しない。同じkey/内容は保存結果を返し、異なる内容はconflict。guestがkeyを変えれば新しい作用なので、package内の再試行も自動的に安全になるわけではない。 + +fingerprintはinstallation/plan、接続binding identityとslotとpolicyの内容digest、method、正規化path、順序を保つquery/header、body hashを含む。secretの値、残りdeadline、attemptは含めない。saved responseはboundedでWorkspace保護データとして保持し、diagnostic logとは分離する。request body/secret headerの生値をreceiptへ保存しない。同一requestの保存結果を返す際にも現在のinstallation/接続/読取り権限を検証する。 + +invocation identityはHostの永続Tool-call/event記録に結び付け、同じTool-callの回復や同じconsumer/eventの再配送では固定する。guestがidentityを指定・変更できない。新しいユーザーの呼出しは新identityとする。返却済みTool結果は再実行せず既存結果を使い、未完了の回復を行う場合だけ同じeffect-key領域を使う。 + +送信前にdurable admissionを記録し、記録不能なら送らない。送信した可能性があるままtimeout/切断/crashした場合は`indeterminate`とし、同じkeyでも汎用Hostは再送しない。provider固有の照合・idempotency APIがある場合だけ、そのadapterの契約で回復できる。Tool結果が失敗でも外部効果が取り消されたとは表示しない。 + +保持期限を過ぎたeffect keyは未使用keyとして扱わず`receipt-expired`で拒否する。古いinvocation/eventの再開も同様で、再実行は明示的な新しい依頼にする。invocationの受理時にHostがreceipt照合期限を固定し、期限の過ぎたidentityには新しいeffect-keyを追加できない。期限情報/tombstoneは関連Worker/eventの保持期間中残し、bodyを削除しても未実行扱いに戻さない。receipt保持とevent再配送可能期間をHostで連動させ、exactly-onceを約束しない。 + +## 8. 共通error、出力、診断 + +Host errorは閉じたWIT variantで返し、少なくともinvalid-input、denied、not-found、conflict、unsupported、quota-exceeded、deadline-exceeded、cancelled、unavailable、indeterminate、receipt-expiredを区別する。guest business errorは宣言されたcodeとbounded messageで返す。未知guest codeはplugin-failedへ正規化し、Host errorの権限をguestが偽装できないようにする。 + +Host errorには安全なmessage、診断ID、外部効果の観測（not-dispatched / response-observed / indeterminate）を付ける。`response-observed`はbusiness成功や外部transaction commitの証明ではない。再試行の可否はerror文字列でなくoperation契約で判断する。trap、不正output、resource超過はHostが実行失敗として記録する。 + +Tool outputは通常のWorker Tool結果・履歴経路へ流す。Pluginがhidden prompt、System instruction、任意Session entryを直接差し込むAPIはない。structured JSONはschema検証し、不正・超過時に切り詰めたJSONを返さない。textの表示短縮はHost projectionで明示する。 + +loggingはlevel、event code、安全なmessageとscalar fieldsに限定する。raw request/response、secret、prompt、host pathを標準診断に保存しない。Pluginが受け取った外部データ自体はuntrusted contentであり、命令や権限として扱わない。 + +## 9. 付随データの契約 + +Server拡張はdomain objectに関連付けるPlugin所有のtyped dataとし、既存tableへの任意column追加では実現しない。物理保存schemaはHostが管理する。`feature-storage`のnative migration/SQLite handleは信頼済みHost用でありguestへ公開しない。 + +addressはHost contextのWorkspace/installationと、認可済みobject resource、宣言collection、schema id/version、keyから決まる。schema id/versionも物理・論理namespaceに含め、新旧artifactが同じcellを別schemaで読み書きしない。guestにWorkspace selectorやraw table名を与えない。最初のobject kindはTicketとし、Hostが存在・Workspace・callerのread/write権限を検証したTicket resourceだけを使う。一般objectへの拡張はadapterと権限を追加して行う。 + +後続の`objects.resolve-ticket(ticket-ref, access)`はreadまたはextension-writeを要求し、現在のWorker主体・呼出元からの委譲とPlugin grantを検証して呼出し内だけ有効なWIT `ticket` resourceを返す。Tool入力のTicket refもevent envelopeのrefも単なる選択子であり、どちらも同じ解決を通す。ServiceにはService用grantを適用し、eventを受けたことだけでwriteを許可しない。削除済み・不可視・別Workspaceの対象はnot-foundとする。 + +`ticket-read.read(borrow)`はread権限を再検査し、title、case state、content digest、bounded bodyとtruncated flagだけを返す専用read APIとする。thread、添付、認証・管理情報、将来増えるfieldを汎用serializeで公開しない。上限は64 KiB、完全本文が必要なら本APIではunsupportedとする。eventの削除通知自体は安全なmetadataとして配送できるが、削除済みTicket resourceをresolve/readできる意味ではない。 + +- packageはcollectionごとにschema id/versionとclosed typed schemaを宣言する。data valueのJSONはそのschemaで検証する。 +- APIは`get`、`put(expected_last_write | never_written, operation_key, value)`、`delete(expected_last_write, operation_key)`、bounded `list(cursor, limit)`。任意query、SQL、cross-object transactionは公開しない。 +- 同じcellへのput/deleteを確定した最終operation identityを、書込みtransaction内で観測値と比較する。CAS成功とoperation receiptを同じtransactionで保存し、自動読直しによるlast-write-winsへ変えない。同じ内容の再書込みや削除・再作成を時刻/内容hashだけで区別できないため、このoperationを比較する。 +- deleteは最終delete operationのtombstoneを保持する。再作成はそのoperationを期待値にし、`never_written`を通さない。これによりABAを防ぐ。`get`は対象objectの認可成功後に限り、present(value, last-write)、never-written、deleted(last-delete)を区別する。不可視objectと不存在は同じnot-foundにする。 +- data mutationのreceiptはHTTPと同じ実行scope（Workspace/installation/Worker/consumer-or-tool-call/logical-invocation）とguest operation-keyに束縛する。fingerprintに完全なcell address、put/delete種別、期待last-write、schemaとvalueのdigestを含める。異入力でkeyを再利用すればconflictとする。現在の対象認可確認後にreceipt照合をCAS検査より先に行い、成功済み同一要求を最新cellとの差で失敗にしない。照合期限・expired・tombstone保持はHTTPと同じinvocation規則を使う。 +- `lookup-mutation(operation-key)`で同じinvocationの保存結果（committed: mutation identityと結果digest / not-recorded / expired）を現在の対象認可の下で照合できる。event再配送では先にreceiptを照合し、成功済みなら新しい値を読み直して同じkeyで別のputを作らない。ownerがnot-recordedを返した場合だけ現在のcellから要求を作れる。照合通信失敗はnot-recordedではない。外部HTTP・data・event ackを跨ぐ原子性は保証せず、handlerは段階ごとの保存結果で回復する。 +- listはstable key順のbounded探索でありsnapshot全体の一貫性は保証しない。cursorはWorkspace/installation/object/queryに束縛し、不正・失効cursorを最初のpageに黙って戻さない。 +- Ticketの通常fieldやworkflow判断をこのdata APIで変更できない。必要ならそのdomainの専用Host APIを別に設計する。 +- 親object削除とPlugin data書込みはowner側の削除fenceで競合を解決する。削除後に新しい子dataを作らず、retention後にHostが回収する。 +- schemaの破壊的変更は新しいschema version/namespaceとする。既存dataをguest起動時にmigrationしない。移行は新旧schemaを明示した認可付き操作として後続で設計し、初期APIでは自動変換しない。 +- rollbackで旧artifactを使う場合も元schemaだけを読む。最新schemaへ読み替えない。 +- revoke/disableではdataを消さない。Workspace管理者の明示purgeをoperation keyと期待data-generationで行い、先に実行をrevoke/fenceする。purge receiptを残し、uninstallとの一操作への暗黙統合をしない。 + +## 10. commit済みeventとService + +最初のhookはcommit済みeventの観察とする。PluginをServer transaction内で実行せず、元操作の成功をPluginの応答待ちにしない。domain ownerがtransactional outboxへeventを記録し、Hostが認可・投影して配送する。共有の無制限event busは提供しない。 + +最初のeventはTicket変更通知とし、envelopeはevent-id、event-type/version、occurred-at、対象resource reference、変更を確定したdomain eventのidentityと対象内容digest、correlation/causation identity、bounded typed payloadを持つ。全文や秘密fieldは載せず、必要な詳細は認可付きAPIで取得する。削除eventは削除済みreferenceと安全なmetadataのみとし、resource accessを復活させない。 + +subscriptionはmanifest宣言・Workspace grant・Profile選択から確定し、guestが任意event名を動的subscribeしない。consumerはWorker/installation/plan内のservice単位であり、別Workerに同じPluginを入れると別consumerになる。Workspace-wide singletonや分散consumer groupは暗黙に作らない。 + +- event-idは再配送でも固定、attemptはHost生成の観測値。consumer内はHost保存順に一件ずつ処理する。Workspace全体や別consumer間の全順序は約束しない。 +- `event-result`はack、retry-after、rejectのvariant。ackはHostのcursor commitで確定し、guestのreturnだけでは確定しない。 +- deliveryは保持期間内のat-least-once。crash/timeoutで再配送され得るため、外部作用はevent-id由来invocationとstable effect-keyを使う。payload/keyを変えた再送を同一作用として扱わない。 +- retry-afterはeffectが結果不明でない場合のみ再配送を予約できる。indeterminateな外部効果を含むeventは要確認でconsumerを止め、汎用retryで再送しない。 +- reject、繰返し失敗、queue上限、retention gapではconsumerをpausedとして可視化する。silent drop、無限retry、詰まったeventの無断skipはしない。復旧・明示skipはWorkspace管理操作として監査する。 +- cursorが保持範囲から外れたらgapを返し、最新位置へ自動復旧しない。payload取得時にも対象への現在のread権限を再検査する。 +- disableは新規planに作用し、既存consumerの即時停止はrevoke/Worker stopで行う。restoreは同じconsumer identity/pin/cursorを使い、replacementは新しいconsumerとして開始する。開始時点以前のeventを自動backfillしない。 +- 同じconsumerの旧・新Runtimeが同時に処理しないようHost実行leaseでfenceする。receipt/cursor mutationもleaseを検証する。 + +WebSocketは別のHost-owned source driverがeventへ変換する。guestの`recv(timeout)` loopは復活させない。network frame自体にdurable delivery保証はないため、domain outbox eventと同じ保証を表示しない。必要なら外部protocolのsequence/resumeをbridge側で使う。 + +処理前の検証・拒否hookは初期APIに含めない。将来導入時は対象操作ごとに入力/返却値/timeout/failure policy/再入・副作用を契約化し、汎用interceptorとしてServer内部を公開しない。 + +## 11. 資源上限と終了 + +以下は新APIの初期defaultとして採用する。既存実装値の転載ではない。Hostは実効limitを起動前に固定して診断へ公開し、運用上限は縮小できる。guestから無制限や上限引上げを要求できない。 + +| 資源 | 初期default | +| --- | --- | +| Tool input / output、event payload | 各64 KiB（UTF-8/serialized bytes） | +| config、付随dataの1 cell | 各64 KiB | +| guest linear memory | 256 MiB / Store | +| Wasmtime fuel | 10,000,000 / export call | +| Tool/event export wall deadline | 30秒、上位caller deadlineが短ければそちら | +| start / stop hook | 5秒 / 2秒 | +| HTTP request body / response body | 各1 MiB | +| HTTP header総量 | 16 KiB、64 pairまで | +| HTTP request deadline | 15秒、残りcall deadline以下 | +| guest発行Host操作 | 同時4、累計32 / call | +| log総量 | 16 KiB / call、超過はdrop件数のみ記録 | +| Service待機queue | 256 event、満杯時は配送をpause | +| event再試行 | 最大5 attempt、待機上限5分、その後pause | +| event自動再配送・effect receipt照合 | 最低7日、期限後は明示expired/gap | +| 管理list | default 50、上限100件 | + +fuelはCPU時間の精密尺度ではない。wall deadline、memory/table/instance/resource数のWasmtime limiterと併用する。HTTPとguest実行で別々に30秒ずつ延長せず、同じcall deadlineから残り時間を計算する。package最大size/file数は既存validator上限を再利用し、値の第二正本を作らない。Workspace全体の保存/queue/実行同時数はHostの有限quotaでadmissionし、個別上限だけで総量を無制限にしない。 + +終了順は新規admission停止、invocation取消、接続/driver停止、期限付きon-stop、登録lease解除、Store/一時resource解放とする。取消中に届いた結果で終了済みinvocationへ追加作用を発行しない。失効後のon-stopは外部write禁止。guestがstopに応答しなくてもHostはresourceを回収する。既に送信された外部効果はrollbackしない。 + +## 12. 初期範囲と契約検証 + +最初の実行APIはTool、connection-bound HTTPS、loggingに絞る。付随データとcommit済みeventは本書の設計を採用する後続interfaceであり、初期worldへ動かないstubを登録しない。WebSocket、Workdir、MCP transport、判断hookも未対応importとして明示拒否する。MCP専用のprocess/credential APIを先行して作らず、必要なHost capabilityを個別に追加する。 + +最初のvertical contract testは、Server operatorでないWorkspace管理者によるupload/install/config、plan生成、Remote/Embedded双方のWasmtime Tool実行、Host HTTPS、通常Tool履歴への結果、同一pin restoreを通す。mock guestだけでHost境界の証明を代替しない。 + +境界testでは少なくとも次を固定する。 + +- 同じblobでも別Workspaceのcatalog/plan/secret/dataを読めず、digestだけのfetchができない。 +- v1/未対応world、manifestと実importの不一致、未宣言Tool、invalid input/outputを実行前または結果公開前に拒否する。 +- path canonicalization、DNS rebinding、redirect、credential header、private/control-plane宛先を実際のHost adapterで検証する。 +- upload/publish/DB commitの部分失敗、CAS競合、同一key再送、異入力key reuse、uninstallとpin作成の競合を検証する。 +- effectの送信前失敗、送信後応答喪失、receipt期限、cancel/revokeを区別し、indeterminateから自動再送しない。 +- fuel/memory/wall deadline、Host待機中の取消、trap、不応答on-stopでもresourceが回収される。 +- 後続data/eventではCAS/ABA、親削除fence、schema rollback、再配送、retention gap、lease競合、外部効果不明時pauseを検証する。 + +採用する契約と実装手段を区別する。DBのtable名、内部trait名、物理blob配置、adapterのmodule分割は実装裁量だが、Workspace分離、pin、APIの型・失敗保証・再試行・拡張境界はこの契約を満たす必要がある。 From e7ebc92f8376730477c8d0ff6ced7909e1231410 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 30 Oct 2026 20:33:57 +0900 Subject: [PATCH 3/6] fix: preserve complete merge request reviews --- crates/merge-request/src/lib.rs | 11 +------- crates/merge-request/tests/store.rs | 43 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/crates/merge-request/src/lib.rs b/crates/merge-request/src/lib.rs index aa7409d14..c339c3a31 100644 --- a/crates/merge-request/src/lib.rs +++ b/crates/merge-request/src/lib.rs @@ -210,12 +210,7 @@ impl MergeRequestThreadEvent { } fn bound_bodies(&mut self) { match self { - Self::Review(value) => { - truncate_body(&mut value.body); - for finding in &mut value.findings { - truncate_body(&mut finding.body); - } - } + Self::Review(_) => {} Self::ReviewRevoked(value) => truncate_body(&mut value.reason), Self::ReviewCancelled(value) => truncate_body(&mut value.reason), Self::Comment(value) => truncate_body(&mut value.body), @@ -939,10 +934,6 @@ impl MergeRequestStore { &self, i: SubmitMergeRequestReview, ) -> Result { - bounded_body("review body", &i.body)?; - for finding in &i.findings { - bounded_body("review finding", &finding.body)?; - } let mut c = self.lock()?; let t = c.transaction()?; let g: Option<(String, String, String, String, String, String)> = t diff --git a/crates/merge-request/tests/store.rs b/crates/merge-request/tests/store.rs index 5cf2837b2..e9e3e83e5 100644 --- a/crates/merge-request/tests/store.rs +++ b/crates/merge-request/tests/store.rs @@ -376,6 +376,49 @@ fn approve_for( fn approve(s: &MergeRequestStore, subject: &str, token: &str) -> ReviewEvent { approve_for(s, "MR", "R", subject, token) } +#[test] +fn review_submission_preserves_long_body_and_findings_on_reload() { + for (body, finding_body) in [ + ("b".repeat(16 * 1024 + 1), "finding".into()), + ("review".into(), "指摘".repeat(4096)), + ] { + let (_dir, store) = fixture(); + open(&store); + request(&store, "source", "token"); + let findings = vec![ReviewFinding { + severity: FindingSeverity::Major, + code: None, + path: None, + line: None, + body: finding_body, + }]; + let submitted = store + .submit_review(SubmitMergeRequestReview { + merge_request_id: "MR".into(), + ticket_id: "T".into(), + current_subject_ref: "source".into(), + capability_token: "token".into(), + decision: ReviewDecision::RequestChanges, + body: body.clone(), + findings: findings.clone(), + now: at(4), + }) + .unwrap(); + assert_eq!(submitted.body, body); + assert_eq!(submitted.findings, findings); + let reloaded = store.get_by_id("W", "MR").unwrap(); + let saved = reloaded + .thread + .iter() + .find_map(|event| match event { + MergeRequestThreadEvent::Review(review) => Some(review), + _ => None, + }) + .unwrap(); + assert_eq!(saved, &submitted); + } +} + #[test] fn review_submission_authorization_rejects_invalid_grants_before_side_effects() { let (_d, store) = fixture(); From 2b4bf4b7bdfd6bcae9025e070477c3929660b621 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 30 Oct 2026 20:33:57 +0900 Subject: [PATCH 4/6] fix: log Workdir cleanup busy reasons --- crates/worker-runtime/src/runtime.rs | 106 ++++++++++++++++-- .../src/working_directory/occupancy.rs | 91 ++++++++++++++- 2 files changed, 183 insertions(+), 14 deletions(-) diff --git a/crates/worker-runtime/src/runtime.rs b/crates/worker-runtime/src/runtime.rs index 57b70bc8e..03c968e20 100644 --- a/crates/worker-runtime/src/runtime.rs +++ b/crates/worker-runtime/src/runtime.rs @@ -101,7 +101,18 @@ impl Drop for WorkdirOperationGuard { } } -fn workdir_operation_busy() -> RuntimeError { +fn workdir_operation_busy(id: &str, action: &str, blocking: &WorkdirOperation) -> RuntimeError { + let (busy_reason, active_use_count) = match blocking { + WorkdirOperation::Use(count) => ("active_use", *count), + WorkdirOperation::Cleanup => ("cleanup_in_progress", 0), + }; + tracing::warn!(target: "yoi::workdir_cleanup", + working_directory_id = id, + action, + busy_source = "runtime_operation", + busy_reason, + active_use_count, + "Workdir operation blocked by busy resource"); RuntimeError::WorkingDirectory( crate::working_directory::WorkingDirectoryDiagnostic::rejected( "working_directory_cleanup_resource_busy", @@ -1148,8 +1159,12 @@ impl Runtime { let backend = { let mut state = self.lock()?; state.ensure_running()?; - if state.workdir_operations.contains_key(working_directory_id) { - return Err(workdir_operation_busy()); + if let Some(blocking) = state.workdir_operations.get(working_directory_id) { + return Err(workdir_operation_busy( + working_directory_id, + "cleanup_working_directory", + blocking, + )); } if let Some(worker_id) = state.worker_id_for_workdir(working_directory_id) { return Err(RuntimeError::InvalidRequest(format!( @@ -6318,11 +6333,8 @@ impl RuntimeState { } fn ensure_workdir_not_removing(&self, id: &str) -> Result<(), RuntimeError> { - if matches!( - self.workdir_operations.get(id), - Some(WorkdirOperation::Cleanup) - ) { - return Err(workdir_operation_busy()); + if let Some(blocking @ WorkdirOperation::Cleanup) = self.workdir_operations.get(id) { + return Err(workdir_operation_busy(id, "admit_use", blocking)); } Ok(()) } @@ -9345,6 +9357,84 @@ mod tests { .block_on(subscription.recv()) } + #[test] + fn busy_admission_logs_runtime_use_and_cleanup_blockers() { + use std::io::{Read, Seek, SeekFrom}; + + let mut output = tempfile::tempfile().unwrap(); + let writer = output.try_clone().unwrap(); + let subscriber = tracing_subscriber::fmt() + .with_ansi(false) + .without_time() + .with_writer(move || writer.try_clone().unwrap()) + .finish(); + tracing::subscriber::with_default(subscriber, || { + tracing::callsite::rebuild_interest_cache(); + let (runtime, backend) = runtime_and_backend(); + let first = runtime.reserve_workdir_use(["workdir-used"]).unwrap(); + let second = runtime.reserve_workdir_use(["workdir-used"]).unwrap(); + assert_workdir_busy( + runtime + .cleanup_working_directory("workdir-used") + .unwrap_err(), + ); + drop((first, second)); + assert!(runtime.inner.lock().unwrap().workdir_operations.is_empty()); + + let (entered, release) = WorkdirEffectGate::install(&backend.cleanup_gate); + let cleanup_runtime = runtime.clone(); + let cleanup = std::thread::spawn(move || { + cleanup_runtime.cleanup_working_directory("workdir-removing") + }); + entered + .recv_timeout(std::time::Duration::from_secs(5)) + .unwrap(); + assert_workdir_busy( + runtime + .cleanup_working_directory("workdir-removing") + .unwrap_err(), + ); + assert_workdir_busy( + runtime + .reserve_workdir_use(["workdir-removing"]) + .unwrap_err(), + ); + release.send(()).unwrap(); + assert_workdir_busy(cleanup.join().unwrap().unwrap_err()); + assert!(runtime.inner.lock().unwrap().workdir_operations.is_empty()); + }); + output.seek(SeekFrom::Start(0)).unwrap(); + let mut log = String::new(); + output.read_to_string(&mut log).unwrap(); + let events: Vec<_> = log + .lines() + .filter(|line| line.contains("busy_source=")) + .collect(); + assert_eq!(events.len(), 3, "{log}"); + for (event, (id, action, reason, count)) in events.iter().zip([ + ("workdir-used", "cleanup_working_directory", "active_use", 2), + ( + "workdir-removing", + "cleanup_working_directory", + "cleanup_in_progress", + 0, + ), + ("workdir-removing", "admit_use", "cleanup_in_progress", 0), + ]) { + for field in [ + "WARN".to_string(), + "yoi::workdir_cleanup".to_string(), + format!("working_directory_id=\"{id}\""), + format!("action=\"{action}\""), + "busy_source=\"runtime_operation\"".to_string(), + format!("busy_reason=\"{reason}\""), + format!("active_use_count={count}"), + ] { + assert!(event.contains(&field), "missing {field}: {event}"); + } + } + } + #[test] fn workdir_removal_reserves_only_its_target_and_releases_on_failure() { let (runtime, backend) = runtime_and_backend(); diff --git a/crates/worker-runtime/src/working_directory/occupancy.rs b/crates/worker-runtime/src/working_directory/occupancy.rs index c3e477bb5..031d1cda7 100644 --- a/crates/worker-runtime/src/working_directory/occupancy.rs +++ b/crates/worker-runtime/src/working_directory/occupancy.rs @@ -28,7 +28,19 @@ pub(super) struct Lease { id: String, } -fn busy() -> WorkingDirectoryDiagnostic { +fn busy(id: &str, action: &str, blocking: &Entry) -> WorkingDirectoryDiagnostic { + let (busy_reason, active_use_count) = match blocking { + Entry::Use(count) => ("active_use", *count), + Entry::Create => ("creation_in_progress", 0), + Entry::Cleanup => ("cleanup_in_progress", 0), + }; + tracing::warn!(target: "yoi::workdir_cleanup", + working_directory_id = id, + action, + busy_source = "materializer_occupancy", + busy_reason, + active_use_count, + "Workdir operation blocked by busy resource"); WorkingDirectoryDiagnostic::new( "working_directory_cleanup_resource_busy", "A filesystem resource is busy or changed during removal. Release the resource and retry.", @@ -42,7 +54,9 @@ impl Occupancy { .lock() .unwrap_or_else(|error| error.into_inner()); match entries.get_mut(id) { - Some(Entry::Cleanup | Entry::Create) => return Err(busy()), + Some(blocking @ (Entry::Cleanup | Entry::Create)) => { + return Err(busy(id, "acquire_use", blocking)); + } Some(Entry::Use(count)) => *count += 1, None => { entries.insert(id.to_string(), Entry::Use(1)); @@ -101,8 +115,8 @@ impl Occupancy { .entries .lock() .unwrap_or_else(|error| error.into_inner()); - if entries.contains_key(id) { - return Err(busy()); + if let Some(blocking) = entries.get(id) { + return Err(busy(id, "acquire_creation", blocking)); } entries.insert(id.to_string(), Entry::Create); Ok(Lease { @@ -116,8 +130,8 @@ impl Occupancy { .entries .lock() .unwrap_or_else(|error| error.into_inner()); - if entries.contains_key(id) { - return Err(busy()); + if let Some(blocking) = entries.get(id) { + return Err(busy(id, "acquire_cleanup", blocking)); } entries.insert(id.to_string(), Entry::Cleanup); Ok(Lease { @@ -131,6 +145,71 @@ impl Occupancy { mod tests { use super::*; + #[test] + fn busy_admission_logs_blocker_and_use_count_without_changing_error() { + use std::io::{Read, Seek, SeekFrom}; + + let mut output = tempfile::tempfile().unwrap(); + let writer = output.try_clone().unwrap(); + let subscriber = tracing_subscriber::fmt() + .with_ansi(false) + .without_time() + .with_writer(move || writer.try_clone().unwrap()) + .finish(); + tracing::subscriber::with_default(subscriber, || { + tracing::callsite::rebuild_interest_cache(); + let occupancy = Occupancy::default(); + for (id, leases) in [ + ( + "workdir-used", + vec![ + occupancy.acquire_use("workdir-used").unwrap(), + occupancy.acquire_use("workdir-used").unwrap(), + ], + ), + ( + "workdir-creating", + vec![occupancy.acquire_creation("workdir-creating").unwrap()], + ), + ( + "workdir-removing", + vec![occupancy.acquire_cleanup("workdir-removing").unwrap()], + ), + ] { + let error = occupancy.acquire_cleanup(id).unwrap_err(); + assert_eq!(error.code, "working_directory_cleanup_resource_busy"); + assert_eq!( + error.message, + "A filesystem resource is busy or changed during removal. Release the resource and retry." + ); + drop(leases); + drop(occupancy.acquire_cleanup(id).unwrap()); + } + }); + output.seek(SeekFrom::Start(0)).unwrap(); + let mut log = String::new(); + output.read_to_string(&mut log).unwrap(); + let events: Vec<_> = log.lines().collect(); + assert_eq!(events.len(), 3, "{log}"); + for (event, (id, reason, count)) in events.iter().zip([ + ("workdir-used", "active_use", 2), + ("workdir-creating", "creation_in_progress", 0), + ("workdir-removing", "cleanup_in_progress", 0), + ]) { + for field in [ + "WARN".to_string(), + "yoi::workdir_cleanup".to_string(), + format!("working_directory_id=\"{id}\""), + "action=\"acquire_cleanup\"".to_string(), + "busy_source=\"materializer_occupancy\"".to_string(), + format!("busy_reason=\"{reason}\""), + format!("active_use_count={count}"), + ] { + assert!(event.contains(&field), "missing {field}: {event}"); + } + } + } + #[test] fn use_and_cleanup_leases_release_during_unwind() { let occupancy = Occupancy::default(); From 00d96b716323079b11b99003296700c27ab70d42 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 30 Oct 2026 20:33:57 +0900 Subject: [PATCH 5/6] fix: scope Workdir removal to explicit requests --- crates/server-api/src/lib.rs | 2 + .../src/feature/builtin/manage_workdir.rs | 97 ++- crates/workspace-server/src/server.rs | 689 ++++++++++++++---- .../src/server/tests/worker_drive_tests.rs | 1 + .../src/server_workspace_config_tests.rs | 1 + .../workspace-server/src/workdir_removal.rs | 266 +++++-- openapi/server-api.json | 7 +- .../src/lib/generated/companion-api.ts | 2 +- .../src/lib/generated/repository-api.ts | 2 +- .../src/lib/generated/runtime-api.ts | 2 +- web/workspace/src/lib/generated/ticket-api.ts | 2 +- .../src/lib/generated/workdir-api.ts | 4 +- .../src/lib/generated/worker-launch-api.ts | 2 +- .../lib/workspace/settings/workdir-removal.ts | 38 +- .../[runtimeId]/workdirs/PageContent.svelte | 5 +- web/workspace/tests/workdir-removal.test.ts | 50 +- 16 files changed, 916 insertions(+), 254 deletions(-) diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index 59b2d6b8f..17dba8b22 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -7064,6 +7064,8 @@ pub struct WorkingDirectoryCleanupTarget { #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(deny_unknown_fields)] pub struct WorkingDirectoryRemovalRequest { + /// Stable for an exact resend; a new explicit request uses a new identity. + pub operation_id: String, pub reason: String, } diff --git a/crates/worker/src/feature/builtin/manage_workdir.rs b/crates/worker/src/feature/builtin/manage_workdir.rs index 9158821de..a197867e2 100644 --- a/crates/worker/src/feature/builtin/manage_workdir.rs +++ b/crates/worker/src/feature/builtin/manage_workdir.rs @@ -778,7 +778,11 @@ impl WorkspaceHttpWorkdirBackend { ) } - fn delete(&self, input: WorkdirDeleteInput) -> Result { + fn delete( + &self, + input: WorkdirDeleteInput, + operation_id: String, + ) -> Result { let workdir_id = validate_identity( &input.working_directory_id, DELETE_TOOL, @@ -790,6 +794,7 @@ impl WorkspaceHttpWorkdirBackend { WorkspaceRequestMethod::Delete, format!("/api/w/{workspace_id}/working-directories/{workdir_path}"), serde_json::to_string(&WorkdirRemovalRequest { + operation_id, reason: validate_delete_reason(&input.reason)?.to_string(), }) .map_err(decode_error)?, @@ -800,7 +805,7 @@ impl WorkspaceHttpWorkdirBackend { response.disposition, removal_guidance(response.failure_category.as_deref()), if response.retryable { - " After resolving the cause, repeat this WorkdirDelete with the same id and reason; current safety conditions will be rechecked." + " After resolving the cause, a new explicit WorkdirDelete request will recheck current safety conditions. This failed request will not be automatically re-executed." } else { "" } @@ -954,9 +959,10 @@ impl Tool for WorkspaceHttpWorkdirTool { self.backend.detach_managed(&input.alias, None).await } - WorkdirOperation::Delete => self - .backend - .delete(parse_input::(input_json)?), + WorkdirOperation::Delete => self.backend.delete( + parse_input::(input_json)?, + ctx.call_id.to_string(), + ), } } } @@ -1018,8 +1024,14 @@ fn removal_guidance(category: Option<&str>) -> &'static str { Some("runtime_unavailable" | "provider_unavailable") => { "Restore the Runtime connection; the registry remains until deletion is confirmed." } - Some("provider_observation_unknown" | "provider_cleanup_outcome_unknown") => { - "Deletion is not confirmed. Re-observe with normal removal after restoring provider access; the registry is retained." + Some("provider_cleanup_outcome_unknown") => { + "Deletion outcome is unknown. Restore provider access and reconcile the existing request; do not submit a fresh deletion intent. Recovery only observes the target and never repeats deletion." + } + Some("provider_observation_unknown") => { + "The target could not be observed before deletion. Restore provider access before a new request." + } + Some("request_interrupted") => { + "The request was interrupted before execution. A new explicit request is required." } Some("unsupported_target") => { "Ask the owner for the provider's supported cleanup operation." @@ -1337,8 +1349,44 @@ mod tests { }) } + #[tokio::test] + async fn delete_call_identity_distinguishes_new_requests_from_exact_resends() { + let client = Arc::new(RecordingWorkspaceClient::new( + (0..3) + .map(|_| { + response(json!({ + "working_directory_id": "wd-request", + "disposition": "attention_required", + "retryable": true, + "failure_category": "resource_busy" + })) + }) + .collect(), + )); + let tool = WorkspaceHttpWorkdirTool { + backend: WorkspaceHttpWorkdirBackend::new(client.clone()), + operation: WorkdirOperation::Delete, + }; + let input = r#"{"working_directory_id":"wd-request","reason":"remove unused Workdir"}"#; + for id in ["delete-1", "delete-1", "delete-2"] { + tool.execute(input, ToolExecutionContext::new(id, "batch", 0)) + .await + .unwrap(); + } + let bodies: Vec = client + .requests() + .iter() + .map(|r| serde_json::from_str(r.body.as_deref().unwrap()).unwrap()) + .collect(); + assert_eq!(bodies[0], bodies[1]); + assert_eq!(bodies[0]["operation_id"], "delete-1"); + assert_eq!(bodies[2]["operation_id"], "delete-2"); + assert_eq!(bodies[0]["reason"], bodies[2]["reason"]); + assert!(delete_schema()["properties"].get("operation_id").is_none()); + } + #[test] - fn ordinary_workdir_delete_reports_safe_cause_and_exact_retry_guidance() { + fn ordinary_workdir_delete_reports_safe_cause_and_new_request_guidance() { let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({ "working_directory_id": "wd-retry", "disposition": "attention_required", @@ -1347,10 +1395,13 @@ mod tests { }))])); let backend = WorkspaceHttpWorkdirBackend::new(client.clone()); let output = backend - .delete(WorkdirDeleteInput { - working_directory_id: "wd-retry".into(), - reason: "remove unused Workdir".into(), - }) + .delete( + WorkdirDeleteInput { + working_directory_id: "wd-retry".into(), + reason: "remove unused Workdir".into(), + }, + "delete-call-1".to_string(), + ) .unwrap(); assert!(output.summary.contains("Confirm its owner")); assert!( @@ -1358,7 +1409,11 @@ mod tests { .summary .contains("Do not recursively delete mounted data") ); - assert!(output.summary.contains("same id and reason")); + assert!( + output + .summary + .contains("new explicit WorkdirDelete request") + ); assert!(output.summary.len() <= 600); let content: serde_json::Value = serde_json::from_str(output.content.as_deref().unwrap()).unwrap(); @@ -1596,10 +1651,13 @@ mod tests { .unwrap(); backend.detach("checkout").unwrap(); backend - .delete(WorkdirDeleteInput { - working_directory_id: "wd-created".to_string(), - reason: "remove stale Workdir".to_string(), - }) + .delete( + WorkdirDeleteInput { + working_directory_id: "wd-created".to_string(), + reason: "remove stale Workdir".to_string(), + }, + "delete-call-1".to_string(), + ) .unwrap(); let requests = client.requests(); @@ -1637,7 +1695,10 @@ mod tests { assert_eq!(requests[4].method, WorkspaceRequestMethod::Delete); let body: serde_json::Value = serde_json::from_str(requests[4].body.as_deref().unwrap()).unwrap(); - assert_eq!(body, json!({"reason": "remove stale Workdir"})); + assert_eq!( + body, + json!({"reason": "remove stale Workdir", "operation_id": "delete-call-1"}) + ); } #[tokio::test] diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index bca0a9580..dce299e60 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -26932,7 +26932,14 @@ async fn cleanup_runtime_working_directory( .host()? .run(move || { let registered_runtime = - registered_workdir_runtime_id(&api, &path.working_directory_id)?; + match registered_workdir_runtime_id(&api, &path.working_directory_id) { + Ok(runtime) => runtime, + Err(error) => api + .config_store + .completed_workdir_removal(api.workspace_id(), &path.working_directory_id)? + .map(|removed| removed.runtime_id) + .ok_or(error)?, + }; if registered_runtime != path.runtime_id { return Err(ApiError::from(Error::WorkspacePermissionDenied( "Workdir does not belong to the requested Runtime".to_string(), @@ -26943,6 +26950,7 @@ async fn cleanup_runtime_working_directory( &path.working_directory_id, &source_actor, &request.reason, + &request.operation_id, ) .map_err(ApiError::from) }) @@ -28230,6 +28238,7 @@ async fn cleanup_working_directory( &path.working_directory_id, &source_actor, &request.reason, + &request.operation_id, ) .map_err(ApiError::from) }) @@ -28969,8 +28978,14 @@ fn workdir_removal_failure_guidance(category: &str) -> &'static str { "runtime_unavailable" | "provider_unavailable" => { "Runtime is unavailable. Reconnect it, then retry deletion; the Workdir registry is retained." } - "provider_observation_unknown" | "provider_cleanup_outcome_unknown" => { - "Runtime has not confirmed deletion. Retry to re-observe the target; the Workdir registry is retained." + "provider_cleanup_outcome_unknown" => { + "Deletion outcome is unknown. Reconcile the existing request; recovery only observes and never repeats deletion." + } + "provider_observation_unknown" => { + "The target could not be observed before deletion. Restore provider access before a new request." + } + "request_interrupted" => { + "The request was interrupted before execution. A new explicit request is required." } "unsupported_target" => "This provider does not support Workdir deletion.", _ => { @@ -29006,7 +29021,7 @@ fn current_workdir_removal_attempt_owner() -> Result fn workdir_removal_attempt_is_orphaned(operation: &WorkdirRemovalOperation) -> Result { let Some(owner) = operation.attempt_owner else { - return Ok(operation.attempt_id.is_none()); + return Ok(true); }; match observe_workdir_removal_owner(owner.process_id) { WorkdirRemovalOwnerObservation::Running { @@ -29112,12 +29127,12 @@ fn execute_reserved_workdir_removal_with_provider( recovery: bool, provider: &dyn WorkdirRemovalRuntimeProvider, ) -> Result { - if operation.state == WorkdirRemovalOperationState::Completed - && operation.disposition == Some(WorkdirRemovalDisposition::Removed) - { + if operation.state != WorkdirRemovalOperationState::Pending { return Ok(operation); } - let operation = if recovery && operation.state == WorkdirRemovalOperationState::Pending { + let unstarted = operation.attempt_id.is_none(); + let reconcile_only = recovery || !unstarted; + let operation = if reconcile_only { if !workdir_removal_attempt_is_orphaned(&operation)? { return Err(Error::WorkdirAttachmentConflict(format!( "Workdir removal operation `{}` still has a live attempt owner", @@ -29141,6 +29156,19 @@ fn execute_reserved_workdir_removal_with_provider( api.workdir_remove_attempt_owner, )? }; + if operation.state != WorkdirRemovalOperationState::Pending { + return Ok(operation); + } + if reconcile_only { + if unstarted { + return api.config_store.fail_workdir_removal_operation( + &operation, + "request_interrupted", + true, + ); + } + return reconcile_workdir_removal(api, &operation, provider); + } let guards = match api.config_store.workdir_removal_guards(&operation) { Ok(guards) => guards, Err( @@ -29232,10 +29260,16 @@ fn execute_reserved_workdir_removal_with_provider( let deleted = match deleted { Ok(deleted) => deleted, Err(error) => { - let (category, retryable) = classify_workdir_provider_error(&error); - return api - .config_store - .fail_workdir_removal_operation(&operation, category, retryable); + if let RuntimeRegistryError::RuntimeOperationFailed { code, .. } = &error + && let Some(category) = workdir_cleanup_diagnostic_category(code) + { + return api.config_store.fail_workdir_removal_operation( + &operation, + category, + category != "unsupported_target", + ); + } + return api.config_store.record_workdir_removal_unknown(&operation); } }; let deletion_confirmed = runtime_reports_workdir_not_found(&deleted) @@ -29257,6 +29291,9 @@ fn execute_reserved_workdir_removal_with_provider( category, "Runtime did not confirm Workdir removal; registry retained" ); + if deleted.state == InternalWorkerOperationState::Accepted { + return api.config_store.record_workdir_removal_unknown(&operation); + } return api.config_store.fail_workdir_removal_operation( &operation, category, @@ -29280,42 +29317,71 @@ fn workdir_removal_execution_lock( .clone()) } +fn reserve_requested_workdir_removal( + api: &WorkspaceApi, + working_directory_id: &str, + source_actor: &str, + reason: &str, + request_id: &str, +) -> Result { + let operation_id = crate::workdir_removal::workdir_removal_request_id( + api.workspace_id(), + source_actor, + request_id, + )?; + if let Some(existing) = api + .config_store + .get_workdir_removal_operation(api.workspace_id(), &operation_id)? + { + if existing.working_directory_id != working_directory_id + || existing.source_actor != source_actor + || existing.reason != reason + { + return Err(Error::WorkdirAttachmentConflict( + "Workdir removal request ID was reused with different input".into(), + )); + } + return Ok(existing); + } + let Some(workdir) = api + .config_store + .get_workdir_registry(api.workspace_id(), working_directory_id)? + else { + return api + .config_store + .completed_workdir_removal(api.workspace_id(), working_directory_id)? + .ok_or_else(|| { + Error::InvalidInput(format!("Unknown Workdir \x60{working_directory_id}\x60")) + }); + }; + let intent = workdir_removal_intent(&workdir, source_actor, reason, request_id)?; + api.config_store.reserve_workdir_removal_operation(&intent) +} + fn execute_workdir_removal( api: &WorkspaceApi, working_directory_id: &str, source_actor: &str, reason: &str, + request_id: &str, ) -> Result { let reason = reason.trim(); - if reason.is_empty() || reason.len() > 500 { + if reason.is_empty() || reason.len() > 500 || reason.chars().any(char::is_control) { return Err(Error::InvalidInput( - "Workdir removal reason must be between 1 and 500 bytes".to_string(), + "Workdir removal reason must be between 1 and 500 bytes without controls".into(), )); } let lock = workdir_removal_execution_lock(api, working_directory_id)?; let _guard = lock .lock() - .map_err(|_| Error::Store("Workdir removal lock was poisoned".to_string()))?; - - let operation = if let Some(existing) = - api.config_store.find_workdir_removal_operation_by_intent( - api.workspace_id(), - working_directory_id, - source_actor, - reason, - )? { - existing - } else { - let workdir = api - .config_store - .get_workdir_registry(api.workspace_id(), working_directory_id)? - .ok_or_else(|| { - Error::InvalidInput(format!("Unknown Workdir `{working_directory_id}`")) - })?; - let intent = workdir_removal_intent(&workdir, source_actor, reason)?; - api.config_store - .reserve_workdir_removal_operation(&intent)? - }; + .map_err(|_| Error::Store("Workdir removal lock was poisoned".into()))?; + let operation = reserve_requested_workdir_removal( + api, + working_directory_id, + source_actor, + reason, + request_id, + )?; execute_reserved_workdir_removal(api, operation, false) .map(|operation| workdir_removal_response(&operation)) } @@ -29326,35 +29392,66 @@ fn execute_workdir_removal_for_workspace_deletion( parent_operation_id: &str, ) -> Result { let source_actor = format!("workspace-deletion:{parent_operation_id}"); - let reason = "Workspace deletion"; + let request_id = format!("{parent_operation_id}:{working_directory_id}"); let lock = workdir_removal_execution_lock(api, working_directory_id)?; let _guard = lock .lock() - .map_err(|_| Error::Store("Workdir removal lock was poisoned".to_string()))?; - let operation = if let Some(existing) = - api.config_store.find_workdir_removal_operation_by_intent( - api.workspace_id(), - working_directory_id, - &source_actor, - reason, - )? { - existing - } else { - let workdir = api - .config_store - .get_workdir_registry(api.workspace_id(), working_directory_id)? - .ok_or_else(|| { - Error::InvalidInput(format!("Unknown Workdir `{working_directory_id}`")) - })?; - let intent = workdir_removal_intent(&workdir, &source_actor, reason)?; - api.config_store - .reserve_workdir_removal_operation(&intent)? - }; + .map_err(|_| Error::Store("Workdir removal lock was poisoned".into()))?; + let operation = reserve_requested_workdir_removal( + api, + working_directory_id, + &source_actor, + "Workspace deletion", + &request_id, + )?; api.config_store .append_workspace_deletion_child_operation(parent_operation_id, &operation.operation_id)?; execute_reserved_workdir_removal(api, operation, false) } +// Recovery may confirm an external effect, but never issues another deletion. +fn reconcile_workdir_removal( + api: &WorkspaceApi, + operation: &WorkdirRemovalOperation, + provider: &dyn WorkdirRemovalRuntimeProvider, +) -> Result { + let current = api + .config_store + .get_workdir_registry(&operation.workspace_id, &operation.working_directory_id)?; + if current.is_none() { + if api + .config_store + .completed_workdir_removal(&operation.workspace_id, &operation.working_directory_id)? + .is_some_and(|removed| { + removed.materialization_fingerprint == operation.materialization_fingerprint + }) + { + return api.config_store.commit_workdir_removal_removed(operation); + } + return api.config_store.fail_workdir_removal_operation( + operation, + "authority_changed", + false, + ); + } + if current.as_ref().is_some_and(|record| { + crate::workdir_removal::workdir_materialization_fingerprint(record) + != operation.materialization_fingerprint + }) { + return api.config_store.fail_workdir_removal_operation( + operation, + "authority_changed", + false, + ); + } + match provider.observe_workdir(&operation.runtime_id, &operation.working_directory_id) { + Ok(observed) if runtime_reports_workdir_not_found(&observed) => { + api.config_store.commit_workdir_removal_removed(operation) + } + _ => api.config_store.record_workdir_removal_unknown(operation), + } +} + fn recover_workdir_removals(api: &WorkspaceApi) -> Result<()> { for operation in api .config_store @@ -29798,6 +29895,7 @@ async fn execute_runtime_cleanup_with_context( removal_candidate.workdir_id.as_str(), &requester, &format!("cleanup target {}", removal_candidate.target_id), + &uuid::Uuid::now_v7().to_string(), ) }) .await?; @@ -33870,8 +33968,7 @@ fn finalize_spawn_compensation_after_worker_delete( api, workdir_id, "backend:worker_spawn_compensation", - "remove Workdir created by rejected Worker spawn", - ) { + "remove Workdir created by rejected Worker spawn", &uuid::Uuid::now_v7().to_string(),) { Ok(result) if result.disposition == WorkingDirectoryRemovalDisposition::Removed => {} Ok(result) => diagnostics.push(spawn_compensation_diagnostic( "worker_spawn_compensation_workdir_cleanup_failed", @@ -33907,8 +34004,7 @@ fn cleanup_spawn_created_workdirs( api, workdir_id, "backend:worker_spawn_compensation", - "remove Workdir created before rejected Worker spawn", - ) { + "remove Workdir created before rejected Worker spawn", &uuid::Uuid::now_v7().to_string(),) { Ok(result) if result.disposition == WorkingDirectoryRemovalDisposition::Removed => {} Ok(result) => diagnostics.push(spawn_compensation_diagnostic( "worker_spawn_compensation_workdir_cleanup_failed", @@ -60666,7 +60762,8 @@ mod tests { let app = build_router(api.clone()); for (workdir_id, path, reason) in fixtures { - let body = serde_json::to_vec(&json!({ "reason": reason })).unwrap(); + let body = serde_json::to_vec(&json!({ "reason": reason, "operation_id": workdir_id })) + .unwrap(); let response = app .clone() .oneshot(runtime_source_request( @@ -60700,11 +60797,14 @@ mod tests { ); let operation = api .config_store - .find_workdir_removal_operation_by_intent( + .get_workdir_removal_operation( TEST_WORKSPACE_ID, - workdir_id, - "worker:runtime-source:worker-source", - reason, + &crate::workdir_removal::workdir_removal_request_id( + TEST_WORKSPACE_ID, + "worker:runtime-source:worker-source", + workdir_id, + ) + .unwrap(), ) .unwrap() .unwrap(); @@ -60712,6 +60812,29 @@ mod tests { operation.source_actor, "worker:runtime-source:worker-source" ); + for request_id in [workdir_id.to_string(), format!("{workdir_id}-fresh")] { + let replay = app + .clone() + .oneshot(runtime_source_request( + &identity, + Some("worker-source"), + "DELETE", + &path, + serde_json::to_vec(&json!({"reason": reason, "operation_id": request_id})) + .unwrap(), + )) + .await + .unwrap(); + assert_eq!(replay.status(), StatusCode::OK); + let result: WorkingDirectoryRemovalResponse = serde_json::from_slice( + &to_bytes(replay.into_body(), usize::MAX).await.unwrap(), + ) + .unwrap(); + assert_eq!( + result.disposition, + WorkingDirectoryRemovalDisposition::Removed + ); + } assert_eq!(operation.state, WorkdirRemovalOperationState::Completed); assert_eq!( operation.disposition, @@ -60754,7 +60877,10 @@ mod tests { None, "DELETE", &runtime_only_path, - serde_json::to_vec(&json!({ "reason": "runtime only" })).unwrap(), + serde_json::to_vec( + &json!({ "reason": "runtime only", "operation_id": "denied-removal" }), + ) + .unwrap(), )) .await .unwrap(); @@ -60773,7 +60899,10 @@ mod tests { worker_runtime::auth::RUNTIME_REQUEST_SOURCE_PROOF_HEADER, "not-a-valid-proof", ) - .body(Body::from(json!({ "reason": "invalid proof" }).to_string())) + .body(Body::from( + json!({ "reason": "invalid proof", "operation_id": "denied-removal" }) + .to_string(), + )) .unwrap(), ) .await @@ -60789,7 +60918,8 @@ mod tests { .uri(&unauthenticated_path) .header(CONTENT_TYPE, "application/json") .body(Body::from( - json!({ "reason": "unauthenticated" }).to_string(), + json!({ "reason": "unauthenticated", "operation_id": "denied-removal" }) + .to_string(), )) .unwrap(), ) @@ -60830,7 +60960,7 @@ mod tests { build_router(api.clone()), "DELETE", &path, - Some(json!({ "reason": reason })), + Some(json!({ "reason": reason, "operation_id": workdir_id })), &token, StatusCode::OK, ) @@ -60844,11 +60974,14 @@ mod tests { ); let operation = api .config_store - .find_workdir_removal_operation_by_intent( + .get_workdir_removal_operation( TEST_WORKSPACE_ID, - workdir_id, - &format!("account:account-{TEST_WORKSPACE_ID}"), - reason, + &crate::workdir_removal::workdir_removal_request_id( + TEST_WORKSPACE_ID, + &format!("account:account-{TEST_WORKSPACE_ID}"), + workdir_id, + ) + .unwrap(), ) .unwrap() .unwrap(); @@ -61181,8 +61314,13 @@ mod tests { .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 intent = workdir_removal_intent( + &record, + "account:owner", + "remove docs checkout", + "test-removal-request", + ) + .unwrap(); let pending = api .config_store .reserve_workdir_removal_operation(&intent) @@ -61958,6 +62096,7 @@ mod tests { &record, "account:owner", &format!("remove {working_directory_id}"), + working_directory_id, ) .unwrap(); let operation = api @@ -61967,6 +62106,212 @@ mod tests { (operation, summary) } + #[tokio::test] + async fn workdir_removal_failure_never_recovers_as_a_job_and_new_request_can_remove() { + let workspace = tempfile::tempdir().unwrap(); + init_clean_git_workspace(workspace.path()); + let api = test_api(workspace.path()).await; + let (operation, summary) = reserve_removal_fixture(&api, "terminal-busy"); + let provider = FakeWorkdirRemovalProvider::new( + workdir_removal_result( + InternalWorkerOperationState::Accepted, + Some(summary.clone()), + Vec::new(), + ), + workdir_removal_result( + InternalWorkerOperationState::Rejected, + None, + vec![RuntimeDiagnostic { + code: "working_directory_cleanup_resource_busy".into(), + severity: HostDiagnosticSeverity::Warning, + message: "busy".into(), + }], + ), + ); + let failed = + execute_reserved_workdir_removal_with_provider(&api, operation, false, &provider) + .unwrap(); + assert_eq!(failed.state, WorkdirRemovalOperationState::Failed); + for recovery in [false, true] { + assert_eq!( + execute_reserved_workdir_removal_with_provider( + &api, + failed.clone(), + recovery, + &provider + ) + .unwrap(), + failed + ); + } + assert_eq!(provider.cleanup_calls(), 1); + assert!( + api.config_store + .recoverable_workdir_removal_operations(api.workspace_id(), 100) + .unwrap() + .is_empty() + ); + let new_request = reserve_requested_workdir_removal( + &api, + &failed.working_directory_id, + &failed.source_actor, + &failed.reason, + "new-explicit-request", + ) + .unwrap(); + assert_ne!(new_request.operation_id, failed.operation_id); + assert!( + reserve_requested_workdir_removal( + &api, + &failed.working_directory_id, + &failed.source_actor, + "changed reason", + "new-explicit-request" + ) + .is_err() + ); + assert!( + reserve_requested_workdir_removal( + &api, + "different-workdir", + &failed.source_actor, + &failed.reason, + "new-explicit-request" + ) + .is_err() + ); + let success = FakeWorkdirRemovalProvider::new( + workdir_removal_result( + InternalWorkerOperationState::Accepted, + Some(summary), + Vec::new(), + ), + confirmed_workdir_cleanup_result(), + ); + let removed = + execute_reserved_workdir_removal_with_provider(&api, new_request, false, &success) + .unwrap(); + assert_eq!( + removed.disposition, + Some(WorkdirRemovalDisposition::Removed) + ); + let already_removed = execute_workdir_removal( + &api, + &failed.working_directory_id, + &failed.source_actor, + &failed.reason, + "another-explicit-request", + ) + .unwrap(); + assert_eq!( + already_removed.disposition, + WorkingDirectoryRemovalDisposition::Removed + ); + assert_eq!( + execute_reserved_workdir_removal_with_provider(&api, failed.clone(), false, &success) + .unwrap(), + failed + ); + assert_eq!(success.cleanup_calls(), 1); + } + + #[tokio::test] + async fn workdir_removal_unknown_outcome_recovery_only_observes_and_confirms_absence() { + let workspace = tempfile::tempdir().unwrap(); + init_clean_git_workspace(workspace.path()); + let api = test_api(workspace.path()).await; + let (operation, summary) = reserve_removal_fixture(&api, "lost-delete-response"); + let provider = FakeWorkdirRemovalProvider::new( + workdir_removal_result( + InternalWorkerOperationState::Accepted, + Some(summary.clone()), + Vec::new(), + ), + confirmed_workdir_cleanup_result(), + ); + *provider.cleanup.lock().unwrap() = + Some(Err(RuntimeRegistryError::RuntimeOperationFailed { + runtime_id: "runtime-test".into(), + code: "transport_timeout".into(), + message: "response lost".into(), + })); + let unknown = + execute_reserved_workdir_removal_with_provider(&api, operation, false, &provider) + .unwrap(); + assert_eq!(unknown.state, WorkdirRemovalOperationState::Pending); + assert_eq!( + unknown.failure_category.as_deref(), + Some("provider_cleanup_outcome_unknown") + ); + assert!(!unknown.retryable); + assert!( + reserve_requested_workdir_removal( + &api, + &unknown.working_directory_id, + &unknown.source_actor, + &unknown.reason, + "do-not-fork-unknown" + ) + .is_err() + ); + *provider.observation.lock().unwrap() = Some(Ok(workdir_removal_result( + InternalWorkerOperationState::Accepted, + Some(summary), + Vec::new(), + ))); + let still_unknown = + execute_reserved_workdir_removal_with_provider(&api, unknown, true, &provider).unwrap(); + assert_eq!(still_unknown.state, WorkdirRemovalOperationState::Pending); + assert_eq!(provider.cleanup_calls(), 1); + *provider.observation.lock().unwrap() = Some(Ok(confirmed_workdir_cleanup_result())); + let removed = + execute_reserved_workdir_removal_with_provider(&api, still_unknown, true, &provider) + .unwrap(); + assert_eq!( + removed.disposition, + Some(WorkdirRemovalDisposition::Removed) + ); + assert_eq!(provider.cleanup_calls(), 1); + assert!( + api.store + .get_workdir_registry(api.workspace_id(), &removed.working_directory_id) + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn workdir_removal_unstarted_request_recovery_does_not_start_deletion() { + let workspace = tempfile::tempdir().unwrap(); + init_clean_git_workspace(workspace.path()); + let api = test_api(workspace.path()).await; + let (operation, summary) = reserve_removal_fixture(&api, "unstarted-removal"); + let provider = FakeWorkdirRemovalProvider::new( + workdir_removal_result( + InternalWorkerOperationState::Accepted, + Some(summary), + Vec::new(), + ), + confirmed_workdir_cleanup_result(), + ); + let interrupted = + execute_reserved_workdir_removal_with_provider(&api, operation, true, &provider) + .unwrap(); + assert_eq!(interrupted.state, WorkdirRemovalOperationState::Failed); + assert_eq!( + interrupted.failure_category.as_deref(), + Some("request_interrupted") + ); + assert_eq!(provider.cleanup_calls(), 0); + assert!(provider.observation.lock().unwrap().is_some()); + assert!( + api.store + .get_workdir_registry(api.workspace_id(), &interrupted.working_directory_id) + .unwrap() + .is_some() + ); + } + #[tokio::test] async fn injectable_provider_covers_cleanup_not_found_unknown_dirty_and_unsupported() { let workspace = tempfile::tempdir().unwrap(); @@ -62089,9 +62434,26 @@ mod tests { confirmed_workdir_cleanup_result(), ); let dirty_attempt_id = dirty.attempt_id.clone(); + let replay = execute_reserved_workdir_removal_with_provider( + &api, + dirty.clone(), + false, + &clean_retry_provider, + ) + .unwrap(); + assert_eq!(replay, dirty); + assert_eq!(clean_retry_provider.cleanup_calls(), 0); + let new_request = reserve_requested_workdir_removal( + &api, + &dirty.working_directory_id, + &dirty.source_actor, + &dirty.reason, + "new-clean-request", + ) + .unwrap(); let removed_after_retry = execute_reserved_workdir_removal_with_provider( &api, - dirty, + new_request, false, &clean_retry_provider, ) @@ -62215,7 +62577,8 @@ mod tests { } #[tokio::test] - async fn workdir_removal_retry_rechecks_pending_materialization_changes_and_attachments() { + async fn new_workdir_removal_request_rechecks_pending_materialization_changes_and_attachments() + { let workspace = tempfile::tempdir().unwrap(); init_clean_git_workspace(workspace.path()); let api = test_api(workspace.path()).await; @@ -62263,8 +62626,16 @@ mod tests { ), confirmed_workdir_cleanup_result(), ); + let request = reserve_requested_workdir_removal( + &api, + &failed.working_directory_id, + &failed.source_actor, + &failed.reason, + "dirty-request", + ) + .unwrap(); let dirty = - execute_reserved_workdir_removal_with_provider(&api, failed, false, &dirty_provider) + execute_reserved_workdir_removal_with_provider(&api, request, false, &dirty_provider) .unwrap(); assert_eq!(dirty.disposition, Some(WorkdirRemovalDisposition::Retained)); assert!(dirty.retryable); @@ -62280,9 +62651,21 @@ mod tests { ), confirmed_workdir_cleanup_result(), ); - let occupied = - execute_reserved_workdir_removal_with_provider(&api, dirty, false, &occupied_provider) - .unwrap(); + let request = reserve_requested_workdir_removal( + &api, + &dirty.working_directory_id, + &dirty.source_actor, + &dirty.reason, + "occupied-request", + ) + .unwrap(); + let occupied = execute_reserved_workdir_removal_with_provider( + &api, + request, + false, + &occupied_provider, + ) + .unwrap(); assert_eq!( occupied.failure_category.as_deref(), Some("blocked_by_live_authority") @@ -62309,14 +62692,18 @@ mod tests { ), confirmed_workdir_cleanup_result(), ); - let removed = execute_reserved_workdir_removal_with_provider( + let request = reserve_requested_workdir_removal( &api, - occupied.clone(), - false, - &retry_provider, + &occupied.working_directory_id, + &occupied.source_actor, + &occupied.reason, + "final-request", ) .unwrap(); - assert_eq!(removed.operation_id, occupied.operation_id); + let removed = + execute_reserved_workdir_removal_with_provider(&api, request, false, &retry_provider) + .unwrap(); + assert_ne!(removed.operation_id, occupied.operation_id); assert_ne!(removed.attempt_id, occupied.attempt_id); assert_eq!( removed.disposition, @@ -62415,7 +62802,15 @@ mod tests { let result = execute_reserved_workdir_removal_with_provider(&api, operation, false, &provider) .unwrap(); - assert!(result.retryable); + assert_eq!(result.retryable, changed_identity); + assert_eq!( + result.state, + if changed_identity { + WorkdirRemovalOperationState::Failed + } else { + WorkdirRemovalOperationState::Pending + } + ); assert_eq!( result.failure_category.as_deref(), Some(if changed_identity { @@ -62449,7 +62844,7 @@ mod tests { } #[tokio::test] - async fn concurrent_orphan_recovery_runs_delayed_provider_cleanup_once() { + async fn concurrent_orphan_recovery_observes_once_without_reissuing_cleanup() { let workspace = tempfile::tempdir().unwrap(); init_clean_git_workspace(workspace.path()); let api = test_api(workspace.path()).await; @@ -62504,7 +62899,9 @@ mod tests { .iter() .filter(|result| { result.as_ref().is_ok_and(|operation| { - operation.disposition == Some(WorkdirRemovalDisposition::Removed) + operation.state == WorkdirRemovalOperationState::Pending + && operation.failure_category.as_deref() + == Some("provider_cleanup_outcome_unknown") }) }) .count(), @@ -62517,7 +62914,13 @@ mod tests { .count(), 1 ); - assert_eq!(provider.cleanup_calls(), 1); + assert_eq!(provider.cleanup_calls(), 0); + assert!( + api.store + .get_workdir_registry(TEST_WORKSPACE_ID, "orphan-race") + .unwrap() + .is_some() + ); } #[test] @@ -62546,65 +62949,64 @@ mod tests { } #[tokio::test] - async fn durable_workdir_removal_retries_provider_unavailable_without_deleting_registry() { + async fn failed_workdir_removal_replays_without_reexecution_and_new_request_is_distinct() { let workspace = tempfile::tempdir().unwrap(); init_clean_git_workspace(workspace.path()); let api = test_api(workspace.path()).await; seed_cleanup_workdir(&api, "missing-clean-workdir", "present", "clean"); - - let first = execute_workdir_removal( - &api, - "missing-clean-workdir", - "account:owner", - "remove stale clean Workdir", - ) - .unwrap(); - assert_eq!( - first.disposition, - WorkingDirectoryRemovalDisposition::AttentionRequired - ); - assert!(first.retryable); + let request = |id| { + execute_workdir_removal( + &api, + "missing-clean-workdir", + "account:owner", + "remove stale clean Workdir", + id, + ) + .unwrap() + }; + let saved = |id| { + api.config_store + .get_workdir_removal_operation( + api.workspace_id(), + &crate::workdir_removal::workdir_removal_request_id( + api.workspace_id(), + "account:owner", + id, + ) + .unwrap(), + ) + .unwrap() + .unwrap() + }; + let first = request("request-1"); assert_eq!( first.failure_category.as_deref(), Some("runtime_unavailable") ); + let first_record = saved("request-1"); + assert_eq!(request("request-1"), first); + assert_eq!(saved("request-1"), first_record); + recover_workdir_removals(&api).unwrap(); + assert_eq!(saved("request-1"), first_record); assert!( - api.store - .get_workdir_registry(&api.config.workspace_id, "missing-clean-workdir") + api.config_store + .recoverable_workdir_removal_operations(api.workspace_id(), 100) .unwrap() - .is_some() + .is_empty() ); - - let first_attempt_id = api - .config_store - .find_workdir_removal_operation_by_intent( - &api.config.workspace_id, + assert_eq!(request("request-2"), first); + assert_ne!(saved("request-2").operation_id, first_record.operation_id); + assert_ne!(saved("request-2").attempt_id, first_record.attempt_id); + assert!( + execute_workdir_removal( + &api, "missing-clean-workdir", "account:owner", - "remove stale clean Workdir", + "different reason", + "request-1" ) - .unwrap() - .unwrap() - .attempt_id; - let retry = execute_workdir_removal( - &api, - "missing-clean-workdir", - "account:owner", - "remove stale clean Workdir", - ) - .unwrap(); - assert_eq!(retry, first); - let operation = api - .config_store - .find_workdir_removal_operation_by_intent( - &api.config.workspace_id, - "missing-clean-workdir", - "account:owner", - "remove stale clean Workdir", - ) - .unwrap() - .unwrap(); - assert_ne!(operation.attempt_id, first_attempt_id); + .is_err() + ); } #[tokio::test] @@ -62621,6 +63023,7 @@ mod tests { "occupied-workdir", "account:owner", "remove occupied Workdir", + &uuid::Uuid::now_v7().to_string(), ) .unwrap(); assert_eq!( @@ -62654,6 +63057,7 @@ mod tests { &record, "account:owner", "do not steal a live provider call", + "test-removal-request", ) .unwrap(); let reserved = api @@ -62696,8 +63100,13 @@ mod tests { .get_workdir_registry(&api.config.workspace_id, "recovery-workdir") .unwrap() .unwrap(); - let intent = - workdir_removal_intent(&record, "account:owner", "recover provider cleanup").unwrap(); + let intent = workdir_removal_intent( + &record, + "account:owner", + "recover provider cleanup", + "test-removal-request", + ) + .unwrap(); let reserved = api .config_store .reserve_workdir_removal_operation(&intent) @@ -62723,12 +63132,12 @@ mod tests { .get_workdir_removal_operation(&api.config.workspace_id, &intent.operation_id) .unwrap() .unwrap(); - assert_eq!(operation.state, WorkdirRemovalOperationState::Failed); + assert_eq!(operation.state, WorkdirRemovalOperationState::Pending); assert_ne!(operation.attempt_id, interrupted_attempt.attempt_id); - assert!(operation.retryable); + assert!(!operation.retryable); assert_eq!( operation.failure_category.as_deref(), - Some("runtime_unavailable") + Some("provider_cleanup_outcome_unknown") ); assert!( api.store diff --git a/crates/workspace-server/src/server/tests/worker_drive_tests.rs b/crates/workspace-server/src/server/tests/worker_drive_tests.rs index f92b6d172..c0001b321 100644 --- a/crates/workspace-server/src/server/tests/worker_drive_tests.rs +++ b/crates/workspace-server/src/server/tests/worker_drive_tests.rs @@ -652,6 +652,7 @@ async fn source_workdir_removal_keeps_published_drive_bytes_and_receipt() { &removal_id, "account:drive-test", "source no longer needed", + "drive-source-removal", ) }) .await diff --git a/crates/workspace-server/src/server_workspace_config_tests.rs b/crates/workspace-server/src/server_workspace_config_tests.rs index 1c932be1a..9bbc39641 100644 --- a/crates/workspace-server/src/server_workspace_config_tests.rs +++ b/crates/workspace-server/src/server_workspace_config_tests.rs @@ -819,6 +819,7 @@ mod workspace_config_integration { }, "owner".into(), WorkingDirectoryRemovalRequest { + operation_id: "unsupported-logical-cleanup".into(), reason: "unsupported logical cleanup".into() } ) diff --git a/crates/workspace-server/src/workdir_removal.rs b/crates/workspace-server/src/workdir_removal.rs index 46378b3a7..c7f3b62e4 100644 --- a/crates/workspace-server/src/workdir_removal.rs +++ b/crates/workspace-server/src/workdir_removal.rs @@ -1,24 +1,15 @@ -//! Persists durable deletion intent and attempts for Repository-backed Workdirs. +//! Records one Repository-backed Workdir removal request and its result. //! -//! Deletion intent fixes Workspace, Workdir, owning Runtime, Repository/materialization, actor, -//! and reason. Endpoints, host paths, and provider handles do not grant caller deletion authority. +//! Caller-generated request identities distinguish an exact resend from a new explicit request; +//! reason text is audit input, never a replay key. Failed and completed records are terminal, +//! including legacy failed records marked retryable. Retryability only advises a new explicit +//! request after resolving its cause; it is not continuing authority to delete. +//! +//! Only one unresolved request per Workdir is allowed. Attempts are claimed transactionally and +//! fenced by process/start marker and attempt identity. Recovery observes uncertain external +//! effects but never reissues deletion. Confirmed absence and the matching materialization permit +//! registry removal and the saved result to commit together. Unconfirmed effects remain pending. //! External Workdirs follow grant revocation, not this operation. -//! -//! Only one pending operation per Workdir is allowed; attempts are claimed transactionally. -//! Process IDs and startup markers are retained. Recovery reclaims the exact owner/attempt by -//! compare-and-set only after confirming the previous owner is gone; live or unobservable owners -//! are not displaced. In-process locks alone are insufficient. -//! -//! Server execution revalidates stored materialization and current attachments, reservations, -//! assignments, retention, and creation operations, then observes occupancy and cleanliness via -//! Runtime. Dirty resources remain; unknown observations or provider failures retain the registry -//! and record attention/retryability. Only confirmed deletion or exact working_directory_not_found -//! is deletion evidence; registry removal and completed/removed commit in the same transaction. -//! -//! Normal deletion, Workspace deletion, cleanup, and startup recovery use the same reserved Server -//! operation execution. Failed DB commits after provider deletion are reobserved through that -//! operation. Recovery relies on provider identity and results, not an exactly-once guarantee -//! across all interruption points. use chrono::Utc; use rusqlite::{Connection, OptionalExtension, TransactionBehavior, params}; @@ -118,10 +109,22 @@ pub fn workdir_materialization_fingerprint(record: &WorkdirRegistryRecord) -> St hex_sha256(&bytes) } +pub fn workdir_removal_request_id( + workspace_id: &str, + source_actor: &str, + request_id: &str, +) -> Result { + validate_bounded("operation id", request_id, 256)?; + let bytes = serde_json::to_vec(&(workspace_id, source_actor, request_id)) + .map_err(|error| Error::Store(error.to_string()))?; + Ok(format!("wdr_{}", &hex_sha256(&bytes)[..32])) +} + pub fn workdir_removal_intent( record: &WorkdirRegistryRecord, source_actor: &str, reason: &str, + request_id: &str, ) -> Result { validate_bounded("source actor", source_actor, MAX_ACTOR_BYTES)?; validate_bounded("reason", reason, MAX_REASON_BYTES)?; @@ -146,7 +149,7 @@ pub fn workdir_removal_intent( )); }; Ok(WorkdirRemovalIntent { - operation_id: format!("wdr_{}", &request_fingerprint[..32]), + operation_id: workdir_removal_request_id(&record.workspace_id, source_actor, request_id)?, request_fingerprint, workspace_id: record.workspace_id.clone(), working_directory_id: record.workdir_id.clone(), @@ -285,17 +288,10 @@ 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 - && operation.disposition == Some(WorkdirRemovalDisposition::Removed) - { + if operation.state != WorkdirRemovalOperationState::Pending { tx.commit()?; return Ok(operation); } - if operation.state == WorkdirRemovalOperationState::Failed && !operation.retryable { - return Err(Error::InvalidInput(format!( - "Workdir removal operation `{operation_id}` is not retryable" - ))); - } if let Some((expected_owner, expected_attempt_id)) = recovery_expected { if operation.state != WorkdirRemovalOperationState::Pending || operation.attempt_owner != expected_owner @@ -473,7 +469,7 @@ impl SqliteWorkspaceStore { &operation.operation_id, &operation.request_fingerprint, )?; - if current.state == WorkdirRemovalOperationState::Completed { + if current.state != WorkdirRemovalOperationState::Pending { tx.commit()?; return Ok(current); } @@ -502,6 +498,39 @@ impl SqliteWorkspaceStore { }) } + /// Keep an uncertain external outcome for observation, not automatic re-execution. + pub fn record_workdir_removal_unknown( + &self, + operation: &WorkdirRemovalOperation, + ) -> Result { + self.with_conn_mut(|conn| { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let current = require_operation( + &tx, + &operation.workspace_id, + &operation.operation_id, + &operation.request_fingerprint, + )?; + if current.state != WorkdirRemovalOperationState::Pending { + tx.commit()?; + return Ok(current); + } + require_matching_attempt(operation, ¤t)?; + tx.execute( + "UPDATE workdir_removal_operations SET retryable=0, disposition='attention_required', failure_category='provider_cleanup_outcome_unknown', attempt_owner_pid=NULL, attempt_owner_start_marker=NULL, updated_at=?1 WHERE workspace_id=?2 AND operation_id=?3", + params![Utc::now().to_rfc3339(), operation.workspace_id, operation.operation_id], + )?; + let updated = require_operation( + &tx, + &operation.workspace_id, + &operation.operation_id, + &operation.request_fingerprint, + )?; + tx.commit()?; + Ok(updated) + }) + } + pub fn commit_workdir_removal_removed( &self, operation: &WorkdirRemovalOperation, @@ -531,7 +560,7 @@ impl SqliteWorkspaceStore { &operation.operation_id, &operation.request_fingerprint, )?; - if current.state == WorkdirRemovalOperationState::Completed { + if current.state != WorkdirRemovalOperationState::Pending { tx.commit()?; return Ok(current); } @@ -541,24 +570,29 @@ impl SqliteWorkspaceStore { &tx, &operation.workspace_id, &operation.working_directory_id, - )? - .ok_or_else(|| Error::RegistryInconsistency( - "Workdir registry row disappeared before durable removal commit".to_string(), - ))?; - require_operation_materialization(operation, ®istry)?; - require_no_removal_blockers( - &tx, - &operation.workspace_id, - &operation.working_directory_id, )?; - let deleted = tx.execute( - "DELETE FROM workdir_registry WHERE workspace_id=?1 AND workdir_id=?2", - params![operation.workspace_id, operation.working_directory_id], - )?; - if deleted != 1 { - return Err(Error::RegistryInconsistency( - "Workdir registry deletion did not remove exactly one row".to_string(), - )); + if let Some(registry) = registry { + require_operation_materialization(operation, ®istry)?; + require_no_removal_blockers( + &tx, + &operation.workspace_id, + &operation.working_directory_id, + )?; + tx.execute( + "DELETE FROM workdir_registry WHERE workspace_id=?1 AND workdir_id=?2", + params![operation.workspace_id, operation.working_directory_id], + )?; + } else { + let removed: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM workdir_removal_operations WHERE workspace_id=?1 AND workdir_id=?2 AND materialization_fingerprint=?3 AND state='completed' AND disposition='removed')", + params![operation.workspace_id, operation.working_directory_id, operation.materialization_fingerprint], + |row| row.get(0), + )?; + if !removed { + return Err(Error::RegistryInconsistency( + "Workdir registry disappeared without confirmed removal".to_string(), + )); + } } } let now = Utc::now().to_rfc3339(); @@ -585,20 +619,18 @@ impl SqliteWorkspaceStore { }) } - pub fn find_workdir_removal_operation_by_intent( + pub fn completed_workdir_removal( &self, workspace_id: &str, working_directory_id: &str, - source_actor: &str, - reason: &str, ) -> Result> { self.with_conn(|conn| { conn.query_row( &format!( - "{} WHERE workspace_id=?1 AND workdir_id=?2 AND source_actor=?3 AND reason=?4 ORDER BY created_at DESC, operation_id DESC LIMIT 1", + "{} WHERE workspace_id=?1 AND workdir_id=?2 AND state='completed' AND disposition='removed' ORDER BY completed_at DESC, operation_id DESC LIMIT 1", operation_select_sql() ), - params![workspace_id, working_directory_id, source_actor, reason], + params![workspace_id, working_directory_id], read_operation, ) .optional() @@ -614,7 +646,7 @@ impl SqliteWorkspaceStore { self.with_conn(|conn| { let mut statement = conn.prepare( &format!( - "{} WHERE workspace_id=?1 AND (state='pending' OR (state='failed' AND retryable=1)) ORDER BY updated_at ASC, operation_id ASC LIMIT ?2", + "{} WHERE workspace_id=?1 AND state='pending' ORDER BY updated_at ASC, operation_id ASC LIMIT ?2", operation_select_sql() ), )?; @@ -1007,8 +1039,13 @@ mod tests { #[tokio::test] async fn exact_replay_reuses_operation_and_conflicting_intent_fails() { let (store, workdir) = seeded_store().await; - let intent = - workdir_removal_intent(&workdir, "worker:W-1", "remove stale Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "worker:W-1", + "remove stale Workdir", + "test-removal-request", + ) + .unwrap(); let first = store.reserve_workdir_removal_operation(&intent).unwrap(); let replay = store.reserve_workdir_removal_operation(&intent).unwrap(); assert_eq!(replay, first); @@ -1023,10 +1060,15 @@ mod tests { } #[tokio::test] - async fn failed_attempt_is_retryable_and_completed_retry_replays() { + async fn failed_request_is_terminal_and_new_request_has_independent_result() { let (store, workdir) = seeded_store().await; - let intent = - workdir_removal_intent(&workdir, "workspace-api", "remove clean Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "test-removal-request", + ) + .unwrap(); let reserved = store.reserve_workdir_removal_operation(&intent).unwrap(); let first = store .begin_workdir_removal_attempt( @@ -1049,7 +1091,32 @@ mod tests { attempt_owner(), ) .unwrap(); - assert_ne!(retry.attempt_id, first.attempt_id); + assert_eq!(retry, failed); + assert!( + store + .recoverable_workdir_removal_operations("workspace-a", 100) + .unwrap() + .is_empty() + ); + let next_intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "new-request", + ) + .unwrap(); + let next = store + .reserve_workdir_removal_operation(&next_intent) + .unwrap(); + let retry = store + .begin_workdir_removal_attempt( + &next.workspace_id, + &next.operation_id, + &next.request_fingerprint, + attempt_owner(), + ) + .unwrap(); + assert_ne!(retry.operation_id, failed.operation_id); let completed = store.commit_workdir_removal_removed(&retry).unwrap(); assert_eq!( completed.disposition, @@ -1075,7 +1142,13 @@ mod tests { #[tokio::test] async fn recovery_and_completion_are_fenced_by_attempt_identity() { let (store, workdir) = seeded_store().await; - let intent = workdir_removal_intent(&workdir, "workspace-api", "remove Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove Workdir", + "test-removal-request", + ) + .unwrap(); let reserved = store.reserve_workdir_removal_operation(&intent).unwrap(); let first = store .begin_workdir_removal_attempt( @@ -1130,8 +1203,13 @@ mod tests { use std::sync::{Arc, Barrier}; let (store, workdir) = seeded_store().await; - let intent = - workdir_removal_intent(&workdir, "workspace-api", "remove clean Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "test-removal-request", + ) + .unwrap(); let operation = store.reserve_workdir_removal_operation(&intent).unwrap(); let barrier = Arc::new(Barrier::new(3)); let provider_calls = Arc::new(AtomicUsize::new(0)); @@ -1218,8 +1296,13 @@ mod tests { "2", ) .unwrap(); - let intent = - workdir_removal_intent(&workdir, "workspace-api", "remove clean Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "test-removal-request", + ) + .unwrap(); store.reserve_workdir_removal_operation(&intent).unwrap(); let error = store @@ -1236,8 +1319,13 @@ mod tests { #[tokio::test] async fn pending_removal_fences_new_attachment_and_retry_rereads_live_reservation() { let (store, workdir) = seeded_store().await; - let intent = - workdir_removal_intent(&workdir, "workspace-api", "remove clean Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "test-removal-request", + ) + .unwrap(); let pending = store.reserve_workdir_removal_operation(&intent).unwrap(); let error = store @@ -1251,11 +1339,22 @@ mod tests { store .reserve_worker_workdir_attachment("workspace-a", "workdir-a", "reservation-b", "3") .unwrap(); + let next_intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove clean Workdir", + "new-request", + ) + .unwrap(); + let next = store + .reserve_workdir_removal_operation(&next_intent) + .unwrap(); + assert_ne!(next.operation_id, failed.operation_id); let retry = store .begin_workdir_removal_attempt( - &failed.workspace_id, - &failed.operation_id, - &failed.request_fingerprint, + &next.workspace_id, + &next.operation_id, + &next.request_fingerprint, attempt_owner(), ) .unwrap(); @@ -1324,7 +1423,13 @@ mod tests { Ok(()) }) .unwrap(); - let intent = workdir_removal_intent(&workdir, "workspace-api", "remove Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "remove Workdir", + "test-removal-request", + ) + .unwrap(); let operation = store.reserve_workdir_removal_operation(&intent).unwrap(); let categories = |guards: Vec| { guards @@ -1447,8 +1552,13 @@ mod tests { #[tokio::test] async fn retained_completion_keeps_registry_and_is_auditable() { let (store, workdir) = seeded_store().await; - let intent = - workdir_removal_intent(&workdir, "workspace-api", "inspect dirty Workdir").unwrap(); + let intent = workdir_removal_intent( + &workdir, + "workspace-api", + "inspect dirty Workdir", + "test-removal-request", + ) + .unwrap(); let operation = store.reserve_workdir_removal_operation(&intent).unwrap(); let retained = store .complete_workdir_removal_retained( @@ -1477,10 +1587,12 @@ mod tests { attempt_owner(), ) .unwrap(); - assert_eq!(retry.state, WorkdirRemovalOperationState::Pending); - assert!(retry.attempt_id.is_some()); - assert_eq!(retry.disposition, None); - assert_eq!(retry.failure_category, None); - assert!(retry.retryable); + assert_eq!(retry, retained); + assert!( + store + .recoverable_workdir_removal_operations("workspace-a", 100) + .unwrap() + .is_empty() + ); } } diff --git a/openapi/server-api.json b/openapi/server-api.json index ac5c7b8ea..fb1437af1 100644 --- a/openapi/server-api.json +++ b/openapi/server-api.json @@ -16290,11 +16290,16 @@ "WorkingDirectoryRemovalRequest": { "additionalProperties": false, "properties": { + "operation_id": { + "description": "Stable for an exact resend; a new explicit request uses a new identity.", + "type": "string" + }, "reason": { "type": "string" } }, "required": [ + "operation_id", "reason" ], "type": "object" @@ -18302,7 +18307,7 @@ "info": { "title": "Yoi Server API", "version": "0.1.0", - "x-yoi-source-digest": "sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4" + "x-yoi-source-digest": "sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962" }, "openapi": "3.1.0", "paths": { diff --git a/web/workspace/src/lib/generated/companion-api.ts b/web/workspace/src/lib/generated/companion-api.ts index c886812bb..e9c1be932 100644 --- a/web/workspace/src/lib/generated/companion-api.ts +++ b/web/workspace/src/lib/generated/companion-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-companion-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: companion-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/companion-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_companion_api_types -- --check diff --git a/web/workspace/src/lib/generated/repository-api.ts b/web/workspace/src/lib/generated/repository-api.ts index 8cafc53b8..c97c6e513 100644 --- a/web/workspace/src/lib/generated/repository-api.ts +++ b/web/workspace/src/lib/generated/repository-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-repository-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: repository-operation-schema-closure,openapi-3.1,format:uint64=safe-number,strict-safe-integers,sorted-output // Output: web/workspace/src/lib/generated/repository-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_repository_openapi_types -- --check diff --git a/web/workspace/src/lib/generated/runtime-api.ts b/web/workspace/src/lib/generated/runtime-api.ts index c12b6c31e..9a9560295 100644 --- a/web/workspace/src/lib/generated/runtime-api.ts +++ b/web/workspace/src/lib/generated/runtime-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-runtime-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: runtime-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/runtime-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_runtime_api_types -- --check diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index b653b6d8a..74e10e252 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-ticket-browser-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: ticket-objective-merge-request-browser-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/ticket-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_ticket_api_types -- --check diff --git a/web/workspace/src/lib/generated/workdir-api.ts b/web/workspace/src/lib/generated/workdir-api.ts index 02dfac558..a100e4cd6 100644 --- a/web/workspace/src/lib/generated/workdir-api.ts +++ b/web/workspace/src/lib/generated/workdir-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-workdir-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: workdir-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/workdir-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_workdir_api_types -- --check @@ -33,7 +33,7 @@ export type WorkingDirectoryOccupancy = { display_name: string; linked_at: strin export type WorkingDirectoryRemovalDisposition = "removed" | "retained" | "attention_required"; -export type WorkingDirectoryRemovalRequest = { reason: string; }; +export type WorkingDirectoryRemovalRequest = { operation_id: string; reason: string; }; export type WorkingDirectoryRemovalResponse = { disposition: WorkingDirectoryRemovalDisposition; failure_category?: string | null; retryable: boolean; working_directory_id: string; }; diff --git a/web/workspace/src/lib/generated/worker-launch-api.ts b/web/workspace/src/lib/generated/worker-launch-api.ts index 19536c02c..01acaa036 100644 --- a/web/workspace/src/lib/generated/worker-launch-api.ts +++ b/web/workspace/src/lib/generated/worker-launch-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-worker-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:7e640cff6bdcf6dc298cfca695c5cb71a520288b20f4f29ec8df8138724b97c4) +// Input contract: 0.1.0 (sha256:3436eada52ceea48f4c50f40195cba02bec37fbbff4ea8f5a332133e6fcde962) // Options: worker-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/worker-launch-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_worker_launch_api_types -- --check diff --git a/web/workspace/src/lib/workspace/settings/workdir-removal.ts b/web/workspace/src/lib/workspace/settings/workdir-removal.ts index 4c7194e86..faace48b6 100644 --- a/web/workspace/src/lib/workspace/settings/workdir-removal.ts +++ b/web/workspace/src/lib/workspace/settings/workdir-removal.ts @@ -30,7 +30,9 @@ const causes: Readonly> = { provider_observation_unknown: "The provider could not confirm the remaining Workdir. Restore provider access so the next removal can recheck its current contents.", provider_cleanup_outcome_unknown: - "The provider has not confirmed deletion. Restore provider access and retry the same removal request to recheck the remaining target. The Workdir stays listed until deletion is confirmed.", + "The provider has not confirmed deletion. Restore provider access and reconcile the same request. Recovery only observes the target; it never repeats deletion.", + request_interrupted: + "The request was interrupted before execution. Submit a new request if removal is still wanted.", authority_invalid: "The removal authority could not be verified. Ask the Workspace administrator to verify the Workdir target and owning Runtime before requesting removal again.", provider_identity_changed: @@ -125,11 +127,39 @@ async function removalBody(response: Response): Promise { } } +// 要求IDは結果不明時だけ保持する。確定した失敗の次の明示要求は別IDとなる。 +export class WorkdirRemovalRequests { + private readonly unresolved = new Map(); + + async remove( + fetchFn: typeof fetch, + workspaceId: string, + runtimeId: string, + workdirId: string, + ): Promise { + const key = JSON.stringify([workspaceId, runtimeId, workdirId]); + const operationId = this.unresolved.get(key) ?? crypto.randomUUID(); + this.unresolved.set(key, operationId); + const result = await removeWorkdir( + fetchFn, + workspaceId, + runtimeId, + workdirId, + operationId, + ); + if (result.failure_category !== "provider_cleanup_outcome_unknown") { + this.unresolved.delete(key); + } + return result; + } +} + export async function removeWorkdir( fetchFn: typeof fetch, workspaceId: string, runtimeId: string, workdirId: string, + operationId: string, ): Promise { const response = await fetchFn( workspaceApiPath( @@ -141,8 +171,10 @@ export async function removeWorkdir( { method: "DELETE", headers: { "content-type": "application/json" }, - // An ordinary repeated request uses the existing durable Backend removal authority. - body: JSON.stringify({ reason: "User requested Workdir deletion" }), + body: JSON.stringify({ + operation_id: operationId, + reason: "User requested Workdir deletion", + }), }, ); if (!response.ok) { diff --git a/web/workspace/src/routes/w/[workspaceId]/settings/runtimes/[runtimeId]/workdirs/PageContent.svelte b/web/workspace/src/routes/w/[workspaceId]/settings/runtimes/[runtimeId]/workdirs/PageContent.svelte index eaf74d5f0..07c10039e 100644 --- a/web/workspace/src/routes/w/[workspaceId]/settings/runtimes/[runtimeId]/workdirs/PageContent.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/settings/runtimes/[runtimeId]/workdirs/PageContent.svelte @@ -2,7 +2,7 @@ import { loadJson, workspaceApiPath } from '#lib/workspace/api/http.ts'; import { parseRuntimeCleanupPlan } from '#lib/workspace/api/runtime-workers.ts'; import { parseWorkingDirectoryListResponse } from '#lib/workspace/api/workdirs.ts'; - import { canRemoveWorkdir, removalCause, removalGuard, removeWorkdir, WorkdirRemovalError } from '#lib/workspace/settings/workdir-removal.ts'; + import { canRemoveWorkdir, removalCause, removalGuard, WorkdirRemovalRequests, WorkdirRemovalError } from '#lib/workspace/settings/workdir-removal.ts'; import { formatCurrentWorkdirCheckout } from '#lib/workspace/settings/workdir-checkout.ts'; import type { CleanupWorkdirCandidate, RuntimeCleanupPlanResponse, WorkingDirectorySummary } from '#lib/workspace/sidebar/types.ts'; import type { PageData as RouteData } from './$types'; @@ -10,6 +10,7 @@ type PageProps = { data: ResolvedPageData }; let { data }: PageProps = $props(); + const removalRequests = new WorkdirRemovalRequests(); let cleanupBusyTarget = $state(null); let refreshing = $state(false); let refreshError = $state(null); @@ -72,7 +73,7 @@ cleanupBusyTarget = workdir.working_directory_id; feedback = null; try { - const result = await removeWorkdir(fetch, workspaceId, runtimeId, workdir.working_directory_id); + const result = await removalRequests.remove(fetch, workspaceId, runtimeId, workdir.working_directory_id); if (workspaceId !== data.workspaceId || runtimeId !== data.runtimeId) return; feedback = { id: workdir.working_directory_id, diff --git a/web/workspace/tests/workdir-removal.test.ts b/web/workspace/tests/workdir-removal.test.ts index 97dfe4aa0..dd3aa422c 100644 --- a/web/workspace/tests/workdir-removal.test.ts +++ b/web/workspace/tests/workdir-removal.test.ts @@ -6,6 +6,7 @@ import { canRemoveWorkdir, removalCause, removeWorkdir, + WorkdirRemovalRequests, } from "../src/lib/workspace/settings/workdir-removal.ts"; import type { CleanupWorkdirCandidate, @@ -57,6 +58,7 @@ Deno.test("removal wire boundary rejects malformed results and mismatched identi "workspace", "runtime", "dir", + "request-1", ); } catch { rejected = true; @@ -72,6 +74,7 @@ Deno.test("removal response size is bounded before decoding provider content", a "workspace", "runtime", "dir", + "request-1", ); } catch { rejected = true; @@ -179,7 +182,7 @@ Deno.test("pending unknown cleanup permits ordinary removal while current dirty ); }); -Deno.test("ordinary retries send the same scoped DELETE body and expose only allowlisted HTTP causes", async () => { +Deno.test("new requests after confirmed failure get distinct IDs and expose only allowlisted HTTP causes", async () => { const requests: { path: string; body: string; method: string }[] = []; const fetchFn = (async (path: RequestInfo | URL, init?: RequestInit) => { requests.push({ @@ -194,8 +197,9 @@ Deno.test("ordinary retries send the same scoped DELETE body and expose only all failure_category: "mount_present", }); }) as typeof fetch; + const removalRequests = new WorkdirRemovalRequests(); for (let i = 0; i < 2; i++) { - const result = await removeWorkdir( + const result = await removalRequests.remove( fetchFn, "workspace/x", "runtime/x", @@ -204,8 +208,9 @@ Deno.test("ordinary retries send the same scoped DELETE body and expose only all assert(result.retryable && result.failure_category === "mount_present"); } assert( - JSON.stringify(requests[0]) === JSON.stringify(requests[1]), - "retry changed ordinary deletion request", + JSON.parse(requests[0].body).operation_id !== + JSON.parse(requests[1].body).operation_id, + "new explicit request reused a failed request identity", ); assert( requests[0].path === @@ -213,8 +218,9 @@ Deno.test("ordinary retries send the same scoped DELETE body and expose only all ); assert( requests[0].method === "DELETE" && - requests[0].body === - JSON.stringify({ reason: "User requested Workdir deletion" }), + JSON.parse(requests[0].body).reason === + "User requested Workdir deletion" && + typeof JSON.parse(requests[0].body).operation_id === "string", ); for ( const [status, category] of [ @@ -236,6 +242,7 @@ Deno.test("ordinary retries send the same scoped DELETE body and expose only all "workspace", "runtime", "dir", + "request-1", ); } catch (error) { message = error instanceof Error ? error.message : String(error); @@ -246,3 +253,34 @@ Deno.test("ordinary retries send the same scoped DELETE body and expose only all ); } }); + +Deno.test("unknown removal outcomes keep the same identity and body until a terminal result", async () => { + for (const transportLoss of [false, true]) { + const calls: string[] = []; + const requester = new WorkdirRemovalRequests(); + const fetchFn = (async (_path: RequestInfo | URL, init?: RequestInit) => { + calls.push(String(init?.body)); + if (calls.length === 1 && transportLoss) { + throw new Error("connection lost"); + } + return Response.json({ + working_directory_id: "dir", + disposition: "attention_required", + retryable: calls.length !== 1, + failure_category: calls.length === 1 + ? "provider_cleanup_outcome_unknown" + : "resource_busy", + }); + }) as typeof fetch; + await requester.remove(fetchFn, "workspace", "runtime", "dir").catch(() => + undefined + ); + await requester.remove(fetchFn, "workspace", "runtime", "dir"); + await requester.remove(fetchFn, "workspace", "runtime", "dir"); + assert(calls[0] === calls[1], "unknown outcome changed request identity"); + assert( + calls[1] !== calls[2], + "confirmed failure kept old request identity", + ); + } +}); From e7351c16b0d023cf5c9289ea8d831b16cec2e250 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 30 Oct 2026 21:22:00 +0900 Subject: [PATCH 6/6] fix: reuse Workdir attachments independently of requested alias --- crates/server-api/src/lib.rs | 7 +- .../src/feature/builtin/manage_workdir.rs | 167 +++++++++++++++--- crates/workspace-server/src/lib.rs | 6 + crates/workspace-server/src/server.rs | 162 ++++++++++++++++- crates/workspace-server/src/store.rs | 105 ++++++----- 5 files changed, 370 insertions(+), 77 deletions(-) diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index 17dba8b22..aec0124a8 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -7604,6 +7604,9 @@ pub struct CurrentWorkerWorkdirAttachmentResponse { #[serde(skip_serializing_if = "Option::is_none")] pub environment: Option, pub attached: bool, + /// The Workdir was already attached to this Worker; alias is its existing name. + #[serde(default)] + pub already_attached: bool, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] @@ -13384,9 +13387,11 @@ mod tests { capabilities: workdir::WorkdirSessionCapabilities::EMPTY, environment: None, attached: false, + already_attached: false, }; let value = serde_json::to_value(&response).unwrap(); - assert_eq!(value.as_object().unwrap().len(), 5); + assert_eq!(value.as_object().unwrap().len(), 6); + assert_eq!(value["already_attached"], false); assert!(value.get("connection_id").is_none()); assert_eq!( serde_json::from_value::(value).unwrap(), diff --git a/crates/worker/src/feature/builtin/manage_workdir.rs b/crates/worker/src/feature/builtin/manage_workdir.rs index a197867e2..66a9d5601 100644 --- a/crates/worker/src/feature/builtin/manage_workdir.rs +++ b/crates/worker/src/feature/builtin/manage_workdir.rs @@ -702,31 +702,46 @@ impl WorkspaceHttpWorkdirBackend { ATTACH_TOOL, "working_directory_id", )?; - let alias_key = workdir::WorkdirAttachmentAlias::new(alias) - .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; - if self.session_router.session(&alias_key).is_some() { - return Err(ToolError::InvalidArgument(format!( - "attachment alias `{alias}` already exists" - ))); - } let response = self.attach_response(alias, working_directory_id)?; - if let Err(error) = self.session_router.attach( - alias_key.clone(), + if !response.attachment.attached + || response.attachment.working_directory_id != working_directory_id + { + return Err(ToolError::ExecutionFailed( + "Workdir attachment response does not match the target".into(), + )); + } + // Backend ownership decides the name. A requested alias cannot rename + // an existing attachment or create a second route to the same Workdir. + let attached_alias = &response.attachment.alias; + let alias_key = workdir::WorkdirAttachmentAlias::new(attached_alias.clone()) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + if let Some(session) = self.session_router.session(&alias_key) { + if session.workdir().id().as_str() != working_directory_id { + return Err(ToolError::ExecutionFailed("Local Workdir attachment differs from Backend; refresh attachments before retrying".into())); + } + } else if let Err(error) = self.session_router.attach( + alias_key, WorkspaceAttachedWorkdirSession::handle_for_workdir_with_environment( self.client.clone(), - alias, + attached_alias, working_directory_id, response.attachment.capabilities, response.environment.clone(), ), ) { - let _ = self.detach(alias); + if !response.attachment.already_attached { + let _ = self.detach(attached_alias); + } return Err(ToolError::ExecutionFailed(format!( "attach Workdir session route: {error}" ))); } workdir_output( - format!("Attached Workdir {working_directory_id} as {alias}"), + if response.attachment.already_attached { + format!("Workdir {working_directory_id} is already attached as {attached_alias}") + } else { + format!("Attached Workdir {working_directory_id} as {attached_alias}") + }, &response, ) } @@ -737,15 +752,31 @@ impl WorkspaceHttpWorkdirBackend { working_directory_id: &str, ) -> Result { let workspace_id = encode_path_segment(self.workspace_id()?); - self.execute_json::(WorkspaceRequest::json( - WorkspaceRequestMethod::Post, - format!("/api/w/{workspace_id}/workers/self/workdir-attachments"), - serde_json::to_string(&WorkdirAttachRequest { - alias: alias.to_string(), - working_directory_id: working_directory_id.to_string(), - }) - .map_err(decode_error)?, - )) + let response = self + .client + .execute(WorkspaceRequest::json( + WorkspaceRequestMethod::Post, + format!("/api/w/{workspace_id}/workers/self/workdir-attachments"), + serde_json::to_string(&WorkdirAttachRequest { + alias: alias.to_string(), + working_directory_id: working_directory_id.to_string(), + }) + .map_err(decode_error)?, + )) + .map_err(|error| { + ToolError::ExecutionFailed(format!("Workspace Workdir request failed: {error}")) + })?; + if response.status == 409 + && let Ok(error) = + serde_json::from_str::(&response.body) + && error.error == "workdir_occupied" + { + return Err(ToolError::StructuredConflict { + code: "workdir_occupied".into(), + message: "Workdir is occupied by another Worker".into(), + }); + } + decode_response(response) } fn detach(&self, alias: &str) -> Result { @@ -1231,6 +1262,8 @@ struct WorkdirAttachmentResponse { working_directory_id: String, capabilities: WorkdirSessionCapabilities, attached: bool, + #[serde(default)] + already_attached: bool, } #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -1701,6 +1734,98 @@ mod tests { ); } + #[test] + fn attach_reports_other_worker_occupancy_without_exposing_backend_details() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![WorkspaceResponse { + status: 409, + body: serde_json::to_string(&server_api::RepositoryApiError::new( + 409, + "workdir_occupied", + "private backend detail", + Vec::new(), + )) + .unwrap(), + }])); + let backend = WorkspaceHttpWorkdirBackend::new(client.clone()); + let error = backend + .attach(WorkdirAttachInput { + alias: "work".into(), + working_directory_id: "occupied".into(), + }) + .unwrap_err(); + assert!( + matches!(error, ToolError::StructuredConflict { code, message } if code == "workdir_occupied" && message == "Workdir is occupied by another Worker") + ); + assert_eq!(client.requests().len(), 1); + } + + #[test] + fn attach_reuses_backend_alias_without_duplicate_routes_or_detach() { + let client = Arc::new(RecordingWorkspaceClient::new((0..3).map(|_| response(json!({ + "workspace_id": "workspace/test", "alias": "existing", "working_directory_id": "wd-existing", + "capabilities": {"bits": 63}, "attached": true, "already_attached": true, + "environment": workdir::WorkdirSessionEnvironment::root(WorkdirSessionCapabilities::ALL) + }))).collect())); + let backend = WorkspaceHttpWorkdirBackend::new(client.clone()); + let occupied_alias = workdir::WorkdirAttachmentAlias::new("other").unwrap(); + backend + .session_router + .attach( + occupied_alias.clone(), + WorkspaceAttachedWorkdirSession::handle_for_workdir( + client.clone(), + "other", + "wd-other", + ), + ) + .unwrap(); + for alias in ["new-name", "existing", "other"] { + let output = backend + .attach(WorkdirAttachInput { + alias: alias.into(), + working_directory_id: "wd-existing".into(), + }) + .unwrap(); + assert_eq!( + output.summary, + "Workdir wd-existing is already attached as existing" + ); + let body: serde_json::Value = + serde_json::from_str(output.content.as_deref().unwrap()).unwrap(); + assert_eq!(body["alias"], "existing"); + assert_eq!(body["already_attached"], true); + } + assert!( + backend + .session_router + .session(&workdir::WorkdirAttachmentAlias::new("new-name").unwrap()) + .is_none() + ); + assert_eq!( + backend + .session_router + .session(&occupied_alias) + .unwrap() + .workdir() + .id() + .as_str(), + "wd-other" + ); + assert!( + backend + .session_router + .session(&workdir::WorkdirAttachmentAlias::new("existing").unwrap()) + .is_some() + ); + assert_eq!(client.requests().len(), 3); + assert!( + client + .requests() + .iter() + .all(|request| request.method == WorkspaceRequestMethod::Post) + ); + } + #[tokio::test] async fn attached_session_proxies_operations_without_runtime_or_session_arguments() { let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({ diff --git a/crates/workspace-server/src/lib.rs b/crates/workspace-server/src/lib.rs index c4a95e8c0..c5e72d17d 100644 --- a/crates/workspace-server/src/lib.rs +++ b/crates/workspace-server/src/lib.rs @@ -134,6 +134,12 @@ pub enum Error { TicketAssignmentConflict(String), #[error("Workdir attachment conflict: {0}")] WorkdirAttachmentConflict(String), + #[error("Workdir {workdir_id} is already attached to Worker {runtime_id}:{worker_id}")] + WorkdirOccupied { + workdir_id: String, + runtime_id: String, + worker_id: String, + }, #[error("Workspace permission denied: {0}")] WorkspacePermissionDenied(String), #[error("Workspace config update conflict: {0}")] diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index dce299e60..1d55a3735 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -21001,15 +21001,17 @@ async fn scoped_attach_current_worker_workdir( run_worker_workdir_db_stage_locked(&api, &_session_guard, move |api| -> ApiResult<_> { let existing = api .store - .list_worker_workdir_links(&api.config.workspace_id, &stage_worker)? + .list_workdir_worker_links(&api.config.workspace_id, &stage_workdir_id)? .into_iter() - .find(|link| link.unlinked_at.is_none() && link.alias == stage_alias); + .next(); if let Some(existing) = existing { - if existing.workdir_id != stage_workdir_id { - return Err(Error::WorkdirAttachmentConflict(format!( - "Worker {}:{} attachment alias `{stage_alias}` is already bound to Workdir {}", - stage_worker.runtime_id, stage_worker.worker_id, existing.workdir_id - )).into()); + if existing.worker != stage_worker { + return Err(Error::WorkdirOccupied { + workdir_id: stage_workdir_id, + runtime_id: existing.worker.runtime_id, + worker_id: existing.worker.worker_id, + } + .into()); } let effective_capabilities = existing.capabilities.intersection(capabilities); if effective_capabilities != existing.capabilities { @@ -21019,6 +21021,21 @@ async fn scoped_attach_current_worker_workdir( } Ok(Some((existing, effective_capabilities))) } else { + if let Some(existing) = api + .store + .list_worker_workdir_links(&api.config.workspace_id, &stage_worker)? + .into_iter() + .find(|link| link.unlinked_at.is_none() && link.alias == stage_alias) + { + return Err(Error::WorkdirAttachmentConflict(format!( + "Worker {}:{} attachment alias '{}' is already bound to Workdir {}", + stage_worker.runtime_id, + stage_worker.worker_id, + stage_alias, + existing.workdir_id + )) + .into()); + } Ok(None) } }) @@ -21031,7 +21048,7 @@ async fn scoped_attach_current_worker_workdir( .map_err(ApiError::from)?; return Ok(Json(server_api::CurrentWorkerWorkdirAttachmentResponse { workspace_id: api.config.workspace_id.clone(), - alias: alias.to_string(), + alias: link.alias.clone(), working_directory_id: workdir_id.to_string(), capabilities: effective_capabilities, environment: Some( @@ -21046,6 +21063,7 @@ async fn scoped_attach_current_worker_workdir( .await?, ), attached: true, + already_attached: true, })); } close_current_worker_attachment_session_locked(&api, &worker).await?; @@ -21107,6 +21125,7 @@ async fn scoped_attach_current_worker_workdir( capabilities, environment: Some(environment), attached: true, + already_attached: false, })) } @@ -21350,6 +21369,7 @@ async fn detach_current_worker_workdir( capabilities: workdir::WorkdirSessionCapabilities::EMPTY, environment: None, attached: false, + already_attached: false, }) } @@ -37665,6 +37685,7 @@ impl ApiError { status.as_u16(), match self.error { Error::RestoreObservationConflict => "restore_observation_conflict", + Error::WorkdirOccupied { .. } => "workdir_occupied", Error::RuntimeBindingIdConflict { .. } => "stale_binding", Error::RuntimeBindingFingerprintConflict { .. } => "fingerprint_in_use", _ => status.canonical_reason().unwrap_or("error"), @@ -37688,6 +37709,7 @@ fn api_error_status(error: &Error) -> StatusCode { } Error::TicketAssignmentConflict(_) | Error::WorkdirAttachmentConflict(_) + | Error::WorkdirOccupied { .. } | Error::WorkspaceConfigConflict(_) | Error::RuntimeBindingConflict(_) | Error::RuntimeBindingIdConflict { .. } @@ -61386,6 +61408,130 @@ mod tests { ); } + #[tokio::test] + async fn current_worker_attach_uses_workdir_ownership_before_requested_alias() { + 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; + let before = api + .store + .list_worker_workdir_links(TEST_WORKSPACE_ID, &fixture.worker) + .unwrap(); + let path = format!("/api/w/{TEST_WORKSPACE_ID}/workers/self/workdir-attachments"); + for alias in ["docs", "different-name", "checkout"] { + let response = build_router(api.clone()) + .oneshot(runtime_source_request( + &identity, + Some(&fixture.worker.worker_id), + "POST", + &path, + serde_json::to_vec( + &json!({"alias": alias, "working_directory_id": fixture.docs_workdir_id}), + ) + .unwrap(), + )) + .await + .unwrap(); + let status = response.status(); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!(status, StatusCode::OK, "{}", String::from_utf8_lossy(&body)); + let response: server_api::CurrentWorkerWorkdirAttachmentResponse = + serde_json::from_slice(&body).unwrap(); + assert!(response.attached && response.already_attached); + assert_eq!(response.alias, "docs"); + assert_eq!(response.working_directory_id, fixture.docs_workdir_id); + assert!(response.environment.is_some()); + assert_eq!( + api.store + .list_worker_workdir_links(TEST_WORKSPACE_ID, &fixture.worker) + .unwrap(), + before + ); + } + api.store + .detach_worker_workdir( + TEST_WORKSPACE_ID, + &fixture.worker, + Some(&fixture.docs_workdir_id), + "test detach", + ) + .unwrap(); + sync_runtime_worker_workdir_attachments(api, &fixture.worker).unwrap(); + for (alias, expected) in [ + ("checkout", StatusCode::CONFLICT), + ("new-docs", StatusCode::OK), + ] { + let response = build_router(api.clone()) + .oneshot(runtime_source_request( + &identity, + Some(&fixture.worker.worker_id), + "POST", + &path, + serde_json::to_vec( + &json!({"alias": alias, "working_directory_id": fixture.docs_workdir_id}), + ) + .unwrap(), + )) + .await + .unwrap(); + let status = response.status(); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!(status, expected, "{}", String::from_utf8_lossy(&body)); + if status == StatusCode::OK { + let response: server_api::CurrentWorkerWorkdirAttachmentResponse = + serde_json::from_slice(&body).unwrap(); + assert!(response.attached); + assert!(!response.already_attached); + assert_eq!(response.alias, "new-docs"); + } + } + let mut other = api + .store + .get_worker_registry(TEST_WORKSPACE_ID, &fixture.worker) + .unwrap() + .unwrap(); + other.worker.worker_id = "another-owner".into(); + api.store.upsert_worker_registry(&other).unwrap(); + let mut occupied = api + .store + .list_workdir_worker_links(TEST_WORKSPACE_ID, &fixture.docs_workdir_id) + .unwrap() + .pop() + .unwrap(); + api.store + .detach_worker_workdir( + TEST_WORKSPACE_ID, + &fixture.worker, + Some(&fixture.docs_workdir_id), + "transfer fixture", + ) + .unwrap(); + sync_runtime_worker_workdir_attachments(api, &fixture.worker).unwrap(); + occupied.worker = other.worker; + api.store.attach_worker_workdir(&occupied).unwrap(); + let response = build_router(api.clone()) + .oneshot(runtime_source_request( + &identity, + Some(&fixture.worker.worker_id), + "POST", + &path, + serde_json::to_vec( + &json!({"alias": "checkout", "working_directory_id": fixture.docs_workdir_id}), + ) + .unwrap(), + )) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::CONFLICT); + let error: server_api::RepositoryApiError = + serde_json::from_slice(&to_bytes(response.into_body(), usize::MAX).await.unwrap()) + .unwrap(); + assert_eq!(error.error, "workdir_occupied"); + } + #[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 85e1f24cd..68860e422 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -11119,49 +11119,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore { record.workdir_id ))); } - let active_for_alias = tx - .query_row( - r#"SELECT workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, linked_at, unlinked_at, connection_id - FROM worker_workdir_links - WHERE workspace_id = ?1 AND runtime_id = ?2 AND worker_id = ?3 - AND alias = ?4 AND unlinked_at IS NULL"#, - params![ - record.workspace_id, - record.worker.runtime_id, - record.worker.worker_id, - record.alias, - ], - read_worker_workdir_link_record, - ) - .optional()?; - if let Some(mut active) = active_for_alias { - if active.workdir_id == record.workdir_id { - let capabilities = active.capabilities.intersection(record.capabilities); - if capabilities != active.capabilities { - tx.execute( - r#"UPDATE worker_workdir_links - SET capabilities = ?1 - WHERE workspace_id = ?2 AND runtime_id = ?3 AND worker_id = ?4 - AND workdir_id = ?5 AND alias = ?6 AND unlinked_at IS NULL"#, - params![ - encode_workdir_link_capabilities(capabilities)?, - record.workspace_id, - record.worker.runtime_id, - record.worker.worker_id, - record.workdir_id, - record.alias, - ], - )?; - active.capabilities = capabilities; - } - tx.commit()?; - return Ok(active); - } - return Err(Error::WorkdirAttachmentConflict(format!( - "Worker {}:{} attachment alias `{}` is already bound to Workdir {}", - record.worker.runtime_id, record.worker.worker_id, record.alias, active.workdir_id - ))); - } + // Ownership is keyed by Workdir, not by the requested local name. let active_for_workdir = tx .query_row( r#"SELECT workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, linked_at, unlinked_at, connection_id @@ -11171,10 +11129,45 @@ impl ControlPlaneStore for SqliteWorkspaceStore { read_worker_workdir_link_record, ) .optional()?; - if let Some(active) = active_for_workdir { + if let Some(mut active) = active_for_workdir { + if active.worker != record.worker { + return Err(Error::WorkdirOccupied { + workdir_id: record.workdir_id.clone(), + runtime_id: active.worker.runtime_id, + worker_id: active.worker.worker_id, + }); + } + let capabilities = active.capabilities.intersection(record.capabilities); + if capabilities != active.capabilities { + tx.execute( + r#"UPDATE worker_workdir_links SET capabilities = ?1 + WHERE workspace_id = ?2 AND runtime_id = ?3 AND worker_id = ?4 + AND workdir_id = ?5 AND alias = ?6 AND unlinked_at IS NULL"#, + params![ + encode_workdir_link_capabilities(capabilities)?, + active.workspace_id, active.worker.runtime_id, active.worker.worker_id, + active.workdir_id, active.alias, + ], + )?; + active.capabilities = capabilities; + } + tx.commit()?; + return Ok(active); + } + let active_for_alias = tx + .query_row( + r#"SELECT workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, linked_at, unlinked_at, connection_id + FROM worker_workdir_links + WHERE workspace_id = ?1 AND runtime_id = ?2 AND worker_id = ?3 + AND alias = ?4 AND unlinked_at IS NULL"#, + params![record.workspace_id, record.worker.runtime_id, record.worker.worker_id, record.alias], + read_worker_workdir_link_record, + ) + .optional()?; + if let Some(active) = active_for_alias { return Err(Error::WorkdirAttachmentConflict(format!( - "Workdir {} is already attached to Worker {}:{}", - record.workdir_id, active.worker.runtime_id, active.worker.worker_id + "Worker {}:{} attachment alias `{}` is already bound to Workdir {}", + record.worker.runtime_id, record.worker.worker_id, record.alias, active.workdir_id ))); } // Direct attachments (including ordinary recovery/reconnection) @@ -25496,6 +25489,24 @@ INSERT INTO worker_registry ( 2 ); + for alias in ["another-name", "docs"] { + let requested = WorkerWorkdirLinkRecord { + alias: alias.into(), + ..link.clone() + }; + assert_eq!( + store.attach_worker_workdir(&requested).unwrap(), + downgraded_link + ); + assert_eq!( + store + .list_worker_workdir_links("local-dev", &worker.worker) + .unwrap() + .len(), + 2 + ); + } + let second_worker = WorkerRegistryRecord { worker: RuntimeWorkerRef::new("embedded", "2"), display_name: "Browser 2".to_string(), @@ -25511,7 +25522,7 @@ INSERT INTO worker_registry ( }; assert!(matches!( store.attach_worker_workdir(&workdir_conflict), - Err(Error::WorkdirAttachmentConflict(_)) + Err(Error::WorkdirOccupied { .. }) )); assert!(matches!( store.detach_worker_workdir("local-dev", &worker.worker, Some("wrong-workdir"), "6",),