diff --git a/CHANGELOG.md b/CHANGELOG.md index 072cc1edf..673dae2b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **wrap/doctor:** make the Claude Remote Control gate warning accurate and stop it firing for users who never had the feature ([#1779](https://github.com/headroomlabs-ai/headroom/issues/1779)). Claude Code 2.1.196 added a client-side check that **deterministically** disables first-party Remote Control (`/remote-control` / `/rc`) whenever `ANTHROPIC_BASE_URL` points at a non-`api.anthropic.com` host — which Headroom always does. The old notice hedged ("may hide the Remote Control menu"); it now states the disable as fact, names the `/rc` command, and detects the installed Claude Code version so the wording is exact (`2.1.196` when known, `2.1.196+` when not). The gate is upstream and RC's control-plane talks to `claude.ai` (not the API host), so Headroom cannot restore it — the warning tells you to run Claude without Headroom for RC sessions. The warning is suppressed for auth modes that never had Remote Control (API-key/PAYG via `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`, and Bedrock/Vertex/Foundry cloud IAM) and on Claude Code builds older than 2.1.196 where RC is unaffected by a custom base URL. Both the `headroom wrap claude` launch banner and `headroom doctor` co-report the sibling base-URL gates Headroom *does* restore — on-demand tool loading (#746, automatic) and the 1M context window (#1158, via `--1m`) — and the wrap-side co-report is session-accurate: it says "already restored via --1m" when the flag is in effect and reports tool deferral OFF (not falsely "kept on") when the user chose `--tool-search false`/`ENABLE_TOOL_SEARCH=false`; the `ENABLE_TOOL_SEARCH=...` banner line got the same accuracy fix. `is_custom_anthropic_base_url` now recognizes scheme-less values (`myproxy.local:8080`, `127.0.0.1:8787`) as custom hosts and degrades gracefully on malformed URLs instead of crashing `doctor`. `doctor` resolves the Claude Code version lazily, so runs with no custom base URL never pay the `claude --version` subprocess. No request bytes are touched (cache-safe); this is UX/notice-only. * **install:** default the docker image to `ghcr.io/headroomlabs-ai/headroom:latest` instead of the dead `ghcr.io/chopratejas/headroom:latest`. After the repo moved to the `headroomlabs-ai` org, GHCR did not redirect the old package, so `headroom install` / `headroom init` and the install scripts pulled a frozen `0.27.0` image while current releases publish to the new path ([#1867](https://github.com/headroomlabs-ai/headroom/issues/1867)). * **transforms/content-router:** stop a profile-derived `read_protection_window` kwarg from weakening an explicit `--protect-tool-results` guarantee. `ContentRouter.apply()` computes `read_protection_window` from `protect_recent_reads_fraction`, where `0.0` (the sentinel `--protect-tool-results` sets) means "protect all excluded-tool output regardless of conversation depth" per #1374's documented contract — but the method then unconditionally overwrote that window with a `read_protection_window` kwarg whenever one was present. `proxy_pipeline_kwargs()` supplies that kwarg on every request from the active `AgentSavingsProfile.protect_recent` (the default `coding` profile sets `protect_recent=2`), so in practice only the last 2 messages ever kept read-protection and older excluded-tool output silently fell through to lossy compression. The runtime kwarg may now only narrow the window when `protect_recent_reads_fraction > 0`; it can no longer shrink the "protect everything" guarantee set by `--protect-tool-results`. +* **memory:** drop a superseded memory from the search indexes so it stops resurfacing. `supersede` set the old memory's `valid_until` in the store and indexed the new version, but never touched the old entry in the vector/text index. Those indexes keep a cached metadata copy (captured at index time with `valid_until=None`), and default search filters superseded rows off that cached copy — so the superseded, outdated version kept coming back from semantic/text search alongside the new one, injecting contradictory facts into recall. `supersede` now removes the old id from the vector and text indexes (mirroring `delete`); the store still keeps the row for `get_history`. * **proxy:** don't let a stray `HEADROOM_QDRANT_PORT` crash proxy startup. `ProxyConfig.memory_qdrant_port` used `qdrant_env.qdrant_env_port` as its field `default_factory`, and that function raises `ValueError` on a non-integer or out-of-range value. Because a `default_factory` runs on **every** `ProxyConfig()` construction, an inherited or typo'd `HEADROOM_QDRANT_PORT` crashed the proxy before it served a request — even though memory (and the qdrant backend) are off by default and unrelated to core proxying. The field now resolves the port through a fail-soft wrapper that falls back to the default (6333) with a warning; the strict `qdrant_env_port()` is unchanged for explicit qdrant setup. * **memory:** size the HNSW `index_batch` resize off the assigned-id high-water mark, not the live entry count. hnswlib never reclaims a slot on `mark_deleted` (used by remove/evict), so its usable capacity is bounded by the number of ids ever assigned (`_next_hnsw_id`). `index_batch` computed `required_capacity = len(self._memory_to_hnsw) + len(new_memories)` — the *live* count — which drops below `_next_hnsw_id` after deletion/eviction churn, so the resize was skipped and `add_items` raised `RuntimeError: number of elements exceeds the specified limit`, crashing the save path on the HNSW backend. It now resizes off `_next_hnsw_id`, matching the single-item `index()` guard. * **memory:** apply the `turn_id` scope filter even when `agent_id` is absent. In `SQLiteMemoryStore._build_query_conditions` the `turn_id` condition was nested inside the `agent_id` block, so a query filtered by `user_id` + `session_id` + `turn_id` (no `agent_id`) dropped the `turn_id` predicate entirely and returned every memory in the session instead of the single turn — an over-broad result that leaks sibling-turn memories into recall (and makes `count()` wrong for that scope). `agent_id` and `turn_id` are now applied independently. diff --git a/headroom/memory/core.py b/headroom/memory/core.py index 746a2e5a9..c5b0ccf9f 100644 --- a/headroom/memory/core.py +++ b/headroom/memory/core.py @@ -562,6 +562,16 @@ class HierarchicalMemory: # Perform supersession in store new_memory = await self._store.supersede(old_memory_id, new_memory, supersede_time) + # Drop the OLD entry from the search indexes. The store keeps its row + # (valid_until is now set) so get_history still works, but the vector and + # text indexes hold a cached metadata copy with valid_until=None, and + # default search filters superseded rows off that cached copy. Without + # this, the superseded (stale) version keeps resurfacing from search + # alongside the new one, so contradictory/outdated facts get recalled + # together. Mirrors delete()'s index removal. + await self._vector_index.remove(old_memory_id) + await self._text_index.remove(old_memory_id) + # Update indexes if new_memory.embedding is not None: await self._vector_index.index(new_memory) diff --git a/tests/test_memory/test_core_operations.py b/tests/test_memory/test_core_operations.py index 8548f8367..29a639d74 100644 --- a/tests/test_memory/test_core_operations.py +++ b/tests/test_memory/test_core_operations.py @@ -20,7 +20,9 @@ import os os.environ["TOKENIZERS_PARALLELISM"] = "false" import functools +import gc import tempfile +import time from pathlib import Path import pytest @@ -70,7 +72,15 @@ def temp_db_path(): path = Path(f.name) yield path # Cleanup - path.unlink(missing_ok=True) + gc.collect() + for attempt in range(5): + try: + path.unlink(missing_ok=True) + break + except PermissionError: + if attempt == 4: + raise + time.sleep(0.1) for suffix in ["-shm", "-wal", ".hnsw"]: Path(str(path) + suffix).unlink(missing_ok=True) @@ -491,6 +501,60 @@ class TestSupersede: found_ids = [r.memory.id for r in results] assert new.id in found_ids + @pytest.mark.asyncio + @network_timeout_handler + async def test_superseded_memory_does_not_resurface_in_search(self, memory_system): + """The superseded (old) version must not keep coming back from search. + + The vector/text index cached the old entry's metadata with + valid_until=None; default search filters superseded rows off that cached + copy, so before the fix a search that matched the old content returned + the stale version alongside the new one. + """ + old = await memory_system.add( + content="User prefers Python", + user_id="alice", + ) + new = await memory_system.supersede( + old.id, + "User now prefers JavaScript frameworks", + ) + + # A search matching the OLD content must not resurface the old entry. + results = await memory_system.search("Python", user_id="alice") + found_ids = [r.memory.id for r in results] + assert old.id not in found_ids + + # The new version is still searchable. + new_results = await memory_system.search("JavaScript", user_id="alice") + assert new.id in [r.memory.id for r in new_results] + + @pytest.mark.asyncio + @network_timeout_handler + async def test_superseded_index_removal_boundary(self, memory_system): + """Boundary of the supersede index-removal (#2143). + + supersede now drops the old id from the vector/text index (not just + flips valid_until on the cached copy), so a search matching the old + content will not surface it even with include_superseded=True — the + entry is gone from the search index, not merely filtered. The store + still keeps the row, so get_history stays the source of truth for the + superseded version. This pins that contract so a future change that + relies on include_superseded search hitting the index fails loudly. + """ + old = await memory_system.add(content="User prefers Python", user_id="alice") + new = await memory_system.supersede( + old.id, + "User now prefers JavaScript frameworks", + ) + + incl = await memory_system.search("Python", user_id="alice", include_superseded=True) + assert old.id not in [r.memory.id for r in incl] + + # Retained in the store for history/audit even though it left the index. + history = await memory_system.get_history(new.id) + assert old.id in [m.id for m in history] + # ============================================================================= # History Tests