From b76ecc45868a339eafc1886b743bfb5b059b56f8 Mon Sep 17 00:00:00 2001 From: Error Date: Sat, 8 Nov 2025 11:02:46 -0600 Subject: [PATCH 1/3] fix: ensure undo restores todo list (#4081) --- packages/opencode/src/session/index.ts | 2 + packages/opencode/src/session/revert.ts | 44 +++-- .../opencode/test/session/revert-todo.test.ts | 154 ++++++++++++++++++ 3 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/test/session/revert-todo.test.ts diff --git a/packages/opencode/src/session/index.ts b/packages/opencode/src/session/index.ts index d0bdfb9e1e2..3341b2fc44d 100644 --- a/packages/opencode/src/session/index.ts +++ b/packages/opencode/src/session/index.ts @@ -17,6 +17,7 @@ import { SessionPrompt } from "./prompt" import { fn } from "@/util/fn" import { Command } from "../command" import { Snapshot } from "@/snapshot" +import { Todo } from "./todo" export namespace Session { const log = Log.create({ service: "session" }) @@ -66,6 +67,7 @@ export namespace Session { partID: z.string().optional(), snapshot: z.string().optional(), diff: z.string().optional(), + todos: Todo.Info.array().optional(), }) .optional(), }) diff --git a/packages/opencode/src/session/revert.ts b/packages/opencode/src/session/revert.ts index dbf81edc782..213e7c7bc01 100644 --- a/packages/opencode/src/session/revert.ts +++ b/packages/opencode/src/session/revert.ts @@ -4,14 +4,24 @@ import { Snapshot } from "../snapshot" import { MessageV2 } from "./message-v2" import { Session } from "." import { Log } from "../util/log" -import { splitWhen } from "remeda" + import { Storage } from "../storage/storage" import { Bus } from "../bus" import { SessionLock } from "./lock" +import { Todo } from "./todo" +import { splitWhen } from "remeda" export namespace SessionRevert { const log = Log.create({ service: "session.revert" }) + function extractTodos(part: MessageV2.Part): Todo.Info[] | undefined { + if (part.type !== "tool") return undefined + if (part.tool !== "todowrite") return undefined + if (part.state.status !== "completed") return undefined + const metadata = part.state.metadata as { todos?: Todo.Info[] } | undefined + return metadata?.todos + } + export const RevertInput = z.object({ sessionID: Identifier.schema("session"), messageID: Identifier.schema("message"), @@ -30,6 +40,7 @@ export namespace SessionRevert { const session = await Session.get(input.sessionID) let revert: Session.Info["revert"] + let todosBefore: Todo.Info[] | undefined const patches: Snapshot.Patch[] = [] for (const msg of all) { if (msg.info.role === "user") lastUser = msg.info @@ -42,23 +53,33 @@ export namespace SessionRevert { continue } - if (!revert) { - if ((msg.info.id === input.messageID && !input.partID) || part.id === input.partID) { - // if no useful parts left in message, same as reverting whole message - const partID = remaining.some((item) => ["text", "tool"].includes(item.type)) ? input.partID : undefined - revert = { - messageID: !partID && lastUser ? lastUser.id : msg.info.id, - partID, - } + const matchesMessage = msg.info.id === input.messageID && !input.partID + const matchesPart = part.id === input.partID + const isTarget = matchesMessage || matchesPart + + if (isTarget) { + const partID = remaining.some((item) => ["text", "tool"].includes(item.type)) ? input.partID : undefined + revert = { + messageID: !partID && lastUser ? lastUser.id : msg.info.id, + partID, + } + } + if (!isTarget) { + const todos = extractTodos(part) + if (todos) { + todosBefore = todos } - remaining.push(part) } + + remaining.push(part) } } if (revert) { const session = await Session.get(input.sessionID) revert.snapshot = session.revert?.snapshot ?? (await Snapshot.track()) + revert.todos = todosBefore ?? [] + await Todo.update({ sessionID: input.sessionID, todos: revert.todos }) await Snapshot.revert(patches) if (revert.snapshot) revert.diff = await Snapshot.diff(revert.snapshot) return Session.update(input.sessionID, (draft) => { @@ -108,6 +129,9 @@ export namespace SessionRevert { }) } } + const todos = session.revert.todos ?? [] + await Todo.update({ sessionID, todos }) + await Session.update(sessionID, (draft) => { draft.revert = undefined }) diff --git a/packages/opencode/test/session/revert-todo.test.ts b/packages/opencode/test/session/revert-todo.test.ts new file mode 100644 index 00000000000..c2f8069d95a --- /dev/null +++ b/packages/opencode/test/session/revert-todo.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, test } from "bun:test" +import path from "path" + +import { Instance } from "../../src/project/instance" +import { Session } from "../../src/session" +import { Todo } from "../../src/session/todo" +import { SessionRevert } from "../../src/session/revert" +import { Identifier } from "../../src/id/id" +import { MessageV2 } from "../../src/session/message-v2" + +const projectRoot = path.join(__dirname, "../..") + +async function writeTodoMessage(sessionID: string, todos: Todo.Info[]) { + const messageID = Identifier.ascending("message") + const timestamp = Date.now() + + await Session.updateMessage({ + id: messageID, + sessionID, + role: "user", + time: { + created: timestamp, + }, + } as MessageV2.User) + + const part: MessageV2.ToolPart = { + id: Identifier.ascending("part"), + sessionID, + messageID, + type: "tool", + callID: Identifier.ascending("message"), + tool: "todowrite", + state: { + status: "completed", + input: { + todos, + }, + output: JSON.stringify(todos), + title: `${todos.length} todos`, + metadata: { + todos, + }, + time: { + start: timestamp, + end: timestamp, + }, + }, + } + + await Session.updatePart(part) + await Todo.update({ sessionID, todos }) + + return messageID +} + +// Regression test for: +// "undo does not undo todo list, resulting in agents continuing on the same path as the undo" +// +// This encodes the EXPECTED behavior: +// - Undoing a message that changed the todo list should restore the previous todos. +// With the current implementation, this should FAIL, exposing the bug. +describe("SessionRevert and Todo integration", () => { + test("undoing a later message that changed todos should restore previous todo list", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await Session.create({}) + + // 1) Initial todos that represent the original plan + const initialTodos: Todo.Info[] = [ + { + id: "step-1", + content: "initial plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, initialTodos) + + // 2) Create a later message (todowrite run) that updates the todo list + const updatedTodos: Todo.Info[] = [ + { + id: "step-2", + content: "updated plan step", + status: "pending", + priority: "high", + }, + ] + const laterMessageID = await writeTodoMessage(session.id, updatedTodos) + + // Sanity check: before undo, we see the updated todos + const beforeRevert = await Todo.get(session.id) + expect(beforeRevert).toEqual(updatedTodos) + + // 4) Perform undo targeting the later message + await SessionRevert.revert({ + sessionID: session.id, + messageID: laterMessageID, + }) + + // Apply cleanup: mimics committing the undo (drop reverted messages/parts) + const sessionAfterRevert = await Session.get(session.id) + await SessionRevert.cleanup(sessionAfterRevert) + + // 5) After undo, we EXPECT todos to be restored to the initial list. + // Today, todo state is not reverted, so this should FAIL until fixed. + const afterRevert = await Todo.get(session.id) + expect(afterRevert).toEqual(initialTodos) + + await Session.remove(session.id) + }, + }) + }) + + test("the tui displays undone todo state after undo", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await Session.create({}) + + const initialTodos: Todo.Info[] = [ + { + id: "step-1", + content: "initial plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, initialTodos) + + const updatedTodos: Todo.Info[] = [ + { + id: "step-2", + content: "updated plan step", + status: "pending", + priority: "high", + }, + ] + const laterMessageID = await writeTodoMessage(session.id, updatedTodos) + + // Undo the latest message but DO NOT trigger cleanup yet (mirrors actual undo UX) + await SessionRevert.revert({ + sessionID: session.id, + messageID: laterMessageID, + }) + + const afterUndo = await Todo.get(session.id) + expect(afterUndo).toEqual(initialTodos) + + await Session.remove(session.id) + }, + }) + }) +}) From fe8bd08ab183de058a9caa734c968876b5bd0f53 Mon Sep 17 00:00:00 2001 From: Error Date: Sat, 8 Nov 2025 11:35:17 -0600 Subject: [PATCH 2/3] test: cover multiple todo undo scenarios --- .../opencode/test/session/revert-todo.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/opencode/test/session/revert-todo.test.ts b/packages/opencode/test/session/revert-todo.test.ts index c2f8069d95a..3cf2afbf00b 100644 --- a/packages/opencode/test/session/revert-todo.test.ts +++ b/packages/opencode/test/session/revert-todo.test.ts @@ -151,4 +151,59 @@ describe("SessionRevert and Todo integration", () => { }, }) }) + + test("undoing among multiple todo updates restores the previous plan", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await Session.create({}) + + const initialTodos: Todo.Info[] = [ + { + id: "step-1", + content: "initial plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, initialTodos) + + const secondTodos: Todo.Info[] = [ + { + id: "step-2", + content: "refined plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, secondTodos) + + const thirdTodos: Todo.Info[] = [ + { + id: "step-3", + content: "latest plan step", + status: "pending", + priority: "high", + }, + ] + const thirdMessageID = await writeTodoMessage(session.id, thirdTodos) + + await SessionRevert.revert({ + sessionID: session.id, + messageID: thirdMessageID, + }) + + const afterUndo = await Todo.get(session.id) + expect(afterUndo).toEqual(secondTodos) + + const sessionAfterRevert = await Session.get(session.id) + await SessionRevert.cleanup(sessionAfterRevert) + + const afterCleanup = await Todo.get(session.id) + expect(afterCleanup).toEqual(secondTodos) + + await Session.remove(session.id) + }, + }) + }) }) From 72e2819b1cc263f507a073be7cfcb87cf73428e8 Mon Sep 17 00:00:00 2001 From: Error Date: Sat, 8 Nov 2025 11:36:58 -0600 Subject: [PATCH 3/3] test: add non-todo and part undo coverage --- .../opencode/test/session/revert-todo.test.ts | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/packages/opencode/test/session/revert-todo.test.ts b/packages/opencode/test/session/revert-todo.test.ts index 3cf2afbf00b..3dd2aae2120 100644 --- a/packages/opencode/test/session/revert-todo.test.ts +++ b/packages/opencode/test/session/revert-todo.test.ts @@ -206,4 +206,114 @@ describe("SessionRevert and Todo integration", () => { }, }) }) + + test("undoing a non-todo message keeps the latest plan", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await Session.create({}) + + const initialTodos: Todo.Info[] = [ + { + id: "step-1", + content: "initial plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, initialTodos) + + const secondTodos: Todo.Info[] = [ + { + id: "step-2", + content: "refined plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, secondTodos) + + const laterMessageID = Identifier.ascending("message") + await Session.updateMessage({ + id: laterMessageID, + sessionID: session.id, + role: "user", + time: { + created: Date.now(), + }, + summary: { + diffs: [], + }, + } as MessageV2.User) + + await SessionRevert.revert({ + sessionID: session.id, + messageID: laterMessageID, + }) + + const afterUndo = await Todo.get(session.id) + expect(afterUndo).toEqual(secondTodos) + + const sessionAfterRevert = await Session.get(session.id) + await SessionRevert.cleanup(sessionAfterRevert) + + const afterCleanup = await Todo.get(session.id) + expect(afterCleanup).toEqual(secondTodos) + + await Session.remove(session.id) + }, + }) + }) + + test("undoing a todowrite part reverts to the previous plan", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await Session.create({}) + + const initialTodos: Todo.Info[] = [ + { + id: "step-1", + content: "initial plan step", + status: "pending", + priority: "high", + }, + ] + await writeTodoMessage(session.id, initialTodos) + + const updatedTodos: Todo.Info[] = [ + { + id: "step-2", + content: "updated plan step", + status: "pending", + priority: "high", + }, + ] + const messageID = await writeTodoMessage(session.id, updatedTodos) + + const messages = await Session.messages({ sessionID: session.id }) + const targetMessage = messages.find((msg) => msg.info.id === messageID) + if (!targetMessage) throw new Error("expected message to exist") + const toolPart = targetMessage.parts.find((part) => part.type === "tool") + if (!toolPart) throw new Error("expected todowrite tool part") + + await SessionRevert.revert({ + sessionID: session.id, + messageID, + partID: toolPart.id, + }) + + const afterUndo = await Todo.get(session.id) + expect(afterUndo).toEqual(initialTodos) + + const sessionAfterRevert = await Session.get(session.id) + await SessionRevert.cleanup(sessionAfterRevert) + + const afterCleanup = await Todo.get(session.id) + expect(afterCleanup).toEqual(initialTodos) + + await Session.remove(session.id) + }, + }) + }) })