From 8ca9b5acfef8b506360a6fc51b2603ce6c7f6090 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 28 May 2026 18:14:22 +0900 Subject: [PATCH] fix(todo): skip unsafe continuation tails --- .../idle-event.test.ts | 94 ++++++++++++++++++- .../todo-continuation-enforcer/idle-event.ts | 8 +- .../pending-question-detection.test.ts | 38 ++++++++ .../pending-question-detection.ts | 20 +++- 4 files changed, 156 insertions(+), 4 deletions(-) diff --git a/src/hooks/todo-continuation-enforcer/idle-event.test.ts b/src/hooks/todo-continuation-enforcer/idle-event.test.ts index 989b0b378..5563f45d3 100644 --- a/src/hooks/todo-continuation-enforcer/idle-event.test.ts +++ b/src/hooks/todo-continuation-enforcer/idle-event.test.ts @@ -40,7 +40,18 @@ function createStateStore(): { resetContinuationProgress: (sessionID: string) => { resetCalls.push(sessionID) }, - cancelCountdown: () => {}, + cancelCountdown: () => { + if (state.countdownTimer) { + clearTimeout(state.countdownTimer) + state.countdownTimer = undefined + } + if (state.countdownInterval) { + clearInterval(state.countdownInterval) + state.countdownInterval = undefined + } + state.countdownStartedAt = undefined + state.inFlight = false + }, cleanup: () => {}, cancelAllCountdowns: () => {}, shutdown: () => {}, @@ -136,4 +147,85 @@ describe("handleSessionIdle", () => { // reset is still called only once (from the first idle) expect(resetCalls).toHaveLength(1) }) + + it("skips todo continuation when the previous internal continuation has only an empty unknown assistant turn", async () => { + // given + const sessionID = "ses_internal_noop_tail" + const { store, trackCalls, state } = createStateStore() + const ctx = { + client: { + session: { + messages: async () => ({ + data: [ + { + info: { role: "user" }, + parts: [{ type: "text", text: "continue\n", synthetic: true }], + }, + { + info: { role: "assistant", finish: "unknown", time: { completed: Date.now() } }, + parts: [{ type: "step-start" }, { type: "step-finish", reason: "unknown" }], + }, + ], + }), + todo: async () => ({ + data: [ + { id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" }, + ], + }), + }, + }, + directory: "/tmp/test", + } + + try { + // when + await handleSessionIdle({ + ctx: ctx as never, + sessionID, + sessionStateStore: store, + }) + + // then + expect(trackCalls).toEqual([]) + expect(state.countdownStartedAt).toBeUndefined() + } finally { + store.cancelCountdown(sessionID) + } + }) + + it("skips todo continuation when session messages cannot be inspected", async () => { + // given + const sessionID = "ses_messages_fetch_fails" + const { store, trackCalls, state } = createStateStore() + const ctx = { + client: { + session: { + messages: async () => { + throw new Error("message endpoint failed") + }, + todo: async () => ({ + data: [ + { id: "todo-1", content: "Finish init-deep", status: "pending", priority: "high" }, + ], + }), + }, + }, + directory: "/tmp/test", + } + + try { + // when + await handleSessionIdle({ + ctx: ctx as never, + sessionID, + sessionStateStore: store, + }) + + // then + expect(trackCalls).toEqual([]) + expect(state.countdownStartedAt).toBeUndefined() + } finally { + store.cancelCountdown(sessionID) + } + }) }) diff --git a/src/hooks/todo-continuation-enforcer/idle-event.ts b/src/hooks/todo-continuation-enforcer/idle-event.ts index bd4306096..1288dd9b3 100644 --- a/src/hooks/todo-continuation-enforcer/idle-event.ts +++ b/src/hooks/todo-continuation-enforcer/idle-event.ts @@ -4,6 +4,7 @@ import { getSessionAgent } from "../../features/claude-code-session-state" import { normalizeSDKResponse } from "../../shared" import { getAgentConfigKey } from "../../shared/agent-display-names" import { log } from "../../shared/logger" +import { latestAssistantTurnBlocksInternalPrompt } from "../../shared/prompt-async-gate/pending-tool-turn" import { isLastAssistantMessageAborted } from "./abort-detection" import { acknowledgeCompactionGuard, isCompactionGuardActive } from "./compaction-guard" @@ -92,8 +93,13 @@ export async function handleSessionIdle(args: { log(`[${HOOK_NAME}] Skipped: pending question awaiting user response`, { sessionID }) return } + if (latestAssistantTurnBlocksInternalPrompt(prefetchedMessages)) { + log(`[${HOOK_NAME}] Skipped: pending internal continuation response`, { sessionID }) + return + } } catch (error) { - log(`[${HOOK_NAME}] Messages fetch failed, continuing`, { sessionID, error: String(error) }) + log(`[${HOOK_NAME}] Messages fetch failed, skipping continuation`, { sessionID, error: String(error) }) + return } let todos: Todo[] = [] diff --git a/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts b/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts index 62c2a8179..ff9e463f8 100644 --- a/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts +++ b/src/hooks/todo-continuation-enforcer/pending-question-detection.test.ts @@ -39,6 +39,44 @@ describe("hasUnansweredQuestion", () => { expect(hasUnansweredQuestion(messages)).toBe(true) }) + test("#given last assistant message with OpenCode question tool field #when checking pending question #then returns true", () => { + const messages = [ + { info: { role: "user" } }, + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "question" }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(true) + }) + + test("#given last assistant message with OpenCode ask_user_question tool field #when checking pending question #then returns true", () => { + const messages = [ + { info: { role: "user" } }, + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "ask_user_question" }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(true) + }) + + test("#given completed OpenCode question tool #when checking pending question #then returns false", () => { + const messages = [ + { + info: { role: "assistant" }, + parts: [ + { type: "tool", tool: "question", state: { status: "completed" } }, + ], + }, + ] + expect(hasUnansweredQuestion(messages)).toBe(false) + }) + test("given user message after question (answered), returns false", () => { const messages = [ { diff --git a/src/hooks/todo-continuation-enforcer/pending-question-detection.ts b/src/hooks/todo-continuation-enforcer/pending-question-detection.ts index f9bd4881e..2217ff6b3 100644 --- a/src/hooks/todo-continuation-enforcer/pending-question-detection.ts +++ b/src/hooks/todo-continuation-enforcer/pending-question-detection.ts @@ -5,7 +5,9 @@ import { HOOK_NAME } from "./constants" interface MessagePart { type?: string name?: string + tool?: string toolName?: string + state?: { status?: string } text?: string synthetic?: boolean } @@ -16,6 +18,20 @@ interface Message { parts?: MessagePart[] } +const QUESTION_TOOL_NAMES = new Set(["question", "ask_user_question", "askuserquestion"]) + +function getToolName(part: MessagePart): string | undefined { + return part.name ?? part.tool ?? part.toolName +} + +function isUnansweredQuestionTool(part: MessagePart): boolean { + const toolName = getToolName(part) + if (!QUESTION_TOOL_NAMES.has(toolName?.toLowerCase() ?? "")) { + return false + } + return part.state?.status !== "completed" +} + export function hasUnansweredQuestion(messages: Message[]): boolean { if (!messages || messages.length === 0) return false @@ -33,8 +49,8 @@ export function hasUnansweredQuestion(messages: Message[]): boolean { if (role === "assistant" && msg.parts) { const hasQuestion = msg.parts.some( (part) => - (part.type === "tool_use" || part.type === "tool-invocation") && - (part.name === "question" || part.toolName === "question"), + (part.type === "tool" || part.type === "tool_use" || part.type === "tool-invocation") && + isUnansweredQuestionTool(part), ) if (hasQuestion) { log(`[${HOOK_NAME}] Detected pending question tool in last assistant message`)