checkpoint

⚒️ Generated with [Fabro](https://fabro.sh)
This commit is contained in:
Fabro 2026-03-19 21:56:46 -04:00
parent 140b789412
commit b721661fca
4 changed files with 879 additions and 11 deletions

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,271 @@
Goal: # Decompose `fabro run` into `create` / `start` / `attach`
## Context
`fabro run` currently does everything in a single process: creates the run directory, sets up the event system, builds the sandbox, runs the workflow engine, writes the conclusion, and renders live progress. The `--detach` flag is a bolt-on that spawns a child process by reconstructing CLI argv — brittle and not composable.
The goal is to decompose into three primitives (Docker-style):
- **`fabro create`** — allocate run, persist spec, return run ID
- **`fabro start`** — spawn a detached engine process (always a separate process)
- **`fabro attach`** — tail progress.jsonl with live rendering + handle interviews
Compositions:
- `fabro run` = create + start + attach (attach opens the file before start, guaranteeing zero missed events)
- `fabro run --detach` = create + start + print run ID
- Standalone `fabro attach <id>` = reconnect to any running/finished run
## Key design decisions
### 1. Attach absorbs `run_progress.rs`
The existing `ProgressUI` (indicatif spinners, stage tracking, tool call rendering) moves into `attach`. Rather than building a new renderer, we add a `handle_json_line(&str)` method to ProgressUI that parses JSONL envelopes and dispatches to the same internal rendering methods (`on_stage_started`, `finish_stage`, `on_tool_call_started`, etc.). This preserves 100% rendering fidelity.
The dispatch pattern already exists in `format_event_pretty()` in `logs.rs` — match on the `"event"` string field, extract typed values from JSON. The internal ProgressUI methods already take simple types (strings, ints), not `WorkflowRunEvent`.
`handle_event(&WorkflowRunEvent)` stays for any in-process callers (API server).
### 2. File-based interview IPC
The engine process uses a new `FileInterviewer` (impl Interviewer) that:
- Writes `interview_request.json` (serialized `Question`) to run_dir
- Polls for `interview_response.json` in run_dir
- Deserializes the `Answer`, cleans up both files
The attach loop watches for `interview_request.json`:
1. Hides indicatif bars (same as `ProgressAwareInterviewer` does today)
2. Prompts user via `ConsoleInterviewer` logic
3. Writes `interview_response.json`
4. Shows bars again
`Question` and `Answer` already derive `Serialize`/`Deserialize`.
### 3. RunSpec persistence
`create` writes `spec.json` to run_dir — a serializable struct with all CLI args needed to run the engine. Replaces the argv-reconstruction in `detach_run()`.
### 4. Engine invocation
`start` spawns `fabro _run_engine --run-dir <dir>` — a hidden internal command that reads `spec.json` and executes the workflow. The child uses `FileInterviewer` instead of `ConsoleInterviewer`.
### 5. Stdin and Ctrl+C
- `fabro run` (foreground): Ctrl+C sends SIGTERM to child (via `run.pid`), waits for conclusion, then exits
- `fabro attach` (standalone): Ctrl+C just detaches, run continues
- Engine process stdin is always `/dev/null` — interviews go through file-based IPC, not stdin
---
## Implementation plan
### Phase 1: Foundation — RunSpec + extract engine
**Step 1: RunSpec struct**
- New file: `lib/crates/fabro-workflows/src/run_spec.rs`
- `#[derive(Serialize, Deserialize)]` struct: run_id, workflow_path (absolute), dot_source, working_directory, goal, model, provider, sandbox_provider, labels, verbose, no_retro, ssh, preserve_sandbox, dry_run, auto_approve, resume, run_branch
- Methods: `save(run_dir)`, `load(run_dir)`
- Register in `lib/crates/fabro-workflows/src/lib.rs`
**Step 2: FileInterviewer**
- New file: `lib/crates/fabro-interview/src/file.rs`
- `FileInterviewer { run_dir: PathBuf }` implementing `Interviewer`
- `ask()`: write `interview_request.json`, poll for `interview_response.json` (100ms interval, respect timeout from Question), deserialize Answer, clean up files
- Register in `lib/crates/fabro-interview/src/lib.rs`
**Step 3: Extract `run_engine()` from `run_command()`**
- Modify: `lib/crates/fabro-cli/src/commands/run.rs`
- New function: `run_engine(spec, run_dir, run_defaults, styles, github_app, git_author) -> Result<()>`
- Contains lines ~629end of current `run_command()`: EventEmitter + JSONL writer + cost accumulator + git SHA tracker, sandbox creation, engine execution, conclusion writing, retro, PR creation, cleanup
- Does NOT register ProgressUI — only writes progress.jsonl
- Uses `FileInterviewer` instead of `ConsoleInterviewer`/`ProgressAwareInterviewer`
**Step 4: Hidden `_run_engine` command**
- Modify: `lib/crates/fabro-cli/src/main.rs`
- Add `_RunEngine { run_dir: PathBuf }` to `Command` enum (hidden)
- Handler: load `spec.json`, load cli_config/github_app/git_author, call `run_engine()`
### Phase 2: Attach — ProgressUI from JSONL + interview handling
**Step 5: Add `handle_json_line` to ProgressUI**
- Modify: `lib/crates/fabro-cli/src/commands/run_progress.rs`
- New method: `handle_json_line(&mut self, line: &str)` that parses envelope JSON and dispatches to existing internal methods:
- `"Sandbox.Initializing"` / `"Sandbox.Ready"``on_sandbox_event()`
- `"SetupStarted"` / `"SetupCompleted"``on_setup_started/completed()`
- `"StageStarted"``on_stage_started(node_id, name, script)`
- `"StageCompleted"` → extract fields, call `finish_stage()`
- `"StageFailed"``finish_stage()` + error info
- `"Agent.ToolCallStarted"` / `"Agent.ToolCallCompleted"``on_tool_call_started/completed()`
- `"Agent.AssistantMessage"` → update stage model display
- `"Agent.CompactionCompleted"` → compaction bar
- `"ParallelBranchStarted"` / `"ParallelBranchCompleted"` → branch tracking
- `"RetroStarted"` / `"RetroCompleted"` / `"RetroFailed"` → retro spinner
- `"SshAccessReady"` → SSH command display
- etc.
- Follows the same pattern as `format_event_pretty()` in `logs.rs` but calls internal rendering methods instead of formatting strings
**Step 6: Attach command**
- New file: `lib/crates/fabro-cli/src/commands/attach.rs`
- `attach_run(run_dir, kill_on_detach: bool) -> Result<ExitCode>`:
1. Read `spec.json` for header info (run_id, workflow name)
2. Create ProgressUI, show header (version, run_id, time, run_dir)
3. Poll loop (100ms):
- Read new lines from `progress.jsonl`, feed to `progress_ui.handle_json_line()`
- Check for `interview_request.json` → hide bars, prompt via ConsoleInterviewer, write `interview_response.json`, show bars
- Exit when `conclusion.json` exists and no new lines
4. Read `conclusion.json` for exit code: Success/PartialSuccess → 0, else → 1
- Ctrl+C: if `kill_on_detach`, SIGTERM to child PID from `run.pid`; otherwise print "Detached" and exit 0
- Add `Attach { run: String, verbose: bool }` to `Command` enum
- Handler: `run_lookup::resolve_run()`, call `attach_run()`
### Phase 3: Create + Start commands
**Step 7: Create command**
- New file: `lib/crates/fabro-cli/src/commands/create.rs`
- `create_run(args, run_defaults, styles) -> Result<(String, PathBuf)>`:
- Extract lines ~440627 from `run_command()`: resolve workflow, parse graph, validate, resolve sandbox/model/provider, create run_dir, write graph.fabro/id.txt/status.json(Submitted)/spec.json
- Returns (run_id, run_dir)
- Add `Create` variant to `Command` enum (same args as RunArgs minus --detach/--run-id/--run-dir)
**Step 8: Start command**
- New file: `lib/crates/fabro-cli/src/commands/start.rs`
- `start_run(run_dir, inherit_stdin: bool) -> Result<u32>` (returns child PID):
- Validate status.json is `Submitted`
- Spawn `fabro _run_engine --run-dir <dir>` as detached child (setsid, stdout/stderr → detach.log, stdin → /dev/null)
- Write child PID to `run.pid`
- Return PID
- Add `Start { run: String }` to `Command` enum
- Handler: resolve run, call `start_run()`
### Phase 4: Recompose `fabro run`
**Step 9: Rewrite `run_command()` as composition**
- `fabro run` (foreground):
```
let (run_id, run_dir) = create_run(args, ...)?;
let _child_pid = start_run(&run_dir)?;
let exit_code = attach_run(&run_dir, kill_on_detach=true)?;
std::process::exit(exit_code);
```
- `fabro run --detach`:
```
let (run_id, run_dir) = create_run(args, ...)?;
start_run(&run_dir)?;
println!("{run_id}");
```
- Delete `detach_run()` from main.rs
- Deprecate `--run-id` / `--run-dir` hidden flags
### Phase 5: Cleanup + testing
**Step 10: Tests**
- `run_spec.rs`: save/load roundtrip
- `file.rs` (FileInterviewer): write request → write response → verify ask() returns correct answer
- `run_progress.rs`: test `handle_json_line` with sample JSONL lines (stage started/completed, tool calls, etc.)
- `attach.rs`: integration test — write JSONL lines to a temp file, verify attach loop renders and exits correctly
- Existing tests remain unchanged
**Step 11: Remove dead code**
- Delete `detach_run()` from main.rs
- Remove `--run-id` / `--run-dir` args from RunArgs
- `ProgressAwareInterviewer` moves into attach.rs (or gets deleted if attach handles the coordination directly)
---
## Files summary
| File | Action |
|------|--------|
| `lib/crates/fabro-workflows/src/run_spec.rs` | **New** — RunSpec struct |
| `lib/crates/fabro-workflows/src/lib.rs` | Modify — register run_spec module |
| `lib/crates/fabro-interview/src/file.rs` | **New** — FileInterviewer |
| `lib/crates/fabro-interview/src/lib.rs` | Modify — register file module |
| `lib/crates/fabro-cli/src/commands/create.rs` | **New** — create logic extracted from run.rs |
| `lib/crates/fabro-cli/src/commands/start.rs` | **New** — spawn detached engine process |
| `lib/crates/fabro-cli/src/commands/attach.rs` | **New** — attach loop + interview handling |
| `lib/crates/fabro-cli/src/commands/run_progress.rs` | Modify — add `handle_json_line()` method |
| `lib/crates/fabro-cli/src/commands/run.rs` | Modify — extract run_engine(), rewrite run_command() as composition |
| `lib/crates/fabro-cli/src/commands/mod.rs` | Modify — register new modules |
| `lib/crates/fabro-cli/src/main.rs` | Modify — add Command variants, delete detach_run() |
## Verification
1. `cargo build --workspace` — compiles
2. `cargo test --workspace` — all existing + new tests pass
3. `cargo clippy --workspace -- -D warnings` — clean
4. Manual: `fabro create <workflow> --goal "test"` → prints run ID, creates spec.json in run dir
5. Manual: `fabro start <run_id>` → spawns engine, status transitions
6. Manual: `fabro attach <run_id>` → live progress with spinners, exits when done with correct exit code
7. Manual: `fabro run <workflow>` → identical UX to current foreground behavior
8. Manual: `fabro run -d <workflow>` → prints run ID, background process runs
9. Manual: Ctrl+C during `fabro run` kills child; Ctrl+C during `fabro attach` detaches
10. Manual: workflow with human gate — `fabro run` shows interactive prompt, answer flows back to engine
## Completed stages
- **toolchain**: success
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
- Stdout:
```
cargo 1.94.0 (85eff7c80 2026-01-15)
```
- Stderr: (empty)
- **preflight_compile**: success
- Script: `cargo check -q --workspace 2>&1`
- Stdout: (empty)
- Stderr: (empty)
- **preflight_lint**: success
- Script: `cargo clippy -q --workspace -- -D warnings 2>&1`
- Stdout: (empty)
- Stderr: (empty)
- **implement**: success
- Model: claude-opus-4-6, 160.5k tokens in / 38.3k out
- Files: /home/daytona/workspace/lib/crates/fabro-cli/src/commands/attach.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/mod.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run_progress.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/start.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/main.rs, /home/daytona/workspace/lib/crates/fabro-interview/Cargo.toml, /home/daytona/workspace/lib/crates/fabro-interview/src/file.rs, /home/daytona/workspace/lib/crates/fabro-interview/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/run_spec.rs
- **simplify_opus**: success
- Model: claude-opus-4-6, 116.5k tokens in / 20.3k out
- Files: /home/daytona/workspace/lib/crates/fabro-cli/src/commands/attach.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run_progress.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/start.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/main.rs, /home/daytona/workspace/lib/crates/fabro-interview/Cargo.toml, /home/daytona/workspace/lib/crates/fabro-interview/src/file.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/run_spec.rs
# Simplify: Code Review and Cleanup
Review all changed files for reuse, quality, and efficiency. Fix any issues found.
## Phase 1: Identify Changes
Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
## Phase 2: Launch Three Review Agents in Parallel
Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
### Agent 1: Code Reuse Review
For each change:
1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
### Agent 2: Code Quality Review
Review the same changes for hacky patterns:
1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
Note: This is a greenfield app, so be aggressive in optimizing quality.
### Agent 3: Efficiency Review
Review the same changes for efficiency:
1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
2. Missed concurrency: independent operations run sequentially when they could run in parallel
3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
5. Memory: unbounded data structures, missing cleanup, event listener leaks
6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
## Phase 3: Fix Issues
Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
When done, briefly summarize what was fixed (or confirm the code was already clean).

View file

@ -0,0 +1,6 @@
{
"status": "fail",
"notes": null,
"failure_reason": "LLM error: Not found on anthropic: model: gpt-54",
"timestamp": "2026-03-20T01:56:46.688134+00:00"
}

View file

@ -0,0 +1,574 @@
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]