Require finalization for every Fabro run

FabroHooks always declares required run finalization instead of only
when the run publishes or checkpoints. Every run takes one path, the
best-effort diff branch in run_finished goes away, and a fork always
declares what its worker's hooks will declare on resume, so the fork no
longer carries its source's flag across.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/internal/run-finalization.md

@@ -1,8 +1,8 @@
 # Required run finalization
 
-Petri owns the durable run result. Fabro declares required finalization when
-its hooks checkpoint or have a publisher, and implements it in
-`FabroHooks::finalize_run`. A failed checkpoint rejects with
+Petri owns the durable run result. Fabro declares required finalization for
+every run and implements it in `FabroHooks::finalize_run`, so a resume or a
+fork always matches the stored declaration. A failed checkpoint rejects with
 `checkpoint_failed` and skips publication. The checkpoint failure cancelled the
 run, so Petri may commit `cancelled`; Fabro reports that finish as a workflow
 failure with the checkpoint's message.
@@ -45,8 +45,8 @@ historical-view repair requires a separate, bounded process over preserved
 source records. No publication is invoked by projection or replay.
 
 A committed resume returns the same overall result without publishing again.
-A fork inherits the source run’s required-finalization declaration while seeding
-its records; its worker restores the actual publication hooks before resume.
+A fork declares required finalization while seeding its records, as every
+Fabro run does; its worker restores the actual publication hooks before resume.
 Unfinished recovery must restore the same finalization requirement. Petri may
 call the restored finalizer again after interruption, including a crash after
 the callback returns but before the terminal record commits. Existing GitHub

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

@@ -132,12 +132,11 @@ pub async fn check(
         .open(&RunKey::new(source.to_string()), Access::Read)
         .await
         .map_err(ForkError::Open)?;
-    check_logs(&*logs, &position).await.map(|_| ())
+    check_logs(&*logs, &position).await
 }
 
-/// [`check`] over the source's opened logs. Returns whether the source
-/// requires run finalization, which the fork inherits.
-async fn check_logs(logs: &dyn RunLogs, position: &ForkPosition) -> Result<bool, ForkError> {
+/// [`check`] over the source's opened logs.
+async fn check_logs(logs: &dyn RunLogs, position: &ForkPosition) -> Result<(), ForkError> {
     let state = host::stored_state(logs).await.map_err(ForkError::Seed)?;
     let Some(execution) = state.executions.get(&position.execution) else {
         return Err(ForkError::Refused(format!(
@@ -173,20 +172,19 @@ async fn check_logs(logs: &dyn RunLogs, position: &ForkPosition) -> Result<bool,
             "the terminal checkpoint has no remaining work to acquire a sandbox; select an earlier checkpoint or retry the workflow from the start".to_string(),
         ));
     }
-    Ok(state.required_finalization)
+    Ok(())
 }
 
-/// Declaration-only hooks used while copying a fork's records. The fork
-/// inherits its source's finalization requirement; its worker installs the
-/// actual publisher before resuming. Never execute with these hooks.
-struct ForkFinalizationRequirement {
-    required: bool,
-}
+/// Declaration-only hooks used while copying a fork's records: every Fabro
+/// run requires finalization ([`crate::hooks::FabroHooks`]), so the fork's
+/// records declare it too, and its worker's hooks match them on resume.
+/// Never execute with these hooks.
+struct ForkFinalizationRequirement;
 
 #[async_trait::async_trait]
 impl ExecutionHooks for ForkFinalizationRequirement {
     fn requires_run_finalization(&self) -> bool {
-        self.required
+        true
     }
 
     async fn finalize_run(
@@ -212,14 +210,14 @@ pub async fn fork(request: ForkRequest) -> Result<Forked, ForkError> {
         .await
         .map_err(ForkError::Open)?;
 
-    let required = check_logs(&*source_logs, &request.position).await?;
+    check_logs(&*source_logs, &request.position).await?;
     let mut options = RunOptions::new(&request.fork_run_dir);
     options.run_key = Some(fork_key.clone());
     // A fork only copies records and acquires no sandbox, so it needs no
     // provider configuration.
     let runtime = providers::standard_runtime(&SandboxProviderConfig::default())
         .options(options)
-        .hooks(Arc::new(ForkFinalizationRequirement { required }))
+        .hooks(Arc::new(ForkFinalizationRequirement))
         .store(Arc::clone(&request.store));
     let forked = host::fork_from(&runtime, &*source_logs, request.position, ForkOptions {
         rerun_last: request.rerun_last,

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

@@ -31,14 +31,13 @@
 //!   `artifact.collected` record, unless the same file with the same content
 //!   was already collected earlier in the run. A failed write is a recorded
 //!   problem on the transition, never a blocked route.
-//! - `finalize_run`, required when the run checkpoints or publishes: the run's
-//!   diff, its run branch against its base commit, as the `run.diff` platform
-//!   record with the patch as a blob; a failed checkpoint, which fails the run
-//!   and skips publication; for a successful run, its publication
-//!   ([`RunPublisher`]: the platform pushes the run branch and opens a pull
-//!   request), whose failure fails the run before its terminal record.
-//! - `run_finished`: best-effort diff preparation for runs without required
-//!   finalization, then the forwarded point, so the local service runs
+//! - `finalize_run`, required for every run: the run's diff, its run branch
+//!   against its base commit, as the `run.diff` platform record with the patch
+//!   as a blob; a failed checkpoint, which fails the run and skips publication;
+//!   for a successful run, its publication ([`RunPublisher`]: the platform
+//!   pushes the run branch and opens a pull request), whose failure fails the
+//!   run before its terminal record.
+//! - `run_finished`: the forwarded point, so the local service runs
 //!   `run_complete` and `run_failed` with the sandbox in place.
 //! - `scope_acquired`: a fresh run's Git target checked out into the workspace
 //!   from inside the scope ([`crate::source`]); a resumed run uses its
@@ -1607,10 +1606,11 @@ impl ExecutionHooks for FabroHooks {
         Ok(report)
     }
 
+    /// Every Fabro run requires finalization, whether or not it publishes
+    /// or checkpoints: one path for every run, and a declaration that a
+    /// resume or a fork always matches.
     fn requires_run_finalization(&self) -> bool {
-        self.publisher.is_some()
-            || self.checkpoint_enabled
-            || self.inner.requires_run_finalization()
+        true
     }
 
     async fn finalize_run(
@@ -1667,17 +1667,8 @@ impl ExecutionHooks for FabroHooks {
     }
 
     async fn run_finished(&self, context: &HookContext, finished: RunFinished) -> Vec<Note> {
-        // Nonpublishing runs keep best-effort diff preparation. Required work
-        // has already run in finalize_run, before Petri commits its outcome.
-        if !self.requires_run_finalization() {
-            if let Err(error) = self.record_run_diff().await {
-                warn!(
-                    run_id = %self.run_id,
-                    error = %error.render(),
-                    "the run's diff was not recorded"
-                );
-            }
-        }
+        // The diff and publication already ran in finalize_run, before Petri
+        // committed the outcome.
         self.inner.run_finished(context, finished).await
     }