Skip to content

fix(todo-continuation): remove activity-based stagnation bypass#3319

Open
EZotoff wants to merge 2 commits intocode-yeongyu:devfrom
EZotoff:fix/remove-activity-stagnation-bypass
Open

fix(todo-continuation): remove activity-based stagnation bypass#3319
EZotoff wants to merge 2 commits intocode-yeongyu:devfrom
EZotoff:fix/remove-activity-stagnation-bypass

Conversation

@EZotoff
Copy link
Copy Markdown

@EZotoff EZotoff commented Apr 10, 2026

Summary

Activity signals (tool calls like compress, grep, bash) were treated as "progress" by the stagnation detector in trackContinuationProgress(), resetting stagnationCount to 0 every cycle. This prevented MAX_STAGNATION_COUNT (3) from ever being reached, causing infinite continuation loops when models degrade to minimal responses in long sessions.

Root Cause

The bug manifests when a model (observed with GLM-5.1 at ~100K tokens) can no longer produce substantive responses but still executes tool calls:

  1. Continuation injected → awaitingPostInjectionProgressCheck = true
  2. Model responds minimally but calls compress/grep → activitySignalCount increments
  3. Next idle → hasObservedExternalActivity = trueprogressSource = "activity"hasProgressed = true
  4. stagnationCount reset to 0 — never reaches 3
  5. Continuation fires again → infinite loop with ~7s cycle time

Fix

Removed all activity-based progress detection. Stagnation now only tracks actual todo state changes:

  • Incomplete count decrease
  • Completed count increase
  • Todo snapshot change (status/content mutations)

Tool-level activity (compress, grep, bash, etc.) no longer resets the stagnation counter.

Changes

File Change
session-state.ts Removed activitySignalCount, lastObservedActivitySignalCount, recordActivity(), ContinuationProgressOptions. Simplified progress to todo-only.
non-idle-events.ts Removed all recordActivity() calls
idle-event.ts Removed shouldAllowActivityProgress() and allowActivityProgress option
types.ts Removed ContinuationProgressOptions interface
session-state.test.ts Replaced 2 activity-based tests with 1 test verifying stagnation works correctly

Testing

  • All existing unit tests pass (session-state, stagnation-detection, continuation-injection, pending-question-detection, compaction-guard, dispose)
  • New test confirms: tool activity without todo changes correctly increments stagnation

Summary by cubic

Remove activity-based stagnation bypass to stop infinite continuation loops in long sessions. Progress is now detected only from todo changes, not tool calls.

  • Bug Fixes

    • Stagnation no longer resets on tool activity; it only resets when incomplete count decreases, completed count increases, or the todo snapshot changes.
  • Refactors

    • Removed recordActivity(), ContinuationProgressOptions (and allowActivityProgress), and activity-related state; progressSource is now "todo" or "none". Renamed the remaining test to reflect todo-only progress checks.

Written for commit b610f81. Summary will update on new commits.

Activity signals (tool calls like compress, grep, bash) were treated as
'progress' by the stagnation detector, resetting the stagnation counter
every cycle. This prevented MAX_STAGNATION_COUNT from being reached,
causing infinite continuation loops when models degrade to minimal
responses in long sessions (e.g. GLM-5.1 at ~100K tokens).

Stagnation now only tracks actual todo state changes: incomplete count
decrease, completed count increase, or todo snapshot change. Tool-level
activity no longer resets the stagnation counter.
Copilot AI review requested due to automatic review settings April 10, 2026 18:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes tool/activity-based “progress” from the todo continuation stagnation detector so stagnation can reliably reach the configured limit and prevent infinite continuation loops.

Changes:

  • Removed activity-signal tracking (recordActivity, counters, and options) from continuation progress tracking.
  • Simplified progress detection to todo-only signals (incomplete/completed counts and todo snapshot changes).
  • Updated callers/tests to align with the new todo-only stagnation behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/hooks/todo-continuation-enforcer/types.ts Removes the ContinuationProgressOptions type that enabled activity-based progress.
src/hooks/todo-continuation-enforcer/session-state.ts Drops activity counters and options; progress/stagnation now determined strictly from todo state.
src/hooks/todo-continuation-enforcer/session-state.test.ts Replaces activity-based assertions with a todo-only stagnation test scenario.
src/hooks/todo-continuation-enforcer/non-idle-events.ts Removes recordActivity() calls from non-idle event handling.
src/hooks/todo-continuation-enforcer/idle-event.ts Removes model-based gating for activity progress and stops passing the option into progress tracking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 147 to 163
test("given tool activity happens after a successful continuation without todo changes, keeps counting stagnation", () => {
// given
const sessionID = "ses-non-codex-activity-progress"
const sessionID = "ses-activity-stagnation"
const state = sessionStateStore.getState(sessionID)
const todos = [
{ id: "1", content: "Task 1", status: "pending", priority: "high" },
]

sessionStateStore.trackContinuationProgress(sessionID, 1, todos)
state.awaitingPostInjectionProgressCheck = true
sessionStateStore.recordActivity(sessionID)

// when
const progressUpdate = sessionStateStore.trackContinuationProgress(
sessionID,
1,
todos,
{ allowActivityProgress: true },
)

// then
expect(progressUpdate.hasProgressed).toBe(true)
expect(progressUpdate.progressSource).toBe("activity")
expect(progressUpdate.stagnationCount).toBe(0)
})

test("given codex activity happens after a successful continuation, keeps counting stagnation", () => {
// given
const sessionID = "ses-codex-activity-stagnation"
const state = sessionStateStore.getState(sessionID)
const todos = [
{ id: "1", content: "Task 1", status: "pending", priority: "high" },
]

sessionStateStore.trackContinuationProgress(sessionID, 1, todos)
state.awaitingPostInjectionProgressCheck = true
sessionStateStore.recordActivity(sessionID)

// when
const progressUpdate = sessionStateStore.trackContinuationProgress(
sessionID,
1,
todos,
{ allowActivityProgress: false },
)
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated test name mentions "tool activity happens" but the test no longer simulates any tool/non-idle activity (it only calls trackContinuationProgress twice with identical inputs). This makes the intent misleading and overlaps with the earlier stagnation test; consider renaming it to reflect "no todo changes" or explicitly simulating tool activity via handleNonIdleEvent to validate the behavior being described.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: Removing activity-based progress bypass may cause regressions in complex tasks where multiple tool calls are required before a todo can be updated, potentially triggering stagnation too early.

Address review feedback: test name no longer references 'tool activity'
since activity tracking was removed.
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Removing activity-based progress bypass might cause regressions by prematurely stopping valid long-running tool sequences (e.g. >3 tool calls) that don't immediately update todos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants