refactor(workflow): centralize run notices

lib/crates/fabro-workflow/src/event.rs

@@ -2888,6 +2888,19 @@ impl Emitter {
         self.emit_with_scope(event, Some(scope));
     }
 
+    pub fn notice(
+        &self,
+        level: RunNoticeLevel,
+        code: impl Into<String>,
+        message: impl Into<String>,
+    ) {
+        self.emit(&Event::RunNotice {
+            level,
+            code: code.into(),
+            message: message.into(),
+        });
+    }
+
     fn emit_with_scope(&self, event: &Event, scope: Option<&StageScope>) {
         self.last_event_at.store(epoch_millis(), Ordering::Relaxed);
         event.trace();

lib/crates/fabro-workflow/src/pipeline/finalize.rs

@@ -1,11 +1,9 @@
-use std::sync::Arc;
-
-use fabro_hooks::{HookContext, HookEvent, HookRunner};
+use fabro_hooks::{HookContext, HookEvent};
 use fabro_types::BilledTokenCounts;
 
 use super::types::{Concluded, FinalizeOptions, Retroed};
 use crate::error::Error;
-use crate::event::{Emitter, Event, RunNoticeLevel};
+use crate::event::RunNoticeLevel;
 use crate::git::MetadataStore;
 use crate::outcome::{Outcome, OutcomeExt, StageStatus};
 use crate::records::{Checkpoint, Conclusion, StageSummary};
@@ -14,19 +12,7 @@ use crate::run_options::RunOptions;
 use crate::run_status::{FailureReason, RunStatus, SuccessReason};
 use crate::runtime_store::RunStoreHandle;
 use crate::sandbox_git::git_push_host;
-
-fn emit_run_notice(
-    emitter: &Emitter,
-    level: RunNoticeLevel,
-    code: impl Into<String>,
-    message: impl Into<String>,
-) {
-    emitter.emit(&Event::RunNotice {
-        level,
-        code: code.into(),
-        message: message.into(),
-    });
-}
+use crate::services::RunServices;
 
 pub fn classify_engine_result(
     engine_result: &Result<Outcome, Error>,
@@ -189,20 +175,8 @@ pub async fn write_finalize_commit(run_options: &RunOptions, run_store: &RunStor
     .await;
 }
 
-async fn run_hooks(
-    hook_runner: Option<&HookRunner>,
-    hook_context: &HookContext,
-    sandbox: Arc<dyn fabro_agent::Sandbox>,
-) {
-    let Some(runner) = hook_runner else {
-        return;
-    };
-    let _ = runner.run(hook_context, sandbox, None).await;
-}
-
 async fn cleanup_sandbox(
-    hook_runner: Option<Arc<HookRunner>>,
-    sandbox: Arc<dyn fabro_agent::Sandbox>,
+    services: &RunServices,
     run_id: &fabro_types::RunId,
     workflow_name: &str,
     preserve: bool,
@@ -212,9 +186,9 @@ async fn cleanup_sandbox(
         *run_id,
         workflow_name.to_string(),
     );
-    run_hooks(hook_runner.as_deref(), &hook_ctx, Arc::clone(&sandbox)).await;
+    let _ = services.run_hooks(&hook_ctx).await;
     if !preserve {
-        sandbox.cleanup().await?;
+        services.sandbox.cleanup().await?;
     }
     Ok(())
 }
@@ -248,25 +222,17 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
 
     if options.preserve_sandbox {
         let info = services.sandbox.sandbox_info();
-        if info.is_empty() {
-            emit_run_notice(
-                &services.emitter,
-                RunNoticeLevel::Info,
-                "sandbox_preserved",
-                "sandbox preserved",
-            );
+        let message = if info.is_empty() {
+            "sandbox preserved".to_string()
         } else {
-            emit_run_notice(
-                &services.emitter,
-                RunNoticeLevel::Info,
-                "sandbox_preserved",
-                format!("sandbox preserved: {info}"),
-            );
-        }
+            format!("sandbox preserved: {info}")
+        };
+        services
+            .emitter
+            .notice(RunNoticeLevel::Info, "sandbox_preserved", message);
     }
     if let Err(e) = cleanup_sandbox(
-        services.hook_runner.clone(),
-        Arc::clone(&services.sandbox),
+        &services,
         &options.run_id,
         &options.workflow_name,
         options.preserve_sandbox,
@@ -274,8 +240,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
     .await
     {
         tracing::warn!(error = %e, "Sandbox cleanup failed");
-        emit_run_notice(
-            &services.emitter,
+        services.emitter.notice(
             RunNoticeLevel::Warn,
             "sandbox_cleanup_failed",
             format!("sandbox cleanup failed: {e}"),
@@ -306,10 +271,9 @@ mod tests {
     use object_store::memory::InMemory;
 
     use super::*;
-    use crate::event::StoreProgressLogger;
+    use crate::event::{Emitter, StoreProgressLogger};
     use crate::pipeline::types::Retroed;
     use crate::run_options::RunOptions;
-    use crate::services::RunServices;
 
     fn test_run_id() -> RunId {
         fixtures::RUN_1

lib/crates/fabro-workflow/src/pipeline/initialize.rs

@@ -52,19 +52,6 @@ async fn run_hooks(
     runner.run(hook_context, sandbox, work_dir).await
 }
 
-fn emit_run_notice(
-    emitter: &Emitter,
-    level: RunNoticeLevel,
-    code: impl Into<String>,
-    message: impl Into<String>,
-) {
-    emitter.emit(&Event::RunNotice {
-        level,
-        code: code.into(),
-        message: message.into(),
-    });
-}
-
 async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<WorktreePlan>, Error> {
     let Some(worktree_mode) = options.worktree_mode else {
         options.run_options.display_base_sha = None;
@@ -113,8 +100,7 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
             WorkdirStrategy::LocalDirectory => None,
         };
         if let Some(env_name) = env_name {
-            emit_run_notice(
-                &options.emitter,
+            options.emitter.notice(
                 RunNoticeLevel::Warn,
                 "dirty_worktree",
                 format!("Uncommitted changes will not be included in the {env_name}."),
@@ -153,14 +139,12 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
                 })
                 .await
                 {
-                    Ok(()) => emit_run_notice(
-                        &options.emitter,
+                    Ok(()) => options.emitter.notice(
                         RunNoticeLevel::Info,
                         "git_push_succeeded",
                         format!("{branch} (synced local commits to remote)"),
                     ),
-                    Err(e) => emit_run_notice(
-                        &options.emitter,
+                    Err(e) => options.emitter.notice(
                         RunNoticeLevel::Warn,
                         "git_push_failed",
                         format!("Failed to push {branch} to origin: {e}"),
@@ -190,8 +174,7 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result<Option<Workt
                     }))
                 }
                 Err(e) => {
-                    emit_run_notice(
-                        &options.emitter,
+                    options.emitter.notice(
                         RunNoticeLevel::Warn,
                         "worktree_setup_failed",
                         format!("Git worktree setup failed ({e}), running without worktree."),
@@ -266,8 +249,7 @@ async fn build_sandbox_env(
                     Ok(token) => {
                         env.insert("GITHUB_TOKEN".to_string(), token);
                     }
-                    Err(e) => emit_run_notice(
-                        emitter,
+                    Err(e) => emitter.notice(
                         RunNoticeLevel::Warn,
                         "github_token_failed",
                         format!("Failed to mint GitHub token: {e}"),
@@ -513,8 +495,7 @@ pub async fn initialize(
                 Arc::new(ReadBeforeWriteSandbox::new(Arc::new(worktree)))
             }
             Err(e) => {
-                emit_run_notice(
-                    &options.emitter,
+                options.emitter.notice(
                     RunNoticeLevel::Warn,
                     "worktree_setup_failed",
                     format!("Git worktree setup failed ({e}), running without worktree."),

lib/crates/fabro-workflow/src/pipeline/pull_request.rs

@@ -12,7 +12,7 @@ use fabro_util::text::strip_goal_decoration;
 use tracing::{debug, info};
 
 use super::types::{Concluded, Finalized, PullRequestOptions};
-use crate::event::{Emitter, Event, RunNoticeLevel};
+use crate::event::{Event, RunNoticeLevel};
 use crate::outcome::{StageStatus, format_cost as outcome_format_cost};
 use crate::records::{Conclusion, RunSpec};
 use crate::runtime_store::RunStoreHandle;
@@ -274,19 +274,6 @@ fn assemble_pr_body(
     parts.join("\n")
 }
 
-fn emit_run_notice(
-    emitter: &Emitter,
-    level: RunNoticeLevel,
-    code: impl Into<String>,
-    message: impl Into<String>,
-) {
-    emitter.emit(&Event::RunNotice {
-        level,
-        code: code.into(),
-        message: message.into(),
-    });
-}
-
 async fn load_pull_request_diff(run_store: &RunStoreHandle) -> String {
     run_store
         .state()
@@ -580,8 +567,7 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
                             services
                                 .emitter
                                 .emit(&Event::PullRequestFailed { error: e.clone() });
-                            emit_run_notice(
-                                &services.emitter,
+                            services.emitter.notice(
                                 RunNoticeLevel::Warn,
                                 "pull_request_failed",
                                 format!("PR creation failed: {e}"),