diff --git a/crates/arc-workflows/src/cli/run.rs b/crates/arc-workflows/src/cli/run.rs index 9439d89bb..7a582336c 100644 --- a/crates/arc-workflows/src/cli/run.rs +++ b/crates/arc-workflows/src/cli/run.rs @@ -712,18 +712,14 @@ pub async fn run_command( Ok(o) => (o.status.to_string(), o.failure_reason().map(String::from)), Err(e) => ("fail".to_string(), Some(e.to_string())), }; - let mut final_json = serde_json::json!({ - "timestamp": Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Millis, true), - "status": status, - "duration_ms": run_duration_ms, - "failure_reason": failure_reason, - }); - if let Some(sha) = last_git_sha.lock().unwrap().clone() { - final_json["final_git_commit_sha"] = serde_json::Value::String(sha); - } - if let Ok(json) = serde_json::to_string_pretty(&final_json) { - let _ = tokio::fs::write(logs_dir.join("final.json"), json).await; - } + let run_final = crate::run_final::RunFinal { + timestamp: Utc::now(), + status, + duration_ms: run_duration_ms, + failure_reason, + final_git_commit_sha: last_git_sha.lock().unwrap().clone(), + }; + let _ = run_final.save(&logs_dir.join("final.json")); } // Finish progress bars before printing summary @@ -991,7 +987,7 @@ async fn run_from_branch( std::env::set_current_dir(&worktree_path)?; let base_sha = crate::git::MetadataStore::read_manifest(&original_cwd, &run_id)? - .and_then(|m| m.get("base_sha").and_then(|v| v.as_str()).map(String::from)); + .and_then(|m| m.base_sha); // Build minimal sandbox (local only for now) let emitter = Arc::new(EventEmitter::new()); diff --git a/crates/arc-workflows/src/cli/runs.rs b/crates/arc-workflows/src/cli/runs.rs index ac9cfea97..91c2cddac 100644 --- a/crates/arc-workflows/src/cli/runs.rs +++ b/crates/arc-workflows/src/cli/runs.rs @@ -88,20 +88,13 @@ pub fn scan_runs(base: &Path) -> Result> { let manifest_path = path.join("manifest.json"); if manifest_path.exists() { - let manifest_text = std::fs::read_to_string(&manifest_path)?; debug!(dir = %dir_name, "reading manifest"); - let manifest: serde_json::Value = serde_json::from_str(&manifest_text)?; + let manifest = crate::manifest::Manifest::load(&manifest_path)?; - let run_id = manifest["run_id"].as_str().unwrap_or(&dir_name).to_string(); - let workflow_name = manifest["workflow_name"] - .as_str() - .unwrap_or("unknown") - .to_string(); - let start_time = manifest["start_time"].as_str().unwrap_or("").to_string(); - let labels: HashMap = manifest - .get("labels") - .and_then(|v| serde_json::from_value(v.clone()).ok()) - .unwrap_or_default(); + let run_id = manifest.run_id; + let workflow_name = manifest.workflow_name; + let start_time = manifest.start_time.to_rfc3339(); + let labels = manifest.labels; let status = read_status(&path); @@ -148,12 +141,8 @@ pub fn scan_runs(base: &Path) -> Result> { fn read_status(run_dir: &Path) -> String { let final_path = run_dir.join("final.json"); if final_path.exists() { - if let Ok(text) = std::fs::read_to_string(&final_path) { - if let Ok(val) = serde_json::from_str::(&text) { - if let Some(status) = val["status"].as_str() { - return status.to_string(); - } - } + if let Ok(run_final) = crate::run_final::RunFinal::load(&final_path) { + return run_final.status; } "unknown".to_string() } else if run_dir.join("run.pid").exists() { @@ -354,10 +343,13 @@ mod tests { Some(serde_json::json!({ "run_id": "abc123", "workflow_name": "my-pipeline", + "goal": "test goal", "start_time": "2026-01-01T12:00:00Z", + "node_count": 2, + "edge_count": 1, "labels": { "env": "prod" } })), - Some(serde_json::json!({ "status": "success" })), + Some(serde_json::json!({ "timestamp": "2026-01-01T12:01:00Z", "status": "success", "duration_ms": 60000 })), false, ); @@ -388,7 +380,10 @@ mod tests { Some(serde_json::json!({ "run_id": "running-1", "workflow_name": "pipeline-a", - "start_time": "2026-01-15T10:00:00Z" + "goal": "", + "start_time": "2026-01-15T10:00:00Z", + "node_count": 1, + "edge_count": 0 })), None, true, @@ -535,9 +530,12 @@ mod tests { Some(serde_json::json!({ "run_id": "to-prune", "workflow_name": "old-pipeline", - "start_time": "2025-01-01T12:00:00Z" + "goal": "", + "start_time": "2025-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 })), - Some(serde_json::json!({ "status": "success" })), + Some(serde_json::json!({ "timestamp": "2025-01-01T12:01:00Z", "status": "success", "duration_ms": 60000 })), false, ); @@ -564,9 +562,12 @@ mod tests { Some(serde_json::json!({ "run_id": "to-prune", "workflow_name": "old-pipeline", - "start_time": "2025-01-01T12:00:00Z" + "goal": "", + "start_time": "2025-01-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 })), - Some(serde_json::json!({ "status": "success" })), + Some(serde_json::json!({ "timestamp": "2025-01-01T12:01:00Z", "status": "success", "duration_ms": 60000 })), false, ); @@ -577,9 +578,12 @@ mod tests { Some(serde_json::json!({ "run_id": "keep-this", "workflow_name": "new-pipeline", - "start_time": "2026-03-01T12:00:00Z" + "goal": "", + "start_time": "2026-03-01T12:00:00Z", + "node_count": 1, + "edge_count": 0 })), - Some(serde_json::json!({ "status": "success" })), + Some(serde_json::json!({ "timestamp": "2026-03-01T12:01:00Z", "status": "success", "duration_ms": 60000 })), false, ); diff --git a/crates/arc-workflows/src/engine.rs b/crates/arc-workflows/src/engine.rs index 6315efc20..e40d1b6ad 100644 --- a/crates/arc-workflows/src/engine.rs +++ b/crates/arc-workflows/src/engine.rs @@ -272,34 +272,26 @@ pub fn resolve_thread_id( // --- Run directory helpers (spec 5.6) --- -/// Write manifest.json at the start of a workflow run. Returns the manifest value. -fn write_manifest(logs_root: &Path, graph: &Graph, config: &RunConfig) -> serde_json::Value { +/// Write manifest.json at the start of a workflow run. Returns the manifest. +fn write_manifest(logs_root: &Path, graph: &Graph, config: &RunConfig) -> crate::manifest::Manifest { let workflow_name = if graph.name.is_empty() { - "unnamed" + "unnamed".to_string() } else { - &graph.name + graph.name.clone() }; - let mut manifest = serde_json::json!({ - "run_id": config.run_id, - "workflow_name": workflow_name, - "goal": graph.goal(), - "start_time": Utc::now().to_rfc3339(), - "node_count": graph.nodes.len(), - "edge_count": graph.edges.len(), - }); - if let Some(ref branch) = config.run_branch { - manifest["run_branch"] = serde_json::Value::String(branch.clone()); - } - if let Some(ref base) = config.base_sha { - manifest["base_sha"] = serde_json::Value::String(base.clone()); - } - if !config.labels.is_empty() { - manifest["labels"] = serde_json::to_value(&config.labels).unwrap_or_default(); - } - if let Ok(json) = serde_json::to_string_pretty(&manifest) { - let _ = std::fs::create_dir_all(logs_root); - let _ = std::fs::write(logs_root.join("manifest.json"), json); - } + let manifest = crate::manifest::Manifest { + run_id: config.run_id.clone(), + workflow_name, + goal: graph.goal().to_string(), + start_time: Utc::now(), + node_count: graph.nodes.len(), + edge_count: graph.edges.len(), + run_branch: config.run_branch.clone(), + base_sha: config.base_sha.clone(), + labels: config.labels.clone(), + }; + let _ = std::fs::create_dir_all(logs_root); + let _ = manifest.save(&logs_root.join("manifest.json")); manifest } @@ -2939,13 +2931,11 @@ mod tests { let manifest_path = dir.path().join("manifest.json"); assert!(manifest_path.exists()); - let manifest: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); - assert_eq!(manifest["workflow_name"], "test_pipeline"); - assert_eq!(manifest["goal"], "Run tests"); - assert!(manifest["start_time"].is_string()); - assert!(manifest["node_count"].is_number()); - assert!(manifest["edge_count"].is_number()); + let manifest = crate::manifest::Manifest::load(&manifest_path).unwrap(); + assert_eq!(manifest.workflow_name, "test_pipeline"); + assert_eq!(manifest.goal, "Run tests"); + assert!(manifest.node_count > 0); + assert!(manifest.edge_count > 0); } #[tokio::test] @@ -2967,11 +2957,9 @@ mod tests { }; engine.run(&g, &config).await.unwrap(); - let manifest: serde_json::Value = serde_json::from_str( - &std::fs::read_to_string(dir.path().join("manifest.json")).unwrap(), - ) - .unwrap(); - assert_eq!(manifest["labels"]["env"], "test"); + let manifest = + crate::manifest::Manifest::load(&dir.path().join("manifest.json")).unwrap(); + assert_eq!(manifest.labels.get("env").map(String::as_str), Some("test")); } #[tokio::test] @@ -2993,11 +2981,9 @@ mod tests { }; engine.run(&g, &config).await.unwrap(); - let manifest: serde_json::Value = serde_json::from_str( - &std::fs::read_to_string(dir.path().join("manifest.json")).unwrap(), - ) - .unwrap(); - assert!(manifest.get("labels").is_none()); + let manifest = + crate::manifest::Manifest::load(&dir.path().join("manifest.json")).unwrap(); + assert!(manifest.labels.is_empty()); } #[tokio::test] @@ -3202,9 +3188,8 @@ mod tests { engine.run(&g, &config).await.unwrap(); let manifest_path = dir.path().join("manifest.json"); - let manifest: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); - assert_eq!(manifest["goal"], "Run tests"); + let manifest = crate::manifest::Manifest::load(&manifest_path).unwrap(); + assert_eq!(manifest.goal, "Run tests"); } #[tokio::test] @@ -3241,9 +3226,8 @@ mod tests { engine.run(&g, &config).await.unwrap(); let manifest_path = dir.path().join("manifest.json"); - let manifest: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); - assert_eq!(manifest["goal"], ""); + let manifest = crate::manifest::Manifest::load(&manifest_path).unwrap(); + assert_eq!(manifest.goal, ""); } // --- Gap #1: Auto status tests --- diff --git a/crates/arc-workflows/src/git.rs b/crates/arc-workflows/src/git.rs index 09a69cd5a..75d308650 100644 --- a/crates/arc-workflows/src/git.rs +++ b/crates/arc-workflows/src/git.rs @@ -346,13 +346,13 @@ impl MetadataStore { } } - /// Read the manifest JSON from the metadata branch. Returns `None` if not found. - pub fn read_manifest(repo_path: &Path, run_id: &str) -> Result> { + /// Read the manifest from the metadata branch. Returns `None` if not found. + pub fn read_manifest(repo_path: &Path, run_id: &str) -> Result> { match Self::read_file(repo_path, run_id, "manifest.json")? { Some(bytes) => { - let val: serde_json::Value = serde_json::from_slice(&bytes) + let manifest: crate::manifest::Manifest = serde_json::from_slice(&bytes) .map_err(|e| git_error(format!("manifest deserialize failed: {e}")))?; - Ok(Some(val)) + Ok(Some(manifest)) } None => Ok(None), } @@ -707,15 +707,15 @@ mod tests { init_repo(dir.path()); let store = MetadataStore::new(dir.path()); - let manifest = br#"{"run_id":"RUN1","pipeline":"test"}"#; + let manifest = br#"{"run_id":"RUN1","workflow_name":"test","goal":"g","start_time":"2025-01-01T00:00:00Z","node_count":2,"edge_count":1}"#; let dot = b"digraph { start -> end }"; store.init_run("RUN1", manifest, dot).unwrap(); let read_manifest = MetadataStore::read_manifest(dir.path(), "RUN1") .unwrap() .unwrap(); - assert_eq!(read_manifest["run_id"], "RUN1"); - assert_eq!(read_manifest["pipeline"], "test"); + assert_eq!(read_manifest.run_id, "RUN1"); + assert_eq!(read_manifest.workflow_name, "test"); let read_dot = MetadataStore::read_graph_dot(dir.path(), "RUN1") .unwrap() diff --git a/crates/arc-workflows/src/lib.rs b/crates/arc-workflows/src/lib.rs index e1d0efe8a..664f15090 100644 --- a/crates/arc-workflows/src/lib.rs +++ b/crates/arc-workflows/src/lib.rs @@ -14,11 +14,13 @@ pub mod graph; pub mod handler; pub mod hook; pub mod interviewer; +pub mod manifest; pub mod outcome; pub mod parser; pub mod preamble; pub mod retro; pub mod retro_agent; +pub mod run_final; pub mod stylesheet; pub mod transform; pub mod validation; diff --git a/crates/arc-workflows/src/manifest.rs b/crates/arc-workflows/src/manifest.rs new file mode 100644 index 000000000..8a7d4c74e --- /dev/null +++ b/crates/arc-workflows/src/manifest.rs @@ -0,0 +1,111 @@ +use std::collections::HashMap; +use std::path::Path; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +use crate::error::{ArcError, Result}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Manifest { + pub run_id: String, + pub workflow_name: String, + pub goal: String, + pub start_time: DateTime, + pub node_count: usize, + pub edge_count: usize, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub run_branch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub base_sha: Option, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub labels: HashMap, +} + +impl Manifest { + pub fn save(&self, path: &Path) -> Result<()> { + let json = serde_json::to_string_pretty(self) + .map_err(|e| ArcError::Checkpoint(format!("manifest serialize failed: {e}")))?; + std::fs::write(path, json)?; + Ok(()) + } + + pub fn load(path: &Path) -> Result { + let data = std::fs::read_to_string(path)?; + let manifest: Self = serde_json::from_str(&data) + .map_err(|e| ArcError::Checkpoint(format!("manifest deserialize failed: {e}")))?; + Ok(manifest) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_manifest() -> Manifest { + Manifest { + run_id: "run-1".to_string(), + workflow_name: "test_pipeline".to_string(), + goal: "Fix the bug".to_string(), + start_time: Utc::now(), + node_count: 3, + edge_count: 2, + run_branch: Some("feature/test".to_string()), + base_sha: Some("abc123".to_string()), + labels: HashMap::from([("env".into(), "test".into())]), + } + } + + #[test] + fn save_and_load_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("manifest.json"); + + let manifest = sample_manifest(); + manifest.save(&path).unwrap(); + let loaded = Manifest::load(&path).unwrap(); + + assert_eq!(loaded.run_id, "run-1"); + assert_eq!(loaded.workflow_name, "test_pipeline"); + assert_eq!(loaded.goal, "Fix the bug"); + assert_eq!(loaded.node_count, 3); + assert_eq!(loaded.edge_count, 2); + assert_eq!(loaded.run_branch.as_deref(), Some("feature/test")); + assert_eq!(loaded.base_sha.as_deref(), Some("abc123")); + assert_eq!(loaded.labels.get("env").map(String::as_str), Some("test")); + } + + #[test] + fn load_nonexistent_file() { + let result = Manifest::load(Path::new("/nonexistent/manifest.json")); + assert!(result.is_err()); + } + + #[test] + fn load_invalid_json() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bad.json"); + std::fs::write(&path, "not json").unwrap(); + + let result = Manifest::load(&path); + assert!(result.is_err()); + } + + #[test] + fn labels_omitted_when_empty() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("manifest.json"); + + let mut manifest = sample_manifest(); + manifest.labels = HashMap::new(); + manifest.run_branch = None; + manifest.base_sha = None; + manifest.save(&path).unwrap(); + + let raw: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert!(raw.get("labels").is_none()); + assert!(raw.get("run_branch").is_none()); + assert!(raw.get("base_sha").is_none()); + } +} diff --git a/crates/arc-workflows/src/run_final.rs b/crates/arc-workflows/src/run_final.rs new file mode 100644 index 000000000..b881ac0cd --- /dev/null +++ b/crates/arc-workflows/src/run_final.rs @@ -0,0 +1,120 @@ +use std::path::Path; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +use crate::error::{ArcError, Result}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RunFinal { + pub timestamp: DateTime, + pub status: String, + pub duration_ms: u64, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub failure_reason: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub final_git_commit_sha: Option, +} + +impl RunFinal { + pub fn save(&self, path: &Path) -> Result<()> { + let json = serde_json::to_string_pretty(self) + .map_err(|e| ArcError::Checkpoint(format!("run_final serialize failed: {e}")))?; + std::fs::write(path, json)?; + Ok(()) + } + + pub fn load(path: &Path) -> Result { + let data = std::fs::read_to_string(path)?; + let run_final: Self = serde_json::from_str(&data) + .map_err(|e| ArcError::Checkpoint(format!("run_final deserialize failed: {e}")))?; + Ok(run_final) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_run_final() -> RunFinal { + RunFinal { + timestamp: Utc::now(), + status: "success".to_string(), + duration_ms: 12345, + failure_reason: None, + final_git_commit_sha: Some("deadbeef".to_string()), + } + } + + #[test] + fn save_and_load_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("final.json"); + + let run_final = sample_run_final(); + run_final.save(&path).unwrap(); + let loaded = RunFinal::load(&path).unwrap(); + + assert_eq!(loaded.status, "success"); + assert_eq!(loaded.duration_ms, 12345); + assert!(loaded.failure_reason.is_none()); + assert_eq!( + loaded.final_git_commit_sha.as_deref(), + Some("deadbeef") + ); + } + + #[test] + fn load_nonexistent_file() { + let result = RunFinal::load(Path::new("/nonexistent/final.json")); + assert!(result.is_err()); + } + + #[test] + fn load_invalid_json() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bad.json"); + std::fs::write(&path, "not json").unwrap(); + + let result = RunFinal::load(&path); + assert!(result.is_err()); + } + + #[test] + fn optional_fields_omitted_when_none() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("final.json"); + + let run_final = RunFinal { + timestamp: Utc::now(), + status: "fail".to_string(), + duration_ms: 500, + failure_reason: None, + final_git_commit_sha: None, + }; + run_final.save(&path).unwrap(); + + let raw: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert!(raw.get("failure_reason").is_none()); + assert!(raw.get("final_git_commit_sha").is_none()); + } + + #[test] + fn failure_reason_present() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("final.json"); + + let run_final = RunFinal { + timestamp: Utc::now(), + status: "fail".to_string(), + duration_ms: 100, + failure_reason: Some("timeout".to_string()), + final_git_commit_sha: None, + }; + run_final.save(&path).unwrap(); + let loaded = RunFinal::load(&path).unwrap(); + + assert_eq!(loaded.failure_reason.as_deref(), Some("timeout")); + } +} diff --git a/crates/arc-workflows/tests/integration.rs b/crates/arc-workflows/tests/integration.rs index cca0415bc..61e5679ec 100644 --- a/crates/arc-workflows/tests/integration.rs +++ b/crates/arc-workflows/tests/integration.rs @@ -10019,7 +10019,7 @@ async fn git_checkpoint_host_writes_shadow_branch() { let manifest = MetadataStore::read_manifest(repo.path(), run_id) .expect("read_manifest should not error") .expect("shadow branch should contain manifest"); - assert_eq!(manifest["run_id"], run_id); + assert_eq!(manifest.run_id, run_id); // Cleanup worktree let _ = std::process::Command::new("git")