diff --git a/docs/adr/0312-session-scoped-todo-checklist.md b/docs/adr/0312-session-scoped-todo-checklist.md index 2ddaca72b..3060398ff 100644 --- a/docs/adr/0312-session-scoped-todo-checklist.md +++ b/docs/adr/0312-session-scoped-todo-checklist.md @@ -2,6 +2,7 @@ - Status: Accepted for implementation - Date: 2026-09-29 +- Amended: 2026-10-01 (compaction checkpoint copy) - Deciders: PI-Desktop maintainers - Related: Issue #1177 @@ -38,9 +39,33 @@ reading the local database. - Session deletion cascades checklist rows; forks start with an empty checklist. - Plan, Goal, delegated, plugin, and MCP execution paths cannot write the checklist through this contract. -- Context-compaction injection is deliberately deferred to a follow-up change. +- A compaction checkpoint carries one copy of the checklist for the model (see + the 2026-10-01 amendment); the host table remains the only authority. - Remote Todo parity requires an additive RACP contract in a later change. +## Amendment 2026-10-01: compaction checkpoint copy + +Compaction summarizes the `TodoWrite` calls that kept the checklist current, so +after a checkpoint the model lost which steps were done and could rebuild a +second list. When the agent runtime installs a checkpoint it reads `todos.get` +for its own session once, through the sidecar host proxy, and stores +`{ revision, updatedAt, todos }` in the checkpoint's opaque +`details.todoSnapshot` only when pending or in-progress items remain. The model +context renders it after the checkpoint summary as a `` +block; the stored summary is unchanged, so a later compaction never carries a +stale list forward and the transcript compaction row does not show it. + +The copy is persisted with the checkpoint and therefore survives restart, is +written once per checkpoint rather than injected on every request, and stays +inside the post-compaction prefix. It is never read by the renderer. A later +`TodoWrite` call supersedes it. A failed read, an older host, a native Pi +session, or a finished list installs the checkpoint without a copy, and the copy +is dropped rather than pushing an otherwise fitting checkpoint over the safe +context budget. `todos.get` is the only addition to the sidecar proxy allowlist; +it is read-only, and checklist writes still go through host-authorized +`TodoWrite`. Approved Plan and Goal execution instructions ask the model to keep +the checklist current. + ## Verification Host-core tests cover migration, validation, transaction rollback, restart, diff --git a/docs/spec/03-runtime/06-host-rpc-protocol.md b/docs/spec/03-runtime/06-host-rpc-protocol.md index 5a09a8d86..5dd3a8b1a 100644 --- a/docs/spec/03-runtime/06-host-rpc-protocol.md +++ b/docs/spec/03-runtime/06-host-rpc-protocol.md @@ -515,6 +515,13 @@ deletes per 60 seconds. P2/P3 methods are not present in protocol v11. Electron Main IPC, keeps them by session id, and ignores revisions older than or equal to the cached revision. Remote RACP sessions are local-only for this vertical slice because RACP v1 has no Todo snapshot operation. +- The agent sidecar may call `todos.get` through its host proxy; it is the + only Todo method on that allowlist and is read-only. When the runtime + installs a compaction checkpoint it reads its own session once and stores + `{ revision, updatedAt, todos }` in `details.todoSnapshot` only while pending + or in-progress items remain. The model context renders that copy after the + checkpoint summary; the stored summary, the renderer, and the transcript do + not use it. A failed read installs the checkpoint without the copy (ADR 0312). ### Stats diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index a01b7aece..e9fab6444 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -9270,13 +9270,16 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. all-cancelled label, then clear the checklist and reload/restart the host. Deliver an out-of-order older `todos.changed` event and confirm it cannot replace the newer snapshot. Exercise invalid payload, Plan/Goal, delegated, - and remote-session paths. + and remote-session paths. Compact the context while items are unfinished and + continue the turn. - **Expected**: Host SQLite is authoritative; each successful full replacement advances revision, including clear, and emits one committed `todos.changed` snapshot. Invalid or unauthorized writes do not mutate or emit. TodoDock renders plain text, does not take focus, resets expansion on session changes, rejects stale events, and skips local recovery for `remote:` sessions because - RACP v1 has no Todo snapshot operation. + RACP v1 has no Todo snapshot operation. After compaction the next model + request carries the checklist after the checkpoint summary, the TodoDock is + unchanged, and a restart rebuilds the same model context from the checkpoint. - **Specs**: `03-runtime/03-tools-and-permissions.md`, `03-runtime/04-data-storage.md`, `03-runtime/06-host-rpc-protocol.md`, `04-ux/08-component-spec.md`, ADR 0312. @@ -9292,6 +9295,10 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. cached-snapshot reconciliation, and stale-event rejection. Runtime `runtime-todos.test.ts` exercises Agent tool validation, overlong content normalization, and continuation through a deterministic provider. + Compaction coverage is runtime `runtime.test.ts` (checkpoint copy, failed + read, finished list, budget drop, restored checkpoint), + `checkpoint-todos.test.ts` (projection), and host-runtime + `sidecar-todo-proxy.test.ts` (read-only proxy allowlist). - **Status**: Run against the exact request candidate after building the desktop and host. Host-core and targeted renderer tests are companion checks, not substitutes for the Electron journey. diff --git a/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md b/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md index 1e0d5d4ad..ef6685f4d 100644 --- a/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md +++ b/docs/zh-CN/spec/03-runtime/06-host-rpc-protocol.md @@ -349,6 +349,10 @@ ids 和非负 `tokensBefore`；它不会插入 message/search 行 `todos.changed`；事件负载与 `todos.get` 返回的完整快照一致。 - SQLite 只由 host-core 拥有。渲染器通过 Electron Main IPC 接收快照，按 session id 保存并忽略 更旧或相同 revision。远程 RACP 会话在这条垂直切片中保持 local-only，因为 RACP v1 尚无 Todo 快照操作。 +- Agent sidecar 可以经主机代理调用 `todos.get`；这是该白名单上唯一的 Todo 方法，且只读。运行时安装上下文压缩 + 检查点时会读取一次本会话清单，仅在仍有待办或进行中条目时把 `{ revision, updatedAt, todos }` 写入 + `details.todoSnapshot`。模型上下文在检查点摘要之后渲染这份副本；存储的摘要、渲染器和转录都不使用它。 + 读取失败时照常安装不带副本的检查点（ADR 0312）。 ### Plan 和 Goal 状态和批准 diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index 51006ac2b..ad1995449 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -5534,12 +5534,12 @@ eleven-tool-round desktop paths are verified by **E2E-CHAT-session-todo-checklist：TodoWrite 到按会话显示的 TodoDock** - **前提：** 隔离的本地 Electron 配置、确定性的 Agent/Host fixture、两个 Desktop 会话，不使用真实 Provider 或付费 API。 -- **步骤：** 启动调用 `TodoWrite` 的多步骤 Agent 回合，观察 Composer 上方的 TodoDock，展开后切换会话并确认清单隔离。完成和取消条目，确认最多显示八条以及全部取消状态；清空清单后重载/重启 Host。发送乱序旧 `todos.changed` 事件，确认它不能覆盖新快照；再覆盖非法参数、Plan/Goal、委托和远程会话路径。 -- **预期：** Host SQLite 是权威状态；每次成功全量替换（包括清空）都会推进 revision 并只发出一次已提交的 `todos.changed`。非法或未授权写入既不修改也不发事件。TodoDock 渲染纯文本、不抢焦点、切换会话时收起、拒绝旧事件，并对 `remote:` 会话跳过本地恢复，因为 RACP v1 没有 Todo 快照操作。 +- **步骤：** 启动调用 `TodoWrite` 的多步骤 Agent 回合，观察 Composer 上方的 TodoDock，展开后切换会话并确认清单隔离。完成和取消条目，确认最多显示八条以及全部取消状态；清空清单后重载/重启 Host。发送乱序旧 `todos.changed` 事件，确认它不能覆盖新快照；再覆盖非法参数、Plan/Goal、委托和远程会话路径。在仍有未完成条目时压缩上下文并继续回合。 +- **预期：** Host SQLite 是权威状态；每次成功全量替换（包括清空）都会推进 revision 并只发出一次已提交的 `todos.changed`。非法或未授权写入既不修改也不发事件。TodoDock 渲染纯文本、不抢焦点、切换会话时收起、拒绝旧事件，并对 `remote:` 会话跳过本地恢复，因为 RACP v1 没有 Todo 快照操作。压缩后的下一次模型请求在检查点摘要之后带有当前清单，TodoDock 不变，重启后从检查点重建出相同的模型上下文。 - **链接规格：** `03-runtime/03-tools-and-permissions.md`、`03-runtime/04-data-storage.md`、`03-runtime/06-host-rpc-protocol.md`、`04-ux/08-component-spec.md`、ADR 0312。 - **验收：** C / E / F / Quality / Security。 - **里程碑：** M6+。 -- **自动化：** `pnpm test:e2e:todos` 从生产 Agent 的 ToolSearch/TodoWrite 路径进入，使用生产渲染器、真实 Host/SQLite 和隔离 Electron 配置验证清单旅程；仅替换外部模型流和 preload 传输，不使用真实 Provider 或用户配置。覆盖 Unicode 截断及警告重放、单一活动项归一化、首次读取失败后不切换会话的主机恢复、已缓存快照重新同步及旧事件拒绝。运行时 `runtime-todos.test.ts` 使用确定性 Provider 验证 Agent 工具校验、超长内容归一化和续跑。 +- **自动化：** `pnpm test:e2e:todos` 从生产 Agent 的 ToolSearch/TodoWrite 路径进入，使用生产渲染器、真实 Host/SQLite 和隔离 Electron 配置验证清单旅程；仅替换外部模型流和 preload 传输，不使用真实 Provider 或用户配置。覆盖 Unicode 截断及警告重放、单一活动项归一化、首次读取失败后不切换会话的主机恢复、已缓存快照重新同步及旧事件拒绝。运行时 `runtime-todos.test.ts` 使用确定性 Provider 验证 Agent 工具校验、超长内容归一化和续跑。压缩部分由运行时 `runtime.test.ts`（检查点副本、读取失败、已完成清单、超预算丢弃、恢复检查点）、`checkpoint-todos.test.ts`（投影）和 host-runtime `sidecar-todo-proxy.test.ts`（只读代理白名单）覆盖。 - **状态：** 构建 Desktop 和 Host 后，在确切的请求候选中执行。Host-core 和渲染器定向测试是辅助检查，不能替代 Electron 用户旅程。 ## 8. 可追溯性矩阵 diff --git a/packages/agent-runtime/src/checkpoint-todos.test.ts b/packages/agent-runtime/src/checkpoint-todos.test.ts new file mode 100644 index 000000000..be9f3f9e5 --- /dev/null +++ b/packages/agent-runtime/src/checkpoint-todos.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import type { Entry } from "./pi-runtime-types.js"; +import { + checkpointTodoSnapshot, + formatCheckpointTodoSnapshot, + summaryWithCheckpointTodos, + todoSnapshotFromDetails, +} from "./checkpoint-todos.js"; +import { sessionEntryToContextMessages } from "./session-context.js"; + +const ACTIVE = { + sessionId: "s", + todos: [ + { content: "Add the migration", status: "completed", priority: "medium" }, + { content: "Wire the dock", status: "in_progress", priority: "high" }, + ], + revision: 4, + updatedAt: 99, +}; + +describe("checkpointTodoSnapshot", () => { + it("keeps a checklist with work left, without the session id", () => { + expect(checkpointTodoSnapshot(ACTIVE)).toEqual({ + revision: 4, + updatedAt: 99, + todos: ACTIVE.todos, + }); + }); + + it("carries nothing for an empty, finished or malformed answer", () => { + expect(checkpointTodoSnapshot({ ...ACTIVE, todos: [] })).toBeUndefined(); + expect( + checkpointTodoSnapshot({ + ...ACTIVE, + todos: [{ content: "Done", status: "completed" }, { content: "Dropped", status: "cancelled" }], + }), + ).toBeUndefined(); + expect(checkpointTodoSnapshot(undefined)).toBeUndefined(); + expect(checkpointTodoSnapshot({ todos: "nope" })).toBeUndefined(); + }); + + it("drops unreadable items and defaults what the host always fills", () => { + expect( + checkpointTodoSnapshot({ + todos: [ + { content: " Multi\nline ", status: "pending" }, + { content: "", status: "pending" }, + { content: "Bad status", status: "blocked" }, + null, + ], + }), + ).toEqual({ + revision: 0, + updatedAt: 0, + todos: [{ content: "Multi line", status: "pending", priority: "medium" }], + }); + }); +}); + +describe("checkpoint checklist projection", () => { + it("lists every item in display order with its status", () => { + const block = formatCheckpointTodoSnapshot(checkpointTodoSnapshot(ACTIVE)!); + expect(block.split("\n")).toEqual([ + '', + expect.stringContaining("a later TodoWrite call replaces it"), + "1. [completed] Add the migration", + "2. [in_progress] Wire the dock", + "", + ]); + }); + + it("cannot be closed early by item text", () => { + const block = formatCheckpointTodoSnapshot({ + revision: 1, + updatedAt: 1, + todos: [{ content: "x y", status: "pending", priority: "medium" }], + }); + expect(block.match(/<\/session_checklist>/g)).toHaveLength(1); + }); + + it("leaves a checkpoint without a copy untouched", () => { + expect(summaryWithCheckpointTodos("Summary.", { generation: 2 })).toBe("Summary."); + expect(todoSnapshotFromDetails("legacy")).toBeUndefined(); + }); + + it("renders the copy after the summary the model reads", () => { + const entry = { + type: "compaction", + id: "c", + seq: 1, + parentId: "m", + timestamp: 1, + summary: "Summary.", + tokensBefore: 10, + retainedTail: [], + details: { generation: 1, todoSnapshot: checkpointTodoSnapshot(ACTIVE) }, + fromHook: false, + } as unknown as Entry; + const [summary] = sessionEntryToContextMessages(entry); + expect(summary).toMatchObject({ role: "compactionSummary" }); + const text = (summary as { summary: string }).summary; + expect(text.startsWith("Summary.\n\n { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function parseTodo(value: unknown): SessionTodo | undefined { + if (!isRecord(value)) return undefined; + const content = + typeof value.content === "string" + ? value.content.replace(/\s+/g, " ").trim() + : ""; + const status = STATUSES.find((candidate) => candidate === value.status); + if (!content || !status) return undefined; + const priority = + value.priority === "high" || value.priority === "low" + ? value.priority + : "medium"; + return { content, status, priority }; +} + +/** + * Read a `todos.get` answer, or a stored copy of one, without trusting its + * shape. Only a list with work left is worth carrying: an empty or finished + * checklist gives the model nothing to resume. + */ +export function checkpointTodoSnapshot( + value: unknown, +): CheckpointTodoSnapshot | undefined { + if (!isRecord(value) || !Array.isArray(value.todos)) return undefined; + const todos = value.todos.flatMap((todo) => parseTodo(todo) ?? []); + if ( + !todos.some( + (todo) => todo.status === "pending" || todo.status === "in_progress", + ) + ) { + return undefined; + } + return { + revision: + typeof value.revision === "number" && Number.isFinite(value.revision) + ? value.revision + : 0, + updatedAt: + typeof value.updatedAt === "number" && Number.isFinite(value.updatedAt) + ? value.updatedAt + : 0, + todos, + }; +} + +export function todoSnapshotFromDetails( + details: unknown, +): CheckpointTodoSnapshot | undefined { + return isRecord(details) + ? checkpointTodoSnapshot(details.todoSnapshot) + : undefined; +} + +const CLOSE_TAG = ""; + +/** The block the model reads after a checkpoint summary. */ +export function formatCheckpointTodoSnapshot( + snapshot: CheckpointTodoSnapshot, +): string { + return [ + ``, + "Your TodoWrite checklist as it stood when this conversation was compacted. Continue from it instead of starting a new list; a later TodoWrite call replaces it, and every call must still resend all items.", + ...snapshot.todos.map( + (todo, index) => + `${index + 1}. [${todo.status}] ${todo.content.replaceAll(CLOSE_TAG, "")}`, + ), + CLOSE_TAG, + ].join("\n"); +} + +/** The summary text the model sees for a checkpoint, checklist included. */ +export function summaryWithCheckpointTodos( + summary: string, + details: unknown, +): string { + const snapshot = todoSnapshotFromDetails(details); + if (!snapshot) return summary; + const block = formatCheckpointTodoSnapshot(snapshot); + return summary.trim() ? `${summary}\n\n${block}` : block; +} diff --git a/packages/agent-runtime/src/runtime.test.ts b/packages/agent-runtime/src/runtime.test.ts index f4d2dec13..6351f9c0a 100644 --- a/packages/agent-runtime/src/runtime.test.ts +++ b/packages/agent-runtime/src/runtime.test.ts @@ -2779,6 +2779,46 @@ describe("DesktopAgentRuntime plan transitions", () => { await runtime.dispose(); }); + it.each(["plan", "goal"] as const)( + "asks an approved %s execution to keep the session checklist (#1177)", + async (kind) => { + const runtime = createRuntime(); + const agent = (runtime as any).agent; + agent.continue = vi.fn(async () => undefined); + agent.waitForIdle = vi.fn(async () => undefined); + runtime.setMode(kind); + + await runtime.executeApprovedPlan( + { + id: `execution-${kind}`, + proposalId: `proposal-${kind}`, + sessionId: "session-1", + kind, + plan: "# Approved", + title: "Approved", + question: "Proceed?", + artifact: { + relativePath: `.pi/${kind}/proposal.md`, + sha256: "abc123", + sizeBytes: 10, + }, + targetPermissionMode: "auto", + state: "running", + }, + `execution-turn-${kind}`, + ); + + const internal = (runtime as any).fullEntries.at(-1).message; + expect(internal.content).toContain( + "activate TodoWrite with ToolSearch if it is deferred", + ); + expect(internal.content).toContain( + "finish with every item completed or cancelled", + ); + await runtime.dispose(); + }, + ); + it("counts request system prompt and tools once despite system transcript rows", async () => { const runtime = createRuntime(); const internal = runtime as any; @@ -6155,9 +6195,11 @@ describe("DesktopAgentRuntime per-turn context protection", () => { true, ); - const checkpoint = host.call.mock.calls[0]?.[0] === "session.appendCompaction" - ? (host.call.mock.calls[0]?.[1] as any).compaction - : undefined; + const checkpoint = ( + host.call.mock.calls.find( + ([method]) => method === "session.appendCompaction", + )?.[1] as any + )?.compaction; expect(checkpoint).toEqual( expect.objectContaining({ throughMessageId: "recent-user", @@ -6913,6 +6955,153 @@ describe("DesktopAgentRuntime inline context compaction", () => { ).toMatchObject({ strategy: "summary" }); await runtime.dispose(); }); + + describe("session checklist (#1177)", () => { + const CHECKLIST = { + sessionId: "session-1", + todos: [ + { content: "Add the migration", status: "completed", priority: "medium" }, + { content: "Wire the dock", status: "in_progress", priority: "high" }, + { content: "Sync the docs", status: "pending", priority: "medium" }, + ], + revision: 7, + updatedAt: 1_700, + }; + + function hostAnswering(todos: () => unknown) { + return { + call: vi.fn(async (method: string) => + method === "todos.get" ? todos() : undefined, + ), + }; + } + + function appendedCheckpoint(host: { call: ReturnType }) { + return host.call.mock.calls.find( + ([method]) => method === "session.appendCompaction", + )?.[1].compaction; + } + + function modelContext(runtime: DesktopAgentRuntime): string { + return JSON.stringify((runtime as any).rebuiltAgentContext().messages); + } + + it("carries the checklist in the checkpoint and shows it after the summary", async () => { + const host = hostAnswering(() => CHECKLIST); + const runtime = createRuntime({ host, history }); + budgetSpy(runtime); + vi.spyOn(runtime as any, "generateCompaction").mockResolvedValue( + summaryResult(), + ); + + await (runtime as any).prepareNextTurn(nextTurn); + + expect(host.call).toHaveBeenCalledWith("todos.get", { + sessionId: "session-1", + }); + const checkpoint = appendedCheckpoint(host); + // The stored summary stays clean; the copy lives in details. + expect(checkpoint.summary).toBe(SUMMARY); + expect(checkpoint.details.todoSnapshot).toEqual({ + revision: 7, + updatedAt: 1_700, + todos: CHECKLIST.todos, + }); + const context = modelContext(runtime); + expect(context).toContain(''); + expect(context).toContain("2. [in_progress] Wire the dock"); + expect(context.indexOf(SUMMARY)).toBeLessThan( + context.indexOf("session_checklist"), + ); + await runtime.dispose(); + }); + + it("installs the checkpoint without a checklist when the host cannot read one", async () => { + const host = hostAnswering(() => { + throw new Error("host method not allowed from sidecar: todos.get"); + }); + const onEvent = vi.fn(); + const runtime = createRuntime({ host, onEvent, history }); + budgetSpy(runtime); + vi.spyOn(runtime as any, "generateCompaction").mockResolvedValue( + summaryResult(), + ); + + await (runtime as any).prepareNextTurn(nextTurn); + + expect(appendedCheckpoint(host).details).not.toHaveProperty("todoSnapshot"); + expect(modelContext(runtime)).not.toContain("session_checklist"); + expect(onEvent.mock.calls.map(([envelope]) => (envelope as any).event)).toContainEqual( + expect.objectContaining({ type: "compaction_end", ok: true }), + ); + await runtime.dispose(); + }); + + it("leaves a finished checklist out of the checkpoint", async () => { + const host = hostAnswering(() => ({ + ...CHECKLIST, + todos: [ + { content: "Add the migration", status: "completed", priority: "medium" }, + { content: "Drop the shim", status: "cancelled", priority: "low" }, + ], + })); + const runtime = createRuntime({ host, history }); + budgetSpy(runtime); + vi.spyOn(runtime as any, "generateCompaction").mockResolvedValue( + summaryResult(), + ); + + await (runtime as any).prepareNextTurn(nextTurn); + + expect(appendedCheckpoint(host).details).not.toHaveProperty("todoSnapshot"); + await runtime.dispose(); + }); + + it("drops the checklist copy rather than pushing a fitting checkpoint over budget", async () => { + const host = hostAnswering(() => CHECKLIST); + const runtime = createRuntime({ host, history }); + vi.spyOn(runtime as any, "contextBudget").mockImplementation( + ((messages: unknown) => { + const text = JSON.stringify(messages); + return text.includes(SUMMARY) && !text.includes("session_checklist") + ? { ...overBudget(), tokens: 40_000 } + : overBudget(); + }) as never, + ); + vi.spyOn(runtime as any, "generateCompaction").mockResolvedValue( + summaryResult(), + ); + + await (runtime as any).prepareNextTurn(nextTurn); + + const checkpoint = appendedCheckpoint(host); + expect(checkpoint.summary).toBe(SUMMARY); + expect(checkpoint.details).not.toHaveProperty("todoSnapshot"); + expect(checkpoint.details).not.toHaveProperty("fallback"); + await runtime.dispose(); + }); + + it("shows the checklist a restored checkpoint carries", async () => { + const runtime = createRuntime({ + history, + compaction: { + id: "compact-1", + summary: SUMMARY, + throughMessageId: "recent-user", + tokensBefore: 220_000, + retainedTail: [], + details: { + generation: 1, + todoSnapshot: { revision: 7, updatedAt: 1_700, todos: CHECKLIST.todos }, + }, + createdAt: "2026-07-28T00:00:02Z", + }, + }); + + expect(modelContext(runtime)).toContain("3. [pending] Sync the docs"); + await runtime.dispose(); + }); + }); }); describe("DesktopAgentRuntime plugin skills (D174)", () => { diff --git a/packages/agent-runtime/src/runtime.ts b/packages/agent-runtime/src/runtime.ts index a3f1cdc62..bbc5dfe90 100644 --- a/packages/agent-runtime/src/runtime.ts +++ b/packages/agent-runtime/src/runtime.ts @@ -140,6 +140,7 @@ import type { Entry, MessageEntry, } from "./pi-runtime-types.js"; +import { checkpointTodoSnapshot } from "./checkpoint-todos.js"; import { initialSystemTranscript, CONTEXT_BUDGET_SECTION, @@ -926,6 +927,14 @@ const CONTEXT_REMINDER_RATIO = 0.15; /** Close enough to the boundary that the next turn is likely to cross it. */ const CONTEXT_FALLBACK_REMINDER_TOKENS = 2_000; +/** + * Approved execution runs in Agent mode, where the session checklist is the + * user's progress view (#1177). TodoWrite is deferred, so the line says how to + * reach it. + */ +const APPROVED_EXECUTION_CHECKLIST_INSTRUCTION = + "Track multi-step execution in the session checklist: activate TodoWrite with ToolSearch if it is deferred, keep the checklist current as you work, and finish with every item completed or cancelled."; + function contextBudgetReminder(remaining: number): string { return [ "", @@ -6956,27 +6965,59 @@ Do not invent objections or turn speculative risks into blockers. Stop when the }); } - private async persistCheckpoint( + /** + * Carry the host's session checklist in the checkpoint, so the model keeps + * the steps a summary would otherwise erase (#1177). The checkpoint never + * depends on it: a host without `todos.get`, a session that keeps no + * checklist, or a failed read installs the checkpoint without one. + */ + private async checkpointWithTodos( checkpoint: ContextCompactionRecord, + ): Promise { + let snapshot: ReturnType; + try { + snapshot = checkpointTodoSnapshot( + await this.host.call("todos.get", { sessionId: this.sessionId }), + ); + } catch { + // Older hosts and native Pi sessions answer with an error; compaction + // must not fail over a convenience copy. + return checkpoint; + } + if (!snapshot) return checkpoint; + return { + ...checkpoint, + details: { + ...(isRecord(checkpoint.details) ? checkpoint.details : {}), + todoSnapshot: snapshot, + }, + }; + } + + private async persistCheckpoint( + summarized: ContextCompactionRecord, reason: ContextCompactionReason, willRetry: boolean, mustFitSafeBudget: boolean, fallback?: ContextCompactionFallback, ): Promise { const systemMessage = systemTranscriptCheckpoint(this.agent.state.messages); - checkpoint = { - ...checkpoint, - details: { ...(isRecord(checkpoint.details) ? checkpoint.details : {}), ...(systemMessage ? { systemMessageJson: JSON.stringify(systemMessage) } : {}) }, + const withSystem: ContextCompactionRecord = { + ...summarized, + details: { ...(isRecord(summarized.details) ? summarized.details : {}), ...(systemMessage ? { systemMessageJson: JSON.stringify(systemMessage) } : {}) }, }; - const compactedBudget = this.contextBudget( - this.liveSessionContext(checkpoint).messages, - ); - if ( - mustFitSafeBudget && - compactedBudget.tokens >= compactedBudget.hardLimit - ) { - return "oversized"; - } + const fits = (candidate: ContextCompactionRecord) => { + const budget = this.contextBudget( + this.liveSessionContext(candidate).messages, + ); + return !mustFitSafeBudget || budget.tokens < budget.hardLimit; + }; + if (!fits(withSystem)) return "oversized"; + // The checklist copy is dropped rather than letting it push a checkpoint + // that fits on its own over the safe budget. + const withTodos = await this.checkpointWithTodos(withSystem); + const checkpoint = + withTodos !== withSystem && fits(withTodos) ? withTodos : withSystem; try { await this.host.call("session.appendCompaction", { sessionId: this.sessionId, @@ -8266,6 +8307,7 @@ Do not invent objections or turn speculative risks into blockers. Stop when the execution.plan, "", "Choose your own approach with the normal Agent tools. Then verify every acceptance criterion yourself, running the checks the contract names rather than assuming they pass.", + APPROVED_EXECUTION_CHECKLIST_INSTRUCTION, "Keep working while a criterion is still unmet and you have an untried approach. Stop early only if a boundary in the contract blocks you or a criterion cannot be verified; say which one and why.", "Finish with a report that walks the acceptance criteria one by one, each marked met or unmet with the evidence you observed.", ].join("\n") @@ -8279,6 +8321,7 @@ Do not invent objections or turn speculative risks into blockers. Stop when the execution.plan, "", "Implement the approved plan with the normal Agent tools, then report the result.", + APPROVED_EXECUTION_CHECKLIST_INSTRUCTION, ].join("\n"); const internalId = `approved-${kind}:${execution.id}`; const internalMessage: AgentMessage = { diff --git a/packages/agent-runtime/src/session-context.ts b/packages/agent-runtime/src/session-context.ts index d930c55be..29ce3f409 100644 --- a/packages/agent-runtime/src/session-context.ts +++ b/packages/agent-runtime/src/session-context.ts @@ -18,6 +18,7 @@ import { createCompactionSummaryMessage, } from "./pi-runtime-messages.js"; import type { Entry } from "./pi-runtime-types.js"; +import { summaryWithCheckpointTodos } from "./checkpoint-todos.js"; import { retainedReasoningFromDetails, retainedReasoningToMessages, @@ -61,8 +62,10 @@ export function sessionEntryToContextMessages( return [ ...(entry.details && typeof entry.details === "object" && "systemMessageJson" in entry.details ? [readSystemMessage(entry.details.systemMessageJson)] : []), + // The checklist copy lives in `details`, not `summary`, so the stored + // summary a later compaction carries forward never holds a stale list. createCompactionSummaryMessage( - entry.summary, + summaryWithCheckpointTodos(entry.summary, entry.details), entry.tokensBefore, entry.timestamp, ), diff --git a/packages/host-runtime/src/agent-sidecar.ts b/packages/host-runtime/src/agent-sidecar.ts index f27168052..693e1fd5f 100644 --- a/packages/host-runtime/src/agent-sidecar.ts +++ b/packages/host-runtime/src/agent-sidecar.ts @@ -56,6 +56,9 @@ const HOST_PROXY_ALLOWED = new Set([ "session.appendMessage", "session.appendCompaction", "session.replaceMessages", + // Read-only: a compaction checkpoint copies the session checklist (#1177). + // Writes stay on `tools.execute` TodoWrite, authorized by host-core. + "todos.get", "workspace.get", "plans.enter", "plans.submit", diff --git a/packages/host-runtime/src/sidecar-todo-proxy.test.ts b/packages/host-runtime/src/sidecar-todo-proxy.test.ts new file mode 100644 index 000000000..4d5b6ec12 --- /dev/null +++ b/packages/host-runtime/src/sidecar-todo-proxy.test.ts @@ -0,0 +1,52 @@ +import { afterEach, expect, it } from "vitest"; +import { AgentSidecar, type SidecarHostLink } from "./agent-sidecar.js"; + +// A real stdio child that forwards each `probe` over the production reverse RPC. +const child = `const rl=require('node:readline').createInterface({input:process.stdin}); +const pending=new Map(); rl.on('line',line=>{const m=JSON.parse(line); +if(m.method==='probe'){pending.set('r'+m.id,m.id);console.log(JSON.stringify({id:'r'+m.id,method:'host.proxy',params:m.params}));} +else if(pending.has(m.id)){console.log(JSON.stringify({...m,id:pending.get(m.id)}));pending.delete(m.id);}});`; +const sidecars: AgentSidecar[] = []; +afterEach(async () => { + await Promise.all(sidecars.splice(0).map((sidecar) => sidecar.dispose())); +}); + +function harness() { + const sidecar = new AgentSidecar({ + launch: { command: process.execPath, args: ["-e", child] }, + onStderr: () => {}, + }); + sidecars.push(sidecar); + const calls: Array<{ method: string; params: unknown }> = []; + const host: SidecarHostLink = { + async call(method: string, params?: unknown): Promise { + calls.push({ method, params }); + return { + sessionId: "s", + todos: [{ content: "Wire it", status: "in_progress", priority: "medium" }], + revision: 3, + updatedAt: 1, + } as T; + }, + onNotification: () => () => {}, + onExit: () => () => {}, + }; + sidecar.setHost(host); + return { sidecar, calls }; +} + +it("lets a compaction checkpoint read the session checklist through the proxy", async () => { + const { sidecar, calls } = harness(); + await expect( + sidecar.call("probe", { method: "todos.get", params: { sessionId: "s" } }), + ).resolves.toMatchObject({ revision: 3, todos: [{ content: "Wire it" }] }); + expect(calls).toEqual([{ method: "todos.get", params: { sessionId: "s" } }]); +}); + +it("still refuses checklist mutation outside TodoWrite", async () => { + const { sidecar, calls } = harness(); + await expect( + sidecar.call("probe", { method: "todos.set", params: { sessionId: "s", todos: [] } }), + ).rejects.toMatchObject({ code: -32601 }); + expect(calls).toEqual([]); +});