mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
parent
03b2c24050
commit
ac141aa71a
4 changed files with 38 additions and 8 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"timestamp": "2026-03-20T01:15:17.539471Z",
|
||||
"current_node": "verify",
|
||||
"timestamp": "2026-03-20T01:15:21.583607Z",
|
||||
"current_node": "fmt",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"toolchain",
|
||||
|
|
@ -9,12 +9,14 @@
|
|||
"implement",
|
||||
"simplify_opus",
|
||||
"simplify_gpt",
|
||||
"verify"
|
||||
"verify",
|
||||
"fmt"
|
||||
],
|
||||
"node_retries": {
|
||||
"verify": 1,
|
||||
"implement": 1,
|
||||
"preflight_lint": 1,
|
||||
"fmt": 1,
|
||||
"preflight_compile": 1,
|
||||
"simplify_gpt": 1,
|
||||
"start": 1,
|
||||
|
|
@ -24,21 +26,23 @@
|
|||
"context_values": {
|
||||
"response.implement": "Both changes look correct:\n\n1. **Inside `execute_with_retry`** (line 1080): `StageStarted` is now emitted at the top of the `for attempt in 1..=policy.max_attempts` loop, using the loop variable `attempt` for the correct attempt number.\n\n2. **In `run_inner`** (line 1862): The old `StageStarted` emission that was before the loop has been removed. The `StageStart` hook remains outside the loop as specified.\n\nLet me do a final summary of the changes:\n\n## Summary\n\n**Problem:** When a stage fails with a transient error and is retried, the CLI progress UI freezes because `StageFailed` removes the stage from `active_stages`, but no `StageStarted` event is emitted for the retry attempt, so subsequent agent events silently drop.\n\n**Fix:** Two edits in `lib/crates/fabro-workflows/src/engine.rs`:\n\n1. **Added `StageStarted` emission inside the retry loop** (in `execute_with_retry`): Moved the event emission to the top of the `for attempt in 1..=policy.max_attempts` loop so every attempt — including retries — emits `StageStarted` with the correct `attempt` number.\n\n2. **Removed the old `StageStarted` emission before the loop** (in `run_inner`): The pre-loop emission with hardcoded `attempt: 1` was removed since it's now handled inside the retry loop. The `StageStart` hook remains outside the loop (hooks shouldn't re-fire on retries).\n\n**New test:** `retry_emits_stage_started_per_attempt` — Creates a handler (`FailOnceThenSucceedHandler`) that returns a retryable error on the first call and succeeds on the second, then verifies that `StageStarted` is emitted twice with `attempt: 1` and `attempt: 2`.\n\n**Verification:**\n- ✅ `cargo test -p fabro-workflows --lib` — all 867 tests pass\n- ✅ `cargo clippy --workspace -- -D warnings` — no warnings",
|
||||
"thread.implement.current_node": "simplify_opus",
|
||||
"internal.thread_id": "simplify_gpt",
|
||||
"internal.thread_id": "verify",
|
||||
"internal.node_visit_count": 1,
|
||||
"outcome": "success",
|
||||
"command.output": "warning: function `init_repo_with_remote` is never used\n --> lib/crates/fabro-workflows/src/git.rs:1153:8\n |\n1153 | fn init_repo_with_remote(dir: &Path) -> (std::path::PathBuf, std::path::PathBuf) {\n | ^^^^^^^^^^^^^^^^^^^^^\n |\n = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default\n\n────────────\n Nextest run ID 6b6eeac2-e45e-43f2-bb54-3e71487b1986 with nextest profile: default\n Starting 3214 tests across 41 binaries (177 tests skipped)\n────────────\n Summary [ 10.679s] 3214 tests run: 3214 passed, 177 skipped\n",
|
||||
"command.output": "",
|
||||
"thread.simplify_opus.current_node": "simplify_gpt",
|
||||
"internal.retry_count.start": 1,
|
||||
"thread.preflight_compile.current_node": "preflight_lint",
|
||||
"graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ",
|
||||
"thread.toolchain.current_node": "preflight_compile",
|
||||
"current.preamble": "Goal: # Emit `StageStarted` on retry attempts\n\n## Context\n\nWhen a stage fails with a transient error and is retried, the CLI progress UI freezes because:\n\n1. `StageFailed` calls `finish_stage()`, removing the stage from `active_stages`\n2. The retry loop in the engine (`continue` at line 1198) re-enters handler execution **without emitting `StageStarted`**\n3. All subsequent agent events for the retry attempt silently drop (no matching entry in `active_stages`)\n\nThe `StageStarted` event already has `attempt` and `max_attempts` fields, so emitting it per-attempt is the intended design — it just wasn't wired up.\n\n## Changes\n\n### 1. Engine: emit `StageStarted` at the top of the retry loop\n\n**File:** `lib/crates/fabro-workflows/src/engine.rs`\n\nMove the `StageStarted` emission from before the loop (line 1852) to inside the loop, right after `for attempt in 1..=policy.max_attempts {` (line 1079). This way every attempt — including retries — emits the event with the correct `attempt` number.\n\nThe existing emission at line 1852 gets replaced, not duplicated. The `attempt` value comes directly from the loop variable (converted via `usize::try_from`).\n\n### 2. Engine: move StageStart hook inside the loop (or keep it outside)\n\nThe `StageStart` hook block (lines 1862-1895) currently runs once before the loop. It should stay outside — hooks shouldn't re-fire on retries. Only the `StageStarted` event emission moves inside.\n\n### 3. UI: no changes needed\n\n`on_stage_started` in `run_progress.rs` already handles being called for the same `node_id` — it inserts a fresh `ActiveStage` into the map, creating a new spinner. The `StageFailed` handler correctly finishes the old spinner. The natural event sequence becomes:\n\n```\nStageStarted (attempt 1) → spinner created\nStageFailed (will_retry) → spinner finished with ✗\nStageStarted (attempt 2) → new spinner created\nAgent events → attach to new spinner\nStageCompleted (attempt 2) → spinner finished with ✓\n```\n\n## Verification\n\n1. `cargo test -p fabro-workflows` — existing tests pass\n2. `cargo clippy --workspace -- -D warnings` — no warnings\n3. Manual: run a workflow that hits a transient LLM error (or mock one) and verify the CLI shows the retry spinner with tool calls\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, 42.2k tokens in / 12.1k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 27.5k tokens in / 10.9k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_gpt**: fail\n\n## Context\n- failure_class: deterministic\n- failure_signature: simplify_gpt|deterministic|api_deterministic|anthropic|not_found\n",
|
||||
"current.preamble": "Goal: # Emit `StageStarted` on retry attempts\n\n## Context\n\nWhen a stage fails with a transient error and is retried, the CLI progress UI freezes because:\n\n1. `StageFailed` calls `finish_stage()`, removing the stage from `active_stages`\n2. The retry loop in the engine (`continue` at line 1198) re-enters handler execution **without emitting `StageStarted`**\n3. All subsequent agent events for the retry attempt silently drop (no matching entry in `active_stages`)\n\nThe `StageStarted` event already has `attempt` and `max_attempts` fields, so emitting it per-attempt is the intended design — it just wasn't wired up.\n\n## Changes\n\n### 1. Engine: emit `StageStarted` at the top of the retry loop\n\n**File:** `lib/crates/fabro-workflows/src/engine.rs`\n\nMove the `StageStarted` emission from before the loop (line 1852) to inside the loop, right after `for attempt in 1..=policy.max_attempts {` (line 1079). This way every attempt — including retries — emits the event with the correct `attempt` number.\n\nThe existing emission at line 1852 gets replaced, not duplicated. The `attempt` value comes directly from the loop variable (converted via `usize::try_from`).\n\n### 2. Engine: move StageStart hook inside the loop (or keep it outside)\n\nThe `StageStart` hook block (lines 1862-1895) currently runs once before the loop. It should stay outside — hooks shouldn't re-fire on retries. Only the `StageStarted` event emission moves inside.\n\n### 3. UI: no changes needed\n\n`on_stage_started` in `run_progress.rs` already handles being called for the same `node_id` — it inserts a fresh `ActiveStage` into the map, creating a new spinner. The `StageFailed` handler correctly finishes the old spinner. The natural event sequence becomes:\n\n```\nStageStarted (attempt 1) → spinner created\nStageFailed (will_retry) → spinner finished with ✗\nStageStarted (attempt 2) → new spinner created\nAgent events → attach to new spinner\nStageCompleted (attempt 2) → spinner finished with ✓\n```\n\n## Verification\n\n1. `cargo test -p fabro-workflows` — existing tests pass\n2. `cargo clippy --workspace -- -D warnings` — no warnings\n3. Manual: run a workflow that hits a transient LLM error (or mock one) and verify the CLI shows the retry spinner with tool calls\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, 42.2k tokens in / 12.1k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 27.5k tokens in / 10.9k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/engine.rs\n- **simplify_gpt**: fail\n- **verify**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1`\n - Stdout:\n ```\n warning: function `init_repo_with_remote` is never used\n --> lib/crates/fabro-workflows/src/git.rs:1153:8\n |\n 1153 | fn init_repo_with_remote(dir: &Path) -> (std::path::PathBuf, std::path::PathBuf) {\n | ^^^^^^^^^^^^^^^^^^^^^\n |\n = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default\n \n ────────────\n Nextest run ID 6b6eeac2-e45e-43f2-bb54-3e71487b1986 with nextest profile: default\n Starting 3214 tests across 41 binaries (177 tests skipped)\n ────────────\n Summary [ 10.679s] 3214 tests run: 3214 passed, 177 skipped\n ```\n - Stderr: (empty)\n",
|
||||
"thread.verify.current_node": "fmt",
|
||||
"command.stderr": "",
|
||||
"internal.retry_count.simplify_opus": 1,
|
||||
"graph.rankdir": "LR",
|
||||
"thread.preflight_lint.current_node": "implement",
|
||||
"internal.fidelity": "compact",
|
||||
"internal.retry_count.fmt": 1,
|
||||
"failure_signature": "",
|
||||
"thread.simplify_gpt.current_node": "verify",
|
||||
"internal.retry_count.implement": 1,
|
||||
|
|
@ -53,7 +57,7 @@
|
|||
"thread.start.current_node": "toolchain",
|
||||
"internal.retry_count.preflight_lint": 1,
|
||||
"last_stage": "simplify_opus",
|
||||
"current_node": "verify",
|
||||
"current_node": "fmt",
|
||||
"internal.retry_count.preflight_compile": 1
|
||||
},
|
||||
"logs": [],
|
||||
|
|
@ -142,6 +146,15 @@
|
|||
"notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1",
|
||||
"duration_ms": 70931
|
||||
},
|
||||
"fmt": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"command.output": "",
|
||||
"command.stderr": ""
|
||||
},
|
||||
"notes": "Script completed: cargo fmt --all 2>&1",
|
||||
"duration_ms": 980
|
||||
},
|
||||
"preflight_lint": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
|
|
@ -152,7 +165,7 @@
|
|||
"duration_ms": 10490
|
||||
}
|
||||
},
|
||||
"next_node_id": "fmt",
|
||||
"next_node_id": "exit",
|
||||
"loop_failure_signatures": {
|
||||
"simplify_gpt|deterministic|api_deterministic|anthropic|not_found": 1
|
||||
},
|
||||
|
|
@ -164,6 +177,7 @@
|
|||
"preflight_compile": 1,
|
||||
"simplify_opus": 1,
|
||||
"implement": 1,
|
||||
"fmt": 1,
|
||||
"simplify_gpt": 1
|
||||
}
|
||||
}
|
||||
5
nodes/fmt/script_invocation.json
Normal file
5
nodes/fmt/script_invocation.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"command": "cargo fmt --all 2>&1",
|
||||
"language": "shell",
|
||||
"timeout_ms": null
|
||||
}
|
||||
5
nodes/fmt/script_timing.json
Normal file
5
nodes/fmt/script_timing.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"duration_ms": 978,
|
||||
"exit_code": 0,
|
||||
"timed_out": false
|
||||
}
|
||||
6
nodes/fmt/status.json
Normal file
6
nodes/fmt/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Script completed: cargo fmt --all 2>&1",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-20T01:15:21.583136+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue