Skip to content

fix: strip leaked memory context from commentary - #12237

Closed
dontcallmejames wants to merge 2 commits into
NousResearch:mainfrom
dontcallmejames:fix/strip-memory-context-commentary-20260418-133147
Closed

fix: strip leaked memory context from commentary#12237
dontcallmejames wants to merge 2 commits into
NousResearch:mainfrom
dontcallmejames:fix/strip-memory-context-commentary-20260418-133147

Conversation

@dontcallmejames

@dontcallmejames dontcallmejames commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • sanitize interim assistant commentary before sending it to the UI layer
  • sanitize streamed deltas before sending them to UI callbacks
  • prevent leaked <memory-context>...</memory-context> blocks from appearing in visible assistant output
  • add regression tests covering both commentary and stream-delta leaks

Repro

A visible assistant message included Hermes's internal memory wrapper.
Example leaked shape:

[System note: The following is recalled memory context, NOT new user input. Treat as informational background data.]

Honcho Context

Example recalled memory content that should remain internal to the agent and never be shown in visible assistant output.

This should never be shown to the user.

Root cause

Two user-visible paths were missing memory-context sanitization:

  • interim commentary emitted through AIAgent._emit_interim_assistant_message()
  • streamed deltas emitted through AIAgent._fire_stream_delta()

Final responses were already sanitized, but these intermediate output paths could still pass raw memory-context fences to UI callbacks.

Fix

Apply sanitize_context(...) before surfacing:

  • interim commentary in _emit_interim_assistant_message()
  • streamed text in _fire_stream_delta()

Test plan

  • pytest tests/run_agent/test_run_agent_codex_responses.py::test_interim_commentary_strips_leaked_memory_context -q -o 'addopts='
  • pytest tests/run_agent/test_run_agent_codex_responses.py::test_stream_delta_strips_leaked_memory_context -q -o 'addopts='
  • pytest tests/run_agent/test_run_agent_codex_responses.py -q -o 'addopts='
  • pytest tests/run_agent/test_run_agent.py -q -o 'addopts=' -k 'MemoryContextSanitization'

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/memory Memory tool and memory providers labels Apr 23, 2026
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jun 29, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for this, and for the clean repro + tests — but we're going to close it, because both paths it touches have moved past this approach on current main.

Stream deltas (_fire_stream_delta) are already sanitized: there's a stateful streaming scrubber (#5719) that catches <memory-context> spans even when they're split across deltas. A per-delta sanitize_context() call is redundant here and actually weaker, since it can't span chunk boundaries.

Interim commentary (_emit_interim_assistant_message) is the more important one. The post-stream sanitize_context() scrub on this path was deliberately removed in commit 4a9ac5c3 ("fix(memory): drop scrub from interim commentary + final response"), which landed after this PR was opened. The reasoning there: post-stream scrubs mutate model output too broadly — a model legitimately writing docs or code that contains a literal <memory-context> string would have it silently stripped. Real leaks are handled delta-by-delta by the streaming scrubber upstream, so the post-stream scrubs were redundant and lossy.

There's now an existing test, test_interim_commentary_preserves_assistant_content, that uses the exact same input as your proposed test and asserts the opposite outcome (markers preserved). This PR would re-introduce the behavior that test was written to guard against.

So this is an intentional-design situation rather than a gap — appreciate the contribution and the thorough test plan regardless.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants