Merge pull request #4077 from PeterPonyu/fix/4013-todo-continuation-enforcer-loop

fix(todo-continuation-enforcer): stop looping after all todos complete (#4013)

src/hooks/todo-continuation-enforcer/idle-event.test.ts

@@ -9,12 +9,15 @@ import type { ContinuationProgressUpdate, SessionState } from "./types"
 function createStateStore(): {
   store: SessionStateStore
   resetCalls: string[]
+  trackCalls: string[]
+  state: SessionState
 } {
   const state: SessionState = {
     stagnationCount: 0,
     consecutiveFailures: 0,
   }
   const resetCalls: string[] = []
+  const trackCalls: string[] = []
   const progressUpdate: ContinuationProgressUpdate = {
     previousStagnationCount: 0,
     stagnationCount: 0,
@@ -24,11 +27,16 @@ function createStateStore(): {
 
   return {
     resetCalls,
+    trackCalls,
+    state,
     store: {
       getState: () => state,
       getExistingState: () => state,
       startPruneInterval: () => {},
-      trackContinuationProgress: () => progressUpdate,
+      trackContinuationProgress: (sessionID: string) => {
+        trackCalls.push(sessionID)
+        return progressUpdate
+      },
       resetContinuationProgress: (sessionID: string) => {
         resetCalls.push(sessionID)
       },
@@ -95,4 +103,37 @@ describe("handleSessionIdle", () => {
     // then
     expect(resetCalls).toEqual([sessionID])
   })
+
+  it("does not re-enter the injection path on subsequent idle events once all todos are complete (#4013 P0.1)", async () => {
+    // given
+    const sessionID = "ses_stop_flag"
+    const { store, resetCalls, trackCalls, state } = createStateStore()
+    const completedTodos = [
+      { id: "todo-1", content: "Ship", status: "completed", priority: "high" },
+    ]
+    const ctx = {
+      client: {
+        session: {
+          messages: async () => ({ data: [] }),
+          todo: async () => ({ data: completedTodos }),
+        },
+      },
+      directory: "/tmp/test",
+    }
+
+    // when — first idle: detects incompleteCount === 0, sets the stop flag
+    await handleSessionIdle({ ctx: ctx as never, sessionID, sessionStateStore: store })
+
+    // sanity: stop flag was set and reset was called
+    expect(state.allTodosCompletedAt).toBeGreaterThan(0)
+    expect(resetCalls).toHaveLength(1)
+
+    // when — second idle: stop flag already set, must bail out immediately
+    await handleSessionIdle({ ctx: ctx as never, sessionID, sessionStateStore: store })
+
+    // then: trackContinuationProgress was never called (injection path never reached)
+    expect(trackCalls).toHaveLength(0)
+    // reset is still called only once (from the first idle)
+    expect(resetCalls).toHaveLength(1)
+  })
 })

src/hooks/todo-continuation-enforcer/idle-event.ts

@@ -37,6 +37,12 @@ export async function handleSessionIdle(args: {
 
   const state = sessionStateStore.getState(sessionID)
   const observedCompactionEpoch = state.recentCompactionEpoch
+
+  if (state.allTodosCompletedAt) {
+    log(`[${HOOK_NAME}] Skipped: all todos were already completed`, { sessionID, allTodosCompletedAt: state.allTodosCompletedAt })
+    return
+  }
+
   if (state.isRecovering) {
     log(`[${HOOK_NAME}] Skipped: in recovery`, { sessionID })
     return
@@ -107,6 +113,7 @@ export async function handleSessionIdle(args: {
 
   const incompleteCount = getIncompleteCount(todos)
   if (incompleteCount === 0) {
+    state.allTodosCompletedAt = Date.now()
     sessionStateStore.resetContinuationProgress(sessionID)
     log(`[${HOOK_NAME}] All todos complete`, { sessionID, total: todos.length })
     return

src/hooks/todo-continuation-enforcer/session-state.test.ts

@@ -144,6 +144,33 @@ describe("createSessionStateStore", () => {
     expect(stagnatedAgainUpdate.stagnationCount).toBe(1)
   })
 
+  test("given only content or priority changes while id→status mapping stays the same, does not treat it as progress (#4013 P0.2)", () => {
+    // given
+    const sessionID = "ses-content-priority-no-progress"
+    const state = sessionStateStore.getState(sessionID)
+    state.lastInjectedAt = Date.now()
+    const initialTodos = [
+      { id: "1", content: "Task 1", status: "pending", priority: "high" },
+      { id: "2", content: "Task 2", status: "pending", priority: "medium" },
+    ]
+    // Same id→status mapping; only content and priority differ
+    const contentChangedTodos = [
+      { id: "1", content: "Task 1 (updated description)", status: "pending", priority: "low" },
+      { id: "2", content: "Task 2 (revised)", status: "pending", priority: "high" },
+    ]
+
+    sessionStateStore.trackContinuationProgress(sessionID, 2, initialTodos)
+    state.awaitingPostInjectionProgressCheck = true
+
+    // when
+    const progressUpdate = sessionStateStore.trackContinuationProgress(sessionID, 2, contentChangedTodos)
+
+    // then — content/priority drift must not reset the stagnation counter
+    expect(progressUpdate.hasProgressed).toBe(false)
+    expect(progressUpdate.progressSource).toBe("none")
+    expect(progressUpdate.stagnationCount).toBe(1)
+  })
+
   test("given no todo changes after a successful continuation, keeps counting stagnation", () => {
     // given
     const sessionID = "ses-no-todo-change-stagnation"

src/hooks/todo-continuation-enforcer/session-state.ts

@@ -43,29 +43,17 @@ export interface SessionStateStore {
 }
 
 function getTodoSnapshot(todos: Todo[]): string {
-  const normalizedTodos = todos
+  // Only compare {id → status} mappings. Content/priority changes do not represent
+  // meaningful progress and must not reset the stagnation counter (issue #4013 P0.2).
+  const entries = todos
     .map((todo) => ({
-      id: todo.id ?? null,
+      key: todo.id ?? `${todo.content}:${todo.priority}`,
-      content: todo.content,
-      priority: todo.priority,
       status: todo.status,
     }))
-    .sort((left, right) => {
+    .sort((left, right) => left.key.localeCompare(right.key))
-      const leftKey = left.id ?? `${left.content}:${left.priority}:${left.status}`
+    .map(({ key, status }) => `${key}=${status}`)
-      const rightKey = right.id ?? `${right.content}:${right.priority}:${right.status}`
-      if (leftKey !== rightKey) {
-        return leftKey.localeCompare(rightKey)
-      }
-      if (left.content !== right.content) {
-        return left.content.localeCompare(right.content)
-      }
-      if (left.priority !== right.priority) {
-        return left.priority.localeCompare(right.priority)
-      }
-      return left.status.localeCompare(right.status)
-    })
 
-  return JSON.stringify(normalizedTodos)
+  return entries.join("|")
 }
 
 export function createSessionStateStore(): SessionStateStore {
@@ -213,6 +201,7 @@ export function createSessionStateStore(): SessionStateStore {
     state.lastIncompleteCount = undefined
     state.stagnationCount = 0
     state.awaitingPostInjectionProgressCheck = false
+    state.allTodosCompletedAt = undefined
     trackedSession.lastCompletedCount = undefined
     trackedSession.lastTodoSnapshot = undefined
   }

src/hooks/todo-continuation-enforcer/types.ts

@@ -36,6 +36,7 @@ export interface SessionState {
   inFlight?: boolean
   stagnationCount: number
   consecutiveFailures: number
+  allTodosCompletedAt?: number
   recentCompactionAt?: number
   recentCompactionEpoch?: number
   acknowledgedCompactionEpoch?: number