Skip to content

fix: update current_turn_user_idx after preflight compression - #39239

Closed
moizogg wants to merge 5 commits into
NousResearch:mainfrom
moizogg:fix/compression-index-staleness
Closed

moizogg wants to merge 5 commits into
NousResearch:mainfrom
moizogg:fix/compression-index-staleness

Conversation

@moizogg

@moizogg moizogg commented Jun 4, 2026

Copy link
Copy Markdown

Problem

When preflight context compression replaces the messages list with a compressed [summary, ...tail] structure, the current_turn_user_idx becomes stale. This causes memory/plugin context injection to target the wrong message.

Impact

  • Memory prefetch context may be injected into tool result messages instead of the user message
  • Plugin context from pre_llm_call hooks may corrupt the summary message
  • Can cause provider API errors from malformed role alternation
  • Silent context loss in long conversations

Fix

After compression completes, scan the compressed messages list from the end to locate the current turn's user message and update both current_turn_user_idx and agent._persist_user_message_idx to the correct position.

moizogg added 5 commits June 2, 2026 12:30
Silent except: pass blocks in is_write_denied made path-resolution
failures invisible when the guard was deciding whether a write target
was denied. Add debug logging so operators can see which paths and
Hermes dir lookups are being skipped without changing behavior.
When preflight context compression replaces the messages list with a
compressed [summary, ...tail] structure, the current_turn_user_idx set
earlier (line 567) becomes stale. This causes memory/plugin context
injection to target the wrong message, potentially corrupting tool
results or the summary message itself.

The fix scans the compressed messages list to locate the current turn's
user message and updates both current_turn_user_idx and
agent._persist_user_message_idx to the correct position.

This ensures that ephemeral context (memory prefetch, plugin hooks) is
injected into the correct user message after compression events.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jun 4, 2026

@naqerl naqerl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the for loop would not found any messge, the original issue persists. It makes sense to set indexes to None in this case.

Nice to have these tests:

  1. Mock compression and ensure that the index changed
  2. Mock compression which deletes all user's messages and ensure, that stale index is not present

Also agent/file_safety.py and agent/context_references.py changes are unrelated and present in the another PR. It will cause merge conflict, so they could be safely removed

Thanks for your time!

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for identifying the stale-index hazard; the premise still holds on current main.

Problems

  • The target code moved in 54870847cb0f530105907b1a793531b8d0f03d78: preflight compression now replaces messages in agent/turn_context.py:450, returns the old index at agent/turn_context.py:575, and agent/conversation_loop.py:801 still uses that index to choose the injection target.
  • The scan in this PR leaves both indexes stale if no matching user message remains (agent/conversation_loop.py:690-692 on the PR head).
  • Current main has additional message-replacing compaction paths at agent/conversation_loop.py:1034, 3143, 3398, 3621, and 4798; this hunk would not cover them.
  • The diff has no regression coverage and includes unrelated logging edits in agent/context_references.py and agent/file_safety.py.

Suggested changes

  • Move this into a shared rebinding helper, use it after each continuing compaction path, and clear both indexes when no current-turn user message survives.
  • Add preflight and no-match regression tests, then split the unrelated logging edits.

Automated hermes-sweeper review.

and _msg.get("content") == user_message):
current_turn_user_idx = _new_idx
agent._persist_user_message_idx = _new_idx
break

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the scan finds no matching user message, this leaves both indexes at their pre-compression values. Clear current_turn_user_idx and _persist_user_message_idx in that case so later injection and persistence cannot target a stale row.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@Fr4nZ82

Fr4nZ82 commented Jul 18, 2026

Copy link
Copy Markdown

Production evidence that this and #43552 are the same stale current_turn_user_idx with two entry points (preflight compression here, message-alternation repair there): full timestamp forensics posted in #43552 (comment) — including a dropped 28 of 52 preflight cut followed 7s later by a request with no <memory-context> anywhere, while the next turn (list length unchanged) injected fine. A fix that re-derives the anchor right before the injection block would close both.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @moizogg for the first report of this class. The preflight case is already covered on main: current_turn_user_idx is re-anchored at turn start. The remaining mid-turn gates (post-tool and pre-API compression) were fixed in #120175, merged as fd94981, which credits this PR. Closing as resolved on main.

@teknium1 teknium1 closed this Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions area/install-update Installer, updater, packaging, wheels, doctor comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants