From a49fbeec5fcda31f1c4917759e2008e14ae02b5a Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 4 Jan 2026 18:12:59 +0900 Subject: [PATCH] refactor(todo-continuation-enforcer): update message mock structure and remove unreliable abort error handling tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add MockMessage interface to match new message structure - Update client mock to include messages() method - Remove abort error detection tests that were unreliable - Simplify error handling logic for better testability 🤖 Generated with assistance of OhMyOpenCode (https://github.com/code-yeongyu/oh-my-opencode) --- src/hooks/todo-continuation-enforcer.test.ts | 309 ++++++------------- src/hooks/todo-continuation-enforcer.ts | 84 +++-- 2 files changed, 138 insertions(+), 255 deletions(-) diff --git a/src/hooks/todo-continuation-enforcer.test.ts b/src/hooks/todo-continuation-enforcer.test.ts index 00709a8..8f6c6f7 100644 --- a/src/hooks/todo-continuation-enforcer.test.ts +++ b/src/hooks/todo-continuation-enforcer.test.ts @@ -8,6 +8,16 @@ describe("todo-continuation-enforcer", () => { let promptCalls: Array<{ sessionID: string; agent?: string; model?: { providerID?: string; modelID?: string }; text: string }> let toastCalls: Array<{ title: string; message: string }> + interface MockMessage { + info: { + id: string + role: "user" | "assistant" + error?: { name: string; data?: { message: string } } + } + } + + let mockMessages: MockMessage[] = [] + function createMockPluginInput() { return { client: { @@ -16,6 +26,7 @@ describe("todo-continuation-enforcer", () => { { id: "1", content: "Task 1", status: "pending", priority: "high" }, { id: "2", content: "Task 2", status: "completed", priority: "medium" }, ]}), + messages: async () => ({ data: mockMessages }), prompt: async (opts: any) => { promptCalls.push({ sessionID: opts.path.id, @@ -51,6 +62,7 @@ describe("todo-continuation-enforcer", () => { beforeEach(() => { promptCalls = [] toastCalls = [] + mockMessages = [] setMainSession(undefined) subagentSessions.clear() }) @@ -165,56 +177,7 @@ describe("todo-continuation-enforcer", () => { expect(promptCalls[0].sessionID).toBe(bgTaskSession) }) - test("should skip injection when abort error occurs immediately before idle", async () => { - // #given - session that just had an abort error - const sessionID = "main-error" - setMainSession(sessionID) - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - - // #when - abort error occurs - await hook.handler({ - event: { type: "session.error", properties: { sessionID, error: { name: "AbortError", message: "aborted" } } }, - }) - - // #when - session goes idle immediately after abort - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 3000)) - - // #then - no continuation injected (abort was immediately before idle) - expect(promptCalls).toHaveLength(0) - }) - - test("should clear abort state on user message and allow injection", async () => { - // #given - session with abort error, then user clears it - const sessionID = "main-error-clear" - setMainSession(sessionID) - - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - - // #when - abort error occurs - await hook.handler({ - event: { type: "session.error", properties: { sessionID, error: { message: "aborted" } } }, - }) - - // #when - user sends message (clears abort state) - await hook.handler({ - event: { type: "message.updated", properties: { info: { sessionID, role: "user" } } }, - }) - - // #when - session goes idle - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 2500)) - - // #then - continuation injected (abort state was cleared by user message) - expect(promptCalls.length).toBe(1) - }) test("should cancel countdown on user message after grace period", async () => { // #given - session starting countdown @@ -430,112 +393,11 @@ describe("todo-continuation-enforcer", () => { expect(promptCalls.length).toBe(2) }, { timeout: 15000 }) - // ============================================================ - // ABORT "IMMEDIATELY BEFORE" DETECTION TESTS - // These tests verify that abort errors only block continuation - // when they occur IMMEDIATELY before session.idle, not based - // on a time-based cooldown. - // ============================================================ - test("should skip injection ONLY when abort error occurs immediately before idle", async () => { - // #given - session with incomplete todos - const sessionID = "main-abort-immediate" - setMainSession(sessionID) - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - // #when - abort error occurs (with abort-specific error) - await hook.handler({ - event: { - type: "session.error", - properties: { - sessionID, - error: { name: "MessageAbortedError", message: "The operation was aborted" } - } - }, - }) - // #when - session goes idle IMMEDIATELY after abort (no other events in between) - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - await new Promise(r => setTimeout(r, 3000)) - - // #then - no continuation injected (abort was immediately before idle) - expect(promptCalls).toHaveLength(0) - }) - - test("should inject normally when abort error is followed by assistant activity before idle", async () => { - // #given - session with incomplete todos - const sessionID = "main-abort-then-assistant" - setMainSession(sessionID) - - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - - // #when - abort error occurs - await hook.handler({ - event: { - type: "session.error", - properties: { - sessionID, - error: { name: "MessageAbortedError", message: "The operation was aborted" } - } - }, - }) - - // #when - assistant sends a message (intervening event clears abort state) - await hook.handler({ - event: { - type: "message.updated", - properties: { info: { sessionID, role: "assistant" } } - }, - }) - - // #when - session goes idle (abort is no longer "immediately before") - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 2500)) - - // #then - continuation injected (abort was NOT immediately before idle) - expect(promptCalls.length).toBe(1) - }) - - test("should inject normally when abort error is followed by tool execution before idle", async () => { - // #given - session with incomplete todos - const sessionID = "main-abort-then-tool" - setMainSession(sessionID) - - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - - // #when - abort error occurs - await hook.handler({ - event: { - type: "session.error", - properties: { - sessionID, - error: { message: "aborted" } - } - }, - }) - - // #when - tool execution happens (intervening event) - await hook.handler({ - event: { type: "tool.execute.after", properties: { sessionID } }, - }) - - // #when - session goes idle - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 2500)) - - // #then - continuation injected (abort was NOT immediately before idle) - expect(promptCalls.length).toBe(1) - }) test("should NOT skip for non-abort errors even if immediately before idle", async () => { // #given - session with incomplete todos @@ -566,74 +428,105 @@ describe("todo-continuation-enforcer", () => { expect(promptCalls.length).toBe(1) }) - test("should inject after abort if time passes and new idle event occurs", async () => { - // #given - session with incomplete todos, abort happened previously - const sessionID = "main-abort-time-passed" + + + + + // ============================================================ + // API-BASED ABORT DETECTION TESTS + // These tests verify that abort is detected by checking + // the last assistant message's error field via session.messages API + // ============================================================ + + test("should skip injection when last assistant message has MessageAbortedError", async () => { + // #given - session where last assistant message was aborted + const sessionID = "main-api-abort" setMainSession(sessionID) + mockMessages = [ + { info: { id: "msg-1", role: "user" } }, + { info: { id: "msg-2", role: "assistant", error: { name: "MessageAbortedError", data: { message: "The operation was aborted" } } } }, + ] + const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - // #when - abort error occurs - await hook.handler({ - event: { - type: "session.error", - properties: { - sessionID, - error: { name: "AbortError", message: "cancelled" } - } - }, - }) - - // #when - first idle (immediately after abort) - should be skipped - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 3500)) - expect(promptCalls).toHaveLength(0) - - // #when - second idle event occurs (abort is no longer "immediately before") - await hook.handler({ - event: { type: "session.idle", properties: { sessionID } }, - }) - - await new Promise(r => setTimeout(r, 3500)) - - // #then - continuation injected on second idle (abort state was consumed) - expect(promptCalls.length).toBe(1) - }, { timeout: 15000 }) - - test("should handle multiple abort errors correctly - only last one matters", async () => { - // #given - session with incomplete todos - const sessionID = "main-multi-abort" - setMainSession(sessionID) - - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) - - // #when - first abort error - await hook.handler({ - event: { - type: "session.error", - properties: { sessionID, error: { message: "aborted" } } - }, - }) - - // #when - second abort error (immediately before idle) - await hook.handler({ - event: { - type: "session.error", - properties: { sessionID, error: { message: "interrupted" } } - }, - }) - - // #when - idle immediately after second abort + // #when - session goes idle await hook.handler({ event: { type: "session.idle", properties: { sessionID } }, }) await new Promise(r => setTimeout(r, 3000)) - // #then - no continuation (abort was immediately before) + // #then - no continuation (last message was aborted) + expect(promptCalls).toHaveLength(0) + }) + + test("should inject when last assistant message has no error", async () => { + // #given - session where last assistant message completed normally + const sessionID = "main-api-no-error" + setMainSession(sessionID) + + mockMessages = [ + { info: { id: "msg-1", role: "user" } }, + { info: { id: "msg-2", role: "assistant" } }, + ] + + const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) + + // #when - session goes idle + await hook.handler({ + event: { type: "session.idle", properties: { sessionID } }, + }) + + await new Promise(r => setTimeout(r, 3000)) + + // #then - continuation injected (no abort) + expect(promptCalls.length).toBe(1) + }) + + test("should inject when last message is from user (not assistant)", async () => { + // #given - session where last message is from user + const sessionID = "main-api-user-last" + setMainSession(sessionID) + + mockMessages = [ + { info: { id: "msg-1", role: "assistant" } }, + { info: { id: "msg-2", role: "user" } }, + ] + + const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) + + // #when - session goes idle + await hook.handler({ + event: { type: "session.idle", properties: { sessionID } }, + }) + + await new Promise(r => setTimeout(r, 3000)) + + // #then - continuation injected (last message is user, not aborted assistant) + expect(promptCalls.length).toBe(1) + }) + + test("should skip when last assistant message has any abort-like error", async () => { + // #given - session where last assistant message has AbortError (DOMException style) + const sessionID = "main-api-abort-dom" + setMainSession(sessionID) + + mockMessages = [ + { info: { id: "msg-1", role: "user" } }, + { info: { id: "msg-2", role: "assistant", error: { name: "AbortError" } } }, + ] + + const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) + + // #when - session goes idle + await hook.handler({ + event: { type: "session.idle", properties: { sessionID } }, + }) + + await new Promise(r => setTimeout(r, 3000)) + + // #then - no continuation (abort error detected) expect(promptCalls).toHaveLength(0) }) }) diff --git a/src/hooks/todo-continuation-enforcer.ts b/src/hooks/todo-continuation-enforcer.ts index 97f4970..5e16354 100644 --- a/src/hooks/todo-continuation-enforcer.ts +++ b/src/hooks/todo-continuation-enforcer.ts @@ -29,7 +29,6 @@ interface Todo { } interface SessionState { - lastEventWasAbortError?: boolean countdownTimer?: ReturnType countdownInterval?: ReturnType isRecovering?: boolean @@ -62,31 +61,30 @@ function getMessageDir(sessionID: string): string | null { return null } -function isAbortError(error: unknown): boolean { - if (!error) return false - - if (typeof error === "object") { - const errObj = error as Record - const name = errObj.name as string | undefined - const message = (errObj.message as string | undefined)?.toLowerCase() ?? "" - - if (name === "MessageAbortedError" || name === "AbortError") return true - if (name === "DOMException" && message.includes("abort")) return true - if (message.includes("aborted") || message.includes("cancelled") || message.includes("interrupted")) return true - } - - if (typeof error === "string") { - const lower = error.toLowerCase() - return lower.includes("abort") || lower.includes("cancel") || lower.includes("interrupt") - } - - return false -} - function getIncompleteCount(todos: Todo[]): number { return todos.filter(t => t.status !== "completed" && t.status !== "cancelled").length } +interface MessageInfo { + id?: string + role?: string + error?: { name?: string; data?: unknown } +} + +function isLastAssistantMessageAborted(messages: Array<{ info?: MessageInfo }>): boolean { + if (!messages || messages.length === 0) return false + + const assistantMessages = messages.filter(m => m.info?.role === "assistant") + if (assistantMessages.length === 0) return false + + const lastAssistant = assistantMessages[assistantMessages.length - 1] + const errorName = lastAssistant.info?.error?.name + + if (!errorName) return false + + return errorName === "MessageAbortedError" || errorName === "AbortError" +} + export function createTodoContinuationEnforcer( ctx: PluginInput, options: TodoContinuationEnforcerOptions = {} @@ -255,12 +253,8 @@ export function createTodoContinuationEnforcer( const sessionID = props?.sessionID as string | undefined if (!sessionID) return - const state = getState(sessionID) - const isAbort = isAbortError(props?.error) - state.lastEventWasAbortError = isAbort cancelCountdown(sessionID) - - log(`[${HOOK_NAME}] session.error`, { sessionID, isAbort }) + log(`[${HOOK_NAME}] session.error`, { sessionID }) return } @@ -286,12 +280,6 @@ export function createTodoContinuationEnforcer( return } - if (state.lastEventWasAbortError) { - state.lastEventWasAbortError = false - log(`[${HOOK_NAME}] Skipped: abort error immediately before idle`, { sessionID }) - return - } - const hasRunningBgTasks = backgroundManager ? backgroundManager.getTasksByParentSession(sessionID).some(t => t.status === "running") : false @@ -301,6 +289,21 @@ export function createTodoContinuationEnforcer( return } + try { + const messagesResp = await ctx.client.session.messages({ + path: { id: sessionID }, + query: { directory: ctx.directory }, + }) + const messages = (messagesResp as { data?: Array<{ info?: MessageInfo }> }).data ?? [] + + if (isLastAssistantMessageAborted(messages)) { + log(`[${HOOK_NAME}] Skipped: last assistant message was aborted`, { sessionID }) + return + } + } catch (err) { + log(`[${HOOK_NAME}] Messages fetch failed, continuing`, { sessionID, error: String(err) }) + } + let todos: Todo[] = [] try { const response = await ctx.client.session.todo({ path: { id: sessionID } }) @@ -332,12 +335,8 @@ export function createTodoContinuationEnforcer( if (!sessionID) return - const state = sessions.get(sessionID) - if (state) { - state.lastEventWasAbortError = false - } - if (role === "user") { + const state = sessions.get(sessionID) if (state?.countdownStartedAt) { const elapsed = Date.now() - state.countdownStartedAt if (elapsed < COUNTDOWN_GRACE_PERIOD_MS) { @@ -346,7 +345,6 @@ export function createTodoContinuationEnforcer( } } cancelCountdown(sessionID) - log(`[${HOOK_NAME}] User message: cleared abort state`, { sessionID }) } if (role === "assistant") { @@ -361,10 +359,6 @@ export function createTodoContinuationEnforcer( const role = info?.role as string | undefined if (sessionID && role === "assistant") { - const state = sessions.get(sessionID) - if (state) { - state.lastEventWasAbortError = false - } cancelCountdown(sessionID) } return @@ -373,10 +367,6 @@ export function createTodoContinuationEnforcer( if (event.type === "tool.execute.before" || event.type === "tool.execute.after") { const sessionID = props?.sessionID as string | undefined if (sessionID) { - const state = sessions.get(sessionID) - if (state) { - state.lastEventWasAbortError = false - } cancelCountdown(sessionID) } return