Skip to content

fix(acp): emit plan session updates for TodoList calls on v2 engine - #2879

Open
strikegoose wants to merge 1 commit into
MoonshotAI:mainfrom
strikegoose:fix/todolist-acp-plan
Open

fix(acp): emit plan session updates for TodoList calls on v2 engine#2879
strikegoose wants to merge 1 commit into
MoonshotAI:mainfrom
strikegoose:fix/todolist-acp-plan

Conversation

@strikegoose

Copy link
Copy Markdown

fix(acp): emit ACP plan updates for TodoList calls on the v2 engine

Problem

When running kimi acp (the native ACP server on the agent-core-v2 engine, registered in apps/kimi-code/src/cli/sub/acp.ts), ACP clients never receive a plan session update, even though the agent actively maintains its todo list with the TodoList tool. Clients that render the ACP plan (e.g. Zed's plan view, Paseo's timeline todo card) stay empty for the whole session.

Reproduction (any ACP client):

  1. Connect any ACP client to kimi acp.
  2. Send a prompt that makes the agent call the TodoList tool (any multi-step task).
  3. Observe: zero plan session updates on the wire. tool_call / tool_call_update notifications for the TodoList call arrive normally.

Root cause

The ACP adapter's plan mapping is complete but unreachable:

  • packages/acp-adapter/src/events-map.ts:417 (todoListToSessionUpdate) and :466 (planFromDisplayBlock) translate a todo_list input-display block into an ACP plan notification.
  • The only emission site is packages/acp-adapter/src/session.ts:1146, gated on event.display being present on the tool.call.started event.

On the v2 engine, TodoListTool.resolveExecution() (packages/agent-core-v2/src/session/todo/tools/todo-list.ts) never attaches a display field to the returned execution, so event.display is always undefined for TodoList calls and the emission gate never passes — the plan mapping is effectively dead code. The v1 engine's TodoList tool (packages/agent-core/src/tools/builtin/state/todo-list.ts:108) does attach display: { kind: 'todo_list', items: [...] }, which is why this worked before the v2 migration.

Fix

Attach the todo_list input-display block in the v2 TodoList tool's resolveExecution(), with the exact shape planFromDisplayBlock expects ({ kind: 'todo_list', items: { title, status }[] }, packages/agent-core-v2/src/tool/toolInputDisplay.ts:52):

  • Write mode ({ todos: [...] }) → items from the requested list.
  • Query mode ({}) → items from the current stored list (ISessionTodoService.getTodos()), matching the v1 behavior.
  • Clear mode ({ todos: [] }) → empty items array.

The display flows through the existing pipeline unchanged: toolExecutorService copies execution.display onto the tool.call.started event (packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts:754), and the ACP adapter emits the plan update from there.

Why no tool.result re-emission

I evaluated also re-emitting the plan on tool.result to cover read/clear operations, and it is not necessary:

  • tool.call.started fires for all three TodoList modes (read / write / clear), and the display attached at resolveExecution() time already carries the correct items for each mode — reads included (current stored list).
  • Clear mode yields an empty items array, and todoListToSessionUpdate deliberately returns null for empty plans (events-map.ts:427 — "no useful client-side state ... deferred until kimi-code grows a clear-plan signal"). Re-emitting on tool.result would hit the same guard, so it changes nothing. Clearing the client-side plan is a separate, pre-existing gap that needs an explicit clear signal, not a second emission of the same payload.

Tests

Extended the existing tool test file packages/agent-core-v2/test/session/todo/tools/todo-list.test.ts (per repo convention, no new test file):

  • resolveExecution attaches a todo_list display block with the requested items (write mode).
  • Query mode display reflects the current stored list.
  • Clear mode display carries an empty items array.

The adapter-side mapping display → plan is already covered by packages/acp-adapter/test/plan-and-commands.test.ts (unit + e2e, including the negative case), so this completes the chain: tool display → tool.call.started → ACP plan.

Verification run locally (Node 26.7.0, pnpm 10.33.0):

  • pnpm typecheck@moonshot-ai/agent-core-v2 clean, @moonshot-ai/acp-adapter clean.
  • pnpm test@moonshot-ai/agent-core-v2: 258 files / 3919 tests passed. @moonshot-ai/acp-adapter: 36 files / 326 tests passed.

Notes

  • No ACP adapter or client changes are required; this only repairs the missing display on the engine side.
  • Includes a patch changeset for @moonshot-ai/agent-core-v2 and @moonshot-ai/kimi-code.

@changeset-bot

changeset-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c28868c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@moonshot-ai/agent-core-v2 Patch
@moonshot-ai/kimi-code Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c28868c3a4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

description,
display: {
kind: 'todo_list',
items: (args.todos ?? this.todo.getTodos()).map((todo) => ({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Emit read-mode plans after execution to avoid stale ACP state

When a single assistant response batches TodoList({ todos: ... }) followed by TodoList({}), the executor prepares all calls and publishes each tool.call.started (and therefore each ACP plan) before executeBatch runs any task, while the default ToolAccesses.all() only serializes the later executions. This read-mode fallback snapshots the pre-write list, so the later read's plan update can overwrite the correct write plan with stale entries; the read tool result will see the updated list after the write executes, but ACP never gets a result-time plan refresh. Emit the query-mode plan from the executed/current list or suppress the started-time display for reads.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant