diff --git a/.specgit.yaml b/.specgit.yaml index 1cfef0a3b..d5493fc86 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,8 @@ version: 1 -delivery: open-security-alerts +delivery: surface-todo-state context: kind: branch - branch: fix/423-open-security-alerts + branch: feat/429-surface-todo-state issues: - - 423 -pr: 424 + - 429 +pr: 430 diff --git a/packages/opencode/src/session/todo-reminders.ts b/packages/opencode/src/session/todo-reminders.ts index 926528813..48f85a325 100644 --- a/packages/opencode/src/session/todo-reminders.ts +++ b/packages/opencode/src/session/todo-reminders.ts @@ -18,8 +18,15 @@ * - freshness guard: the session's last assistant message already contains * a successful todowrite call — the model just updated the list itself, * so this step's request does not nag about it + * + * Second surfacing point (#429): before a non-todowrite tool call executes, + * the current uncompleted list is returned once per assistant turn so long + * multi-tool turns re-see it mid-flight. Turn-scoped dedup keeps parallel + * tool calls from repeating the same reminder N times — the original reason + * #389 refined this seam away; pure-reasoning steps stay covered by the + * per-step injection above. */ -import { Effect } from "effect" +import { Cause, Effect } from "effect" import { SessionV1 } from "@opencode-ai/core/v1/session" import type { SessionID } from "./schema" import { PartID } from "./schema" @@ -27,6 +34,14 @@ import { Todo } from "./todo" const TODO_WRITE_TOOL = "todowrite" +// sessionID -> assistant messageID that already received the pre-tool-call +// reminder this turn. One entry per live session, overwritten each turn. +const remindedTurns = new Map() + +function uncompletedOf(todos: Todo.Info[]): Todo.Info[] { + return todos.filter((item) => item.status !== "completed" && item.status !== "cancelled") +} + function turnJustUpdatedTodos(messages: SessionV1.WithParts[]): boolean { const lastAssistant = messages.findLast((msg) => msg.info.role === "assistant") if (!lastAssistant) return false @@ -51,7 +66,7 @@ export const apply = Effect.fn("TodoReminders.apply")(function* (input: { }) { const todo = yield* Todo.Service const todos = yield* todo.get(input.sessionID) - const uncompleted = todos.filter((item) => item.status !== "completed" && item.status !== "cancelled") + const uncompleted = uncompletedOf(todos) if (uncompleted.length === 0) return input.messages const userMessage = input.messages.findLast((msg) => msg.info.role === "user") if (!userMessage) return input.messages @@ -67,4 +82,32 @@ export const apply = Effect.fn("TodoReminders.apply")(function* (input: { return input.messages }) +/** + * Reminder string for the pre-tool-call seam, or undefined when the call must + * stay clean: todowrite itself, a turn already surfaced this turn, or nothing + * uncompleted. Marks the turn only when a string is actually returned. + */ +export const preToolCall = Effect.fn("TodoReminders.preToolCall")(function* (input: { + sessionID: SessionID + messageID: string + tool: string +}) { + if (input.tool === TODO_WRITE_TOOL) return undefined + if (remindedTurns.get(input.sessionID) === input.messageID) return undefined + // The reminder is decoration on top of tool execution: a missing or failing + // Todo service must degrade to "no reminder", never kill the tool call. + const uncompleted = yield* Effect.gen(function* () { + const todo = yield* Todo.Service + const todos = yield* todo.get(input.sessionID) + return uncompletedOf(todos) + }).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) ? Effect.failCause(cause) : Effect.succeed([] as Todo.Info[]), + ), + ) + if (uncompleted.length === 0) return undefined + remindedTurns.set(input.sessionID, input.messageID) + return renderReminder(uncompleted) +}) + export * as TodoReminders from "./todo-reminders" diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index c4f237060..c0aa7494d 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -22,6 +22,7 @@ import { MessageV2 } from "./message-v2" import { Session } from "./session" import { SessionProcessor } from "./processor" import { PartID } from "./schema" +import { TodoReminders } from "./todo-reminders" import { EffectBridge } from "@/effect/bridge" import { SessionContext } from "@/effect/session-context" import { ProviderV2 } from "@opencode-ai/core/provider" @@ -128,6 +129,13 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { ) // SettingsHook PreToolUse let preContexts: string[] = [] + // Native todo surfacing (#429): once per assistant turn, before + // any non-todowrite tool result, re-show the uncompleted list. + const todoReminder = yield* TodoReminders.preToolCall({ + sessionID: ctx.sessionID, + messageID: input.processor.message.id, + tool: item.id, + }) if (settingsHook) { const preResult = yield* settingsHook .trigger( @@ -187,8 +195,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { } // PreToolUse additionalContexts: prepend so the model sees any hook-injected // gate/reminder before the tool result (mirrors PostToolUse surfacing below). - if (preContexts.length) { - output.output = `${preContexts.join("\n\n")}\n\n${output.output ?? ""}` + const preLines = [todoReminder, ...preContexts].filter((line): line is string => Boolean(line)) + if (preLines.length) { + output.output = `${preLines.join("\n\n")}\n\n${output.output ?? ""}` } yield* plugin.trigger( "tool.execute.after", @@ -522,6 +531,12 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { ) // SettingsHook PreToolUse let preContexts: string[] = [] + // Native todo surfacing (#429): same per-turn contract as native tools. + const mcpTodoReminder = yield* TodoReminders.preToolCall({ + sessionID: ctx.sessionID, + messageID: input.processor.message.id, + tool: key, + }) if (settingsHook) { const preResult = yield* settingsHook .trigger( @@ -643,8 +658,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { } // PreToolUse additionalContexts: prepend so the model sees any hook-injected // gate/reminder before the tool result (mirrors PostToolUse surfacing below). - if (preContexts.length) { - output.output = `${preContexts.join("\n\n")}\n\n${output.output ?? ""}` + const preLines = [mcpTodoReminder, ...preContexts].filter((line): line is string => Boolean(line)) + if (preLines.length) { + output.output = `${preLines.join("\n\n")}\n\n${output.output ?? ""}` } // SettingsHook PostToolUse if (settingsHook) { diff --git a/packages/opencode/test/session/todo-reminders.test.ts b/packages/opencode/test/session/todo-reminders.test.ts index 0d5c8fb9e..db4a86ec7 100644 --- a/packages/opencode/test/session/todo-reminders.test.ts +++ b/packages/opencode/test/session/todo-reminders.test.ts @@ -293,3 +293,78 @@ describe("TodoReminders run-loop guarantees (issue #389 review)", () => { }), ) }) + +describe("TodoReminders.preToolCall (#429)", () => { + const sessionID = SessionID.make("ses_1") + const todos: Todo.Info[] = [ + { content: "ship feature", status: "in_progress", priority: "high" }, + { content: "done already", status: "completed", priority: "low" }, + ] + + runtime.effect("returns the reminder before a non-todowrite tool call", () => + Effect.gen(function* () { + const reminder = yield* TodoReminders.preToolCall({ + sessionID, + messageID: "msg_pre_1", + tool: "bash", + }).pipe(Effect.provide(makeTodoLayer(todos))) + expect(reminder).toContain("[todo reminder]") + expect(reminder).toContain("ship feature") + expect(reminder).toContain("todowrite") + }), + ) + + runtime.effect("injects at most once per assistant turn", () => + Effect.gen(function* () { + const layer = makeTodoLayer(todos) + const first = yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_2", tool: "bash" }).pipe( + Effect.provide(layer), + ) + const second = yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_2", tool: "read" }).pipe( + Effect.provide(layer), + ) + expect(first).toBeDefined() + expect(second).toBeUndefined() + }), + ) + + runtime.effect("re-arms on a new assistant turn", () => + Effect.gen(function* () { + const layer = makeTodoLayer(todos) + yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_3", tool: "bash" }).pipe( + Effect.provide(layer), + ) + const next = yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_4", tool: "read" }).pipe( + Effect.provide(layer), + ) + expect(next).toBeDefined() + }), + ) + + runtime.effect("never surfaces for todowrite and leaves the turn unmarked", () => + Effect.gen(function* () { + const layer = makeTodoLayer(todos) + const write = yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_5", tool: "todowrite" }).pipe( + Effect.provide(layer), + ) + expect(write).toBeUndefined() + // todowrite must not consume the turn's single shot either. + const other = yield* TodoReminders.preToolCall({ sessionID, messageID: "msg_pre_5", tool: "grep" }).pipe( + Effect.provide(layer), + ) + expect(other).toBeDefined() + }), + ) + + runtime.effect("returns undefined when nothing is uncompleted", () => + Effect.gen(function* () { + const settled: Todo.Info[] = [{ content: "a", status: "completed", priority: "low" }] + const none = yield* TodoReminders.preToolCall({ + sessionID, + messageID: "msg_pre_6", + tool: "bash", + }).pipe(Effect.provide(makeTodoLayer(settled))) + expect(none).toBeUndefined() + }), + ) +})