diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 1d3b19e96a56..b31d30580ccf 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -3913,3 +3913,26 @@ This compaction should PRIORITISE preserving all information related to the focu self._last_compression_made_progress = True return compressed + + +def is_compaction_summary_message(message: Any) -> bool: + """Return True when *message* is a context-compaction handoff summary. + + Public API for consumers outside the compressor (memory providers, + frontends) that must not treat compaction summaries as real user or + assistant turns — e.g. fact extraction harvesting the compactor's own + output as user statements (#57682). + + Prefers the in-process ``COMPRESSED_SUMMARY_METADATA_KEY`` marker and + falls back to the content heuristics in ``_is_context_summary_content`` + (which cover the merged-into-tail and historical-prefix cases), because + the metadata key is stripped by the wire sanitizers and does not survive + all session-store round-trips. + """ + if isinstance(message, dict): + if message.get(COMPRESSED_SUMMARY_METADATA_KEY): + return True + content = message.get("content") + else: + content = message + return ContextCompressor._is_context_summary_content(content) diff --git a/plugins/memory/holographic/__init__.py b/plugins/memory/holographic/__init__.py index 7e2e387902e0..f44b880bb687 100644 --- a/plugins/memory/holographic/__init__.py +++ b/plugins/memory/holographic/__init__.py @@ -24,6 +24,7 @@ from typing import Any, Dict, List from agent.memory_provider import MemoryProvider from tools.registry import tool_error +from utils import is_truthy_value from .store import MemoryStore from .retrieval import FactRetriever from hermes_cli.config import cfg_get @@ -235,7 +236,10 @@ class HolographicMemoryProvider(MemoryProvider): return tool_error(f"Unknown tool: {tool_name}") def on_session_end(self, messages: List[Dict[str, Any]]) -> None: - if not self._config.get("auto_extract", False): + # is_truthy_value: the config schema declares auto_extract as a string + # enum ("false"/"true"), and a plain truthiness check treats the string + # "false" as enabled (#57682). + if not is_truthy_value(self._config.get("auto_extract", False)): return if not self._store or not messages: return @@ -368,6 +372,10 @@ class HolographicMemoryProvider(MemoryProvider): # -- Auto-extraction (on_session_end) ------------------------------------ def _auto_extract_facts(self, messages: list) -> None: + # Local import (pattern used in initialize()): the compressor module is + # heavier than this plugin and is only needed when auto_extract is on. + from agent.context_compressor import is_compaction_summary_message + _PREF_PATTERNS = [ re.compile(r'\bI\s+(?:prefer|like|love|use|want|need)\s+(.+)', re.IGNORECASE), re.compile(r'\bmy\s+(?:favorite|preferred|default)\s+\w+\s+is\s+(.+)', re.IGNORECASE), @@ -382,6 +390,12 @@ class HolographicMemoryProvider(MemoryProvider): for msg in messages: if msg.get("role") != "user": continue + # Compaction handoff summaries can be inserted as role="user" + # messages; their prose reliably matches the decision patterns, so + # without this guard the compactor's own output is stored as a + # durable "fact" on every rollover (#57682). + if is_compaction_summary_message(msg): + continue content = msg.get("content", "") if not isinstance(content, str) or len(content) < 10: continue diff --git a/tests/plugins/memory/test_holographic_auto_extract.py b/tests/plugins/memory/test_holographic_auto_extract.py new file mode 100644 index 000000000000..11b7cf3afef1 --- /dev/null +++ b/tests/plugins/memory/test_holographic_auto_extract.py @@ -0,0 +1,148 @@ +"""Regression tests for #57682 — holographic auto_extract harvested +context-compaction handoff summaries into fact_store, and ran even when +configured off. + +Two compounding defects: + +1. Gate: the plugin's config schema declares ``auto_extract`` as a string enum + (``"false"``/``"true"``), and the ``on_session_end`` gate used plain + truthiness — ``not "false"`` is ``False`` — so extraction ran despite being + explicitly configured off. + +2. Eligibility: ``_auto_extract_facts`` scanned every ``role == "user"`` + message. Compaction handoff summaries can be inserted as ``role="user"`` + messages, and their prose reliably matches the decision patterns + (``we decided/agreed/chose``, ``the project uses/needs/requires``), so the + compactor's own output was stored as a durable ``project`` fact on every + session rollover that followed a compaction. +""" + +import pytest + +from agent.context_compressor import ( + COMPRESSED_SUMMARY_METADATA_KEY, + SUMMARY_PREFIX, + _MERGED_PRIOR_CONTEXT_HEADER, + _MERGED_SUMMARY_DELIMITER, + is_compaction_summary_message, +) +from plugins.memory.holographic import HolographicMemoryProvider + + +def _make_provider(tmp_path, **config): + base = {"db_path": str(tmp_path / "memory_store.db"), "hrr_dim": 64} + base.update(config) + provider = HolographicMemoryProvider(config=base) + provider.initialize(session_id="test-session") + return provider + + +def _fact_contents(provider): + return [f["content"] for f in provider._store.list_facts(limit=100)] + + +def _user(content, **extra): + msg = {"role": "user", "content": content} + msg.update(extra) + return msg + + +DECISION_MSG = "we decided to use PostgreSQL for the persistence layer" +SUMMARY_MSG = ( + f"{SUMMARY_PREFIX}\n## Historical Task Snapshot\n" + "The project uses a kanban board for all dispatch. " + "We agreed to route reviews through the fan-in consumer." +) + + +# --------------------------------------------------------------------------- +# Defect 1 — string-boolean gate +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("off_value", [False, "false", "False", "no", "off", "0", None, ""]) +def test_auto_extract_off_values_disable_extraction(tmp_path, off_value): + provider = _make_provider(tmp_path, auto_extract=off_value) + provider.on_session_end([_user(DECISION_MSG)]) + assert _fact_contents(provider) == [] + provider.shutdown() + + +@pytest.mark.parametrize("on_value", [True, "true", "True", "1", "yes", "on"]) +def test_auto_extract_on_values_enable_extraction(tmp_path, on_value): + provider = _make_provider(tmp_path, auto_extract=on_value) + provider.on_session_end([_user(DECISION_MSG)]) + facts = _fact_contents(provider) + assert len(facts) == 1 + assert "PostgreSQL" in facts[0] + provider.shutdown() + + +# --------------------------------------------------------------------------- +# Defect 2 — compaction summaries harvested as facts +# --------------------------------------------------------------------------- + + +def test_compaction_summary_not_harvested(tmp_path): + """The exact failure mode from #57682: summary prose matches the decision + patterns but must not become a fact.""" + provider = _make_provider(tmp_path, auto_extract=True) + provider.on_session_end([_user(SUMMARY_MSG)]) + assert _fact_contents(provider) == [] + provider.shutdown() + + +def test_metadata_marked_summary_not_harvested(tmp_path): + """In-process summaries carry COMPRESSED_SUMMARY_METADATA_KEY even if a + future prefix rewrite changes the content sentinel.""" + provider = _make_provider(tmp_path, auto_extract=True) + marked = _user(DECISION_MSG, **{COMPRESSED_SUMMARY_METADATA_KEY: True}) + provider.on_session_end([marked]) + assert _fact_contents(provider) == [] + provider.shutdown() + + +def test_merged_into_tail_summary_not_harvested(tmp_path): + """Merge-into-tail summaries embed the handoff prefix after the delimiter, + not at the start of the message.""" + provider = _make_provider(tmp_path, auto_extract=True) + merged = _user( + f"{_MERGED_PRIOR_CONTEXT_HEADER}\nplease fix the login bug\n" + f"{_MERGED_SUMMARY_DELIMITER}\n{SUMMARY_MSG}" + ) + provider.on_session_end([merged]) + assert _fact_contents(provider) == [] + provider.shutdown() + + +def test_real_user_messages_still_extracted_alongside_summary(tmp_path): + """The guard must skip only the summary, not suppress extraction for the + genuine user turns around it.""" + provider = _make_provider(tmp_path, auto_extract=True) + provider.on_session_end( + [ + _user(SUMMARY_MSG), + _user("I prefer tabs over spaces for indentation"), + ] + ) + facts = _fact_contents(provider) + assert len(facts) == 1 + assert "tabs over spaces" in facts[0] + provider.shutdown() + + +# --------------------------------------------------------------------------- +# is_compaction_summary_message — public helper contract +# --------------------------------------------------------------------------- + + +def test_helper_detects_prefix_metadata_and_merged_forms(): + assert is_compaction_summary_message(_user(SUMMARY_MSG)) + assert is_compaction_summary_message( + _user("anything", **{COMPRESSED_SUMMARY_METADATA_KEY: True}) + ) + assert is_compaction_summary_message( + _user(f"prior tail\n{_MERGED_SUMMARY_DELIMITER}\n{SUMMARY_MSG}") + ) + assert not is_compaction_summary_message(_user(DECISION_MSG)) + assert not is_compaction_summary_message(_user(""))