Repository navigation
Enforce revocation at retrieval, add erasure and provenance - #11
Merged
Merged
Conversation
The live count used element_count, which also counts labels left behind when an id is added again. After delete-then-add, k exceeded the real live set, hnswlib raised, and search() swallowed it as "no matches". Count live ids from the uuid map instead, and tolerate mark_deleted on a label that is already deleted. Reinstating a revoked fact depends on this path.
arXiv 2609.08258 found that agent-memory systems keep a revoked fact retrievable: it outranks its replacement and drives the unsafe action. This store had the same gap. supersede() only set superseded_by, which the retriever filtered behind a config flag, and re-recording the old text produced a fresh, unmarked fact that ranked first again. Visibility is now decided by one SQL view, hidden_facts. Every store read selects from live_facts, live_events or live_messages, so vector hits (including from a stale in-memory index), keyword search, session ranking, events, transcript reads, tools, HTTP and MCP inherit the same rule. The retriever re-checks visibility after reranking, which closes the window for a revocation that lands mid-query. A structural test fails if a store read touches the base tables outside the governance methods. A tombstone hides the fact by id, every same-scope fact whose normalized text matches (so a write cannot un-revoke by re-inserting the claim), and facts recorded with derived_from pointing at a hidden fact, which covers the paper's write-back path. reinstate() is the only way back and is recorded on the tombstone. erase() hard-deletes the fact, its copies, derived facts, linked events and transcript turns in one transaction, turns on SQLite and FTS5 secure delete, checkpoints the WAL, rebuilds the vector index without the erased vectors, and leaves a record with ids and counts but no content. Every recall result now carries provenance (writer, source, time, derivation, status and when it was checked). Governance state lives in side tables so 0.2 databases upgrade on open; existing HTTP response shapes are unchanged.
The README section states where enforcement lives, what a tombstone hides, the re-insertion rule, and a table of what erase() can and cannot vouch for per layer, so callers do not read more into "deleted" than the backend delivers. uv.lock was also stale on main (0.1.0, missing typer).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
Supersession, reinstatement, tenant isolation, erasure concurrency, and vector lifecycle issues remain unresolved.
6 open findings
What changed in this PR
Adds centralized revocation enforcement, hard erasure, provenance reporting, and related vector-index fixes.
Changes:
- Enforces tombstones across storage and retrieval paths.
- Adds erasure and provenance APIs, HTTP/MCP support, and documentation.
- Fixes HNSW delete/re-add behavior and expands governance tests.
| File | Description |
|---|---|
uv.lock |
Updates package metadata and Typer dependency. |
pyproject.toml |
Bumps version to 0.3.0. |
CHANGELOG.md |
Documents governance features and fixes. |
README.md |
Documents revocation, erasure, and provenance. |
src/engram/__init__.py |
Exports governance types. |
src/engram/engram.py |
Adds governance facade APIs. |
src/engram/governance.py |
Defines normalization and governance models. |
src/engram/retrieve/base.py |
Adds provenance to retrieval results. |
src/engram/retrieve/hybrid.py |
Adds batched lookup and final visibility checks. |
src/engram/server/http.py |
Adds provenance HTTP support. |
src/engram/server/mcp.py |
Adds provenance and derivation options. |
src/engram/store/base.py |
Extends the storage contract. |
src/engram/store/schema.sql |
Adds governance tables and live views. |
src/engram/store/sqlite.py |
Implements revocation, provenance, and erasure. |
src/engram/vector/base.py |
Adds physical vector purging. |
src/engram/vector/hnsw.py |
Fixes re-adds and implements index rebuilding. |
tests/unit/test_cli.py |
Updates version assertion. |
tests/unit/test_engram_init_exports.py |
Tests governance exports. |
tests/unit/test_erasure.py |
Covers hard-erasure behavior. |
tests/unit/test_provenance.py |
Covers provenance surfaces. |
tests/unit/test_revocation.py |
Covers revocation enforcement. |
tests/unit/test_revocation_choke_point.py |
Guards centralized visibility enforcement. |
tests/unit/test_vector.py |
Covers vector re-add behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+353
to
+356
| source_message_id=source_message_id, | ||
| ) | ||
| await self._store.upsert_fact( | ||
| fact, writer=actor, source="record", derived_from=derived_from |
Member
Author
There was a problem hiding this comment.
Fixed in 0050d70: upsert_fact refuses a source_message_id that is not a message in the fact's scope (NotFoundError). Test: test_source_message_must_belong_to_the_same_scope.
Comment on lines
+854
to
+858
| root = await self._governance_fact(fact_id, scope) | ||
| if root is None: | ||
| raise NotFoundError(f"fact {fact_id} not found in scope") | ||
|
|
||
| erased: dict[str, Fact] = {} |
Member
Author
There was a problem hiding this comment.
Fixed in 0050d70: erase now runs discovery and deletion inside one BEGIN IMMEDIATE transaction (revoke does the same).
Comment on lines
+227
to
+230
| rev = await self._store.revoke(fact_id, scope, reason=reason, actor=actor) | ||
| # Not needed for correctness (the store withholds it), but keeps the | ||
| # revoked vector from taking a slot in the candidate pool. | ||
| await self._vec.delete(fact_id, scope) |
Member
Author
There was a problem hiding this comment.
Fixed in 0050d70: revoke and supersede drop the vectors of every fact the tombstone now hides, and record() skips the vector for a fact that is born hidden. Test: test_hidden_copies_do_not_occupy_vector_slots.
Comment on lines
+276
to
+279
| record = await self._store.erase(fact_id, scope, reason=reason, actor=actor) | ||
| removed = await self._vec.purge(record.fact_ids, scope) | ||
| return await self._store.finalize_erasure( | ||
| record, {"vectors": removed}, {"vector_index_rebuilt": True} |
Member
Author
There was a problem hiding this comment.
Partly. The SQL side is atomic and the record exists once it commits. Vector purge and the WAL checkpoint run after; the vector index is in-memory only, so a crash in between drops it anyway, and the record's guarantees only report what finalize actually did. A resumable pending state is noted as open in the PR body rather than added here.
Comment on lines
+719
to
+720
| if existing is not None: | ||
| return self._row_to_revocation(existing) |
Member
Author
There was a problem hiding this comment.
Fixed in 0050d70: supersede on an already revoked fact attaches the replacement to the existing tombstone and links both facts; a different replacement than the recorded one is refused. Test: test_supersede_of_an_already_revoked_fact_still_links_the_replacement.
Comment on lines
+885
to
+888
| norms = {normalize_text(f.text) for f in erased.values()} - {""} | ||
| norm_reason = normalize_text(reason) | ||
| if any(_contains_phrase(norm_reason, n) for n in norms): | ||
| raise ValueError("erasure reason must not quote the content being erased") |
Member
Author
There was a problem hiding this comment.
Tightened in 0050d70: reason and actor are refused if they contain any three-token run of an erased text, and other tombstone reasons quoting it are scrubbed. A single distinctive word can still pass; that limit is stated in the README, which recommends keeping audit fields to ticket numbers and roles.
An adversarial review of the first commit reproduced paths where revoked content still reached the agent or survived erasure: - Transcript turns and extracted events carry the claim without a link to the fact, so the link-based view rules missed them, and query-time re-extraction fed revoked text straight back to the reader. Tombstones now also match items that equal or quote the revoked text (whole-token containment for texts of four or more tokens). Matches are computed when the revocation or the item is written and stored in content_matches, so the view stays an indexed lookup. Re-extraction and extract() drop anything a tombstone would withhold. - Invisible characters outside category Cf (variation selectors, CGJ, Hangul filler, braille blank) split words and changed the hash. All default-ignorable code points are now removed. In the other direction, mapping symbols to spaces made "> 10000" and "< 10000" one claim, which hid the correct replacement; symbols, a leading minus and % # @ & are now kept, and empty normalized texts get no content tombstone. - Reinstating a born-hidden copy lifted the original's tombstone, so a write was a handle to un-revoke. Reinstate now only works on the revoked fact itself. - Erasing a revoked fact dropped its hash, so the claim could be written back. The hash is kept by default (block_reinsertion=False drops it). Erasure now also removes facts and events that quote the text, scrubs other audit text quoting it, checks the actor as well as the reason against three-token runs, and holds a write transaction from discovery to commit so a concurrent copy cannot slip in. - The session-wide transcript rule keyed on any hidden fact, so an unauthenticated write of revoked text could withhold a session. It now keys on an operator revocation. - Store upserts could overwrite a row in another scope by id, and source_message_id was not scope-checked. - supersede() on an already revoked fact dropped the replacement link; governance calls given a Fact ignored its scope; hidden copies kept their vectors and crowded the candidate pool. Each fix has a test that failed on the previous commit, and every new guard was mutation-checked against an unmutated control.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
"Revoked but Still Authoritative" (Shen, Toyoda, Leung, arXiv 2609.08258) loaded five agent-memory systems with a revoked policy and its replacement, across nine policy scenarios and nine models. No system enforced revocation by default. Where the revocation label was visible to retrieval, the revoked fact was returned in 81/81 scenarios, ranked first in every one, and agents took the unsafe action in 43.1% of trials (pooled). The paper also shows the write-back path: an agent that acts on the revoked fact and records its decision puts that conclusion back in the store as an ordinary current record, which a store-level filter does not catch.
This library had the same gap.
supersede()setsuperseded_by, the retriever filtered on it behindRetrievalConfig.exclude_superseded, and recording the old text again created a fresh unmarked fact.tests/unit/test_revocation.pyreproduces the scenario: a control test shows the old deploy-approval policy ranks first and a deterministic agent that follows the top memory deploys after one approval. Onmain, the write-back test and the history-mode test fail with the old policy ranked first; on this branch they pass.Design
Threat model. A fact is revoked by an operator with a reason. After that, nothing that reads memory on behalf of an agent should see it: not the facade, not the reader tools, not query-time re-extraction from transcripts, not HTTP or MCP, not a second process whose in-memory vector index still holds the vector. Writers (agents, ingestion, an unauthenticated
/v1client) must not be able to undo a revocation by writing.One choke point. Visibility is decided by a single SQL view,
hidden_facts(inschema.sql, recreated on every open). Every store read selects fromlive_facts,live_eventsorlive_messages, built on it. The vector index returns ids only and ids resolve throughlive_facts, so a stale index cannot hand back text. The retriever's last step fetches provenance fromlive_facts, which doubles as a re-check after reranking (a revocation that lands while a reranker awaits is still applied).test_revocation_choke_point.pyparsessqlite.pyand fails if any method outside an exact allowlist of five governance methods readsfacts,eventsormessagesdirectly; none of those five returns fact text.What a tombstone hides.
upsert_fact;% # @ &kept so "> 10000" and "< 10000" differ). Matches are computed at write time intocontent_matchesso the view stays an indexed lookup;derived_frompointing at a hidden fact, transitively (the paper's write-back case);source_fact_idsinclude a hidden fact;source_message_id; for a fact an operator revoked by id with no link, all turns of its session (keyed on the revocation, so a plain write cannot withhold a session).extract()drop anything a tombstone would withhold.Re-insertion rule (decided). A write cannot un-revoke. Re-recording the claim stores it born hidden;
record(reject_revoked=True)raisesRevokedContentErrorinstead. Tombstones live in their own table, so re-upserting the fact object (clearingsuperseded_by,invalid_at, metadata) changes nothing.reinstate(fact, actor, reason)on the revoked fact itself is the only way back and is recorded on the tombstone; reinstating a born-hidden copy is refused. Paraphrases, "one" vs "1", and cross-script homoglyphs are not matched, by design; there is a test pinning that so changing it is deliberate.API (Python).
revoke(fact_or_id, *, reason, actor),supersede(old, new, actor=, reason=)(now returns theRevocation; rejects a revoked replacement and a replacement with the same normalized text),reinstate(...),erase(...),explain(fact_id),revocations(),erasures(),context_entries(),record(actor=, derived_from=, source_message_id=, reject_revoked=). Revoke, reinstate and erase are not exposed over HTTP or MCP in this PR because/v1/memory/*is unauthenticated by design (v0.2 spec); that should follow auth on/v1.Erasure.
erase()deletes, in one transaction: the fact, same-text copies in scope, derived facts (transitively), events listing them, cited transcript turns, and any transcript turn in scope containing an erased text. Also removes facts and events that quote the text, and scrubs any audit text quoting it. Tombstones on erased facts lose their normalized text but keep the content hash by default so revoked content stays blocked (block_reinsertion=Falsedrops it; the README notes a short text's hash can be confirmed by guessing). Discovery and deletion run in oneBEGIN IMMEDIATEtransaction. The vector index for the scope is rebuilt without the erased vectors (hnswlibmark_deletedkeeps them in memory). Returns anErasureRecordwith ids, counts, actor, reason and per-layerguarantees, and no content; a reason or actor containing any three-token run of the content is rejected (a single distinctive word can still pass; documented).What it can and cannot show:
PRAGMA secure_delete=ON, FTS5secure-delete(SQLite 3.44+, reported), WAL checkpoint with truncate. A test greps the db,-waland-shmfiles for the erased token (control: present before). If another connection holds a read snapshot the checkpoint cannot finish and the record sayswal_checkpointed: false; a test holds a reader open to check that.Provenance.
ScoredFact.provenanceon every recall result: writer, source (record/extract/upsert/legacy), recorded_at, session, source message,derived_from, supersession links, status andchecked_at.explain()also reports why a hidden fact is hidden (revoked,content_tombstone,derived_from_revoked) and which tombstones cause it. HTTP addsPOST /v1/memory/recall_with_provenance; MCPmemory_recalltakesinclude_provenance.Compatibility. No Rust LTS wire change: existing HTTP routes keep their exact response shapes (asserted in a test); new request fields (
actor,derived_from) are optional; new route and MCP arguments are additive. Governance state is in side tables (fact_meta,fact_derivations,revocations,erasures) created on open; 0.2 rows are backfilled with content hashes (tested).RetrievalConfig.exclude_superseded=Falseno longer surfaces facts revoked viasupersede().Cost. Recall latency on 5,000 facts / 50 tombstones / 200 queries: 5.9 ms on
main, 5.7 ms here (the per-idget_factloop in the retriever became one batchedget_facts).Also fixed
HNSW search returned no vector hits after a vector was deleted and re-added (the live count included stale labels,
kexceeded the live set, hnswlib raised and the error was swallowed). Separate commit with its own failing test.Independent review
An adversarial reviewer (separate agent, read-only) reproduced: unlinked transcript turns and extractor-produced events still carrying revoked text, re-extraction feeding it to the reader, invisible characters outside category Cf escaping the hash, symbol folding that merged opposite claims, reinstate of a copy un-revoking the original, erasure re-opening revoked content and leaving quoting facts and audit text behind, a session-withholding write, and a cross-scope upsert by id. Copilot flagged the erase race, scope of
source_message_id, hidden copies crowding the vector pool, and the supersede-on-revoked link. All are fixed in the last commit with tests that fail on the commit before it; the reviewer's repro scripts now hit the guards.Still open by design and documented: paraphrases; a single distinctive word in an erasure reason; a 0.2 library opening the same file ignores the views; erasure finalization (vector purge, WAL checkpoint) runs after the SQL commit, so a crash in between leaves the record without the vector entry (the index is in-memory, so a crash drops it anyway).
Verification
ruff check,ruff format --check,mypy --strict, full pytest (443 passed, 5 skipped for local Ollama) with coverage gate.Version bumped to 0.3.0; not published.
🤖 Generated with Claude Code