fabro/nodes/simplify_opus/diff.patch
Fabro b721661fca checkpoint
⚒️ Generated with [Fabro](https://fabro.sh)
2026-03-19 21:56:46 -04:00

574 lines
23 KiB
Diff

diff --git a/lib/crates/fabro-cli/src/commands/attach.rs b/lib/crates/fabro-cli/src/commands/attach.rs
index 3d8f3931..03f6610c 100644
--- a/lib/crates/fabro-cli/src/commands/attach.rs
+++ b/lib/crates/fabro-cli/src/commands/attach.rs
@@ -2,7 +2,7 @@ use std::io::{BufRead, BufReader, IsTerminal};
use std::path::Path;
use std::process::ExitCode;
use std::sync::atomic::{AtomicBool, Ordering};
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
use anyhow::{bail, Result};
@@ -26,7 +26,7 @@ pub async fn attach_run(
let pid_path = run_dir.join("run.pid");
let is_tty = std::io::stderr().is_terminal();
- let progress_ui = Arc::new(Mutex::new(run_progress::ProgressUI::new(is_tty, false)));
+ let mut progress_ui = run_progress::ProgressUI::new(is_tty, false);
// Install Ctrl+C handler
let cancelled = Arc::new(AtomicBool::new(false));
@@ -57,6 +57,7 @@ pub async fn attach_run(
let file = std::fs::File::open(&progress_path)?;
let mut reader = BufReader::new(file);
let mut line = String::new();
+ let mut cached_pid: Option<u32> = None;
loop {
if cancelled.load(Ordering::Relaxed) {
@@ -85,10 +86,7 @@ pub async fn attach_run(
}
let trimmed = line.trim();
if !trimmed.is_empty() {
- progress_ui
- .lock()
- .expect("progress lock poisoned")
- .handle_json_line(trimmed);
+ progress_ui.handle_json_line(trimmed);
}
}
@@ -99,10 +97,7 @@ pub async fn attach_run(
serde_json::from_str::<fabro_interview::Question>(&request_data)
{
// Hide progress bars during interview
- progress_ui
- .lock()
- .expect("progress lock poisoned")
- .hide_bars();
+ progress_ui.hide_bars();
// Prompt user via ConsoleInterviewer
let interviewer = ConsoleInterviewer::new(styles);
@@ -114,10 +109,7 @@ pub async fn attach_run(
}
// Show progress bars again
- progress_ui
- .lock()
- .expect("progress lock poisoned")
- .show_bars();
+ progress_ui.show_bars();
}
}
}
@@ -125,28 +117,36 @@ pub async fn attach_run(
// Check if run is complete
if conclusion_path.exists() {
// Drain any remaining lines
- drain_remaining(&mut reader, &mut line, &progress_ui);
+ drain_remaining(&mut reader, &mut line, &mut progress_ui);
break;
}
- // Check if engine process is still alive (if PID file exists)
- if pid_path.exists() {
- if let Ok(pid_str) = std::fs::read_to_string(&pid_path) {
- if let Ok(pid) = pid_str.trim().parse::<u32>() {
- if !process_alive(pid) && !conclusion_path.exists() {
- // Engine died without writing conclusion — drain and exit
- drain_remaining(&mut reader, &mut line, &progress_ui);
- break;
+ // Check if engine process is still alive (cache PID after first read)
+ let engine_alive = match cached_pid {
+ Some(pid) => process_alive(pid),
+ None => {
+ if let Ok(pid_str) = std::fs::read_to_string(&pid_path) {
+ if let Ok(pid) = pid_str.trim().parse::<u32>() {
+ cached_pid = Some(pid);
+ process_alive(pid)
+ } else {
+ true
}
+ } else {
+ true // no PID file yet, assume alive
}
}
+ };
+ if !engine_alive && !conclusion_path.exists() {
+ drain_remaining(&mut reader, &mut line, &mut progress_ui);
+ break;
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
// Finish progress bars
- progress_ui.lock().expect("progress lock poisoned").finish();
+ progress_ui.finish();
// Determine exit code from conclusion
if conclusion_path.exists() {
@@ -173,7 +173,7 @@ pub async fn attach_run(
fn drain_remaining(
reader: &mut BufReader<std::fs::File>,
line: &mut String,
- progress_ui: &Arc<Mutex<run_progress::ProgressUI>>,
+ progress_ui: &mut run_progress::ProgressUI,
) {
loop {
line.clear();
@@ -182,10 +182,7 @@ fn drain_remaining(
Ok(_) => {
let trimmed = line.trim();
if !trimmed.is_empty() {
- progress_ui
- .lock()
- .expect("progress lock poisoned")
- .handle_json_line(trimmed);
+ progress_ui.handle_json_line(trimmed);
}
}
Err(_) => break,
diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs
index f3782cec..d5db8393 100644
--- a/lib/crates/fabro-cli/src/commands/create.rs
+++ b/lib/crates/fabro-cli/src/commands/create.rs
@@ -1,118 +1,21 @@
use std::path::PathBuf;
-use anyhow::{bail, Context};
+use anyhow::bail;
use chrono::Local;
+use fabro_config::project as project_config;
use fabro_config::run::RunDefaults;
-use fabro_config::{project as project_config, sandbox as sandbox_config};
use fabro_validate::Severity;
use fabro_workflows::run_spec::RunSpec;
use fabro_workflows::sandbox_provider::SandboxProvider;
use fabro_workflows::workflow::WorkflowBuilder;
-use super::run::RunArgs;
+use super::run::{
+ apply_goal_override, resolve_cli_goal, resolve_model_provider, resolve_sandbox_provider,
+ RunArgs,
+};
use super::shared::{print_diagnostics, read_workflow_file, relative_path};
use fabro_util::terminal::Styles;
-/// Resolve goal from `--goal` string or `--goal-file` path.
-fn resolve_cli_goal(
- goal: &Option<String>,
- goal_file: &Option<PathBuf>,
-) -> anyhow::Result<Option<String>> {
- match (goal, goal_file) {
- (Some(g), _) => Ok(Some(g.clone())),
- (_, Some(path)) => {
- let path = fabro_util::path::expand_tilde(path);
- let content = std::fs::read_to_string(&path)
- .with_context(|| format!("failed to read goal file: {}", path.display()))?;
- tracing::debug!(path = %path.display(), "Goal loaded from file");
- Ok(Some(content))
- }
- _ => Ok(None),
- }
-}
-
-/// Apply goal to the graph from TOML config or CLI flag.
-fn apply_goal_override(
- graph: &mut fabro_graphviz::graph::Graph,
- cli_goal: Option<&str>,
- toml_goal: Option<&str>,
-) {
- let goal = cli_goal.or(toml_goal);
- if let Some(goal) = goal {
- graph.attrs.insert(
- "goal".to_string(),
- fabro_graphviz::graph::AttrValue::String(goal.to_string()),
- );
- }
-}
-
-/// Parse sandbox provider from an optional `SandboxConfig`.
-fn parse_sandbox_provider(
- sandbox: Option<&sandbox_config::SandboxConfig>,
-) -> anyhow::Result<Option<SandboxProvider>> {
- sandbox
- .and_then(|s| s.provider.as_deref())
- .map(|s| s.parse::<SandboxProvider>())
- .transpose()
- .map_err(|e| anyhow::anyhow!("Invalid sandbox provider: {e}"))
-}
-
-/// Resolve sandbox provider: CLI flag > TOML config > run defaults > default.
-fn resolve_sandbox_provider(
- cli: Option<SandboxProvider>,
- run_cfg: Option<&fabro_config::run::WorkflowRunConfig>,
- run_defaults: &RunDefaults,
-) -> anyhow::Result<SandboxProvider> {
- let toml = parse_sandbox_provider(run_cfg.and_then(|c| c.sandbox.as_ref()))?;
- let defaults = parse_sandbox_provider(run_defaults.sandbox.as_ref())?;
- Ok(cli.or(toml).or(defaults).unwrap_or_default())
-}
-
-/// Resolve model and provider through the full precedence chain.
-fn resolve_model_provider(
- cli_model: Option<&str>,
- cli_provider: Option<&str>,
- run_cfg: Option<&fabro_config::run::WorkflowRunConfig>,
- run_defaults: &RunDefaults,
- graph: &fabro_graphviz::graph::Graph,
-) -> (String, Option<String>) {
- let toml_model = run_cfg
- .and_then(|c| c.llm.as_ref())
- .and_then(|l| l.model.as_deref());
- let toml_provider = run_cfg
- .and_then(|c| c.llm.as_ref())
- .and_then(|l| l.provider.as_deref());
- let defaults_model = run_defaults.llm.as_ref().and_then(|l| l.model.as_deref());
- let defaults_provider = run_defaults
- .llm
- .as_ref()
- .and_then(|l| l.provider.as_deref());
-
- let provider = cli_provider
- .or(toml_provider)
- .or(defaults_provider)
- .or_else(|| graph.attrs.get("default_provider").and_then(|v| v.as_str()))
- .map(String::from);
-
- let model = cli_model
- .or(toml_model)
- .or(defaults_model)
- .or_else(|| graph.attrs.get("default_model").and_then(|v| v.as_str()))
- .map(String::from)
- .unwrap_or_else(|| {
- provider
- .as_deref()
- .and_then(fabro_llm::catalog::default_model_for_provider)
- .unwrap_or_else(fabro_llm::catalog::default_model_from_env)
- .id
- });
-
- match fabro_llm::catalog::get_model_info(&model) {
- Some(info) => (info.id, provider.or(Some(info.provider))),
- None => (model, provider),
- }
-}
-
/// Create a workflow run: allocate run directory, persist spec, return (run_id, run_dir).
///
/// This does NOT execute the workflow — it only prepares the run directory.
diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs
index da58742f..2bca60f6 100644
--- a/lib/crates/fabro-cli/src/commands/run.rs
+++ b/lib/crates/fabro-cli/src/commands/run.rs
@@ -57,6 +57,19 @@ impl From<CliSandboxProvider> for SandboxProvider {
}
}
+impl From<SandboxProvider> for CliSandboxProvider {
+ fn from(value: SandboxProvider) -> Self {
+ match value {
+ SandboxProvider::Local => Self::Local,
+ SandboxProvider::Docker => Self::Docker,
+ SandboxProvider::Daytona => Self::Daytona,
+ #[cfg(feature = "exedev")]
+ SandboxProvider::Exe => Self::Exe,
+ SandboxProvider::Ssh => Self::Ssh,
+ }
+ }
+}
+
#[derive(Args)]
pub struct RunArgs {
/// Path to a .fabro workflow file or .toml task config (not required with --run-branch)
@@ -137,7 +150,7 @@ pub struct RunArgs {
}
/// Resolve goal from `--goal` string or `--goal-file` path.
-fn resolve_cli_goal(
+pub(crate) fn resolve_cli_goal(
goal: &Option<String>,
goal_file: &Option<PathBuf>,
) -> anyhow::Result<Option<String>> {
@@ -156,7 +169,7 @@ fn resolve_cli_goal(
/// Apply goal to the graph from TOML config or CLI flag.
/// Precedence: CLI `--goal` / `--goal-file` > TOML `goal` > DOT `graph [goal="..."]`.
-fn apply_goal_override(
+pub(crate) fn apply_goal_override(
graph: &mut fabro_graphviz::graph::Graph,
cli_goal: Option<&str>,
toml_goal: Option<&str>,
@@ -174,7 +187,7 @@ fn apply_goal_override(
/// Resolve model and provider through the full precedence chain:
/// CLI flag > TOML config > run defaults > DOT graph attrs > provider-specific defaults.
/// Then resolve through the catalog for alias expansion.
-fn resolve_model_provider(
+pub(crate) fn resolve_model_provider(
cli_model: Option<&str>,
cli_provider: Option<&str>,
run_cfg: Option<&WorkflowRunConfig>,
@@ -221,7 +234,7 @@ fn resolve_model_provider(
}
/// Parse sandbox provider from an optional `SandboxConfig`.
-fn parse_sandbox_provider(
+pub(crate) fn parse_sandbox_provider(
sandbox: Option<&sandbox_config::SandboxConfig>,
) -> anyhow::Result<Option<SandboxProvider>> {
sandbox
@@ -232,7 +245,7 @@ fn parse_sandbox_provider(
}
/// Resolve sandbox provider: CLI flag > TOML config > run defaults > default.
-fn resolve_sandbox_provider(
+pub(crate) fn resolve_sandbox_provider(
cli: Option<SandboxProvider>,
run_cfg: Option<&WorkflowRunConfig>,
run_defaults: &RunDefaults,
diff --git a/lib/crates/fabro-cli/src/commands/run_progress.rs b/lib/crates/fabro-cli/src/commands/run_progress.rs
index 2619cee0..dc0bebea 100644
--- a/lib/crates/fabro-cli/src/commands/run_progress.rs
+++ b/lib/crates/fabro-cli/src/commands/run_progress.rs
@@ -776,15 +776,13 @@ impl ProgressUI {
let stage = str_field("stage").unwrap_or("?");
let tool_name = str_field("tool_name").unwrap_or("?");
let tool_call_id = str_field("tool_call_id").unwrap_or("?");
- let arguments = envelope
- .get("arguments")
- .cloned()
- .unwrap_or(serde_json::Value::Object(serde_json::Map::new()));
+ let empty = serde_json::Value::Object(serde_json::Map::new());
+ let arguments = envelope.get("arguments").unwrap_or(&empty);
// Update tool_call count
if let Some(counts) = self.stage_counts.get_mut(stage) {
counts.1 += 1;
}
- self.on_tool_call_started(stage, tool_name, tool_call_id, &arguments);
+ self.on_tool_call_started(stage, tool_name, tool_call_id, arguments);
}
"Agent.ToolCallCompleted" => {
let stage = str_field("stage").unwrap_or("?");
@@ -1560,43 +1558,33 @@ impl ProgressAwareInterviewer {
pub fn new(inner: ConsoleInterviewer, progress: Arc<Mutex<ProgressUI>>) -> Self {
Self { inner, progress }
}
-
- fn hide_bars(&self) {
- let ui = self.progress.lock().expect("progress lock poisoned");
- if let ProgressRenderer::Tty(tty) = &ui.renderer {
- tty.multi.set_draw_target(ProgressDrawTarget::hidden());
- }
- }
-
- fn show_bars(&self) {
- let ui = self.progress.lock().expect("progress lock poisoned");
- if let ProgressRenderer::Tty(tty) = &ui.renderer {
- tty.multi.set_draw_target(ProgressDrawTarget::stderr());
- }
- }
}
#[async_trait]
impl Interviewer for ProgressAwareInterviewer {
async fn ask(&self, question: Question) -> Answer {
- {
- let ui = self.progress.lock().expect("progress lock poisoned");
- if let ProgressRenderer::Tty(tty) = &ui.renderer {
- let sep = tty.multi.add(ProgressBar::new_spinner());
- sep.set_style(style_empty());
- sep.finish();
- tty.multi.set_draw_target(ProgressDrawTarget::hidden());
- }
- }
+ self.progress
+ .lock()
+ .expect("progress lock poisoned")
+ .hide_bars();
let answer = self.inner.ask(question).await;
- self.show_bars();
+ self.progress
+ .lock()
+ .expect("progress lock poisoned")
+ .show_bars();
answer
}
async fn inform(&self, message: &str, stage: &str) {
- self.hide_bars();
+ self.progress
+ .lock()
+ .expect("progress lock poisoned")
+ .hide_bars();
self.inner.inform(message, stage).await;
- self.show_bars();
+ self.progress
+ .lock()
+ .expect("progress lock poisoned")
+ .show_bars();
}
}
diff --git a/lib/crates/fabro-cli/src/commands/start.rs b/lib/crates/fabro-cli/src/commands/start.rs
index f4213f6a..840d8919 100644
--- a/lib/crates/fabro-cli/src/commands/start.rs
+++ b/lib/crates/fabro-cli/src/commands/start.rs
@@ -9,25 +9,19 @@ use anyhow::{bail, Result};
pub fn start_run(run_dir: &Path) -> Result<u32> {
// Validate status is Submitted
let status_path = run_dir.join("status.json");
- if status_path.exists() {
- let record = fabro_workflows::run_status::RunStatusRecord::load(&status_path)
- .map_err(|e| anyhow::anyhow!("Failed to read status.json: {e}"))?;
- if record.status != fabro_workflows::run_status::RunStatus::Submitted {
+ match fabro_workflows::run_status::RunStatusRecord::load(&status_path) {
+ Ok(record) if record.status != fabro_workflows::run_status::RunStatus::Submitted => {
bail!(
"Cannot start run: status is {:?}, expected Submitted",
record.status
);
}
+ _ => {} // No status file or Submitted — proceed
}
- // Validate spec.json exists
- let spec_path = run_dir.join("spec.json");
- if !spec_path.exists() {
- bail!(
- "Cannot start run: spec.json not found in {}",
- run_dir.display()
- );
- }
+ // Validate spec.json is loadable
+ fabro_workflows::run_spec::RunSpec::load(run_dir)
+ .map_err(|e| anyhow::anyhow!("Cannot start run: failed to load spec.json: {e}"))?;
let log_file = std::fs::File::create(run_dir.join("detach.log"))?;
diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs
index 2c8f5c8a..28c73b23 100644
--- a/lib/crates/fabro-cli/src/main.rs
+++ b/lib/crates/fabro-cli/src/main.rs
@@ -728,24 +728,7 @@ async fn main_inner() -> (String, Result<()>) {
.sandbox_provider
.parse::<fabro_workflows::sandbox_provider::SandboxProvider>()
.ok()
- .map(|sp| match sp {
- fabro_workflows::sandbox_provider::SandboxProvider::Local => {
- commands::run::CliSandboxProvider::Local
- }
- fabro_workflows::sandbox_provider::SandboxProvider::Docker => {
- commands::run::CliSandboxProvider::Docker
- }
- fabro_workflows::sandbox_provider::SandboxProvider::Daytona => {
- commands::run::CliSandboxProvider::Daytona
- }
- fabro_workflows::sandbox_provider::SandboxProvider::Ssh => {
- commands::run::CliSandboxProvider::Ssh
- }
- #[cfg(feature = "exedev")]
- fabro_workflows::sandbox_provider::SandboxProvider::Exe => {
- commands::run::CliSandboxProvider::Exe
- }
- }),
+ .map(commands::run::CliSandboxProvider::from),
label: spec
.labels
.into_iter()
diff --git a/lib/crates/fabro-interview/Cargo.toml b/lib/crates/fabro-interview/Cargo.toml
index d641da18..ef57afbe 100644
--- a/lib/crates/fabro-interview/Cargo.toml
+++ b/lib/crates/fabro-interview/Cargo.toml
@@ -19,4 +19,4 @@ fabro-util = { path = "../fabro-util" }
[dev-dependencies]
tokio = { workspace = true, features = ["test-util", "macros"] }
-tempfile = "3"
\ No newline at end of file
+tempfile = "3"
diff --git a/lib/crates/fabro-interview/src/file.rs b/lib/crates/fabro-interview/src/file.rs
index 1f0def2a..2a7e41dc 100644
--- a/lib/crates/fabro-interview/src/file.rs
+++ b/lib/crates/fabro-interview/src/file.rs
@@ -45,23 +45,24 @@ impl Interviewer for FileInterviewer {
let poll = async {
let response_path = self.response_path();
loop {
- if response_path.exists() {
- match tokio::fs::read_to_string(&response_path).await {
- Ok(data) => match serde_json::from_str::<Answer>(&data) {
- Ok(answer) => {
- // Clean up both files
- let _ = tokio::fs::remove_file(&request_path).await;
- let _ = tokio::fs::remove_file(&response_path).await;
- return answer;
- }
- Err(e) => {
- tracing::warn!(error = %e, "Failed to parse interview response, retrying");
- // File might be partially written, wait and retry
- }
- },
+ match tokio::fs::read_to_string(&response_path).await {
+ Ok(data) => match serde_json::from_str::<Answer>(&data) {
+ Ok(answer) => {
+ // Clean up both files
+ let _ = tokio::fs::remove_file(&request_path).await;
+ let _ = tokio::fs::remove_file(&response_path).await;
+ return answer;
+ }
Err(e) => {
- tracing::warn!(error = %e, "Failed to read interview response, retrying");
+ tracing::warn!(error = %e, "Failed to parse interview response, retrying");
+ // File might be partially written, wait and retry
}
+ },
+ Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+ // Not written yet, poll again
+ }
+ Err(e) => {
+ tracing::warn!(error = %e, "Failed to read interview response, retrying");
}
}
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
diff --git a/lib/crates/fabro-workflows/src/run_spec.rs b/lib/crates/fabro-workflows/src/run_spec.rs
index da5d51f8..df5dbac5 100644
--- a/lib/crates/fabro-workflows/src/run_spec.rs
+++ b/lib/crates/fabro-workflows/src/run_spec.rs
@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use serde::{Deserialize, Serialize};
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct RunSpec {
pub run_id: String,
pub workflow_path: PathBuf,
@@ -78,23 +78,7 @@ mod tests {
spec.save(dir.path()).unwrap();
let loaded = RunSpec::load(dir.path()).unwrap();
- assert_eq!(loaded.run_id, spec.run_id);
- assert_eq!(loaded.workflow_path, spec.workflow_path);
- assert_eq!(loaded.dot_source, spec.dot_source);
- assert_eq!(loaded.working_directory, spec.working_directory);
- assert_eq!(loaded.goal, spec.goal);
- assert_eq!(loaded.model, spec.model);
- assert_eq!(loaded.provider, spec.provider);
- assert_eq!(loaded.sandbox_provider, spec.sandbox_provider);
- assert_eq!(loaded.labels, spec.labels);
- assert_eq!(loaded.verbose, spec.verbose);
- assert_eq!(loaded.no_retro, spec.no_retro);
- assert_eq!(loaded.ssh, spec.ssh);
- assert_eq!(loaded.preserve_sandbox, spec.preserve_sandbox);
- assert_eq!(loaded.dry_run, spec.dry_run);
- assert_eq!(loaded.auto_approve, spec.auto_approve);
- assert_eq!(loaded.resume, spec.resume);
- assert_eq!(loaded.run_branch, spec.run_branch);
+ assert_eq!(loaded, spec);
}
#[test]