fix(recover): do not purge memories that have no source - #9
Merged
Conversation
An absent `source` was normalised to "unknown", failed the trusted-set membership test, and was classified PURGE. Running `memorywire recover` (which applies by default on the CLI, where --dry-run is opt-in) against a store written before 0.2.0 — when --source landed on `remember` — therefore soft-deleted every row while reporting "untrusted source 'unknown'". Absence of provenance is not evidence of untrusted origin. Unsourced records are now kept with the verdict reason "clean (no source recorded)", so they stay visible in the report rather than being silently skipped, and are still passed through the content detectors — a directive hiding in an unsourced memory is quarantined as before. Records with an explicitly untrusted source are unaffected. The previous behaviour is available as opt-in via purge_unsourced (default False) on classify(), Recoverer and the MCP recover tool, and --purge-unsourced on the CLI. Regression test covers the disaster case directly: three memories with no source, recover with no dry-run, all three survive; plus the opt-in path. Documented in docs/recovery.md, along with the CLI/MCP dry-run asymmetry.
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.
The defect
classify()normalised an absentsourceto"unknown", which then failed thetrusted-set membership test and was classified
PURGE.Because
--dry-runis opt-in on the CLI,memorywire recover --agent X --store Yapplies by default. So running it against a store written before 0.2.0 — when
--sourcelanded onremember— soft-deleted every row, while the report explainedeach one as
untrusted source 'unknown'.sourceis an optional field. Absence of provenance is not evidence of untrusted origin.The fix
clean (no source recorded),so it stays visible in the report rather than being silently skipped.
memory is quarantined (soft-delete, restorable) exactly as before.
sourceare unaffected.purge_unsourced(defaultFalse) onclassify(),Recovererand the MCPrecovertool, and--purge-unsourcedon the CLI.Verification
Regression tests cover the disaster case directly — three memories with no source,
recoverwith no dry-run, assert all three survive — plus the opt-in path, thedetector path on unsourced records, and that explicitly-untrusted records still purge
regardless of the flag.
Reproduced end to end against a real store before and after:
323 unit tests pass; ruff check and ruff format clean.
Docs
docs/recovery.mdgains a "Memories with no source" section, corrects the verdict table(the purge row described an explicitly-untrusted value while the code fired on absence),
and notes the asymmetry that
--dry-runis opt-in on the CLI while the MCP tool defaultsdry_runto true.Reported by an external implementer building an adapter against the wire format.