Keep every workspace under Petri's retention and say why

Petri's retention decides whether a released workspace is kept or removed.
Fabro's lifecycle settings decide whether a sandbox keeps running after
the run and whether a delete may remove it; none asks for removal at the
run's end, and the sandbox tab, `fabro cp`, the run's delete and the
sandbox scenarios read the container after the run. So the mapping is
`Retention::Always` for every setting, named once as `engine::RETENTION`
with the reasoning, instead of a per-setting function that released a
finished sandbox.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

lib/apps/fabro-cli/src/commands/run/petri_worker.rs

@@ -205,7 +205,6 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> {
             .environment
             .provider
             .clone(),
-        retention: engine::retention(&worker.run_state.spec.settings.run.environment),
         cancel: cancel_token.clone(),
         controls: controls.clone(),
         interviewer: Arc::new(petri_interviewer),

lib/apps/fabro-server/src/server/petri_runs.rs

@@ -318,7 +318,6 @@ pub(crate) async fn execute(state: Arc<AppState>, run_id: RunId) {
             .observe_store(Arc::new(SqliteRunStore::new(state.db_pool.clone()))),
         runtime: runtime_spec(&state, &eligible, dry_run),
         provider: run_state.spec.settings.run.environment.provider.clone(),
-        retention: engine::retention(&run_state.spec.settings.run.environment),
         cancel,
         // The in-process test path drives no pause or steer: the server's
         // transports for those name the worker.

lib/components/fabro-petri/README.md

@@ -133,12 +133,15 @@ notifications and pairings (recorded, not shown).
 
 ### Retention
 
-`engine::retention` maps the run's environment settings onto Petri's
-workspace retention: `preserve = true` or `stop_on_terminal = false` keeps
-every workspace (`Retention::Always`), as does the local provider, whose
-host workspaces live under the run's scratch directory and go with it;
-otherwise a failed scope's workspace is kept for debugging and a successful
-one is released (`Retention::OnFailure`, Petri's default).
+Petri's retention decides, at a scope's release, whether its workspace is
+kept or removed. Fabro's environment lifecycle settings decide something
+else: `stop_on_terminal` whether a sandbox keeps running after the run,
+`preserve` whether the run's delete may remove it. Neither asks for a
+sandbox to be removed when the run ends (the legacy executor stopped a
+container and left it for the sandbox tab, `fabro cp`, the delete and
+`fabro system prune`; a host workspace goes with the run's scratch
+directory), so `engine::RETENTION` maps every setting to
+`Retention::Always`, and no Fabro setting names `OnFailure` or `Never`.
 
 Every run executes on Petri. The server side is `fabro-server`'s
 `server::petri_runs`; the worker side is `fabro-cli`'s

lib/components/fabro-petri/src/engine.rs

@@ -23,10 +23,9 @@
 //! hook service: the checkpoint commit before every durable finish and its
 //! platform record after every route, with a failed commit ending the run
 //! as a `checkpoint_failed` failure. What the standalone runner's defaults
-//! give the run: Petri's local hook service for `[[run.hooks]]` and no host
-//! tools. The workspaces' retention comes from the run's environment
-//! settings through [`retention`]. Cancellation rides the caller's token:
-//! when it fires, the root
+//! give the run: Petri's local hook service for `[[run.hooks]]`, no host
+//! tools, and `Retention::Always` for every workspace (see [`RETENTION`]).
+//! Cancellation rides the caller's token: when it fires, the root
 //! invocation is cancelled politely and Petri records why. The run's other
 //! controls (pause, unpause, steer) are the caller's [`RunControls`]: its
 //! pause gate is installed over the run's hooks, it observes the run, and
@@ -48,7 +47,6 @@
 use std::path::PathBuf;
 use std::sync::Arc;
 
-use fabro_types::settings::run::RunEnvironmentSettings;
 use fabro_types::{FailureReason, RunId, SandboxProviderKind};
 use petri_execution::host::{self, HostError, HostRun};
 use petri_execution::inspect::{self, InspectError, RunInspection};
@@ -96,8 +94,6 @@ pub struct RunRequest {
     pub runtime:     RuntimeSpec,
     /// The sandbox provider Fabro resolved for the run's environment.
     pub provider:    SandboxProviderKind,
-    /// When the run's workspaces are kept after their scope is released.
-    pub retention:   Retention,
     /// Fires to cancel the run.
     pub cancel:      CancellationToken,
     /// The run's pause, unpause and steer controls, which the caller keeps
@@ -178,7 +174,7 @@ pub async fn run(request: RunRequest) -> Result<RunOutcome, RunError> {
     let key = RunKey::new(request.run_id.as_str());
     let mut options = RunOptions::new(&request.run_dir);
     options.run_key = Some(key.clone());
-    options.retention = request.retention;
+    options.retention = RETENTION;
     options.sandbox.backend = backend;
     // Fabro's hooks restore a sandbox workspace from its snapshots at the
     // scope's acquisition, so a lease whose sandbox is gone gets a fresh
@@ -285,30 +281,20 @@ pub async fn run(request: RunRequest) -> Result<RunOutcome, RunError> {
     Ok(outcome)
 }
 
-/// When Petri keeps a run's workspaces after their scope is released, from
-/// the run's environment settings:
+/// When Petri keeps a run's workspaces after their scope is released.
 ///
-/// - `[environments.<id>.lifecycle] preserve = true` asks for the sandbox to
-///   stay after the run, so every workspace is kept (`Retention::Always`).
-/// - The local provider keeps every workspace too: a host workspace lives under
-///   the run's own scratch directory, which `fabro system prune` removes with
-///   the run, and the legacy executor never removed it on its own.
-/// - `stop_on_terminal = false` asks for the sandbox to outlive the run, so its
-///   workspaces are kept (`Retention::Always`).
-/// - Otherwise the sandbox is released with the run and Petri's default
-///   applies: a failed scope's workspace is kept for debugging, a successful
-///   one is not (`Retention::OnFailure`).
-#[must_use]
-pub fn retention(environment: &RunEnvironmentSettings) -> Retention {
-    let keep = environment.lifecycle.preserve
-        || !environment.lifecycle.stop_on_terminal
-        || environment.provider == SandboxProviderKind::LOCAL;
-    if keep {
-        Retention::Always
-    } else {
-        Retention::OnFailure
-    }
-}
+/// Petri's retention makes one choice at release: keep the workspace (a
+/// host directory, a container, a remote sandbox) or remove it. Fabro's
+/// environment lifecycle settings make different choices: `stop_on_terminal`
+/// says whether a sandbox keeps running after the run, and `preserve` says
+/// whether deleting the run may remove it. Neither asks for a sandbox to be
+/// removed when the run ends: the legacy executor stopped a container at
+/// the end and left it for the sandbox tab, `fabro cp`, the run's delete
+/// and `fabro system prune`, and a host workspace lives under the run's
+/// scratch directory, which goes with the run. So every setting maps to
+/// `Retention::Always`, and `Retention::OnFailure` and `Retention::Never`
+/// have no Fabro setting that names them.
+pub const RETENTION: Retention = Retention::Always;
 
 /// The Fabro run id the run key names. A key that is not one (a test's
 /// bare key) still gets hooks, under a fresh id for its platform records.
@@ -491,41 +477,8 @@ async fn write_receipt(run_dir: &std::path::Path, receipt: &petri_execution::Int
 
 #[cfg(test)]
 mod tests {
-    use fabro_types::settings::run::EnvironmentLifecycleSettings;
-
     use super::*;
 
-    fn environment(provider: SandboxProviderKind) -> RunEnvironmentSettings {
-        let mut environment = RunEnvironmentSettings::from_environment(
-            "test".to_string(),
-            fabro_types::settings::run::EnvironmentSettings::default(),
-        );
-        environment.provider = provider;
-        environment
-    }
-
-    #[test]
-    fn retention_follows_the_environment_lifecycle() {
-        let mut docker = environment(SandboxProviderKind::DOCKER);
-        assert_eq!(retention(&docker), Retention::OnFailure);
-        docker.lifecycle = EnvironmentLifecycleSettings {
-            preserve:         true,
-            stop_on_terminal: true,
-            auto_stop:        None,
-        };
-        assert_eq!(retention(&docker), Retention::Always);
-        docker.lifecycle = EnvironmentLifecycleSettings {
-            preserve:         false,
-            stop_on_terminal: false,
-            auto_stop:        None,
-        };
-        assert_eq!(retention(&docker), Retention::Always);
-        assert_eq!(
-            retention(&environment(SandboxProviderKind::LOCAL)),
-            Retention::Always
-        );
-    }
-
     fn outcome_with(status: RunStatus, failure: Option<&str>, complete: bool) -> RunOutcome {
         RunOutcome {
             status,

lib/components/fabro-petri/src/hooks.rs

@@ -36,8 +36,8 @@
 //!   with the sandbox in place. Fabro's own end-of-run work (the terminal
 //!   lifecycle event, notifications on it) is the run lifecycle path's, on the
 //!   worker's and server's side of the engine, and the workspace's retention is
-//!   Petri's, mapped from the run's environment settings by
-//!   [`engine::retention`](crate::engine::retention).
+//!   Petri's, `Retention::Always` for every Fabro setting
+//!   ([`engine::RETENTION`](crate::engine::RETENTION)).
 //!
 //! # Operation identities
 //!
@@ -965,14 +965,12 @@ impl FabroHooks {
             .get_or_try_init(|| self.load_recorded())
             .await?;
         let branch = match self.branch.get() {
-            Some(branch) => branch.clone(),
-            None => match self.stored_branch().await? {
-                Some(branch) => branch,
-                None => {
-                    debug!(run_id = %self.run_id, "no run branch is recorded; no run diff");
-                    return Ok(());
-                }
-            },
+            Some(branch) => Some(branch.clone()),
+            None => self.stored_branch().await?,
+        };
+        let Some(branch) = branch else {
+            debug!(run_id = %self.run_id, "no run branch is recorded; no run diff");
+            return Ok(());
         };
         let Some(base_sha) = branch.base_sha.clone() else {
             return Ok(());

lib/components/fabro-petri/tests/hooks.rs

@@ -26,7 +26,7 @@ use fabro_petri::blobs::Blobs;
 use fabro_petri::check::{self, Bundle, CheckRequest, Launch};
 use fabro_petri::checkpoint::{CHECKPOINT_FAILED_CLASS, CheckpointKey, RunWorkspaces};
 use fabro_petri::controls::RunControls;
-use fabro_petri::engine::{self, Execution, Retention, RunRequest, RunStatus};
+use fabro_petri::engine::{self, Execution, RunRequest, RunStatus};
 use fabro_petri::hooks::HooksSpec;
 use fabro_petri::platform_records::PlatformRecords;
 use fabro_petri::recovery::{self, Recovery, RecoveryRequest};
@@ -199,7 +199,6 @@ impl Harness {
             store: Arc::clone(&self.store) as Arc<dyn petri_store::RunStore>,
             runtime: RuntimeSpec::default(),
             provider,
-            retention: Retention::Always,
             cancel: CancellationToken::new(),
             controls: RunControls::new(),
             interviewer,
@@ -816,7 +815,6 @@ async fn a_run_hook_blocks_a_tool_effect_through_the_forwarded_service() {
             ..RuntimeSpec::default()
         },
         provider: SandboxProviderKind::LOCAL,
-        retention: Retention::Always,
         cancel: CancellationToken::new(),
         controls: RunControls::new(),
         interviewer,

lib/components/fabro-petri/tests/support/mod.rs

@@ -16,7 +16,7 @@ use std::time::{Duration, Instant};
 use fabro_petri::admission::AdmittedGraphs;
 use fabro_petri::check::{self, Bundle, CheckRequest, Launch};
 use fabro_petri::controls::RunControls;
-use fabro_petri::engine::{Execution, Retention, RunRequest};
+use fabro_petri::engine::{Execution, RunRequest};
 use fabro_petri::interview::{Approval, FabroInterviewer, QuestionNotice, QuestionSink};
 use fabro_petri::runtime::RuntimeSpec;
 use fabro_types::SandboxProviderKind;
@@ -115,7 +115,6 @@ pub(crate) fn run_request(
         store,
         runtime,
         provider: SandboxProviderKind::LOCAL,
-        retention: Retention::Always,
         cancel: CancellationToken::new(),
         controls: RunControls::new(),
         observers: vec![interviewer.observer()],