diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a92f3db36..117a73366 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,7 @@ - Narrowed the `history://` contract in the system prompt to match the implementation: it serves registered agents process-wide plus persisted subagents discoverable from their artifact trees, but does not discover unregistered top-level sessions solely from persisted session files ([#5839](https://github.com/can1357/oh-my-pi/issues/5839)). - Fixed expanded `!` bash and `eval` output keeping a stale `… N more lines (ctrl+o to expand)` footer after Ctrl+O revealed every line ([#5842](https://github.com/can1357/oh-my-pi/issues/5842)). - Fixed MCP reauthentication continuing to an authorization URL without `client_id` after dynamic client registration fails; the registration error now blocks the flow with the provider response details ([#5852](https://github.com/can1357/oh-my-pi/issues/5852)). +- Fixed collapsed todo views hiding the in-progress task in large phases. Both the transient `Todo` tool result and the sticky `Todos` HUD now share one walking-viewport policy: completed/abandoned tasks are omitted, every active task (the in-progress one, or a pending task a live subagent is executing) is pulled to the head in todo order, remaining rows fill with the following pending tasks, and an explicit `… N more active todos` summary is shown when active work alone exceeds the preview cap ([#5873](https://github.com/can1357/oh-my-pi/issues/5873)). ## [17.0.2] - 2026-07-17 diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index de20f09c4..3a1937eac 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -123,7 +123,12 @@ import type { LspStartupServerInfo } from "../tools"; import { normalizeLocalScheme } from "../tools/path-utils"; import { replaceTabs, TRUNCATE_LENGTHS, truncateToWidth } from "../tools/render-utils"; import { setAutoQaConsentHandler } from "../tools/report-tool-issue"; -import { formatPhaseDisplayName, todoMatchesAnyDescription } from "../tools/todo"; +import { + formatPhaseDisplayName, + selectCollapsedTodos, + setActiveTodoDescriptionsProvider, + todoMatchesAnyDescription, +} from "../tools/todo"; import { ToolError } from "../tools/tool-errors"; import { vocalizer } from "../tts/vocalizer"; import { renderTreeList } from "../tui/tree-list"; @@ -951,6 +956,9 @@ export class InteractiveMode implements InteractiveModeContext { this.#observerRegistry.onChange(kind => { this.#scheduleObserverUiSync(kind); }); + // Let the transient todo tool result light up pending todos executed by a + // live subagent, matching the sticky HUD's active set (#5873). + setActiveTodoDescriptionsProvider(() => this.#getActiveSubagentDescriptions()); // Load initial todos await this.#loadTodoList(); @@ -1895,15 +1903,29 @@ export class InteractiveMode implements InteractiveModeContext { const isMatched = (todo: TodoItem): boolean => activeDescs.length > 0 && todoMatchesAnyDescription(todo.content, activeDescs); - // Task subtree for a phase. Collapsed previews the first open tasks — the - // stage's `done/total` makes the hidden count obvious, so there is no - // "… more" row; expanded lists every task. + // Task subtree for a phase. Collapsed runs the shared walking-viewport + // policy (completed/abandoned omitted, active work pulled to the head, + // then following pending tasks) so the HUD and the transient tool result + // can never disagree about the current work (#5873). Expanded lists all. const renderTasks = (phase: TodoPhase): string[] => { - const open = phase.tasks.filter(t => t.status === "pending" || t.status === "in_progress"); - const base = expanded ? phase.tasks : open.length > 0 ? open : phase.tasks; - const items = expanded ? base : base.slice(0, activeTaskCap); + if (expanded) { + return renderTreeList( + { + items: phase.tasks, + expanded: true, + renderItem: todo => this.#formatTodoLine(todo, "", isMatched(todo)), + }, + theme, + ); + } + const selection = selectCollapsedTodos(phase.tasks, isMatched, activeTaskCap); return renderTreeList( - { items, expanded: true, renderItem: todo => this.#formatTodoLine(todo, "", isMatched(todo)) }, + { + items: selection.items, + itemType: "task", + trailingSummary: selection.summary, + renderItem: todo => this.#formatTodoLine(todo, "", isMatched(todo)), + }, theme, ); }; diff --git a/packages/coding-agent/src/tools/todo.ts b/packages/coding-agent/src/tools/todo.ts index 095e995e5..d34f4bc11 100644 --- a/packages/coding-agent/src/tools/todo.ts +++ b/packages/coding-agent/src/tools/todo.ts @@ -12,7 +12,7 @@ import type { ToolSession } from "../sdk"; import type { SessionEntry } from "../session/session-entries"; import { framedBlock, renderStatusLine, renderTreeList } from "../tui"; import { normalizePathLikeInput, resolveToCwd } from "./path-utils"; -import { formatErrorDetail, PREVIEW_LIMITS } from "./render-utils"; +import { formatErrorDetail, formatMoreItems, PREVIEW_LIMITS, pluralize } from "./render-utils"; // ============================================================================= // Types @@ -214,6 +214,80 @@ export function todoMatchesAnyDescription(content: string, descriptions: readonl return false; } +/** + * A todo the collapsed viewport treats as current work: the literal + * `in_progress` task or a pending task a live subagent is executing. Both + * collapsed views (transient tool result + sticky HUD) run this same policy so + * they can never disagree about what the agent is doing (#5873). + */ +function isActiveTodo(task: T, isMatched: (task: T) => boolean): boolean { + return task.status === "in_progress" || (task.status === "pending" && isMatched(task)); +} + +/** Result of {@link selectCollapsedTodos}: the rows to render plus an optional + * summary line (empty string ⇒ no summary row). */ +export interface CollapsedTodoSelection { + items: T[]; + summary: string; +} + +/** + * Walking-viewport selection for a phase's collapsed todo preview (#5873). + * + * Policy, applied to `tasks` in todo order: + * 1. While the phase has open work, completed/abandoned tasks are omitted. A + * phase with no open tasks left falls back to its closed tasks so the sticky + * HUD's closed-todo persistence still has something to render. + * 2. Every active task (in-progress, or pending matched to a live subagent) is + * placed at the head in stable todo order — never dropped for lying outside + * an ordinary window. + * 3. Remaining rows up to `cap` are filled with the pending tasks that follow + * the first active one, in todo order (falling back to leading pending tasks + * when no active task exists), so a freshly-promoted task leads the preview. + * 4. When active tasks alone exceed `cap`, only the first `cap` active tasks are + * shown and the summary counts the hidden *active* todos, never replacing + * them with unrelated pending rows. + * + * The summary otherwise counts the remaining tasks in the display base. Returns + * the whole base with an empty summary when it already fits. + */ +export function selectCollapsedTodos( + tasks: T[], + isMatched: (task: T) => boolean, + cap: number, +): CollapsedTodoSelection { + const open = tasks.filter(task => task.status === "pending" || task.status === "in_progress"); + // No open work: fall back to the closed tasks so a settled phase still + // renders (HUD closed-todo persistence). Closed tasks are never active. + const base = open.length > 0 ? open : tasks; + if (base.length <= cap) return { items: base, summary: "" }; + + const active = base.filter(task => isActiveTodo(task, isMatched)); + // Only when active work strictly exceeds the cap do we drop pending rows and + // count hidden *actives*. At exactly `cap` actives, fall through so the normal + // branch still surfaces any following pending work in the summary. + if (active.length > cap) { + const hiddenActive = active.length - cap; + return { + items: active.slice(0, cap), + summary: `… ${hiddenActive} more active ${pluralize("todo", hiddenActive)}`, + }; + } + + // Fill trailing rows with tasks following the first active one, so the + // promoted/current task leads and its successors follow in todo order. + const firstActiveIdx = active.length > 0 ? base.indexOf(active[0]) : 0; + const fill: T[] = []; + for (let i = firstActiveIdx; i < base.length && active.length + fill.length < cap; i++) { + const task = base[i]; + if (isActiveTodo(task, isMatched)) continue; + fill.push(task); + } + const items = [...active, ...fill]; + const hidden = base.length - items.length; + return { items, summary: hidden > 0 ? formatMoreItems(hidden, "todo") : "" }; +} + function resolveTaskOrError( phases: TodoPhase[], content: string | undefined, @@ -755,6 +829,7 @@ function formatTodoLine( prefix: string, completionKeys: Set, frame: number | undefined, + matched = false, ): string { const checkbox = uiTheme.checkbox; switch (item.status) { @@ -771,7 +846,9 @@ function formatTodoLine( case "abandoned": return uiTheme.fg("error", `${prefix}${checkbox.unchecked} ${strikethroughText(item.content)}`); default: - return uiTheme.fg("dim", `${prefix}${checkbox.unchecked} ${item.content}`); + // A pending todo lit by a live subagent match renders accent, matching + // the sticky HUD's convention (#5873). + return uiTheme.fg(matched ? "accent" : "dim", `${prefix}${checkbox.unchecked} ${item.content}`); } } @@ -822,6 +899,21 @@ function formatPhaseSummary(phase: TodoPhase, oneBasedIndex: number, uiTheme: Th return `${name}${uiTheme.fg("dim", ` ${done}/${total}`)}`; } +/** + * Live subagent descriptions the transient tool result uses to detect + * pending todos being executed by an in-flight subagent, so its collapsed + * viewport surfaces the same active work the sticky HUD does (#5873). Wired + * once by interactive mode from its observer registry; returns `[]` outside an + * interactive session (tests, SDK, transcript rebuilds), where only literal + * `in_progress` counts as active. + */ +let activeTodoDescriptionsProvider: () => readonly string[] = () => []; + +/** Wire the live-subagent description source for {@link todoToolRenderer}. */ +export function setActiveTodoDescriptionsProvider(provider: () => readonly string[]): void { + activeTodoDescriptionsProvider = provider; +} + export const todoToolRenderer = { renderCall(args: TodoRenderArgs, options: RenderResultOptions, uiTheme: Theme): Component { // `args` is the raw partially-parsed JSON from the streaming tool-call @@ -903,6 +995,12 @@ export const todoToolRenderer = { // a single task flip doesn't redraw every phase's full task list. The // manual expand toggle (and the no-signal fallback) still shows all. const touched = expanded || !multiPhase ? null : computeTouchedPhases(args, phases, completedTasks); + // A pending todo counts as active work when an in-flight subagent is + // executing it — the transient result surfaces the same active set the + // sticky HUD does (#5873). Empty outside an interactive session. + const activeDescs = expanded ? [] : activeTodoDescriptionsProvider(); + const isMatched = (task: TodoItem): boolean => + activeDescs.length > 0 && todoMatchesAnyDescription(task.content, activeDescs); const bodyLines: string[] = []; for (let p = 0; p < phases.length; p++) { const phase = phases[p]; @@ -914,17 +1012,32 @@ export const todoToolRenderer = { bodyLines.push(uiTheme.fg("accent", chalk.bold(formatPhaseDisplayName(phase.name, p + 1)))); } const completionKeys = completionKeysByPhase.get(phase.name) ?? EMPTY_COMPLETION_KEYS; - const treeLines = renderTreeList( - { - items: phase.tasks, - expanded, - maxCollapsed: PREVIEW_LIMITS.COLLAPSED_ITEMS, - itemType: "todo", - truncateFrom: "start", - renderItem: todo => formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame), - }, - uiTheme, - ); + // Collapsed: walking viewport — completed/abandoned omitted, active + // work (in-progress / subagent-matched) pulled to the head, then + // following pending tasks (#5873). Expanded: every task in order. + const treeLines = expanded + ? renderTreeList( + { + items: phase.tasks, + expanded, + itemType: "todo", + renderItem: todo => formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame), + }, + uiTheme, + ) + : (() => { + const selection = selectCollapsedTodos(phase.tasks, isMatched, PREVIEW_LIMITS.COLLAPSED_ITEMS); + return renderTreeList( + { + items: selection.items, + itemType: "todo", + trailingSummary: selection.summary, + renderItem: todo => + formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame, isMatched(todo)), + }, + uiTheme, + ); + })(); for (const line of treeLines) { bodyLines.push(`${indent}${line}`); } diff --git a/packages/coding-agent/src/tui/tree-list.ts b/packages/coding-agent/src/tui/tree-list.ts index 6c00c2c39..f182e1303 100644 --- a/packages/coding-agent/src/tui/tree-list.ts +++ b/packages/coding-agent/src/tui/tree-list.ts @@ -18,6 +18,13 @@ export interface TreeListOptions { maxCollapsedLines?: number; itemType?: string; truncateFrom?: "start" | "end"; + /** Caller-supplied trailing summary line. When set (and not expanded), + * `renderTreeList` renders exactly the provided `items` (the caller has + * already applied its own selection/cap) and appends this text as the + * final `└` row, with the last item using `├`. Empty string renders the + * items with no summary. Bypasses the built-in truncation/`maxCollapsed` + * path. */ + trailingSummary?: string; /** Called once per item with `isLast: false` during budget calculation; * line count MUST NOT vary based on `isLast`. */ renderItem: (item: T, context: TreeContext) => string | string[]; @@ -36,6 +43,38 @@ export function renderTreeList(options: TreeListOptions, theme: Theme): st const maxItems = expanded ? items.length : Math.min(items.length, maxCollapsed); const linesBudget = !expanded && maxCollapsedLines !== undefined ? maxCollapsedLines : Infinity; + // Caller-driven collapse: render exactly the provided items (the caller + // already picked/capped them) plus an optional trailing summary row. The + // walking-viewport todo policy uses this so item selection lives in the + // todo domain, not here. + if (!expanded && options.trailingSummary !== undefined) { + const summary = options.trailingSummary; + const lines: string[] = []; + for (let i = 0; i < items.length; i++) { + const rendered = renderItem(items[i], { + index: i, + isLast: false, + depth: 0, + theme, + prefix: "", + continuePrefix: "", + }); + const itemLines = Array.isArray(rendered) ? rendered : rendered ? [rendered] : []; + if (itemLines.length === 0) continue; + const isLast = summary === "" && i === items.length - 1; + const prefix = `${theme.fg("dim", getTreeBranch(isLast, theme))} `; + const continuePrefix = `${theme.fg("dim", getTreeContinuePrefix(isLast, theme))}`; + lines.push(`${prefix}${replaceTabs(itemLines[0]!)}`); + for (let j = 1; j < itemLines.length; j++) { + lines.push(`${continuePrefix}${replaceTabs(itemLines[j]!)}`); + } + } + if (summary !== "") { + lines.push(`${theme.fg("dim", theme.tree.last)} ${theme.fg("muted", summary)}`); + } + return lines; + } + const candidateIndices: number[] = []; if (truncateFrom === "start") { const startCandidateIdx = Math.max(0, items.length - maxItems); diff --git a/packages/coding-agent/test/tools/todo.test.ts b/packages/coding-agent/test/tools/todo.test.ts index f593d1e8b..283ebc05a 100644 --- a/packages/coding-agent/test/tools/todo.test.ts +++ b/packages/coding-agent/test/tools/todo.test.ts @@ -6,7 +6,9 @@ import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { nextActionableTask, resolveTodoMarkdownPath, + selectCollapsedTodos, TODO_STRIKE_HOLD_FRAMES, + type TodoItem, type TodoPhase, TodoTool, todoMatchesAnyDescription, @@ -420,8 +422,9 @@ describe("todoToolRenderer.renderResult phase collapsing", () => { task: "a1", }); const rendered = Bun.stripANSI(component.render(100).join("\n")); - // Active phase renders its full task list. - expect(rendered).toContain("a1"); + // Active phase's collapsed viewport omits the completed task and shows the + // promoted current one (#5873). + expect(rendered).not.toContain("a1"); expect(rendered).toContain("a2"); // Untouched phases collapse: headers + progress counts, no task contents. expect(rendered).toContain("II. Beta"); @@ -465,6 +468,88 @@ describe("todoToolRenderer.renderResult phase collapsing", () => { }); }); +describe("selectCollapsedTodos walking viewport (#5873)", () => { + const mk = (n: number, inProgress: number[]): TodoItem[] => + Array.from({ length: n }, (_, i) => ({ + content: `Task ${i + 1}`, + status: inProgress.includes(i + 1) ? "in_progress" : "pending", + })); + const never = () => false; + const contents = (sel: { items: TodoItem[] }) => sel.items.map(t => t.content); + + it("starts at the sole in-progress task and fills with following tasks", () => { + const sel = selectCollapsedTodos(mk(14, [6]), never, 8); + expect(contents(sel)).toEqual([ + "Task 6", + "Task 7", + "Task 8", + "Task 9", + "Task 10", + "Task 11", + "Task 12", + "Task 13", + ]); + expect(sel.summary).toContain("6 more todos"); + }); + + it("omits completed and abandoned tasks in collapsed mode", () => { + const tasks: TodoItem[] = [ + { content: "done", status: "completed" }, + { content: "dropped", status: "abandoned" }, + { content: "current", status: "in_progress" }, + { content: "next", status: "pending" }, + ]; + const sel = selectCollapsedTodos(tasks, never, 5); + expect(contents(sel)).toEqual(["current", "next"]); + expect(sel.summary).toBe(""); + }); + + it("places every subagent-matched todo at the head in todo order", () => { + const tasks = mk(14, []); // all pending + const matched = (t: TodoItem) => t.content === "Task 3" || t.content === "Task 9"; + const sel = selectCollapsedTodos(tasks, matched, 5); + // Both matched actives lead, then following pending fill from the first active. + expect(contents(sel).slice(0, 2)).toEqual(["Task 3", "Task 9"]); + expect(contents(sel)).toHaveLength(5); + }); + + it("caps active todos and counts the hidden actives in the summary", () => { + const tasks = mk(10, []); + const matched = (t: TodoItem) => + ["Task 1", "Task 2", "Task 3", "Task 4", "Task 5", "Task 6", "Task 7"].includes(t.content); + const sel = selectCollapsedTodos(tasks, matched, 5); + expect(contents(sel)).toEqual(["Task 1", "Task 2", "Task 3", "Task 4", "Task 5"]); + expect(sel.summary).toBe("… 2 more active todos"); + // No unrelated pending rows leak in. + expect(contents(sel).some(c => ["Task 8", "Task 9", "Task 10"].includes(c))).toBe(false); + }); + + it("keeps a summary when actives exactly fill the cap but pending remains", () => { + // 5 matched actives + 1 trailing pending, cap 5. The active-overflow branch + // must NOT swallow the hidden pending work with an empty summary (#5878). + const tasks = mk(6, []); + const matched = (t: TodoItem) => ["Task 1", "Task 2", "Task 3", "Task 4", "Task 5"].includes(t.content); + const sel = selectCollapsedTodos(tasks, matched, 5); + expect(contents(sel)).toEqual(["Task 1", "Task 2", "Task 3", "Task 4", "Task 5"]); + expect(sel.summary).toBe("… 1 more todo"); + }); + + it("returns the whole open set with no summary when it fits", () => { + const sel = selectCollapsedTodos(mk(3, [2]), never, 5); + expect(contents(sel)).toEqual(["Task 1", "Task 2", "Task 3"]); + expect(sel.summary).toBe(""); + }); + + it("falls back to closed tasks when the phase has no open work", () => { + const tasks: TodoItem[] = [ + { content: "done a", status: "completed" }, + { content: "done b", status: "completed" }, + ]; + const sel = selectCollapsedTodos(tasks, never, 5); + expect(contents(sel)).toEqual(["done a", "done b"]); + }); +}); + describe("todoToolRenderer.renderCall malformed-args regression (#2005)", () => { // Reporter saw `TypeError: args?.ops?.map is not a function` against // Xiaomi Token Plan's Anthropic protocol because `parseStreamingJson`