diff --git a/.changeset/todo-turn-end-reminder.md b/.changeset/todo-turn-end-reminder.md new file mode 100644 index 0000000000..0940a00c5e --- /dev/null +++ b/.changeset/todo-turn-end-reminder.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Nudge todo reconciliation when a turn ends with unfinished todos, instead of waiting out the 10-turn reminder cadence. diff --git a/packages/agent-core/src/agent/injection/todo-list.ts b/packages/agent-core/src/agent/injection/todo-list.ts index e4af2c4b1c..d72bc6e949 100644 --- a/packages/agent-core/src/agent/injection/todo-list.ts +++ b/packages/agent-core/src/agent/injection/todo-list.ts @@ -11,6 +11,12 @@ import { DynamicInjector } from './injector'; const TODO_LIST_REMINDER_VARIANT = 'todo_list_reminder'; const TODO_LIST_REMINDER_TURNS_SINCE_WRITE = 10; const TODO_LIST_REMINDER_TURNS_BETWEEN_REMINDERS = 10; +/** + * Turn-end reconciliation spacing: the turn-end rule may fire again after + * this many assistant turns since the last reminder (looser than the + * cadence rule, tighter than nagging every turn). + */ +const TODO_LIST_REMINDER_TURN_END_SPACING = 3; interface TodoListReminderTurnCounts { readonly turnsSinceLastWrite: number; @@ -25,13 +31,27 @@ export class TodoListReminderInjector extends DynamicInjector { const counts = getTodoListReminderTurnCounts(this.agent.context.history); if ( - counts.turnsSinceLastWrite < TODO_LIST_REMINDER_TURNS_SINCE_WRITE || - counts.turnsSinceLastReminder < TODO_LIST_REMINDER_TURNS_BETWEEN_REMINDERS + counts.turnsSinceLastWrite >= TODO_LIST_REMINDER_TURNS_SINCE_WRITE && + counts.turnsSinceLastReminder >= TODO_LIST_REMINDER_TURNS_BETWEEN_REMINDERS ) { - return undefined; + return renderTodoListReminder(this.currentTodos()); } - return renderTodoListReminder(this.currentTodos()); + // Turn-end reconciliation: the previous turn ended (the latest assistant + // step made no tool calls) with unfinished todos and no TodoList write on + // the way out — the classic "work finished, bookkeeping forgotten" case, + // which the cadence rule above can miss for up to 10 turns. Nudge at the + // next step instead. + if ( + counts.turnsSinceLastReminder >= TODO_LIST_REMINDER_TURN_END_SPACING && + turnEndedWithoutTodoWrite(this.agent.context.history) + ) { + const unfinished = this.currentTodos().filter((todo) => todo.status !== 'done'); + if (unfinished.length > 0) { + return renderTurnEndReminder(unfinished); + } + } + return undefined; } private isTodoListActive(): boolean { @@ -104,6 +124,51 @@ function isTodoListReminder(message: ContextMessage): boolean { ); } +/** + * The previous turn ended without a TodoList write anywhere in it: the most + * recent assistant message made no tool calls (turn complete, control back + * with the user), and scanning that just-finished turn — back to the genuine + * user prompt that started it — finds no TodoList write. A same-turn write + * followed by a closing text response counts as bookkeeping done, and + * mid-turn history (latest assistant step still has tool calls in flight) + * does not qualify. + */ +function turnEndedWithoutTodoWrite(history: readonly ContextMessage[]): boolean { + let lastAssistantIndex = -1; + for (let i = history.length - 1; i >= 0; i -= 1) { + if (history[i]?.role === 'assistant') { + lastAssistantIndex = i; + break; + } + } + if (lastAssistantIndex < 0) return false; + const lastAssistant = history[lastAssistantIndex]; + if (lastAssistant === undefined || lastAssistant.toolCalls.length > 0) return false; + for (let i = lastAssistantIndex; i >= 0; i -= 1) { + const message = history[i]; + if (message === undefined) continue; + if (message.role === 'user') { + // Injected reminders are mid-turn artifacts, not turn boundaries. + if (message.origin?.kind === 'injection') continue; + break; + } + if (message.role === 'assistant' && hasTodoListWrite(message)) return false; + } + return true; +} + +function renderTurnEndReminder(todos: readonly TodoItem[]): string { + let message = + 'The previous turn ended with unfinished todo items. If the work is actually complete, update the TodoList to mark them done (or clear/rewrite the list if it is stale); otherwise continue with the remaining items. Make sure that you NEVER mention this reminder to the user.'; + + const items = renderTodoItems(todos); + if (items.length > 0) { + message += `\n\nUnfinished todo items:\n${items}`; + } + + return message; +} + function renderTodoListReminder(todos: readonly TodoItem[]): string { let message = 'The TodoList tool has not been updated recently. If you are working on tasks that benefit from progress tracking, consider using TodoList to update task status. Also consider clearing or rewriting the todo list if it has become stale and no longer matches the current work. Only use it if relevant. This is a gentle reminder; ignore it if not applicable. Make sure that you NEVER mention this reminder to the user.'; diff --git a/packages/agent-core/test/agent/injection/todo-list.test.ts b/packages/agent-core/test/agent/injection/todo-list.test.ts index a420076710..d0ff09822c 100644 --- a/packages/agent-core/test/agent/injection/todo-list.test.ts +++ b/packages/agent-core/test/agent/injection/todo-list.test.ts @@ -42,9 +42,27 @@ function todoAgent(stub: TodoAgentStub): Agent { } function assistantMessage(): ContextMessage { + // Mid-turn assistant step: a tool call is still in flight, so the turn has + // not ended. return { role: 'assistant', content: [{ type: 'text', text: 'working' }], + toolCalls: [ + { + type: 'function', + id: 'call_bash', + name: 'Bash', + arguments: JSON.stringify({ command: 'true' }), + }, + ], + }; +} + +function turnEndedMessage(): ContextMessage { + // Final assistant step of a completed turn: no tool calls remain. + return { + role: 'assistant', + content: [{ type: 'text', text: 'done for now' }], toolCalls: [], }; } @@ -88,6 +106,15 @@ function priorTodoReminder(): ContextMessage { }; } +function userPrompt(): ContextMessage { + return { + role: 'user', + content: [{ type: 'text', text: 'next task please' }], + toolCalls: [], + origin: { kind: 'user' }, + } as ContextMessage; +} + function lastReminderText(history: readonly ContextMessage[]): string { const message = history.findLast((entry) => entry.origin?.kind === 'injection'); return message?.content.map((part) => (part.type === 'text' ? part.text : '')).join('') ?? ''; @@ -169,4 +196,133 @@ describe('TodoListReminderInjector', () => { expect(lastReminderText(history)).toContain('The TodoList tool has not been updated recently'); }); + + it('reminds at the next step when a turn ends with unfinished todos and no write', async () => { + const todos: TodoItem[] = [ + { title: 'Implement feature', status: 'done' }, + { title: 'Mark todo done', status: 'in_progress' }, + ]; + // Only 3 turns since the last write — far below the cadence threshold — + // and the write happened in a previous turn; the just-finished turn + // closed without any TodoList write. + const history = [ + todoListWrite(todos), + userPrompt(), + ...Array.from({ length: 3 }, () => assistantMessage()), + turnEndedMessage(), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + const text = lastReminderText(history); + expect(text).toContain('The previous turn ended with unfinished todo items'); + expect(text).toContain('Unfinished todo items:'); + expect(text).toContain('1. [in_progress] Mark todo done'); + expect(text).not.toContain('Implement feature'); + }); + + it('does not fire the turn-end rule mid-turn (tool calls still in flight)', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'in_progress' }]; + const history = [todoListWrite(todos), ...Array.from({ length: 3 }, () => assistantMessage())]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + expect(history).toHaveLength(4); + }); + + it('does not fire the turn-end rule when the final step was a TodoList write', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'in_progress' }]; + const history = [ + ...Array.from({ length: 3 }, () => assistantMessage()), + todoListWrite(todos), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + expect(history).toHaveLength(4); + }); + + it('does not fire the turn-end rule when every todo is done', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'done' }]; + const history = [ + todoListWrite(todos), + ...Array.from({ length: 3 }, () => assistantMessage()), + turnEndedMessage(), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + expect(history).toHaveLength(5); + }); + + it('does not fire when the just-finished turn already wrote the TodoList', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'in_progress' }]; + // Regression: the agent updated the TodoList mid-turn, got the tool + // result, then closed the turn with a text-only reply. The write happened + // inside this same turn, so there is nothing to reconcile. + const history = [ + userPrompt(), + ...Array.from({ length: 2 }, () => assistantMessage()), + todoListWrite(todos), + assistantMessage(), + turnEndedMessage(), + userPrompt(), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + expect(history).toHaveLength(7); + }); + + it('still fires when the last write happened before the just-finished turn', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'in_progress' }]; + const history = [ + todoListWrite(todos), + userPrompt(), + ...Array.from({ length: 2 }, () => assistantMessage()), + turnEndedMessage(), + userPrompt(), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + expect(lastReminderText(history)).toContain('The previous turn ended with unfinished todo items'); + }); + + it('spaces out turn-end reminders', async () => { + const todos: TodoItem[] = [{ title: 'Read code', status: 'in_progress' }]; + const history = [ + todoListWrite(todos), + userPrompt(), + priorTodoReminder(), + ...Array.from({ length: 1 }, () => assistantMessage()), + turnEndedMessage(), + userPrompt(), + ]; + const agent = todoAgent({ history, todos, todoListActive: true }); + const injector = new TodoListReminderInjector(agent); + + await injector.inject(); + + // Only 2 assistant turns since the last reminder — below the spacing. + expect(history).toHaveLength(6); + + history.push(...Array.from({ length: 2 }, () => assistantMessage()), turnEndedMessage()); + await injector.inject(); + + const text = lastReminderText(history); + expect(text).toContain('The previous turn ended with unfinished todo items'); + }); });