mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
Merge pull request #604 from fabro-sh/fix/root-agent-todo-projection
Fix root-agent TODO projection ownership
This commit is contained in:
commit
43dcfa3b09
4 changed files with 138 additions and 67 deletions
|
|
@ -10529,7 +10529,11 @@ components:
|
|||
oneOf:
|
||||
- $ref: "#/components/schemas/TodoListProjection"
|
||||
- type: "null"
|
||||
description: Projected todo / task list for this stage.
|
||||
description: |
|
||||
Todo / task list owned by this stage's root agent session. OpenAI
|
||||
child sessions have separate per-session plans that do not appear
|
||||
here. Anthropic task lists are root-scoped and shared with child
|
||||
sessions, so child mutations of that shared list do appear here.
|
||||
subagents:
|
||||
type: array
|
||||
description: Subagents spawned by this stage, in replay/insertion order.
|
||||
|
|
|
|||
|
|
@ -229,6 +229,11 @@ and focused on the task.
|
|||
{file_edit_failure_guidance}
|
||||
- Do not `git commit` your changes or create new git branches unless explicitly requested.
|
||||
|
||||
# Planning
|
||||
|
||||
If you create a checklist or task list, you update item statuses incrementally as each item is \
|
||||
completed rather than marking every item done only at the end.
|
||||
|
||||
# Validating Your Work
|
||||
|
||||
If the codebase has tests or the ability to build or run, consider using them to verify your \
|
||||
|
|
@ -338,6 +343,17 @@ mod tests {
|
|||
assert!(prompt.contains("existing code conventions"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_system_prompt_matches_codex_incremental_plan_guidance() {
|
||||
let profile = OpenAiProfile::new("gpt-5.5");
|
||||
let env = MockSandbox::linux();
|
||||
let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]);
|
||||
assert!(prompt.contains(
|
||||
"update item statuses incrementally as each item is completed rather than \
|
||||
marking every item done only at the end"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_system_prompt_includes_memory() {
|
||||
let profile = OpenAiProfile::new("o3-mini");
|
||||
|
|
|
|||
|
|
@ -546,18 +546,27 @@ impl RunProjectionReducer for RunProjection {
|
|||
stage.state = StageState::from(outcome);
|
||||
}
|
||||
EventBody::TodoCreated(props) => {
|
||||
if !should_project_root_agent_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, stored.parent_session_id.as_deref());
|
||||
apply_todo_created(stage, props);
|
||||
}
|
||||
EventBody::TodoUpdated(props) => {
|
||||
if !should_project_root_agent_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_root_agent_todo_event(stored, props.list_kind) {
|
||||
return Ok(());
|
||||
}
|
||||
let Some(stage) = stage_at_stored_or_current_visit(self, stored, event.seq) else {
|
||||
return Ok(());
|
||||
};
|
||||
|
|
@ -681,36 +690,37 @@ impl RunProjectionReducer for RunProjection {
|
|||
}
|
||||
}
|
||||
|
||||
fn apply_todo_created(
|
||||
stage: &mut StageProjection,
|
||||
props: &TodoCreatedProps,
|
||||
parent_session_id: Option<&str>,
|
||||
) {
|
||||
// OpenAI plan lists are scoped per agent session (`openai_plan:<session_id>`).
|
||||
// A child subagent must not displace a different list already in the slot
|
||||
// (which would clobber the root agent's plan); but it may create the slot
|
||||
// when empty, or extend its own list. Anthropic task lists are root-scoped
|
||||
// and pass through unchanged.
|
||||
if matches!(props.list_kind, TodoListKind::OpenAiPlan)
|
||||
&& parent_session_id.is_some()
|
||||
&& stage
|
||||
.todos
|
||||
.as_ref()
|
||||
.is_some_and(|list| list.list_id != props.list_id)
|
||||
{
|
||||
return;
|
||||
/// Decide whether a TODO event should mutate
|
||||
/// `StageProjection.root_agent_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 root-agent projection excludes those child plans, while the underlying
|
||||
/// events remain in the run event log. Anthropic task lists are root-scoped
|
||||
/// (`anthropic_tasks:<root_session_id>`) and intentionally shared with
|
||||
/// subagents, so they always project.
|
||||
fn should_project_root_agent_todo_event(stored: &RunEvent, list_kind: TodoListKind) -> bool {
|
||||
match list_kind {
|
||||
TodoListKind::OpenAiPlan => stored.parent_session_id.is_none(),
|
||||
TodoListKind::AnthropicTasks => true,
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_todo_created(stage: &mut StageProjection, props: &TodoCreatedProps) {
|
||||
if stage
|
||||
.todos
|
||||
.root_agent_todos
|
||||
.as_ref()
|
||||
.is_none_or(|list| list.list_id != props.list_id || list.kind != props.list_kind)
|
||||
{
|
||||
stage.todos = Some(TodoListProjection::new(
|
||||
stage.root_agent_todos = Some(TodoListProjection::new(
|
||||
props.list_kind,
|
||||
props.list_id.clone(),
|
||||
));
|
||||
}
|
||||
let list = stage.todos.as_mut().expect("todo list was just inserted");
|
||||
let list = stage
|
||||
.root_agent_todos
|
||||
.as_mut()
|
||||
.expect("todo list was just inserted");
|
||||
list.upsert(TodoProjection {
|
||||
id: props.todo_id.clone(),
|
||||
status: props.status,
|
||||
|
|
@ -727,7 +737,7 @@ fn apply_todo_created(
|
|||
|
||||
fn apply_todo_updated(stage: &mut StageProjection, props: &TodoUpdatedProps) {
|
||||
if let Some(list) = stage
|
||||
.todos
|
||||
.root_agent_todos
|
||||
.as_mut()
|
||||
.filter(|list| list.list_id == props.list_id)
|
||||
{
|
||||
|
|
@ -737,7 +747,7 @@ fn apply_todo_updated(stage: &mut StageProjection, props: &TodoUpdatedProps) {
|
|||
|
||||
fn apply_todo_deleted(stage: &mut StageProjection, props: &TodoDeletedProps) {
|
||||
let Some(list) = stage
|
||||
.todos
|
||||
.root_agent_todos
|
||||
.as_mut()
|
||||
.filter(|list| list.list_id == props.list_id)
|
||||
else {
|
||||
|
|
@ -745,7 +755,7 @@ fn apply_todo_deleted(stage: &mut StageProjection, props: &TodoDeletedProps) {
|
|||
};
|
||||
list.remove(&props.todo_id);
|
||||
if list.items.is_empty() {
|
||||
stage.todos = None;
|
||||
stage.root_agent_todos = None;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -4445,11 +4455,14 @@ mod tests {
|
|||
StageId::new("code", 1)
|
||||
}
|
||||
|
||||
fn stage_todos<'a>(state: &'a RunProjection, stage_id: &StageId) -> &'a TodoListProjection {
|
||||
fn root_agent_todos<'a>(
|
||||
state: &'a RunProjection,
|
||||
stage_id: &StageId,
|
||||
) -> &'a TodoListProjection {
|
||||
state
|
||||
.stage(stage_id)
|
||||
.and_then(|stage| stage.todos.as_ref())
|
||||
.expect("stage todos present")
|
||||
.and_then(|stage| stage.root_agent_todos.as_ref())
|
||||
.expect("root agent todos present")
|
||||
}
|
||||
|
||||
fn child_stage_event(seq: u32, body: EventBody, stage_id: StageId) -> EventEnvelope {
|
||||
|
|
@ -4539,7 +4552,7 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = stage_todos(&state, &stage_id);
|
||||
let projection = root_agent_todos(&state, &stage_id);
|
||||
assert_eq!(projection.list_id, list);
|
||||
assert_eq!(projection.items.len(), 2);
|
||||
assert_eq!(projection.items[0].id, "a");
|
||||
|
|
@ -4574,7 +4587,7 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = stage_todos(&state, &stage_id);
|
||||
let projection = root_agent_todos(&state, &stage_id);
|
||||
assert_eq!(projection.items.len(), 1);
|
||||
assert_eq!(projection.items[0].id, "b");
|
||||
}
|
||||
|
|
@ -4613,9 +4626,12 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(stage_todos(&state, &plan_one).items[0].subject, "p1");
|
||||
assert_eq!(stage_todos(&state, &plan_two).items[0].subject, "p2");
|
||||
assert_eq!(stage_todos(&state, &claude).items[0].subject, "claude task");
|
||||
assert_eq!(root_agent_todos(&state, &plan_one).items[0].subject, "p1");
|
||||
assert_eq!(root_agent_todos(&state, &plan_two).items[0].subject, "p2");
|
||||
assert_eq!(
|
||||
root_agent_todos(&state, &claude).items[0].subject,
|
||||
"claude task"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -4701,7 +4717,7 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = stage_todos(&state, &stage_id);
|
||||
let projection = root_agent_todos(&state, &stage_id);
|
||||
assert_eq!(projection.list_id, root_list);
|
||||
assert_eq!(projection.kind, TodoListKind::OpenAiPlan);
|
||||
assert_eq!(projection.items.len(), 2);
|
||||
|
|
@ -4712,50 +4728,80 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn child_openai_plan_projects_when_no_root_plan_exists() {
|
||||
// Regression: when a stage's root agent never calls update_plan but
|
||||
// a subagent does, the subagent's plan must still surface in
|
||||
// StageProjection.todos. Observed on run 01KSDXK5DJ61CFCK9YSDR8AETQ
|
||||
// (implement@1), where a gpt-5.5 root delegated to a subagent that
|
||||
// owned the only plan list — and the sidebar showed no todos.
|
||||
fn child_openai_plan_does_not_project_when_root_has_no_plan() {
|
||||
let mut state = initialized_projection();
|
||||
let stage_id = stage_id();
|
||||
let child_list = "openai_plan:child_session";
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
.apply_event(&test_stage_event(
|
||||
1,
|
||||
created(child_list, TodoListKind::OpenAiPlan, "c-a", 0, "first"),
|
||||
EventBody::StageStarted(started_props()),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
2,
|
||||
created(child_list, TodoListKind::OpenAiPlan, "c-b", 1, "second"),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
3,
|
||||
updated_status(
|
||||
child_list,
|
||||
created(
|
||||
"openai_plan:child_session",
|
||||
TodoListKind::OpenAiPlan,
|
||||
"c-a",
|
||||
TodoStatus::Completed,
|
||||
0,
|
||||
"child work",
|
||||
),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = stage_todos(&state, &stage_id);
|
||||
assert_eq!(projection.list_id, child_list);
|
||||
assert_eq!(projection.kind, TodoListKind::OpenAiPlan);
|
||||
assert_eq!(projection.items.len(), 2);
|
||||
assert_eq!(projection.items[0].id, "c-a");
|
||||
assert_eq!(projection.items[0].status, TodoStatus::Completed);
|
||||
assert_eq!(projection.items[1].id, "c-b");
|
||||
assert_eq!(projection.items[1].status, TodoStatus::Pending);
|
||||
let stage = state.stage(&stage_id).expect("stage projection present");
|
||||
assert!(
|
||||
stage.root_agent_todos.is_none(),
|
||||
"a child session's plan must not become the stage's root plan"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_openai_plan_projects_after_earlier_child_plan_event() {
|
||||
let mut state = initialized_projection();
|
||||
let stage_id = stage_id();
|
||||
let root_list = "openai_plan:root_session";
|
||||
state
|
||||
.apply_event(&test_stage_event(
|
||||
1,
|
||||
EventBody::StageStarted(started_props()),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&child_stage_event(
|
||||
2,
|
||||
created(
|
||||
"openai_plan:child_session",
|
||||
TodoListKind::OpenAiPlan,
|
||||
"child-a",
|
||||
0,
|
||||
"child work",
|
||||
),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
state
|
||||
.apply_event(&test_stage_event(
|
||||
3,
|
||||
created(
|
||||
root_list,
|
||||
TodoListKind::OpenAiPlan,
|
||||
"root-a",
|
||||
0,
|
||||
"root work",
|
||||
),
|
||||
stage_id.clone(),
|
||||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = root_agent_todos(&state, &stage_id);
|
||||
assert_eq!(projection.list_id, root_list);
|
||||
assert_eq!(projection.items.len(), 1);
|
||||
assert_eq!(projection.items[0].subject, "root work");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -4789,7 +4835,7 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
let projection = stage_todos(&state, &stage_id);
|
||||
let projection = root_agent_todos(&state, &stage_id);
|
||||
assert_eq!(projection.list_id, list);
|
||||
assert_eq!(projection.kind, TodoListKind::AnthropicTasks);
|
||||
assert_eq!(projection.items.len(), 1);
|
||||
|
|
@ -4855,7 +4901,7 @@ mod tests {
|
|||
))
|
||||
.unwrap();
|
||||
|
||||
let todo = &stage_todos(&state, &stage_id).items[0];
|
||||
let todo = &root_agent_todos(&state, &stage_id).items[0];
|
||||
assert!(!todo.metadata.contains_key("k1"));
|
||||
assert_eq!(todo.metadata.get("k2"), Some(&serde_json::json!("v2")));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -351,8 +351,13 @@ pub struct StageProjection {
|
|||
pub usage: BilledTokenCounts,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model: Option<ModelRef>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub todos: Option<TodoListProjection>,
|
||||
/// Todo/task list owned by the stage's root agent session.
|
||||
///
|
||||
/// OpenAI child sessions own separate per-session plans and do not appear
|
||||
/// here. Anthropic task lists are root-scoped and shared with child
|
||||
/// sessions, so child mutations of that shared list do appear here.
|
||||
#[serde(default, rename = "todos", skip_serializing_if = "Option::is_none")]
|
||||
pub root_agent_todos: Option<TodoListProjection>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub subagents: Vec<SubAgentProjection>,
|
||||
#[serde(default, skip_serializing_if = "SkillsProjection::is_empty")]
|
||||
|
|
@ -439,7 +444,7 @@ impl StageProjection {
|
|||
timing: None,
|
||||
usage: BilledTokenCounts::default(),
|
||||
model: None,
|
||||
todos: None,
|
||||
root_agent_todos: None,
|
||||
subagents: Vec::new(),
|
||||
skills: SkillsProjection::default(),
|
||||
permission_level: None,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue