fix(todo-continuation-enforcer): replace time-based cooldown with event-order abort detection
Replace the time-based ERROR_COOLDOWN_MS mechanism with an event-order based lastEventWasAbortError flag. This fixes the bug where abort errors permanently block todo continuation since session.idle only fires once during the cooldown. Now abort errors only skip continuation when they occur IMMEDIATELY before session.idle event. Any intervening event (message, tool execution) clears the abort state, allowing normal continuation flow on the next idle. Comprehensive tests added to verify: - Abort detection correctly blocks continuation when immediately before idle - Intervening events (assistant message, tool execution) clear abort state - Non-abort errors don't block continuation - Multiple abort events (only last one matters) - No time-based throttle preventing consecutive injections 🤖 Generated with assistance of [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
This commit is contained in:
@@ -164,42 +164,42 @@ describe("todo-continuation-enforcer", () => {
|
|||||||
expect(promptCalls[0].sessionID).toBe(bgTaskSession)
|
expect(promptCalls[0].sessionID).toBe(bgTaskSession)
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should skip injection after recent error", async () => {
|
test("should skip injection when abort error occurs immediately before idle", async () => {
|
||||||
// #given - session that just had an error
|
// #given - session that just had an abort error
|
||||||
const sessionID = "main-error"
|
const sessionID = "main-error"
|
||||||
setMainSession(sessionID)
|
setMainSession(sessionID)
|
||||||
|
|
||||||
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
||||||
|
|
||||||
// #when - session error occurs
|
// #when - abort error occurs
|
||||||
await hook.handler({
|
await hook.handler({
|
||||||
event: { type: "session.error", properties: { sessionID, error: new Error("test") } },
|
event: { type: "session.error", properties: { sessionID, error: { name: "AbortError", message: "aborted" } } },
|
||||||
})
|
})
|
||||||
|
|
||||||
// #when - session goes idle immediately after
|
// #when - session goes idle immediately after abort
|
||||||
await hook.handler({
|
await hook.handler({
|
||||||
event: { type: "session.idle", properties: { sessionID } },
|
event: { type: "session.idle", properties: { sessionID } },
|
||||||
})
|
})
|
||||||
|
|
||||||
await new Promise(r => setTimeout(r, 3000))
|
await new Promise(r => setTimeout(r, 3000))
|
||||||
|
|
||||||
// #then - no continuation injected (error cooldown)
|
// #then - no continuation injected (abort was immediately before idle)
|
||||||
expect(promptCalls).toHaveLength(0)
|
expect(promptCalls).toHaveLength(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should clear error state on user message and allow injection", async () => {
|
test("should clear abort state on user message and allow injection", async () => {
|
||||||
// #given - session with error, then user clears it
|
// #given - session with abort error, then user clears it
|
||||||
const sessionID = "main-error-clear"
|
const sessionID = "main-error-clear"
|
||||||
setMainSession(sessionID)
|
setMainSession(sessionID)
|
||||||
|
|
||||||
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
||||||
|
|
||||||
// #when - error occurs
|
// #when - abort error occurs
|
||||||
await hook.handler({
|
await hook.handler({
|
||||||
event: { type: "session.error", properties: { sessionID } },
|
event: { type: "session.error", properties: { sessionID, error: { message: "aborted" } } },
|
||||||
})
|
})
|
||||||
|
|
||||||
// #when - user sends message (clears error immediately)
|
// #when - user sends message (clears abort state)
|
||||||
await hook.handler({
|
await hook.handler({
|
||||||
event: { type: "message.updated", properties: { info: { sessionID, role: "user" } } },
|
event: { type: "message.updated", properties: { info: { sessionID, role: "user" } } },
|
||||||
})
|
})
|
||||||
@@ -211,7 +211,7 @@ describe("todo-continuation-enforcer", () => {
|
|||||||
|
|
||||||
await new Promise(r => setTimeout(r, 2500))
|
await new Promise(r => setTimeout(r, 2500))
|
||||||
|
|
||||||
// #then - continuation injected (error was cleared by user message)
|
// #then - continuation injected (abort state was cleared by user message)
|
||||||
expect(promptCalls.length).toBe(1)
|
expect(promptCalls.length).toBe(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -401,4 +401,211 @@ describe("todo-continuation-enforcer", () => {
|
|||||||
// #then - second injection also happened (no throttle blocking)
|
// #then - second injection also happened (no throttle blocking)
|
||||||
expect(promptCalls.length).toBe(2)
|
expect(promptCalls.length).toBe(2)
|
||||||
}, { timeout: 10000 })
|
}, { timeout: 10000 })
|
||||||
|
|
||||||
|
// ============================================================
|
||||||
|
// 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
|
||||||
|
const sessionID = "main-noabort-error"
|
||||||
|
setMainSession(sessionID)
|
||||||
|
|
||||||
|
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
||||||
|
|
||||||
|
// #when - non-abort error occurs (e.g., network error, API error)
|
||||||
|
await hook.handler({
|
||||||
|
event: {
|
||||||
|
type: "session.error",
|
||||||
|
properties: {
|
||||||
|
sessionID,
|
||||||
|
error: { name: "NetworkError", message: "Connection failed" }
|
||||||
|
}
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// #when - session goes idle immediately after
|
||||||
|
await hook.handler({
|
||||||
|
event: { type: "session.idle", properties: { sessionID } },
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise(r => setTimeout(r, 2500))
|
||||||
|
|
||||||
|
// #then - continuation injected (non-abort errors don't block)
|
||||||
|
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"
|
||||||
|
setMainSession(sessionID)
|
||||||
|
|
||||||
|
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, 3000))
|
||||||
|
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, 2500))
|
||||||
|
|
||||||
|
// #then - continuation injected on second idle (abort state was consumed)
|
||||||
|
expect(promptCalls.length).toBe(1)
|
||||||
|
}, { timeout: 10000 })
|
||||||
|
|
||||||
|
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
|
||||||
|
await hook.handler({
|
||||||
|
event: { type: "session.idle", properties: { sessionID } },
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise(r => setTimeout(r, 3000))
|
||||||
|
|
||||||
|
// #then - no continuation (abort was immediately before)
|
||||||
|
expect(promptCalls).toHaveLength(0)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ interface Todo {
|
|||||||
}
|
}
|
||||||
|
|
||||||
interface SessionState {
|
interface SessionState {
|
||||||
lastErrorAt?: number
|
lastEventWasAbortError?: boolean
|
||||||
countdownTimer?: ReturnType<typeof setTimeout>
|
countdownTimer?: ReturnType<typeof setTimeout>
|
||||||
countdownInterval?: ReturnType<typeof setInterval>
|
countdownInterval?: ReturnType<typeof setInterval>
|
||||||
isRecovering?: boolean
|
isRecovering?: boolean
|
||||||
@@ -45,7 +45,6 @@ Incomplete tasks remain in your todo list. Continue working on the next pending
|
|||||||
|
|
||||||
const COUNTDOWN_SECONDS = 2
|
const COUNTDOWN_SECONDS = 2
|
||||||
const TOAST_DURATION_MS = 900
|
const TOAST_DURATION_MS = 900
|
||||||
const ERROR_COOLDOWN_MS = 3_000
|
|
||||||
|
|
||||||
function getMessageDir(sessionID: string): string | null {
|
function getMessageDir(sessionID: string): string | null {
|
||||||
if (!existsSync(MESSAGE_STORAGE)) return null
|
if (!existsSync(MESSAGE_STORAGE)) return null
|
||||||
@@ -155,10 +154,7 @@ export function createTodoContinuationEnforcer(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if (state?.lastErrorAt && Date.now() - state.lastErrorAt < ERROR_COOLDOWN_MS) {
|
|
||||||
log(`[${HOOK_NAME}] Skipped injection: recent error`, { sessionID })
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
const hasRunningBgTasks = backgroundManager
|
const hasRunningBgTasks = backgroundManager
|
||||||
? backgroundManager.getTasksByParentSession(sessionID).some(t => t.status === "running")
|
? backgroundManager.getTasksByParentSession(sessionID).some(t => t.status === "running")
|
||||||
@@ -251,10 +247,11 @@ export function createTodoContinuationEnforcer(
|
|||||||
if (!sessionID) return
|
if (!sessionID) return
|
||||||
|
|
||||||
const state = getState(sessionID)
|
const state = getState(sessionID)
|
||||||
state.lastErrorAt = Date.now()
|
const isAbort = isAbortError(props?.error)
|
||||||
|
state.lastEventWasAbortError = isAbort
|
||||||
cancelCountdown(sessionID)
|
cancelCountdown(sessionID)
|
||||||
|
|
||||||
log(`[${HOOK_NAME}] session.error`, { sessionID, isAbort: isAbortError(props?.error) })
|
log(`[${HOOK_NAME}] session.error`, { sessionID, isAbort })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -280,8 +277,9 @@ export function createTodoContinuationEnforcer(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if (state.lastErrorAt && Date.now() - state.lastErrorAt < ERROR_COOLDOWN_MS) {
|
if (state.lastEventWasAbortError) {
|
||||||
log(`[${HOOK_NAME}] Skipped: recent error (cooldown)`, { sessionID })
|
state.lastEventWasAbortError = false
|
||||||
|
log(`[${HOOK_NAME}] Skipped: abort error immediately before idle`, { sessionID })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -325,13 +323,14 @@ export function createTodoContinuationEnforcer(
|
|||||||
|
|
||||||
if (!sessionID) return
|
if (!sessionID) return
|
||||||
|
|
||||||
|
const state = sessions.get(sessionID)
|
||||||
|
if (state) {
|
||||||
|
state.lastEventWasAbortError = false
|
||||||
|
}
|
||||||
|
|
||||||
if (role === "user") {
|
if (role === "user") {
|
||||||
const state = sessions.get(sessionID)
|
|
||||||
if (state) {
|
|
||||||
state.lastErrorAt = undefined
|
|
||||||
}
|
|
||||||
cancelCountdown(sessionID)
|
cancelCountdown(sessionID)
|
||||||
log(`[${HOOK_NAME}] User message: cleared error state`, { sessionID })
|
log(`[${HOOK_NAME}] User message: cleared abort state`, { sessionID })
|
||||||
}
|
}
|
||||||
|
|
||||||
if (role === "assistant") {
|
if (role === "assistant") {
|
||||||
@@ -346,6 +345,10 @@ export function createTodoContinuationEnforcer(
|
|||||||
const role = info?.role as string | undefined
|
const role = info?.role as string | undefined
|
||||||
|
|
||||||
if (sessionID && role === "assistant") {
|
if (sessionID && role === "assistant") {
|
||||||
|
const state = sessions.get(sessionID)
|
||||||
|
if (state) {
|
||||||
|
state.lastEventWasAbortError = false
|
||||||
|
}
|
||||||
cancelCountdown(sessionID)
|
cancelCountdown(sessionID)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
@@ -354,6 +357,10 @@ export function createTodoContinuationEnforcer(
|
|||||||
if (event.type === "tool.execute.before" || event.type === "tool.execute.after") {
|
if (event.type === "tool.execute.before" || event.type === "tool.execute.after") {
|
||||||
const sessionID = props?.sessionID as string | undefined
|
const sessionID = props?.sessionID as string | undefined
|
||||||
if (sessionID) {
|
if (sessionID) {
|
||||||
|
const state = sessions.get(sessionID)
|
||||||
|
if (state) {
|
||||||
|
state.lastEventWasAbortError = false
|
||||||
|
}
|
||||||
cancelCountdown(sessionID)
|
cancelCountdown(sessionID)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
|
|||||||
Reference in New Issue
Block a user