From 334ee795702e2f1fa99517c6ea97400bec25176b Mon Sep 17 00:00:00 2001 From: Fabro Date: Mon, 16 Mar 2026 01:01:57 -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 | 34 ++- nodes/simplify_gpt/diff.patch | 390 ++++++++++++++++++++++++++++ nodes/verify/script_invocation.json | 5 + nodes/verify/script_timing.json | 5 + nodes/verify/status.json | 6 + 5 files changed, 430 insertions(+), 10 deletions(-) create mode 100644 nodes/simplify_gpt/diff.patch create mode 100644 nodes/verify/script_invocation.json create mode 100644 nodes/verify/script_timing.json create mode 100644 nodes/verify/status.json diff --git a/checkpoint.json b/checkpoint.json index 778ff418e..1b5454466 100644 --- a/checkpoint.json +++ b/checkpoint.json @@ -1,6 +1,6 @@ { - "timestamp": "2026-03-16T05:01:09.792908Z", - "current_node": "simplify_gpt", + "timestamp": "2026-03-16T05:01:57.085924Z", + "current_node": "verify", "completed_nodes": [ "start", "toolchain", @@ -9,7 +9,8 @@ "implement", "simplify_opus", "simplify_gemini", - "simplify_gpt" + "simplify_gpt", + "verify" ], "node_retries": { "implement": 1, @@ -19,7 +20,8 @@ "preflight_compile": 1, "start": 1, "simplify_opus": 1, - "simplify_gemini": 1 + "simplify_gemini": 1, + "verify": 1 }, "context_values": { "internal.retry_count.simplify_gemini": 1, @@ -27,7 +29,7 @@ "last_response": "All clean. Here's a summary of what was fixed:\n\n## Summary of Changes\n\n### Fixed Issues\n\n1. **Eliminated `FinalizeConfig` struct (−52 lines)** — Changed `run_with_lifecycle` to take `config: &mut ", "graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ", "outcome": "success", - "command.output": "", + "command.output": "────────────\n Nextest run ID 6d5ca597-5977-4dbf-99b7-477603e0ee65 with nextest profile: default\n Starting 3416 tests across 38 binaries (183 tests skipped)\n────────────\n Summary [ 15.408s] 3416 tests run: 3416 passed, 183 skipped\n", "internal.retry_count.simplify_gpt": 1, "internal.retry_count.start": 1, "thread.simplify_opus.current_node": "simplify_gemini", @@ -35,6 +37,7 @@ "internal.run_id": "01KKTC38AXE3WSR695SJXKK70Y", "response.simplify_opus": "All tests pass. Here's a summary of what was fixed:\n\n## Summary of Fixes\n\n### Critical Bug Fix\n1. **Stale `FinalizeConfig` for remote sandboxes** — `FinalizeConfig` was being built *before* `run_with_lifecycle()` consumed the `RunConfig`, but the engine mutates `config.run_branch`, `config.base_sha`, `config.base_branch`, `config.meta_branch`, and `config.git_checkpoint_enabled` during remote git setup. This meant PR creation, finalize commits, and checkpoint tracking silently broke for remote sandbox runs (Daytona, SSH, exe.dev).\n - **Fix**: Changed `run_with_lifecycle()` to return `(Outcome, RunConfig)` so the caller gets the mutated config back. Moved `FinalizeConfig` construction to *after* the engine call. Added `FinalizeConfig::from_run_config()` and `FinalizeConfig::from_run_config_owned()` constructors to centralize the conversion (eliminating duplicate field-by-field construction in both `run_command` and `run_from_branch`).\n\n### Minor Cleanups\n2. **Missing trailing newline in `hooks.mdx`** — restored the standard trailing newline.\n3. **Unnecessary `setup_commands.clone()`** — the variable was never used after being passed to `LifecycleConfig`, so replaced `.clone()` with a direct move.\n4. **Verbose `sandbox_info_opt` chain** — replaced `.map(|sb| { ... }).unwrap_or(None)` with the idiomatic `.and_then(|sb| { ... })`.\n\n### Findings Not Fixed (false positives or not worth addressing)\n- The `deferred_sandbox: Arc>>>` triple-wrapping is necessary — each layer serves a distinct purpose (trait object sharing, deferred initialization, synchronous closure mutability, cross-closure sharing).\n- The devcontainer command vector clones are necessary since `devcontainer_config` is borrowed earlier for environment extraction.\n- The `SandboxProvider` → `&str` match doesn't have an existing `Display`/`as_str()` impl on the enum, and adding one would be scope creep.\n- The stringly-typed `provider_name == \"docker\"` comparisons in the event listener are a pre-existing pattern from the original code; not worth refactoring in this PR.", "internal.retry_count.toolchain": 1, + "internal.retry_count.verify": 1, "thread.preflight_compile.current_node": "preflight_lint", "internal.retry_count.preflight_lint": 1, "graph.goal": "# Plan: Move sandbox lifecycle into the engine\n\n## Context\n\n`SandboxReady` and `SandboxCleanup` hook events are defined in `HookEvent` but never fired. They can't fire naturally because sandbox init/cleanup happens in `cli/run.rs` — outside the engine where all other hooks fire. The root cause is that `cli/run.rs` is a ~2000-line god function orchestrating everything: sandbox creation, init, setup commands, devcontainer lifecycle, engine construction, engine run, retro, PR creation, and cleanup. The engine receives an already-initialized sandbox and has no role in lifecycle management.\n\nThis refactor moves sandbox initialization and setup into the engine so `SandboxReady` fires naturally alongside `RunStart`. Cleanup stays callable from the CLI (the retro agent needs the sandbox alive after the graph completes) but fires through an engine method so `SandboxCleanup` also uses the hook infrastructure.\n\n## Design\n\n### Two new engine methods\n\n**`run_with_lifecycle(graph, config, lifecycle, checkpoint?) -> Result`**\n1. `sandbox.initialize()`\n2. Fire `SandboxReady` hook (blocking — can abort run)\n3. Emit `SandboxInitialized` event (CLI listener writes `sandbox.json`, updates progress UI)\n4. Remote git setup if `sandbox.is_remote()` — produces `base_sha`, `run_branch`, `base_branch`, merged into `config`\n5. Run setup commands inside sandbox\n6. Run devcontainer lifecycle phases inside sandbox\n7. Call existing `run_internal()` (unchanged — fires RunStart → graph → RunComplete)\n8. Return outcome (sandbox still alive)\n\n**`cleanup_sandbox(run_id, workflow_name, preserve) -> Result<()>`**\n1. Fire `SandboxCleanup` hook (non-blocking)\n2. If `!preserve`: call `sandbox.cleanup()`\n\nExisting `run()` is unchanged — API server and integration tests keep using it with pre-initialized sandboxes.\n\n### New config struct\n\n```rust\n// engine.rs\npub struct LifecycleConfig {\n pub setup_commands: Vec,\n pub setup_command_timeout_ms: u64,\n pub devcontainer_phases: Vec<(String, Vec)>,\n}\n```\n\n`run_with_lifecycle` takes `config: RunConfig` by value (currently `&RunConfig`) so it can fill in remote git values. `run_internal` continues to take `&RunConfig`.\n\n### CLI flow after refactor\n\n```\nsandbox = create_sandbox() // unchanged\nsandbox = ReadBeforeWriteSandbox::new(sandbox) // moved before init (delegate_sandbox! delegates initialize)\nengine = build_engine(sandbox, hook_runner, ...)\noutcome = engine.run_with_lifecycle(graph, config, lifecycle)\n// retro, conclusion, PR creation — sandbox still alive\nengine.cleanup_sandbox(run_id, workflow_name, preserve)\n```\n\nSingle scopeguard around the entire block that calls `engine.cleanup_sandbox()` on panic.\n\n## Steps\n\n### 1. `hook/types.rs` — Make `SandboxReady` blocking by default\n- Add `Self::SandboxReady` to `is_blocking_by_default()` match arm (line 29-32)\n- `SandboxCleanup` stays non-blocking (correct default)\n\n### 2. `engine.rs` — Add `LifecycleConfig` struct and `run_with_lifecycle` method\n- Define `LifecycleConfig` (setup_commands, setup_command_timeout_ms, devcontainer_phases)\n- Add `pub async fn run_with_lifecycle(self, graph, config, lifecycle, checkpoint) -> Result` that:\n - Calls `self.services.sandbox.initialize()`\n - Fires `SandboxReady` hook via `self.run_hooks()`\n - Emits `WorkflowRunEvent::SandboxInitialized { working_directory }` via emitter\n - Calls remote git setup if `sandbox.is_remote()`, fills config.base_sha/run_branch/base_branch/git_checkpoint_enabled\n - Runs setup commands via `sandbox.exec_command()`, emitting Setup* events\n - Runs devcontainer lifecycle via `devcontainer_bridge::run_devcontainer_lifecycle()`\n - Calls `self.run_internal()` (or `run_from_checkpoint` path)\n - Returns outcome\n\n### 3. `engine.rs` — Add `cleanup_sandbox` method\n- `pub async fn cleanup_sandbox(&self, run_id, workflow_name, preserve) -> Result<(), String>`\n- Fires `SandboxCleanup` hook\n- If `!preserve`: calls `self.services.sandbox.cleanup()`\n\n### 4. `engine.rs` — Move `setup_remote_git` from `cli/run.rs`\n- Move the `setup_remote_git()` function (cli/run.rs line 1636-1679) into engine.rs\n- It only uses `sandbox.exec_command()` and `run_id` — no CLI dependencies\n\n### 5. `event.rs` — Add `SandboxInitialized` event variant\n- Add `WorkflowRunEvent::SandboxInitialized { working_directory: String }` variant\n- This replaces the inline sandbox.json writing in cli/run.rs\n\n### 6. `cli/run.rs` — Register event listener for sandbox.json\n- Before calling `run_with_lifecycle`, register a listener on the emitter for `SandboxInitialized`\n- Listener captures the pre-built `SandboxRecord` template (all provider-specific fields filled, `working_directory` empty)\n- On event: fill `working_directory` from event, call `record.save()`\n- Also update progress UI `set_working_directory` in the same listener\n\n### 7. `cli/run.rs` — Refactor `run_command` to use new engine methods\n- Move `ReadBeforeWriteSandbox` wrapping to before engine construction (currently at line 966, after init — delegate_sandbox! macro delegates initialize so wrapping before init works)\n- Move `HookRunner` creation earlier (before engine construction) — currently line 1222, move to ~line 800\n- Remove: `sandbox.initialize()` (line 886), remote git setup (lines 982-996), setup commands (lines 1031-1072), devcontainer lifecycle (lines 1074-1091)\n- Build `LifecycleConfig` from `setup_commands` and `devcontainer_config`\n- Build `RunConfig` without remote git fields (leave base_sha/run_branch/base_branch as None for remote — engine fills them)\n- Call `engine.run_with_lifecycle()` instead of `engine.run()`\n- Replace cleanup section (lines 1587-1604) with `engine.cleanup_sandbox()`\n- Replace two scopeguards with one that calls `engine.cleanup_sandbox()` on panic\n- Remove `status_guard` for SandboxInitFailed — engine handles init errors\n\n### 8. `cli/run.rs` — Refactor `run_from_branch` to use new engine methods\n- Use `run_with_lifecycle` with empty `LifecycleConfig` (no setup commands, no devcontainer)\n- Add `cleanup_sandbox()` call (currently no scopeguard — this is an improvement)\n- This gives the resume path hooks for free (currently has zero hooks)\n\n### 9. `docs/agents/hooks.mdx` — Update docs\n- Remove any \"reserved\" annotations for `sandbox_ready` / `sandbox_cleanup`\n- Note that `sandbox_ready` is blocking by default\n\n## Files to modify\n- `lib/crates/fabro-workflows/src/hook/types.rs` — SandboxReady blocking default\n- `lib/crates/fabro-workflows/src/engine.rs` — LifecycleConfig, run_with_lifecycle, cleanup_sandbox, setup_remote_git\n- `lib/crates/fabro-workflows/src/event.rs` — SandboxInitialized event variant\n- `lib/crates/fabro-workflows/src/cli/run.rs` — major simplification of run_command and run_from_branch\n- `docs/agents/hooks.mdx` — remove \"reserved\" annotations\n\n## Files unchanged\n- `lib/crates/fabro-workflows/src/handler/mod.rs` — EngineServices unchanged\n- `lib/crates/fabro-workflows/src/hook/runner.rs` — handles any HookEvent generically\n- `lib/crates/fabro-agent/src/sandbox.rs` — Sandbox trait unchanged\n- `lib/crates/fabro-agent/src/read_before_write_sandbox.rs` — delegate_sandbox! already delegates initialize/cleanup\n- All sandbox implementations — unchanged\n\n## Verification\n1. `cargo build --workspace` — compile check\n2. `cargo test --workspace` — all existing tests pass (existing `run()` path unchanged)\n3. `cargo clippy --workspace -- -D warnings` — no new warnings\n4. Manual test: `fabro run` with a workflow that has `sandbox_ready` and `sandbox_cleanup` hooks configured — verify hooks fire\n5. Manual test: `fabro run --sandbox daytona` — verify remote git setup still works through the engine\n6. Manual test: `fabro run` with `--preserve-sandbox` — verify cleanup is skipped but SandboxCleanup hook still fires\n", @@ -43,10 +46,11 @@ "graph.rankdir": "LR", "internal.retry_count.preflight_compile": 1, "internal.retry_count.simplify_opus": 1, + "thread.simplify_gpt.current_node": "verify", "response.simplify_gpt": "All clean. Here's a summary of what was fixed:\n\n## Summary of Changes\n\n### Fixed Issues\n\n1. **Eliminated `FinalizeConfig` struct (−52 lines)** — Changed `run_with_lifecycle` to take `config: &mut RunConfig` instead of consuming `config: RunConfig` by value. This means:\n - The caller retains ownership of `RunConfig` and can read any fields mutated by remote git setup (e.g., `run_branch`, `base_sha`) directly after the call\n - Deleted the entire `FinalizeConfig` struct, its `from_run_config()` and `from_run_config_owned()` constructors\n - Renamed `write_finalize_commit_from(&FinalizeConfig)` back to `write_finalize_commit(&RunConfig)`\n - Simplified all `match &engine_result { Ok((ref o, _)) => ... }` to `match &engine_result { Ok(ref o) => ... }`\n - Eliminated all `finalize_config.field` references, using `config.field` directly\n - The error path is now actually more correct: if remote git setup succeeded but a later step failed, the caller still sees the populated metadata branch fields (previously they were reset to `None` in `from_run_config_owned`)\n\n2. **Eliminated duplicated `run_internal` calls in `run_with_lifecycle` (−12 lines)** — Step 7 was manually calling `run_internal` with identical logic to the existing `run()` and `run_from_checkpoint()` methods. Now it simply delegates to those existing public methods.\n\n3. **Fixed misleading `_ssh_command` variable name** — The variable is used in the `tracing::info!` macro, so the leading underscore was incorrect. Changed to `ssh_command`.\n\n**Net impact: −83 lines** (44 insertions, 127 deletions across the cleanup diff)", "thread.simplify_gemini.current_node": "simplify_gpt", - "current.preamble": "Goal: # Plan: Move sandbox lifecycle into the engine\n\n## Context\n\n`SandboxReady` and `SandboxCleanup` hook events are defined in `HookEvent` but never fired. They can't fire naturally because sandbox init/cleanup happens in `cli/run.rs` — outside the engine where all other hooks fire. The root cause is that `cli/run.rs` is a ~2000-line god function orchestrating everything: sandbox creation, init, setup commands, devcontainer lifecycle, engine construction, engine run, retro, PR creation, and cleanup. The engine receives an already-initialized sandbox and has no role in lifecycle management.\n\nThis refactor moves sandbox initialization and setup into the engine so `SandboxReady` fires naturally alongside `RunStart`. Cleanup stays callable from the CLI (the retro agent needs the sandbox alive after the graph completes) but fires through an engine method so `SandboxCleanup` also uses the hook infrastructure.\n\n## Design\n\n### Two new engine methods\n\n**`run_with_lifecycle(graph, config, lifecycle, checkpoint?) -> Result`**\n1. `sandbox.initialize()`\n2. Fire `SandboxReady` hook (blocking — can abort run)\n3. Emit `SandboxInitialized` event (CLI listener writes `sandbox.json`, updates progress UI)\n4. Remote git setup if `sandbox.is_remote()` — produces `base_sha`, `run_branch`, `base_branch`, merged into `config`\n5. Run setup commands inside sandbox\n6. Run devcontainer lifecycle phases inside sandbox\n7. Call existing `run_internal()` (unchanged — fires RunStart → graph → RunComplete)\n8. Return outcome (sandbox still alive)\n\n**`cleanup_sandbox(run_id, workflow_name, preserve) -> Result<()>`**\n1. Fire `SandboxCleanup` hook (non-blocking)\n2. If `!preserve`: call `sandbox.cleanup()`\n\nExisting `run()` is unchanged — API server and integration tests keep using it with pre-initialized sandboxes.\n\n### New config struct\n\n```rust\n// engine.rs\npub struct LifecycleConfig {\n pub setup_commands: Vec,\n pub setup_command_timeout_ms: u64,\n pub devcontainer_phases: Vec<(String, Vec)>,\n}\n```\n\n`run_with_lifecycle` takes `config: RunConfig` by value (currently `&RunConfig`) so it can fill in remote git values. `run_internal` continues to take `&RunConfig`.\n\n### CLI flow after refactor\n\n```\nsandbox = create_sandbox() // unchanged\nsandbox = ReadBeforeWriteSandbox::new(sandbox) // moved before init (delegate_sandbox! delegates initialize)\nengine = build_engine(sandbox, hook_runner, ...)\noutcome = engine.run_with_lifecycle(graph, config, lifecycle)\n// retro, conclusion, PR creation — sandbox still alive\nengine.cleanup_sandbox(run_id, workflow_name, preserve)\n```\n\nSingle scopeguard around the entire block that calls `engine.cleanup_sandbox()` on panic.\n\n## Steps\n\n### 1. `hook/types.rs` — Make `SandboxReady` blocking by default\n- Add `Self::SandboxReady` to `is_blocking_by_default()` match arm (line 29-32)\n- `SandboxCleanup` stays non-blocking (correct default)\n\n### 2. `engine.rs` — Add `LifecycleConfig` struct and `run_with_lifecycle` method\n- Define `LifecycleConfig` (setup_commands, setup_command_timeout_ms, devcontainer_phases)\n- Add `pub async fn run_with_lifecycle(self, graph, config, lifecycle, checkpoint) -> Result` that:\n - Calls `self.services.sandbox.initialize()`\n - Fires `SandboxReady` hook via `self.run_hooks()`\n - Emits `WorkflowRunEvent::SandboxInitialized { working_directory }` via emitter\n - Calls remote git setup if `sandbox.is_remote()`, fills config.base_sha/run_branch/base_branch/git_checkpoint_enabled\n - Runs setup commands via `sandbox.exec_command()`, emitting Setup* events\n - Runs devcontainer lifecycle via `devcontainer_bridge::run_devcontainer_lifecycle()`\n - Calls `self.run_internal()` (or `run_from_checkpoint` path)\n - Returns outcome\n\n### 3. `engine.rs` — Add `cleanup_sandbox` method\n- `pub async fn cleanup_sandbox(&self, run_id, workflow_name, preserve) -> Result<(), String>`\n- Fires `SandboxCleanup` hook\n- If `!preserve`: calls `self.services.sandbox.cleanup()`\n\n### 4. `engine.rs` — Move `setup_remote_git` from `cli/run.rs`\n- Move the `setup_remote_git()` function (cli/run.rs line 1636-1679) into engine.rs\n- It only uses `sandbox.exec_command()` and `run_id` — no CLI dependencies\n\n### 5. `event.rs` — Add `SandboxInitialized` event variant\n- Add `WorkflowRunEvent::SandboxInitialized { working_directory: String }` variant\n- This replaces the inline sandbox.json writing in cli/run.rs\n\n### 6. `cli/run.rs` — Register event listener for sandbox.json\n- Before calling `run_with_lifecycle`, register a listener on the emitter for `SandboxInitialized`\n- Listener captures the pre-built `SandboxRecord` template (all provider-specific fields filled, `working_directory` empty)\n- On event: fill `working_directory` from event, call `record.save()`\n- Also update progress UI `set_working_directory` in the same listener\n\n### 7. `cli/run.rs` — Refactor `run_command` to use new engine methods\n- Move `ReadBeforeWriteSandbox` wrapping to before engine construction (currently at line 966, after init — delegate_sandbox! macro delegates initialize so wrapping before init works)\n- Move `HookRunner` creation earlier (before engine construction) — currently line 1222, move to ~line 800\n- Remove: `sandbox.initialize()` (line 886), remote git setup (lines 982-996), setup commands (lines 1031-1072), devcontainer lifecycle (lines 1074-1091)\n- Build `LifecycleConfig` from `setup_commands` and `devcontainer_config`\n- Build `RunConfig` without remote git fields (leave base_sha/run_branch/base_branch as None for remote — engine fills them)\n- Call `engine.run_with_lifecycle()` instead of `engine.run()`\n- Replace cleanup section (lines 1587-1604) with `engine.cleanup_sandbox()`\n- Replace two scopeguards with one that calls `engine.cleanup_sandbox()` on panic\n- Remove `status_guard` for SandboxInitFailed — engine handles init errors\n\n### 8. `cli/run.rs` — Refactor `run_from_branch` to use new engine methods\n- Use `run_with_lifecycle` with empty `LifecycleConfig` (no setup commands, no devcontainer)\n- Add `cleanup_sandbox()` call (currently no scopeguard — this is an improvement)\n- This gives the resume path hooks for free (currently has zero hooks)\n\n### 9. `docs/agents/hooks.mdx` — Update docs\n- Remove any \"reserved\" annotations for `sandbox_ready` / `sandbox_cleanup`\n- Note that `sandbox_ready` is blocking by default\n\n## Files to modify\n- `lib/crates/fabro-workflows/src/hook/types.rs` — SandboxReady blocking default\n- `lib/crates/fabro-workflows/src/engine.rs` — LifecycleConfig, run_with_lifecycle, cleanup_sandbox, setup_remote_git\n- `lib/crates/fabro-workflows/src/event.rs` — SandboxInitialized event variant\n- `lib/crates/fabro-workflows/src/cli/run.rs` — major simplification of run_command and run_from_branch\n- `docs/agents/hooks.mdx` — remove \"reserved\" annotations\n\n## Files unchanged\n- `lib/crates/fabro-workflows/src/handler/mod.rs` — EngineServices unchanged\n- `lib/crates/fabro-workflows/src/hook/runner.rs` — handles any HookEvent generically\n- `lib/crates/fabro-agent/src/sandbox.rs` — Sandbox trait unchanged\n- `lib/crates/fabro-agent/src/read_before_write_sandbox.rs` — delegate_sandbox! already delegates initialize/cleanup\n- All sandbox implementations — unchanged\n\n## Verification\n1. `cargo build --workspace` — compile check\n2. `cargo test --workspace` — all existing tests pass (existing `run()` path unchanged)\n3. `cargo clippy --workspace -- -D warnings` — no new warnings\n4. Manual test: `fabro run` with a workflow that has `sandbox_ready` and `sandbox_cleanup` hooks configured — verify hooks fire\n5. Manual test: `fabro run --sandbox daytona` — verify remote git setup still works through the engine\n6. Manual test: `fabro run` with `--preserve-sandbox` — verify cleanup is skipped but SandboxCleanup hook still fires\n\n\n## Completed stages\n- **toolchain**: success\n - Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **implement**: success\n - Model: claude-opus-4-6, 172.3k tokens in / 48.5k out\n - Files: /home/daytona/workspace/docs/agents/hooks.mdx, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/event.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/hook/types.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 90.5k tokens in / 16.9k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_gemini**: success\n - Model: claude-opus-4-6, 92.0k tokens in / 18.0k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n", - "current_node": "simplify_gpt", + "current.preamble": "Goal: # Plan: Move sandbox lifecycle into the engine\n\n## Context\n\n`SandboxReady` and `SandboxCleanup` hook events are defined in `HookEvent` but never fired. They can't fire naturally because sandbox init/cleanup happens in `cli/run.rs` — outside the engine where all other hooks fire. The root cause is that `cli/run.rs` is a ~2000-line god function orchestrating everything: sandbox creation, init, setup commands, devcontainer lifecycle, engine construction, engine run, retro, PR creation, and cleanup. The engine receives an already-initialized sandbox and has no role in lifecycle management.\n\nThis refactor moves sandbox initialization and setup into the engine so `SandboxReady` fires naturally alongside `RunStart`. Cleanup stays callable from the CLI (the retro agent needs the sandbox alive after the graph completes) but fires through an engine method so `SandboxCleanup` also uses the hook infrastructure.\n\n## Design\n\n### Two new engine methods\n\n**`run_with_lifecycle(graph, config, lifecycle, checkpoint?) -> Result`**\n1. `sandbox.initialize()`\n2. Fire `SandboxReady` hook (blocking — can abort run)\n3. Emit `SandboxInitialized` event (CLI listener writes `sandbox.json`, updates progress UI)\n4. Remote git setup if `sandbox.is_remote()` — produces `base_sha`, `run_branch`, `base_branch`, merged into `config`\n5. Run setup commands inside sandbox\n6. Run devcontainer lifecycle phases inside sandbox\n7. Call existing `run_internal()` (unchanged — fires RunStart → graph → RunComplete)\n8. Return outcome (sandbox still alive)\n\n**`cleanup_sandbox(run_id, workflow_name, preserve) -> Result<()>`**\n1. Fire `SandboxCleanup` hook (non-blocking)\n2. If `!preserve`: call `sandbox.cleanup()`\n\nExisting `run()` is unchanged — API server and integration tests keep using it with pre-initialized sandboxes.\n\n### New config struct\n\n```rust\n// engine.rs\npub struct LifecycleConfig {\n pub setup_commands: Vec,\n pub setup_command_timeout_ms: u64,\n pub devcontainer_phases: Vec<(String, Vec)>,\n}\n```\n\n`run_with_lifecycle` takes `config: RunConfig` by value (currently `&RunConfig`) so it can fill in remote git values. `run_internal` continues to take `&RunConfig`.\n\n### CLI flow after refactor\n\n```\nsandbox = create_sandbox() // unchanged\nsandbox = ReadBeforeWriteSandbox::new(sandbox) // moved before init (delegate_sandbox! delegates initialize)\nengine = build_engine(sandbox, hook_runner, ...)\noutcome = engine.run_with_lifecycle(graph, config, lifecycle)\n// retro, conclusion, PR creation — sandbox still alive\nengine.cleanup_sandbox(run_id, workflow_name, preserve)\n```\n\nSingle scopeguard around the entire block that calls `engine.cleanup_sandbox()` on panic.\n\n## Steps\n\n### 1. `hook/types.rs` — Make `SandboxReady` blocking by default\n- Add `Self::SandboxReady` to `is_blocking_by_default()` match arm (line 29-32)\n- `SandboxCleanup` stays non-blocking (correct default)\n\n### 2. `engine.rs` — Add `LifecycleConfig` struct and `run_with_lifecycle` method\n- Define `LifecycleConfig` (setup_commands, setup_command_timeout_ms, devcontainer_phases)\n- Add `pub async fn run_with_lifecycle(self, graph, config, lifecycle, checkpoint) -> Result` that:\n - Calls `self.services.sandbox.initialize()`\n - Fires `SandboxReady` hook via `self.run_hooks()`\n - Emits `WorkflowRunEvent::SandboxInitialized { working_directory }` via emitter\n - Calls remote git setup if `sandbox.is_remote()`, fills config.base_sha/run_branch/base_branch/git_checkpoint_enabled\n - Runs setup commands via `sandbox.exec_command()`, emitting Setup* events\n - Runs devcontainer lifecycle via `devcontainer_bridge::run_devcontainer_lifecycle()`\n - Calls `self.run_internal()` (or `run_from_checkpoint` path)\n - Returns outcome\n\n### 3. `engine.rs` — Add `cleanup_sandbox` method\n- `pub async fn cleanup_sandbox(&self, run_id, workflow_name, preserve) -> Result<(), String>`\n- Fires `SandboxCleanup` hook\n- If `!preserve`: calls `self.services.sandbox.cleanup()`\n\n### 4. `engine.rs` — Move `setup_remote_git` from `cli/run.rs`\n- Move the `setup_remote_git()` function (cli/run.rs line 1636-1679) into engine.rs\n- It only uses `sandbox.exec_command()` and `run_id` — no CLI dependencies\n\n### 5. `event.rs` — Add `SandboxInitialized` event variant\n- Add `WorkflowRunEvent::SandboxInitialized { working_directory: String }` variant\n- This replaces the inline sandbox.json writing in cli/run.rs\n\n### 6. `cli/run.rs` — Register event listener for sandbox.json\n- Before calling `run_with_lifecycle`, register a listener on the emitter for `SandboxInitialized`\n- Listener captures the pre-built `SandboxRecord` template (all provider-specific fields filled, `working_directory` empty)\n- On event: fill `working_directory` from event, call `record.save()`\n- Also update progress UI `set_working_directory` in the same listener\n\n### 7. `cli/run.rs` — Refactor `run_command` to use new engine methods\n- Move `ReadBeforeWriteSandbox` wrapping to before engine construction (currently at line 966, after init — delegate_sandbox! macro delegates initialize so wrapping before init works)\n- Move `HookRunner` creation earlier (before engine construction) — currently line 1222, move to ~line 800\n- Remove: `sandbox.initialize()` (line 886), remote git setup (lines 982-996), setup commands (lines 1031-1072), devcontainer lifecycle (lines 1074-1091)\n- Build `LifecycleConfig` from `setup_commands` and `devcontainer_config`\n- Build `RunConfig` without remote git fields (leave base_sha/run_branch/base_branch as None for remote — engine fills them)\n- Call `engine.run_with_lifecycle()` instead of `engine.run()`\n- Replace cleanup section (lines 1587-1604) with `engine.cleanup_sandbox()`\n- Replace two scopeguards with one that calls `engine.cleanup_sandbox()` on panic\n- Remove `status_guard` for SandboxInitFailed — engine handles init errors\n\n### 8. `cli/run.rs` — Refactor `run_from_branch` to use new engine methods\n- Use `run_with_lifecycle` with empty `LifecycleConfig` (no setup commands, no devcontainer)\n- Add `cleanup_sandbox()` call (currently no scopeguard — this is an improvement)\n- This gives the resume path hooks for free (currently has zero hooks)\n\n### 9. `docs/agents/hooks.mdx` — Update docs\n- Remove any \"reserved\" annotations for `sandbox_ready` / `sandbox_cleanup`\n- Note that `sandbox_ready` is blocking by default\n\n## Files to modify\n- `lib/crates/fabro-workflows/src/hook/types.rs` — SandboxReady blocking default\n- `lib/crates/fabro-workflows/src/engine.rs` — LifecycleConfig, run_with_lifecycle, cleanup_sandbox, setup_remote_git\n- `lib/crates/fabro-workflows/src/event.rs` — SandboxInitialized event variant\n- `lib/crates/fabro-workflows/src/cli/run.rs` — major simplification of run_command and run_from_branch\n- `docs/agents/hooks.mdx` — remove \"reserved\" annotations\n\n## Files unchanged\n- `lib/crates/fabro-workflows/src/handler/mod.rs` — EngineServices unchanged\n- `lib/crates/fabro-workflows/src/hook/runner.rs` — handles any HookEvent generically\n- `lib/crates/fabro-agent/src/sandbox.rs` — Sandbox trait unchanged\n- `lib/crates/fabro-agent/src/read_before_write_sandbox.rs` — delegate_sandbox! already delegates initialize/cleanup\n- All sandbox implementations — unchanged\n\n## Verification\n1. `cargo build --workspace` — compile check\n2. `cargo test --workspace` — all existing tests pass (existing `run()` path unchanged)\n3. `cargo clippy --workspace -- -D warnings` — no new warnings\n4. Manual test: `fabro run` with a workflow that has `sandbox_ready` and `sandbox_cleanup` hooks configured — verify hooks fire\n5. Manual test: `fabro run --sandbox daytona` — verify remote git setup still works through the engine\n6. Manual test: `fabro run` with `--preserve-sandbox` — verify cleanup is skipped but SandboxCleanup hook still fires\n\n\n## Completed stages\n- **toolchain**: success\n - Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **implement**: success\n - Model: claude-opus-4-6, 172.3k tokens in / 48.5k out\n - Files: /home/daytona/workspace/docs/agents/hooks.mdx, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/event.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/hook/types.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 90.5k tokens in / 16.9k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_gemini**: success\n - Model: claude-opus-4-6, 92.0k tokens in / 18.0k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_gpt**: success\n - Model: claude-opus-4-6, 81.9k tokens in / 21.5k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n", + "current_node": "verify", "thread.toolchain.current_node": "preflight_compile", "thread.preflight_lint.current_node": "implement", "response.implement": "All 1655 unit tests pass. 179 integration tests pass. No failures.\n\nLet me do one final summary of all the changes made:\n\n## Summary of Changes\n\n### 1. `lib/crates/fabro-workflows/src/hook/types.rs`\n- Added `Self::SandboxReady` to `is_blocking_by_default()` match arm — `SandboxReady` is now blocking by default\n- Updated test `hook_event_blocking_defaults` to verify `SandboxReady` is blocking and `SandboxCleanup` is not\n\n### 2. `lib/crates/fabro-workflows/src/event.rs`\n- Added `WorkflowRunEvent::SandboxInitialized { working_directory: String }` variant\n- Added `trace()` implementation for the new variant (logs via `info!`)\n- Added serialization/deserialization test\n- Added `flatten_event` test\n\n### 3. `lib/crates/fabro-workflows/src/engine.rs`\n- Added `setup_remote_git()` public function (moved from `cli/run.rs`)\n- Added `LifecycleConfig` struct with `setup_commands`, `setup_command_timeout_ms`, and `devcontainer_phases`\n- Added `run_with_lifecycle()` method that:\n 1. Calls `sandbox.initialize()`\n 2. Fires `SandboxReady` hook (blocking — can abort run)\n 3. Emits `SandboxInitialized` event\n 4. Runs remote git setup if sandbox is remote and config doesn't already have git info\n 5. Runs setup commands inside sandbox\n 6. Runs devcontainer lifecycle phases inside sandbox\n 7. Calls `run_internal()` (or checkpoint resume path)\n- Added `cleanup_sandbox()` method that fires `SandboxCleanup` hook and conditionally cleans up\n- Added 5 tests for the new methods:\n - `run_with_lifecycle_fires_sandbox_initialized_event`\n - `run_with_lifecycle_runs_setup_commands`\n - `run_with_lifecycle_setup_failure_aborts_run`\n - `cleanup_sandbox_fires_hook`\n - `run_with_lifecycle_emits_events_in_order`\n\n### 4. `lib/crates/fabro-workflows/src/cli/run.rs`\n- **`run_command`**: \n - Moved `ReadBeforeWriteSandbox` wrapping before engine construction\n - Removed inline `sandbox.initialize()`, remote git setup, setup commands, devcontainer lifecycle\n - Added `SandboxInitialized` event listener (registered before emitter Arc wrapping) that writes `sandbox.json` and updates progress UI\n - Added SSH access listener\n - Builds `LifecycleConfig` and calls `engine.run_with_lifecycle()` instead of `engine.run()`\n - Replaced inline cleanup with `engine.cleanup_sandbox()`\n - Added `FinalizeConfig` struct to capture config fields needed after `run_with_lifecycle` consumes config\n- **`run_from_branch`**: \n - Moved `ReadBeforeWriteSandbox` wrapping before engine construction\n - Remote sandbox init + branch checkout converted to setup_commands in `LifecycleConfig`\n - Uses `run_with_lifecycle()` instead of `run_from_checkpoint()`\n - Added `engine.cleanup_sandbox()` call (previously just did `sandbox.cleanup()`)\n - Added `FinalizeConfig` for post-run operations\n- Removed `setup_remote_git()` function (moved to engine.rs)\n- Added `FinalizeConfig` struct and `write_finalize_commit_from()` function\n\n### 5. `docs/agents/hooks.mdx`\n- Updated `sandbox_ready` to show \"Yes\" for blocking by default\n- Updated description to \"After the sandbox is initialized and ready\"\n- Added `sandbox_ready` to the \"Blocking by default\" list", @@ -55,7 +59,7 @@ "response.simplify_gemini": "Everything passes. Here's a summary of what was fixed:\n\n## Summary\n\n### Fixed Issues\n\n1. **Stringly-typed provider comparison** (`cli/run.rs`): Replaced `provider_name == \"docker\"` and `provider_name == \"ssh\"` string comparisons with `provider == SandboxProvider::Docker` and `provider == SandboxProvider::Ssh` enum comparisons. This eliminated the intermediate `sandbox_provider_name` match arm and the `provider_name: String` allocation, using the `Copy`-implementing `SandboxProvider` enum directly in the event listener closure.\n\n2. **Test boilerplate** (`engine.rs`): Extracted `test_run_config(run_dir, run_id)` and `test_lifecycle(setup_commands)` helper functions that build default `RunConfig` and `LifecycleConfig` for tests. The 5 new lifecycle tests each had ~20 lines of identical config construction that now reduce to single-line calls. This removed ~95 lines of duplicated test code.\n\n### Findings Not Fixed (by design)\n\n- **`FinalizeConfig`**: While it copies fields from `RunConfig`, it serves a genuine purpose — the error path needs config values when `run_with_lifecycle` fails and doesn't return a `RunConfig`. Eliminating it would require restructuring the entire post-engine control flow, which would be a larger refactor with no functional benefit.\n- **`deferred_sandbox` pattern (`Arc>>>`**: This is inherently needed because event listeners are registered before the sandbox is created, but need access to it later. The sandbox can't be created earlier (it depends on provider config), and the listeners must be registered before the emitter is wrapped in `Arc`. The pattern is local to this scope and the mutex is only locked once (when the event fires).\n- **Existing 43 `RunConfig` constructions in tests**: The new helpers only apply to the 5 new tests. Backfilling all existing tests would be scope creep unrelated to this PR.", "internal.fidelity": "compact", "last_stage": "simplify_gpt", - "internal.thread_id": "simplify_gemini", + "internal.thread_id": "simplify_gpt", "internal.retry_count.implement": 1 }, "logs": [], @@ -185,9 +189,18 @@ }, "notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1", "duration_ms": 16704 + }, + "verify": { + "status": "success", + "context_updates": { + "command.output": "────────────\n Nextest run ID 6d5ca597-5977-4dbf-99b7-477603e0ee65 with nextest profile: default\n Starting 3416 tests across 38 binaries (183 tests skipped)\n────────────\n Summary [ 15.408s] 3416 tests run: 3416 passed, 183 skipped\n", + "command.stderr": "" + }, + "notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", + "duration_ms": 19254 } }, - "next_node_id": "verify", + "next_node_id": "fmt", "node_visits": { "preflight_compile": 1, "toolchain": 1, @@ -196,6 +209,7 @@ "start": 1, "preflight_lint": 1, "simplify_opus": 1, - "simplify_gemini": 1 + "simplify_gemini": 1, + "verify": 1 } } \ No newline at end of file diff --git a/nodes/simplify_gpt/diff.patch b/nodes/simplify_gpt/diff.patch new file mode 100644 index 000000000..01b48504a --- /dev/null +++ b/nodes/simplify_gpt/diff.patch @@ -0,0 +1,390 @@ +diff --git a/lib/crates/fabro-workflows/src/cli/run.rs b/lib/crates/fabro-workflows/src/cli/run.rs +index 806d1ae..ad8ff7a 100644 +--- a/lib/crates/fabro-workflows/src/cli/run.rs ++++ b/lib/crates/fabro-workflows/src/cli/run.rs +@@ -866,10 +866,10 @@ pub async fn run_command( + let sb = Arc::clone(sb); + rt.spawn(async move { + match sb.ssh_access_command().await { +- Ok(Some(_ssh_command)) => { ++ Ok(Some(ssh_command)) => { + // Note: we can't emit from here since emitter is shared; + // SSH access info is logged via tracing. +- tracing::info!(ssh_command = _ssh_command, "SSH access ready"); ++ tracing::info!(ssh_command, "SSH access ready"); + } + Ok(None) => {} + Err(e) => { +@@ -1128,7 +1128,7 @@ pub async fn run_command( + .map(|c| c.checkpoint.exclude_globs.clone()) + .unwrap_or_default(); + let pr_cfg = run_cfg.as_ref().and_then(|c| c.pull_request.as_ref()); +- let config = RunConfig { ++ let mut config = RunConfig { + run_dir: run_dir.clone(), + cancel_token: None, + dry_run: dry_run_mode, +@@ -1194,11 +1194,11 @@ pub async fn run_command( + let engine_result = if let Some(ref checkpoint_path) = args.resume { + let checkpoint = Checkpoint::load(checkpoint_path)?; + engine +- .run_with_lifecycle(&graph, config, lifecycle, Some(&checkpoint)) ++ .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) + .await + } else { + engine +- .run_with_lifecycle(&graph, config, lifecycle, None) ++ .run_with_lifecycle(&graph, &mut config, lifecycle, None) + .await + }; + let run_duration_ms = run_start.elapsed().as_millis() as u64; +@@ -1206,24 +1206,15 @@ pub async fn run_command( + // Restore cwd (worktree is kept for `fabro cp` access; pruned separately) + let _ = std::env::set_current_dir(&original_cwd); + +- // Build FinalizeConfig from the (potentially mutated) config returned by the engine. +- // For remote sandboxes, run_with_lifecycle fills run_branch, base_sha, base_branch, etc. +- let finalize_config = match &engine_result { +- Ok((_, ref config)) => FinalizeConfig::from_run_config(config), +- Err(_) => { +- FinalizeConfig::from_run_config_owned(run_id.clone(), None, Some(original_cwd.clone())) +- } +- }; +- + { + let (status, failure_reason) = match &engine_result { +- Ok((ref o, _)) => (o.status.clone(), o.failure_reason().map(String::from)), ++ Ok(ref o) => (o.status.clone(), o.failure_reason().map(String::from)), + Err(e) => (crate::outcome::StageStatus::Fail, Some(e.to_string())), + }; + + // Map engine result to RunStatus + StatusReason + let (run_status, status_reason) = match &engine_result { +- Ok((ref o, _)) => match o.status { ++ Ok(ref o) => match o.status { + StageStatus::Success | StageStatus::Skipped => ( + crate::run_status::RunStatus::Succeeded, + Some(crate::run_status::StatusReason::Completed), +@@ -1301,14 +1292,14 @@ pub async fn run_command( + // Auto-derive retro (always, cheap) and optionally run retro agent + if !args.no_retro && super::project_config::is_retro_enabled() { + let (failed, failure_reason) = match &engine_result { +- Ok((ref o, _)) => ( ++ Ok(ref o) => ( + o.status == StageStatus::Fail, + o.failure_reason().map(String::from), + ), + Err(e) => (true, Some(e.to_string())), + }; + generate_retro( +- &finalize_config.run_id, ++ &config.run_id, + &graph.name, + graph.goal(), + &run_dir, +@@ -1330,18 +1321,18 @@ pub async fn run_command( + progress_ui.lock().expect("progress lock poisoned").finish(); + + // Write finalize commit with retro.json + final node files (captures last diff.patch) +- write_finalize_commit_from(&finalize_config, &run_dir).await; ++ write_finalize_commit(&config, &run_dir).await; + + // Auto-create PR on successful completion (skip in dry-run mode) + let mut pushed_branch: Option = None; + let mut pr_url: Option = None; +- if !finalize_config.pull_request_enabled { ++ if !config.pull_request_enabled { + debug!("Skipping PR creation: pull_request not enabled in config"); + } else if dry_run_mode { + debug!("Skipping PR creation: dry-run mode"); + } else if let Err(ref e) = engine_result { + debug!(error = %e, "Skipping PR creation: engine returned an error"); +- } else if let Ok((ref outcome, _)) = engine_result { ++ } else if let Ok(ref outcome) = engine_result { + if !matches!( + outcome.status, + StageStatus::Success | StageStatus::PartialSuccess +@@ -1357,14 +1348,14 @@ pub async fn run_command( + Some(ref creds), + Some(ref origin), + ) = ( +- &finalize_config.base_branch, +- &finalize_config.run_branch, ++ &config.base_branch, ++ &config.run_branch, + &github_app, + &origin_url, + ) { + // Run branch was pushed during checkpoint commits; + // just record it for the PR creation. +- if finalize_config.git_checkpoint_enabled { ++ if config.git_checkpoint_enabled { + pushed_branch = Some(run_branch.clone()); + } + +@@ -1376,7 +1367,7 @@ pub async fn run_command( + graph.goal(), + &diff, + &model, +- finalize_config.pull_request_draft, ++ config.pull_request_draft, + &run_dir, + ) + .await +@@ -1385,7 +1376,7 @@ pub async fn run_command( + emitter.emit(&crate::event::WorkflowRunEvent::PullRequestCreated { + pr_url: record.html_url.clone(), + pr_number: record.number, +- draft: finalize_config.pull_request_draft, ++ draft: config.pull_request_draft, + }); + pr_url = Some(record.html_url.clone()); + if let Err(e) = record.save(&run_dir.join("pull_request.json")) { +@@ -1407,7 +1398,7 @@ pub async fn run_command( + } + } + +- let (outcome, _) = engine_result?; ++ let outcome = engine_result?; + + // 8. Print result + eprintln!("\n{}", styles.bold.apply_to("=== Run Result ==="),); +@@ -1759,7 +1750,7 @@ async fn run_from_branch( + } + + let meta_branch = Some(crate::git::MetadataStore::branch_name(&run_id)); +- let config = RunConfig { ++ let mut config = RunConfig { + run_dir: run_dir.clone(), + cancel_token: None, + dry_run: dry_run_mode, +@@ -1792,25 +1783,17 @@ async fn run_from_branch( + + let run_start = Instant::now(); + let engine_result = engine +- .run_with_lifecycle(&graph, config, lifecycle, Some(&checkpoint)) ++ .run_with_lifecycle(&graph, &mut config, lifecycle, Some(&checkpoint)) + .await; + let run_duration_ms = run_start.elapsed().as_millis() as u64; + + // Restore cwd (worktree is kept for `fabro cp` access; pruned separately) + let _ = std::env::set_current_dir(&original_cwd); + +- // Build FinalizeConfig from the (potentially mutated) config returned by the engine. +- let finalize_config = match &engine_result { +- Ok((_, ref config)) => FinalizeConfig::from_run_config(config), +- Err(_) => { +- FinalizeConfig::from_run_config_owned(run_id.clone(), None, Some(original_cwd.clone())) +- } +- }; +- + // Auto-derive retro + if !args.no_retro && super::project_config::is_retro_enabled() { + let (failed, failure_reason) = match &engine_result { +- Ok((ref o, _)) => ( ++ Ok(ref o) => ( + o.status == StageStatus::Fail, + o.failure_reason().map(String::from), + ), +@@ -1824,7 +1807,7 @@ async fn run_from_branch( + }; + + generate_retro( +- &finalize_config.run_id, ++ &config.run_id, + &graph.name, + graph.goal(), + &run_dir, +@@ -1843,14 +1826,14 @@ async fn run_from_branch( + } + + // Write finalize commit with retro.json + final node files (captures last diff.patch) +- write_finalize_commit_from(&finalize_config, &run_dir).await; ++ write_finalize_commit(&config, &run_dir).await; + + // Cleanup sandbox via engine (fires SandboxCleanup hook) + let _ = engine +- .cleanup_sandbox(&finalize_config.run_id, &graph.name, false) ++ .cleanup_sandbox(&config.run_id, &graph.name, false) + .await; + +- let (outcome, _) = engine_result?; ++ let outcome = engine_result?; + + eprintln!("\n{}", styles.bold.apply_to("=== Run Result ==="),); + eprintln!("{}", styles.dim.apply_to(format!("Run: {run_id}"))); +@@ -2271,63 +2254,11 @@ async fn run_preflight( + } + } + +-/// Subset of `RunConfig` fields needed after `run_with_lifecycle` consumes the config. +-struct FinalizeConfig { +- run_id: String, +- meta_branch: Option, +- host_repo_path: Option, +- git_author: crate::git::GitAuthor, +- github_app: Option, +- base_branch: Option, +- run_branch: Option, +- git_checkpoint_enabled: bool, +- pull_request_enabled: bool, +- pull_request_draft: bool, +-} +- +-impl FinalizeConfig { +- /// Build from the (potentially mutated) `RunConfig` returned by the engine. +- fn from_run_config(config: &RunConfig) -> Self { +- Self { +- run_id: config.run_id.clone(), +- meta_branch: config.meta_branch.clone(), +- host_repo_path: config.host_repo_path.clone(), +- git_author: config.git_author.clone(), +- github_app: config.github_app.clone(), +- base_branch: config.base_branch.clone(), +- run_branch: config.run_branch.clone(), +- git_checkpoint_enabled: config.git_checkpoint_enabled, +- pull_request_enabled: config.pull_request_enabled, +- pull_request_draft: config.pull_request_draft, +- } +- } +- +- /// Minimal fallback for error paths where the engine didn't return a config. +- fn from_run_config_owned( +- run_id: String, +- meta_branch: Option, +- host_repo_path: Option, +- ) -> Self { +- Self { +- run_id, +- meta_branch, +- host_repo_path, +- git_author: crate::git::GitAuthor::default(), +- github_app: None, +- base_branch: None, +- run_branch: None, +- git_checkpoint_enabled: false, +- pull_request_enabled: false, +- pull_request_draft: false, +- } +- } +-} +- + /// Write a finalize commit to the shadow branch with retro.json and final node files. + /// + /// This captures the last diff.patch (written after the final checkpoint) and retro.json. + /// Best-effort: errors are logged as warnings. +-async fn write_finalize_commit_from(config: &FinalizeConfig, run_dir: &std::path::Path) { ++async fn write_finalize_commit(config: &RunConfig, run_dir: &std::path::Path) { + let (Some(ref meta_branch), Some(ref repo_path)) = + (&config.meta_branch, &config.host_repo_path) + else { +diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs +index f01f447..963844b 100644 +--- a/lib/crates/fabro-workflows/src/engine.rs ++++ b/lib/crates/fabro-workflows/src/engine.rs +@@ -1266,16 +1266,15 @@ impl WorkflowRunEngine { + /// The sandbox is left alive after return so the caller can run retro, PR creation, etc. + /// Call `cleanup_sandbox()` when done. + /// +- /// Returns both the outcome and the (potentially mutated) config — remote git setup +- /// fills `run_branch`, `base_sha`, `base_branch`, `meta_branch`, and +- /// `git_checkpoint_enabled`, which callers need for PR creation and finalize commits. ++ /// The config is taken by mutable reference so the caller retains ownership ++ /// and can read any fields mutated by remote git setup after the call. + pub async fn run_with_lifecycle( + &self, + graph: &Graph, +- mut config: RunConfig, ++ config: &mut RunConfig, + lifecycle: LifecycleConfig, + checkpoint: Option<&Checkpoint>, +- ) -> Result<(Outcome, RunConfig)> { ++ ) -> Result { + // 1. Initialize sandbox + self.services + .sandbox +@@ -1389,23 +1388,11 @@ impl WorkflowRunEngine { + } + + // 7. Execute the workflow graph +- let outcome = if let Some(cp) = checkpoint { +- let loop_state = LoopState { +- node_visits: HashMap::new(), +- loop_failure_signatures: cp.loop_failure_signatures.clone(), +- restart_failure_signatures: cp.restart_failure_signatures.clone(), +- }; +- let (outcome, _context) = self +- .run_internal(graph, &config, Some(cp), None, None, loop_state) +- .await?; +- outcome ++ if let Some(cp) = checkpoint { ++ self.run_from_checkpoint(graph, config, cp).await + } else { +- let (outcome, _context) = self +- .run_internal(graph, &config, None, None, None, LoopState::default()) +- .await?; +- outcome +- }; +- Ok((outcome, config)) ++ self.run(graph, config).await ++ } + } + + /// Fire the `SandboxCleanup` hook and optionally clean up the sandbox. +@@ -5715,13 +5702,9 @@ mod tests { + }); + + let engine = WorkflowRunEngine::new(make_registry(), Arc::new(emitter), local_env()); +- let (outcome, _) = engine +- .run_with_lifecycle( +- &g, +- test_run_config(dir.path(), "lifecycle-test"), +- test_lifecycle(Vec::new()), +- None, +- ) ++ let mut config = test_run_config(dir.path(), "lifecycle-test"); ++ let outcome = engine ++ .run_with_lifecycle(&g, &mut config, test_lifecycle(Vec::new()), None) + .await + .unwrap(); + assert_eq!(outcome.status, StageStatus::Success); +@@ -5750,10 +5733,11 @@ mod tests { + }); + + let engine = WorkflowRunEngine::new(make_registry(), Arc::new(emitter), local_env()); +- let (outcome, _) = engine ++ let mut config = test_run_config(dir.path(), "setup-test"); ++ let outcome = engine + .run_with_lifecycle( + &g, +- test_run_config(dir.path(), "setup-test"), ++ &mut config, + test_lifecycle(vec!["echo hello".to_string()]), + None, + ) +@@ -5779,10 +5763,11 @@ mod tests { + + let engine = + WorkflowRunEngine::new(make_registry(), Arc::new(EventEmitter::new()), local_env()); ++ let mut config = test_run_config(dir.path(), "setup-fail-test"); + let result = engine + .run_with_lifecycle( + &g, +- test_run_config(dir.path(), "setup-fail-test"), ++ &mut config, + test_lifecycle(vec!["exit 1".to_string()]), + None, + ) +@@ -5825,10 +5810,11 @@ mod tests { + }); + + let engine = WorkflowRunEngine::new(make_registry(), Arc::new(emitter), local_env()); ++ let mut config = test_run_config(dir.path(), "order-test"); + engine + .run_with_lifecycle( + &g, +- test_run_config(dir.path(), "order-test"), ++ &mut config, + test_lifecycle(vec!["echo ok".to_string()]), + None, + ) diff --git a/nodes/verify/script_invocation.json b/nodes/verify/script_invocation.json new file mode 100644 index 000000000..c2b2fcf73 --- /dev/null +++ b/nodes/verify/script_invocation.json @@ -0,0 +1,5 @@ +{ + "command": "cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", + "language": "shell", + "timeout_ms": null +} \ No newline at end of file diff --git a/nodes/verify/script_timing.json b/nodes/verify/script_timing.json new file mode 100644 index 000000000..bfe77a50a --- /dev/null +++ b/nodes/verify/script_timing.json @@ -0,0 +1,5 @@ +{ + "duration_ms": 19251, + "exit_code": 0, + "timed_out": false +} \ No newline at end of file diff --git a/nodes/verify/status.json b/nodes/verify/status.json new file mode 100644 index 000000000..b4cea891e --- /dev/null +++ b/nodes/verify/status.json @@ -0,0 +1,6 @@ +{ + "status": "success", + "notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", + "failure_reason": null, + "timestamp": "2026-03-16T05:01:57.085173+00:00" +} \ No newline at end of file