Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 134
fix: escalating circuit breaker for doom loops in headless mode#658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7671cf97d9b09d558b9faac01755File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -27,6 +27,15 @@ export namespace SessionProcessor { | ||||||
| // 30 catches pathological patterns while avoiding false positives for power users. | ||||||
| const TOOL_REPEAT_THRESHOLD = 30 | ||||||
| // altimate_change end | ||||||
| // altimate_change start — escalating circuit breaker for doom loops | ||||||
| // When the repeat threshold is hit and auto-accepted (headless, config allow), the | ||||||
| // counter resets and the loop continues indefinitely. Escalation levels: | ||||||
| // 1st hit (30 calls): ask permission (existing behavior) | ||||||
| // 2nd hit (60 calls): ask + inject synthetic warning telling model to change approach | ||||||
| // 3rd hit (90 calls): force-stop the session — the model is stuck | ||||||
| const DOOM_LOOP_WARN_ESCALATION = 2 // hits before injecting warning | ||||||
| const DOOM_LOOP_STOP_ESCALATION = 3 // hits before force-stopping | ||||||
| // altimate_change end | ||||||
| const log = Log.create({ service: "session.processor" }) | ||||||
| export type Info = Awaited<ReturnType<typeof create>> | ||||||
| @@ -42,6 +51,9 @@ export namespace SessionProcessor { | ||||||
| // altimate_change start — per-tool call counter for varied-input loop detection | ||||||
| const toolCallCounts: Record<string, number> = {} | ||||||
| // altimate_change end | ||||||
| // altimate_change start — escalation counter: how many times each tool has hit TOOL_REPEAT_THRESHOLD | ||||||
| const toolLoopHits: Record<string, number> = {} | ||||||
| // altimate_change end | ||||||
| let snapshot: string | undefined | ||||||
| let blocked = false | ||||||
| let attempt = 0 | ||||||
| @@ -201,20 +213,77 @@ export namespace SessionProcessor { | ||||||
| }) | ||||||
| } | ||||||
| // altimate_change start — per-tool repeat counter (catches varied-input loops like todowrite 2,080x) | ||||||
| // altimate_change start — per-tool repeat counter with escalating circuit breaker | ||||||
| // Counter is scoped to the processor lifetime (create() call), so it accumulates | ||||||
| // across multiple process() invocations within a session. This is intentional: | ||||||
| // cross-turn accumulation catches slow-burn loops that stay under the threshold | ||||||
| // per-turn but add up over the session. | ||||||
| toolCallCounts[value.toolName] = (toolCallCounts[value.toolName] ?? 0) + 1 | ||||||
| if (toolCallCounts[value.toolName] >= TOOL_REPEAT_THRESHOLD) { | ||||||
| toolLoopHits[value.toolName] = (toolLoopHits[value.toolName] ?? 0) + 1 | ||||||
| const hits = toolLoopHits[value.toolName] | ||||||
| const totalCalls = hits * TOOL_REPEAT_THRESHOLD | ||||||
| Telemetry.track({ | ||||||
| type: "doom_loop_detected", | ||||||
| timestamp: Date.now(), | ||||||
| session_id: input.sessionID, | ||||||
| tool_name: value.toolName, | ||||||
| repeat_count: toolCallCounts[value.toolName], | ||||||
| repeat_count: totalCalls, | ||||||
| escalation_level: hits, | ||||||
| }) | ||||||
| // Escalation level 3+: force-stop — the model is irretrievably stuck | ||||||
| if (hits >= DOOM_LOOP_STOP_ESCALATION) { | ||||||
| log.warn("doom loop circuit breaker: force-stopping session", { | ||||||
| tool: value.toolName, | ||||||
| totalCalls, | ||||||
| hits, | ||||||
| sessionID: input.sessionID, | ||||||
| }) | ||||||
| await Session.updatePart({ | ||||||
| id: PartID.ascending(), | ||||||
| messageID: input.assistantMessage.id, | ||||||
| sessionID: input.assistantMessage.sessionID, | ||||||
| type: "text", | ||||||
| synthetic: true, | ||||||
| text: | ||||||
| `⚠️ altimate-code: session stopped — \`${value.toolName}\` was called ${totalCalls} times, ` + | ||||||
| `indicating the agent is stuck in a loop. Please start a new session with a revised prompt.`, | ||||||
| time: { start: Date.now(), end: Date.now() }, | ||||||
| }) | ||||||
| blocked = true | ||||||
| toolCallCounts[value.toolName] = 0 | ||||||
| toolLoopHits[value.toolName] = 0 | ||||||
| break | ||||||
| } | ||||||
| // Escalation level 2: warn the model via synthetic message | ||||||
| if (hits >= DOOM_LOOP_WARN_ESCALATION) { | ||||||
| log.warn("doom loop escalation: injecting warning", { | ||||||
| tool: value.toolName, | ||||||
| totalCalls, | ||||||
| hits, | ||||||
| sessionID: input.sessionID, | ||||||
| }) | ||||||
| await Session.updatePart({ | ||||||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 finding (warning) At escalation level 2, a warning text part is written to | ||||||
| id: PartID.ascending(), | ||||||
| messageID: input.assistantMessage.id, | ||||||
| sessionID: input.assistantMessage.sessionID, | ||||||
| type: "text", | ||||||
| synthetic: true, | ||||||
| ||||||
| synthetic: true, | |
| synthetic: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the 60-call warning model-visible.
Line 274 marks this warning as synthetic: true, but Lines 433-435 in the same file describe synthetic text as TUI-only and excluded from replay to the LLM. That makes the warn tier ineffective in the exact headless/auto-accept path this circuit breaker is trying to correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/session/processor.ts` around lines 269 - 280, The
warning inserted via Session.updatePart (using PartID.ascending(),
input.assistantMessage.id, input.assistantMessage.sessionID, value.toolName and
totalCalls) is marked synthetic:true which makes it TUI-only and excluded from
LLM replay; change the part creation so the warning is not synthetic (remove or
set synthetic to false) so the message is visible to the model/auto-accept path,
keeping the same text, type:"text" and timestamps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope the early stream break to the new hard-stop path only.
blocked is also set on Lines 345-350 for normal permission/question rejections. With this unconditional break, those pre-existing denial paths now short-circuit the rest of the stream too, which can skip the finish-step bookkeeping on Lines 372-460. A dedicated forceStopped flag would preserve the old rejection flow while still halting doom-loop stops immediately.
💡 Suggested change
- let blocked = false+ let blocked = false+ let forceStopped = false
...
- blocked = true+ blocked = true+ forceStopped = true
...
- if (blocked) break+ if (forceStopped) break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/session/processor.ts` around lines 549 - 550, The early
stream break currently uses the shared variable "blocked" which also represents
normal permission/question rejections; introduce a new boolean "forceStopped"
scoped alongside "blocked" and set it only in the doom-loop hard-stop path
(where the code currently sets "blocked" for force-stop), then change the
immediate break condition to test "forceStopped" (if (forceStopped) break) so
normal rejection flows still run the finish-step bookkeeping in the rest of the
stream; ensure "forceStopped" is initialized in the same scope as "blocked" and
is not used elsewhere.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 finding (warning)
At escalation level 3 (force-stop),
toolCallCounts[value.toolName]is correctly reset to 0 at line 256, buttoolLoopHits[value.toolName]is left at 3. In current code this is harmless becauseblocked = truecauses the session to return "stop" and the processor is not reused. However, the two maps are now permanently out of sync: if any future code path ever callsprocess()again on the same processor instance after a force-stop (e.g., an error-recovery refactor), the very next threshold hit would read hits=4, immediately triggering another force-stop instead of cycling through the warn phase first. The force-stop branch should also includetoolLoopHits[value.toolName] = 0to keep both maps consistent.