From 514cac59f5b7ec8725cbff8339ab160cf37e62e7 Mon Sep 17 00:00:00 2001 From: Fabro Date: Sat, 21 Mar 2026 10:45:27 -0400 Subject: [PATCH] checkpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⚒️ Generated with [Fabro](https://fabro.sh) --- checkpoint.json | 75 +- nodes/implement/diff.patch | 1512 ++++++++++++++++++++++++ nodes/simplify_opus/prompt.md | 173 +++ nodes/simplify_opus/provider_used.json | 5 + nodes/simplify_opus/response.md | 23 + nodes/simplify_opus/status.json | 6 + 6 files changed, 1773 insertions(+), 21 deletions(-) create mode 100644 nodes/implement/diff.patch create mode 100644 nodes/simplify_opus/prompt.md create mode 100644 nodes/simplify_opus/provider_used.json create mode 100644 nodes/simplify_opus/response.md create mode 100644 nodes/simplify_opus/status.json diff --git a/checkpoint.json b/checkpoint.json index 70a5f0d48..793920819 100644 --- a/checkpoint.json +++ b/checkpoint.json @@ -1,15 +1,17 @@ { - "timestamp": "2026-03-21T14:35:45.956931Z", - "current_node": "implement", + "timestamp": "2026-03-21T14:45:27.209309Z", + "current_node": "simplify_opus", "completed_nodes": [ "start", "toolchain", "preflight_compile", "preflight_lint", - "implement" + "implement", + "simplify_opus" ], "node_retries": { "toolchain": 1, + "simplify_opus": 1, "preflight_compile": 1, "preflight_lint": 1, "implement": 1, @@ -18,30 +20,33 @@ "context_values": { "internal.run_id": "01KM8C0SVZW77C5W018CYEVE4Y", "graph.rankdir": "LR", - "command.output": "", - "internal.fidelity": "compact", - "last_stage": "implement", - "thread.start.current_node": "toolchain", - "internal.retry_count.preflight_compile": 1, + "last_stage": "simplify_opus", + "last_response": "Here's a summary of what was fixed:\n\n### Fixes Applied\n\n1. **Eliminated ~120 lines of duplicated code in `resume.rs`** (604 → 420 lines, -184 lines): The two code paths (checkpoint-file and git-bran", "response.implement": "These are all valid usages - the PR code uses `run_branch` from the manifest (different from the CLI arg), and resume.rs naturally uses `run_branch` as a local variable. The `run.rs` references are the engine's `RunConfig.run_branch` field which is still needed. All correct.\n\nLet me do a final summary of all the changes made:\n\n## Summary of Changes\n\n### New file: `lib/crates/fabro-cli/src/commands/resume.rs`\n- New `ResumeArgs` struct with `run`, `checkpoint`, `workflow`, and all shared run options\n- `resume_command()` function with two paths:\n - **Checkpoint path**: loads checkpoint file + workflow, runs engine locally\n - **Run ID path**: resolves run ID by prefix (like rewind/fork), reads checkpoint/graph from git metadata branch, resumes with worktree\n\n### Modified: `lib/crates/fabro-cli/src/commands/run.rs`\n- Removed `resume` and `run_branch` fields from `RunArgs`\n- Simplified `workflow` arg (no longer `required_unless_present`)\n- Simplified `preflight` conflicts (removed `resume`/`run_branch`)\n- Simplified `detach` conflicts (removed `resume`/`run_branch`)\n- Removed `run_branch` early-return at top of `run_command()`\n- Simplified engine call: always passes `None` for checkpoint\n- Removed entire `run_from_branch()` function (~320 lines)\n- Made 10 helper functions `pub(crate)`: `local_sandbox_with_callback`, `resolve_ssh_config`, `resolve_ssh_clone_params`, `resolve_exe_config`, `resolve_exe_clone_params`, `resolve_preserve_sandbox`, `print_final_output`, `print_assets`, `write_finalize_commit`, `generate_retro`\n\n### Modified: `lib/crates/fabro-cli/src/commands/mod.rs`\n- Added `pub mod resume;`\n\n### Modified: `lib/crates/fabro-cli/src/main.rs`\n- Added `Resume(commands::resume::ResumeArgs)` to `Command` enum\n- Added `Command::Resume(_) => \"resume\"` to command_name match\n- Added dispatch handler for `Command::Resume`\n- Removed `resume` and `run_branch` from `_run_engine` `RunArgs` reconstruction\n\n### Modified: `lib/crates/fabro-workflows/src/run_spec.rs`\n- Removed `resume` and `run_branch` fields from `RunSpec`\n- Added `#[serde(default)]` for backward compat with existing spec.json files\n- Added `Default` impl for `RunSpec`\n- Updated `sample_spec()` in tests\n\n### Modified: `lib/crates/fabro-cli/src/commands/create.rs`\n- Removed lines setting `resume` and `run_branch` in the spec\n\n### Modified: `lib/crates/fabro-cli/src/commands/start.rs`\n- Removed `resume` and `run_branch` from test spec construction\n\n### Modified: `lib/crates/fabro-cli/src/commands/rewind.rs`\n- Changed hint to `\"To resume: fabro resume {short_prefix}\"`\n\n### Modified: `lib/crates/fabro-cli/src/commands/fork.rs`\n- Changed hint to `\"To resume: fabro resume {short_prefix}\"`\n\n### Modified: `lib/crates/fabro-cli/tests/cli.rs`\n- Removed `detach_conflicts_with_resume` test\n- Removed `resume`/`run_branch` from spec JSON in test helpers\n- Added `resume_help_shows_expected_args` test\n- Added `resume_requires_run_or_checkpoint` test\n- Added `run_help_no_longer_shows_resume_or_run_branch` test\n\n### Documentation updates:\n- **`docs/reference/cli.mdx`**: Added `fabro resume` section, removed `--resume`/`--run-branch` from `fabro run`, updated rewind/fork examples\n- **`docs/execution/checkpoints.mdx`**: Updated all resume examples to use `fabro resume`\n- **`docs/core-concepts/how-fabro-works.mdx`**: Updated resume examples to use `fabro resume`", - "last_response": "These are all valid usages - the PR code uses `run_branch` from the manifest (different from the CLI arg), and resume.rs naturally uses `run_branch` as a local variable. The `run.rs` references are th", - "current_node": "implement", - "thread.preflight_compile.current_node": "preflight_lint", - "internal.thread_id": "preflight_lint", "internal.retry_count.start": 1, - "thread.toolchain.current_node": "preflight_compile", "failure_signature": "", - "outcome": "success", + "internal.retry_count.toolchain": 1, "internal.node_visit_count": 1, "failure_class": "", - "internal.retry_count.toolchain": 1, - "thread.preflight_lint.current_node": "implement", "graph.goal": "# Plan: Extract `fabro resume` subcommand\n\n## Context\n\nResume functionality is currently embedded in `fabro run` via `--resume` (checkpoint file) and `--run-branch` (git branch). This makes the `run` command's arg surface complex with `conflicts_with` annotations, and the UX is unintuitive — users must construct `fabro/run/RUN_ID` branch names manually. The new `fabro resume` subcommand provides a cleaner interface: `fabro resume RUN_ID_OR_PREFIX`.\n\n## New `ResumeArgs` struct\n\n```rust\npub struct ResumeArgs {\n /// Run ID, prefix, or branch (fabro/run/...)\n #[arg(required_unless_present = \"checkpoint\")]\n pub run: Option,\n\n /// Resume from a checkpoint file (requires --workflow)\n #[arg(long)]\n pub checkpoint: Option,\n\n /// Override workflow graph (required with --checkpoint)\n #[arg(long)]\n pub workflow: Option,\n\n // Shared run options: run_dir, dry_run, auto_approve, goal, goal_file,\n // model, provider, verbose, sandbox, no_retro, ssh, preserve_sandbox\n}\n```\n\n**Run ID resolution** (at top of `resume_command()`):\n- If `run` starts with `fabro/run/` → strip prefix to get run_id\n- Otherwise → call `find_run_id_by_prefix(&repo, &run)` (same as `rewind`/`fork`)\n- Then construct branch name as `fabro/run/{run_id}`\n\n## Files to modify\n\n### 1. New: `lib/crates/fabro-cli/src/commands/resume.rs`\n- Define `ResumeArgs` struct\n- Move `run_from_branch()` body (~315 lines, `run.rs:1811-2125`) into `pub async fn resume_command()`\n- Add run ID resolution logic at top (prefix → full ID via `find_run_id_by_prefix`)\n- Add `--checkpoint` path: validate `--workflow` is present, load graph via `prepare_from_file()`, load checkpoint via `Checkpoint::load()`, then run engine\n\n### 2. `lib/crates/fabro-cli/src/commands/run.rs`\n- **Remove from `RunArgs`**: `resume` field (line 97-99), `run_branch` field (line 101-103)\n- **Simplify `workflow`**: remove `required_unless_present = \"run_branch\"` — it's now always required\n- **Update `conflicts_with_all`**: remove `\"resume\"`/`\"run_branch\"` from `preflight` (line 90) and `detach` (line 146)\n- **Remove** `run_from_branch()` function (lines 1811-2125)\n- **Remove** the `run_branch` early-return at top of `run_command()` (lines 602-604)\n- **Simplify** engine call: remove `if let Some(ref checkpoint_path) = args.resume` branch (lines 1467-1476), always pass `None` for checkpoint\n- **Widen visibility** of helpers used by `resume.rs`:\n - `local_sandbox_with_callback` (line 439) → `pub(crate)`\n - `resolve_ssh_config` (line 341) → `pub(crate)`\n - `resolve_ssh_clone_params` (line 355) → `pub(crate)`\n - `resolve_exe_config` (line 313) → `pub(crate)`\n - `resolve_exe_clone_params` (line 328) → `pub(crate)`\n - `resolve_preserve_sandbox` (line 261) → `pub(crate)`\n - `generate_retro` (line 2560) → `pub(crate)`\n - `write_finalize_commit` (line 2523) → `pub(crate)`\n - `print_final_output` (line 2128) → `pub(crate)`\n - `print_assets` (line 2149) → `pub(crate)`\n\n### 3. `lib/crates/fabro-cli/src/commands/mod.rs`\n- Add `pub mod resume;`\n\n### 4. `lib/crates/fabro-cli/src/main.rs`\n- Add `Resume(commands::resume::ResumeArgs)` to `Command` enum (near line 170, alongside `Rewind`/`Fork`)\n- Add `Command::Resume(_) => \"resume\"` to command_name match\n- Add dispatch handler (pattern follows `Rewind`/`Fork`/`Wait` — create styles, load cli_config, build github_app/git_author, call `resume_command()`)\n\n### 5. `lib/crates/fabro-workflows/src/run_spec.rs`\n- Remove `resume` and `run_branch` fields from `RunSpec`\n- Add `#[serde(default)]` to `RunSpec` for backward compat with existing `spec.json` files\n- Update `sample_spec()` in tests\n\n### 6. `lib/crates/fabro-cli/src/commands/create.rs`\n- Remove lines 86-87 that set `resume` and `run_branch` in the spec\n\n### 7. `lib/crates/fabro-cli/src/main.rs` (`_run_engine` handler)\n- Remove lines setting `resume` and `run_branch` when reconstructing `RunArgs` from `RunSpec`\n\n### 8. `lib/crates/fabro-cli/src/commands/rewind.rs` (line 48-52)\n- Change hint: `\"To resume: fabro resume {run_id}\"` (use short prefix)\n\n### 9. `lib/crates/fabro-cli/src/commands/fork.rs` (line 56-60)\n- Change hint: `\"To resume: fabro resume {new_run_id}\"` (use short prefix)\n\n### 10. `lib/crates/fabro-cli/tests/cli.rs`\n- Update/remove tests referencing `--resume` or `--run-branch` on `fabro run`\n- Add basic parse test for `fabro resume`\n\n### 11. Documentation (`docs/`)\n- Update `docs/reference/cli.mdx`: add `fabro resume` section, remove `--resume`/`--run-branch` from `fabro run`\n- Update `docs/execution/checkpoints.mdx`: change resume examples\n- Update any other docs referencing `fabro run --run-branch` or `fabro run --resume`\n\n## Verification\n\n1. `cargo build --workspace` — compiles cleanly\n2. `cargo test --workspace` — all tests pass\n3. `cargo clippy --workspace -- -D warnings` — no warnings\n4. Manual: `fabro resume --help` shows expected args\n5. Manual: `fabro run --help` no longer shows `--resume` or `--run-branch`\n", - "graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ", - "current.preamble": "Goal: # Plan: Extract `fabro resume` subcommand\n\n## Context\n\nResume functionality is currently embedded in `fabro run` via `--resume` (checkpoint file) and `--run-branch` (git branch). This makes the `run` command's arg surface complex with `conflicts_with` annotations, and the UX is unintuitive — users must construct `fabro/run/RUN_ID` branch names manually. The new `fabro resume` subcommand provides a cleaner interface: `fabro resume RUN_ID_OR_PREFIX`.\n\n## New `ResumeArgs` struct\n\n```rust\npub struct ResumeArgs {\n /// Run ID, prefix, or branch (fabro/run/...)\n #[arg(required_unless_present = \"checkpoint\")]\n pub run: Option,\n\n /// Resume from a checkpoint file (requires --workflow)\n #[arg(long)]\n pub checkpoint: Option,\n\n /// Override workflow graph (required with --checkpoint)\n #[arg(long)]\n pub workflow: Option,\n\n // Shared run options: run_dir, dry_run, auto_approve, goal, goal_file,\n // model, provider, verbose, sandbox, no_retro, ssh, preserve_sandbox\n}\n```\n\n**Run ID resolution** (at top of `resume_command()`):\n- If `run` starts with `fabro/run/` → strip prefix to get run_id\n- Otherwise → call `find_run_id_by_prefix(&repo, &run)` (same as `rewind`/`fork`)\n- Then construct branch name as `fabro/run/{run_id}`\n\n## Files to modify\n\n### 1. New: `lib/crates/fabro-cli/src/commands/resume.rs`\n- Define `ResumeArgs` struct\n- Move `run_from_branch()` body (~315 lines, `run.rs:1811-2125`) into `pub async fn resume_command()`\n- Add run ID resolution logic at top (prefix → full ID via `find_run_id_by_prefix`)\n- Add `--checkpoint` path: validate `--workflow` is present, load graph via `prepare_from_file()`, load checkpoint via `Checkpoint::load()`, then run engine\n\n### 2. `lib/crates/fabro-cli/src/commands/run.rs`\n- **Remove from `RunArgs`**: `resume` field (line 97-99), `run_branch` field (line 101-103)\n- **Simplify `workflow`**: remove `required_unless_present = \"run_branch\"` — it's now always required\n- **Update `conflicts_with_all`**: remove `\"resume\"`/`\"run_branch\"` from `preflight` (line 90) and `detach` (line 146)\n- **Remove** `run_from_branch()` function (lines 1811-2125)\n- **Remove** the `run_branch` early-return at top of `run_command()` (lines 602-604)\n- **Simplify** engine call: remove `if let Some(ref checkpoint_path) = args.resume` branch (lines 1467-1476), always pass `None` for checkpoint\n- **Widen visibility** of helpers used by `resume.rs`:\n - `local_sandbox_with_callback` (line 439) → `pub(crate)`\n - `resolve_ssh_config` (line 341) → `pub(crate)`\n - `resolve_ssh_clone_params` (line 355) → `pub(crate)`\n - `resolve_exe_config` (line 313) → `pub(crate)`\n - `resolve_exe_clone_params` (line 328) → `pub(crate)`\n - `resolve_preserve_sandbox` (line 261) → `pub(crate)`\n - `generate_retro` (line 2560) → `pub(crate)`\n - `write_finalize_commit` (line 2523) → `pub(crate)`\n - `print_final_output` (line 2128) → `pub(crate)`\n - `print_assets` (line 2149) → `pub(crate)`\n\n### 3. `lib/crates/fabro-cli/src/commands/mod.rs`\n- Add `pub mod resume;`\n\n### 4. `lib/crates/fabro-cli/src/main.rs`\n- Add `Resume(commands::resume::ResumeArgs)` to `Command` enum (near line 170, alongside `Rewind`/`Fork`)\n- Add `Command::Resume(_) => \"resume\"` to command_name match\n- Add dispatch handler (pattern follows `Rewind`/`Fork`/`Wait` — create styles, load cli_config, build github_app/git_author, call `resume_command()`)\n\n### 5. `lib/crates/fabro-workflows/src/run_spec.rs`\n- Remove `resume` and `run_branch` fields from `RunSpec`\n- Add `#[serde(default)]` to `RunSpec` for backward compat with existing `spec.json` files\n- Update `sample_spec()` in tests\n\n### 6. `lib/crates/fabro-cli/src/commands/create.rs`\n- Remove lines 86-87 that set `resume` and `run_branch` in the spec\n\n### 7. `lib/crates/fabro-cli/src/main.rs` (`_run_engine` handler)\n- Remove lines setting `resume` and `run_branch` when reconstructing `RunArgs` from `RunSpec`\n\n### 8. `lib/crates/fabro-cli/src/commands/rewind.rs` (line 48-52)\n- Change hint: `\"To resume: fabro resume {run_id}\"` (use short prefix)\n\n### 9. `lib/crates/fabro-cli/src/commands/fork.rs` (line 56-60)\n- Change hint: `\"To resume: fabro resume {new_run_id}\"` (use short prefix)\n\n### 10. `lib/crates/fabro-cli/tests/cli.rs`\n- Update/remove tests referencing `--resume` or `--run-branch` on `fabro run`\n- Add basic parse test for `fabro resume`\n\n### 11. Documentation (`docs/`)\n- Update `docs/reference/cli.mdx`: add `fabro resume` section, remove `--resume`/`--run-branch` from `fabro run`\n- Update `docs/execution/checkpoints.mdx`: change resume examples\n- Update any other docs referencing `fabro run --run-branch` or `fabro run --resume`\n\n## Verification\n\n1. `cargo build --workspace` — compiles cleanly\n2. `cargo test --workspace` — all tests pass\n3. `cargo clippy --workspace -- -D warnings` — no warnings\n4. Manual: `fabro resume --help` shows expected args\n5. Manual: `fabro run --help` no longer shows `--resume` or `--run-branch`\n\n\n## Completed stages\n- **toolchain**: success\n - 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`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n", - "command.stderr": "", "internal.retry_count.preflight_lint": 1, - "internal.retry_count.implement": 1 + "response.simplify_opus": "Here's a summary of what was fixed:\n\n### Fixes Applied\n\n1. **Eliminated ~120 lines of duplicated code in `resume.rs`** (604 → 420 lines, -184 lines): The two code paths (checkpoint-file and git-branch) had nearly identical blocks for: interviewer creation, dry-run detection, model/provider resolution, engine construction, retro generation, result printing, and exit code logic. Restructured into three functions:\n - `prepare_from_checkpoint()` — loads checkpoint/graph from files, creates simple local sandbox\n - `prepare_from_branch()` — resolves run ID from git, reads metadata, sets up worktree sandbox\n - `run_resumed()` — shared tail handling engine setup, execution, retro, and result display\n - A `ResumeContext` struct carries the divergent state between the two paths\n\n2. **Extracted `default_run_dir()` helper in `run.rs`** — The run-dir defaulting logic (`~/.fabro/runs/YYYYMMDD-RUNID` or `/tmp/fabro-dry-run/RUNID`) was duplicated 4 times across `run.rs`, `create.rs`, and resume.rs. Now it's a single `pub(crate)` function reused by all three.\n\n3. **Used `resolve_model_provider()` in resume.rs** — The original code had an inline simplified version that skipped model alias expansion (e.g., `--model opus` wouldn't resolve to `claude-opus-4-6`) and ignored graph `default_model`/`default_provider` attributes. Now it calls the full precedence chain through the existing tested helper.\n\n4. **Simplified `RunSpec` Default impl** — Replaced the manual 16-field `impl Default for RunSpec` with `#[derive(Default)]` since all fields (`String`, `PathBuf`, `HashMap`, `bool`, `Option`) have built-in Default impls.\n\n5. **Fixed missing trailing newlines** in `docs/core-concepts/how-fabro-works.mdx` and `docs/reference/cli.mdx`.\n\n### Verification\n- `cargo build --workspace` ✅\n- `cargo test --workspace` ✅ (all tests pass)\n- `cargo clippy --workspace -- -D warnings` ✅ (no warnings)\n- `cargo fmt --check --all` ✅", + "command.output": "", + "internal.fidelity": "compact", + "thread.start.current_node": "toolchain", + "internal.retry_count.preflight_compile": 1, + "internal.retry_count.simplify_opus": 1, + "current_node": "simplify_opus", + "thread.preflight_compile.current_node": "preflight_lint", + "internal.thread_id": "implement", + "thread.toolchain.current_node": "preflight_compile", + "outcome": "success", + "thread.preflight_lint.current_node": "implement", + "graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ", + "current.preamble": "Goal: # Plan: Extract `fabro resume` subcommand\n\n## Context\n\nResume functionality is currently embedded in `fabro run` via `--resume` (checkpoint file) and `--run-branch` (git branch). This makes the `run` command's arg surface complex with `conflicts_with` annotations, and the UX is unintuitive — users must construct `fabro/run/RUN_ID` branch names manually. The new `fabro resume` subcommand provides a cleaner interface: `fabro resume RUN_ID_OR_PREFIX`.\n\n## New `ResumeArgs` struct\n\n```rust\npub struct ResumeArgs {\n /// Run ID, prefix, or branch (fabro/run/...)\n #[arg(required_unless_present = \"checkpoint\")]\n pub run: Option,\n\n /// Resume from a checkpoint file (requires --workflow)\n #[arg(long)]\n pub checkpoint: Option,\n\n /// Override workflow graph (required with --checkpoint)\n #[arg(long)]\n pub workflow: Option,\n\n // Shared run options: run_dir, dry_run, auto_approve, goal, goal_file,\n // model, provider, verbose, sandbox, no_retro, ssh, preserve_sandbox\n}\n```\n\n**Run ID resolution** (at top of `resume_command()`):\n- If `run` starts with `fabro/run/` → strip prefix to get run_id\n- Otherwise → call `find_run_id_by_prefix(&repo, &run)` (same as `rewind`/`fork`)\n- Then construct branch name as `fabro/run/{run_id}`\n\n## Files to modify\n\n### 1. New: `lib/crates/fabro-cli/src/commands/resume.rs`\n- Define `ResumeArgs` struct\n- Move `run_from_branch()` body (~315 lines, `run.rs:1811-2125`) into `pub async fn resume_command()`\n- Add run ID resolution logic at top (prefix → full ID via `find_run_id_by_prefix`)\n- Add `--checkpoint` path: validate `--workflow` is present, load graph via `prepare_from_file()`, load checkpoint via `Checkpoint::load()`, then run engine\n\n### 2. `lib/crates/fabro-cli/src/commands/run.rs`\n- **Remove from `RunArgs`**: `resume` field (line 97-99), `run_branch` field (line 101-103)\n- **Simplify `workflow`**: remove `required_unless_present = \"run_branch\"` — it's now always required\n- **Update `conflicts_with_all`**: remove `\"resume\"`/`\"run_branch\"` from `preflight` (line 90) and `detach` (line 146)\n- **Remove** `run_from_branch()` function (lines 1811-2125)\n- **Remove** the `run_branch` early-return at top of `run_command()` (lines 602-604)\n- **Simplify** engine call: remove `if let Some(ref checkpoint_path) = args.resume` branch (lines 1467-1476), always pass `None` for checkpoint\n- **Widen visibility** of helpers used by `resume.rs`:\n - `local_sandbox_with_callback` (line 439) → `pub(crate)`\n - `resolve_ssh_config` (line 341) → `pub(crate)`\n - `resolve_ssh_clone_params` (line 355) → `pub(crate)`\n - `resolve_exe_config` (line 313) → `pub(crate)`\n - `resolve_exe_clone_params` (line 328) → `pub(crate)`\n - `resolve_preserve_sandbox` (line 261) → `pub(crate)`\n - `generate_retro` (line 2560) → `pub(crate)`\n - `write_finalize_commit` (line 2523) → `pub(crate)`\n - `print_final_output` (line 2128) → `pub(crate)`\n - `print_assets` (line 2149) → `pub(crate)`\n\n### 3. `lib/crates/fabro-cli/src/commands/mod.rs`\n- Add `pub mod resume;`\n\n### 4. `lib/crates/fabro-cli/src/main.rs`\n- Add `Resume(commands::resume::ResumeArgs)` to `Command` enum (near line 170, alongside `Rewind`/`Fork`)\n- Add `Command::Resume(_) => \"resume\"` to command_name match\n- Add dispatch handler (pattern follows `Rewind`/`Fork`/`Wait` — create styles, load cli_config, build github_app/git_author, call `resume_command()`)\n\n### 5. `lib/crates/fabro-workflows/src/run_spec.rs`\n- Remove `resume` and `run_branch` fields from `RunSpec`\n- Add `#[serde(default)]` to `RunSpec` for backward compat with existing `spec.json` files\n- Update `sample_spec()` in tests\n\n### 6. `lib/crates/fabro-cli/src/commands/create.rs`\n- Remove lines 86-87 that set `resume` and `run_branch` in the spec\n\n### 7. `lib/crates/fabro-cli/src/main.rs` (`_run_engine` handler)\n- Remove lines setting `resume` and `run_branch` when reconstructing `RunArgs` from `RunSpec`\n\n### 8. `lib/crates/fabro-cli/src/commands/rewind.rs` (line 48-52)\n- Change hint: `\"To resume: fabro resume {run_id}\"` (use short prefix)\n\n### 9. `lib/crates/fabro-cli/src/commands/fork.rs` (line 56-60)\n- Change hint: `\"To resume: fabro resume {new_run_id}\"` (use short prefix)\n\n### 10. `lib/crates/fabro-cli/tests/cli.rs`\n- Update/remove tests referencing `--resume` or `--run-branch` on `fabro run`\n- Add basic parse test for `fabro resume`\n\n### 11. Documentation (`docs/`)\n- Update `docs/reference/cli.mdx`: add `fabro resume` section, remove `--resume`/`--run-branch` from `fabro run`\n- Update `docs/execution/checkpoints.mdx`: change resume examples\n- Update any other docs referencing `fabro run --run-branch` or `fabro run --resume`\n\n## Verification\n\n1. `cargo build --workspace` — compiles cleanly\n2. `cargo test --workspace` — all tests pass\n3. `cargo clippy --workspace -- -D warnings` — no warnings\n4. Manual: `fabro resume --help` shows expected args\n5. Manual: `fabro run --help` no longer shows `--resume` or `--run-branch`\n\n\n## Completed stages\n- **toolchain**: success\n - 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`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **implement**: success\n - Model: claude-opus-4-6, 91.6k tokens in / 34.3k out\n - Files: /home/daytona/workspace/docs/core-concepts/how-fabro-works.mdx, /home/daytona/workspace/docs/execution/checkpoints.mdx, /home/daytona/workspace/docs/reference/cli.mdx, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/fork.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/mod.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/resume.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/rewind.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.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-cli/tests/cli.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/run_spec.rs\n", + "command.stderr": "", + "internal.retry_count.implement": 1, + "thread.implement.current_node": "simplify_opus" }, "logs": [], "node_outcomes": { @@ -101,6 +106,33 @@ "notes": "Script completed: cargo check -q --workspace 2>&1", "duration_ms": 70163 }, + "simplify_opus": { + "status": "success", + "context_updates": { + "last_response": "Here's a summary of what was fixed:\n\n### Fixes Applied\n\n1. **Eliminated ~120 lines of duplicated code in `resume.rs`** (604 → 420 lines, -184 lines): The two code paths (checkpoint-file and git-bran", + "response.simplify_opus": "Here's a summary of what was fixed:\n\n### Fixes Applied\n\n1. **Eliminated ~120 lines of duplicated code in `resume.rs`** (604 → 420 lines, -184 lines): The two code paths (checkpoint-file and git-branch) had nearly identical blocks for: interviewer creation, dry-run detection, model/provider resolution, engine construction, retro generation, result printing, and exit code logic. Restructured into three functions:\n - `prepare_from_checkpoint()` — loads checkpoint/graph from files, creates simple local sandbox\n - `prepare_from_branch()` — resolves run ID from git, reads metadata, sets up worktree sandbox\n - `run_resumed()` — shared tail handling engine setup, execution, retro, and result display\n - A `ResumeContext` struct carries the divergent state between the two paths\n\n2. **Extracted `default_run_dir()` helper in `run.rs`** — The run-dir defaulting logic (`~/.fabro/runs/YYYYMMDD-RUNID` or `/tmp/fabro-dry-run/RUNID`) was duplicated 4 times across `run.rs`, `create.rs`, and resume.rs. Now it's a single `pub(crate)` function reused by all three.\n\n3. **Used `resolve_model_provider()` in resume.rs** — The original code had an inline simplified version that skipped model alias expansion (e.g., `--model opus` wouldn't resolve to `claude-opus-4-6`) and ignored graph `default_model`/`default_provider` attributes. Now it calls the full precedence chain through the existing tested helper.\n\n4. **Simplified `RunSpec` Default impl** — Replaced the manual 16-field `impl Default for RunSpec` with `#[derive(Default)]` since all fields (`String`, `PathBuf`, `HashMap`, `bool`, `Option`) have built-in Default impls.\n\n5. **Fixed missing trailing newlines** in `docs/core-concepts/how-fabro-works.mdx` and `docs/reference/cli.mdx`.\n\n### Verification\n- `cargo build --workspace` ✅\n- `cargo test --workspace` ✅ (all tests pass)\n- `cargo clippy --workspace -- -D warnings` ✅ (no warnings)\n- `cargo fmt --check --all` ✅", + "last_stage": "simplify_opus" + }, + "notes": "Stage completed: simplify_opus", + "usage": { + "model": "claude-opus-4-6", + "input_tokens": 78418, + "output_tokens": 20993, + "cache_read_tokens": 1885952, + "cache_write_tokens": 87627, + "reasoning_tokens": 1099, + "cost": 2.750745 + }, + "files_touched": [ + "/home/daytona/workspace/docs/core-concepts/how-fabro-works.mdx", + "/home/daytona/workspace/docs/reference/cli.mdx", + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs", + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/resume.rs", + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs", + "/home/daytona/workspace/lib/crates/fabro-workflows/src/run_spec.rs" + ], + "duration_ms": 578632 + }, "preflight_lint": { "status": "success", "context_updates": { @@ -111,9 +143,10 @@ "duration_ms": 13195 } }, - "next_node_id": "simplify_opus", + "next_node_id": "simplify_gpt", "node_visits": { "preflight_lint": 1, + "simplify_opus": 1, "implement": 1, "start": 1, "toolchain": 1, diff --git a/nodes/implement/diff.patch b/nodes/implement/diff.patch new file mode 100644 index 000000000..7e97a8015 --- /dev/null +++ b/nodes/implement/diff.patch @@ -0,0 +1,1512 @@ +diff --git a/docs/core-concepts/how-fabro-works.mdx b/docs/core-concepts/how-fabro-works.mdx +index 2fe9c59a..b190d19b 100644 +--- a/docs/core-concepts/how-fabro-works.mdx ++++ b/docs/core-concepts/how-fabro-works.mdx +@@ -95,13 +95,13 @@ See [Observability](/execution/observability) for more on querying run data. + Because Fabro checkpoints after every stage, interrupted runs can be resumed from where they left off: + + ```bash +-fabro run --resume path/to/checkpoint.json ++fabro resume + ``` + +-Or resume from a git run branch: ++Or resume from a checkpoint file: + + ```bash +-fabro run --run-branch fabro/runs/abc123 ++fabro resume --checkpoint path/to/checkpoint.json --workflow workflow.fabro + ``` + +-The engine restores the full context, node visit counts, and retry state, then continues execution from the next node. ++The engine restores the full context, node visit counts, and retry state, then continues execution from the next node. +\ No newline at end of file +diff --git a/docs/execution/checkpoints.mdx b/docs/execution/checkpoints.mdx +index 1923f9f7..4f04a71a 100644 +--- a/docs/execution/checkpoints.mdx ++++ b/docs/execution/checkpoints.mdx +@@ -98,7 +98,7 @@ There are two ways to resume an interrupted run: + Resume from a `checkpoint.json` saved in the run directory: + + ```bash +-fabro run workflow.fabro --resume path/to/logs/checkpoint.json ++fabro resume --checkpoint path/to/logs/checkpoint.json --workflow workflow.fabro + ``` + + Fabro loads the checkpoint, restores the context and execution state, and continues from the next node after the checkpoint. +@@ -108,7 +108,7 @@ Fabro loads the checkpoint, restores the context and execution state, and contin + Resume from the Git branches created during a previous run: + + ```bash +-fabro run --run-branch fabro/run/01JKXYZ... ++fabro resume 01JKXYZ + ``` + + This reads the checkpoint, manifest, and Graphviz graph from the metadata branch (`fabro/meta/01JKXYZ...`), re-attaches a worktree to the existing run branch, and resumes execution. No workflow file argument is needed — everything is recovered from Git. +@@ -163,7 +163,7 @@ fabro rewind --list + fabro rewind plan@2 + + # Resume from the rewound point +-fabro run --run-branch fabro/run/ ++fabro resume + ``` + + See [`fabro rewind`](/reference/cli#fabro-rewind) for the full command reference. +@@ -180,7 +180,7 @@ fabro fork --list + fabro fork plan@2 + + # Resume the forked run +-fabro run --run-branch fabro/run/ ++fabro resume + ``` + + Use **rewind** when you want to redo a run from an earlier point (destructive — resets the original). Use **fork** when you want to try a different approach while keeping the original run as a reference. +diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx +index e07dc2da..3f108e58 100644 +--- a/docs/reference/cli.mdx ++++ b/docs/reference/cli.mdx +@@ -42,18 +42,15 @@ Launch a workflow from a `.fabro` workflow file or `.toml` task config. + ```bash + fabro run + fabro run run.toml +-fabro run --run-branch fabro/run/abc123 + ``` + + | Argument / Flag | Description | + |---|---| +-| `` | Path to a `.fabro` workflow file, `.toml` task config, or workflow name (resolved from `fabro/workflows/` in the project, then `~/.fabro/workflows/`). Not required when using `--run-branch`. | ++| `` | Path to a `.fabro` workflow file, `.toml` task config, or workflow name (resolved from `fabro/workflows/` in the project, then `~/.fabro/workflows/`). | + | `--run-dir ` | Run output directory | + | `--dry-run` | Execute with a simulated LLM backend | + | `--preflight` | Validate run configuration without executing | + | `--auto-approve` | Auto-approve all human gates | +-| `--resume ` | Resume from a checkpoint file | +-| `--run-branch ` | Resume from a git run branch (reads checkpoint and graph from metadata branch) | + | `--model ` | Override default LLM model | + | `--provider ` | Override default LLM provider | + | `-v, --verbose` | Enable verbose output | +@@ -67,9 +64,37 @@ fabro run --run-branch fabro/run/abc123 + | `-d, --detach` | Fork the workflow as a background process and print the run ID. Reconnect later with `fabro logs -f`. | + + +-`--preflight` conflicts with `--resume`, `--run-branch`, `--dry-run`, and `--detach`. `--run-branch` conflicts with `--resume`. ++`--preflight` conflicts with `--dry-run` and `--detach`. + + ++## `fabro resume` ++ ++Resume an interrupted workflow run from its last checkpoint. ++ ++```bash ++fabro resume ++fabro resume --workflow updated.fabro ++fabro resume --checkpoint path/to/checkpoint.json --workflow workflow.fabro ++``` ++ ++| Argument / Flag | Description | ++|---|---| ++| `` | Run ID, prefix, or branch (`fabro/run/...`). Not required when using `--checkpoint`. | ++| `--checkpoint ` | Resume from a checkpoint file (requires `--workflow`) | ++| `--workflow ` | Override workflow graph (required with `--checkpoint`) | ++| `--run-dir ` | Run output directory | ++| `--dry-run` | Execute with a simulated LLM backend | ++| `--auto-approve` | Auto-approve all human gates | ++| `--model ` | Override default LLM model | ++| `--provider ` | Override default LLM provider | ++| `-v, --verbose` | Enable verbose output | ++| `--sandbox ` | Sandbox for agent tools: `local`, `docker`, `daytona`, `ssh`, or `exe` | ++| `--goal ` | Override the workflow goal | ++| `--goal-file ` | Read the goal from a file | ++| `--no-retro` | Skip retro generation after the run | ++| `--ssh` | Create SSH access to the sandbox | ++| `--preserve-sandbox` | Keep the sandbox alive after the run finishes | ++ + ## `fabro ps` + + List workflow runs. By default, shows only active (running) runs — similar to `docker ps`. Use `-a` to include completed runs. +@@ -399,7 +424,7 @@ fabro skill install --for user --dir claude + + ## `fabro rewind` + +-Rewind a workflow run to an earlier checkpoint. This resets both the run branch and metadata branch refs so that `fabro run --run-branch` resumes from the target checkpoint. ++Rewind a workflow run to an earlier checkpoint. This resets both the run branch and metadata branch refs so that `fabro resume` continues from the target checkpoint. + + ```bash + fabro rewind [TARGET] +@@ -424,7 +449,7 @@ Target formats: + After rewinding, resume from the earlier point: + + ```bash +-fabro run --run-branch fabro/run/ ++fabro resume + ``` + + See [Checkpoints](/execution/checkpoints#rewinding-to-an-earlier-checkpoint) for background on how checkpointing works. +@@ -448,7 +473,7 @@ fabro fork --list + Target formats are the same as [`fabro rewind`](#fabro-rewind). After forking, resume the new run: + + ```bash +-fabro run --run-branch fabro/run/ ++fabro resume + ``` + + See [Checkpoints — Forking a run](/execution/checkpoints#forking-a-run) for when to use fork vs. rewind. +@@ -786,4 +811,4 @@ Open the Fabro Discord community invite in your default browser. + + ```bash + fabro discord +-``` ++``` +\ No newline at end of file +diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs +index a226f2c0..6a0123ca 100644 +--- a/lib/crates/fabro-cli/src/commands/create.rs ++++ b/lib/crates/fabro-cli/src/commands/create.rs +@@ -83,8 +83,6 @@ pub async fn create_run( + preserve_sandbox: args.preserve_sandbox, + dry_run: args.dry_run, + auto_approve: args.auto_approve, +- resume: args.resume.clone(), +- run_branch: args.run_branch.clone(), + }; + spec.save(&run_dir)?; + +diff --git a/lib/crates/fabro-cli/src/commands/fork.rs b/lib/crates/fabro-cli/src/commands/fork.rs +index d17f67cb..06436c77 100644 +--- a/lib/crates/fabro-cli/src/commands/fork.rs ++++ b/lib/crates/fabro-cli/src/commands/fork.rs +@@ -54,9 +54,8 @@ pub fn run(args: &ForkArgs, styles: &Styles) -> Result<()> { + &new_run_id[..8.min(new_run_id.len())] + ); + eprintln!( +- "To resume: fabro run --run-branch {}{}", +- fabro_workflows::git::RUN_BRANCH_PREFIX, +- new_run_id ++ "To resume: fabro resume {}", ++ &new_run_id[..8.min(new_run_id.len())] + ); + + Ok(()) +diff --git a/lib/crates/fabro-cli/src/commands/mod.rs b/lib/crates/fabro-cli/src/commands/mod.rs +index 64dea84b..e925eb06 100644 +--- a/lib/crates/fabro-cli/src/commands/mod.rs ++++ b/lib/crates/fabro-cli/src/commands/mod.rs +@@ -11,6 +11,7 @@ pub mod parse; + pub mod pr; + pub mod preview; + pub mod provider; ++pub mod resume; + pub mod rewind; + pub mod run; + pub(crate) mod run_progress; +diff --git a/lib/crates/fabro-cli/src/commands/resume.rs b/lib/crates/fabro-cli/src/commands/resume.rs +new file mode 100644 +index 00000000..3bd9cb71 +--- /dev/null ++++ b/lib/crates/fabro-cli/src/commands/resume.rs +@@ -0,0 +1,604 @@ ++use std::collections::HashMap; ++use std::path::PathBuf; ++use std::sync::Arc; ++use std::time::{Duration, Instant}; ++ ++use anyhow::{bail, Context}; ++use clap::Args; ++use fabro_agent::{Sandbox, WorktreeConfig, WorktreeSandbox}; ++use fabro_config::run::RunDefaults; ++use fabro_interview::{AutoApproveInterviewer, ConsoleInterviewer, Interviewer}; ++use fabro_model::Provider; ++use fabro_util::terminal::Styles; ++use fabro_workflows::backend::{AgentApiBackend, AgentCliBackend, BackendRouter}; ++use fabro_workflows::checkpoint::Checkpoint; ++use fabro_workflows::engine::RunConfig; ++use fabro_workflows::event::EventEmitter; ++use fabro_workflows::outcome::StageStatus; ++use fabro_workflows::sandbox_provider::SandboxProvider; ++use indicatif::HumanDuration; ++ ++use super::run::{ ++ apply_goal_override, generate_retro, local_sandbox_with_callback, print_assets, ++ print_final_output, resolve_cli_goal, resolve_sandbox_provider, resolve_ssh_clone_params, ++ resolve_ssh_config, write_finalize_commit, CliSandboxProvider, ++}; ++use crate::commands::shared::{print_diagnostics, tilde_path}; ++use fabro_config::project as project_config; ++use fabro_validate::Severity; ++ ++#[derive(Debug, Args)] ++pub struct ResumeArgs { ++ /// Run ID, prefix, or branch (fabro/run/...) ++ #[arg(required_unless_present = "checkpoint")] ++ pub run: Option, ++ ++ /// Resume from a checkpoint file (requires --workflow) ++ #[arg(long)] ++ pub checkpoint: Option, ++ ++ /// Override workflow graph (required with --checkpoint) ++ #[arg(long)] ++ pub workflow: Option, ++ ++ /// Run output directory ++ #[arg(long)] ++ pub run_dir: Option, ++ ++ /// Execute with simulated LLM backend ++ #[arg(long)] ++ pub dry_run: bool, ++ ++ /// Auto-approve all human gates ++ #[arg(long)] ++ pub auto_approve: bool, ++ ++ /// Override the workflow goal (exposed as $goal in prompts) ++ #[arg(long)] ++ pub goal: Option, ++ ++ /// Read the workflow goal from a file ++ #[arg(long, conflicts_with = "goal")] ++ pub goal_file: Option, ++ ++ /// Override default LLM model ++ #[arg(long)] ++ pub model: Option, ++ ++ /// Override default LLM provider ++ #[arg(long)] ++ pub provider: Option, ++ ++ /// Enable verbose output ++ #[arg(short, long)] ++ pub verbose: bool, ++ ++ /// Sandbox for agent tools ++ #[arg(long, value_enum)] ++ pub sandbox: Option, ++ ++ /// Skip retro generation after the run ++ #[arg(long)] ++ pub no_retro: bool, ++ ++ /// Create SSH access to the Daytona sandbox and print the connection command ++ #[arg(long)] ++ pub ssh: bool, ++ ++ /// Keep the sandbox alive after the run finishes (for debugging) ++ #[arg(long)] ++ pub preserve_sandbox: bool, ++} ++ ++/// Resume an interrupted workflow run. ++/// ++/// # Errors ++/// ++/// Returns an error if the run cannot be found, the checkpoint cannot be loaded, ++/// or the workflow cannot be resumed. ++pub async fn resume_command( ++ args: ResumeArgs, ++ run_defaults: RunDefaults, ++ styles: &'static Styles, ++ github_app: Option, ++ git_author: fabro_workflows::git::GitAuthor, ++) -> anyhow::Result<()> { ++ // Checkpoint-file path: load checkpoint and graph from files ++ if let Some(ref checkpoint_path) = args.checkpoint { ++ let workflow_path = args ++ .workflow ++ .as_ref() ++ .ok_or_else(|| anyhow::anyhow!("--workflow is required when using --checkpoint"))?; ++ let checkpoint = Checkpoint::load(checkpoint_path)?; ++ let (mut graph, diagnostics) = fabro_workflows::workflow::prepare_from_file(workflow_path)?; ++ ++ let cli_goal = resolve_cli_goal(&args.goal, &args.goal_file)?; ++ apply_goal_override(&mut graph, cli_goal.as_deref(), None); ++ ++ eprintln!( ++ "{} {} from checkpoint {}", ++ styles.bold.apply_to("Resuming workflow:"), ++ graph.name, ++ styles.dim.apply_to(checkpoint_path.display()), ++ ); ++ ++ print_diagnostics(&diagnostics, styles); ++ if diagnostics.iter().any(|d| d.severity == Severity::Error) { ++ bail!("Validation failed"); ++ } ++ ++ let run_id = ulid::Ulid::new().to_string(); ++ let run_dir = args.run_dir.unwrap_or_else(|| { ++ if args.dry_run { ++ std::env::temp_dir().join("fabro-dry-run").join(&run_id) ++ } else { ++ let base = dirs::home_dir() ++ .expect("could not determine home directory") ++ .join(".fabro") ++ .join("runs"); ++ base.join(format!( ++ "{}-{}", ++ chrono::Local::now().format("%Y%m%d"), ++ run_id ++ )) ++ } ++ }); ++ tokio::fs::create_dir_all(&run_dir).await?; ++ fabro_util::run_log::activate(&run_dir.join("cli.log")) ++ .context("Failed to activate per-run log")?; ++ ++ let original_cwd = std::env::current_dir()?; ++ let emitter = Arc::new(EventEmitter::new()); ++ ++ let sandbox: Arc = ++ local_sandbox_with_callback(original_cwd.clone(), Arc::clone(&emitter)); ++ let sandbox: Arc = Arc::new(fabro_agent::ReadBeforeWriteSandbox::new(sandbox)); ++ ++ let interviewer: Arc = if args.auto_approve { ++ Arc::new(AutoApproveInterviewer) ++ } else { ++ Arc::new(ConsoleInterviewer::new(styles)) ++ }; ++ ++ let dry_run_mode = args.dry_run ++ || fabro_llm::client::Client::from_env() ++ .await ++ .map(|c| c.provider_names().is_empty()) ++ .unwrap_or(true); ++ ++ let model = args ++ .model ++ .unwrap_or_else(|| fabro_model::default_model_from_env().id); ++ let provider_enum = args ++ .provider ++ .as_deref() ++ .map(|s| s.parse::()) ++ .transpose() ++ .map_err(|e| anyhow::anyhow!("{e}"))? ++ .unwrap_or_else(Provider::default_from_env); ++ ++ let fallback_chain = Vec::new(); ++ ++ let registry = fabro_workflows::handler::default_registry(interviewer.clone(), || { ++ if dry_run_mode { ++ None ++ } else { ++ let api = ++ AgentApiBackend::new(model.clone(), provider_enum, fallback_chain.clone()); ++ let cli = AgentCliBackend::new(model.clone(), provider_enum); ++ Some(Box::new(BackendRouter::new(Box::new(api), cli))) ++ } ++ }); ++ let mut engine = fabro_workflows::engine::WorkflowRunEngine::with_interviewer( ++ registry, ++ Arc::clone(&emitter), ++ interviewer, ++ Arc::clone(&sandbox), ++ ); ++ if dry_run_mode { ++ engine.set_dry_run(true); ++ } ++ ++ let mut config = RunConfig { ++ run_dir: run_dir.clone(), ++ cancel_token: None, ++ dry_run: dry_run_mode, ++ run_id: run_id.clone(), ++ git_checkpoint_enabled: false, ++ host_repo_path: None, ++ base_sha: None, ++ run_branch: None, ++ meta_branch: None, ++ labels: HashMap::new(), ++ checkpoint_exclude_globs: Vec::new(), ++ github_app: github_app.clone(), ++ git_author, ++ base_branch: None, ++ pull_request: None, ++ asset_globs: Vec::new(), ++ workflow_slug: None, ++ }; ++ ++ let lifecycle = fabro_workflows::engine::LifecycleConfig { ++ setup_commands: Vec::new(), ++ setup_command_timeout_ms: 60_000, ++ devcontainer_phases: Vec::new(), ++ }; ++ ++ let run_start = Instant::now(); ++ let engine_result = engine ++ .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) ++ .await; ++ let run_duration_ms = run_start.elapsed().as_millis() as u64; ++ ++ if !args.no_retro && project_config::is_retro_enabled() { ++ let failed = match &engine_result { ++ Ok(ref o) => o.status == StageStatus::Fail, ++ Err(_) => true, ++ }; ++ ++ let llm_client = if dry_run_mode { ++ None ++ } else { ++ fabro_llm::client::Client::from_env().await.ok() ++ }; ++ ++ generate_retro( ++ &config.run_id, ++ &graph.name, ++ graph.goal(), ++ &run_dir, ++ failed, ++ run_duration_ms, ++ dry_run_mode, ++ llm_client.as_ref(), ++ &sandbox, ++ provider_enum, ++ &model, ++ styles, ++ Some(Arc::clone(&emitter)), ++ ) ++ .await; ++ } ++ ++ let _ = engine ++ .cleanup_sandbox(&config.run_id, &graph.name, false) ++ .await; ++ ++ let outcome = engine_result?; ++ ++ eprintln!("\n{}", styles.bold.apply_to("=== Run Result ===")); ++ eprintln!("{}", styles.dim.apply_to(format!("Run: {run_id}"))); ++ let status_str = outcome.status.to_string().to_uppercase(); ++ let status_color = match outcome.status { ++ StageStatus::Success | StageStatus::PartialSuccess => &styles.bold_green, ++ _ => &styles.bold_red, ++ }; ++ eprintln!("Status: {}", status_color.apply_to(&status_str)); ++ eprintln!( ++ "Duration: {}", ++ HumanDuration(Duration::from_millis(run_duration_ms)) ++ ); ++ eprintln!( ++ "{}", ++ styles ++ .dim ++ .apply_to(format!("Run: {}", tilde_path(&run_dir))) ++ ); ++ ++ print_final_output(&run_dir, styles); ++ print_assets(&run_dir, styles); ++ ++ fabro_util::run_log::deactivate(); ++ match outcome.status { ++ StageStatus::Success | StageStatus::PartialSuccess => return Ok(()), ++ _ => std::process::exit(1), ++ } ++ } ++ ++ // Run-ID path: resolve run_id and resume from git metadata ++ let run_arg = args.run.as_deref().expect("run is required"); ++ ++ let (run_id, run_branch) = ++ if let Some(stripped) = run_arg.strip_prefix(fabro_workflows::git::RUN_BRANCH_PREFIX) { ++ let id = stripped.to_string(); ++ let branch = run_arg.to_string(); ++ (id, branch) ++ } else { ++ let repo = git2::Repository::discover(".").context("not in a git repository")?; ++ let id = fabro_workflows::run_rewind::find_run_id_by_prefix(&repo, run_arg)?; ++ let branch = format!("{}{}", fabro_workflows::git::RUN_BRANCH_PREFIX, id); ++ (id, branch) ++ }; ++ ++ let original_cwd = std::env::current_dir()?; ++ ++ // Read checkpoint from metadata branch ++ let checkpoint = fabro_workflows::git::MetadataStore::read_checkpoint(&original_cwd, &run_id)? ++ .ok_or_else(|| { ++ anyhow::anyhow!("no checkpoint found on metadata branch for run {run_id}") ++ })?; ++ ++ // Read graph DOT from metadata branch ++ let source = fabro_workflows::git::MetadataStore::read_graph_dot(&original_cwd, &run_id)? ++ .ok_or_else(|| { ++ anyhow::anyhow!("no graph.fabro found on metadata branch for run {run_id}") ++ })?; ++ ++ // If --workflow was also provided, use it instead (allows overriding) ++ let (mut graph, diagnostics) = if let Some(ref workflow_path) = args.workflow { ++ fabro_workflows::workflow::prepare_from_file(workflow_path)? ++ } else { ++ fabro_workflows::workflow::WorkflowBuilder::new().prepare(&source)? ++ }; ++ let cli_goal = resolve_cli_goal(&args.goal, &args.goal_file)?; ++ apply_goal_override(&mut graph, cli_goal.as_deref(), None); ++ ++ eprintln!( ++ "{} {} from branch {} ({})", ++ styles.bold.apply_to("Resuming workflow:"), ++ graph.name, ++ styles.dim.apply_to(&run_branch), ++ run_id, ++ ); ++ ++ print_diagnostics(&diagnostics, styles); ++ if diagnostics.iter().any(|d| d.severity == Severity::Error) { ++ bail!("Validation failed"); ++ } ++ ++ // Set up logs directory ++ let run_dir = args.run_dir.unwrap_or_else(|| { ++ if args.dry_run { ++ std::env::temp_dir().join("fabro-dry-run").join(&run_id) ++ } else { ++ let base = dirs::home_dir() ++ .expect("could not determine home directory") ++ .join(".fabro") ++ .join("runs"); ++ base.join(format!( ++ "{}-{}", ++ chrono::Local::now().format("%Y%m%d"), ++ run_id ++ )) ++ } ++ }); ++ tokio::fs::create_dir_all(&run_dir).await?; ++ fabro_util::run_log::activate(&run_dir.join("cli.log")) ++ .context("Failed to activate per-run log")?; ++ tokio::fs::write(run_dir.join("graph.fabro"), &source).await?; ++ ++ let base_sha = fabro_workflows::git::MetadataStore::read_manifest(&original_cwd, &run_id)? ++ .and_then(|m| m.base_sha); ++ ++ // Resolve sandbox provider ++ let sandbox_provider = if args.dry_run { ++ SandboxProvider::Local ++ } else { ++ resolve_sandbox_provider(args.sandbox.map(Into::into), None, &run_defaults)? ++ }; ++ ++ let emitter = Arc::new(EventEmitter::new()); ++ let (sandbox, _worktree_path): (Arc, Option) = match sandbox_provider { ++ SandboxProvider::Local | SandboxProvider::Docker => { ++ // Re-attach worktree to the existing run branch via WorktreeSandbox. ++ let wt = run_dir.join("worktree"); ++ let wt_str = wt.to_string_lossy().into_owned(); ++ ++ let inner = local_sandbox_with_callback(original_cwd.clone(), Arc::clone(&emitter)); ++ let wt_config = WorktreeConfig { ++ branch_name: run_branch.clone(), ++ base_sha: base_sha.clone().unwrap_or_default(), ++ worktree_path: wt_str.clone(), ++ skip_branch_creation: true, // branch already exists on resume ++ }; ++ let mut wt_sandbox = WorktreeSandbox::new(inner, wt_config); ++ wt_sandbox.set_event_callback(Arc::clone(&emitter).worktree_callback()); ++ ++ wt_sandbox ++ .initialize() ++ .await ++ .map_err(|e| anyhow::anyhow!("failed to attach worktree to {run_branch}: {e}"))?; ++ std::env::set_current_dir(&wt)?; ++ (Arc::new(wt_sandbox) as Arc, Some(wt)) ++ } ++ #[cfg(feature = "exedev")] ++ SandboxProvider::Exe => { ++ let exe_config = super::run::resolve_exe_config(None, &run_defaults); ++ let clone_params = super::run::resolve_exe_clone_params(&original_cwd); ++ let mgmt_ssh = fabro_sandbox::exe::OpensshRunner::connect_raw("exe.dev") ++ .await ++ .map_err(|e| anyhow::anyhow!("Failed to connect to exe.dev: {e}"))?; ++ let config = exe_config.unwrap_or_default(); ++ let mut env = fabro_sandbox::exe::ExeSandbox::new( ++ Box::new(mgmt_ssh), ++ config, ++ clone_params, ++ Some(run_id.clone()), ++ github_app.clone(), ++ ); ++ let emitter_cb = Arc::clone(&emitter); ++ env.set_event_callback(Arc::new(move |event| { ++ emitter_cb.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); ++ })); ++ (Arc::new(env), None) ++ } ++ SandboxProvider::Ssh => { ++ let config = resolve_ssh_config(None, &run_defaults) ++ .ok_or_else(|| anyhow::anyhow!("--sandbox ssh requires [sandbox.ssh] config"))?; ++ let clone_params = resolve_ssh_clone_params(&original_cwd); ++ let mut env = fabro_sandbox::ssh::SshSandbox::new( ++ config, ++ clone_params, ++ Some(run_id.clone()), ++ github_app.clone(), ++ ); ++ let emitter_cb = Arc::clone(&emitter); ++ env.set_event_callback(Arc::new(move |event| { ++ emitter_cb.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); ++ })); ++ (Arc::new(env), None) ++ } ++ SandboxProvider::Daytona => { ++ bail!("resume is not yet supported with --sandbox daytona"); ++ } ++ }; ++ ++ // Wrap with ReadBeforeWriteSandbox to enforce read-before-write guard ++ let sandbox: Arc = Arc::new(fabro_agent::ReadBeforeWriteSandbox::new(sandbox)); ++ ++ // Let the sandbox provide any commands needed to resume on the existing run branch ++ let resume_setup_commands: Vec = sandbox.resume_setup_commands(&run_branch); ++ ++ // Build interviewer ++ let interviewer: Arc = if args.auto_approve { ++ Arc::new(AutoApproveInterviewer) ++ } else { ++ Arc::new(ConsoleInterviewer::new(styles)) ++ }; ++ ++ // Build engine with a backend ++ let dry_run_mode = args.dry_run ++ || fabro_llm::client::Client::from_env() ++ .await ++ .map(|c| c.provider_names().is_empty()) ++ .unwrap_or(true); ++ ++ let model = args ++ .model ++ .unwrap_or_else(|| fabro_model::default_model_from_env().id); ++ let provider_enum = args ++ .provider ++ .as_deref() ++ .map(|s| s.parse::()) ++ .transpose() ++ .map_err(|e| anyhow::anyhow!("{e}"))? ++ .unwrap_or_else(Provider::default_from_env); ++ ++ // No fallback config available for branch resume; use empty chain. ++ let fallback_chain = Vec::new(); ++ ++ let registry = fabro_workflows::handler::default_registry(interviewer.clone(), || { ++ if dry_run_mode { ++ None ++ } else { ++ let api = AgentApiBackend::new(model.clone(), provider_enum, fallback_chain.clone()); ++ let cli = AgentCliBackend::new(model.clone(), provider_enum); ++ Some(Box::new(BackendRouter::new(Box::new(api), cli))) ++ } ++ }); ++ let mut engine = fabro_workflows::engine::WorkflowRunEngine::with_interviewer( ++ registry, ++ Arc::clone(&emitter), ++ interviewer, ++ Arc::clone(&sandbox), ++ ); ++ if dry_run_mode { ++ engine.set_dry_run(true); ++ } ++ ++ let meta_branch = Some(fabro_workflows::git::MetadataStore::branch_name(&run_id)); ++ let mut config = RunConfig { ++ run_dir: run_dir.clone(), ++ cancel_token: None, ++ dry_run: dry_run_mode, ++ run_id: run_id.clone(), ++ git_checkpoint_enabled: true, // always true for resume (worktree or sandbox git is set up) ++ host_repo_path: Some(original_cwd.clone()), ++ base_sha, ++ run_branch: Some(run_branch.to_string()), ++ meta_branch, ++ labels: HashMap::new(), ++ checkpoint_exclude_globs: Vec::new(), ++ github_app: github_app.clone(), ++ git_author, ++ base_branch: None, ++ pull_request: None, ++ asset_globs: Vec::new(), ++ workflow_slug: None, ++ }; ++ ++ let lifecycle = fabro_workflows::engine::LifecycleConfig { ++ setup_commands: resume_setup_commands, ++ setup_command_timeout_ms: 60_000, ++ devcontainer_phases: Vec::new(), ++ }; ++ ++ let run_start = Instant::now(); ++ let engine_result = engine ++ .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) ++ .await; ++ let run_duration_ms = run_start.elapsed().as_millis() as u64; ++ ++ // Restore cwd (worktree is kept for `fabro cp` access; pruned separately) ++ let _ = std::env::set_current_dir(&original_cwd); ++ ++ // Auto-derive retro ++ if !args.no_retro && project_config::is_retro_enabled() { ++ let failed = match &engine_result { ++ Ok(ref o) => o.status == StageStatus::Fail, ++ Err(_) => true, ++ }; ++ ++ let llm_client = if dry_run_mode { ++ None ++ } else { ++ fabro_llm::client::Client::from_env().await.ok() ++ }; ++ ++ generate_retro( ++ &config.run_id, ++ &graph.name, ++ graph.goal(), ++ &run_dir, ++ failed, ++ run_duration_ms, ++ dry_run_mode, ++ llm_client.as_ref(), ++ &sandbox, ++ provider_enum, ++ &model, ++ styles, ++ Some(Arc::clone(&emitter)), ++ ) ++ .await; ++ } ++ ++ // Write finalize commit with retro.json + final node files (captures last diff.patch) ++ write_finalize_commit(&config, &run_dir).await; ++ ++ // Cleanup sandbox via engine (fires SandboxCleanup hook) ++ let _ = engine ++ .cleanup_sandbox(&config.run_id, &graph.name, false) ++ .await; ++ ++ let outcome = engine_result?; ++ ++ eprintln!("\n{}", styles.bold.apply_to("=== Run Result ===")); ++ eprintln!("{}", styles.dim.apply_to(format!("Run: {run_id}"))); ++ let status_str = outcome.status.to_string().to_uppercase(); ++ let status_color = match outcome.status { ++ StageStatus::Success | StageStatus::PartialSuccess => &styles.bold_green, ++ _ => &styles.bold_red, ++ }; ++ eprintln!("Status: {}", status_color.apply_to(&status_str)); ++ eprintln!( ++ "Duration: {}", ++ HumanDuration(Duration::from_millis(run_duration_ms)) ++ ); ++ eprintln!( ++ "{}", ++ styles ++ .dim ++ .apply_to(format!("Run: {}", tilde_path(&run_dir))) ++ ); ++ ++ print_final_output(&run_dir, styles); ++ print_assets(&run_dir, styles); ++ ++ fabro_util::run_log::deactivate(); ++ match outcome.status { ++ StageStatus::Success | StageStatus::PartialSuccess => Ok(()), ++ _ => std::process::exit(1), ++ } ++} +diff --git a/lib/crates/fabro-cli/src/commands/rewind.rs b/lib/crates/fabro-cli/src/commands/rewind.rs +index 1b84eb8f..dbbcd5e8 100644 +--- a/lib/crates/fabro-cli/src/commands/rewind.rs ++++ b/lib/crates/fabro-cli/src/commands/rewind.rs +@@ -46,9 +46,8 @@ pub fn run(args: &RewindArgs, styles: &Styles) -> Result<()> { + fabro_workflows::run_rewind::execute_rewind(&store, &run_id, entry, !args.no_push)?; + + eprintln!( +- "\nTo resume: fabro run --run-branch {}{}", +- fabro_workflows::git::RUN_BRANCH_PREFIX, +- run_id ++ "\nTo resume: fabro resume {}", ++ &run_id[..8.min(run_id.len())] + ); + + Ok(()) +diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs +index cd53fb4d..f5b5bd96 100644 +--- a/lib/crates/fabro-cli/src/commands/run.rs ++++ b/lib/crates/fabro-cli/src/commands/run.rs +@@ -74,8 +74,7 @@ impl From for CliSandboxProvider { + + #[derive(Args)] + pub struct RunArgs { +- /// Path to a .fabro workflow file or .toml task config (not required with --run-branch) +- #[arg(required_unless_present = "run_branch")] ++ /// Path to a .fabro workflow file or .toml task config + pub workflow: Option, + + /// Run output directory +@@ -87,21 +86,13 @@ pub struct RunArgs { + pub dry_run: bool, + + /// Validate run configuration without executing +- #[arg(long, conflicts_with_all = ["resume", "run_branch", "dry_run"])] ++ #[arg(long, conflicts_with = "dry_run")] + pub preflight: bool, + + /// Auto-approve all human gates + #[arg(long)] + pub auto_approve: bool, + +- /// Resume from a checkpoint file +- #[arg(long)] +- pub resume: Option, +- +- /// Resume from a git run branch (reads checkpoint and graph from metadata branch) +- #[arg(long, conflicts_with = "resume")] +- pub run_branch: Option, +- + /// Override the workflow goal (exposed as $goal in prompts) + #[arg(long)] + pub goal: Option, +@@ -143,7 +134,7 @@ pub struct RunArgs { + pub preserve_sandbox: bool, + + /// Run the workflow in the background and print the run ID +- #[arg(short = 'd', long, conflicts_with_all = ["resume", "run_branch", "preflight"])] ++ #[arg(short = 'd', long, conflicts_with = "preflight")] + pub detach: bool, + + /// Pre-generated run ID (used internally by --detach) +@@ -258,7 +249,7 @@ pub(crate) fn resolve_sandbox_provider( + } + + /// Resolve preserve-sandbox: CLI flag > TOML config > run defaults > false. +-fn resolve_preserve_sandbox( ++pub(crate) fn resolve_preserve_sandbox( + cli: bool, + run_cfg: Option<&WorkflowRunConfig>, + run_defaults: &RunDefaults, +@@ -310,7 +301,7 @@ fn resolve_daytona_config( + + #[cfg(feature = "exedev")] + /// Resolve exe.dev config: TOML config > run defaults. +-fn resolve_exe_config( ++pub(crate) fn resolve_exe_config( + run_cfg: Option<&WorkflowRunConfig>, + run_defaults: &RunDefaults, + ) -> Option { +@@ -325,7 +316,9 @@ fn resolve_exe_config( + /// + /// Returns `None` if no git repo is detected. Credential resolution is + /// handled by ExeSandbox itself via its `github_app` field. +-fn resolve_exe_clone_params(cwd: &std::path::Path) -> Option { ++pub(crate) fn resolve_exe_clone_params( ++ cwd: &std::path::Path, ++) -> Option { + let (detected_url, branch) = match fabro_sandbox::daytona::detect_repo_info(cwd) { + Ok(info) => info, + Err(e) => { +@@ -338,7 +331,7 @@ fn resolve_exe_clone_params(cwd: &std::path::Path) -> Option run defaults. +-fn resolve_ssh_config( ++pub(crate) fn resolve_ssh_config( + run_cfg: Option<&WorkflowRunConfig>, + run_defaults: &RunDefaults, + ) -> Option { +@@ -352,7 +345,9 @@ fn resolve_ssh_config( + /// + /// Returns `None` if no git repo is detected. Credential resolution is + /// handled by SshSandbox itself via its `github_app` field. +-fn resolve_ssh_clone_params(cwd: &std::path::Path) -> Option { ++pub(crate) fn resolve_ssh_clone_params( ++ cwd: &std::path::Path, ++) -> Option { + let (detected_url, branch) = match fabro_sandbox::daytona::detect_repo_info(cwd) { + Ok(info) => info, + Err(e) => { +@@ -436,7 +431,10 @@ struct CostAccumulator { + } + + /// Create a [`LocalSandbox`] wired to emit [`WorkflowRunEvent::Sandbox`] events. +-fn local_sandbox_with_callback(cwd: PathBuf, emitter: Arc) -> Arc { ++pub(crate) fn local_sandbox_with_callback( ++ cwd: PathBuf, ++ emitter: Arc, ++) -> Arc { + let mut env = LocalSandbox::new(cwd); + env.set_event_callback(Arc::new(move |event| { + emitter.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); +@@ -599,11 +597,6 @@ pub async fn run_command( + github_app: Option, + git_author: fabro_workflows::git::GitAuthor, + ) -> anyhow::Result<()> { +- // Handle --run-branch resume: read everything from git metadata +- if let Some(branch) = args.run_branch.clone() { +- return run_from_branch(args, &branch, styles, git_author, run_defaults, github_app).await; +- } +- + let PreparedWorkflow { + source, + graph, +@@ -1464,16 +1457,9 @@ pub async fn run_command( + }); + + let run_start = Instant::now(); +- let engine_result = if let Some(ref checkpoint_path) = args.resume { +- let checkpoint = Checkpoint::load(checkpoint_path)?; +- engine +- .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) +- .await +- } else { +- engine +- .run_with_lifecycle(&graph, &mut config, lifecycle, None) +- .await +- }; ++ let engine_result = engine ++ .run_with_lifecycle(&graph, &mut config, lifecycle, None) ++ .await; + let run_duration_ms = run_start.elapsed().as_millis() as u64; + + // Restore cwd (worktree is kept for `fabro cp` access; pruned separately) +@@ -1803,329 +1789,8 @@ pub async fn run_command( + } + } + +-/// Resume a workflow run from a git run branch. +-/// +-/// Reads the checkpoint, manifest, and graph DOT from the metadata branch +-/// (`fabro/meta/{run_id}`), re-attaches a worktree to the existing run branch, +-/// and resumes execution via `run_from_checkpoint()`. +-async fn run_from_branch( +- args: RunArgs, +- run_branch: &str, +- styles: &'static Styles, +- git_author: fabro_workflows::git::GitAuthor, +- run_defaults: RunDefaults, +- github_app: Option, +-) -> anyhow::Result<()> { +- // Extract run_id from branch name: "fabro/run/{run_id}" -> "{run_id}" +- let run_id = run_branch +- .strip_prefix(fabro_workflows::git::RUN_BRANCH_PREFIX) +- .ok_or_else(|| { +- anyhow::anyhow!( +- "invalid run branch format: expected '{}', got '{run_branch}'", +- fabro_workflows::git::RUN_BRANCH_PREFIX, +- ) +- })? +- .to_string(); +- +- let original_cwd = std::env::current_dir()?; +- +- // Read checkpoint from metadata branch +- let checkpoint = fabro_workflows::git::MetadataStore::read_checkpoint(&original_cwd, &run_id)? +- .ok_or_else(|| { +- anyhow::anyhow!("no checkpoint found on metadata branch for run {run_id}") +- })?; +- +- // Read graph DOT from metadata branch +- let source = fabro_workflows::git::MetadataStore::read_graph_dot(&original_cwd, &run_id)? +- .ok_or_else(|| { +- anyhow::anyhow!("no graph.fabro found on metadata branch for run {run_id}") +- })?; +- +- // If --pipeline was also provided, use it instead (allows overriding) +- let (mut graph, diagnostics) = if let Some(ref workflow_path) = args.workflow { +- fabro_workflows::workflow::prepare_from_file(workflow_path)? +- } else { +- fabro_workflows::workflow::WorkflowBuilder::new().prepare(&source)? +- }; +- let cli_goal = resolve_cli_goal(&args.goal, &args.goal_file)?; +- apply_goal_override(&mut graph, cli_goal.as_deref(), None); +- +- eprintln!( +- "{} {} from branch {} ({})", +- styles.bold.apply_to("Resuming workflow:"), +- graph.name, +- styles.dim.apply_to(run_branch), +- run_id, +- ); +- +- print_diagnostics(&diagnostics, styles); +- if diagnostics.iter().any(|d| d.severity == Severity::Error) { +- anyhow::bail!("Validation failed"); +- } +- +- // Set up logs directory +- let run_dir = args.run_dir.unwrap_or_else(|| { +- if args.dry_run { +- std::env::temp_dir().join("fabro-dry-run").join(&run_id) +- } else { +- let base = dirs::home_dir() +- .expect("could not determine home directory") +- .join(".fabro") +- .join("runs"); +- base.join(format!( +- "{}-{}", +- chrono::Local::now().format("%Y%m%d"), +- run_id +- )) +- } +- }); +- tokio::fs::create_dir_all(&run_dir).await?; +- fabro_util::run_log::activate(&run_dir.join("cli.log")) +- .context("Failed to activate per-run log")?; +- tokio::fs::write(run_dir.join("graph.fabro"), &source).await?; +- +- let base_sha = fabro_workflows::git::MetadataStore::read_manifest(&original_cwd, &run_id)? +- .and_then(|m| m.base_sha); +- +- // Resolve sandbox provider +- let sandbox_provider = if args.dry_run { +- SandboxProvider::Local +- } else { +- resolve_sandbox_provider(args.sandbox.map(Into::into), None, &run_defaults)? +- }; +- +- let emitter = Arc::new(EventEmitter::new()); +- let (sandbox, _worktree_path): (Arc, Option) = +- match sandbox_provider { +- SandboxProvider::Local | SandboxProvider::Docker => { +- // Re-attach worktree to the existing run branch via WorktreeSandbox. +- let wt = run_dir.join("worktree"); +- let wt_str = wt.to_string_lossy().into_owned(); +- +- let inner = local_sandbox_with_callback(original_cwd.clone(), Arc::clone(&emitter)); +- let wt_config = WorktreeConfig { +- branch_name: run_branch.to_string(), +- base_sha: base_sha.clone().unwrap_or_default(), +- worktree_path: wt_str.clone(), +- skip_branch_creation: true, // branch already exists on resume +- }; +- let mut wt_sandbox = WorktreeSandbox::new(inner, wt_config); +- wt_sandbox.set_event_callback(Arc::clone(&emitter).worktree_callback()); +- +- wt_sandbox.initialize().await.map_err(|e| { +- anyhow::anyhow!("failed to attach worktree to {run_branch}: {e}") +- })?; +- std::env::set_current_dir(&wt)?; +- ( +- Arc::new(wt_sandbox) as Arc, +- Some(wt), +- ) +- } +- #[cfg(feature = "exedev")] +- SandboxProvider::Exe => { +- let exe_config = resolve_exe_config(None, &run_defaults); +- let clone_params = resolve_exe_clone_params(&original_cwd); +- let mgmt_ssh = fabro_sandbox::exe::OpensshRunner::connect_raw("exe.dev") +- .await +- .map_err(|e| anyhow::anyhow!("Failed to connect to exe.dev: {e}"))?; +- let config = exe_config.unwrap_or_default(); +- let mut env = fabro_sandbox::exe::ExeSandbox::new( +- Box::new(mgmt_ssh), +- config, +- clone_params, +- Some(run_id.clone()), +- github_app.clone(), +- ); +- let emitter_cb = Arc::clone(&emitter); +- env.set_event_callback(Arc::new(move |event| { +- emitter_cb.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); +- })); +- (Arc::new(env), None) +- } +- SandboxProvider::Ssh => { +- let config = resolve_ssh_config(None, &run_defaults).ok_or_else(|| { +- anyhow::anyhow!("--sandbox ssh requires [sandbox.ssh] config") +- })?; +- let clone_params = resolve_ssh_clone_params(&original_cwd); +- let mut env = fabro_sandbox::ssh::SshSandbox::new( +- config, +- clone_params, +- Some(run_id.clone()), +- github_app.clone(), +- ); +- let emitter_cb = Arc::clone(&emitter); +- env.set_event_callback(Arc::new(move |event| { +- emitter_cb.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); +- })); +- (Arc::new(env), None) +- } +- SandboxProvider::Daytona => { +- bail!("--run-branch resume is not yet supported with --sandbox daytona"); +- } +- }; +- +- // Wrap with ReadBeforeWriteSandbox to enforce read-before-write guard +- let sandbox: Arc = +- Arc::new(fabro_agent::ReadBeforeWriteSandbox::new(sandbox)); +- +- // Let the sandbox provide any commands needed to resume on the existing run branch +- let resume_setup_commands: Vec = sandbox.resume_setup_commands(run_branch); +- +- // Build interviewer +- let interviewer: Arc = if args.auto_approve { +- Arc::new(AutoApproveInterviewer) +- } else { +- Arc::new(ConsoleInterviewer::new(styles)) +- }; +- +- // Build engine with a backend +- let dry_run_mode = args.dry_run +- || fabro_llm::client::Client::from_env() +- .await +- .map(|c| c.provider_names().is_empty()) +- .unwrap_or(true); +- +- let model = args +- .model +- .unwrap_or_else(|| fabro_model::default_model_from_env().id); +- let provider_enum = args +- .provider +- .as_deref() +- .map(|s| s.parse::()) +- .transpose() +- .map_err(|e| anyhow::anyhow!("{e}"))? +- .unwrap_or_else(fabro_model::Provider::default_from_env); +- +- // No fallback config available for branch resume; use empty chain. +- let fallback_chain = Vec::new(); +- +- let registry = fabro_workflows::handler::default_registry(interviewer.clone(), || { +- if dry_run_mode { +- None +- } else { +- let api = AgentApiBackend::new(model.clone(), provider_enum, fallback_chain.clone()); +- let cli = AgentCliBackend::new(model.clone(), provider_enum); +- Some(Box::new(BackendRouter::new(Box::new(api), cli))) +- } +- }); +- let mut engine = fabro_workflows::engine::WorkflowRunEngine::with_interviewer( +- registry, +- Arc::clone(&emitter), +- interviewer, +- Arc::clone(&sandbox), +- ); +- if dry_run_mode { +- engine.set_dry_run(true); +- } +- +- let meta_branch = Some(fabro_workflows::git::MetadataStore::branch_name(&run_id)); +- let mut config = RunConfig { +- run_dir: run_dir.clone(), +- cancel_token: None, +- dry_run: dry_run_mode, +- run_id: run_id.clone(), +- git_checkpoint_enabled: true, // always true for resume (worktree or sandbox git is set up) +- host_repo_path: Some(original_cwd.clone()), +- base_sha, +- run_branch: Some(run_branch.to_string()), +- meta_branch, +- labels: HashMap::new(), +- checkpoint_exclude_globs: Vec::new(), +- github_app: github_app.clone(), +- git_author, +- base_branch: None, +- pull_request: None, +- asset_globs: Vec::new(), +- workflow_slug: None, +- }; +- +- let lifecycle = fabro_workflows::engine::LifecycleConfig { +- setup_commands: resume_setup_commands, +- setup_command_timeout_ms: 60_000, +- devcontainer_phases: Vec::new(), +- }; +- +- let run_start = Instant::now(); +- let engine_result = engine +- .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) +- .await; +- let run_duration_ms = run_start.elapsed().as_millis() as u64; +- +- // Restore cwd (worktree is kept for `fabro cp` access; pruned separately) +- let _ = std::env::set_current_dir(&original_cwd); +- +- // Auto-derive retro +- if !args.no_retro && project_config::is_retro_enabled() { +- let failed = match &engine_result { +- Ok(ref o) => o.status == StageStatus::Fail, +- Err(_) => true, +- }; +- +- let llm_client = if dry_run_mode { +- None +- } else { +- fabro_llm::client::Client::from_env().await.ok() +- }; +- +- generate_retro( +- &config.run_id, +- &graph.name, +- graph.goal(), +- &run_dir, +- failed, +- run_duration_ms, +- dry_run_mode, +- llm_client.as_ref(), +- &sandbox, +- provider_enum, +- &model, +- styles, +- Some(Arc::clone(&emitter)), +- ) +- .await; +- } +- +- // Write finalize commit with retro.json + final node files (captures last diff.patch) +- write_finalize_commit(&config, &run_dir).await; +- +- // Cleanup sandbox via engine (fires SandboxCleanup hook) +- let _ = engine +- .cleanup_sandbox(&config.run_id, &graph.name, false) +- .await; +- +- let outcome = engine_result?; +- +- eprintln!("\n{}", styles.bold.apply_to("=== Run Result ==="),); +- eprintln!("{}", styles.dim.apply_to(format!("Run: {run_id}"))); +- let status_str = outcome.status.to_string().to_uppercase(); +- let status_color = match outcome.status { +- StageStatus::Success | StageStatus::PartialSuccess => &styles.bold_green, +- _ => &styles.bold_red, +- }; +- eprintln!("Status: {}", status_color.apply_to(&status_str),); +- eprintln!( +- "Duration: {}", +- HumanDuration(Duration::from_millis(run_duration_ms)) +- ); +- eprintln!( +- "{}", +- styles +- .dim +- .apply_to(format!("Run: {}", tilde_path(&run_dir))) +- ); +- +- print_final_output(&run_dir, styles); +- print_assets(&run_dir, styles); +- +- fabro_util::run_log::deactivate(); +- match outcome.status { +- StageStatus::Success | StageStatus::PartialSuccess => Ok(()), +- _ => std::process::exit(1), +- } +-} +- + /// Print the final stage output from the checkpoint, if available. +-fn print_final_output(run_dir: &std::path::Path, styles: &Styles) { ++pub(crate) fn print_final_output(run_dir: &std::path::Path, styles: &Styles) { + let Ok(checkpoint) = Checkpoint::load(&run_dir.join("checkpoint.json")) else { + return; + }; +@@ -2146,7 +1811,7 @@ fn print_final_output(run_dir: &std::path::Path, styles: &Styles) { + } + + /// Print collected asset paths, if any. +-fn print_assets(run_dir: &std::path::Path, styles: &Styles) { ++pub(crate) fn print_assets(run_dir: &std::path::Path, styles: &Styles) { + let paths = fabro_workflows::asset_snapshot::collect_asset_paths(run_dir); + if paths.is_empty() { + return; +@@ -2520,7 +2185,7 @@ async fn run_preflight( + /// + /// This captures the last diff.patch (written after the final checkpoint) and retro.json. + /// Best-effort: errors are logged as warnings. +-async fn write_finalize_commit(config: &RunConfig, run_dir: &std::path::Path) { ++pub(crate) async fn write_finalize_commit(config: &RunConfig, run_dir: &std::path::Path) { + let (Some(ref meta_branch), Some(ref repo_path)) = + (&config.meta_branch, &config.host_repo_path) + else { +@@ -2557,7 +2222,7 @@ async fn write_finalize_commit(config: &RunConfig, run_dir: &std::path::Path) { + /// Derives a basic retro from the checkpoint, then optionally runs the retro agent + /// for a richer narrative. Errors are logged as warnings rather than propagated. + #[allow(clippy::too_many_arguments)] +-async fn generate_retro( ++pub(crate) async fn generate_retro( + run_id: &str, + workflow_name: &str, + goal: &str, +diff --git a/lib/crates/fabro-cli/src/commands/start.rs b/lib/crates/fabro-cli/src/commands/start.rs +index b5791df2..00e38bb9 100644 +--- a/lib/crates/fabro-cli/src/commands/start.rs ++++ b/lib/crates/fabro-cli/src/commands/start.rs +@@ -86,8 +86,6 @@ mod tests { + preserve_sandbox: false, + dry_run: false, + auto_approve: true, +- resume: None, +- run_branch: None, + } + } + +diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs +index 7a97ddf4..5d5bc6ae 100644 +--- a/lib/crates/fabro-cli/src/main.rs ++++ b/lib/crates/fabro-cli/src/main.rs +@@ -164,6 +164,8 @@ enum Command { + #[command(subcommand)] + command: SecretCommand, + }, ++ /// Resume an interrupted workflow run ++ Resume(commands::resume::ResumeArgs), + /// Rewind a workflow run to an earlier checkpoint + Rewind(commands::rewind::RewindArgs), + /// Fork a workflow run from an earlier checkpoint into a new run +@@ -435,6 +437,7 @@ async fn main_inner() -> (String, Result<()>) { + SecretCommand::Rm(_) => "secret rm", + SecretCommand::Set(_) => "secret set", + }, ++ Command::Resume(_) => "resume", + Command::Rewind(_) => "rewind", + Command::Fork(_) => "fork", + Command::Wait(_) => "wait", +@@ -737,8 +740,6 @@ async fn main_inner() -> (String, Result<()>) { + dry_run: spec.dry_run, + preflight: false, + auto_approve: spec.auto_approve, +- resume: spec.resume, +- run_branch: spec.run_branch, + goal: spec.goal, + goal_file: None, + model: Some(spec.model), +@@ -919,6 +920,24 @@ async fn main_inner() -> (String, Result<()>) { + commands::secret::set_command(&args)?; + } + }, ++ Command::Resume(args) => { ++ let styles: &'static fabro_util::terminal::Styles = ++ Box::leak(Box::new(fabro_util::terminal::Styles::detect_stderr())); ++ let cli_config = cli_config::load_cli_config(None)?; ++ let github_app = build_github_app_credentials(cli_config.app_id()); ++ let git_author = fabro_workflows::git::GitAuthor::from_options( ++ cli_config.git_author().and_then(|a| a.name.clone()), ++ cli_config.git_author().and_then(|a| a.email.clone()), ++ ); ++ commands::resume::resume_command( ++ args, ++ cli_config.run_defaults, ++ styles, ++ github_app, ++ git_author, ++ ) ++ .await?; ++ } + Command::Rewind(args) => { + let styles = fabro_util::terminal::Styles::detect_stderr(); + commands::rewind::run(&args, &styles)?; +diff --git a/lib/crates/fabro-cli/tests/cli.rs b/lib/crates/fabro-cli/tests/cli.rs +index 302c3a51..a3a85b6f 100644 +--- a/lib/crates/fabro-cli/tests/cli.rs ++++ b/lib/crates/fabro-cli/tests/cli.rs +@@ -513,19 +513,31 @@ fn detach_creates_run_dir_with_detach_log() { + ); + } + ++// == Resume =================================================================== ++ + #[test] +-fn detach_conflicts_with_resume() { ++fn resume_help_shows_expected_args() { + arc() +- .args([ +- "run", +- "--detach", +- "--resume", +- "/tmp/fake-checkpoint.json", +- "../../../test/simple.fabro", +- ]) ++ .args(["resume", "--help"]) + .assert() +- .failure() +- .stderr(predicate::str::contains("cannot be used with")); ++ .success() ++ .stdout(predicate::str::contains("--checkpoint")) ++ .stdout(predicate::str::contains("--workflow")); ++} ++ ++#[test] ++fn resume_requires_run_or_checkpoint() { ++ arc().args(["resume"]).assert().failure(); ++} ++ ++#[test] ++fn run_help_no_longer_shows_resume_or_run_branch() { ++ arc() ++ .args(["run", "--help"]) ++ .assert() ++ .success() ++ .stdout(predicate::str::contains("--resume").not()) ++ .stdout(predicate::str::contains("--run-branch").not()); + } + + // == Bug regression: create/start/attach lifecycle ============================ +@@ -572,9 +584,7 @@ fn setup_run_dir( + "ssh": false, + "preserve_sandbox": false, + "dry_run": true, +- "auto_approve": true, +- "resume": null, +- "run_branch": null ++ "auto_approve": true + }); + if let (Some(base), Some(overrides)) = (spec.as_object_mut(), spec_overrides.as_object()) { + for (k, v) in overrides { +@@ -625,9 +635,7 @@ digraph G { + "ssh": false, + "preserve_sandbox": false, + "dry_run": true, +- "auto_approve": true, +- "resume": null, +- "run_branch": null ++ "auto_approve": true + }); + std::fs::write( + run_dir.join("spec.json"), +diff --git a/lib/crates/fabro-workflows/src/run_spec.rs b/lib/crates/fabro-workflows/src/run_spec.rs +index df5dbac5..efe424f9 100644 +--- a/lib/crates/fabro-workflows/src/run_spec.rs ++++ b/lib/crates/fabro-workflows/src/run_spec.rs +@@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; + use serde::{Deserialize, Serialize}; + + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] ++#[serde(default)] + pub struct RunSpec { + pub run_id: String, + pub workflow_path: PathBuf, +@@ -20,8 +21,28 @@ pub struct RunSpec { + pub preserve_sandbox: bool, + pub dry_run: bool, + pub auto_approve: bool, +- pub resume: Option, +- pub run_branch: Option, ++} ++ ++impl Default for RunSpec { ++ fn default() -> Self { ++ Self { ++ run_id: String::new(), ++ workflow_path: PathBuf::new(), ++ dot_source: String::new(), ++ working_directory: PathBuf::new(), ++ goal: None, ++ model: String::new(), ++ provider: None, ++ sandbox_provider: String::new(), ++ labels: HashMap::new(), ++ verbose: false, ++ no_retro: false, ++ ssh: false, ++ preserve_sandbox: false, ++ dry_run: false, ++ auto_approve: false, ++ } ++ } + } + + impl RunSpec { +@@ -65,8 +86,6 @@ mod tests { + preserve_sandbox: false, + dry_run: false, + auto_approve: true, +- resume: Some(PathBuf::from("/tmp/checkpoint")), +- run_branch: Some("fabro/run/abc123".to_string()), + } + } + diff --git a/nodes/simplify_opus/prompt.md b/nodes/simplify_opus/prompt.md new file mode 100644 index 000000000..e12aa4f6b --- /dev/null +++ b/nodes/simplify_opus/prompt.md @@ -0,0 +1,173 @@ +Goal: # Plan: Extract `fabro resume` subcommand + +## Context + +Resume functionality is currently embedded in `fabro run` via `--resume` (checkpoint file) and `--run-branch` (git branch). This makes the `run` command's arg surface complex with `conflicts_with` annotations, and the UX is unintuitive — users must construct `fabro/run/RUN_ID` branch names manually. The new `fabro resume` subcommand provides a cleaner interface: `fabro resume RUN_ID_OR_PREFIX`. + +## New `ResumeArgs` struct + +```rust +pub struct ResumeArgs { + /// Run ID, prefix, or branch (fabro/run/...) + #[arg(required_unless_present = "checkpoint")] + pub run: Option, + + /// Resume from a checkpoint file (requires --workflow) + #[arg(long)] + pub checkpoint: Option, + + /// Override workflow graph (required with --checkpoint) + #[arg(long)] + pub workflow: Option, + + // Shared run options: run_dir, dry_run, auto_approve, goal, goal_file, + // model, provider, verbose, sandbox, no_retro, ssh, preserve_sandbox +} +``` + +**Run ID resolution** (at top of `resume_command()`): +- If `run` starts with `fabro/run/` → strip prefix to get run_id +- Otherwise → call `find_run_id_by_prefix(&repo, &run)` (same as `rewind`/`fork`) +- Then construct branch name as `fabro/run/{run_id}` + +## Files to modify + +### 1. New: `lib/crates/fabro-cli/src/commands/resume.rs` +- Define `ResumeArgs` struct +- Move `run_from_branch()` body (~315 lines, `run.rs:1811-2125`) into `pub async fn resume_command()` +- Add run ID resolution logic at top (prefix → full ID via `find_run_id_by_prefix`) +- Add `--checkpoint` path: validate `--workflow` is present, load graph via `prepare_from_file()`, load checkpoint via `Checkpoint::load()`, then run engine + +### 2. `lib/crates/fabro-cli/src/commands/run.rs` +- **Remove from `RunArgs`**: `resume` field (line 97-99), `run_branch` field (line 101-103) +- **Simplify `workflow`**: remove `required_unless_present = "run_branch"` — it's now always required +- **Update `conflicts_with_all`**: remove `"resume"`/`"run_branch"` from `preflight` (line 90) and `detach` (line 146) +- **Remove** `run_from_branch()` function (lines 1811-2125) +- **Remove** the `run_branch` early-return at top of `run_command()` (lines 602-604) +- **Simplify** engine call: remove `if let Some(ref checkpoint_path) = args.resume` branch (lines 1467-1476), always pass `None` for checkpoint +- **Widen visibility** of helpers used by `resume.rs`: + - `local_sandbox_with_callback` (line 439) → `pub(crate)` + - `resolve_ssh_config` (line 341) → `pub(crate)` + - `resolve_ssh_clone_params` (line 355) → `pub(crate)` + - `resolve_exe_config` (line 313) → `pub(crate)` + - `resolve_exe_clone_params` (line 328) → `pub(crate)` + - `resolve_preserve_sandbox` (line 261) → `pub(crate)` + - `generate_retro` (line 2560) → `pub(crate)` + - `write_finalize_commit` (line 2523) → `pub(crate)` + - `print_final_output` (line 2128) → `pub(crate)` + - `print_assets` (line 2149) → `pub(crate)` + +### 3. `lib/crates/fabro-cli/src/commands/mod.rs` +- Add `pub mod resume;` + +### 4. `lib/crates/fabro-cli/src/main.rs` +- Add `Resume(commands::resume::ResumeArgs)` to `Command` enum (near line 170, alongside `Rewind`/`Fork`) +- Add `Command::Resume(_) => "resume"` to command_name match +- Add dispatch handler (pattern follows `Rewind`/`Fork`/`Wait` — create styles, load cli_config, build github_app/git_author, call `resume_command()`) + +### 5. `lib/crates/fabro-workflows/src/run_spec.rs` +- Remove `resume` and `run_branch` fields from `RunSpec` +- Add `#[serde(default)]` to `RunSpec` for backward compat with existing `spec.json` files +- Update `sample_spec()` in tests + +### 6. `lib/crates/fabro-cli/src/commands/create.rs` +- Remove lines 86-87 that set `resume` and `run_branch` in the spec + +### 7. `lib/crates/fabro-cli/src/main.rs` (`_run_engine` handler) +- Remove lines setting `resume` and `run_branch` when reconstructing `RunArgs` from `RunSpec` + +### 8. `lib/crates/fabro-cli/src/commands/rewind.rs` (line 48-52) +- Change hint: `"To resume: fabro resume {run_id}"` (use short prefix) + +### 9. `lib/crates/fabro-cli/src/commands/fork.rs` (line 56-60) +- Change hint: `"To resume: fabro resume {new_run_id}"` (use short prefix) + +### 10. `lib/crates/fabro-cli/tests/cli.rs` +- Update/remove tests referencing `--resume` or `--run-branch` on `fabro run` +- Add basic parse test for `fabro resume` + +### 11. Documentation (`docs/`) +- Update `docs/reference/cli.mdx`: add `fabro resume` section, remove `--resume`/`--run-branch` from `fabro run` +- Update `docs/execution/checkpoints.mdx`: change resume examples +- Update any other docs referencing `fabro run --run-branch` or `fabro run --resume` + +## Verification + +1. `cargo build --workspace` — compiles cleanly +2. `cargo test --workspace` — all tests pass +3. `cargo clippy --workspace -- -D warnings` — no warnings +4. Manual: `fabro resume --help` shows expected args +5. Manual: `fabro run --help` no longer shows `--resume` or `--run-branch` + + +## 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, 91.6k tokens in / 34.3k out + - Files: /home/daytona/workspace/docs/core-concepts/how-fabro-works.mdx, /home/daytona/workspace/docs/execution/checkpoints.mdx, /home/daytona/workspace/docs/reference/cli.mdx, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/fork.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/mod.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/resume.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/rewind.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.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-cli/tests/cli.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). \ No newline at end of file diff --git a/nodes/simplify_opus/provider_used.json b/nodes/simplify_opus/provider_used.json new file mode 100644 index 000000000..c1905ff10 --- /dev/null +++ b/nodes/simplify_opus/provider_used.json @@ -0,0 +1,5 @@ +{ + "mode": "agent", + "provider": "anthropic", + "model": "claude-opus-4-6" +} \ No newline at end of file diff --git a/nodes/simplify_opus/response.md b/nodes/simplify_opus/response.md new file mode 100644 index 000000000..6d25046c5 --- /dev/null +++ b/nodes/simplify_opus/response.md @@ -0,0 +1,23 @@ +Here's a summary of what was fixed: + +### Fixes Applied + +1. **Eliminated ~120 lines of duplicated code in `resume.rs`** (604 → 420 lines, -184 lines): The two code paths (checkpoint-file and git-branch) had nearly identical blocks for: interviewer creation, dry-run detection, model/provider resolution, engine construction, retro generation, result printing, and exit code logic. Restructured into three functions: + - `prepare_from_checkpoint()` — loads checkpoint/graph from files, creates simple local sandbox + - `prepare_from_branch()` — resolves run ID from git, reads metadata, sets up worktree sandbox + - `run_resumed()` — shared tail handling engine setup, execution, retro, and result display + - A `ResumeContext` struct carries the divergent state between the two paths + +2. **Extracted `default_run_dir()` helper in `run.rs`** — The run-dir defaulting logic (`~/.fabro/runs/YYYYMMDD-RUNID` or `/tmp/fabro-dry-run/RUNID`) was duplicated 4 times across `run.rs`, `create.rs`, and resume.rs. Now it's a single `pub(crate)` function reused by all three. + +3. **Used `resolve_model_provider()` in resume.rs** — The original code had an inline simplified version that skipped model alias expansion (e.g., `--model opus` wouldn't resolve to `claude-opus-4-6`) and ignored graph `default_model`/`default_provider` attributes. Now it calls the full precedence chain through the existing tested helper. + +4. **Simplified `RunSpec` Default impl** — Replaced the manual 16-field `impl Default for RunSpec` with `#[derive(Default)]` since all fields (`String`, `PathBuf`, `HashMap`, `bool`, `Option`) have built-in Default impls. + +5. **Fixed missing trailing newlines** in `docs/core-concepts/how-fabro-works.mdx` and `docs/reference/cli.mdx`. + +### Verification +- `cargo build --workspace` ✅ +- `cargo test --workspace` ✅ (all tests pass) +- `cargo clippy --workspace -- -D warnings` ✅ (no warnings) +- `cargo fmt --check --all` ✅ \ No newline at end of file diff --git a/nodes/simplify_opus/status.json b/nodes/simplify_opus/status.json new file mode 100644 index 000000000..79a1988d9 --- /dev/null +++ b/nodes/simplify_opus/status.json @@ -0,0 +1,6 @@ +{ + "status": "success", + "notes": "Stage completed: simplify_opus", + "failure_reason": null, + "timestamp": "2026-03-21T14:45:27.208961+00:00" +} \ No newline at end of file