Merge pull request #1878 from code-yeongyu/fix/1806-todo-enforcer-cooldown
fix: apply cooldown on injection failure and add max retry limit (#1806)
This commit is contained in:
commit
7e6982c8d8
@ -18,3 +18,5 @@ export const COUNTDOWN_GRACE_PERIOD_MS = 500
|
||||
|
||||
export const ABORT_WINDOW_MS = 3000
|
||||
export const CONTINUATION_COOLDOWN_MS = 30_000
|
||||
export const MAX_CONSECUTIVE_FAILURES = 5
|
||||
export const FAILURE_RESET_WINDOW_MS = 5 * 60 * 1000
|
||||
|
||||
@ -141,11 +141,14 @@ ${todoList}`
|
||||
if (injectionState) {
|
||||
injectionState.inFlight = false
|
||||
injectionState.lastInjectedAt = Date.now()
|
||||
injectionState.consecutiveFailures = 0
|
||||
}
|
||||
} catch (error) {
|
||||
log(`[${HOOK_NAME}] Injection failed`, { sessionID, error: String(error) })
|
||||
if (injectionState) {
|
||||
injectionState.inFlight = false
|
||||
injectionState.lastInjectedAt = Date.now()
|
||||
injectionState.consecutiveFailures = (injectionState.consecutiveFailures ?? 0) + 1
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -8,7 +8,9 @@ import {
|
||||
ABORT_WINDOW_MS,
|
||||
CONTINUATION_COOLDOWN_MS,
|
||||
DEFAULT_SKIP_AGENTS,
|
||||
FAILURE_RESET_WINDOW_MS,
|
||||
HOOK_NAME,
|
||||
MAX_CONSECUTIVE_FAILURES,
|
||||
} from "./constants"
|
||||
import { isLastAssistantMessageAborted } from "./abort-detection"
|
||||
import { getIncompleteCount } from "./todo"
|
||||
@ -99,8 +101,35 @@ export async function handleSessionIdle(args: {
|
||||
return
|
||||
}
|
||||
|
||||
if (state.lastInjectedAt && Date.now() - state.lastInjectedAt < CONTINUATION_COOLDOWN_MS) {
|
||||
log(`[${HOOK_NAME}] Skipped: cooldown active`, { sessionID })
|
||||
if (
|
||||
state.consecutiveFailures >= MAX_CONSECUTIVE_FAILURES
|
||||
&& state.lastInjectedAt
|
||||
&& Date.now() - state.lastInjectedAt >= FAILURE_RESET_WINDOW_MS
|
||||
) {
|
||||
state.consecutiveFailures = 0
|
||||
log(`[${HOOK_NAME}] Reset consecutive failures after recovery window`, {
|
||||
sessionID,
|
||||
failureResetWindowMs: FAILURE_RESET_WINDOW_MS,
|
||||
})
|
||||
}
|
||||
|
||||
if (state.consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) {
|
||||
log(`[${HOOK_NAME}] Skipped: max consecutive failures reached`, {
|
||||
sessionID,
|
||||
consecutiveFailures: state.consecutiveFailures,
|
||||
maxConsecutiveFailures: MAX_CONSECUTIVE_FAILURES,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
const effectiveCooldown =
|
||||
CONTINUATION_COOLDOWN_MS * Math.pow(2, Math.min(state.consecutiveFailures, 5))
|
||||
if (state.lastInjectedAt && Date.now() - state.lastInjectedAt < effectiveCooldown) {
|
||||
log(`[${HOOK_NAME}] Skipped: cooldown active`, {
|
||||
sessionID,
|
||||
effectiveCooldown,
|
||||
consecutiveFailures: state.consecutiveFailures,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@ -45,7 +45,9 @@ export function createSessionStateStore(): SessionStateStore {
|
||||
return existing.state
|
||||
}
|
||||
|
||||
const state: SessionState = {}
|
||||
const state: SessionState = {
|
||||
consecutiveFailures: 0,
|
||||
}
|
||||
sessions.set(sessionID, { state, lastAccessedAt: Date.now() })
|
||||
return state
|
||||
}
|
||||
|
||||
@ -4,7 +4,11 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"
|
||||
import type { BackgroundManager } from "../../features/background-agent"
|
||||
import { setMainSession, subagentSessions, _resetForTesting } from "../../features/claude-code-session-state"
|
||||
import { createTodoContinuationEnforcer } from "."
|
||||
import { CONTINUATION_COOLDOWN_MS } from "./constants"
|
||||
import {
|
||||
CONTINUATION_COOLDOWN_MS,
|
||||
FAILURE_RESET_WINDOW_MS,
|
||||
MAX_CONSECUTIVE_FAILURES,
|
||||
} from "./constants"
|
||||
|
||||
type TimerCallback = (...args: any[]) => void
|
||||
|
||||
@ -164,6 +168,15 @@ describe("todo-continuation-enforcer", () => {
|
||||
}
|
||||
}
|
||||
|
||||
interface PromptRequestOptions {
|
||||
path: { id: string }
|
||||
body: {
|
||||
agent?: string
|
||||
model?: { providerID?: string; modelID?: string }
|
||||
parts: Array<{ text: string }>
|
||||
}
|
||||
}
|
||||
|
||||
let mockMessages: MockMessage[] = []
|
||||
|
||||
function createMockPluginInput() {
|
||||
@ -551,6 +564,164 @@ describe("todo-continuation-enforcer", () => {
|
||||
expect(promptCalls).toHaveLength(2)
|
||||
}, { timeout: 15000 })
|
||||
|
||||
test("should apply cooldown even after injection failure", async () => {
|
||||
//#given
|
||||
const sessionID = "main-failure-cooldown"
|
||||
setMainSession(sessionID)
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.promptAsync = async (opts: PromptRequestOptions) => {
|
||||
promptCalls.push({
|
||||
sessionID: opts.path.id,
|
||||
agent: opts.body.agent,
|
||||
model: opts.body.model,
|
||||
text: opts.body.parts[0].text,
|
||||
})
|
||||
throw new Error("simulated auth failure")
|
||||
}
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
//#when
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
test("should stop retries after max consecutive failures", async () => {
|
||||
//#given
|
||||
const sessionID = "main-max-consecutive-failures"
|
||||
setMainSession(sessionID)
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.promptAsync = async (opts: PromptRequestOptions) => {
|
||||
promptCalls.push({
|
||||
sessionID: opts.path.id,
|
||||
agent: opts.body.agent,
|
||||
model: opts.body.model,
|
||||
text: opts.body.parts[0].text,
|
||||
})
|
||||
throw new Error("simulated auth failure")
|
||||
}
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
//#when
|
||||
for (let index = 0; index < MAX_CONSECUTIVE_FAILURES; index++) {
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
if (index < MAX_CONSECUTIVE_FAILURES - 1) {
|
||||
await fakeTimers.advanceClockBy(1_000_000)
|
||||
}
|
||||
}
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(MAX_CONSECUTIVE_FAILURES)
|
||||
}, { timeout: 30000 })
|
||||
|
||||
test("should resume retries after reset window when max failures reached", async () => {
|
||||
//#given
|
||||
const sessionID = "main-recovery-after-max-failures"
|
||||
setMainSession(sessionID)
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.promptAsync = async (opts: PromptRequestOptions) => {
|
||||
promptCalls.push({
|
||||
sessionID: opts.path.id,
|
||||
agent: opts.body.agent,
|
||||
model: opts.body.model,
|
||||
text: opts.body.parts[0].text,
|
||||
})
|
||||
throw new Error("simulated auth failure")
|
||||
}
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
//#when
|
||||
for (let index = 0; index < MAX_CONSECUTIVE_FAILURES; index++) {
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
if (index < MAX_CONSECUTIVE_FAILURES - 1) {
|
||||
await fakeTimers.advanceClockBy(1_000_000)
|
||||
}
|
||||
}
|
||||
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
await fakeTimers.advanceClockBy(FAILURE_RESET_WINDOW_MS)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(MAX_CONSECUTIVE_FAILURES + 1)
|
||||
}, { timeout: 30000 })
|
||||
|
||||
test("should increase cooldown exponentially after consecutive failures", async () => {
|
||||
//#given
|
||||
const sessionID = "main-exponential-backoff"
|
||||
setMainSession(sessionID)
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.promptAsync = async (opts: PromptRequestOptions) => {
|
||||
promptCalls.push({
|
||||
sessionID: opts.path.id,
|
||||
agent: opts.body.agent,
|
||||
model: opts.body.model,
|
||||
text: opts.body.parts[0].text,
|
||||
})
|
||||
throw new Error("simulated auth failure")
|
||||
}
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
//#when
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
await fakeTimers.advanceClockBy(CONTINUATION_COOLDOWN_MS)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
await fakeTimers.advanceClockBy(CONTINUATION_COOLDOWN_MS)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(2)
|
||||
}, { timeout: 30000 })
|
||||
|
||||
test("should reset consecutive failure count after successful injection", async () => {
|
||||
//#given
|
||||
const sessionID = "main-reset-consecutive-failures"
|
||||
setMainSession(sessionID)
|
||||
let shouldFail = true
|
||||
const mockInput = createMockPluginInput()
|
||||
mockInput.client.session.promptAsync = async (opts: PromptRequestOptions) => {
|
||||
promptCalls.push({
|
||||
sessionID: opts.path.id,
|
||||
agent: opts.body.agent,
|
||||
model: opts.body.model,
|
||||
text: opts.body.parts[0].text,
|
||||
})
|
||||
if (shouldFail) {
|
||||
shouldFail = false
|
||||
throw new Error("simulated auth failure")
|
||||
}
|
||||
return {}
|
||||
}
|
||||
const hook = createTodoContinuationEnforcer(mockInput, {})
|
||||
|
||||
//#when
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
await fakeTimers.advanceClockBy(CONTINUATION_COOLDOWN_MS * 2)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
await fakeTimers.advanceClockBy(CONTINUATION_COOLDOWN_MS)
|
||||
await hook.handler({ event: { type: "session.idle", properties: { sessionID } } })
|
||||
await fakeTimers.advanceBy(2500, true)
|
||||
|
||||
//#then
|
||||
expect(promptCalls).toHaveLength(3)
|
||||
}, { timeout: 30000 })
|
||||
|
||||
test("should keep injecting even when todos remain unchanged across cycles", async () => {
|
||||
//#given
|
||||
const sessionID = "main-no-stagnation-cap"
|
||||
|
||||
@ -29,6 +29,7 @@ export interface SessionState {
|
||||
abortDetectedAt?: number
|
||||
lastInjectedAt?: number
|
||||
inFlight?: boolean
|
||||
consecutiveFailures: number
|
||||
}
|
||||
|
||||
export interface MessageInfo {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user