scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix: Prefer root stage TODO projection over child OpenAI plan events (#384)

Browse source 
## Summary

The stage insights sidebar could show a stale child TODO list (e.g.
`0/3`) even after the root agent plan completed, because child OpenAI
subagent sessions emit their own `todo.created` events on the same
`stage_id`, replacing the root projection and causing later root
`todo.updated` completions to be ignored.

## What changed and why

OpenAI `update_plan` lists are scoped per agent session as
`openai_plan:<session_id>`. A stage can have both a root session and
child/subagent sessions, but `StageProjection.todos` is a single slot —
whichever session emitted `TodoCreated` last won. The stage sidebar is a
root-agent summary, so the projection should always reflect the root
session's list.

**The fix** adds a `should_project_stage_todo_event` guard before each
of the `TodoCreated`, `TodoUpdated`, and `TodoDeleted` match arms in
`RunProjectionReducer`. It drops an event when `list_kind == OpenAiPlan`
and `stored.parent_session_id.is_some()` (i.e. the event came from a
child session). Root OpenAI events and all Anthropic task events pass
through unchanged.

This is intentionally the minimal change: no schema additions, no
frontend changes, no multi-list projection. Historical event logs replay
correctly because `parent_session_id` is already persisted on child
event envelopes.

### Plan Summary

- Add `should_project_stage_todo_event(stored, list_kind) -> bool`
helper in `run_state.rs`
- Gate all three TODO event arms on this helper
- Add three regression tests in the existing `todo_reducer` module: root
list survives child plan events, child events alone do not create a
projection, Anthropic child-session events still project

### Bundled in this diff (unrelated to the core fix)

- Removed retired OpenAI models `gpt-5-mini` and `gpt-5.2-codex` from
catalog, docs, and tests
- Added `2026-05-23` and `2026-05-24` changelog entries; backfilled
`2026-05-22` items
- `EventExportActions` in `run-stages.tsx` now only renders on the
`debug` tab
- `listLifecycleStatusLabel` suppresses the lifecycle label for
`initializing` runs
- `Provider` gains an `expected_secret_name` optional field; round-trip
tests updated
- CLI snapshot: cost column removed from stage completion display


### Fabro Details

<details>
<summary>Ran 10 stages in 37m 7s for $9.70</summary>

| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 1s | – | 0 |
| preflight_compile | 2m 4s | – | 0 |
| preflight_lint | 1m 58s | – | 0 |
| fix_lints | 1m 29s | $0.32 | 0 |
| implement | 3m 27s | $3.01 | 0 |
| simplify_opus | 3m 15s | $0.80 | 0 |
| simplify_gpt | 2m 3s | $1.97 | 0 |
| verify | 14m 20s | – | 0 |
| fixup | 7m 35s | $3.61 | 0 |
| **Total** | **37m 7s** | **$9.70** | **0** |

</details>

<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>

```dot
digraph ImplementPlan {
    graph [
        goal="Implement and simplify",
        model_stylesheet="
            * { model: claude-opus-4-7; }
        "
    ]
    rankdir=LR

    start [shape=Mdiamond, label="Start"]
    exit  [shape=Msquare, label="Exit"]

    toolchain         [label="Toolchain", shape=parallelogram, 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", max_retries=0]
    preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
    preflight_lint    [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0]
    fix_lints         [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
    implement         [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"]
    simplify_opus     [label="Simplify (Opus)", prompt="@prompts/simplify.md"]
    simplify_gpt      [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"]
    verify            [label="Verify", shape=parallelogram, script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"]
    fixup             [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3]

    start -> toolchain
    toolchain -> preflight_compile [condition="outcome=succeeded"]
    toolchain -> exit
    preflight_compile -> preflight_lint [condition="outcome=succeeded"]
    preflight_compile -> exit
    preflight_lint -> implement [condition="outcome=succeeded"]
    preflight_lint -> fix_lints
    fix_lints -> preflight_lint
    implement -> simplify_opus -> simplify_gpt -> verify
    verify -> exit  [condition="outcome=succeeded"]
    verify -> fixup
    fixup -> verify
}

```

</details>

⚒️ Generated with [Fabro](https://fabro.sh)

---------

Co-authored-by: Fabro <noreply@fabro.sh>
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
This commit is contained in:
fabro-sh-0530[bot] 2026-05-24 12:30:41 -04:00 • committed by GitHub
parent c78f218154
commit 61e26a6bff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 188 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

189
lib/crates/fabro-store/src/run_state.rs
View file

@ -15,7 +15,7 @@ use fabro_types::{
RunControlAction, RunDiff, RunEvent, RunId, RunLifecycle, RunLinks, RunModel, RunOrigin,
RunProjection, RunSandbox, RunSandboxRuntime, RunSize, RunSpec, RunStatus, RunTimestamps,
SandboxProvider, StageCompletion, StageHandler, StageId, StageModelUsage, StageOutcome,
StageProjection, StageState, StartRecord, SubAgentProjection, SubAgentStatus,
StageProjection, StageState, StartRecord, SubAgentProjection, SubAgentStatus, TodoListKind,
TodoListProjection, TodoProjection, WorkflowRef, first_event_seq,
};
use fabro_util::error::render_compact_with_causes;
@ -485,18 +485,27 @@ impl RunProjectionReducer for RunProjection {
stage.parallel_results = Some(parallel_results);
}
EventBody::TodoCreated(props) => {
if !should_project_stage_todo_event(stored, props.list_kind) {
return Ok(());
}
let Some(stage) = stage_at_stored_or_current_visit(self, stored, event.seq) else {
return Ok(());
};
apply_todo_created(stage, props);
}
EventBody::TodoUpdated(props) => {
if !should_project_stage_todo_event(stored, props.list_kind) {
return Ok(());
}
let Some(stage) = stage_at_stored_or_current_visit(self, stored, event.seq) else {
return Ok(());
};
apply_todo_updated(stage, props);
}
EventBody::TodoDeleted(props) => {
if !should_project_stage_todo_event(stored, props.list_kind) {
return Ok(());
}
let Some(stage) = stage_at_stored_or_current_visit(self, stored, event.seq) else {
return Ok(());
};
@ -622,6 +631,20 @@ impl RunProjectionReducer for RunProjection {
}
}
/// Decide whether a TODO event should mutate `StageProjection.todos`.
///
/// OpenAI plan lists are scoped per agent session (`openai_plan:<session_id>`),
/// so a child/subagent session emits its own list events on the same stage.
/// The stage sidebar represents the root stage agent, so we drop child OpenAI
/// plan events here to keep the projection on the root session's list.
/// Anthropic task lists are root-scoped (`anthropic_tasks:<root_session_id>`)
/// and intentionally shared with subagents, so they always project.
fn should_project_stage_todo_event(stored: &RunEvent, list_kind: TodoListKind) -> bool {
let is_child_openai_plan_event =
matches!(list_kind, TodoListKind::OpenAiPlan) && stored.parent_session_id.is_some();
!is_child_openai_plan_event
}
fn apply_todo_created(stage: &mut StageProjection, props: &TodoCreatedProps) {
if stage
.todos
@ -3985,6 +4008,13 @@ mod tests {
.expect("stage todos present")
}
fn child_stage_event(seq: u32, body: EventBody, stage_id: StageId) -> EventEnvelope {
let mut event = test_stage_event(seq, body, stage_id);
event.event.session_id = Some(format!("child-session-{seq}"));
event.event.parent_session_id = Some("root-session".to_string());
event
}
fn created(
list: &str,
list_kind: TodoListKind,
@ -4144,6 +4174,163 @@ mod tests {
assert_eq!(stage_todos(&state, &claude).items[0].subject, "claude task");
}
#[test]
fn root_openai_plan_remains_projected_after_child_plan_events() {
let mut state = initialized_projection();
let stage_id = stage_id();
let root_list = "openai_plan:root_session";
let child_list = "openai_plan:child_session";
state
.apply_event(&test_stage_event(
1,
created(
root_list,
TodoListKind::OpenAiPlan,
"root-a",
0,
"root first",
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&test_stage_event(
2,
created(
root_list,
TodoListKind::OpenAiPlan,
"root-b",
1,
"root second",
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&child_stage_event(
3,
created(
child_list,
TodoListKind::OpenAiPlan,
"child-a",
0,
"child first",
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&child_stage_event(
4,
created(
child_list,
TodoListKind::OpenAiPlan,
"child-b",
1,
"child second",
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&test_stage_event(
5,
updated_status(
root_list,
TodoListKind::OpenAiPlan,
"root-a",
TodoStatus::Completed,
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&test_stage_event(
6,
updated_status(
root_list,
TodoListKind::OpenAiPlan,
"root-b",
TodoStatus::Completed,
),
stage_id.clone(),
))
.unwrap();
let projection = stage_todos(&state, &stage_id);
assert_eq!(projection.list_id, root_list);
assert_eq!(projection.kind, TodoListKind::OpenAiPlan);
assert_eq!(projection.items.len(), 2);
assert_eq!(projection.items[0].id, "root-a");
assert_eq!(projection.items[0].status, TodoStatus::Completed);
assert_eq!(projection.items[1].id, "root-b");
assert_eq!(projection.items[1].status, TodoStatus::Completed);
}
#[test]
fn child_openai_plan_events_do_not_create_stage_todos() {
let mut state = initialized_projection();
let stage_id = stage_id();
state
.apply_event(&child_stage_event(
1,
created(
"openai_plan:child_session",
TodoListKind::OpenAiPlan,
"child-a",
0,
"child first",
),
stage_id.clone(),
))
.unwrap();
assert!(
state
.stage(&stage_id)
.is_none_or(|stage| stage.todos.is_none())
);
}
#[test]
fn anthropic_child_session_task_events_still_project() {
let mut state = initialized_projection();
let stage_id = stage_id();
let list = "anthropic_tasks:root_session";
state
.apply_event(&child_stage_event(
1,
created(
list,
TodoListKind::AnthropicTasks,
"task-a",
0,
"task first",
),
stage_id.clone(),
))
.unwrap();
state
.apply_event(&child_stage_event(
2,
updated_status(
list,
TodoListKind::AnthropicTasks,
"task-a",
TodoStatus::Completed,
),
stage_id.clone(),
))
.unwrap();
let projection = stage_todos(&state, &stage_id);
assert_eq!(projection.list_id, list);
assert_eq!(projection.kind, TodoListKind::AnthropicTasks);
assert_eq!(projection.items.len(), 1);
assert_eq!(projection.items[0].id, "task-a");
assert_eq!(projection.items[0].status, TodoStatus::Completed);
}
#[test]
fn metadata_patch_merges_and_null_deletes() {
let mut state = initialized_projection();