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

fix(todo): signal removal intent so agent stops rebuilding cleared todos

Browse Source 
The /todo rm reminder was a generic "user manually modified" message
showing an empty list, so the model read the cleared list as missing and
re-populated it on the next turn. buildSystemReminder now emits an explicit
do-not-recreate / do-not-re-add directive for removals while keeping
status mutations (done/drop) neutral.

Fixes #5258
This commit is contained in:
roboomp
2026-07-14 17:33:21 +00:00
parent 20c0a2e410
commit fd3f15c915
3 changed files with 72 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed the agent rebuilding a todo list the user just cleared with `/todo rm`: the manual-edit system reminder now states removal intent ("intentionally cleared/removed … Do NOT recreate/re-add") so the model stops re-populating the list without an explicit request. ([#5258](https://github.com/can1357/oh-my-pi/issues/5258))
## [16.4.6] - 2026-07-12
### Added
packages/coding-agent/src/modes/controllers/todo-command-controller.ts
+18 -14
View File
@@ -116,16 +116,18 @@ function findTaskFuzzy(phases: TodoPhase[], query: string): { task: TodoItem; ph
// Build system reminder
// =============================================================================
function buildSystemReminder(action: string, phases: TodoPhase[]): string {
function buildSystemReminder(action: string, phases: TodoPhase[], removed = false): string {
const md = phases.length === 0 ? "(empty)" : phasesToMarkdown(phases).trimEnd();
return [
"<system-reminder>",
`The user manually modified the todo list (${action}).`,
"Current todo list:",
"",
md,
"</system-reminder>",
].join("\n");
const lines = ["<system-reminder>", `The user manually modified the todo list (${action}).`];
if (removed) {
lines.push(
phases.length === 0
? "The user intentionally cleared the todo list. Do NOT recreate or re-populate it unless the user explicitly asks; continue the current request without a todo list."
: "The user intentionally removed the entries no longer shown below. Do NOT re-add them unless the user explicitly asks.",
);
}
lines.push("Current todo list:", "", md, "</system-reminder>");
return lines.join("\n");
}
export class TodoCommandController {
@@ -366,7 +368,7 @@ export class TodoCommandController {
const current = this.#currentPhases();
const trimmed = rest.trim();
if (!trimmed) {
this.#commit([], "/todo rm (all)");
this.#commit([], "/todo rm (all)", { removed: true });
this.ctx.showStatus("Cleared all todos.");
return;
}
@@ -377,7 +379,7 @@ export class TodoCommandController {
this.ctx.showError(errors.join("; "));
return;
}
this.#commit(phases, `/todo rm ${taskHit.task.content}`);
this.#commit(phases, `/todo rm ${taskHit.task.content}`, { removed: true });
this.ctx.showStatus(`Removed: ${taskHit.task.content}`);
return;
}
@@ -388,7 +390,7 @@ export class TodoCommandController {
this.ctx.showError(errors.join("; "));
return;
}
this.#commit(phases, `/todo rm ${phaseHit.name}`);
this.#commit(phases, `/todo rm ${phaseHit.name}`, { removed: true });
this.ctx.showStatus(`Removed phase: ${phaseHit.name}`);
return;
}
@@ -454,7 +456,7 @@ export class TodoCommandController {
}
}
#commit(nextPhases: TodoPhase[], action: string): void {
#commit(nextPhases: TodoPhase[], action: string, opts?: { removed?: boolean }): void {
// 1. In-memory + UI state
this.ctx.session.setTodoPhases(nextPhases);
this.ctx.setTodos(nextPhases);
@@ -463,7 +465,9 @@ export class TodoCommandController {
this.ctx.sessionManager.appendCustomEntry(USER_TODO_EDIT_CUSTOM_TYPE, { phases: nextPhases });
// 3. Inject system reminder so the agent learns about the change next turn.
const reminderText = buildSystemReminder(action, nextPhases);
// Removals carry explicit intent so the agent does not rebuild the
// cleared/removed items on its next turn (issue #5258).
const reminderText = buildSystemReminder(action, nextPhases, opts?.removed ?? false);
const message = {
role: "developer" as const,
content: [{ type: "text" as const, text: reminderText }],
packages/coding-agent/test/modes/controllers/todo-command-controller.test.ts
+50 -1
View File
@@ -1,4 +1,4 @@
import { afterEach, describe, expect, it, vi } from "bun:test";
import { afterEach, describe, expect, it, type Mock, vi } from "bun:test";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
@@ -152,4 +152,53 @@ describe("TodoCommandController", () => {
expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("Failed to write todos:"));
expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("internal scheme"));
});
function reminderTextFrom(ctx: InteractiveModeContext): string {
const appendMessage = ctx.agent.appendMessage as unknown as Mock<(message: unknown) => void>;
const message = appendMessage.mock.calls[0][0] as { content: Array<{ text: string }> };
return message.content[0].text;
}
it("tells the model not to recreate the list after /todo rm (all)", async () => {
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-rm-all-"));
const phases: TodoPhase[] = [
{ name: "Foundation", tasks: [{ content: "Scaffold crate", status: "in_progress" }] },
];
const ctx = createContext(tempRoot, phases);
const controller = new TodoCommandController(ctx);
await controller.handleTodoCommand("rm");
expect(ctx.agent.appendMessage).toHaveBeenCalledTimes(1);
const text = reminderTextFrom(ctx);
expect(text).toContain("intentionally cleared the todo list");
expect(text).toMatch(/Do NOT recreate/i);
});
it("tells the model not to re-add a removed phase after /todo rm <phase>", async () => {
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-rm-phase-"));
const phases: TodoPhase[] = [
{ name: "Foundation", tasks: [{ content: "Scaffold crate", status: "completed" }] },
{ name: "Auth", tasks: [{ content: "Port credential store", status: "pending" }] },
];
const ctx = createContext(tempRoot, phases);
const controller = new TodoCommandController(ctx);
await controller.handleTodoCommand("rm Auth");
expect(reminderTextFrom(ctx)).toMatch(/Do NOT re-add them/i);
});
it("keeps status-mutation reminders neutral (no do-not-recreate directive)", async () => {
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pi-tui-todo-done-"));
const phases: TodoPhase[] = [
{ name: "Foundation", tasks: [{ content: "Scaffold crate", status: "in_progress" }] },
];
const ctx = createContext(tempRoot, phases);
const controller = new TodoCommandController(ctx);
await controller.handleTodoCommand("done");
expect(reminderTextFrom(ctx)).not.toMatch(/Do NOT/i);
});
});