mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-07 03:00:29 +00:00
Fabro
parent
88684f34e6
commit
7fcc9750fc
6 changed files with 357 additions and 24 deletions
|
|
@ -1,14 +1,16 @@
|
|||
{
|
||||
"timestamp": "2026-03-21T15:54:40.225490Z",
|
||||
"current_node": "implement",
|
||||
"timestamp": "2026-03-21T16:03:23.636321Z",
|
||||
"current_node": "simplify_opus",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"toolchain",
|
||||
"preflight_compile",
|
||||
"preflight_lint",
|
||||
"implement"
|
||||
"implement",
|
||||
"simplify_opus"
|
||||
],
|
||||
"node_retries": {
|
||||
"simplify_opus": 1,
|
||||
"preflight_compile": 1,
|
||||
"implement": 1,
|
||||
"toolchain": 1,
|
||||
|
|
@ -16,32 +18,35 @@
|
|||
"start": 1
|
||||
},
|
||||
"context_values": {
|
||||
"current.preamble": "Goal: # Fix: Workflow TOML config lost in detach mode\n\n## Context\n\nWhen running `fabro run -d implement-plan`, the `[pull_request]` config from `fabro.toml` / `cli.toml` / `workflow.toml` is silently dropped, so no PR is created despite `enabled = true` at all config levels.\n\n**Root cause chain**:\n1. `create.rs` tries to copy the workflow TOML as `run.toml`, but checks the **raw CLI arg** (`\"implement-plan\"` — no extension) instead of the resolved path. So `run.toml` is never saved.\n2. `RunEngine` always uses cached `graph.fabro` (a DOT file), so `prepare_workflow` returns `run_cfg = None`, losing all TOML-level config.\n3. `pull_request` and `asset_globs` in `RunConfig` only check `run_cfg` without falling back to `run_defaults`.\n\n## Fix 1: Serialize merged `run_cfg` to `run.toml` in create.rs\n\n**File**: `lib/crates/fabro-cli/src/commands/create.rs`\n\nReplace the raw-file-copy block (lines 53-58) with serialization of the already-merged `WorkflowRunConfig`:\n\n- Change `let prep = prepare_workflow(...)` to `let mut prep = ...`\n- Replace the extension check with:\n ```rust\n if let Some(mut cfg) = prep.run_cfg.take() {\n cfg.graph = \"graph.fabro\".to_string();\n let toml_str = toml::to_string_pretty(&cfg)\n .context(\"Failed to serialize run config\")?;\n tokio::fs::write(run_dir.join(\"run.toml\"), toml_str).await?;\n }\n ```\n- Add `use anyhow::Context;` if needed\n\n**Why serialize instead of copy**: The raw TOML's `graph` field (e.g. `\"workflow.fabro\"`) would point to a nonexistent file in the run dir. Serializing lets us rewrite `graph` to `\"graph.fabro\"` (the cached name). The serialized config also has all defaults merged, env vars resolved, and dockerfiles inlined — making the run dir self-contained.\n\n**Why `take()` not `clone()`**: `WorkflowRunConfig` doesn't derive `Clone`, and `prep.run_cfg` is unused after this point in `create.rs`.\n\n## Fix 2: Use `run.toml` in RunEngine path\n\n**File**: `lib/crates/fabro-cli/src/main.rs` (lines 727-733)\n\nReplace the workflow path resolution to use `run.toml`:\n\n```rust\nlet cached_toml = run_dir.join(\"run.toml\");\nlet workflow_path = if cached_toml.exists() {\n cached_toml\n} else {\n run_dir.join(\"graph.fabro\")\n};\n```\n\nWhen `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and `resolve_graph_path` resolves `\"graph.fabro\"` relative to the run dir — pointing to the cached graph that already exists there.\n\n## Fix 3: Add `run_defaults` fallbacks (defense-in-depth)\n\n**File**: `lib/crates/fabro-cli/src/commands/run.rs`\n\nEven with Fixes 1+2, bare `.fabro` files passed directly would still hit `run_cfg = None`. Add fallbacks matching the pattern already used elsewhere in the file:\n\n**3a. `pull_request`** (line 1424-1428):\n```rust\npull_request: run_cfg\n .as_ref()\n .and_then(|c| c.pull_request.as_ref())\n .or(run_defaults.pull_request.as_ref())\n .filter(|p| p.enabled)\n .cloned(),\n```\n\n**3b. `asset_globs`** (line 1429-1433):\n```rust\nasset_globs: run_cfg\n .as_ref()\n .and_then(|c| c.assets.as_ref())\n .or(run_defaults.assets.as_ref())\n .map(|a| a.include.clone())\n .unwrap_or_default(),\n```\n\n**3c. `devcontainer`** (line 969-973):\n```rust\nlet devcontainer_config = if run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.devcontainer)\n .unwrap_or(false)\n```\n\n**3d. `sandbox.env`** (line 1300-1306):\n```rust\nif let Some(toml_env) = run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.env.clone())\n```\n\n## Verification\n\n1. `cargo build --workspace`\n2. `cargo test --workspace`\n3. `cargo clippy --workspace -- -D warnings`\n4. Manual: `fabro run -d implement-plan` with `[pull_request] enabled = true` in `fabro.toml` → verify `run.toml` in run dir has `graph = \"graph.fabro\"` and `[pull_request]` → verify PR created\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, 20.7k tokens in / 5.3k out\n - Files: /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/main.rs\n",
|
||||
"graph.goal": "# Fix: Workflow TOML config lost in detach mode\n\n## Context\n\nWhen running `fabro run -d implement-plan`, the `[pull_request]` config from `fabro.toml` / `cli.toml` / `workflow.toml` is silently dropped, so no PR is created despite `enabled = true` at all config levels.\n\n**Root cause chain**:\n1. `create.rs` tries to copy the workflow TOML as `run.toml`, but checks the **raw CLI arg** (`\"implement-plan\"` — no extension) instead of the resolved path. So `run.toml` is never saved.\n2. `RunEngine` always uses cached `graph.fabro` (a DOT file), so `prepare_workflow` returns `run_cfg = None`, losing all TOML-level config.\n3. `pull_request` and `asset_globs` in `RunConfig` only check `run_cfg` without falling back to `run_defaults`.\n\n## Fix 1: Serialize merged `run_cfg` to `run.toml` in create.rs\n\n**File**: `lib/crates/fabro-cli/src/commands/create.rs`\n\nReplace the raw-file-copy block (lines 53-58) with serialization of the already-merged `WorkflowRunConfig`:\n\n- Change `let prep = prepare_workflow(...)` to `let mut prep = ...`\n- Replace the extension check with:\n ```rust\n if let Some(mut cfg) = prep.run_cfg.take() {\n cfg.graph = \"graph.fabro\".to_string();\n let toml_str = toml::to_string_pretty(&cfg)\n .context(\"Failed to serialize run config\")?;\n tokio::fs::write(run_dir.join(\"run.toml\"), toml_str).await?;\n }\n ```\n- Add `use anyhow::Context;` if needed\n\n**Why serialize instead of copy**: The raw TOML's `graph` field (e.g. `\"workflow.fabro\"`) would point to a nonexistent file in the run dir. Serializing lets us rewrite `graph` to `\"graph.fabro\"` (the cached name). The serialized config also has all defaults merged, env vars resolved, and dockerfiles inlined — making the run dir self-contained.\n\n**Why `take()` not `clone()`**: `WorkflowRunConfig` doesn't derive `Clone`, and `prep.run_cfg` is unused after this point in `create.rs`.\n\n## Fix 2: Use `run.toml` in RunEngine path\n\n**File**: `lib/crates/fabro-cli/src/main.rs` (lines 727-733)\n\nReplace the workflow path resolution to use `run.toml`:\n\n```rust\nlet cached_toml = run_dir.join(\"run.toml\");\nlet workflow_path = if cached_toml.exists() {\n cached_toml\n} else {\n run_dir.join(\"graph.fabro\")\n};\n```\n\nWhen `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and `resolve_graph_path` resolves `\"graph.fabro\"` relative to the run dir — pointing to the cached graph that already exists there.\n\n## Fix 3: Add `run_defaults` fallbacks (defense-in-depth)\n\n**File**: `lib/crates/fabro-cli/src/commands/run.rs`\n\nEven with Fixes 1+2, bare `.fabro` files passed directly would still hit `run_cfg = None`. Add fallbacks matching the pattern already used elsewhere in the file:\n\n**3a. `pull_request`** (line 1424-1428):\n```rust\npull_request: run_cfg\n .as_ref()\n .and_then(|c| c.pull_request.as_ref())\n .or(run_defaults.pull_request.as_ref())\n .filter(|p| p.enabled)\n .cloned(),\n```\n\n**3b. `asset_globs`** (line 1429-1433):\n```rust\nasset_globs: run_cfg\n .as_ref()\n .and_then(|c| c.assets.as_ref())\n .or(run_defaults.assets.as_ref())\n .map(|a| a.include.clone())\n .unwrap_or_default(),\n```\n\n**3c. `devcontainer`** (line 969-973):\n```rust\nlet devcontainer_config = if run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.devcontainer)\n .unwrap_or(false)\n```\n\n**3d. `sandbox.env`** (line 1300-1306):\n```rust\nif let Some(toml_env) = run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.env.clone())\n```\n\n## Verification\n\n1. `cargo build --workspace`\n2. `cargo test --workspace`\n3. `cargo clippy --workspace -- -D warnings`\n4. Manual: `fabro run -d implement-plan` with `[pull_request] enabled = true` in `fabro.toml` → verify `run.toml` in run dir has `graph = \"graph.fabro\"` and `[pull_request]` → verify PR created\n",
|
||||
"response.simplify_opus": "## Summary\n\n**Fixed one real issue, accepted pre-existing snapshots, skipped false positives:**\n\n### Fixed\n1. **`run.rs` raw TOML copy → merged serialization** (the main fix): `run.rs` line 701-704 was still copying the raw TOML file from disk (the old buggy behavior), while `create.rs` had been fixed to serialize the merged config. This meant `fabro run` (non-detached) would write a `run.toml` with the wrong `graph` path and without defaults merged. Now both `create.rs` and `run.rs` use the same approach: serialize the merged `WorkflowRunConfig` with `graph` rewritten to `\"graph.fabro\"`.\n\n2. **Accepted pre-existing snapshot updates**: 7 `fabro-model` catalog test snapshots needed updating for the new `effort` field in `ModelFeatures`. Removed the stale `.pending-snap` file.\n\n### Skipped (not worth addressing)\n- **Magic string constants for `\"graph.fabro\"`/`\"run.toml\"`**: Valid observation (18+ occurrences across 7+ files), but a cross-crate refactor beyond the scope of this fix.\n- **Extract `resolve_sandbox_config` helper**: The two inline `.or()` additions match the existing code pattern. Extracting a helper for two call sites is over-abstraction.\n- **No efficiency issues**: All three changes are on cold paths with negligible cost.",
|
||||
"internal.retry_count.toolchain": 1,
|
||||
"thread.start.current_node": "toolchain",
|
||||
"outcome": "success",
|
||||
"failure_class": "",
|
||||
"last_response": "## Summary\n\n**Fixed one real issue, accepted pre-existing snapshots, skipped false positives:**\n\n### Fixed\n1. **`run.rs` raw TOML copy → merged serialization** (the main fix): `run.rs` line 701-704 ",
|
||||
"internal.thread_id": "implement",
|
||||
"command.stderr": "",
|
||||
"command.output": "",
|
||||
"graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n ",
|
||||
"internal.run_id": "01KM8HA16H7HVBT2P63W2FSWT5",
|
||||
"thread.preflight_lint.current_node": "implement",
|
||||
"internal.retry_count.implement": 1,
|
||||
"last_stage": "implement",
|
||||
"current.preamble": "Goal: # Fix: Workflow TOML config lost in detach mode\n\n## Context\n\nWhen running `fabro run -d implement-plan`, the `[pull_request]` config from `fabro.toml` / `cli.toml` / `workflow.toml` is silently dropped, so no PR is created despite `enabled = true` at all config levels.\n\n**Root cause chain**:\n1. `create.rs` tries to copy the workflow TOML as `run.toml`, but checks the **raw CLI arg** (`\"implement-plan\"` — no extension) instead of the resolved path. So `run.toml` is never saved.\n2. `RunEngine` always uses cached `graph.fabro` (a DOT file), so `prepare_workflow` returns `run_cfg = None`, losing all TOML-level config.\n3. `pull_request` and `asset_globs` in `RunConfig` only check `run_cfg` without falling back to `run_defaults`.\n\n## Fix 1: Serialize merged `run_cfg` to `run.toml` in create.rs\n\n**File**: `lib/crates/fabro-cli/src/commands/create.rs`\n\nReplace the raw-file-copy block (lines 53-58) with serialization of the already-merged `WorkflowRunConfig`:\n\n- Change `let prep = prepare_workflow(...)` to `let mut prep = ...`\n- Replace the extension check with:\n ```rust\n if let Some(mut cfg) = prep.run_cfg.take() {\n cfg.graph = \"graph.fabro\".to_string();\n let toml_str = toml::to_string_pretty(&cfg)\n .context(\"Failed to serialize run config\")?;\n tokio::fs::write(run_dir.join(\"run.toml\"), toml_str).await?;\n }\n ```\n- Add `use anyhow::Context;` if needed\n\n**Why serialize instead of copy**: The raw TOML's `graph` field (e.g. `\"workflow.fabro\"`) would point to a nonexistent file in the run dir. Serializing lets us rewrite `graph` to `\"graph.fabro\"` (the cached name). The serialized config also has all defaults merged, env vars resolved, and dockerfiles inlined — making the run dir self-contained.\n\n**Why `take()` not `clone()`**: `WorkflowRunConfig` doesn't derive `Clone`, and `prep.run_cfg` is unused after this point in `create.rs`.\n\n## Fix 2: Use `run.toml` in RunEngine path\n\n**File**: `lib/crates/fabro-cli/src/main.rs` (lines 727-733)\n\nReplace the workflow path resolution to use `run.toml`:\n\n```rust\nlet cached_toml = run_dir.join(\"run.toml\");\nlet workflow_path = if cached_toml.exists() {\n cached_toml\n} else {\n run_dir.join(\"graph.fabro\")\n};\n```\n\nWhen `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and `resolve_graph_path` resolves `\"graph.fabro\"` relative to the run dir — pointing to the cached graph that already exists there.\n\n## Fix 3: Add `run_defaults` fallbacks (defense-in-depth)\n\n**File**: `lib/crates/fabro-cli/src/commands/run.rs`\n\nEven with Fixes 1+2, bare `.fabro` files passed directly would still hit `run_cfg = None`. Add fallbacks matching the pattern already used elsewhere in the file:\n\n**3a. `pull_request`** (line 1424-1428):\n```rust\npull_request: run_cfg\n .as_ref()\n .and_then(|c| c.pull_request.as_ref())\n .or(run_defaults.pull_request.as_ref())\n .filter(|p| p.enabled)\n .cloned(),\n```\n\n**3b. `asset_globs`** (line 1429-1433):\n```rust\nasset_globs: run_cfg\n .as_ref()\n .and_then(|c| c.assets.as_ref())\n .or(run_defaults.assets.as_ref())\n .map(|a| a.include.clone())\n .unwrap_or_default(),\n```\n\n**3c. `devcontainer`** (line 969-973):\n```rust\nlet devcontainer_config = if run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.devcontainer)\n .unwrap_or(false)\n```\n\n**3d. `sandbox.env`** (line 1300-1306):\n```rust\nif let Some(toml_env) = run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.env.clone())\n```\n\n## Verification\n\n1. `cargo build --workspace`\n2. `cargo test --workspace`\n3. `cargo clippy --workspace -- -D warnings`\n4. Manual: `fabro run -d implement-plan` with `[pull_request] enabled = true` in `fabro.toml` → verify `run.toml` in run dir has `graph = \"graph.fabro\"` and `[pull_request]` → verify PR created\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",
|
||||
"graph.goal": "# Fix: Workflow TOML config lost in detach mode\n\n## Context\n\nWhen running `fabro run -d implement-plan`, the `[pull_request]` config from `fabro.toml` / `cli.toml` / `workflow.toml` is silently dropped, so no PR is created despite `enabled = true` at all config levels.\n\n**Root cause chain**:\n1. `create.rs` tries to copy the workflow TOML as `run.toml`, but checks the **raw CLI arg** (`\"implement-plan\"` — no extension) instead of the resolved path. So `run.toml` is never saved.\n2. `RunEngine` always uses cached `graph.fabro` (a DOT file), so `prepare_workflow` returns `run_cfg = None`, losing all TOML-level config.\n3. `pull_request` and `asset_globs` in `RunConfig` only check `run_cfg` without falling back to `run_defaults`.\n\n## Fix 1: Serialize merged `run_cfg` to `run.toml` in create.rs\n\n**File**: `lib/crates/fabro-cli/src/commands/create.rs`\n\nReplace the raw-file-copy block (lines 53-58) with serialization of the already-merged `WorkflowRunConfig`:\n\n- Change `let prep = prepare_workflow(...)` to `let mut prep = ...`\n- Replace the extension check with:\n ```rust\n if let Some(mut cfg) = prep.run_cfg.take() {\n cfg.graph = \"graph.fabro\".to_string();\n let toml_str = toml::to_string_pretty(&cfg)\n .context(\"Failed to serialize run config\")?;\n tokio::fs::write(run_dir.join(\"run.toml\"), toml_str).await?;\n }\n ```\n- Add `use anyhow::Context;` if needed\n\n**Why serialize instead of copy**: The raw TOML's `graph` field (e.g. `\"workflow.fabro\"`) would point to a nonexistent file in the run dir. Serializing lets us rewrite `graph` to `\"graph.fabro\"` (the cached name). The serialized config also has all defaults merged, env vars resolved, and dockerfiles inlined — making the run dir self-contained.\n\n**Why `take()` not `clone()`**: `WorkflowRunConfig` doesn't derive `Clone`, and `prep.run_cfg` is unused after this point in `create.rs`.\n\n## Fix 2: Use `run.toml` in RunEngine path\n\n**File**: `lib/crates/fabro-cli/src/main.rs` (lines 727-733)\n\nReplace the workflow path resolution to use `run.toml`:\n\n```rust\nlet cached_toml = run_dir.join(\"run.toml\");\nlet workflow_path = if cached_toml.exists() {\n cached_toml\n} else {\n run_dir.join(\"graph.fabro\")\n};\n```\n\nWhen `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and `resolve_graph_path` resolves `\"graph.fabro\"` relative to the run dir — pointing to the cached graph that already exists there.\n\n## Fix 3: Add `run_defaults` fallbacks (defense-in-depth)\n\n**File**: `lib/crates/fabro-cli/src/commands/run.rs`\n\nEven with Fixes 1+2, bare `.fabro` files passed directly would still hit `run_cfg = None`. Add fallbacks matching the pattern already used elsewhere in the file:\n\n**3a. `pull_request`** (line 1424-1428):\n```rust\npull_request: run_cfg\n .as_ref()\n .and_then(|c| c.pull_request.as_ref())\n .or(run_defaults.pull_request.as_ref())\n .filter(|p| p.enabled)\n .cloned(),\n```\n\n**3b. `asset_globs`** (line 1429-1433):\n```rust\nasset_globs: run_cfg\n .as_ref()\n .and_then(|c| c.assets.as_ref())\n .or(run_defaults.assets.as_ref())\n .map(|a| a.include.clone())\n .unwrap_or_default(),\n```\n\n**3c. `devcontainer`** (line 969-973):\n```rust\nlet devcontainer_config = if run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.devcontainer)\n .unwrap_or(false)\n```\n\n**3d. `sandbox.env`** (line 1300-1306):\n```rust\nif let Some(toml_env) = run_cfg\n .as_ref()\n .and_then(|c| c.sandbox.as_ref())\n .or(run_defaults.sandbox.as_ref())\n .and_then(|s| s.env.clone())\n```\n\n## Verification\n\n1. `cargo build --workspace`\n2. `cargo test --workspace`\n3. `cargo clippy --workspace -- -D warnings`\n4. Manual: `fabro run -d implement-plan` with `[pull_request] enabled = true` in `fabro.toml` → verify `run.toml` in run dir has `graph = \"graph.fabro\"` and `[pull_request]` → verify PR created\n",
|
||||
"last_stage": "simplify_opus",
|
||||
"graph.rankdir": "LR",
|
||||
"internal.fidelity": "compact",
|
||||
"internal.retry_count.toolchain": 1,
|
||||
"current_node": "implement",
|
||||
"thread.start.current_node": "toolchain",
|
||||
"failure_signature": "",
|
||||
"outcome": "success",
|
||||
"internal.node_visit_count": 1,
|
||||
"thread.toolchain.current_node": "preflight_compile",
|
||||
"internal.retry_count.preflight_compile": 1,
|
||||
"failure_class": "",
|
||||
"thread.preflight_compile.current_node": "preflight_lint",
|
||||
"internal.retry_count.preflight_lint": 1,
|
||||
"internal.thread_id": "preflight_lint",
|
||||
"last_response": "All four changes are in place. Let me summarize what was done:\n\n## Summary of changes\n\n### Fix 1: `lib/crates/fabro-cli/src/commands/create.rs`\n- Added `use anyhow::Context;`\n- Changed `let prep` to `",
|
||||
"internal.retry_count.start": 1,
|
||||
"current_node": "simplify_opus",
|
||||
"internal.retry_count.preflight_compile": 1,
|
||||
"failure_signature": "",
|
||||
"thread.toolchain.current_node": "preflight_compile",
|
||||
"internal.node_visit_count": 1,
|
||||
"thread.preflight_compile.current_node": "preflight_lint",
|
||||
"response.implement": "All four changes are in place. Let me summarize what was done:\n\n## Summary of changes\n\n### Fix 1: `lib/crates/fabro-cli/src/commands/create.rs`\n- Added `use anyhow::Context;`\n- Changed `let prep` to `let mut prep` to allow taking `run_cfg`\n- Replaced the raw-file-copy block (which checked the CLI arg's extension — always failing for bare workflow names like `\"implement-plan\"`) with serialization of the already-merged `WorkflowRunConfig`. The serialized config rewrites `graph` to `\"graph.fabro\"` (the cached name in the run dir), making the run directory self-contained.\n\n### Fix 2: `lib/crates/fabro-cli/src/main.rs` (RunEngine path)\n- Changed the workflow path resolution in the `RunEngine` command to prefer `run.toml` over `graph.fabro`. When `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and resolves `\"graph.fabro\"` relative to the run dir — restoring all TOML-level config (pull_request, assets, etc.).\n\n### Fix 3: `lib/crates/fabro-cli/src/commands/run.rs` (defense-in-depth fallbacks)\n- **3a. `pull_request`**: Added `.or(run_defaults.pull_request.as_ref())` fallback\n- **3b. `asset_globs`**: Added `.or(run_defaults.assets.as_ref())` fallback\n- **3c. `devcontainer`**: Added `.or(run_defaults.sandbox.as_ref())` fallback\n- **3d. `sandbox.env`**: Added `.or(run_defaults.sandbox.as_ref())` fallback\n\nAll changes compile cleanly, pass all `fabro-cli` tests, and pass clippy with `-D warnings`.",
|
||||
"command.stderr": "",
|
||||
"command.output": "",
|
||||
"graph.model_stylesheet": "\n * { model: claude-opus-4-6; }\n "
|
||||
"internal.retry_count.simplify_opus": 1,
|
||||
"internal.retry_count.start": 1,
|
||||
"thread.implement.current_node": "simplify_opus"
|
||||
},
|
||||
"logs": [],
|
||||
"node_outcomes": {
|
||||
|
|
@ -91,6 +96,28 @@
|
|||
],
|
||||
"duration_ms": 294549
|
||||
},
|
||||
"simplify_opus": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"response.simplify_opus": "## Summary\n\n**Fixed one real issue, accepted pre-existing snapshots, skipped false positives:**\n\n### Fixed\n1. **`run.rs` raw TOML copy → merged serialization** (the main fix): `run.rs` line 701-704 was still copying the raw TOML file from disk (the old buggy behavior), while `create.rs` had been fixed to serialize the merged config. This meant `fabro run` (non-detached) would write a `run.toml` with the wrong `graph` path and without defaults merged. Now both `create.rs` and `run.rs` use the same approach: serialize the merged `WorkflowRunConfig` with `graph` rewritten to `\"graph.fabro\"`.\n\n2. **Accepted pre-existing snapshot updates**: 7 `fabro-model` catalog test snapshots needed updating for the new `effort` field in `ModelFeatures`. Removed the stale `.pending-snap` file.\n\n### Skipped (not worth addressing)\n- **Magic string constants for `\"graph.fabro\"`/`\"run.toml\"`**: Valid observation (18+ occurrences across 7+ files), but a cross-crate refactor beyond the scope of this fix.\n- **Extract `resolve_sandbox_config` helper**: The two inline `.or()` additions match the existing code pattern. Extracting a helper for two call sites is over-abstraction.\n- **No efficiency issues**: All three changes are on cold paths with negligible cost.",
|
||||
"last_response": "## Summary\n\n**Fixed one real issue, accepted pre-existing snapshots, skipped false positives:**\n\n### Fixed\n1. **`run.rs` raw TOML copy → merged serialization** (the main fix): `run.rs` line 701-704 ",
|
||||
"last_stage": "simplify_opus"
|
||||
},
|
||||
"notes": "Stage completed: simplify_opus",
|
||||
"usage": {
|
||||
"model": "claude-opus-4-6",
|
||||
"input_tokens": 50738,
|
||||
"output_tokens": 13409,
|
||||
"cache_read_tokens": 1265424,
|
||||
"cache_write_tokens": 61115,
|
||||
"reasoning_tokens": 2081,
|
||||
"cost": 1.7667450000000002
|
||||
},
|
||||
"files_touched": [
|
||||
"/home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs"
|
||||
],
|
||||
"duration_ms": 521238
|
||||
},
|
||||
"preflight_lint": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
|
|
@ -101,12 +128,13 @@
|
|||
"duration_ms": 13456
|
||||
}
|
||||
},
|
||||
"next_node_id": "simplify_opus",
|
||||
"next_node_id": "simplify_gpt",
|
||||
"node_visits": {
|
||||
"start": 1,
|
||||
"preflight_lint": 1,
|
||||
"implement": 1,
|
||||
"toolchain": 1,
|
||||
"preflight_compile": 1
|
||||
"preflight_compile": 1,
|
||||
"simplify_opus": 1
|
||||
}
|
||||
}
|
||||
107
nodes/implement/diff.patch
Normal file
107
nodes/implement/diff.patch
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs
|
||||
index d9611dd9..41b2895c 100644
|
||||
--- a/lib/crates/fabro-cli/src/commands/create.rs
|
||||
+++ b/lib/crates/fabro-cli/src/commands/create.rs
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
+use anyhow::Context;
|
||||
use chrono::Local;
|
||||
use fabro_config::run::RunDefaults;
|
||||
use fabro_workflows::run_spec::RunSpec;
|
||||
@@ -21,7 +22,7 @@ pub async fn create_run(
|
||||
.as_ref()
|
||||
.ok_or_else(|| anyhow::anyhow!("--workflow is required"))?;
|
||||
|
||||
- let prep = prepare_workflow(args, run_defaults, styles, quiet)?;
|
||||
+ let mut prep = prepare_workflow(args, run_defaults, styles, quiet)?;
|
||||
|
||||
let goal = prep.graph.goal();
|
||||
|
||||
@@ -50,11 +51,11 @@ pub async fn create_run(
|
||||
None,
|
||||
);
|
||||
|
||||
- // Save TOML config alongside the run if present
|
||||
- if workflow_path.extension().is_some_and(|ext| ext == "toml") {
|
||||
- if let Ok(toml_contents) = tokio::fs::read(workflow_path).await {
|
||||
- tokio::fs::write(run_dir.join("run.toml"), toml_contents).await?;
|
||||
- }
|
||||
+ // Serialize the merged run config so the run dir is self-contained
|
||||
+ if let Some(mut cfg) = prep.run_cfg.take() {
|
||||
+ cfg.graph = "graph.fabro".to_string();
|
||||
+ let toml_str = toml::to_string_pretty(&cfg).context("Failed to serialize run config")?;
|
||||
+ tokio::fs::write(run_dir.join("run.toml"), toml_str).await?;
|
||||
}
|
||||
|
||||
// Build and save RunSpec
|
||||
diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs
|
||||
index 7faa494b..c5336799 100644
|
||||
--- a/lib/crates/fabro-cli/src/commands/run.rs
|
||||
+++ b/lib/crates/fabro-cli/src/commands/run.rs
|
||||
@@ -969,6 +969,7 @@ pub async fn run_command(
|
||||
let devcontainer_config = if run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.sandbox.as_ref())
|
||||
+ .or(run_defaults.sandbox.as_ref())
|
||||
.and_then(|s| s.devcontainer)
|
||||
.unwrap_or(false)
|
||||
{
|
||||
@@ -1300,6 +1301,7 @@ pub async fn run_command(
|
||||
if let Some(toml_env) = run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.sandbox.as_ref())
|
||||
+ .or(run_defaults.sandbox.as_ref())
|
||||
.and_then(|s| s.env.clone())
|
||||
{
|
||||
env.extend(toml_env);
|
||||
@@ -1424,11 +1426,13 @@ pub async fn run_command(
|
||||
pull_request: run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.pull_request.as_ref())
|
||||
+ .or(run_defaults.pull_request.as_ref())
|
||||
.filter(|p| p.enabled)
|
||||
.cloned(),
|
||||
asset_globs: run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.assets.as_ref())
|
||||
+ .or(run_defaults.assets.as_ref())
|
||||
.map(|a| a.include.clone())
|
||||
.unwrap_or_default(),
|
||||
workflow_slug: workflow_slug.clone(),
|
||||
diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs
|
||||
index a1f13478..ad9bb158 100644
|
||||
--- a/lib/crates/fabro-cli/src/main.rs
|
||||
+++ b/lib/crates/fabro-cli/src/main.rs
|
||||
@@ -724,12 +724,13 @@ async fn main_inner() -> (String, Result<()>) {
|
||||
)
|
||||
})?;
|
||||
|
||||
- // Use the cached graph snapshot instead of the original file
|
||||
- let cached_graph = run_dir.join("graph.fabro");
|
||||
- let workflow_path = if cached_graph.exists() {
|
||||
- cached_graph
|
||||
+ // Prefer cached run.toml (carries full TOML config including
|
||||
+ // pull_request, assets, etc.), fall back to bare graph snapshot.
|
||||
+ let cached_toml = run_dir.join("run.toml");
|
||||
+ let workflow_path = if cached_toml.exists() {
|
||||
+ cached_toml
|
||||
} else {
|
||||
- spec.workflow_path
|
||||
+ run_dir.join("graph.fabro")
|
||||
};
|
||||
|
||||
let run_args = commands::run::RunArgs {
|
||||
diff --git a/lib/crates/fabro-model/src/.catalog.rs.pending-snap b/lib/crates/fabro-model/src/.catalog.rs.pending-snap
|
||||
new file mode 100644
|
||||
index 00000000..c30cbf5d
|
||||
--- /dev/null
|
||||
+++ b/lib/crates/fabro-model/src/.catalog.rs.pending-snap
|
||||
@@ -0,0 +1,7 @@
|
||||
+{"run_id":"1774108419-651283619","line":604,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"gpt_5_3_codex_spark_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":604,"expression":"m"},"snapshot":"ModelInfo {\n id: \"gpt-5.3-codex-spark\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.3 Codex Spark\",\n limits: ModelLimits {\n context_window: 131072,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: false,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: None,\n output_cost_per_mtok: None,\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 1000.0,\n ),\n aliases: [\n \"codex-spark\",\n ],\n default: false,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"gpt-5.3-codex-spark\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.3 Codex Spark\",\n limits: ModelLimits {\n context_window: 131072,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: false,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: None,\n output_cost_per_mtok: None,\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 1000.0,\n ),\n aliases: [\n \"codex-spark\",\n ],\n default: false,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":333,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"gemini_3_1_flash_lite_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":333,"expression":"m"},"snapshot":"ModelInfo {\n id: \"gemini-3.1-flash-lite-preview\",\n provider: \"gemini\",\n family: \"gemini-3\",\n display_name: \"Gemini 3.1 Flash Lite (Preview)\",\n limits: ModelLimits {\n context_window: 1048576,\n max_output: Some(\n 65536,\n ),\n },\n training: Some(\n \"2025-01-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.25,\n ),\n output_cost_per_mtok: Some(\n 1.5,\n ),\n cache_input_cost_per_mtok: Some(\n 0.0625,\n ),\n },\n estimated_output_tps: Some(\n 200.0,\n ),\n aliases: [\n \"gemini-flash-lite\",\n ],\n default: false,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"gemini-3.1-flash-lite-preview\",\n provider: \"gemini\",\n family: \"gemini-3\",\n display_name: \"Gemini 3.1 Flash Lite (Preview)\",\n limits: ModelLimits {\n context_window: 1048576,\n max_output: Some(\n 65536,\n ),\n },\n training: Some(\n \"2025-01-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.25,\n ),\n output_cost_per_mtok: Some(\n 1.5,\n ),\n cache_input_cost_per_mtok: Some(\n 0.0625,\n ),\n },\n estimated_output_tps: Some(\n 200.0,\n ),\n aliases: [\n \"gemini-flash-lite\",\n ],\n default: false,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":254,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"get_model_info_by_id","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":254,"expression":"info"},"snapshot":"ModelInfo {\n id: \"claude-opus-4-6\",\n provider: \"anthropic\",\n family: \"claude-4\",\n display_name: \"Claude Opus 4.6\",\n limits: ModelLimits {\n context_window: 1000000,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 15.0,\n ),\n output_cost_per_mtok: Some(\n 75.0,\n ),\n cache_input_cost_per_mtok: Some(\n 1.5,\n ),\n },\n estimated_output_tps: Some(\n 25.0,\n ),\n aliases: [\n \"opus\",\n \"claude-opus\",\n ],\n default: false,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"claude-opus-4-6\",\n provider: \"anthropic\",\n family: \"claude-4\",\n display_name: \"Claude Opus 4.6\",\n limits: ModelLimits {\n context_window: 1000000,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 15.0,\n ),\n output_cost_per_mtok: Some(\n 75.0,\n ),\n cache_input_cost_per_mtok: Some(\n 1.5,\n ),\n },\n estimated_output_tps: Some(\n 25.0,\n ),\n aliases: [\n \"opus\",\n \"claude-opus\",\n ],\n default: false,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":492,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"gpt_5_4_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":492,"expression":"m"},"snapshot":"ModelInfo {\n id: \"gpt-5.4\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.4\",\n limits: ModelLimits {\n context_window: 1047576,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 2.5,\n ),\n output_cost_per_mtok: Some(\n 15.0,\n ),\n cache_input_cost_per_mtok: Some(\n 0.25,\n ),\n },\n estimated_output_tps: Some(\n 70.0,\n ),\n aliases: [\n \"gpt54\",\n \"gpt-54\",\n ],\n default: true,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"gpt-5.4\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.4\",\n limits: ModelLimits {\n context_window: 1047576,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 2.5,\n ),\n output_cost_per_mtok: Some(\n 15.0,\n ),\n cache_input_cost_per_mtok: Some(\n 0.25,\n ),\n },\n estimated_output_tps: Some(\n 70.0,\n ),\n aliases: [\n \"gpt54\",\n \"gpt-54\",\n ],\n default: true,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":538,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"gpt_5_4_pro_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":538,"expression":"m"},"snapshot":"ModelInfo {\n id: \"gpt-5.4-pro\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.4 Pro\",\n limits: ModelLimits {\n context_window: 1047576,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 30.0,\n ),\n output_cost_per_mtok: Some(\n 180.0,\n ),\n cache_input_cost_per_mtok: Some(\n 3.0,\n ),\n },\n estimated_output_tps: Some(\n 20.0,\n ),\n aliases: [\n \"gpt54-pro\",\n \"gpt-54-pro\",\n ],\n default: false,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"gpt-5.4-pro\",\n provider: \"openai\",\n family: \"gpt-5\",\n display_name: \"GPT-5.4 Pro\",\n limits: ModelLimits {\n context_window: 1047576,\n max_output: Some(\n 128000,\n ),\n },\n training: Some(\n \"2025-08-31\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 30.0,\n ),\n output_cost_per_mtok: Some(\n 180.0,\n ),\n cache_input_cost_per_mtok: Some(\n 3.0,\n ),\n },\n estimated_output_tps: Some(\n 20.0,\n ),\n aliases: [\n \"gpt54-pro\",\n \"gpt-54-pro\",\n ],\n default: false,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":446,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"mercury_2_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":446,"expression":"m"},"snapshot":"ModelInfo {\n id: \"mercury-2\",\n provider: \"inception\",\n family: \"mercury\",\n display_name: \"Mercury 2\",\n limits: ModelLimits {\n context_window: 131072,\n max_output: Some(\n 50000,\n ),\n },\n training: None,\n features: ModelFeatures {\n tools: true,\n vision: false,\n reasoning: true,\n effort: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.25,\n ),\n output_cost_per_mtok: Some(\n 0.75,\n ),\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 1000.0,\n ),\n aliases: [\n \"mercury\",\n ],\n default: true,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"mercury-2\",\n provider: \"inception\",\n family: \"mercury\",\n display_name: \"Mercury 2\",\n limits: ModelLimits {\n context_window: 131072,\n max_output: Some(\n 50000,\n ),\n },\n training: None,\n features: ModelFeatures {\n tools: true,\n vision: false,\n reasoning: true,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.25,\n ),\n output_cost_per_mtok: Some(\n 0.75,\n ),\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 1000.0,\n ),\n aliases: [\n \"mercury\",\n ],\n default: true,\n}"}}
|
||||
+{"run_id":"1774108419-651283619","line":386,"new":{"module_name":"fabro_model__catalog__tests","snapshot_name":"kimi_k2_5_in_catalog","metadata":{"source":"lib/crates/fabro-model/src/catalog.rs","assertion_line":386,"expression":"m"},"snapshot":"ModelInfo {\n id: \"kimi-k2.5\",\n provider: \"kimi\",\n family: \"kimi-k2\",\n display_name: \"Kimi K2.5\",\n limits: ModelLimits {\n context_window: 262144,\n max_output: Some(\n 16000,\n ),\n },\n training: Some(\n \"2025-10-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: false,\n effort: false,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.6,\n ),\n output_cost_per_mtok: Some(\n 3.0,\n ),\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 50.0,\n ),\n aliases: [\n \"kimi\",\n ],\n default: true,\n}"},"old":{"module_name":"fabro_model__catalog__tests","metadata":{},"snapshot":"ModelInfo {\n id: \"kimi-k2.5\",\n provider: \"kimi\",\n family: \"kimi-k2\",\n display_name: \"Kimi K2.5\",\n limits: ModelLimits {\n context_window: 262144,\n max_output: Some(\n 16000,\n ),\n },\n training: Some(\n \"2025-10-01\",\n ),\n features: ModelFeatures {\n tools: true,\n vision: true,\n reasoning: false,\n },\n costs: ModelCosts {\n input_cost_per_mtok: Some(\n 0.6,\n ),\n output_cost_per_mtok: Some(\n 3.0,\n ),\n cache_input_cost_per_mtok: None,\n },\n estimated_output_tps: Some(\n 50.0,\n ),\n aliases: [\n \"kimi\",\n ],\n default: true,\n}"}}
|
||||
174
nodes/simplify_opus/prompt.md
Normal file
174
nodes/simplify_opus/prompt.md
Normal file
|
|
@ -0,0 +1,174 @@
|
|||
Goal: # Fix: Workflow TOML config lost in detach mode
|
||||
|
||||
## Context
|
||||
|
||||
When running `fabro run -d implement-plan`, the `[pull_request]` config from `fabro.toml` / `cli.toml` / `workflow.toml` is silently dropped, so no PR is created despite `enabled = true` at all config levels.
|
||||
|
||||
**Root cause chain**:
|
||||
1. `create.rs` tries to copy the workflow TOML as `run.toml`, but checks the **raw CLI arg** (`"implement-plan"` — no extension) instead of the resolved path. So `run.toml` is never saved.
|
||||
2. `RunEngine` always uses cached `graph.fabro` (a DOT file), so `prepare_workflow` returns `run_cfg = None`, losing all TOML-level config.
|
||||
3. `pull_request` and `asset_globs` in `RunConfig` only check `run_cfg` without falling back to `run_defaults`.
|
||||
|
||||
## Fix 1: Serialize merged `run_cfg` to `run.toml` in create.rs
|
||||
|
||||
**File**: `lib/crates/fabro-cli/src/commands/create.rs`
|
||||
|
||||
Replace the raw-file-copy block (lines 53-58) with serialization of the already-merged `WorkflowRunConfig`:
|
||||
|
||||
- Change `let prep = prepare_workflow(...)` to `let mut prep = ...`
|
||||
- Replace the extension check with:
|
||||
```rust
|
||||
if let Some(mut cfg) = prep.run_cfg.take() {
|
||||
cfg.graph = "graph.fabro".to_string();
|
||||
let toml_str = toml::to_string_pretty(&cfg)
|
||||
.context("Failed to serialize run config")?;
|
||||
tokio::fs::write(run_dir.join("run.toml"), toml_str).await?;
|
||||
}
|
||||
```
|
||||
- Add `use anyhow::Context;` if needed
|
||||
|
||||
**Why serialize instead of copy**: The raw TOML's `graph` field (e.g. `"workflow.fabro"`) would point to a nonexistent file in the run dir. Serializing lets us rewrite `graph` to `"graph.fabro"` (the cached name). The serialized config also has all defaults merged, env vars resolved, and dockerfiles inlined — making the run dir self-contained.
|
||||
|
||||
**Why `take()` not `clone()`**: `WorkflowRunConfig` doesn't derive `Clone`, and `prep.run_cfg` is unused after this point in `create.rs`.
|
||||
|
||||
## Fix 2: Use `run.toml` in RunEngine path
|
||||
|
||||
**File**: `lib/crates/fabro-cli/src/main.rs` (lines 727-733)
|
||||
|
||||
Replace the workflow path resolution to use `run.toml`:
|
||||
|
||||
```rust
|
||||
let cached_toml = run_dir.join("run.toml");
|
||||
let workflow_path = if cached_toml.exists() {
|
||||
cached_toml
|
||||
} else {
|
||||
run_dir.join("graph.fabro")
|
||||
};
|
||||
```
|
||||
|
||||
When `run.toml` exists, `prepare_workflow` → `resolve_workflow` sees `.toml`, calls `load_run_config`, and `resolve_graph_path` resolves `"graph.fabro"` relative to the run dir — pointing to the cached graph that already exists there.
|
||||
|
||||
## Fix 3: Add `run_defaults` fallbacks (defense-in-depth)
|
||||
|
||||
**File**: `lib/crates/fabro-cli/src/commands/run.rs`
|
||||
|
||||
Even with Fixes 1+2, bare `.fabro` files passed directly would still hit `run_cfg = None`. Add fallbacks matching the pattern already used elsewhere in the file:
|
||||
|
||||
**3a. `pull_request`** (line 1424-1428):
|
||||
```rust
|
||||
pull_request: run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.pull_request.as_ref())
|
||||
.or(run_defaults.pull_request.as_ref())
|
||||
.filter(|p| p.enabled)
|
||||
.cloned(),
|
||||
```
|
||||
|
||||
**3b. `asset_globs`** (line 1429-1433):
|
||||
```rust
|
||||
asset_globs: run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.assets.as_ref())
|
||||
.or(run_defaults.assets.as_ref())
|
||||
.map(|a| a.include.clone())
|
||||
.unwrap_or_default(),
|
||||
```
|
||||
|
||||
**3c. `devcontainer`** (line 969-973):
|
||||
```rust
|
||||
let devcontainer_config = if run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.sandbox.as_ref())
|
||||
.or(run_defaults.sandbox.as_ref())
|
||||
.and_then(|s| s.devcontainer)
|
||||
.unwrap_or(false)
|
||||
```
|
||||
|
||||
**3d. `sandbox.env`** (line 1300-1306):
|
||||
```rust
|
||||
if let Some(toml_env) = run_cfg
|
||||
.as_ref()
|
||||
.and_then(|c| c.sandbox.as_ref())
|
||||
.or(run_defaults.sandbox.as_ref())
|
||||
.and_then(|s| s.env.clone())
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
1. `cargo build --workspace`
|
||||
2. `cargo test --workspace`
|
||||
3. `cargo clippy --workspace -- -D warnings`
|
||||
4. Manual: `fabro run -d implement-plan` with `[pull_request] enabled = true` in `fabro.toml` → verify `run.toml` in run dir has `graph = "graph.fabro"` and `[pull_request]` → verify PR created
|
||||
|
||||
|
||||
## Completed stages
|
||||
- **toolchain**: success
|
||||
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
|
||||
- Stdout:
|
||||
```
|
||||
cargo 1.94.0 (85eff7c80 2026-01-15)
|
||||
```
|
||||
- Stderr: (empty)
|
||||
- **preflight_compile**: success
|
||||
- Script: `cargo check -q --workspace 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **preflight_lint**: success
|
||||
- Script: `cargo clippy -q --workspace -- -D warnings 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **implement**: success
|
||||
- Model: claude-opus-4-6, 20.7k tokens in / 5.3k out
|
||||
- Files: /home/daytona/workspace/lib/crates/fabro-cli/src/commands/create.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs, /home/daytona/workspace/lib/crates/fabro-cli/src/main.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).
|
||||
5
nodes/simplify_opus/provider_used.json
Normal file
5
nodes/simplify_opus/provider_used.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"mode": "agent",
|
||||
"provider": "anthropic",
|
||||
"model": "claude-opus-4-6"
|
||||
}
|
||||
13
nodes/simplify_opus/response.md
Normal file
13
nodes/simplify_opus/response.md
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
## Summary
|
||||
|
||||
**Fixed one real issue, accepted pre-existing snapshots, skipped false positives:**
|
||||
|
||||
### Fixed
|
||||
1. **`run.rs` raw TOML copy → merged serialization** (the main fix): `run.rs` line 701-704 was still copying the raw TOML file from disk (the old buggy behavior), while `create.rs` had been fixed to serialize the merged config. This meant `fabro run` (non-detached) would write a `run.toml` with the wrong `graph` path and without defaults merged. Now both `create.rs` and `run.rs` use the same approach: serialize the merged `WorkflowRunConfig` with `graph` rewritten to `"graph.fabro"`.
|
||||
|
||||
2. **Accepted pre-existing snapshot updates**: 7 `fabro-model` catalog test snapshots needed updating for the new `effort` field in `ModelFeatures`. Removed the stale `.pending-snap` file.
|
||||
|
||||
### Skipped (not worth addressing)
|
||||
- **Magic string constants for `"graph.fabro"`/`"run.toml"`**: Valid observation (18+ occurrences across 7+ files), but a cross-crate refactor beyond the scope of this fix.
|
||||
- **Extract `resolve_sandbox_config` helper**: The two inline `.or()` additions match the existing code pattern. Extracting a helper for two call sites is over-abstraction.
|
||||
- **No efficiency issues**: All three changes are on cold paths with negligible cost.
|
||||
6
nodes/simplify_opus/status.json
Normal file
6
nodes/simplify_opus/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Stage completed: simplify_opus",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-21T16:03:23.635566+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue