From c2c351c47219a27c63e5bc73643f05ff1df88132 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Sun, 15 Mar 2026 16:18:27 -0400
Subject: [PATCH] Simplify status.txt implementation: reduce boilerplate and
 fix double read

- Add StatusInfo::simple() helper to eliminate repeated 4-field constructions
- Eliminate double read of status.txt in scan_runs() by calling read_status()
  once and branching on Unknown vs non-Unknown
- Use write_status_file() consistently in engine.rs instead of raw fs::write

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 lib/crates/fabro-workflows/src/cli/runs.rs | 88 ++++++++++------------
 lib/crates/fabro-workflows/src/engine.rs   |  2 +-
 2 files changed, 39 insertions(+), 51 deletions(-)

diff --git a/lib/crates/fabro-workflows/src/cli/runs.rs b/lib/crates/fabro-workflows/src/cli/runs.rs
index 7b5773740..021c62a65 100644
--- a/lib/crates/fabro-workflows/src/cli/runs.rs
+++ b/lib/crates/fabro-workflows/src/cli/runs.rs
@@ -182,9 +182,27 @@ pub fn scan_runs(base: &Path) -> Result<Vec<RunInfo>> {
                 .map(|s| s.trim().to_string())
                 .unwrap_or_else(|_| dir_name.clone());
 
-            if read_status_file(&path).is_some() {
+            let si = read_status(&path);
+            if matches!(si.status, RunStatus::Unknown) {
+                // True orphan — no manifest, no status.txt
+                runs.push(RunInfo {
+                    run_id,
+                    dir_name,
+                    workflow_name: "[no manifest]".to_string(),
+                    workflow_slug: None,
+                    status: si.status,
+                    start_time: mtime,
+                    labels: HashMap::new(),
+                    duration_ms: None,
+                    total_cost: None,
+                    host_repo_path: None,
+                    start_time_dt: mtime_dt,
+                    end_time: None,
+                    path,
+                    is_orphan: true,
+                });
+            } else {
                 // Has status.txt → run is initializing, not an orphan
-                let si = read_status(&path);
                 runs.push(RunInfo {
                     run_id,
                     dir_name,
@@ -201,24 +219,6 @@ pub fn scan_runs(base: &Path) -> Result<Vec<RunInfo>> {
                     path,
                     is_orphan: false,
                 });
-            } else {
-                // True orphan — no manifest, no status.txt
-                runs.push(RunInfo {
-                    run_id,
-                    dir_name,
-                    workflow_name: "[no manifest]".to_string(),
-                    workflow_slug: None,
-                    status: RunStatus::Unknown,
-                    start_time: mtime,
-                    labels: HashMap::new(),
-                    duration_ms: None,
-                    total_cost: None,
-                    host_repo_path: None,
-                    start_time_dt: mtime_dt,
-                    end_time: None,
-                    path,
-                    is_orphan: true,
-                });
             }
         }
     }
@@ -235,6 +235,17 @@ struct StatusInfo {
     total_cost: Option<f64>,
 }
 
+impl StatusInfo {
+    fn simple(status: RunStatus) -> Self {
+        Self {
+            status,
+            end_time: None,
+            duration_ms: None,
+            total_cost: None,
+        }
+    }
+}
+
 /// Write the run lifecycle status to `status.txt` (best-effort).
 pub fn write_status_file(run_dir: &Path, status: &str) {
     let _ = std::fs::write(run_dir.join("status.txt"), status);
@@ -260,42 +271,19 @@ fn read_status(run_dir: &Path) -> StatusInfo {
     // 2. status.txt — explicit lifecycle tracking
     if let Some(status_str) = read_status_file(run_dir) {
         return match status_str.as_str() {
-            "starting" | "running" => StatusInfo {
-                status: RunStatus::Running,
-                end_time: None,
-                duration_ms: None,
-                total_cost: None,
-            },
+            "starting" | "running" => StatusInfo::simple(RunStatus::Running),
             // concluded without conclusion.json → treat as failed
-            "concluded" => StatusInfo {
-                status: RunStatus::Concluded(crate::outcome::StageStatus::Fail),
-                end_time: None,
-                duration_ms: None,
-                total_cost: None,
-            },
-            _ => StatusInfo {
-                status: RunStatus::Unknown,
-                end_time: None,
-                duration_ms: None,
-                total_cost: None,
-            },
+            "concluded" => {
+                StatusInfo::simple(RunStatus::Concluded(crate::outcome::StageStatus::Fail))
+            }
+            _ => StatusInfo::simple(RunStatus::Unknown),
         };
     }
     // 3. Legacy fallback: run.pid exists → Running
     if run_dir.join("run.pid").exists() {
-        return StatusInfo {
-            status: RunStatus::Running,
-            end_time: None,
-            duration_ms: None,
-            total_cost: None,
-        };
-    }
-    StatusInfo {
-        status: RunStatus::Unknown,
-        end_time: None,
-        duration_ms: None,
-        total_cost: None,
+        return StatusInfo::simple(RunStatus::Running);
     }
+    StatusInfo::simple(RunStatus::Unknown)
 }
 
 /// Which run statuses to include in filtered results.
diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs
index 0fdebfe2d..34613bffe 100644
--- a/lib/crates/fabro-workflows/src/engine.rs
+++ b/lib/crates/fabro-workflows/src/engine.rs
@@ -1251,7 +1251,7 @@ impl WorkflowRunEngine {
 
         // Write manifest.json (spec 5.6)
         let manifest = write_manifest(&config.run_dir, graph, config);
-        let _ = std::fs::write(config.run_dir.join("status.txt"), "running");
+        crate::cli::runs::write_status_file(&config.run_dir, "running");
 
         // Initialize metadata branch for git-native checkpoint storage (best-effort)
         if let (Some(_), Some(ref repo_path)) = (&config.meta_branch, &config.host_repo_path) {