fix(todo-continuation-enforcer): redesign with version-token state machine
Fixes race conditions, enables continuous enforcement, and eliminates false positives/negatives. - Complete redesign using version-token state machine for race condition prevention - Replaced 5 separate Sets with single Map<sessionID, SessionState> - Changed cancelCountdown() to invalidate() that ALWAYS bumps version regardless of mode - Added background task check BEFORE starting countdown (prevents toast spam when bg tasks running) - Added lastAttemptedAt throttling (10s minimum between attempts, set BEFORE API call) - Removed non-interactive preemptive injection (all paths now use countdown) - Added 3 version checks in executeInjection (start, after todo fetch, before API call) - Removed remindedSessions flag for continuous enforcement Fixes: 1. Race condition where session.idle fired before message.updated cleared reminded state 2. Single-shot behavior that prevented multiple reminders 3. Phantom reminders sent even after agent started working 4. Toast spam when background tasks are running 🤖 Generated with assistance of OhMyOpenCode (https://github.com/code-yeongyu/oh-my-opencode)
This commit is contained in:
@@ -55,7 +55,7 @@ interface SessionState {
|
|||||||
version: number // Monotonic generation token - increment to invalidate pending callbacks
|
version: number // Monotonic generation token - increment to invalidate pending callbacks
|
||||||
mode: SessionMode
|
mode: SessionMode
|
||||||
timer?: ReturnType<typeof setTimeout> // Pending countdown timer
|
timer?: ReturnType<typeof setTimeout> // Pending countdown timer
|
||||||
lastInjectedAt?: number // Timestamp of last injection (anti-spam)
|
lastAttemptedAt?: number // Timestamp of last injection attempt (throttle all attempts)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
@@ -131,18 +131,22 @@ export function createTodoContinuationEnforcer(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Cancel any pending countdown by incrementing version and clearing timer.
|
* Invalidate any pending or in-flight operation by incrementing version.
|
||||||
* This invalidates any async callbacks that were started with the old version.
|
* ALWAYS bumps version regardless of current mode to prevent last-mile races.
|
||||||
*/
|
*/
|
||||||
function cancelCountdown(sessionID: string, reason: string): void {
|
function invalidate(sessionID: string, reason: string): void {
|
||||||
const state = sessions.get(sessionID)
|
const state = sessions.get(sessionID)
|
||||||
if (!state) return
|
if (!state) return
|
||||||
|
|
||||||
if (state.mode === "countingDown" || state.timer) {
|
// Skip if in recovery mode (external control)
|
||||||
state.version++
|
if (state.mode === "recovering") return
|
||||||
clearTimer(state)
|
|
||||||
|
state.version++
|
||||||
|
clearTimer(state)
|
||||||
|
|
||||||
|
if (state.mode !== "idle" && state.mode !== "errorBypass") {
|
||||||
|
log(`[${HOOK_NAME}] Invalidated`, { sessionID, reason, prevMode: state.mode, newVersion: state.version })
|
||||||
state.mode = "idle"
|
state.mode = "idle"
|
||||||
log(`[${HOOK_NAME}] Countdown cancelled`, { sessionID, reason, newVersion: state.version })
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -161,7 +165,7 @@ export function createTodoContinuationEnforcer(
|
|||||||
|
|
||||||
const markRecovering = (sessionID: string): void => {
|
const markRecovering = (sessionID: string): void => {
|
||||||
const state = getOrCreateState(sessionID)
|
const state = getOrCreateState(sessionID)
|
||||||
cancelCountdown(sessionID, "entering recovery mode")
|
invalidate(sessionID, "entering recovery mode")
|
||||||
state.mode = "recovering"
|
state.mode = "recovering"
|
||||||
log(`[${HOOK_NAME}] Session marked as recovering`, { sessionID })
|
log(`[${HOOK_NAME}] Session marked as recovering`, { sessionID })
|
||||||
}
|
}
|
||||||
@@ -213,9 +217,9 @@ export function createTodoContinuationEnforcer(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Throttle check: minimum interval between injections
|
// Throttle check: minimum interval between injection attempts
|
||||||
if (state.lastInjectedAt) {
|
if (state.lastAttemptedAt) {
|
||||||
const elapsed = Date.now() - state.lastInjectedAt
|
const elapsed = Date.now() - state.lastAttemptedAt
|
||||||
if (elapsed < MIN_INJECTION_INTERVAL_MS) {
|
if (elapsed < MIN_INJECTION_INTERVAL_MS) {
|
||||||
log(`[${HOOK_NAME}] Injection throttled: too soon since last injection`, {
|
log(`[${HOOK_NAME}] Injection throttled: too soon since last injection`, {
|
||||||
sessionID, elapsedMs: elapsed, minIntervalMs: MIN_INJECTION_INTERVAL_MS
|
sessionID, elapsedMs: elapsed, minIntervalMs: MIN_INJECTION_INTERVAL_MS
|
||||||
@@ -299,6 +303,9 @@ export function createTodoContinuationEnforcer(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Set lastAttemptedAt BEFORE calling API (throttle attempts, not just successes)
|
||||||
|
state.lastAttemptedAt = Date.now()
|
||||||
|
|
||||||
try {
|
try {
|
||||||
log(`[${HOOK_NAME}] Injecting continuation prompt`, {
|
log(`[${HOOK_NAME}] Injecting continuation prompt`, {
|
||||||
sessionID,
|
sessionID,
|
||||||
@@ -315,7 +322,6 @@ export function createTodoContinuationEnforcer(
|
|||||||
query: { directory: ctx.directory },
|
query: { directory: ctx.directory },
|
||||||
})
|
})
|
||||||
|
|
||||||
state.lastInjectedAt = Date.now()
|
|
||||||
log(`[${HOOK_NAME}] Continuation prompt injected successfully`, { sessionID })
|
log(`[${HOOK_NAME}] Continuation prompt injected successfully`, { sessionID })
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
log(`[${HOOK_NAME}] Prompt injection failed`, { sessionID, error: String(err) })
|
log(`[${HOOK_NAME}] Prompt injection failed`, { sessionID, error: String(err) })
|
||||||
@@ -332,7 +338,7 @@ export function createTodoContinuationEnforcer(
|
|||||||
const state = getOrCreateState(sessionID)
|
const state = getOrCreateState(sessionID)
|
||||||
|
|
||||||
// Cancel any existing countdown
|
// Cancel any existing countdown
|
||||||
cancelCountdown(sessionID, "starting new countdown")
|
invalidate(sessionID, "starting new countdown")
|
||||||
|
|
||||||
// Increment version for this new countdown
|
// Increment version for this new countdown
|
||||||
state.version++
|
state.version++
|
||||||
@@ -388,7 +394,7 @@ export function createTodoContinuationEnforcer(
|
|||||||
const isInterrupt = detectInterrupt(props?.error)
|
const isInterrupt = detectInterrupt(props?.error)
|
||||||
const state = getOrCreateState(sessionID)
|
const state = getOrCreateState(sessionID)
|
||||||
|
|
||||||
cancelCountdown(sessionID, isInterrupt ? "user interrupt" : "session error")
|
invalidate(sessionID, isInterrupt ? "user interrupt" : "session error")
|
||||||
state.mode = "errorBypass"
|
state.mode = "errorBypass"
|
||||||
|
|
||||||
log(`[${HOOK_NAME}] session.error received`, { sessionID, isInterrupt, error: props?.error })
|
log(`[${HOOK_NAME}] session.error received`, { sessionID, isInterrupt, error: props?.error })
|
||||||
@@ -452,13 +458,22 @@ export function createTodoContinuationEnforcer(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Skip if background tasks are running (avoid toast spam with no injection)
|
||||||
|
const hasRunningBgTasks = backgroundManager
|
||||||
|
? backgroundManager.getTasksByParentSession(sessionID).some((t) => t.status === "running")
|
||||||
|
: false
|
||||||
|
|
||||||
|
if (hasRunningBgTasks) {
|
||||||
|
log(`[${HOOK_NAME}] Skipped: background tasks still running`, { sessionID })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
log(`[${HOOK_NAME}] Found incomplete todos`, {
|
log(`[${HOOK_NAME}] Found incomplete todos`, {
|
||||||
sessionID,
|
sessionID,
|
||||||
incomplete: incompleteCount,
|
incomplete: incompleteCount,
|
||||||
total: todos.length
|
total: todos.length
|
||||||
})
|
})
|
||||||
|
|
||||||
// Start countdown
|
|
||||||
startCountdown(sessionID, incompleteCount)
|
startCountdown(sessionID, incompleteCount)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -476,13 +491,13 @@ export function createTodoContinuationEnforcer(
|
|||||||
|
|
||||||
// User message: Always cancel countdown
|
// User message: Always cancel countdown
|
||||||
if (role === "user") {
|
if (role === "user") {
|
||||||
cancelCountdown(sessionID, "user message received")
|
invalidate(sessionID, "user message received")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Assistant message WITHOUT finish: Agent is working, cancel countdown
|
// Assistant message WITHOUT finish: Agent is working, cancel countdown
|
||||||
if (role === "assistant" && !finish) {
|
if (role === "assistant" && !finish) {
|
||||||
cancelCountdown(sessionID, "assistant is working (streaming)")
|
invalidate(sessionID, "assistant is working (streaming)")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -503,7 +518,7 @@ 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") {
|
||||||
cancelCountdown(sessionID, "assistant streaming")
|
invalidate(sessionID, "assistant streaming")
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -514,7 +529,7 @@ 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) {
|
||||||
cancelCountdown(sessionID, `tool execution (${event.type})`)
|
invalidate(sessionID, `tool execution (${event.type})`)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user