smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): resolved duplicate todo reminders in terminal scrollback

Browse Source 
- Anchors the incomplete-todo reminder block inside the scrollback transcript instead of a floating live container.
- Eliminates duplicate reminder copies piling up in terminal scrollback during terminal reflows.
- Removes the dedicated `todoReminderContainer` and simplifies state synchronization on todo reload.
- Updates tests to verify sequential reminders commit as separate blocks and are left intact when tools succeed.
This commit is contained in:
can1357
2026-06-30 23:32:58 +02:00
parent 31d9fb1145
commit 112317bc8e
7 changed files with 24 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -14,6 +14,7 @@
- Fixed Git subcommands ignoring repository paths by stripping ambient Git environment variables
- Fixed concurrent bash commands cross-killing each other on cancel/timeout. Cancellation cleanup previously walked the whole host process tree and signalled every descendant spawned since a per-run baseline, so cancelling or timing out one command could SIGTERM an unrelated command's child still running in parallel (it looked "new" relative to the canceller's baseline). Each run now tracks only the processes it actually spawned (via a brush-core spawn-observer hook) and scopes its TERM/KILL waves to that set, leaving concurrent runs untouched.
- Fixed `/skill:<name>` invocation losing the user's prompt context when the slash token was reached mid-prompt via the autocomplete. The slash-command parser now recognizes a `/skill:<name>` token surrounded by whitespace in non-slash, non-local-execution drafts (in addition to the leading form) and threads the surrounding prose through to the skill as `args`, so the typed prompt survives both in the editor (see the TUI changelog) and in the dispatched skill message. Drafts that already begin with another slash command (`/compact /skill:foo`), a bash sigil (`!echo /skill:foo`, `!!echo /skill:foo`), or a python sigil (`$ run.py /skill:foo`, `$$ run.py /skill:foo`) keep their existing dispatcher precedence and are not hijacked by the mid-prompt skill parser. Applies to the interactive TUI, ACP, and RPC dispatch paths via the shared `parseSkillInvocation` helper in `extensibility/skills` ([#3913](https://github.com/can1357/oh-my-pi/issues/3913)).
- Fixed the incomplete-todo reminder drifting to the bottom of the screen and piling up as dozens of duplicate copies in native scrollback. The reminder rendered in a dedicated anchored live-region container (`todoReminderContainer`) pinned above the editor, so it re-rendered in place every frame and — being taller than the viewport on short terminals while the subagent/job HUD churned below it — had its top rows committed to scrollback again on each reflow. It is now committed once into the transcript as a regular block (the same path TTSR notifications use), so it stays anchored in history where it fired.
## [16.2.9] - 2026-06-30
packages/coding-agent/src/modes/components/todo-reminder.ts
+5 -1
View File
@@ -3,7 +3,9 @@ import { theme } from "../../modes/theme/theme";
import type { TodoItem } from "../../tools/todo";
/**
* Component that renders a todo completion reminder notification.
* Component that renders a todo completion reminder notification, committed into
* the transcript like a TTSR notification so it stays anchored in history rather
* than floating above the editor.
* Shows when the agent stops with incomplete todos.
*/
export class TodoReminderComponent extends Container {
@@ -16,6 +18,8 @@ export class TodoReminderComponent extends Container {
) {
super();
this.addChild(new Spacer(1));
this.#box = new Box(1, 1, t => theme.inverse(theme.fg("warning", t)));
this.#box.setIgnoreTight(true);
this.addChild(this.#box);
packages/coding-agent/src/modes/controllers/event-controller.ts
+1 -7
View File
@@ -978,13 +978,9 @@ export class EventController {
}
// Update todo display when todo tool completes
if (event.toolName === "todo" && !event.isError) {
const hadTodoReminder = (this.ctx.todoReminderContainer?.children.length ?? 0) > 0;
this.ctx.todoReminderContainer?.clear();
const details = event.result.details as { phases?: TodoPhase[] } | undefined;
if (details?.phases) {
this.ctx.setTodos(details.phases);
} else if (hadTodoReminder) {
this.ctx.ui.requestRender();
}
} else if (event.toolName === "todo" && event.isError) {
const textContent = event.result.content.find(
@@ -1281,9 +1277,7 @@ export class EventController {
async #handleTodoReminder(event: Extract<AgentSessionEvent, { type: "todo_reminder" }>): Promise<void> {
const component = new TodoReminderComponent(event.todos, event.attempt, event.maxAttempts);
this.ctx.todoReminderContainer.clear();
this.ctx.todoReminderContainer.addChild(component);
this.ctx.ui.requestRender();
this.ctx.present(component);
}
async #handleTodoAutoClear(_event: Extract<AgentSessionEvent, { type: "todo_auto_clear" }>): Promise<void> {
packages/coding-agent/src/modes/interactive-mode.ts
-6
View File
@@ -379,7 +379,6 @@ export class InteractiveMode implements InteractiveModeContext {
chatContainer: TranscriptContainer;
pendingMessagesContainer: Container;
statusContainer: Container;
todoReminderContainer: Container;
todoContainer: Container;
subagentContainer: Container;
btwContainer: Container;
@@ -548,7 +547,6 @@ export class InteractiveMode implements InteractiveModeContext {
this.retryLoader = undefined;
}
this.statusContainer.clear();
this.todoReminderContainer.clear();
this.pendingMessagesContainer.clear();
this.#cancelModelCycleClearTimer();
this.modelCycleContainer.clear();
@@ -628,7 +626,6 @@ export class InteractiveMode implements InteractiveModeContext {
this.chatContainer = new TranscriptContainer();
this.pendingMessagesContainer = new Container();
this.statusContainer = new AnchoredLiveContainer();
this.todoReminderContainer = new AnchoredLiveContainer();
this.todoContainer = new AnchoredLiveContainer();
this.subagentContainer = new AnchoredLiveContainer();
this.btwContainer = new AnchoredLiveContainer();
@@ -848,7 +845,6 @@ export class InteractiveMode implements InteractiveModeContext {
this.ui.addChild(this.chatContainer);
this.ui.addChild(this.pendingMessagesContainer);
this.ui.addChild(this.todoReminderContainer);
this.ui.addChild(this.todoContainer);
this.ui.addChild(this.subagentContainer);
this.ui.addChild(this.btwContainer);
@@ -4077,14 +4073,12 @@ export class InteractiveMode implements InteractiveModeContext {
},
];
}
this.todoReminderContainer.clear();
this.#syncTodoAutoClearTimer();
this.#renderTodoList();
this.ui.requestRender();
}
async reloadTodos(): Promise<void> {
this.todoReminderContainer.clear();
await this.#loadTodoList();
this.ui.requestRender();
}
packages/coding-agent/src/modes/types.ts
-1
View File
@@ -96,7 +96,6 @@ export interface InteractiveModeContext {
chatContainer: TranscriptContainer;
pendingMessagesContainer: Container;
statusContainer: Container;
todoReminderContainer: Container;
todoContainer: Container;
subagentContainer: Container;
btwContainer: Container;
packages/coding-agent/test/event-controller-todo-reminder.test.ts
+15 -30
View File
@@ -3,14 +3,12 @@ import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/eve
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session";
import { Container } from "@oh-my-pi/pi-tui";
beforeAll(async () => {
await initTheme(false);
});
function createContext() {
const todoReminderContainer = new Container();
const present = vi.fn();
const ctx = {
isInitialized: true,
@@ -26,11 +24,10 @@ function createContext() {
// handlers). Leaving it false matches the implicit assumption in this
// fixture: the todo HUD lifecycle is independent of the working loader.
viewSession: { isStreaming: false },
todoReminderContainer,
setTodos: vi.fn(),
present,
} as unknown as InteractiveModeContext;
return { ctx, todoReminderContainer, present };
return { ctx, present };
}
function reminder(attempt: number, content = "pending task"): Extract<AgentSessionEvent, { type: "todo_reminder" }> {
@@ -42,42 +39,28 @@ function reminder(attempt: number, content = "pending task"): Extract<AgentSessi
};
}
describe("EventController todo reminder HUD", () => {
it("renders reminders in the anchored container instead of durable chat history", async () => {
const { ctx, todoReminderContainer, present } = createContext();
describe("EventController todo reminder", () => {
it("commits each reminder into durable chat history", async () => {
const { ctx, present } = createContext();
const controller = new EventController(ctx);
await controller.handleEvent(reminder(1, "old task"));
expect(todoReminderContainer.children).toHaveLength(1);
expect(present).not.toHaveBeenCalled();
expect(present).toHaveBeenCalledTimes(1);
// A second reminder is a distinct escalation, committed as its own block —
// not merged into or replacing the first.
await controller.handleEvent(reminder(2, "new task"));
expect(todoReminderContainer.children).toHaveLength(1);
expect(present).not.toHaveBeenCalled();
expect(ctx.ui.requestRender).toHaveBeenCalled();
expect(present).toHaveBeenCalledTimes(2);
expect(present.mock.calls[0]![0]).not.toBe(present.mock.calls[1]![0]);
});
it("keeps reminders visible when an auto-continued turn starts", async () => {
const { ctx, todoReminderContainer } = createContext();
const controller = new EventController(ctx);
await controller.handleEvent(reminder(1));
const visibleReminder = todoReminderContainer.children[0];
await controller.handleEvent({ type: "agent_start" } as Extract<AgentSessionEvent, { type: "agent_start" }>);
expect(todoReminderContainer.children).toHaveLength(1);
expect(todoReminderContainer.children[0]).toBe(visibleReminder);
expect(ctx.ensureLoadingAnimation).toHaveBeenCalled();
});
it("clears reminders when a todo tool succeeds", async () => {
const { ctx, todoReminderContainer } = createContext();
it("leaves committed reminders untouched when a todo tool succeeds", async () => {
const { ctx, present } = createContext();
const controller = new EventController(ctx);
const phases = [{ name: "Implementation", tasks: [{ content: "done task", status: "completed" as const }] }];
await controller.handleEvent(reminder(1));
expect(todoReminderContainer.children).toHaveLength(1);
expect(present).toHaveBeenCalledTimes(1);
await controller.handleEvent({
type: "tool_execution_end",
@@ -87,7 +70,9 @@ describe("EventController todo reminder HUD", () => {
result: { content: [{ type: "text", text: "" }], details: { phases } },
} as Extract<AgentSessionEvent, { type: "tool_execution_end" }>);
expect(todoReminderContainer.children).toHaveLength(0);
// The reminder stays in history (no retroactive removal); only the sticky
// HUD updates via setTodos.
expect(present).toHaveBeenCalledTimes(1);
expect(ctx.setTodos).toHaveBeenCalledWith(phases);
});
});
packages/coding-agent/test/interactive-mode-todo-clear.test.ts
+2 -38
View File
@@ -11,7 +11,7 @@ import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manage
import { TASK_SUBAGENT_LIFECYCLE_CHANNEL } from "@oh-my-pi/pi-coding-agent/task";
import type { TodoPhase } from "@oh-my-pi/pi-coding-agent/tools/todo";
import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus";
import { type NativeScrollbackLiveRegion, Text } from "@oh-my-pi/pi-tui";
import type { NativeScrollbackLiveRegion } from "@oh-my-pi/pi-tui";
import { TempDir } from "@oh-my-pi/pi-utils";
function renderTodos(mode: InteractiveMode): string {
@@ -125,48 +125,13 @@ describe("InteractiveMode todo HUD persistence", () => {
expect(liveRegion.getNativeScrollbackLiveRegionStart?.()).toBeUndefined();
});
it("keeps the todo reminder panel in the live region while visible", async () => {
await createMode(-1);
const liveRegion = mode.todoReminderContainer as unknown as NativeScrollbackLiveRegion;
expect(liveRegion.getNativeScrollbackLiveRegionStart?.()).toBeUndefined();
mode.todoReminderContainer.addChild(new Text("todo reminder", 0, 0));
expect(liveRegion.getNativeScrollbackLiveRegionStart?.()).toBe(0);
mode.todoReminderContainer.clear();
expect(liveRegion.getNativeScrollbackLiveRegionStart?.()).toBeUndefined();
});
it("clears stale todo reminders when todo state reloads", async () => {
await createMode(-1);
mode.todoReminderContainer.addChild(new Text("stale reminder", 0, 0));
session.setTodoPhases([{ name: "Implementation", tasks: [{ content: "current task", status: "pending" }] }]);
await mode.reloadTodos();
expect(mode.todoReminderContainer.children).toHaveLength(0);
expect(renderTodos(mode)).toContain("current task");
});
it("clears stale todo reminders when direct todo state updates", async () => {
await createMode(-1);
mode.todoReminderContainer.addChild(new Text("stale reminder", 0, 0));
mode.setTodos([{ name: "Implementation", tasks: [{ content: "current task", status: "pending" }] }]);
expect(mode.todoReminderContainer.children).toHaveLength(0);
expect(renderTodos(mode)).toContain("current task");
});
it("clears stale todo reminders when subagent reconciliation updates todo state", async () => {
it("marks todos complete when subagent reconciliation reports a finished agent", async () => {
await createMode(-1);
vi.spyOn(mode.statusLine, "watchBranch").mockImplementation(() => {});
session.setTodoPhases([
{ name: "Implementation", tasks: [{ content: "Fix review comments", status: "pending" }] },
]);
mode.setTodos(session.getTodoPhases());
mode.todoReminderContainer.addChild(new Text("stale reminder", 0, 0));
await mode.init();
eventBus.emit(TASK_SUBAGENT_LIFECYCLE_CHANNEL, {
@@ -178,7 +143,6 @@ describe("InteractiveMode todo HUD persistence", () => {
detached: true,
});
expect(mode.todoReminderContainer.children).toHaveLength(0);
expect(session.getTodoPhases()[0]?.tasks[0]?.status).toBe("completed");
});
});