Skip to content

fix(forget): delete across every substrate holding the content (#366) - #374

Merged
cdeust merged 2 commits into
mainfrom
fix/forget-cross-substrate-366
Aug 7, 2026
Merged

fix(forget): delete across every substrate holding the content (#366)#374
cdeust merged 2 commits into
mainfrom
fix/forget-cross-substrate-366

Conversation

@cdeust

@cdeust cdeust commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #366.

PRIVACY.md tells users "The forget tool deletes individual memories." A hard delete issued a single DELETE FROM memories. Two substrates kept the content:

  • the raw full text of an oversized auto-capture, in its content-addressed artifact under ~/.claude/methodology/artifacts/artifact_store.py had no removal path at all, so the row's gist went and the complete original stayed readable on disk;
  • wiki claim events derived from the memory, which survive by design: pg_schema.py:221 declares wiki.claim_events.memory_id ON DELETE SET NULL, so after the row goes the claim persists and can no longer be found by id.

Root cause, not just the symptom

The missing unlink was downstream of a missing contract: the artifact pointer format was duplicated as an f-string in hooks/post_tool_capture and handlers/backfill_helpers, so nothing could parse it back safely and a drift in either writer would have broken any deleter silently. format_artifact_pointer / parse_artifact_pointer now own that format in core, and both writers call it.

Three things that would have made a naive fix wrong

  1. Artifacts are content-addressed with dedup — byte-identical captures map to ONE file, so unlinking it for the first of two referrers strips the survivor's content. artifact_gc counts referrers first and fails closed: an unknown count keeps the file.
  2. Claims must be deleted BEFORE the row — the FK nulls the link, after which the claim cannot be found by memory_id.
  3. The reference count must be taken AFTER the row is gone — otherwise the memory being forgotten counts itself and nothing is ever collected.

Neither ordering is visible from the happy path; each has its own test.

soft=true keeps the artifact (a soft delete is recoverable by design) and reports artifact_deleted: false rather than omitting it, so a caller can tell kept on purpose from we forgot to look.

Completion Ledger

Test names below are enumerated from git diff origin/main...HEAD — not recalled.

Diff paths → asserting test

Path in the diff Asserting test
hard delete removes the artifact test_hard_delete_removes_the_artifact_file
the removal is reported in the result (§13 F1) test_hard_delete_reports_artifact_removal
soft delete retains the artifact test_soft_delete_keeps_the_artifact
refused (protected) delete touches nothing test_refused_delete_keeps_the_artifact
shared artifact survives while another referrer lives test_shared_artifact_survives_while_another_memory_references_it
no-artifact memory reports false, not None test_memory_without_artifact_reports_false_not_none
derived wiki claims removed + count reported test_hard_delete_removes_derived_claims_and_reports_the_count
zero-claims case test_memory_with_no_claims_reports_zero
already-absent artifact emits an actionable signal naming the path test_already_absent_artifact_logs_the_path_it_expected
pointer format/parse round trip test_round_trip_recovers_the_path, test_round_trip_inside_a_full_memory_body
path containing spaces not clipped test_path_containing_spaces_survives
char-count suffix not absorbed into the path test_char_count_suffix_is_not_absorbed_into_the_path
absent / empty / malformed pointer → None (never a guessed path) test_absent_pointer_returns_none, test_empty_content_returns_none, test_malformed_pointer_returns_none_rather_than_a_bad_path
two pointers in one body → first wins test_first_pointer_wins_when_a_body_carries_two

§13.1 checklist

Item Evidence
A1 happy paths, output quoted Full suite 7,090 passed, 81 skipped, 123 subtests, 0 failures (229s, post-rebase)
A2 edge cases shared artifact, absent file, no pointer, malformed pointer, two pointers, zero claims
A3 every failure arm asserted incl. the signal reference-count failure fails closed; already-absent path asserts the log names the path
A5 partial failure delete_artifact_if_unreferenced never raises; each non-removal path logs why and returns False
B1–B3 concurrency N/A — no new concurrency; deletion is a single request path
C2 resource lifecycle this PR adds the only artifact-reclaim path; the store previously only grew
D1–D3 security parameterised SQL only; no secrets; deletion narrows exposure
E1 API additive artifact_deleted / claims_deleted added to the result; no field removed
E3 persisted-data compat no schema change
F1 signals asserted above
G3 determinism tests use the conftest-isolated CORTEX_CLAUDE_DIR + throwaway DB
G4 negative assertions soft/refused/shared cases assert the artifact is still present
G5 lint ruff check + ruff format --check clean on all changed files
H1 rules sizes 110/176/234 lines; core/ imports no os/pathlib/infrastructure
H4 docs PRIVACY.md scope + both exceptions, docs/mcp-tools.md, CHANGELOG
§12 mutation 0 surviving mutants, 347/347 on artifact_gc.py, gist_extraction.py, forget.py
H7 boy-scout (§14) the run surfaced 5 pre-existing gaps in extract_gist/_fill_tail — a reversed step made the tail range EMPTY (the end of every dump vanished) and break-instead-of-continue truncated it; all fixed here, not deferred

Verification honesty

The first mutation run reported 3 survivors and I treated it as trustworthy. It was not: the run that matters reported 8, six of them in this PR's own new code, including _delete_derived_claims — where no test proved claims were actually deleted, so half of #366's purpose was unasserted while the suite stayed green. All 8 are now killed.

🤖 Generated with Claude Code

@cdeust cdeust closed this Aug 7, 2026
@cdeust cdeust reopened this Aug 7, 2026
cdeust and others added 2 commits August 7, 2026 10:41
PRIVACY.md tells users "The `forget` tool deletes individual memories", but a
hard delete issued a single DELETE FROM memories. Two substrates kept the
content:

  - the raw full text of an oversized auto-capture, in its content-addressed
    artifact under ~/.claude/methodology/artifacts/ — artifact_store.py had no
    removal path at all, so the row's gist went and the complete original
    stayed readable on disk;
  - wiki claim events derived from the memory, which survive by design:
    pg_schema.py declares wiki.claim_events.memory_id ON DELETE SET NULL, so
    after the row goes the claim persists and can no longer be found by id.

Root cause of the first was not the missing unlink but the missing contract:
the artifact pointer format was duplicated as an f-string in
hooks/post_tool_capture and handlers/backfill_helpers, so nothing could parse
it back safely. format_artifact_pointer / parse_artifact_pointer now own that
format in core, and both writers call it.

Removal is reference-counted. store_artifact is content-addressed with dedup,
so byte-identical captures map to ONE file; unlinking it for the first of two
referrers would strip the survivor's content. artifact_gc counts referrers
first and fails CLOSED — an unknown count keeps the file.

Two orderings are load-bearing and each has its own test: claims must be
deleted BEFORE the row (the FK nulls the link), and the artifact reference
count must be taken AFTER it (or the memory counts itself and nothing is ever
collected). Neither is discoverable from the happy path.

soft=true keeps the artifact: a soft delete is recoverable by design, so its
content must survive with it. Reported as artifact_deleted=false rather than
omitted, so a caller can tell "kept on purpose" from "we forgot to look".

PRIVACY.md now states the exact scope including both exceptions, and adds the
artifacts directory to the user's controls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scoping mutmut to the three changed files reported 8 genuine survivors, 6 of
them in the code this series added. Each was a real gap, not an equivalent:

  forget/_delete_derived_claims: no test proved claims are actually deleted —
    passing the wrong arity still returned 0 through the mechanism boundary, so
    half of #366's purpose was unasserted. Now seeds a wiki claim_event, forgets
    the memory, and asserts both the reported count and that the claim is gone.
  forget/_handler_impl: the claims_deleted key was never read by a test, so
    renaming it survived. Asserted, alongside the zero case.
  artifact_gc/delete_artifact_if_unreferenced: the already-absent branch logged
    a path nothing checked. §13 F1 wants the signal itself asserted, so the test
    now pins that the message names the path it expected.

Two pre-existing gaps in the same file, in scope under §14 because the run that
saw them was this change's own verification:

  _fill_signal's budget check and char charge. Both inflate the gist past its
    stated bound, which defeats the reason the budget exists (an oversized
    capture must stop outranking curated memories). Bounded by a measured
    assertion plus a check that the elision marker's kept count agrees with the
    budget actually consumed.
  extract_gist's elision total and _fill_tail's direction/skip, from the first
    round: a reversed step makes the tail range EMPTY so the end of every dump
    vanishes, and break-instead-of-continue truncates the tail whenever a signal
    line sits inside the tail window.

Scoped mutation run now reports 0 surviving mutants, 347/347.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cdeust
cdeust force-pushed the fix/forget-cross-substrate-366 branch from b030073 to 39f476e Compare August 7, 2026 08:41
@cdeust
cdeust merged commit 9381f75 into main Aug 7, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

privacy: forget() deletes one row — raw artifacts are never deleted and wiki claims are only SET NULL, so PRIVACY.md overstates deletion

1 participant