diff --git a/acp_adapter/tools.py b/acp_adapter/tools.py index bb997dc234..e3ce3b1149 100644 --- a/acp_adapter/tools.py +++ b/acp_adapter/tools.py @@ -274,13 +274,27 @@ def _format_todo_result(result: Optional[str]) -> Optional[str]: "cancelled": "✗", } lines = ["**Todo list**", ""] - for item in data["todos"]: - if not isinstance(item, dict): - continue + todos = [t for t in data["todos"] if isinstance(t, dict)] + ids = {str(t.get("id") or "") for t in todos} + + def _depth(item: Dict[str, Any]) -> int: + depth, seen = 0, set() + node: Optional[Dict[str, Any]] = item + by_id = {str(t.get("id") or ""): t for t in todos} + while node is not None: + parent = str(node.get("parent") or "") + if not parent or parent not in ids or parent in seen: + break + seen.add(parent) + depth += 1 + node = by_id.get(parent) + return min(depth, 4) + + for item in todos: status = str(item.get("status") or "pending") content = str(item.get("content") or item.get("id") or "").strip() if content: - lines.append(f"- {icon.get(status, '•')} {content}") + lines.append(f"{' ' * _depth(item)}- {icon.get(status, '•')} {content}") if summary: cancelled = summary.get("cancelled", 0) lines.extend([ diff --git a/apps/desktop/src/app/chat/composer/status-stack/status-row.tsx b/apps/desktop/src/app/chat/composer/status-stack/status-row.tsx index e227815467..9c0959b92f 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/status-row.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/status-row.tsx @@ -114,7 +114,15 @@ export const StatusItemRow = memo(function StatusItemRow({ item, onDismiss, onOp return ( + {leadingGlyph(item, s)} + + ) : ( + leadingGlyph(item, s) + ) + } onActivate={onActivate} trailing={ action ? ( diff --git a/apps/desktop/src/lib/todos.test.ts b/apps/desktop/src/lib/todos.test.ts index a19752c737..4e0963159f 100644 --- a/apps/desktop/src/lib/todos.test.ts +++ b/apps/desktop/src/lib/todos.test.ts @@ -1,6 +1,51 @@ import { describe, expect, it } from 'vitest' -import { latestSessionTodos, parseTodos } from './todos' +import { latestSessionTodos, parseTodos, todoTree } from './todos' + +describe('todoTree', () => { + it('orders parents before children with depths', () => { + const tree = todoTree([ + { content: 'WP1', id: 'wp1', status: 'in_progress' }, + { content: 'WP2', id: 'wp2', status: 'pending' }, + { content: 'T1', id: 't1', parent: 'wp1', status: 'pending' }, + { content: 'T2', id: 't2', parent: 'wp1', status: 'pending' } + ]) + + expect(tree.map(([t, d]) => [t.id, d])).toEqual([ + ['wp1', 0], + ['t1', 1], + ['t2', 1], + ['wp2', 0] + ]) + }) + + it('degrades dangling and self parents to roots', () => { + const tree = todoTree([ + { content: 'A', id: 'a', parent: 'ghost', status: 'pending' }, + { content: 'B', id: 'b', parent: 'b', status: 'pending' } + ]) + + expect(tree.map(([t, d]) => [t.id, d])).toEqual([ + ['a', 0], + ['b', 0] + ]) + }) + + it('keeps cycle members instead of dropping them', () => { + const tree = todoTree([ + { content: 'A', id: 'a', parent: 'b', status: 'pending' }, + { content: 'B', id: 'b', parent: 'a', status: 'pending' } + ]) + + expect(tree.map(([t]) => t.id).sort()).toEqual(['a', 'b']) + }) + + it('preserves parent through parseTodos', () => { + expect(parseTodos([{ content: 'x', id: 'c', parent: 'p', status: 'pending' }])).toEqual([ + { content: 'x', id: 'c', parent: 'p', status: 'pending' } + ]) + }) +}) describe('parseTodos', () => { it('parses todo arrays with valid ids, content, and statuses', () => { diff --git a/apps/desktop/src/lib/todos.ts b/apps/desktop/src/lib/todos.ts index 6a5d8eea06..d12f9de370 100644 --- a/apps/desktop/src/lib/todos.ts +++ b/apps/desktop/src/lib/todos.ts @@ -3,6 +3,8 @@ export type TodoStatus = 'pending' | 'in_progress' | 'completed' | 'cancelled' export interface TodoItem { content: string id: string + /** Optional id of another item — renders this as a nested subtask. */ + parent?: string status: TodoStatus } @@ -19,8 +21,9 @@ function parseArray(value: unknown[]): TodoItem[] { const id = String(item.id ?? '').trim() const content = String(item.content ?? '').trim() + const parent = String(item.parent ?? '').trim() - return id && content ? [{ content, id, status: item.status }] : [] + return id && content ? [{ content, id, status: item.status, ...(parent && parent !== id ? { parent } : {}) }] : [] }) } @@ -50,6 +53,54 @@ function parse(value: unknown, depth: number): null | TodoItem[] { export const parseTodos = (value: unknown): null | TodoItem[] => parse(value, 0) +/** DFS order of a (possibly nested) todo list: [item, depth] pairs, parents + * before children. Dangling/cyclic parents degrade to depth 0. */ +export function todoTree(todos: readonly TodoItem[]): [TodoItem, number][] { + const ids = new Set(todos.map(t => t.id)) + const kids = new Map() + const roots: TodoItem[] = [] + + for (const t of todos) { + if (t.parent && ids.has(t.parent) && t.parent !== t.id) { + const list = kids.get(t.parent) ?? [] + list.push(t) + kids.set(t.parent, list) + } else { + roots.push(t) + } + } + + const out: [TodoItem, number][] = [] + const seen = new Set() + + const walk = (item: TodoItem, depth: number) => { + if (seen.has(item.id)) { + return + } + + seen.add(item.id) + out.push([item, depth]) + + for (const kid of kids.get(item.id) ?? []) { + walk(kid, depth + 1) + } + } + + for (const root of roots) { + walk(root, 0) + } + + // Cycle members never reach a root — append them flat so nothing is lost. + for (const t of todos) { + if (!seen.has(t.id)) { + seen.add(t.id) + out.push([t, 0]) + } + } + + return out +} + /** Latest parseable todo list from one message's aui content parts (tool-call * parts named `todo`; live parts carry `todos`, hydrated ones args/result). */ export function todosFromMessageContent(content: unknown): null | TodoItem[] { diff --git a/apps/desktop/src/store/composer-status.ts b/apps/desktop/src/store/composer-status.ts index d93263cc5a..98de044fe2 100644 --- a/apps/desktop/src/store/composer-status.ts +++ b/apps/desktop/src/store/composer-status.ts @@ -3,7 +3,7 @@ import { atom, computed } from 'nanostores' import { translateNow } from '@/i18n' import { stableArray } from '@/lib/stable-array' -import type { TodoItem, TodoStatus } from '@/lib/todos' +import { type TodoItem, type TodoStatus, todoTree } from '@/lib/todos' import { $gateway } from './gateway' import { $goalsBySession, type GoalStatus } from './goals' @@ -23,6 +23,8 @@ export interface ComposerStatusItem { exitCode?: number /** subagent: active tool label shown on the right. */ currentTool?: string + /** todo: nesting depth (0 = top-level) for indented subtask rows. */ + depth?: number /** goal: active | paused | waiting | done. */ goalStatus?: GoalStatus id: string @@ -146,7 +148,8 @@ const subToItem = (s: SubagentProgress): ComposerStatusItem => ({ type: 'subagent' }) -const todoToItem = (t: TodoItem): ComposerStatusItem => ({ +const todoToItem = (t: TodoItem, depth: number): ComposerStatusItem => ({ + depth, id: `todo:${t.id}`, state: t.status === 'in_progress' ? 'running' : 'done', title: t.content, @@ -183,6 +186,7 @@ const sameStatusItem = (a: ComposerStatusItem, b: ComposerStatusItem) => a.currentTool === b.currentTool && a.goalStatus === b.goalStatus && a.todoStatus === b.todoStatus && + a.depth === b.depth && a.sessionId === b.sessionId const stabilizeItems = (prev: ComposerStatusItem[] | undefined, next: ComposerStatusItem[]): ComposerStatusItem[] => { @@ -209,7 +213,7 @@ export const $statusItemsBySession = computed( } for (const [sid, list] of Object.entries(todos)) { - push(sid, list.map(todoToItem)) + push(sid, todoTree(list).map(([t, depth]) => todoToItem(t, depth))) } for (const [sid, goal] of Object.entries(goals)) { diff --git a/tests/tools/test_todo_nested.py b/tests/tools/test_todo_nested.py new file mode 100644 index 0000000000..98b936b6a4 --- /dev/null +++ b/tests/tools/test_todo_nested.py @@ -0,0 +1,167 @@ +"""Tests for nested subtasks in the todo tool (parent field). + +Covers: validation of the parent field, dangling/cyclic parent +sanitization, merge-mode parent updates, tree-aware post-compression +injection, and the flat-only reorder guard. +""" + +import json + +from tools.todo_tool import TodoStore, todo_tool + + +def _item(i, status="pending", parent=None, content=None): + d = {"id": i, "content": content or f"task {i}", "status": status} + if parent is not None: + d["parent"] = parent + return d + + +class TestParentValidation: + def test_parent_preserved(self): + store = TodoStore() + items = store.write([_item("a"), _item("a1", parent="a")]) + assert items[1]["parent"] == "a" + + def test_self_parent_dropped(self): + store = TodoStore() + items = store.write([_item("a", parent="a")]) + assert "parent" not in items[0] + + def test_dangling_parent_dropped(self): + store = TodoStore() + items = store.write([_item("a", parent="ghost")]) + assert "parent" not in items[0] + + def test_cycle_broken(self): + store = TodoStore() + items = store.write([_item("a", parent="b"), _item("b", parent="a")]) + # At least one link removed; no item can walk a loop + by_id = {i["id"]: i for i in items} + for item in items: + seen = set() + node = item + while node.get("parent"): + assert node["parent"] not in seen + seen.add(node["id"]) + node = by_id[node["parent"]] + + def test_empty_parent_omitted(self): + store = TodoStore() + items = store.write([_item("a", parent="")]) + assert "parent" not in items[0] + + +class TestMergeParent: + def test_merge_sets_parent(self): + store = TodoStore() + store.write([_item("a"), _item("b")]) + items = store.write([{"id": "b", "parent": "a"}], merge=True) + by_id = {i["id"]: i for i in items} + assert by_id["b"]["parent"] == "a" + + def test_merge_clears_parent_with_empty_string(self): + store = TodoStore() + store.write([_item("a"), _item("b", parent="a")]) + items = store.write([{"id": "b", "parent": ""}], merge=True) + by_id = {i["id"]: i for i in items} + assert "parent" not in by_id["b"] + + def test_merge_new_child_appended(self): + store = TodoStore() + store.write([_item("a")]) + items = store.write([_item("a2", parent="a")], merge=True) + assert items[-1] == {"id": "a2", "content": "task a2", "status": "pending", "parent": "a"} + + +class TestNestedInjection: + def test_children_indented(self): + store = TodoStore() + store.write([ + _item("wp1", status="in_progress"), + _item("t1", parent="wp1"), + _item("t2", parent="wp1"), + ]) + text = store.format_for_injection() + assert text is not None + lines = text.split("\n") + assert lines[1].startswith("- [>] wp1.") + assert lines[2].startswith(" - [ ] t1.") + assert lines[3].startswith(" - [ ] t2.") + + def test_completed_parent_kept_when_child_active(self): + store = TodoStore() + store.write([ + _item("wp1", status="completed"), + _item("t1", status="pending", parent="wp1"), + ]) + text = store.format_for_injection() + assert text is not None + assert "wp1" in text # parent context survives + assert "[x]" in text + assert " - [ ] t1." in text + + def test_finished_subtree_omitted(self): + store = TodoStore() + store.write([ + _item("wp1", status="completed"), + _item("t1", status="completed", parent="wp1"), + _item("wp2", status="pending"), + ]) + text = store.format_for_injection() + assert text is not None + assert "wp1" not in text + assert "t1" not in text + assert "wp2" in text + + def test_all_finished_returns_none(self): + store = TodoStore() + store.write([ + _item("a", status="completed"), + _item("a1", status="cancelled", parent="a"), + ]) + assert store.format_for_injection() is None + + def test_flat_injection_unchanged(self): + store = TodoStore() + store.write([_item("a", status="in_progress"), _item("b")]) + text = store.format_for_injection() + assert text is not None + assert text.split("\n")[1] == "- [>] a. task a (in_progress)" + + +class TestOrderGuard: + def test_nested_list_keeps_authored_order(self): + store = TodoStore() + items = store.write([ + _item("a", status="pending"), + _item("b", status="in_progress"), + _item("b1", parent="b"), + ]) + assert [i["id"] for i in items] == ["a", "b", "b1"] + + def test_flat_list_still_reordered(self): + store = TodoStore() + items = store.write([ + _item("a", status="pending"), + _item("b", status="in_progress"), + ]) + assert [i["id"] for i in items] == ["b", "a"] + + +class TestToolRoundTrip: + def test_parent_survives_json(self): + store = TodoStore() + out = todo_tool(todos=[_item("a"), _item("a1", parent="a")], store=store) + data = json.loads(out) + assert data["todos"][1]["parent"] == "a" + assert data["summary"]["total"] == 2 + + def test_hydration_replay_preserves_parent(self): + # Simulate _hydrate_todo_store: write the previous tool result's + # todos array back into a fresh store in replace mode. + store = TodoStore() + out = json.loads(todo_tool(todos=[_item("a"), _item("a1", parent="a")], store=store)) + fresh = TodoStore() + replayed = fresh.write(out["todos"], merge=False) + assert replayed[1]["parent"] == "a" diff --git a/tools/todo_tool.py b/tools/todo_tool.py index 8cd74c5f87..213f65b28d 100644 --- a/tools/todo_tool.py +++ b/tools/todo_tool.py @@ -51,6 +51,7 @@ class TodoStore: - id: unique string identifier (agent-chosen) - content: task description - status: pending | in_progress | completed | cancelled + - parent: optional id of another item, for nested subtasks """ def __init__(self): @@ -86,6 +87,12 @@ class TodoStore: status = str(t["status"]).strip().lower() if status in VALID_STATUSES: existing[item_id]["status"] = status + if "parent" in t: + parent = str(t["parent"] or "").strip() + if parent: + existing[item_id]["parent"] = parent + else: + existing[item_id].pop("parent", None) else: # New item -- validate fully and append to end validated = self._validate(t) @@ -105,6 +112,7 @@ class TodoStore: # (list order is priority). if len(self._items) > MAX_TODO_ITEMS: self._items = self._items[:MAX_TODO_ITEMS] + self._sanitize_parents(self._items) return self.read() def read(self) -> List[Dict[str, str]]: @@ -134,18 +142,39 @@ class TodoStore: } # Only inject pending/in_progress items — completed/cancelled ones - # cause the model to re-do finished work after compression. - active_items = [ - item for item in self._items - if item["status"] in {"pending", "in_progress"} - ] - if not active_items: - return None + # cause the model to re-do finished work after compression. A parent + # is kept (with its real status marker) when any descendant is + # active, so subtasks keep their context. + active = {"pending", "in_progress"} + children: Dict[str, List[Dict[str, str]]] = {} + roots: List[Dict[str, str]] = [] + for item in self._items: + parent = item.get("parent") + if parent: + children.setdefault(parent, []).append(item) + else: + roots.append(item) + + def render(item: Dict[str, str], depth: int, out: List[str]) -> bool: + kid_lines: List[str] = [] + has_active_kid = False + for kid in children.get(item["id"], []): + has_active_kid |= render(kid, depth + 1, kid_lines) + keep = item["status"] in active or has_active_kid + if keep: + marker = markers.get(item["status"], "[?]") + out.append( + f"{' ' * depth}- {marker} {item['id']}. " + f"{item['content']} ({item['status']})" + ) + out.extend(kid_lines) + return keep lines = [TODO_INJECTION_HEADER] - for item in active_items: - marker = markers.get(item["status"], "[?]") - lines.append(f"- {marker} {item['id']}. {item['content']} ({item['status']})") + for item in roots: + render(item, 0, lines) + if len(lines) == 1: + return None return "\n".join(lines) @@ -187,7 +216,34 @@ class TodoStore: if status not in VALID_STATUSES: status = "pending" - return {"id": item_id, "content": content, "status": status} + result = {"id": item_id, "content": content, "status": status} + parent = str(item.get("parent") or "").strip() + if parent and parent != item_id: + result["parent"] = parent + return result + + @staticmethod + def _sanitize_parents(items: List[Dict[str, str]]) -> None: + """Drop dangling parent refs and break cycles (in place). + + A parent pointing at a missing id, or a chain that loops back on + itself, would corrupt tree rendering — such items become roots. + """ + ids = {item["id"] for item in items} + by_id = {item["id"]: item for item in items} + for item in items: + parent = item.get("parent") + if parent and parent not in ids: + item.pop("parent", None) + for item in items: + seen = {item["id"]} + node = item + while node.get("parent"): + if node["parent"] in seen: + item.pop("parent", None) + break + seen.add(node["parent"]) + node = by_id[node["parent"]] @staticmethod def _dedupe_by_id(todos: List[Dict[str, Any]]) -> List[Dict[str, Any]]: @@ -205,6 +261,10 @@ class TodoStore: @staticmethod def _normalize_order(items: List[Dict[str, str]]) -> List[Dict[str, str]]: """Lift the active step ahead of any earlier unfinished placeholders.""" + # Nested lists keep authored order — reordering a flat position would + # tear a subtask away from its siblings. + if any(item.get("parent") for item in items): + return items active_index = next( (i for i, item in enumerate(items) if item["status"] == "in_progress"), None, @@ -303,6 +363,7 @@ TODO_SCHEMA = { "item so none are silently dropped. " "Call with no parameters to read the current list.\n" "List order is priority. Only ONE item in_progress at a time. " + "Break large phases into subtasks via parent. " "Mark an item completed only after the work is verified done, never " "based on intent. If something fails, cancel it and add a revised " "item. Always returns the full current list." @@ -326,6 +387,10 @@ TODO_SCHEMA = { "status": { "type": "string", "enum": ["pending", "in_progress", "completed", "cancelled"] + }, + "parent": { + "type": "string", + "description": "Optional id of another item, making this a nested subtask. Omit for top-level." } }, "required": ["id", "content", "status"] diff --git a/website/docs/reference/tools-reference.md b/website/docs/reference/tools-reference.md index 3a2ee0fe2d..706f718a23 100644 --- a/website/docs/reference/tools-reference.md +++ b/website/docs/reference/tools-reference.md @@ -278,7 +278,7 @@ hours of quiet from the rotation. | Tool | Description | Requires environment | |------|-------------|----------------------| -| `todo` | Manage your task list for the current session. Use for complex tasks with 3+ steps or when the user provides multiple tasks. Call with no parameters to read the current list. Writing: - Provide 'todos' array to create/update items - merge=… | — | +| `todo` | Manage your task list for the current session. Use for complex tasks with 3+ steps or when the user provides multiple tasks. Call with no parameters to read the current list. Items may nest: an item's optional `parent` field points at another item's id, making it a subtask — surfaces render the tree indented. | — | ## `vision` toolset