fix(mcp): smem_edit recomputes expires_at when the type changes, without reviving tombstones or ephemerals - #214
Merged
acidkill merged 1 commit intoSep 7, 2026
Conversation
…out reviving tombstones or ephemerals _edit's type-change branch swapped memory_type and left the OLD type's TTL on the record. DEFAULT_EXPIRY_DAYS is only consulted at creation time, via remember_handler.expires_in_days, and TypedMemory is a frozen dataclass with no __post_init__, so nothing recomputed it afterwards either. A DECISION (90d) edited to FACT (no default expiry) still expired about ninety days out; the other way round, a FACT edited to TODO or ERROR (30d each) never picked up their finite expiry and persisted indefinitely. After the swap, _edit now reads DEFAULT_EXPIRY_DAYS for the new type and either clears expires_at or calls TypedMemory.extend_expiry — a helper that already existed and had no caller in mcp/. Two cases must be left alone, and the recompute is guarded against both: * A soft-deleted memory. _forget(hard=False) tombstones by setting expires_at=utcnow(); recomputing from the new type's default would hand a deliberately forgotten memory an open-ended life. The guard skips any record whose expires_at is at or before now. * An ephemeral memory. remember_handler gives these a one-day TTL and flags the anchor neuron ephemeral=True; clearing that TTL would make an "auto-expires, never synced" memory permanent. The guard reads the anchor neuron and skips the recompute when the flag is set. The type swap itself still applies in both cases — only the TTL recompute is skipped. _forget stays untouched; it remains the only other writer of expires_at. No new argument is added to smem_edit: exposing expires_at explicitly is a schema change and is the maintainer's call, not this fix's. Four tests in tests/unit/test_edit_forget.py, one per direction and one per guard: test_edit_type_change_recomputes_ttl (DECISION to FACT clears the expiry), test_edit_type_change_picks_up_finite_expiry (FACT to TODO gains one), test_edit_type_change_does_not_resurrect_soft_deleted, and test_edit_type_change_preserves_ephemeral_ttl.
acidkill
approved these changes
Sep 7, 2026
acidkill
left a comment
Owner
There was a problem hiding this comment.
Reviewed the full diff. Type change recomputes expires_at from DEFAULT_EXPIRY_DAYS[new_type]; the tombstone/ephemeral carve-outs prevent resurrecting soft-deleted memories or making ephemerals immortal. Both directions tested.
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.
Summary
smem_editnow recomputesexpires_atfrom the new type's default when the memory type changes, instead of leaving the old type's TTL in place.Why
_edit's type-change branch swappedmemory_typeand left the old TTL untouched.DEFAULT_EXPIRY_DAYSis consulted only at creation time, throughremember_handler'sexpires_in_days, andTypedMemoryis a frozen dataclass with no__post_init__, so nothing recomputed it afterwards either.The result was wrong in both directions. A
DECISION(90 days by default) edited toFACT(no default expiry) still expired roughly ninety days out, so a fact quietly vanished on the decision's clock. Going the other way, aFACTedited toTODOorERROR(30 days each) never acquired a finite expiry and persisted indefinitely.The fix reads
DEFAULT_EXPIRY_DAYSfor the new type after the swap and either clearsexpires_ator callsTypedMemory.extend_expiry— a helper that already existed on the dataclass and, until now, had no caller anywhere inmcp/.Two memories the recompute must not touch
An unconditional recompute is a data-integrity bug of its own, so it is guarded twice:
_forget(hard=False)tombstones by settingexpires_at=utcnow(). Recomputing from the new type's default would hand a deliberately forgotten memory an open-ended life — aFACTedit would make it immortal. The guard skips any record whoseexpires_atis at or before now.remember_handlergives these a one-day TTL and flags the anchor neuronephemeral=True. Clearing that expiry would turn a memory documented as "auto-expires after 24h, never synced" into a permanent one. The guard reads the anchor neuron and skips the recompute when the flag is set.In both cases the type swap still applies; only the TTL recompute is skipped.
_forgetis untouched and remains the only other writer ofexpires_at.Changes
mcp/lifecycle_handler.py: importDEFAULT_EXPIRY_DAYS; after the type swap, compute the tombstone and ephemeral conditions and, when neither holds, clear or extend the expiry.tests/unit/test_edit_forget.py: four tests, capturing theTypedMemoryhanded tostorage.update_typed_memory— the interface between_editand persistence, rather than any downstream effect.test_edit_type_change_recomputes_ttl(DECISION to FACT clears it),test_edit_type_change_picks_up_finite_expiry(FACT to TODO gains 30 days),test_edit_type_change_does_not_resurrect_soft_deleted, andtest_edit_type_change_preserves_ephemeral_ttl.Not touched, on purpose
No new argument is added to
smem_edit. Exposingexpires_atexplicitly would be the more flexible answer, but it is a schema change and a decision for you rather than for a bug fix. If you would rather have that, this PR is easy to rebase into it.Test plan
pytest tests/unit/test_edit_forget.py— 19 passed, against 15 onmain; the delta is exactly the four tests added here.mainand keeping the tests: 2 failed, 17 passed — the two direction tests fail, and the two guard tests pass, because with no recompute at all there is nothing for a guard to prevent.pytest tests/ -m "not stress" -n 4against a live SurrealDB v3.2.0 — 7287 passed, 48 skipped, 1 xfailed, which ismain's 7283 plus the four tests here. Two tests intests/unit/test_dashboard_brains_scope.pyfail on this branch and onmainalike: they want a live database and collide with one another under-n. Both pass when that file is run on its own.ruff check src/ tests/clean;ruff format --check src/ tests/reports 739 files already formatted.mypy src/ --ignore-missing-imports— success, no issues found in 354 source files.main.CHANGELOG.mduntouched — left to the release entry, as with fix(storage): bind datetimes in time comparisons so they select by value #191–fix(memory): refresh content-derived fields on compress, restore, and refine #193.Verified by
@RobertSigmundsson