Merge pull request #3065 from code-yeongyu/fix/issue-2984-v2
fix: reset consecutiveFailures on abort so session recovers after user cancel
This commit is contained in:
@@ -61,6 +61,11 @@ export async function injectContinuation(args: {
|
||||
return
|
||||
}
|
||||
|
||||
if (state?.wasCancelled) {
|
||||
log(`[${HOOK_NAME}] Skipped injection: session was cancelled`, { sessionID })
|
||||
return
|
||||
}
|
||||
|
||||
if (isContinuationStopped?.(sessionID)) {
|
||||
log(`[${HOOK_NAME}] Skipped injection: continuation stopped for session`, { sessionID })
|
||||
return
|
||||
@@ -145,6 +150,11 @@ Remaining tasks:
|
||||
${todoList}`
|
||||
|
||||
const injectionState = sessionStateStore.getExistingState(sessionID)
|
||||
if (injectionState?.wasCancelled) {
|
||||
log(`[${HOOK_NAME}] Skipped injection: session was cancelled before prompt`, { sessionID })
|
||||
return
|
||||
}
|
||||
|
||||
if (injectionState) {
|
||||
injectionState.inFlight = true
|
||||
}
|
||||
|
||||
@@ -37,7 +37,13 @@ export function createTodoContinuationHandler(args: {
|
||||
const error = props?.error as { name?: string } | undefined
|
||||
if (error?.name === "MessageAbortedError" || error?.name === "AbortError") {
|
||||
const state = sessionStateStore.getState(sessionID)
|
||||
state.wasCancelled = true
|
||||
state.abortDetectedAt = Date.now()
|
||||
state.lastIncompleteCount = undefined
|
||||
state.lastInjectedAt = undefined
|
||||
state.awaitingPostInjectionProgressCheck = false
|
||||
state.stagnationCount = 0
|
||||
state.consecutiveFailures = 0
|
||||
log(`[${HOOK_NAME}] Abort detected via session.error`, { sessionID, errorName: error.name })
|
||||
}
|
||||
|
||||
|
||||
@@ -42,6 +42,11 @@ export async function handleSessionIdle(args: {
|
||||
return
|
||||
}
|
||||
|
||||
if (state.wasCancelled) {
|
||||
log(`[${HOOK_NAME}] Skipped: session was cancelled`, { sessionID })
|
||||
return
|
||||
}
|
||||
|
||||
if (state.abortDetectedAt) {
|
||||
const timeSinceAbort = Date.now() - state.abortDetectedAt
|
||||
if (timeSinceAbort < ABORT_WINDOW_MS) {
|
||||
|
||||
@@ -25,14 +25,20 @@ export function handleNonIdleEvent(args: {
|
||||
return
|
||||
}
|
||||
}
|
||||
if (state) state.abortDetectedAt = undefined
|
||||
if (state) {
|
||||
state.abortDetectedAt = undefined
|
||||
state.wasCancelled = false
|
||||
}
|
||||
sessionStateStore.cancelCountdown(sessionID)
|
||||
return
|
||||
}
|
||||
|
||||
if (role === "assistant") {
|
||||
const state = sessionStateStore.getExistingState(sessionID)
|
||||
if (state) state.abortDetectedAt = undefined
|
||||
if (state) {
|
||||
state.abortDetectedAt = undefined
|
||||
state.wasCancelled = false
|
||||
}
|
||||
sessionStateStore.cancelCountdown(sessionID)
|
||||
return
|
||||
}
|
||||
@@ -60,7 +66,10 @@ export function handleNonIdleEvent(args: {
|
||||
const sessionID = properties?.sessionID as string | undefined
|
||||
if (sessionID) {
|
||||
const state = sessionStateStore.getExistingState(sessionID)
|
||||
if (state) state.abortDetectedAt = undefined
|
||||
if (state) {
|
||||
state.abortDetectedAt = undefined
|
||||
state.wasCancelled = false
|
||||
}
|
||||
sessionStateStore.cancelCountdown(sessionID)
|
||||
}
|
||||
return
|
||||
@@ -70,7 +79,10 @@ export function handleNonIdleEvent(args: {
|
||||
const sessionID = properties?.sessionID as string | undefined
|
||||
if (sessionID) {
|
||||
const state = sessionStateStore.getExistingState(sessionID)
|
||||
if (state) state.abortDetectedAt = undefined
|
||||
if (state) {
|
||||
state.abortDetectedAt = undefined
|
||||
state.wasCancelled = false
|
||||
}
|
||||
sessionStateStore.cancelCountdown(sessionID)
|
||||
}
|
||||
return
|
||||
|
||||
@@ -1270,7 +1270,7 @@ describe("todo-continuation-enforcer", () => {
|
||||
expect(promptCalls).toHaveLength(0)
|
||||
})
|
||||
|
||||
test("should inject when abort flag is stale (>3s old)", async () => {
|
||||
test("should keep skipping after cancel even when the abort window is stale", async () => {
|
||||
fakeTimers.restore()
|
||||
// given - session with incomplete todos and old abort timestamp
|
||||
const sessionID = "main-stale-abort"
|
||||
@@ -1299,8 +1299,7 @@ describe("todo-continuation-enforcer", () => {
|
||||
|
||||
await wait(3000)
|
||||
|
||||
// then - continuation injected (abort flag is stale)
|
||||
expect(promptCalls.length).toBeGreaterThan(0)
|
||||
expect(promptCalls).toHaveLength(0)
|
||||
}, { timeout: 15000 })
|
||||
|
||||
test("should clear abort flag on user message activity", async () => {
|
||||
@@ -1343,6 +1342,44 @@ describe("todo-continuation-enforcer", () => {
|
||||
expect(promptCalls.length).toBeGreaterThan(0)
|
||||
}, { timeout: 15000 })
|
||||
|
||||
test("should reset failure state and keep skipping after a cancelled run", async () => {
|
||||
fakeTimers.restore()
|
||||
const sessionID = "main-reset-after-cancel"
|
||||
setMainSession(sessionID)
|
||||
mockMessages = [
|
||||
{ info: { id: "msg-1", role: "user" } },
|
||||
{ info: { id: "msg-2", role: "assistant" } },
|
||||
]
|
||||
|
||||
const hook = createTodoContinuationEnforcer(createMockPluginInput(), {})
|
||||
|
||||
await hook.handler({
|
||||
event: { type: "session.idle", properties: { sessionID } },
|
||||
})
|
||||
|
||||
await wait(2500)
|
||||
expect(promptCalls.length).toBeGreaterThan(0)
|
||||
|
||||
promptCalls.length = 0
|
||||
|
||||
await hook.handler({
|
||||
event: {
|
||||
type: "session.error",
|
||||
properties: { sessionID, error: { name: "MessageAbortedError" } },
|
||||
},
|
||||
})
|
||||
|
||||
await wait(3100)
|
||||
|
||||
await hook.handler({
|
||||
event: { type: "session.idle", properties: { sessionID } },
|
||||
})
|
||||
|
||||
await wait(2500)
|
||||
|
||||
expect(promptCalls).toHaveLength(0)
|
||||
}, { timeout: 15000 })
|
||||
|
||||
test("should clear abort flag on assistant message activity", async () => {
|
||||
fakeTimers.restore()
|
||||
// given - session with abort detected
|
||||
@@ -1866,4 +1903,64 @@ describe("todo-continuation-enforcer", () => {
|
||||
expect(promptCalls).toHaveLength(0)
|
||||
})
|
||||
|
||||
test("should reset consecutiveFailures after user-initiated abort and resume after fresh activity [regression #2984]", async () => {
|
||||
fakeTimers.restore()
|
||||
const sessionID = "main-abort-recovery"
|
||||
setMainSession(sessionID)
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.todo = async () => ({
|
||||
data: [
|
||||
{ id: "1", content: "Write tests", status: "pending", priority: "high" },
|
||||
],
|
||||
})
|
||||
|
||||
let shouldFail = true
|
||||
let promptCallCount = 0
|
||||
mockInput.client.session.promptAsync = async (_opts: PromptRequestOptions) => {
|
||||
promptCallCount++
|
||||
if (shouldFail) {
|
||||
throw new Error("promptAsync failed (3ms) unknown error")
|
||||
}
|
||||
promptCalls.push({
|
||||
sessionID: _opts.path.id,
|
||||
agent: _opts.body.agent,
|
||||
model: _opts.body.model,
|
||||
text: _opts.body.parts[0].text,
|
||||
})
|
||||
}
|
||||
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await wait(2500)
|
||||
expect(promptCallCount).toBe(1)
|
||||
|
||||
await hook.handler({
|
||||
event: {
|
||||
type: "session.error",
|
||||
properties: { sessionID, error: { name: "MessageAbortedError" } },
|
||||
},
|
||||
})
|
||||
|
||||
shouldFail = false
|
||||
await wait(9000)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await wait(2500)
|
||||
expect(promptCallCount).toBe(1)
|
||||
|
||||
await hook.handler({
|
||||
event: {
|
||||
type: "message.updated",
|
||||
properties: { info: { sessionID, role: "user" } },
|
||||
},
|
||||
})
|
||||
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await wait(2500)
|
||||
|
||||
expect(promptCallCount).toBe(2)
|
||||
expect(promptCalls).toHaveLength(1)
|
||||
}, { timeout: 20000 })
|
||||
|
||||
|
||||
})
|
||||
|
||||
@@ -26,6 +26,7 @@ export interface SessionState {
|
||||
countdownTimer?: ReturnType<typeof setTimeout>
|
||||
countdownInterval?: ReturnType<typeof setInterval>
|
||||
isRecovering?: boolean
|
||||
wasCancelled?: boolean
|
||||
countdownStartedAt?: number
|
||||
abortDetectedAt?: number
|
||||
lastIncompleteCount?: number
|
||||
|
||||
Reference in New Issue
Block a user