AltimateAI / AltimateAI/altimate-code

SessionProcessor closure state claims session-scope but resets per-step (warning false-positive + doom-loop detector silently degraded)

Abierto
#889 1 comentario 0 reacciones 0 asignados Ver en GitHub
bug
Lenguaje dominante
TypeScript
Estrellas
811
Forks
134
Merge medio
3 d 2 h
PR fusionados (30 d)
50

Descripción

## Symptom

State declared in `SessionProcessor.create()`'s closure scope is documented as "session-scoped, accumulates across `process()` invocations within a session" (see `processor.ts:207-210`), but `loop()` in `session/prompt.ts:485` creates a fresh `SessionProcessor` *every step*. The closure variables therefore reset on each step, not each session.

This is a documentation/implementation mismatch — the comments describe an intent that the code does not deliver.

## Known victims

1. **`plan_no_tool_generation` warning** — `sessionToolCallsMade` (`processor.ts:57`). On a multi-step plan session that runs tools across early steps and produces a final text-only step, the warning fires on the last step even though the agent did its job correctly. Patched as a targeted workaround in #888 (PR for #887) by also scanning `streamInput.messages` for prior assistant tool-call content at warning-evaluation time. Not a structural fix.

2. **Doom-loop detector** — `toolCallCounts` (`processor.ts:43`, used in 207-219). The comment explicitly states "cross-turn accumulation catches slow-burn loops that stay under the threshold per-turn but add up over the session" — but because the counter resets per step, slow-burn loops that cross step boundaries cannot trigger the threshold. The detector still catches *within-step* hot loops (e.g. todowrite 2,080x in a single step) and is presumably what saved us from realising sooner, but the documented cross-turn behavior is broken. Unpatched in #888.

There may be other consumers of the same closure variables (e.g. `toolcalls` map at line 41) where per-step vs session scoping matters for correctness — worth an audit.

## Proper fix (options)

The targeted workaround in #888 only addresses victim 1, and only by reading from the conversation history (which the warning happens to have access to). It does not generalize to the doom-loop detector, whose state has no equivalent representation in the message stream.

1. **Move `SessionProcessor.create()` outside the per-step `while (true)` body in `loop()`** so a single processor instance handles all steps of a session. This is the change that most closely matches the comments' intent. Risk: anything else in `create()`'s closure that *should* be per-step would silently break. Needs an audit of all closure variables before flipping.

2. **Lift the two affected counters to a session-keyed `Map` at module scope.** Smaller blast radius than #1, but adds memory-leak surface (need explicit cleanup when sessions end / abort).

3. **Keep the per-step semantics and update the comments + variable names** to match reality. Then re-derive the cross-turn doom-loop signal from the message stream the same way the plan-no-tool workaround does (count `tool-call` content parts across the session). Risk: every consumer that thinks it's session-scoped has to be migrated individually.

I'd lean toward option 1 *if* the closure audit comes back clean. Otherwise option 3 is the safest; it doesn't pretend the bug is fixed and forces each consumer to make an explicit choice.

## Acceptance criteria

- [ ] Closure variables in `SessionProcessor.create()` are audited; each one is documented as either *intentionally per-step* or *session-wide*.
- [ ] Variables that should be session-wide actually behave that way at runtime.
- [ ] `toolCallCounts` cross-turn behavior matches its comment, OR the comment is rewritten to match the implementation.
- [ ] `plan_no_tool_generation` warning is driven by whatever the canonical session-wide signal is (not the workaround scan added in #888).
- [ ] Regression test for cross-turn doom-loop detection.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.