From 57def3488285d0df31acd3dc18d1b1734d63c662 Mon Sep 17 00:00:00 2001 From: Fabro Date: Thu, 19 Mar 2026 21:51:52 -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 | 73 +- nodes/implement/diff.patch | 1243 ++++++++++++++++++++++++ nodes/simplify_opus/prompt.md | 230 +++++ nodes/simplify_opus/provider_used.json | 5 + nodes/simplify_opus/response.md | 24 + nodes/simplify_opus/status.json | 6 + 6 files changed, 1559 insertions(+), 22 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 6802433fe..828878785 100644 --- a/checkpoint.json +++ b/checkpoint.json @@ -1,44 +1,49 @@ { - "timestamp": "2026-03-20T01:36:44.876931Z", - "current_node": "implement", + "timestamp": "2026-03-20T01:51:52.226793Z", + "current_node": "simplify_opus", "completed_nodes": [ "start", "toolchain", "preflight_compile", "preflight_lint", - "implement" + "implement", + "simplify_opus" ], "node_retries": { "preflight_compile": 1, "start": 1, "implement": 1, + "simplify_opus": 1, "toolchain": 1, "preflight_lint": 1 }, "context_values": { - "internal.run_id": "01KM4CC4HA0SYCXGQW7BJWJE8F", "internal.retry_count.preflight_lint": 1, - "internal.retry_count.toolchain": 1, - "command.output": "", - "current.preamble": "Goal: # Unified WorktreeSandbox\n\n## Context\n\nWorktree management is currently split across two locations with duplicated logic:\n\n1. **Parallel branches** (`parallel.rs`): Inline git commands in `ParallelHandler::execute()`, a thin `WorktreeSandbox` decorator, and separate local/remote code paths\n2. **Top-level CLI run** (`run.rs`): A `setup_worktree()` function using synchronous git helpers, with the worktree path fed into a plain `LocalSandbox`\n\nThe goal is a single `WorktreeSandbox` type that wraps any `Arc`, manages the worktree lifecycle in `initialize()`/`cleanup()`, and eliminates the local/remote branching.\n\n## Plan\n\n### Step 1: Create `WorktreeSandbox` in fabro-sandbox\n\n**New file:** `lib/crates/fabro-sandbox/src/worktree.rs`\n\nDefine:\n\n```rust\npub enum WorktreeEvent {\n BranchCreated { branch: String, sha: String },\n WorktreeAdded { path: String, branch: String },\n WorktreeRemoved { path: String },\n Reset { sha: String },\n}\n\npub type WorktreeEventCallback = Arc;\n\npub struct WorktreeConfig {\n pub branch_name: String,\n pub base_sha: String,\n pub worktree_path: String,\n /// Skip branch creation and reset (for resume, where branch already exists).\n pub skip_branch_creation: bool,\n}\n\npub struct WorktreeSandbox {\n inner: Arc,\n config: WorktreeConfig,\n event_callback: Option,\n}\n```\n\n**Constructor + getters:** `new(inner, config)`, `set_event_callback()`, `branch_name()`, `base_sha()`, `worktree_path()`\n\n**`initialize()`:**\n1. If `!skip_branch_creation`: `git branch --force {branch_name} {base_sha}` via `inner.exec_command()`, emit `BranchCreated`\n2. `git worktree remove --force {path}` (best-effort), then `git worktree add {path} {branch}`, emit `WorktreeAdded`\n3. If `!skip_branch_creation`: `git reset --hard {base_sha}` in worktree dir, emit `Reset`\n\nDoes NOT call `inner.initialize()` — the inner sandbox's lifecycle is managed separately.\n\n**`cleanup()`:** `git worktree remove --force {path}`, emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`.\n\n**`working_directory()`:** Returns `config.worktree_path`.\n\n**`exec_command()`:** Defaults `working_dir` to `config.worktree_path` when `None`, delegates to inner.\n\n**All other Sandbox methods:** Delegate to inner. Must be a manual `impl Sandbox` block (can't use `delegate_sandbox!` since it generates `initialize`/`cleanup`/`working_directory`/`exec_command` which we need to override).\n\nAll interpolated values in git commands use `shell_quote()`.\n\n### Step 2: Register module and re-exports\n\n- `lib/crates/fabro-sandbox/src/lib.rs`: Add `pub mod worktree;` and `pub use worktree::WorktreeSandbox;`\n- `lib/crates/fabro-agent/src/sandbox.rs`: Add re-export of `WorktreeSandbox`\n\n### Step 3: Unit tests for WorktreeSandbox\n\nIn `worktree.rs` `#[cfg(test)]` module, using `MockSandbox`:\n\n- `initialize()` issues correct git commands (branch, worktree remove, worktree add, reset) and emits events\n- `skip_branch_creation` skips branch + reset, only does worktree add\n- `cleanup()` issues `worktree remove` and emits `WorktreeRemoved`\n- `working_directory()` returns worktree path\n- `exec_command()` with `None` working_dir defaults to worktree path\n- `exec_command()` with explicit working_dir passes it through\n- `initialize()` propagates errors on non-zero exit\n\n**MockSandbox enhancement:** Add `captured_commands: Mutex>` field to `test_support.rs` to capture the sequence of `exec_command` calls (current `captured_command` only stores the last one). Append to vec in `exec_command()` impl.\n\n### Step 4: Refactor parallel.rs\n\n- **Remove** the private `WorktreeSandbox` struct (lines 28-126) and `use fabro_agent::LocalSandbox`\n- **Replace** the inline git setup loop (lines 361-450) with:\n - Construct `WorktreeConfig` with branch name, base SHA, worktree path\n - Create `WorktreeSandbox::new(Arc::clone(&services.sandbox), config)`\n - Wire event callback to bridge `WorktreeEvent` → `WorkflowRunEvent`\n - Call `initialize().await`\n- This eliminates the `if services.sandbox.is_remote()` branch (lines 442-449) — `WorktreeSandbox` works the same for any inner sandbox\n- **Cleanup loop** (lines 659-668): Keep calling `git_remove_worktree()` on the parent sandbox (the `WorktreeSandbox` Arc is consumed by the spawned task and dropped). Alternatively, could store the sandbox Arc in `BranchResult` and call `.cleanup()`, but the current approach is simpler.\n\n### Step 5: Refactor run.rs — new runs\n\nReplace `setup_worktree()` call (lines 830-845) + separate `LocalSandbox` construction with:\n\n```\nif workdir_strategy == LocalWorktree:\n base_sha = git::head_sha()\n branch_name = \"fabro/run/{run_id}\"\n inner = Arc::new(LocalSandbox::new(original_cwd))\n wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { ... })\n wt_sandbox.set_event_callback(bridge to WorkflowRunEvent)\n wt_sandbox.initialize().await\n std::env::set_current_dir(&worktree_path) // stays in CLI, not in sandbox\n sandbox = Arc::new(wt_sandbox)\n // store base_sha, branch_name for RunConfig\n```\n\n**Delete** the `setup_worktree()` function (lines 1696-1714) — its logic is absorbed above.\n\n`std::env::set_current_dir()` stays in `run.rs` — it's a process-global side effect that belongs to the CLI.\n\n### Step 6: Refactor run.rs — resume (run_from_branch)\n\nReplace worktree re-attachment (lines 1810-1822) with:\n\n```\ninner = Arc::new(LocalSandbox::new(original_cwd))\nwt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig {\n branch_name: run_branch,\n base_sha: base_sha.unwrap_or_default(),\n worktree_path: wt_str,\n skip_branch_creation: true, // branch already exists\n})\nwt_sandbox.initialize().await\nstd::env::set_current_dir(&wt)\n```\n\n### Step 7: Verify\n\n- `cargo build --workspace`\n- `cargo test --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- Manual: `fabro run` with worktree mode enabled on a local workflow\n- Manual: `fabro run --run-branch` to test resume path\n\n## Files to modify\n\n| File | Change |\n|---|---|\n| `lib/crates/fabro-sandbox/src/worktree.rs` | **New** — WorktreeSandbox, WorktreeConfig, WorktreeEvent, impl Sandbox, tests |\n| `lib/crates/fabro-sandbox/src/lib.rs` | Add module + re-export |\n| `lib/crates/fabro-sandbox/src/test_support.rs` | Add `captured_commands: Mutex>` to MockSandbox |\n| `lib/crates/fabro-agent/src/sandbox.rs` | Add WorktreeSandbox re-export |\n| `lib/crates/fabro-workflows/src/handler/parallel.rs` | Remove old WorktreeSandbox, use new one |\n| `lib/crates/fabro-cli/src/commands/run.rs` | Replace setup_worktree + run_from_branch worktree logic |\n\n## Functions that become removable\n\n| Function | Location | Reason |\n|---|---|---|\n| `setup_worktree()` | `run.rs:1696` | Logic absorbed into WorktreeSandbox |\n| Old `WorktreeSandbox` struct | `parallel.rs:28-126` | Replaced by shared WorktreeSandbox |\n\nEngine git helpers (`git_add_worktree`, `git_remove_worktree`, etc. in `engine.rs`) stay — still used by parallel cleanup and potentially other callers. Sync git helpers in `git.rs` also stay.\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", - "thread.preflight_lint.current_node": "implement", - "graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ", - "graph.rankdir": "LR", - "last_response": "Zero failures and zero errors. Let me provide a summary of all changes made:\n\n## Summary of Changes\n\n### New file: `lib/crates/fabro-sandbox/src/worktree.rs`\n- Defines `WorktreeEvent` enum (BranchCrea", - "graph.goal": "# Unified WorktreeSandbox\n\n## Context\n\nWorktree management is currently split across two locations with duplicated logic:\n\n1. **Parallel branches** (`parallel.rs`): Inline git commands in `ParallelHandler::execute()`, a thin `WorktreeSandbox` decorator, and separate local/remote code paths\n2. **Top-level CLI run** (`run.rs`): A `setup_worktree()` function using synchronous git helpers, with the worktree path fed into a plain `LocalSandbox`\n\nThe goal is a single `WorktreeSandbox` type that wraps any `Arc`, manages the worktree lifecycle in `initialize()`/`cleanup()`, and eliminates the local/remote branching.\n\n## Plan\n\n### Step 1: Create `WorktreeSandbox` in fabro-sandbox\n\n**New file:** `lib/crates/fabro-sandbox/src/worktree.rs`\n\nDefine:\n\n```rust\npub enum WorktreeEvent {\n BranchCreated { branch: String, sha: String },\n WorktreeAdded { path: String, branch: String },\n WorktreeRemoved { path: String },\n Reset { sha: String },\n}\n\npub type WorktreeEventCallback = Arc;\n\npub struct WorktreeConfig {\n pub branch_name: String,\n pub base_sha: String,\n pub worktree_path: String,\n /// Skip branch creation and reset (for resume, where branch already exists).\n pub skip_branch_creation: bool,\n}\n\npub struct WorktreeSandbox {\n inner: Arc,\n config: WorktreeConfig,\n event_callback: Option,\n}\n```\n\n**Constructor + getters:** `new(inner, config)`, `set_event_callback()`, `branch_name()`, `base_sha()`, `worktree_path()`\n\n**`initialize()`:**\n1. If `!skip_branch_creation`: `git branch --force {branch_name} {base_sha}` via `inner.exec_command()`, emit `BranchCreated`\n2. `git worktree remove --force {path}` (best-effort), then `git worktree add {path} {branch}`, emit `WorktreeAdded`\n3. If `!skip_branch_creation`: `git reset --hard {base_sha}` in worktree dir, emit `Reset`\n\nDoes NOT call `inner.initialize()` — the inner sandbox's lifecycle is managed separately.\n\n**`cleanup()`:** `git worktree remove --force {path}`, emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`.\n\n**`working_directory()`:** Returns `config.worktree_path`.\n\n**`exec_command()`:** Defaults `working_dir` to `config.worktree_path` when `None`, delegates to inner.\n\n**All other Sandbox methods:** Delegate to inner. Must be a manual `impl Sandbox` block (can't use `delegate_sandbox!` since it generates `initialize`/`cleanup`/`working_directory`/`exec_command` which we need to override).\n\nAll interpolated values in git commands use `shell_quote()`.\n\n### Step 2: Register module and re-exports\n\n- `lib/crates/fabro-sandbox/src/lib.rs`: Add `pub mod worktree;` and `pub use worktree::WorktreeSandbox;`\n- `lib/crates/fabro-agent/src/sandbox.rs`: Add re-export of `WorktreeSandbox`\n\n### Step 3: Unit tests for WorktreeSandbox\n\nIn `worktree.rs` `#[cfg(test)]` module, using `MockSandbox`:\n\n- `initialize()` issues correct git commands (branch, worktree remove, worktree add, reset) and emits events\n- `skip_branch_creation` skips branch + reset, only does worktree add\n- `cleanup()` issues `worktree remove` and emits `WorktreeRemoved`\n- `working_directory()` returns worktree path\n- `exec_command()` with `None` working_dir defaults to worktree path\n- `exec_command()` with explicit working_dir passes it through\n- `initialize()` propagates errors on non-zero exit\n\n**MockSandbox enhancement:** Add `captured_commands: Mutex>` field to `test_support.rs` to capture the sequence of `exec_command` calls (current `captured_command` only stores the last one). Append to vec in `exec_command()` impl.\n\n### Step 4: Refactor parallel.rs\n\n- **Remove** the private `WorktreeSandbox` struct (lines 28-126) and `use fabro_agent::LocalSandbox`\n- **Replace** the inline git setup loop (lines 361-450) with:\n - Construct `WorktreeConfig` with branch name, base SHA, worktree path\n - Create `WorktreeSandbox::new(Arc::clone(&services.sandbox), config)`\n - Wire event callback to bridge `WorktreeEvent` → `WorkflowRunEvent`\n - Call `initialize().await`\n- This eliminates the `if services.sandbox.is_remote()` branch (lines 442-449) — `WorktreeSandbox` works the same for any inner sandbox\n- **Cleanup loop** (lines 659-668): Keep calling `git_remove_worktree()` on the parent sandbox (the `WorktreeSandbox` Arc is consumed by the spawned task and dropped). Alternatively, could store the sandbox Arc in `BranchResult` and call `.cleanup()`, but the current approach is simpler.\n\n### Step 5: Refactor run.rs — new runs\n\nReplace `setup_worktree()` call (lines 830-845) + separate `LocalSandbox` construction with:\n\n```\nif workdir_strategy == LocalWorktree:\n base_sha = git::head_sha()\n branch_name = \"fabro/run/{run_id}\"\n inner = Arc::new(LocalSandbox::new(original_cwd))\n wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { ... })\n wt_sandbox.set_event_callback(bridge to WorkflowRunEvent)\n wt_sandbox.initialize().await\n std::env::set_current_dir(&worktree_path) // stays in CLI, not in sandbox\n sandbox = Arc::new(wt_sandbox)\n // store base_sha, branch_name for RunConfig\n```\n\n**Delete** the `setup_worktree()` function (lines 1696-1714) — its logic is absorbed above.\n\n`std::env::set_current_dir()` stays in `run.rs` — it's a process-global side effect that belongs to the CLI.\n\n### Step 6: Refactor run.rs — resume (run_from_branch)\n\nReplace worktree re-attachment (lines 1810-1822) with:\n\n```\ninner = Arc::new(LocalSandbox::new(original_cwd))\nwt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig {\n branch_name: run_branch,\n base_sha: base_sha.unwrap_or_default(),\n worktree_path: wt_str,\n skip_branch_creation: true, // branch already exists\n})\nwt_sandbox.initialize().await\nstd::env::set_current_dir(&wt)\n```\n\n### Step 7: Verify\n\n- `cargo build --workspace`\n- `cargo test --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- Manual: `fabro run` with worktree mode enabled on a local workflow\n- Manual: `fabro run --run-branch` to test resume path\n\n## Files to modify\n\n| File | Change |\n|---|---|\n| `lib/crates/fabro-sandbox/src/worktree.rs` | **New** — WorktreeSandbox, WorktreeConfig, WorktreeEvent, impl Sandbox, tests |\n| `lib/crates/fabro-sandbox/src/lib.rs` | Add module + re-export |\n| `lib/crates/fabro-sandbox/src/test_support.rs` | Add `captured_commands: Mutex>` to MockSandbox |\n| `lib/crates/fabro-agent/src/sandbox.rs` | Add WorktreeSandbox re-export |\n| `lib/crates/fabro-workflows/src/handler/parallel.rs` | Remove old WorktreeSandbox, use new one |\n| `lib/crates/fabro-cli/src/commands/run.rs` | Replace setup_worktree + run_from_branch worktree logic |\n\n## Functions that become removable\n\n| Function | Location | Reason |\n|---|---|---|\n| `setup_worktree()` | `run.rs:1696` | Logic absorbed into WorktreeSandbox |\n| Old `WorktreeSandbox` struct | `parallel.rs:28-126` | Replaced by shared WorktreeSandbox |\n\nEngine git helpers (`git_add_worktree`, `git_remove_worktree`, etc. in `engine.rs`) stay — still used by parallel cleanup and potentially other callers. Sync git helpers in `git.rs` also stay.\n", - "internal.retry_count.start": 1, - "failure_signature": "", - "last_stage": "implement", - "internal.thread_id": "preflight_lint", - "current_node": "implement", - "internal.fidelity": "compact", - "outcome": "success", - "command.stderr": "", - "thread.toolchain.current_node": "preflight_compile", + "last_response": "Zero warnings. Here's a summary of what was fixed:\n\n## Fixes Applied\n\n### 1. Extracted `WorktreeEvent → WorkflowRunEvent` callback helper (3 → 1)\nThe same 12-line match block mapping `WorktreeEven", + "current.preamble": "Goal: # Unified WorktreeSandbox\n\n## Context\n\nWorktree management is currently split across two locations with duplicated logic:\n\n1. **Parallel branches** (`parallel.rs`): Inline git commands in `ParallelHandler::execute()`, a thin `WorktreeSandbox` decorator, and separate local/remote code paths\n2. **Top-level CLI run** (`run.rs`): A `setup_worktree()` function using synchronous git helpers, with the worktree path fed into a plain `LocalSandbox`\n\nThe goal is a single `WorktreeSandbox` type that wraps any `Arc`, manages the worktree lifecycle in `initialize()`/`cleanup()`, and eliminates the local/remote branching.\n\n## Plan\n\n### Step 1: Create `WorktreeSandbox` in fabro-sandbox\n\n**New file:** `lib/crates/fabro-sandbox/src/worktree.rs`\n\nDefine:\n\n```rust\npub enum WorktreeEvent {\n BranchCreated { branch: String, sha: String },\n WorktreeAdded { path: String, branch: String },\n WorktreeRemoved { path: String },\n Reset { sha: String },\n}\n\npub type WorktreeEventCallback = Arc;\n\npub struct WorktreeConfig {\n pub branch_name: String,\n pub base_sha: String,\n pub worktree_path: String,\n /// Skip branch creation and reset (for resume, where branch already exists).\n pub skip_branch_creation: bool,\n}\n\npub struct WorktreeSandbox {\n inner: Arc,\n config: WorktreeConfig,\n event_callback: Option,\n}\n```\n\n**Constructor + getters:** `new(inner, config)`, `set_event_callback()`, `branch_name()`, `base_sha()`, `worktree_path()`\n\n**`initialize()`:**\n1. If `!skip_branch_creation`: `git branch --force {branch_name} {base_sha}` via `inner.exec_command()`, emit `BranchCreated`\n2. `git worktree remove --force {path}` (best-effort), then `git worktree add {path} {branch}`, emit `WorktreeAdded`\n3. If `!skip_branch_creation`: `git reset --hard {base_sha}` in worktree dir, emit `Reset`\n\nDoes NOT call `inner.initialize()` — the inner sandbox's lifecycle is managed separately.\n\n**`cleanup()`:** `git worktree remove --force {path}`, emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`.\n\n**`working_directory()`:** Returns `config.worktree_path`.\n\n**`exec_command()`:** Defaults `working_dir` to `config.worktree_path` when `None`, delegates to inner.\n\n**All other Sandbox methods:** Delegate to inner. Must be a manual `impl Sandbox` block (can't use `delegate_sandbox!` since it generates `initialize`/`cleanup`/`working_directory`/`exec_command` which we need to override).\n\nAll interpolated values in git commands use `shell_quote()`.\n\n### Step 2: Register module and re-exports\n\n- `lib/crates/fabro-sandbox/src/lib.rs`: Add `pub mod worktree;` and `pub use worktree::WorktreeSandbox;`\n- `lib/crates/fabro-agent/src/sandbox.rs`: Add re-export of `WorktreeSandbox`\n\n### Step 3: Unit tests for WorktreeSandbox\n\nIn `worktree.rs` `#[cfg(test)]` module, using `MockSandbox`:\n\n- `initialize()` issues correct git commands (branch, worktree remove, worktree add, reset) and emits events\n- `skip_branch_creation` skips branch + reset, only does worktree add\n- `cleanup()` issues `worktree remove` and emits `WorktreeRemoved`\n- `working_directory()` returns worktree path\n- `exec_command()` with `None` working_dir defaults to worktree path\n- `exec_command()` with explicit working_dir passes it through\n- `initialize()` propagates errors on non-zero exit\n\n**MockSandbox enhancement:** Add `captured_commands: Mutex>` field to `test_support.rs` to capture the sequence of `exec_command` calls (current `captured_command` only stores the last one). Append to vec in `exec_command()` impl.\n\n### Step 4: Refactor parallel.rs\n\n- **Remove** the private `WorktreeSandbox` struct (lines 28-126) and `use fabro_agent::LocalSandbox`\n- **Replace** the inline git setup loop (lines 361-450) with:\n - Construct `WorktreeConfig` with branch name, base SHA, worktree path\n - Create `WorktreeSandbox::new(Arc::clone(&services.sandbox), config)`\n - Wire event callback to bridge `WorktreeEvent` → `WorkflowRunEvent`\n - Call `initialize().await`\n- This eliminates the `if services.sandbox.is_remote()` branch (lines 442-449) — `WorktreeSandbox` works the same for any inner sandbox\n- **Cleanup loop** (lines 659-668): Keep calling `git_remove_worktree()` on the parent sandbox (the `WorktreeSandbox` Arc is consumed by the spawned task and dropped). Alternatively, could store the sandbox Arc in `BranchResult` and call `.cleanup()`, but the current approach is simpler.\n\n### Step 5: Refactor run.rs — new runs\n\nReplace `setup_worktree()` call (lines 830-845) + separate `LocalSandbox` construction with:\n\n```\nif workdir_strategy == LocalWorktree:\n base_sha = git::head_sha()\n branch_name = \"fabro/run/{run_id}\"\n inner = Arc::new(LocalSandbox::new(original_cwd))\n wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { ... })\n wt_sandbox.set_event_callback(bridge to WorkflowRunEvent)\n wt_sandbox.initialize().await\n std::env::set_current_dir(&worktree_path) // stays in CLI, not in sandbox\n sandbox = Arc::new(wt_sandbox)\n // store base_sha, branch_name for RunConfig\n```\n\n**Delete** the `setup_worktree()` function (lines 1696-1714) — its logic is absorbed above.\n\n`std::env::set_current_dir()` stays in `run.rs` — it's a process-global side effect that belongs to the CLI.\n\n### Step 6: Refactor run.rs — resume (run_from_branch)\n\nReplace worktree re-attachment (lines 1810-1822) with:\n\n```\ninner = Arc::new(LocalSandbox::new(original_cwd))\nwt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig {\n branch_name: run_branch,\n base_sha: base_sha.unwrap_or_default(),\n worktree_path: wt_str,\n skip_branch_creation: true, // branch already exists\n})\nwt_sandbox.initialize().await\nstd::env::set_current_dir(&wt)\n```\n\n### Step 7: Verify\n\n- `cargo build --workspace`\n- `cargo test --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- Manual: `fabro run` with worktree mode enabled on a local workflow\n- Manual: `fabro run --run-branch` to test resume path\n\n## Files to modify\n\n| File | Change |\n|---|---|\n| `lib/crates/fabro-sandbox/src/worktree.rs` | **New** — WorktreeSandbox, WorktreeConfig, WorktreeEvent, impl Sandbox, tests |\n| `lib/crates/fabro-sandbox/src/lib.rs` | Add module + re-export |\n| `lib/crates/fabro-sandbox/src/test_support.rs` | Add `captured_commands: Mutex>` to MockSandbox |\n| `lib/crates/fabro-agent/src/sandbox.rs` | Add WorktreeSandbox re-export |\n| `lib/crates/fabro-workflows/src/handler/parallel.rs` | Remove old WorktreeSandbox, use new one |\n| `lib/crates/fabro-cli/src/commands/run.rs` | Replace setup_worktree + run_from_branch worktree logic |\n\n## Functions that become removable\n\n| Function | Location | Reason |\n|---|---|---|\n| `setup_worktree()` | `run.rs:1696` | Logic absorbed into WorktreeSandbox |\n| Old `WorktreeSandbox` struct | `parallel.rs:28-126` | Replaced by shared WorktreeSandbox |\n\nEngine git helpers (`git_add_worktree`, `git_remove_worktree`, etc. in `engine.rs`) stay — still used by parallel cleanup and potentially other callers. Sync git helpers in `git.rs` also stay.\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-sonnet-4-6, 190.9k tokens in / 105.0k out\n - Files: /home/daytona/workspace/lib/crates/fabro-agent/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-agent/src/sandbox.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/test_support.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/worktree.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/handler/parallel.rs\n", + "last_stage": "simplify_opus", + "response.simplify_opus": "Zero warnings. Here's a summary of what was fixed:\n\n## Fixes Applied\n\n### 1. Extracted `WorktreeEvent → WorkflowRunEvent` callback helper (3 → 1)\nThe same 12-line match block mapping `WorktreeEvent` variants to `WorkflowRunEvent` was duplicated in three places:\n- `run.rs`: new-run path (`SandboxProvider::Local`)\n- `run.rs`: resume path (`run_from_branch`)\n- `parallel.rs`: branch worktree setup\n\n**Fix:** Added `fn worktree_event_callback(emitter: Arc) -> WorktreeEventCallback` to each file as a local helper, replacing all three inline match blocks with a single call.\n\n### 2. Extracted `LocalSandbox + event_callback` helper (3 → 1)\nIn `run.rs`, the pattern `let mut env = LocalSandbox::new(cwd); env.set_event_callback(...); Arc::new(env)` appeared twice in the same match arm (fallback on error + else branch) and again in `run_from_branch`.\n\n**Fix:** Added `fn local_sandbox_with_callback(cwd: PathBuf, emitter: Arc) -> Arc` and replaced all three sites.\n\n### 3. Eliminated `.clone().unwrap()` on `Option`\n`worktree_base_sha.clone().unwrap()` (and same for `branch_name`, `wt_path`) cloned the entire `Option` before unwrapping. \n\n**Fix:** Changed `if worktree_base_sha.is_some()` to `if let (Some(base_sha), Some(branch_name), Some(wt_path)) = (worktree_base_sha.as_ref(), ...)` — borrows the inner values directly, then only clones the `String`/`PathBuf` when actually needed for `WorktreeConfig`.\n\n### 4. Fixed `.to_string_lossy().to_string()` → `.into_owned()` (3 sites)\n`.to_string_lossy().to_string()` calls `.to_string()` on a `Cow`, which allocates a new `String` even when `Cow` is already `Owned`. `.into_owned()` moves directly from `Cow::Owned` without the extra allocation.", + "current_node": "simplify_opus", "internal.node_visit_count": 1, "response.implement": "Zero failures and zero errors. Let me provide a summary of all changes made:\n\n## Summary of Changes\n\n### New file: `lib/crates/fabro-sandbox/src/worktree.rs`\n- Defines `WorktreeEvent` enum (BranchCreated, WorktreeAdded, WorktreeRemoved, Reset)\n- Defines `WorktreeEventCallback` type alias\n- Defines `WorktreeConfig` struct (branch_name, base_sha, worktree_path, skip_branch_creation)\n- Implements `WorktreeSandbox` struct wrapping `Arc` with:\n - `initialize()`: runs git branch + worktree remove + worktree add + reset, emitting events; does NOT call inner's `initialize()`\n - `cleanup()`: runs git worktree remove, emits WorktreeRemoved; does NOT call inner's `cleanup()`\n - `working_directory()`: returns `config.worktree_path`\n - `exec_command()`: defaults `working_dir` to `config.worktree_path` when `None`\n - All other `Sandbox` methods delegate to inner\n- 14 unit tests covering all specified behaviors (TDD green)\n\n### Modified: `lib/crates/fabro-sandbox/src/test_support.rs`\n- Added `captured_commands: Mutex>` to `MockSandbox` (captures all commands in sequence)\n- Added `captured_working_dirs: Mutex>>` to `MockSandbox` (captures working_dir per call)\n- Updated `exec_command()` to populate both new fields alongside existing `captured_command`\n\n### Modified: `lib/crates/fabro-sandbox/src/lib.rs`\n- Added `pub mod worktree;`\n- Added `pub use worktree::{WorktreeConfig, WorktreeEvent, WorktreeEventCallback, WorktreeSandbox};`\n\n### Modified: `lib/crates/fabro-agent/src/sandbox.rs` + `lib.rs`\n- Re-exported `WorktreeConfig`, `WorktreeEvent`, `WorktreeEventCallback`, `WorktreeSandbox` from `fabro_sandbox`\n- Also re-exported `shell_quote` (which was already in `sandbox.rs` but not in `lib.rs`)\n\n### Modified: `lib/crates/fabro-workflows/src/handler/parallel.rs`\n- Removed the private `WorktreeSandbox` struct and its full `impl Sandbox` (126 lines deleted)\n- Removed `use fabro_agent::LocalSandbox`\n- Added `use fabro_agent::{WorktreeConfig, WorktreeEvent, WorktreeSandbox}`\n- Replaced the inline git setup loop (branch creation, worktree add, reset, `is_remote()` branch for sandbox type) with a `WorktreeSandbox` that handles all of this uniformly for any inner sandbox\n\n### Modified: `lib/crates/fabro-cli/src/commands/run.rs`\n- Added `WorktreeConfig`, `WorktreeEvent`, `WorktreeSandbox` to imports\n- **New runs**: Replaced `setup_worktree()` call with inline config computation (head_sha, branch_name, worktree_path), and replaced `LocalSandbox::new(cwd)` in `SandboxProvider::Local` with `WorktreeSandbox::new(LocalSandbox(original_cwd), config)` + `initialize().await` + `set_current_dir()`; graceful fallback on failure\n- **Resume** (`run_from_branch`): Replaced `replace_worktree()` + `LocalSandbox::new(wt)` with `WorktreeSandbox::new(LocalSandbox(original_cwd), {skip_branch_creation: true})` + `initialize().await`\n- Deleted `setup_worktree()` function (logic absorbed into `WorktreeSandbox`)\n- Replaced `worktree_work_dir.is_some()` with `worktree_path.is_some()` for `meta_branch` and `git_checkpoint_enabled`", "internal.retry_count.preflight_compile": 1, "internal.retry_count.implement": 1, + "internal.run_id": "01KM4CC4HA0SYCXGQW7BJWJE8F", + "thread.preflight_lint.current_node": "implement", + "internal.retry_count.toolchain": 1, + "command.output": "", + "graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ", + "graph.rankdir": "LR", + "internal.retry_count.simplify_opus": 1, + "graph.goal": "# Unified WorktreeSandbox\n\n## Context\n\nWorktree management is currently split across two locations with duplicated logic:\n\n1. **Parallel branches** (`parallel.rs`): Inline git commands in `ParallelHandler::execute()`, a thin `WorktreeSandbox` decorator, and separate local/remote code paths\n2. **Top-level CLI run** (`run.rs`): A `setup_worktree()` function using synchronous git helpers, with the worktree path fed into a plain `LocalSandbox`\n\nThe goal is a single `WorktreeSandbox` type that wraps any `Arc`, manages the worktree lifecycle in `initialize()`/`cleanup()`, and eliminates the local/remote branching.\n\n## Plan\n\n### Step 1: Create `WorktreeSandbox` in fabro-sandbox\n\n**New file:** `lib/crates/fabro-sandbox/src/worktree.rs`\n\nDefine:\n\n```rust\npub enum WorktreeEvent {\n BranchCreated { branch: String, sha: String },\n WorktreeAdded { path: String, branch: String },\n WorktreeRemoved { path: String },\n Reset { sha: String },\n}\n\npub type WorktreeEventCallback = Arc;\n\npub struct WorktreeConfig {\n pub branch_name: String,\n pub base_sha: String,\n pub worktree_path: String,\n /// Skip branch creation and reset (for resume, where branch already exists).\n pub skip_branch_creation: bool,\n}\n\npub struct WorktreeSandbox {\n inner: Arc,\n config: WorktreeConfig,\n event_callback: Option,\n}\n```\n\n**Constructor + getters:** `new(inner, config)`, `set_event_callback()`, `branch_name()`, `base_sha()`, `worktree_path()`\n\n**`initialize()`:**\n1. If `!skip_branch_creation`: `git branch --force {branch_name} {base_sha}` via `inner.exec_command()`, emit `BranchCreated`\n2. `git worktree remove --force {path}` (best-effort), then `git worktree add {path} {branch}`, emit `WorktreeAdded`\n3. If `!skip_branch_creation`: `git reset --hard {base_sha}` in worktree dir, emit `Reset`\n\nDoes NOT call `inner.initialize()` — the inner sandbox's lifecycle is managed separately.\n\n**`cleanup()`:** `git worktree remove --force {path}`, emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`.\n\n**`working_directory()`:** Returns `config.worktree_path`.\n\n**`exec_command()`:** Defaults `working_dir` to `config.worktree_path` when `None`, delegates to inner.\n\n**All other Sandbox methods:** Delegate to inner. Must be a manual `impl Sandbox` block (can't use `delegate_sandbox!` since it generates `initialize`/`cleanup`/`working_directory`/`exec_command` which we need to override).\n\nAll interpolated values in git commands use `shell_quote()`.\n\n### Step 2: Register module and re-exports\n\n- `lib/crates/fabro-sandbox/src/lib.rs`: Add `pub mod worktree;` and `pub use worktree::WorktreeSandbox;`\n- `lib/crates/fabro-agent/src/sandbox.rs`: Add re-export of `WorktreeSandbox`\n\n### Step 3: Unit tests for WorktreeSandbox\n\nIn `worktree.rs` `#[cfg(test)]` module, using `MockSandbox`:\n\n- `initialize()` issues correct git commands (branch, worktree remove, worktree add, reset) and emits events\n- `skip_branch_creation` skips branch + reset, only does worktree add\n- `cleanup()` issues `worktree remove` and emits `WorktreeRemoved`\n- `working_directory()` returns worktree path\n- `exec_command()` with `None` working_dir defaults to worktree path\n- `exec_command()` with explicit working_dir passes it through\n- `initialize()` propagates errors on non-zero exit\n\n**MockSandbox enhancement:** Add `captured_commands: Mutex>` field to `test_support.rs` to capture the sequence of `exec_command` calls (current `captured_command` only stores the last one). Append to vec in `exec_command()` impl.\n\n### Step 4: Refactor parallel.rs\n\n- **Remove** the private `WorktreeSandbox` struct (lines 28-126) and `use fabro_agent::LocalSandbox`\n- **Replace** the inline git setup loop (lines 361-450) with:\n - Construct `WorktreeConfig` with branch name, base SHA, worktree path\n - Create `WorktreeSandbox::new(Arc::clone(&services.sandbox), config)`\n - Wire event callback to bridge `WorktreeEvent` → `WorkflowRunEvent`\n - Call `initialize().await`\n- This eliminates the `if services.sandbox.is_remote()` branch (lines 442-449) — `WorktreeSandbox` works the same for any inner sandbox\n- **Cleanup loop** (lines 659-668): Keep calling `git_remove_worktree()` on the parent sandbox (the `WorktreeSandbox` Arc is consumed by the spawned task and dropped). Alternatively, could store the sandbox Arc in `BranchResult` and call `.cleanup()`, but the current approach is simpler.\n\n### Step 5: Refactor run.rs — new runs\n\nReplace `setup_worktree()` call (lines 830-845) + separate `LocalSandbox` construction with:\n\n```\nif workdir_strategy == LocalWorktree:\n base_sha = git::head_sha()\n branch_name = \"fabro/run/{run_id}\"\n inner = Arc::new(LocalSandbox::new(original_cwd))\n wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { ... })\n wt_sandbox.set_event_callback(bridge to WorkflowRunEvent)\n wt_sandbox.initialize().await\n std::env::set_current_dir(&worktree_path) // stays in CLI, not in sandbox\n sandbox = Arc::new(wt_sandbox)\n // store base_sha, branch_name for RunConfig\n```\n\n**Delete** the `setup_worktree()` function (lines 1696-1714) — its logic is absorbed above.\n\n`std::env::set_current_dir()` stays in `run.rs` — it's a process-global side effect that belongs to the CLI.\n\n### Step 6: Refactor run.rs — resume (run_from_branch)\n\nReplace worktree re-attachment (lines 1810-1822) with:\n\n```\ninner = Arc::new(LocalSandbox::new(original_cwd))\nwt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig {\n branch_name: run_branch,\n base_sha: base_sha.unwrap_or_default(),\n worktree_path: wt_str,\n skip_branch_creation: true, // branch already exists\n})\nwt_sandbox.initialize().await\nstd::env::set_current_dir(&wt)\n```\n\n### Step 7: Verify\n\n- `cargo build --workspace`\n- `cargo test --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- Manual: `fabro run` with worktree mode enabled on a local workflow\n- Manual: `fabro run --run-branch` to test resume path\n\n## Files to modify\n\n| File | Change |\n|---|---|\n| `lib/crates/fabro-sandbox/src/worktree.rs` | **New** — WorktreeSandbox, WorktreeConfig, WorktreeEvent, impl Sandbox, tests |\n| `lib/crates/fabro-sandbox/src/lib.rs` | Add module + re-export |\n| `lib/crates/fabro-sandbox/src/test_support.rs` | Add `captured_commands: Mutex>` to MockSandbox |\n| `lib/crates/fabro-agent/src/sandbox.rs` | Add WorktreeSandbox re-export |\n| `lib/crates/fabro-workflows/src/handler/parallel.rs` | Remove old WorktreeSandbox, use new one |\n| `lib/crates/fabro-cli/src/commands/run.rs` | Replace setup_worktree + run_from_branch worktree logic |\n\n## Functions that become removable\n\n| Function | Location | Reason |\n|---|---|---|\n| `setup_worktree()` | `run.rs:1696` | Logic absorbed into WorktreeSandbox |\n| Old `WorktreeSandbox` struct | `parallel.rs:28-126` | Replaced by shared WorktreeSandbox |\n\nEngine git helpers (`git_add_worktree`, `git_remove_worktree`, etc. in `engine.rs`) stay — still used by parallel cleanup and potentially other callers. Sync git helpers in `git.rs` also stay.\n", + "internal.retry_count.start": 1, + "failure_signature": "", + "internal.thread_id": "implement", + "outcome": "success", + "internal.fidelity": "compact", + "command.stderr": "", + "thread.toolchain.current_node": "preflight_compile", + "thread.implement.current_node": "simplify_opus", "failure_class": "", "thread.preflight_compile.current_node": "preflight_lint", "thread.start.current_node": "toolchain" @@ -82,6 +87,29 @@ "notes": "Script completed: cargo check -q --workspace 2>&1", "duration_ms": 71558 }, + "simplify_opus": { + "status": "success", + "context_updates": { + "last_stage": "simplify_opus", + "last_response": "Zero warnings. Here's a summary of what was fixed:\n\n## Fixes Applied\n\n### 1. Extracted `WorktreeEvent → WorkflowRunEvent` callback helper (3 → 1)\nThe same 12-line match block mapping `WorktreeEven", + "response.simplify_opus": "Zero warnings. Here's a summary of what was fixed:\n\n## Fixes Applied\n\n### 1. Extracted `WorktreeEvent → WorkflowRunEvent` callback helper (3 → 1)\nThe same 12-line match block mapping `WorktreeEvent` variants to `WorkflowRunEvent` was duplicated in three places:\n- `run.rs`: new-run path (`SandboxProvider::Local`)\n- `run.rs`: resume path (`run_from_branch`)\n- `parallel.rs`: branch worktree setup\n\n**Fix:** Added `fn worktree_event_callback(emitter: Arc) -> WorktreeEventCallback` to each file as a local helper, replacing all three inline match blocks with a single call.\n\n### 2. Extracted `LocalSandbox + event_callback` helper (3 → 1)\nIn `run.rs`, the pattern `let mut env = LocalSandbox::new(cwd); env.set_event_callback(...); Arc::new(env)` appeared twice in the same match arm (fallback on error + else branch) and again in `run_from_branch`.\n\n**Fix:** Added `fn local_sandbox_with_callback(cwd: PathBuf, emitter: Arc) -> Arc` and replaced all three sites.\n\n### 3. Eliminated `.clone().unwrap()` on `Option`\n`worktree_base_sha.clone().unwrap()` (and same for `branch_name`, `wt_path`) cloned the entire `Option` before unwrapping. \n\n**Fix:** Changed `if worktree_base_sha.is_some()` to `if let (Some(base_sha), Some(branch_name), Some(wt_path)) = (worktree_base_sha.as_ref(), ...)` — borrows the inner values directly, then only clones the `String`/`PathBuf` when actually needed for `WorktreeConfig`.\n\n### 4. Fixed `.to_string_lossy().to_string()` → `.into_owned()` (3 sites)\n`.to_string_lossy().to_string()` calls `.to_string()` on a `Cow`, which allocates a new `String` even when `Cow` is already `Owned`. `.into_owned()` moves directly from `Cow::Owned` without the extra allocation." + }, + "notes": "Stage completed: simplify_opus", + "usage": { + "model": "claude-sonnet-4-6", + "input_tokens": 84776, + "output_tokens": 30187, + "cache_read_tokens": 2554347, + "cache_write_tokens": 107677, + "reasoning_tokens": 4594, + "cost": 0.707133 + }, + "files_touched": [ + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs", + "/home/daytona/workspace/lib/crates/fabro-workflows/src/handler/parallel.rs" + ], + "duration_ms": 905106 + }, "toolchain": { "status": "success", "context_updates": { @@ -105,11 +133,12 @@ "duration_ms": 0 } }, - "next_node_id": "simplify_opus", + "next_node_id": "simplify_gpt", "node_visits": { "preflight_compile": 1, "preflight_lint": 1, "start": 1, + "simplify_opus": 1, "toolchain": 1, "implement": 1 } diff --git a/nodes/implement/diff.patch b/nodes/implement/diff.patch new file mode 100644 index 000000000..5ec533931 --- /dev/null +++ b/nodes/implement/diff.patch @@ -0,0 +1,1243 @@ +diff --git a/lib/crates/fabro-agent/src/lib.rs b/lib/crates/fabro-agent/src/lib.rs +index 614e6a3a..e150b663 100644 +--- a/lib/crates/fabro-agent/src/lib.rs ++++ b/lib/crates/fabro-agent/src/lib.rs +@@ -40,8 +40,8 @@ pub use profiles::{AnthropicProfile, EnvContext, GeminiProfile, OpenAiProfile}; + pub use provider_profile::{ProfileCapabilities, ProviderProfile}; + pub use read_before_write_sandbox::ReadBeforeWriteSandbox; + pub use sandbox::{ +- format_lines_numbered, DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, +- SandboxEventCallback, ++ format_lines_numbered, shell_quote, DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, ++ SandboxEventCallback, WorktreeConfig, WorktreeEvent, WorktreeEventCallback, WorktreeSandbox, + }; + pub use session::Session; + pub use skills::Skill; +diff --git a/lib/crates/fabro-agent/src/sandbox.rs b/lib/crates/fabro-agent/src/sandbox.rs +index 887a5682..706ec106 100644 +--- a/lib/crates/fabro-agent/src/sandbox.rs ++++ b/lib/crates/fabro-agent/src/sandbox.rs +@@ -1,7 +1,7 @@ + // Re-export all sandbox types from fabro-sandbox. + pub use fabro_sandbox::{ + format_lines_numbered, shell_quote, DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, +- SandboxEventCallback, ++ SandboxEventCallback, WorktreeConfig, WorktreeEvent, WorktreeEventCallback, WorktreeSandbox, + }; + + // Re-export the delegate_sandbox! macro at crate root so existing +diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs +index da58742f..6f76725f 100644 +--- a/lib/crates/fabro-cli/src/commands/run.rs ++++ b/lib/crates/fabro-cli/src/commands/run.rs +@@ -7,7 +7,10 @@ use std::time::Instant; + use anyhow::{bail, Context}; + use chrono::{Local, Utc}; + use clap::{Args, ValueEnum}; +-use fabro_agent::{DockerSandbox, DockerSandboxConfig, LocalSandbox, Sandbox}; ++use fabro_agent::{ ++ DockerSandbox, DockerSandboxConfig, LocalSandbox, Sandbox, WorktreeConfig, WorktreeEvent, ++ WorktreeSandbox, ++}; + use fabro_config::run::{RunDefaults, WorkflowRunConfig}; + use fabro_config::{project as project_config, run as run_config, sandbox as sandbox_config}; + use fabro_interview::{AutoApproveInterviewer, ConsoleInterviewer, Interviewer}; +@@ -827,22 +830,29 @@ pub async fn run_command( + } + } + +- // Set up git worktree for local isolation. +- let (worktree_work_dir, worktree_path, worktree_branch, worktree_base_sha) = +- if workdir_strategy == WorkdirStrategy::LocalWorktree { +- match setup_worktree(&original_cwd, &run_dir, &run_id) { +- Ok((wd, wt, branch, base)) => (Some(wd), Some(wt), Some(branch), Some(base)), +- Err(e) => { +- eprintln!( +- "{} Git worktree setup failed ({e}), running without worktree.", +- styles.yellow.apply_to("Warning:"), +- ); +- (None, None, None, None) +- } ++ // Compute worktree configuration for local isolation. ++ // The actual git setup (branch, worktree add, reset) happens inside the sandbox ++ // creation block for SandboxProvider::Local below. ++ let (mut worktree_path, mut worktree_branch, mut worktree_base_sha) = if workdir_strategy ++ == WorkdirStrategy::LocalWorktree ++ { ++ match fabro_workflows::git::head_sha(&original_cwd) { ++ Ok(base_sha) => { ++ let branch_name = format!("{}{run_id}", fabro_workflows::git::RUN_BRANCH_PREFIX); ++ let wt_path = run_dir.join("worktree"); ++ (Some(wt_path), Some(branch_name), Some(base_sha)) + } +- } else { +- (None, None, None, None) +- }; ++ Err(e) => { ++ eprintln!( ++ "{} Git worktree setup failed ({e}), running without worktree.", ++ styles.yellow.apply_to("Warning:"), ++ ); ++ (None, None, None) ++ } ++ } ++ } else { ++ (None, None, None) ++ }; + + if let Some(ref wt) = worktree_path { + progress_ui +@@ -1115,12 +1125,85 @@ pub async fn run_command( + Arc::new(env) + } + SandboxProvider::Local => { +- let mut env = LocalSandbox::new(cwd.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) ++ if worktree_base_sha.is_some() { ++ // Set up a WorktreeSandbox for git-isolated local execution. ++ let base_sha = worktree_base_sha.clone().unwrap(); ++ let branch_name = worktree_branch.clone().unwrap(); ++ let wt_path = worktree_path.clone().unwrap(); ++ let wt_path_str = wt_path.to_string_lossy().to_string(); ++ ++ let mut inner = LocalSandbox::new(original_cwd.clone()); ++ let emitter_inner = Arc::clone(&emitter); ++ inner.set_event_callback(Arc::new(move |event| { ++ emitter_inner ++ .emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); ++ })); ++ ++ let wt_config = WorktreeConfig { ++ branch_name: branch_name.clone(), ++ base_sha: base_sha.clone(), ++ worktree_path: wt_path_str.clone(), ++ skip_branch_creation: false, ++ }; ++ let mut wt_sandbox = WorktreeSandbox::new(Arc::new(inner), wt_config); ++ let emitter_wt = Arc::clone(&emitter); ++ wt_sandbox.set_event_callback(Arc::new(move |event| match event { ++ WorktreeEvent::BranchCreated { branch, sha } => { ++ emitter_wt.emit(&fabro_workflows::event::WorkflowRunEvent::GitBranch { ++ branch, ++ sha, ++ }); ++ } ++ WorktreeEvent::WorktreeAdded { path, branch } => { ++ emitter_wt.emit( ++ &fabro_workflows::event::WorkflowRunEvent::GitWorktreeAdd { ++ path, ++ branch, ++ }, ++ ); ++ } ++ WorktreeEvent::WorktreeRemoved { path } => { ++ emitter_wt.emit( ++ &fabro_workflows::event::WorkflowRunEvent::GitWorktreeRemove { path }, ++ ); ++ } ++ WorktreeEvent::Reset { sha } => { ++ emitter_wt ++ .emit(&fabro_workflows::event::WorkflowRunEvent::GitReset { sha }); ++ } ++ })); ++ ++ match wt_sandbox.initialize().await { ++ Ok(()) => { ++ std::env::set_current_dir(&wt_path)?; ++ Arc::new(wt_sandbox) as Arc ++ } ++ Err(e) => { ++ eprintln!( ++ "{} Git worktree setup failed ({e}), running without worktree.", ++ styles.yellow.apply_to("Warning:"), ++ ); ++ // Reset so RunConfig does not enable git checkpointing ++ worktree_path = None; ++ worktree_branch = None; ++ worktree_base_sha = None; ++ let mut env = LocalSandbox::new(cwd.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) as Arc ++ } ++ } ++ } else { ++ let mut env = LocalSandbox::new(cwd.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) ++ } + } + }; + +@@ -1276,7 +1359,7 @@ pub async fn run_command( + + // 7. Execute + // Set up metadata branch for git checkpointing (host or remote — engine fills remote) +- let meta_branch = if worktree_work_dir.is_some() { ++ let meta_branch = if worktree_path.is_some() { + Some(fabro_workflows::git::MetadataStore::branch_name(&run_id)) + } else { + None +@@ -1290,7 +1373,7 @@ pub async fn run_command( + cancel_token: None, + dry_run: dry_run_mode, + run_id: run_id.clone(), +- git_checkpoint_enabled: worktree_work_dir.is_some(), ++ git_checkpoint_enabled: worktree_path.is_some(), + host_repo_path: Some(original_cwd.clone()), + base_sha: worktree_base_sha, + run_branch: worktree_branch, +@@ -1690,29 +1773,6 @@ pub async fn run_command( + } + } + +-/// Set up a git worktree for an isolated workflow run. +-/// Caller must have already verified the repo is clean via `git::ensure_clean`. +-/// Returns (work_dir, worktree_path, branch_name, base_sha) on success. +-fn setup_worktree( +- original_cwd: &std::path::Path, +- run_dir: &std::path::Path, +- run_id: &str, +-) -> anyhow::Result<(PathBuf, PathBuf, String, String)> { +- let base_sha = +- fabro_workflows::git::head_sha(original_cwd).map_err(|e| anyhow::anyhow!("{e}"))?; +- let branch_name = format!("{}{run_id}", fabro_workflows::git::RUN_BRANCH_PREFIX); +- fabro_workflows::git::create_branch(original_cwd, &branch_name) +- .map_err(|e| anyhow::anyhow!("{e}"))?; +- +- let worktree_path = run_dir.join("worktree"); +- fabro_workflows::git::replace_worktree(original_cwd, &worktree_path, &branch_name) +- .map_err(|e| anyhow::anyhow!("{e}"))?; +- +- std::env::set_current_dir(&worktree_path)?; +- +- Ok((worktree_path.clone(), worktree_path, branch_name, base_sha)) +-} +- + /// Resume a workflow run from a git run branch. + /// + /// Reads the checkpoint, manifest, and graph DOT from the metadata branch +@@ -1808,18 +1868,59 @@ async fn run_from_branch( + let (sandbox, worktree_path): (Arc, Option) = + match sandbox_provider { + SandboxProvider::Local | SandboxProvider::Docker => { +- // Re-attach worktree to the existing run branch ++ // Re-attach worktree to the existing run branch via WorktreeSandbox. + let wt = run_dir.join("worktree"); +- fabro_workflows::git::replace_worktree(&original_cwd, &wt, run_branch).map_err( +- |e| anyhow::anyhow!("failed to attach worktree to {run_branch}: {e}"), +- )?; +- std::env::set_current_dir(&wt)?; +- let mut env = fabro_agent::LocalSandbox::new(wt.clone()); +- let emitter_cb = Arc::clone(&emitter); +- env.set_event_callback(Arc::new(move |event| { +- emitter_cb.emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); ++ let wt_str = wt.to_string_lossy().to_string(); ++ ++ let mut inner = fabro_agent::LocalSandbox::new(original_cwd.clone()); ++ let emitter_inner = Arc::clone(&emitter); ++ inner.set_event_callback(Arc::new(move |event| { ++ emitter_inner ++ .emit(&fabro_workflows::event::WorkflowRunEvent::Sandbox { event }); + })); +- (Arc::new(env), Some(wt)) ++ ++ 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(Arc::new(inner), wt_config); ++ let emitter_wt = Arc::clone(&emitter); ++ wt_sandbox.set_event_callback(Arc::new(move |event| match event { ++ WorktreeEvent::BranchCreated { branch, sha } => { ++ emitter_wt.emit(&fabro_workflows::event::WorkflowRunEvent::GitBranch { ++ branch, ++ sha, ++ }); ++ } ++ WorktreeEvent::WorktreeAdded { path, branch } => { ++ emitter_wt.emit( ++ &fabro_workflows::event::WorkflowRunEvent::GitWorktreeAdd { ++ path, ++ branch, ++ }, ++ ); ++ } ++ WorktreeEvent::WorktreeRemoved { path } => { ++ emitter_wt.emit( ++ &fabro_workflows::event::WorkflowRunEvent::GitWorktreeRemove { path }, ++ ); ++ } ++ WorktreeEvent::Reset { sha } => { ++ emitter_wt ++ .emit(&fabro_workflows::event::WorkflowRunEvent::GitReset { sha }); ++ } ++ })); ++ ++ 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 => { +diff --git a/lib/crates/fabro-sandbox/src/lib.rs b/lib/crates/fabro-sandbox/src/lib.rs +index b54a8d70..aa62a339 100644 +--- a/lib/crates/fabro-sandbox/src/lib.rs ++++ b/lib/crates/fabro-sandbox/src/lib.rs +@@ -2,6 +2,8 @@ pub mod sandbox; + + pub mod read_guard; + ++pub mod worktree; ++ + #[cfg(feature = "ssh")] + pub(crate) mod ssh_common; + +@@ -33,6 +35,8 @@ pub use sandbox::{ + + pub use read_guard::ReadBeforeWriteSandbox; + ++pub use worktree::{WorktreeConfig, WorktreeEvent, WorktreeEventCallback, WorktreeSandbox}; ++ + #[cfg(feature = "local")] + pub use local::LocalSandbox; + +diff --git a/lib/crates/fabro-sandbox/src/test_support.rs b/lib/crates/fabro-sandbox/src/test_support.rs +index e8b01b1e..79cbb65d 100644 +--- a/lib/crates/fabro-sandbox/src/test_support.rs ++++ b/lib/crates/fabro-sandbox/src/test_support.rs +@@ -20,8 +20,12 @@ pub struct MockSandbox { + pub written_files: Mutex>, + /// Captures the `timeout_ms` argument from `exec_command` calls. + pub captured_timeout: Mutex>, +- /// Captures the `command` argument from `exec_command` calls. ++ /// Captures the `command` argument from `exec_command` calls (last only). + pub captured_command: Mutex>, ++ /// Captures all `command` arguments from `exec_command` calls in order. ++ pub captured_commands: Mutex>, ++ /// Captures all `working_dir` arguments from `exec_command` calls in order. ++ pub captured_working_dirs: Mutex>>, + /// Captures the `env_vars` argument from `exec_command` calls. + pub captured_env_vars: Mutex>>, + pub event_callback: Option, +@@ -67,6 +71,8 @@ impl Default for MockSandbox { + written_files: Mutex::new(Vec::new()), + captured_timeout: Mutex::new(None), + captured_command: Mutex::new(None), ++ captured_commands: Mutex::new(Vec::new()), ++ captured_working_dirs: Mutex::new(Vec::new()), + captured_env_vars: Mutex::new(None), + event_callback: None, + } +@@ -126,7 +132,7 @@ impl Sandbox for MockSandbox { + &self, + command: &str, + timeout_ms: u64, +- _working_dir: Option<&str>, ++ working_dir: Option<&str>, + env_vars: Option<&std::collections::HashMap>, + _cancel_token: Option, + ) -> Result { +@@ -138,6 +144,14 @@ impl Sandbox for MockSandbox { + .captured_command + .lock() + .expect("captured_command lock poisoned") = Some(command.to_string()); ++ self.captured_commands ++ .lock() ++ .expect("captured_commands lock poisoned") ++ .push(command.to_string()); ++ self.captured_working_dirs ++ .lock() ++ .expect("captured_working_dirs lock poisoned") ++ .push(working_dir.map(String::from)); + *self + .captured_env_vars + .lock() +diff --git a/lib/crates/fabro-sandbox/src/worktree.rs b/lib/crates/fabro-sandbox/src/worktree.rs +new file mode 100644 +index 00000000..3283ae14 +--- /dev/null ++++ b/lib/crates/fabro-sandbox/src/worktree.rs +@@ -0,0 +1,637 @@ ++use async_trait::async_trait; ++use std::collections::HashMap; ++use std::path::Path; ++use std::sync::Arc; ++use tokio_util::sync::CancellationToken; ++ ++use crate::{shell_quote, DirEntry, ExecResult, GrepOptions, Sandbox}; ++ ++/// Git command prefix that disables background maintenance. ++const GIT: &str = "git -c maintenance.auto=0 -c gc.auto=0"; ++ ++// --------------------------------------------------------------------------- ++// Public types ++// --------------------------------------------------------------------------- ++ ++/// Events emitted during worktree lifecycle operations. ++pub enum WorktreeEvent { ++ BranchCreated { branch: String, sha: String }, ++ WorktreeAdded { path: String, branch: String }, ++ WorktreeRemoved { path: String }, ++ Reset { sha: String }, ++} ++ ++/// Callback type for worktree lifecycle events. ++pub type WorktreeEventCallback = Arc; ++ ++/// Configuration for a `WorktreeSandbox`. ++pub struct WorktreeConfig { ++ pub branch_name: String, ++ pub base_sha: String, ++ pub worktree_path: String, ++ /// Skip branch creation and hard reset (for resume, where branch already exists). ++ pub skip_branch_creation: bool, ++} ++ ++/// Wraps any `Sandbox`, manages a git worktree lifecycle in `initialize()`/`cleanup()`, ++/// and overrides `working_directory()` and `exec_command()` to use the worktree path. ++/// ++/// `initialize()` and `cleanup()` do NOT call the inner sandbox's lifecycle methods. ++/// The inner sandbox's lifecycle is managed separately by the caller. ++pub struct WorktreeSandbox { ++ inner: Arc, ++ config: WorktreeConfig, ++ event_callback: Option, ++} ++ ++impl WorktreeSandbox { ++ /// Create a new `WorktreeSandbox` wrapping `inner` with the given configuration. ++ pub fn new(inner: Arc, config: WorktreeConfig) -> Self { ++ Self { ++ inner, ++ config, ++ event_callback: None, ++ } ++ } ++ ++ /// Set the callback to receive worktree lifecycle events. ++ pub fn set_event_callback(&mut self, cb: WorktreeEventCallback) { ++ self.event_callback = Some(cb); ++ } ++ ++ /// The git branch name managed by this sandbox. ++ pub fn branch_name(&self) -> &str { ++ &self.config.branch_name ++ } ++ ++ /// The base commit SHA used when initializing the worktree. ++ pub fn base_sha(&self) -> &str { ++ &self.config.base_sha ++ } ++ ++ /// The filesystem path to the worktree directory. ++ pub fn worktree_path(&self) -> &str { ++ &self.config.worktree_path ++ } ++ ++ fn emit(&self, event: WorktreeEvent) { ++ if let Some(ref cb) = self.event_callback { ++ cb(event); ++ } ++ } ++} ++ ++// --------------------------------------------------------------------------- ++// Sandbox implementation ++// --------------------------------------------------------------------------- ++ ++#[async_trait] ++impl Sandbox for WorktreeSandbox { ++ // --- Lifecycle --- ++ ++ /// Set up the git worktree: ++ /// 1. Unless `skip_branch_creation`: force-create the branch at `base_sha`, emit `BranchCreated`. ++ /// 2. Best-effort remove any stale worktree, then add fresh one, emit `WorktreeAdded`. ++ /// 3. Unless `skip_branch_creation`: hard-reset the worktree to `base_sha`, emit `Reset`. ++ /// ++ /// Does NOT call `inner.initialize()`. ++ async fn initialize(&self) -> Result<(), String> { ++ let path = shell_quote(&self.config.worktree_path); ++ let branch = shell_quote(&self.config.branch_name); ++ let sha = shell_quote(&self.config.base_sha); ++ ++ if !self.config.skip_branch_creation { ++ let cmd = format!("{GIT} branch --force {branch} {sha}"); ++ let result = self ++ .inner ++ .exec_command(&cmd, 30_000, None, None, None) ++ .await?; ++ if result.exit_code != 0 { ++ return Err(format!( ++ "git branch --force failed (exit {}): {}", ++ result.exit_code, ++ result.stderr.trim() ++ )); ++ } ++ self.emit(WorktreeEvent::BranchCreated { ++ branch: self.config.branch_name.clone(), ++ sha: self.config.base_sha.clone(), ++ }); ++ } ++ ++ // Best-effort remove any stale worktree registration + directory ++ let rm_cmd = format!("{GIT} worktree remove --force {path}"); ++ let _ = self ++ .inner ++ .exec_command(&rm_cmd, 30_000, None, None, None) ++ .await; ++ ++ let add_cmd = format!("{GIT} worktree add {path} {branch}"); ++ let result = self ++ .inner ++ .exec_command(&add_cmd, 30_000, None, None, None) ++ .await?; ++ if result.exit_code != 0 { ++ return Err(format!( ++ "git worktree add failed (exit {}): {}", ++ result.exit_code, ++ result.stderr.trim() ++ )); ++ } ++ self.emit(WorktreeEvent::WorktreeAdded { ++ path: self.config.worktree_path.clone(), ++ branch: self.config.branch_name.clone(), ++ }); ++ ++ if !self.config.skip_branch_creation { ++ let reset_cmd = format!("{GIT} reset --hard {sha}"); ++ let result = self ++ .inner ++ .exec_command( ++ &reset_cmd, ++ 30_000, ++ Some(&self.config.worktree_path), ++ None, ++ None, ++ ) ++ .await?; ++ if result.exit_code != 0 { ++ return Err(format!( ++ "git reset --hard failed (exit {}): {}", ++ result.exit_code, ++ result.stderr.trim() ++ )); ++ } ++ self.emit(WorktreeEvent::Reset { ++ sha: self.config.base_sha.clone(), ++ }); ++ } ++ ++ Ok(()) ++ } ++ ++ /// Remove the git worktree and emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`. ++ async fn cleanup(&self) -> Result<(), String> { ++ let path = shell_quote(&self.config.worktree_path); ++ let cmd = format!("{GIT} worktree remove --force {path}"); ++ let _ = self ++ .inner ++ .exec_command(&cmd, 30_000, None, None, None) ++ .await; ++ self.emit(WorktreeEvent::WorktreeRemoved { ++ path: self.config.worktree_path.clone(), ++ }); ++ Ok(()) ++ } ++ ++ fn working_directory(&self) -> &str { ++ &self.config.worktree_path ++ } ++ ++ /// Execute a command, defaulting `working_dir` to the worktree path when `None`. ++ async fn exec_command( ++ &self, ++ command: &str, ++ timeout_ms: u64, ++ working_dir: Option<&str>, ++ env_vars: Option<&HashMap>, ++ cancel_token: Option, ++ ) -> Result { ++ let wd = working_dir.unwrap_or(&self.config.worktree_path); ++ self.inner ++ .exec_command(command, timeout_ms, Some(wd), env_vars, cancel_token) ++ .await ++ } ++ ++ // --- Delegated methods --- ++ ++ async fn read_file( ++ &self, ++ path: &str, ++ offset: Option, ++ limit: Option, ++ ) -> Result { ++ self.inner.read_file(path, offset, limit).await ++ } ++ ++ async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { ++ self.inner.write_file(path, content).await ++ } ++ ++ async fn delete_file(&self, path: &str) -> Result<(), String> { ++ self.inner.delete_file(path).await ++ } ++ ++ async fn file_exists(&self, path: &str) -> Result { ++ self.inner.file_exists(path).await ++ } ++ ++ async fn list_directory( ++ &self, ++ path: &str, ++ depth: Option, ++ ) -> Result, String> { ++ self.inner.list_directory(path, depth).await ++ } ++ ++ async fn grep( ++ &self, ++ pattern: &str, ++ path: &str, ++ options: &GrepOptions, ++ ) -> Result, String> { ++ self.inner.grep(pattern, path, options).await ++ } ++ ++ async fn glob(&self, pattern: &str, path: Option<&str>) -> Result, String> { ++ self.inner.glob(pattern, path).await ++ } ++ ++ async fn download_file_to_local( ++ &self, ++ remote_path: &str, ++ local_path: &Path, ++ ) -> Result<(), String> { ++ self.inner ++ .download_file_to_local(remote_path, local_path) ++ .await ++ } ++ ++ async fn upload_file_from_local( ++ &self, ++ local_path: &Path, ++ remote_path: &str, ++ ) -> Result<(), String> { ++ self.inner ++ .upload_file_from_local(local_path, remote_path) ++ .await ++ } ++ ++ fn platform(&self) -> &str { ++ self.inner.platform() ++ } ++ ++ fn os_version(&self) -> String { ++ self.inner.os_version() ++ } ++ ++ fn sandbox_info(&self) -> String { ++ self.inner.sandbox_info() ++ } ++ ++ async fn refresh_push_credentials(&self) -> Result<(), String> { ++ self.inner.refresh_push_credentials().await ++ } ++ ++ async fn set_autostop_interval(&self, minutes: i32) -> Result<(), String> { ++ self.inner.set_autostop_interval(minutes).await ++ } ++ ++ fn is_remote(&self) -> bool { ++ self.inner.is_remote() ++ } ++ ++ async fn ssh_access_command(&self) -> Result, String> { ++ self.inner.ssh_access_command().await ++ } ++ ++ fn origin_url(&self) -> Option<&str> { ++ self.inner.origin_url() ++ } ++ ++ async fn get_preview_url( ++ &self, ++ port: u16, ++ ) -> Result)>, String> { ++ self.inner.get_preview_url(port).await ++ } ++ ++ fn mark_agent_read(&self, path: &str) { ++ self.inner.mark_agent_read(path); ++ } ++} ++ ++// --------------------------------------------------------------------------- ++// Tests ++// --------------------------------------------------------------------------- ++ ++#[cfg(test)] ++mod tests { ++ use super::*; ++ use crate::test_support::MockSandbox; ++ use std::sync::Mutex; ++ ++ fn make_config(wt_path: &str) -> WorktreeConfig { ++ WorktreeConfig { ++ branch_name: "fabro/run/test-branch".to_string(), ++ base_sha: "abc123def456".to_string(), ++ worktree_path: wt_path.to_string(), ++ skip_branch_creation: false, ++ } ++ } ++ ++ fn make_config_skip(wt_path: &str) -> WorktreeConfig { ++ WorktreeConfig { ++ branch_name: "fabro/run/test-branch".to_string(), ++ base_sha: "abc123def456".to_string(), ++ worktree_path: wt_path.to_string(), ++ skip_branch_creation: true, ++ } ++ } ++ ++ /// Create a shared mock and return both the `Arc` (passed to WorktreeSandbox) ++ /// and the `Arc` (used to assert captured state). ++ fn make_mock() -> (Arc, Arc) { ++ let mock = Arc::new(MockSandbox::linux()); ++ let as_sandbox: Arc = mock.clone(); ++ (as_sandbox, mock) ++ } ++ ++ // ----------------------------------------------------------------------- ++ // initialize() — full setup (skip_branch_creation = false) ++ // ----------------------------------------------------------------------- ++ ++ #[tokio::test] ++ async fn initialize_issues_correct_git_commands() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ wt.initialize().await.unwrap(); ++ ++ let cmds = mock.captured_commands.lock().unwrap().clone(); ++ // branch --force, worktree remove (best-effort), worktree add, reset --hard ++ assert_eq!(cmds.len(), 4, "expected 4 git commands, got: {cmds:?}"); ++ assert!(cmds[0].contains("branch --force"), "cmd[0]: {}", cmds[0]); ++ assert!( ++ cmds[1].contains("worktree remove --force"), ++ "cmd[1]: {}", ++ cmds[1] ++ ); ++ assert!(cmds[2].contains("worktree add"), "cmd[2]: {}", cmds[2]); ++ assert!(cmds[3].contains("reset --hard"), "cmd[3]: {}", cmds[3]); ++ } ++ ++ #[tokio::test] ++ async fn initialize_emits_branch_worktree_reset_events() { ++ let (inner, _mock) = make_mock(); ++ let mut wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ let events: Arc>> = Arc::new(Mutex::new(Vec::new())); ++ let events_clone = Arc::clone(&events); ++ wt.set_event_callback(Arc::new(move |event| { ++ let label = match &event { ++ WorktreeEvent::BranchCreated { .. } => "BranchCreated", ++ WorktreeEvent::WorktreeAdded { .. } => "WorktreeAdded", ++ WorktreeEvent::WorktreeRemoved { .. } => "WorktreeRemoved", ++ WorktreeEvent::Reset { .. } => "Reset", ++ }; ++ events_clone.lock().unwrap().push(label.to_string()); ++ })); ++ ++ wt.initialize().await.unwrap(); ++ ++ let captured = events.lock().unwrap(); ++ assert_eq!(*captured, vec!["BranchCreated", "WorktreeAdded", "Reset"]); ++ } ++ ++ #[tokio::test] ++ async fn initialize_uses_shell_quoted_values_in_commands() { ++ let (inner, mock) = make_mock(); ++ let config = WorktreeConfig { ++ branch_name: "fabro/run/my-branch".to_string(), ++ base_sha: "deadbeef".to_string(), ++ worktree_path: "/tmp/my worktree".to_string(), // path with space ++ skip_branch_creation: false, ++ }; ++ let wt = WorktreeSandbox::new(inner, config); ++ ++ wt.initialize().await.unwrap(); ++ ++ let cmds = mock.captured_commands.lock().unwrap().clone(); ++ // The path "/tmp/my worktree" should be quoted in shell commands ++ assert!( ++ cmds[1].contains("'/tmp/my worktree'") || cmds[1].contains("\"/tmp/my worktree\""), ++ "worktree path should be shell-quoted: {}", ++ cmds[1] ++ ); ++ } ++ ++ #[tokio::test] ++ async fn initialize_reset_uses_worktree_path_as_working_dir() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ wt.initialize().await.unwrap(); ++ ++ let wdirs = mock.captured_working_dirs.lock().unwrap().clone(); ++ // reset command is at index 3, should use worktree path ++ assert_eq!( ++ wdirs[3], ++ Some("/tmp/wt".to_string()), ++ "reset --hard should run in worktree dir" ++ ); ++ // branch, remove, add commands use None (inner's default) ++ assert_eq!(wdirs[0], None, "branch command should use inner default"); ++ assert_eq!(wdirs[1], None, "worktree remove should use inner default"); ++ assert_eq!(wdirs[2], None, "worktree add should use inner default"); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // initialize() — skip_branch_creation = true ++ // ----------------------------------------------------------------------- ++ ++ #[tokio::test] ++ async fn initialize_skip_branch_creation_issues_only_worktree_commands() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config_skip("/tmp/wt")); ++ ++ wt.initialize().await.unwrap(); ++ ++ let cmds = mock.captured_commands.lock().unwrap().clone(); ++ // Only worktree remove (best-effort) and worktree add ++ assert_eq!(cmds.len(), 2, "expected 2 git commands, got: {cmds:?}"); ++ assert!( ++ cmds[0].contains("worktree remove --force"), ++ "cmd[0]: {}", ++ cmds[0] ++ ); ++ assert!(cmds[1].contains("worktree add"), "cmd[1]: {}", cmds[1]); ++ } ++ ++ #[tokio::test] ++ async fn initialize_skip_branch_creation_emits_only_worktree_added() { ++ let (inner, _mock) = make_mock(); ++ let mut wt = WorktreeSandbox::new(inner, make_config_skip("/tmp/wt")); ++ ++ let events: Arc>> = Arc::new(Mutex::new(Vec::new())); ++ let events_clone = Arc::clone(&events); ++ wt.set_event_callback(Arc::new(move |event| { ++ let label = match &event { ++ WorktreeEvent::BranchCreated { .. } => "BranchCreated", ++ WorktreeEvent::WorktreeAdded { .. } => "WorktreeAdded", ++ WorktreeEvent::WorktreeRemoved { .. } => "WorktreeRemoved", ++ WorktreeEvent::Reset { .. } => "Reset", ++ }; ++ events_clone.lock().unwrap().push(label.to_string()); ++ })); ++ ++ wt.initialize().await.unwrap(); ++ ++ let captured = events.lock().unwrap(); ++ assert_eq!(*captured, vec!["WorktreeAdded"]); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // initialize() — error propagation ++ // ----------------------------------------------------------------------- ++ ++ #[tokio::test] ++ async fn initialize_propagates_error_on_nonzero_exit() { ++ let inner: Arc = Arc::new(MockSandbox { ++ exec_result: ExecResult { ++ stdout: String::new(), ++ stderr: "fatal: not a git repo".to_string(), ++ exit_code: 128, ++ timed_out: false, ++ duration_ms: 5, ++ }, ++ ..MockSandbox::linux() ++ }); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ let result = wt.initialize().await; ++ ++ assert!(result.is_err(), "should return Err on non-zero exit"); ++ let err = result.unwrap_err(); ++ assert!( ++ err.contains("branch --force failed") || err.contains("128"), ++ "error should mention the failure: {err}" ++ ); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // cleanup() ++ // ----------------------------------------------------------------------- ++ ++ #[tokio::test] ++ async fn cleanup_issues_worktree_remove_command() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ wt.cleanup().await.unwrap(); ++ ++ let cmds = mock.captured_commands.lock().unwrap().clone(); ++ assert_eq!(cmds.len(), 1, "cleanup should issue exactly one command"); ++ assert!( ++ cmds[0].contains("worktree remove --force"), ++ "cleanup command should remove the worktree: {}", ++ cmds[0] ++ ); ++ } ++ ++ #[tokio::test] ++ async fn cleanup_emits_worktree_removed_event() { ++ let (inner, _mock) = make_mock(); ++ let mut wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ let removed_path: Arc>> = Arc::new(Mutex::new(None)); ++ let path_clone = Arc::clone(&removed_path); ++ wt.set_event_callback(Arc::new(move |event| { ++ if let WorktreeEvent::WorktreeRemoved { path } = event { ++ *path_clone.lock().unwrap() = Some(path); ++ } ++ })); ++ ++ wt.cleanup().await.unwrap(); ++ ++ assert_eq!(*removed_path.lock().unwrap(), Some("/tmp/wt".to_string())); ++ } ++ ++ #[tokio::test] ++ async fn cleanup_succeeds_even_if_worktree_remove_fails() { ++ let inner: Arc = Arc::new(MockSandbox { ++ exec_result: ExecResult { ++ stdout: String::new(), ++ stderr: String::new(), ++ exit_code: 1, // non-zero, but cleanup should still succeed ++ timed_out: false, ++ duration_ms: 0, ++ }, ++ ..MockSandbox::linux() ++ }); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ let result = wt.cleanup().await; ++ assert!(result.is_ok(), "cleanup should succeed even if git fails"); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // working_directory() ++ // ----------------------------------------------------------------------- ++ ++ #[test] ++ fn working_directory_returns_worktree_path() { ++ let (inner, _mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/my_worktree")); ++ ++ assert_eq!(wt.working_directory(), "/tmp/my_worktree"); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // exec_command() working_dir defaulting ++ // ----------------------------------------------------------------------- ++ ++ #[tokio::test] ++ async fn exec_command_none_working_dir_defaults_to_worktree_path() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ wt.exec_command("echo hello", 5000, None, None, None) ++ .await ++ .unwrap(); ++ ++ let wdirs = mock.captured_working_dirs.lock().unwrap().clone(); ++ assert_eq!( ++ wdirs.last(), ++ Some(&Some("/tmp/wt".to_string())), ++ "None working_dir should be replaced with worktree path" ++ ); ++ } ++ ++ #[tokio::test] ++ async fn exec_command_explicit_working_dir_passes_through() { ++ let (inner, mock) = make_mock(); ++ let wt = WorktreeSandbox::new(inner, make_config("/tmp/wt")); ++ ++ wt.exec_command("echo hello", 5000, Some("/explicit/path"), None, None) ++ .await ++ .unwrap(); ++ ++ let wdirs = mock.captured_working_dirs.lock().unwrap().clone(); ++ assert_eq!( ++ wdirs.last(), ++ Some(&Some("/explicit/path".to_string())), ++ "explicit working_dir should be passed through unchanged" ++ ); ++ } ++ ++ // ----------------------------------------------------------------------- ++ // Accessors ++ // ----------------------------------------------------------------------- ++ ++ #[test] ++ fn accessors_return_config_values() { ++ let (inner, _mock) = make_mock(); ++ let config = WorktreeConfig { ++ branch_name: "my-branch".to_string(), ++ base_sha: "sha123".to_string(), ++ worktree_path: "/path/to/wt".to_string(), ++ skip_branch_creation: false, ++ }; ++ let wt = WorktreeSandbox::new(inner, config); ++ ++ assert_eq!(wt.branch_name(), "my-branch"); ++ assert_eq!(wt.base_sha(), "sha123"); ++ assert_eq!(wt.worktree_path(), "/path/to/wt"); ++ } ++} +diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs +index b1e73dc5..36db9f35 100644 +--- a/lib/crates/fabro-workflows/src/handler/parallel.rs ++++ b/lib/crates/fabro-workflows/src/handler/parallel.rs +@@ -3,7 +3,7 @@ use std::sync::Arc; + use std::time::Instant; + + use async_trait::async_trait; +-use fabro_agent::Sandbox; ++use fabro_agent::{Sandbox, WorktreeConfig, WorktreeEvent, WorktreeSandbox}; + use tokio::sync::Semaphore; + + use crate::context::keys; +@@ -13,118 +13,11 @@ use crate::error::FabroError; + use crate::event::WorkflowRunEvent; + use crate::millis_u64; + use crate::outcome::{Outcome, StageStatus}; +-use fabro_agent::LocalSandbox; + use fabro_graphviz::graph::{Graph, Node}; + use fabro_hooks::{HookContext, HookEvent}; + + use super::{EngineServices, Handler}; + +-// --------------------------------------------------------------------------- +-// WorktreeSandbox — decorates a Sandbox with a custom working dir +-// --------------------------------------------------------------------------- +- +-/// Wraps an existing `Sandbox` so that all operations use a +-/// different working directory (the worktree path inside a remote sandbox). +-struct WorktreeSandbox { +- inner: Arc, +- worktree_dir: String, +-} +- +-#[async_trait] +-impl Sandbox for WorktreeSandbox { +- async fn read_file( +- &self, +- path: &str, +- offset: Option, +- limit: Option, +- ) -> Result { +- self.inner.read_file(path, offset, limit).await +- } +- async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { +- self.inner.write_file(path, content).await +- } +- async fn delete_file(&self, path: &str) -> Result<(), String> { +- self.inner.delete_file(path).await +- } +- async fn file_exists(&self, path: &str) -> Result { +- self.inner.file_exists(path).await +- } +- async fn list_directory( +- &self, +- path: &str, +- depth: Option, +- ) -> Result, String> { +- self.inner.list_directory(path, depth).await +- } +- async fn exec_command( +- &self, +- command: &str, +- timeout_ms: u64, +- working_dir: Option<&str>, +- env_vars: Option<&std::collections::HashMap>, +- cancel_token: Option, +- ) -> Result { +- // Default to worktree dir when no explicit working_dir is given +- let wd = working_dir.unwrap_or(&self.worktree_dir); +- self.inner +- .exec_command(command, timeout_ms, Some(wd), env_vars, cancel_token) +- .await +- } +- async fn grep( +- &self, +- pattern: &str, +- path: &str, +- options: &fabro_agent::sandbox::GrepOptions, +- ) -> Result, String> { +- self.inner.grep(pattern, path, options).await +- } +- async fn glob(&self, pattern: &str, path: Option<&str>) -> Result, String> { +- self.inner.glob(pattern, path).await +- } +- async fn download_file_to_local( +- &self, +- remote_path: &str, +- local_path: &std::path::Path, +- ) -> Result<(), String> { +- self.inner +- .download_file_to_local(remote_path, local_path) +- .await +- } +- async fn upload_file_from_local( +- &self, +- local_path: &std::path::Path, +- remote_path: &str, +- ) -> Result<(), String> { +- self.inner +- .upload_file_from_local(local_path, remote_path) +- .await +- } +- async fn initialize(&self) -> Result<(), String> { +- self.inner.initialize().await +- } +- async fn cleanup(&self) -> Result<(), String> { +- self.inner.cleanup().await +- } +- fn working_directory(&self) -> &str { +- &self.worktree_dir +- } +- fn platform(&self) -> &str { +- self.inner.platform() +- } +- fn os_version(&self) -> String { +- self.inner.os_version() +- } +- fn is_remote(&self) -> bool { +- self.inner.is_remote() +- } +- async fn ssh_access_command(&self) -> Result, String> { +- self.inner.ssh_access_command().await +- } +- fn origin_url(&self) -> Option<&str> { +- self.inner.origin_url() +- } +-} +- + /// Fans out execution to multiple branches concurrently. + /// Each branch gets an isolated context clone and runs independently. + pub struct ParallelHandler; +@@ -374,7 +267,7 @@ impl Handler for ParallelHandler { + crate::git::sanitize_ref_component(branch_key), + ); + +- // Compute worktree path ++ // Compute worktree path (local vs remote path schemes differ) + let wt_path_str = if services.sandbox.is_remote() { + format!( + "{}/.fabro/runs/{}/parallel/{}/{}", +@@ -394,59 +287,38 @@ impl Handler for ParallelHandler { + }; + tracing::debug!(branch = %branch_name, path = %wt_path_str, "Creating worktree for parallel branch"); + +- // Create branch + worktree + reset via sandbox +- if !crate::engine::git_create_branch_at(&*services.sandbox, &branch_name, bsha) ++ // Set up worktree via WorktreeSandbox ++ let wt_config = WorktreeConfig { ++ branch_name: branch_name.clone(), ++ base_sha: bsha.clone(), ++ worktree_path: wt_path_str.clone(), ++ skip_branch_creation: false, ++ }; ++ let mut wt_sandbox = WorktreeSandbox::new(Arc::clone(&services.sandbox), wt_config); ++ let emitter_wt = Arc::clone(&services.emitter); ++ wt_sandbox.set_event_callback(Arc::new(move |event| match event { ++ WorktreeEvent::BranchCreated { branch, sha } => { ++ emitter_wt.emit(&WorkflowRunEvent::GitBranch { branch, sha }); ++ } ++ WorktreeEvent::WorktreeAdded { path, branch } => { ++ emitter_wt.emit(&WorkflowRunEvent::GitWorktreeAdd { path, branch }); ++ } ++ WorktreeEvent::WorktreeRemoved { path } => { ++ emitter_wt.emit(&WorkflowRunEvent::GitWorktreeRemove { path }); ++ } ++ WorktreeEvent::Reset { sha } => { ++ emitter_wt.emit(&WorkflowRunEvent::GitReset { sha }); ++ } ++ })); ++ wt_sandbox ++ .initialize() + .await +- { +- return Err(FabroError::handler(format!( +- "failed to create branch {branch_name}" +- ))); +- } +- services.emitter.emit(&WorkflowRunEvent::GitBranch { +- branch: branch_name.clone(), +- sha: bsha.clone(), +- }); +- if !crate::engine::git_replace_worktree( +- &*services.sandbox, +- &wt_path_str, +- &branch_name, +- ) +- .await +- { +- return Err(FabroError::handler(format!( +- "failed to add worktree {wt_path_str}" +- ))); +- } +- services.emitter.emit(&WorkflowRunEvent::GitWorktreeAdd { +- path: wt_path_str.clone(), +- branch: branch_name.clone(), +- }); +- let reset_cmd = format!("{} reset --hard {bsha}", crate::engine::GIT_REMOTE); +- let reset_result = services +- .sandbox +- .exec_command(&reset_cmd, 30_000, Some(&wt_path_str), None, None) +- .await; +- if !matches!(reset_result, Ok(ref r) if r.exit_code == 0) { +- return Err(FabroError::handler(format!( +- "failed to reset worktree {wt_path_str}" +- ))); +- } +- services +- .emitter +- .emit(&WorkflowRunEvent::GitReset { sha: bsha.clone() }); ++ .map_err(|e| FabroError::handler(format!("worktree setup failed: {e}")))?; + + branch_context.set(keys::INTERNAL_WORK_DIR, serde_json::json!(&wt_path_str)); + +- // Create appropriate sandbox wrapper + let wt_path = PathBuf::from(&wt_path_str); +- let env: Arc = if services.sandbox.is_remote() { +- Arc::new(WorktreeSandbox { +- inner: Arc::clone(&services.sandbox), +- worktree_dir: wt_path_str, +- }) +- } else { +- Arc::new(LocalSandbox::new(wt_path.clone())) +- }; ++ let env: Arc = Arc::new(wt_sandbox); + (env, Some(wt_path)) + } else { + (Arc::clone(&services.sandbox), None) diff --git a/nodes/simplify_opus/prompt.md b/nodes/simplify_opus/prompt.md new file mode 100644 index 000000000..a10bddf4f --- /dev/null +++ b/nodes/simplify_opus/prompt.md @@ -0,0 +1,230 @@ +Goal: # Unified WorktreeSandbox + +## Context + +Worktree management is currently split across two locations with duplicated logic: + +1. **Parallel branches** (`parallel.rs`): Inline git commands in `ParallelHandler::execute()`, a thin `WorktreeSandbox` decorator, and separate local/remote code paths +2. **Top-level CLI run** (`run.rs`): A `setup_worktree()` function using synchronous git helpers, with the worktree path fed into a plain `LocalSandbox` + +The goal is a single `WorktreeSandbox` type that wraps any `Arc`, manages the worktree lifecycle in `initialize()`/`cleanup()`, and eliminates the local/remote branching. + +## Plan + +### Step 1: Create `WorktreeSandbox` in fabro-sandbox + +**New file:** `lib/crates/fabro-sandbox/src/worktree.rs` + +Define: + +```rust +pub enum WorktreeEvent { + BranchCreated { branch: String, sha: String }, + WorktreeAdded { path: String, branch: String }, + WorktreeRemoved { path: String }, + Reset { sha: String }, +} + +pub type WorktreeEventCallback = Arc; + +pub struct WorktreeConfig { + pub branch_name: String, + pub base_sha: String, + pub worktree_path: String, + /// Skip branch creation and reset (for resume, where branch already exists). + pub skip_branch_creation: bool, +} + +pub struct WorktreeSandbox { + inner: Arc, + config: WorktreeConfig, + event_callback: Option, +} +``` + +**Constructor + getters:** `new(inner, config)`, `set_event_callback()`, `branch_name()`, `base_sha()`, `worktree_path()` + +**`initialize()`:** +1. If `!skip_branch_creation`: `git branch --force {branch_name} {base_sha}` via `inner.exec_command()`, emit `BranchCreated` +2. `git worktree remove --force {path}` (best-effort), then `git worktree add {path} {branch}`, emit `WorktreeAdded` +3. If `!skip_branch_creation`: `git reset --hard {base_sha}` in worktree dir, emit `Reset` + +Does NOT call `inner.initialize()` — the inner sandbox's lifecycle is managed separately. + +**`cleanup()`:** `git worktree remove --force {path}`, emit `WorktreeRemoved`. Does NOT call `inner.cleanup()`. + +**`working_directory()`:** Returns `config.worktree_path`. + +**`exec_command()`:** Defaults `working_dir` to `config.worktree_path` when `None`, delegates to inner. + +**All other Sandbox methods:** Delegate to inner. Must be a manual `impl Sandbox` block (can't use `delegate_sandbox!` since it generates `initialize`/`cleanup`/`working_directory`/`exec_command` which we need to override). + +All interpolated values in git commands use `shell_quote()`. + +### Step 2: Register module and re-exports + +- `lib/crates/fabro-sandbox/src/lib.rs`: Add `pub mod worktree;` and `pub use worktree::WorktreeSandbox;` +- `lib/crates/fabro-agent/src/sandbox.rs`: Add re-export of `WorktreeSandbox` + +### Step 3: Unit tests for WorktreeSandbox + +In `worktree.rs` `#[cfg(test)]` module, using `MockSandbox`: + +- `initialize()` issues correct git commands (branch, worktree remove, worktree add, reset) and emits events +- `skip_branch_creation` skips branch + reset, only does worktree add +- `cleanup()` issues `worktree remove` and emits `WorktreeRemoved` +- `working_directory()` returns worktree path +- `exec_command()` with `None` working_dir defaults to worktree path +- `exec_command()` with explicit working_dir passes it through +- `initialize()` propagates errors on non-zero exit + +**MockSandbox enhancement:** Add `captured_commands: Mutex>` field to `test_support.rs` to capture the sequence of `exec_command` calls (current `captured_command` only stores the last one). Append to vec in `exec_command()` impl. + +### Step 4: Refactor parallel.rs + +- **Remove** the private `WorktreeSandbox` struct (lines 28-126) and `use fabro_agent::LocalSandbox` +- **Replace** the inline git setup loop (lines 361-450) with: + - Construct `WorktreeConfig` with branch name, base SHA, worktree path + - Create `WorktreeSandbox::new(Arc::clone(&services.sandbox), config)` + - Wire event callback to bridge `WorktreeEvent` → `WorkflowRunEvent` + - Call `initialize().await` +- This eliminates the `if services.sandbox.is_remote()` branch (lines 442-449) — `WorktreeSandbox` works the same for any inner sandbox +- **Cleanup loop** (lines 659-668): Keep calling `git_remove_worktree()` on the parent sandbox (the `WorktreeSandbox` Arc is consumed by the spawned task and dropped). Alternatively, could store the sandbox Arc in `BranchResult` and call `.cleanup()`, but the current approach is simpler. + +### Step 5: Refactor run.rs — new runs + +Replace `setup_worktree()` call (lines 830-845) + separate `LocalSandbox` construction with: + +``` +if workdir_strategy == LocalWorktree: + base_sha = git::head_sha() + branch_name = "fabro/run/{run_id}" + inner = Arc::new(LocalSandbox::new(original_cwd)) + wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { ... }) + wt_sandbox.set_event_callback(bridge to WorkflowRunEvent) + wt_sandbox.initialize().await + std::env::set_current_dir(&worktree_path) // stays in CLI, not in sandbox + sandbox = Arc::new(wt_sandbox) + // store base_sha, branch_name for RunConfig +``` + +**Delete** the `setup_worktree()` function (lines 1696-1714) — its logic is absorbed above. + +`std::env::set_current_dir()` stays in `run.rs` — it's a process-global side effect that belongs to the CLI. + +### Step 6: Refactor run.rs — resume (run_from_branch) + +Replace worktree re-attachment (lines 1810-1822) with: + +``` +inner = Arc::new(LocalSandbox::new(original_cwd)) +wt_sandbox = WorktreeSandbox::new(inner, WorktreeConfig { + branch_name: run_branch, + base_sha: base_sha.unwrap_or_default(), + worktree_path: wt_str, + skip_branch_creation: true, // branch already exists +}) +wt_sandbox.initialize().await +std::env::set_current_dir(&wt) +``` + +### Step 7: Verify + +- `cargo build --workspace` +- `cargo test --workspace` +- `cargo clippy --workspace -- -D warnings` +- Manual: `fabro run` with worktree mode enabled on a local workflow +- Manual: `fabro run --run-branch` to test resume path + +## Files to modify + +| File | Change | +|---|---| +| `lib/crates/fabro-sandbox/src/worktree.rs` | **New** — WorktreeSandbox, WorktreeConfig, WorktreeEvent, impl Sandbox, tests | +| `lib/crates/fabro-sandbox/src/lib.rs` | Add module + re-export | +| `lib/crates/fabro-sandbox/src/test_support.rs` | Add `captured_commands: Mutex>` to MockSandbox | +| `lib/crates/fabro-agent/src/sandbox.rs` | Add WorktreeSandbox re-export | +| `lib/crates/fabro-workflows/src/handler/parallel.rs` | Remove old WorktreeSandbox, use new one | +| `lib/crates/fabro-cli/src/commands/run.rs` | Replace setup_worktree + run_from_branch worktree logic | + +## Functions that become removable + +| Function | Location | Reason | +|---|---|---| +| `setup_worktree()` | `run.rs:1696` | Logic absorbed into WorktreeSandbox | +| Old `WorktreeSandbox` struct | `parallel.rs:28-126` | Replaced by shared WorktreeSandbox | + +Engine git helpers (`git_add_worktree`, `git_remove_worktree`, etc. in `engine.rs`) stay — still used by parallel cleanup and potentially other callers. Sync git helpers in `git.rs` also stay. + + +## 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-sonnet-4-6, 190.9k tokens in / 105.0k out + - Files: /home/daytona/workspace/lib/crates/fabro-agent/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-agent/src/sandbox.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/lib.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/test_support.rs, /home/daytona/workspace/lib/crates/fabro-sandbox/src/worktree.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/handler/parallel.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..d0418b4c6 --- /dev/null +++ b/nodes/simplify_opus/provider_used.json @@ -0,0 +1,5 @@ +{ + "mode": "agent", + "provider": "anthropic", + "model": "claude-sonnet-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..5f74d6c2e --- /dev/null +++ b/nodes/simplify_opus/response.md @@ -0,0 +1,24 @@ +Zero warnings. Here's a summary of what was fixed: + +## Fixes Applied + +### 1. Extracted `WorktreeEvent → WorkflowRunEvent` callback helper (3 → 1) +The same 12-line match block mapping `WorktreeEvent` variants to `WorkflowRunEvent` was duplicated in three places: +- `run.rs`: new-run path (`SandboxProvider::Local`) +- `run.rs`: resume path (`run_from_branch`) +- `parallel.rs`: branch worktree setup + +**Fix:** Added `fn worktree_event_callback(emitter: Arc) -> WorktreeEventCallback` to each file as a local helper, replacing all three inline match blocks with a single call. + +### 2. Extracted `LocalSandbox + event_callback` helper (3 → 1) +In `run.rs`, the pattern `let mut env = LocalSandbox::new(cwd); env.set_event_callback(...); Arc::new(env)` appeared twice in the same match arm (fallback on error + else branch) and again in `run_from_branch`. + +**Fix:** Added `fn local_sandbox_with_callback(cwd: PathBuf, emitter: Arc) -> Arc` and replaced all three sites. + +### 3. Eliminated `.clone().unwrap()` on `Option` +`worktree_base_sha.clone().unwrap()` (and same for `branch_name`, `wt_path`) cloned the entire `Option` before unwrapping. + +**Fix:** Changed `if worktree_base_sha.is_some()` to `if let (Some(base_sha), Some(branch_name), Some(wt_path)) = (worktree_base_sha.as_ref(), ...)` — borrows the inner values directly, then only clones the `String`/`PathBuf` when actually needed for `WorktreeConfig`. + +### 4. Fixed `.to_string_lossy().to_string()` → `.into_owned()` (3 sites) +`.to_string_lossy().to_string()` calls `.to_string()` on a `Cow`, which allocates a new `String` even when `Cow` is already `Owned`. `.into_owned()` moves directly from `Cow::Owned` without the extra allocation. \ 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..daa5653fc --- /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-20T01:51:52.226293+00:00" +} \ No newline at end of file