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

CHANGELOG.md

@@ -6,6 +6,32 @@ adheres to [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- **`forget` now deletes across every substrate that holds the content
+  (issue #366).** PRIVACY.md told users "The `forget` tool deletes individual
+  memories", but a hard delete issued a single `DELETE FROM memories`: the raw
+  full text of an oversized auto-capture stayed on disk in its content-addressed
+  artifact (`artifact_store` had no removal path at all), and wiki claim events
+  derived from the memory survived with `memory_id` nulled by
+  `ON DELETE SET NULL`. A hard delete now removes the row, the derived claims,
+  and the artifact, reporting `artifact_deleted` / `claims_deleted` in its
+  result. Two behaviours are deliberate and asserted: an artifact shared by a
+  still-live memory is kept (content addressing dedups identical output to one
+  file, so unconditional removal would strip the survivor's content), and a
+  `soft=true` delete retains the artifact because it is recoverable by design.
+  Deletion ordering is load-bearing in both directions — claims must go before
+  the row (the FK nulls the link) and the artifact reference count must be taken
+  after it (or the memory counts itself) — and each ordering has its own test.
+  PRIVACY.md now states the exact scope, including both exceptions.
+- **The artifact pointer format has one definition (issue #366).** It was
+  duplicated as an f-string in `hooks/post_tool_capture` and
+  `handlers/backfill_helpers`, so no reader could parse it safely. Both writers
+  now call `core.gist_extraction.format_artifact_pointer`, with
+  `parse_artifact_pointer` as its inverse; the round trip is tested for paths
+  containing spaces and for malformed pointers, which resolve to "no artifact"
+  rather than a guessed path.
+
 ### Added
 
 - **Native Codex local plugin packaging.** A dedicated

PRIVACY.md

@@ -40,7 +40,10 @@ like any other memory (locally) — avoid doing so.
   connects to any database you did not configure.
 
 You own this data. Deleting the database file (or the relevant rows) permanently
-removes it. The `forget` tool deletes individual memories.
+removes it. The `forget` tool deletes individual memories: a hard delete
+(the default) removes the memory row, any wiki claims derived from it, and the
+raw-output artifact file its body pointed at — see *Your controls* below for the
+exact scope, including the one case where an artifact is deliberately kept.
 
 ## What leaves your machine
 
@@ -87,8 +90,28 @@ memories over time; this is a local maintenance operation, not a transfer.
 
 ## Your controls
 
-- `forget` — delete a specific memory.
+- `forget` — delete a specific memory. A **hard** delete (the default) removes,
+  across every substrate that holds the content:
+  1. the memory row itself;
+  2. the wiki claim events derived from that memory;
+  3. the raw-output **artifact** file the memory body pointed at, under
+     `~/.claude/methodology/artifacts/` — an oversized auto-capture keeps only a
+     short gist in the row and the full raw text in that file, so deleting the
+     row alone would leave your content on disk.
+
+  Two deliberate exceptions, both reported in the tool's result so you can see
+  which applied:
+  - **Artifacts are content-addressed and shared.** If a second memory still
+    points at the same artifact (identical captured output dedups to one file),
+    the file is kept until the last memory referring to it is forgotten —
+    otherwise the surviving memory would lose its content. `artifact_deleted`
+    is `false` in that case.
+  - **`soft=true` keeps everything but the row's visibility.** A soft delete is
+    recoverable by design (it sets `is_stale` and `heat=0`), so the artifact is
+    deliberately retained. Use a hard delete when you want the content gone.
 - Delete `~/.claude/methodology/memory.db` — remove all SQLite-stored data.
+- Delete `~/.claude/methodology/artifacts/` — remove all raw-output artifacts,
+  including any left by soft deletes or still shared between memories.
 - For PostgreSQL, manage retention directly in your database.
 
 ## Contact

docs/mcp-tools.md

@@ -32,7 +32,7 @@ upstream MCP server is configured (55 total with both present).
 | `narrative` | Generate project narrative from stored memories | <500ms |
 | `memory_stats` | Memory system diagnostics | <50ms |
 | `import_sessions` | Import conversation history into memory store | varies |
-| `forget` | Hard/soft delete with is_protected guard | <50ms |
+| `forget` | Hard/soft delete with is_protected guard; hard delete is cross-substrate (row + derived wiki claims + unreferenced raw artifact) | <50ms |
 | `validate_memory` | Validate memories against filesystem state | <500ms |
 | `rate_memory` | Useful/not-useful feedback → metamemory confidence | <50ms |
 | `seed_project` | 5-stage codebase bootstrap | varies |

mcp_server/core/gist_extraction.py

@@ -22,6 +22,8 @@
 
 from __future__ import annotations
 
+import re
+
 # source: measured p90 curated memory length, production DB 2026-06-10
 # (3,041 chars, n=68) — see docs/provenance/bounded-io-phase2-design.md. Makes
 # auto-captures size-comparable to curated content, removing the
@@ -67,6 +69,54 @@
 ]
 
 
+# ── Artifact pointer: ONE definition, used by every writer and the reader ──
+#
+# The pointer line is the only link from a memory body to its full raw content
+# on disk. It was previously built by two duplicated f-strings: one in
+# hooks/post_tool_capture._gist_or_full, one in handlers/backfill_helpers. That
+# meant no reader could parse it safely — a drift in either writer would break
+# the parse silently. The forget handler must find and remove that artifact,
+# see issue #366, so the format is defined once here, in the layer both
+# writers already import.
+_ARTIFACT_LABEL = "**Artifact:**"
+
+# Matches the line format emitted by format_artifact_pointer. The path group is
+# non-greedy and backtick-delimited, so a path containing spaces is preserved
+# and a trailing "(N chars ...)" suffix is never swallowed into it.
+_ARTIFACT_POINTER_RE = re.compile(
+    r"\*\*Artifact:\*\*\s+`(?P<path>[^`]+)`",
+)
+
+
+def format_artifact_pointer(path: str, char_count: int) -> str:
+    """Render the pointer line linking a memory body to its raw artifact.
+
+    Pre: ``path`` is the artifact path as a string; ``char_count`` is the length
+    of the full raw output the artifact holds.
+    Post: returns a single line that ``parse_artifact_pointer`` recovers
+    ``path`` from. This is the ONLY place the format is defined.
+    """
+    return f"{_ARTIFACT_LABEL} `{path}` ({char_count} chars full output)"
+
+
+def parse_artifact_pointer(content: str) -> str | None:
+    """Recover the artifact path from a memory body, or None when absent.
+
+    Pre: ``content`` is a memory body (may be empty, may contain no pointer).
+    Post: returns the path string from the FIRST pointer line produced by
+    ``format_artifact_pointer``, else None. Never raises — a body with no
+    pointer, or a malformed one, yields None so callers treat "no artifact" and
+    "unparseable" identically (both mean: nothing safe to delete).
+    """
+    if not content:
+        return None
+    match = _ARTIFACT_POINTER_RE.search(content)
+    if match is None:
+        return None
+    path = match.group("path").strip()
+    return path or None
+
+
 def needs_gist(output: str) -> bool:
     """True when output exceeds the gist budget and should be artifact-backed.
 

mcp_server/handlers/backfill_helpers.py

@@ -12,7 +12,11 @@
 from mcp_server.infrastructure.memory_store import MemoryStore
 from mcp_server.observability import silent_failure
 from mcp_server.shared.domain_mapping import resolve_domain
-from mcp_server.core.gist_extraction import extract_gist, needs_gist
+from mcp_server.core.gist_extraction import (
+    extract_gist,
+    format_artifact_pointer,
+    needs_gist,
+)
 from mcp_server.infrastructure.artifact_store import store_artifact
 
 # Core concept keywords for entity linking
@@ -195,7 +199,7 @@ def gist_oversized_content(content: str) -> str:
     except Exception as exc:  # noqa: BLE001 — mechanism boundary; failure is observable via silent_failure
         silent_failure.note("backfill.artifact_store", exc)
         return content
-    pointer = f"**Artifact:** `{path}` ({len(content)} chars full output)"
+    pointer = format_artifact_pointer(str(path), len(content))
     return f"{extract_gist(content)}\n\n{pointer}"
 
 

mcp_server/handlers/forget.py

@@ -6,13 +6,19 @@
 
 from __future__ import annotations
 
+import logging
 from typing import Any
 
+from mcp_server.core.gist_extraction import parse_artifact_pointer
+from mcp_server.infrastructure.artifact_gc import delete_artifact_if_unreferenced
 from mcp_server.infrastructure.memory_config import get_memory_settings
 from mcp_server.infrastructure.memory_store import MemoryStore, get_shared_store
+from mcp_server.infrastructure.pg_store_wiki import delete_claims_for_memory
 from mcp_server.handlers._tool_meta import DESTRUCTIVE
 from mcp_server.handlers._telemetry_wrap import instrument
 
+logger = logging.getLogger(__name__)
+
 # ── Schema ────────────────────────────────────────────────────────────────
 
 schema = {
@@ -109,18 +115,62 @@ async def _handler_impl(args: dict[str, Any] | None = None) -> dict[str, Any]:
             "method": "soft",
             "memory_id": memory_id,
             "content_preview": mem["content"][:80],
+            # Soft delete is recoverable, so the raw content must survive with
+            # it. Reported explicitly rather than omitted so a caller can tell
+            # "kept on purpose" from "we forgot to look" (issue #366).
+            "artifact_deleted": False,
         }
 
-    # Hard delete
+    # ── Hard delete: cross-substrate, and the two orderings below are
+    # load-bearing (issue #366).
+    #
+    # The artifact path must be read BEFORE the row goes — it lives in the
+    # memory body, which is about to be unreachable.
+    artifact_path = parse_artifact_pointer(mem.get("content") or "")
+
+    # Wiki claims must go BEFORE the row: wiki.claim_events.memory_id is
+    # ON DELETE SET NULL (infrastructure/pg_schema.py), so once the memory row
+    # is deleted the claim survives with a NULL link and can no longer be
+    # found by memory_id at all.
+    claims_deleted = _delete_derived_claims(store, memory_id)
+
     deleted = store.delete_memory(memory_id)
+
+    # The reference check must run AFTER the row is gone, or the memory being
+    # forgotten counts itself as a live referrer and nothing is ever collected.
+    artifact_deleted = (
+        delete_artifact_if_unreferenced(store._conn, artifact_path)
+        if deleted
+        else False
+    )
+
     return {
         "deleted": deleted,
         "method": "hard",
         "memory_id": memory_id,
         "content_preview": mem["content"][:80],
+        "artifact_deleted": artifact_deleted,
+        "claims_deleted": claims_deleted,
     }
 
 
+def _delete_derived_claims(store: MemoryStore, memory_id: int) -> int:
+    """Remove wiki claim_events derived from ``memory_id``; return the count.
+
+    Never raises: a wiki table that is absent or unreachable (SQLite installs
+    without the wiki schema, a degraded connection) must not block the deletion
+    the user asked for. Returns 0 when nothing was removed, and logs the reason
+    so a silent miss is still observable.
+    """
+    try:
+        return int(delete_claims_for_memory(store._conn, memory_id))
+    except Exception as exc:  # noqa: BLE001 — mechanism boundary, see docstring
+        logger.warning(
+            "wiki claims for memory %s could not be removed: %s", memory_id, exc
+        )
+        return 0
+
+
 # Telemetry-instrumented public entry. Records latency / byte volume
 # / result count per call (Popper C6 read/write ratio audit).
 handler = instrument("forget", _handler_impl, result_count_key=None)

mcp_server/hooks/post_tool_capture.py

@@ -22,6 +22,7 @@
 from mcp_server.core.gist_extraction import (
     HIGH_VALUE_PATTERNS,
     extract_gist,
+    format_artifact_pointer,
     needs_gist,
 )
 from mcp_server.shared.redaction import scrub_secrets
@@ -195,7 +196,7 @@ def _gist_or_full(output: str) -> tuple[str, str | None]:
     except Exception as exc:  # noqa: BLE001 — hook boundary — failure is logged to the hook log; the hook stays non-fatal
         _log(f"artifact write failed (non-fatal, full output kept): {exc}")
         return output, None
-    pointer = f"**Artifact:** `{path}` ({len(output)} chars full output)"
+    pointer = format_artifact_pointer(str(path), len(output))
     return extract_gist(output), pointer
 
 

mcp_server/infrastructure/artifact_gc.py

@@ -0,0 +1,110 @@
+"""Reference-counted removal of raw-output artifacts.
+
+`artifact_store` writes content-addressed artifacts and never removes them.
+That is safe while nothing deletes memories, but `forget` claims — in
+PRIVACY.md, to users — that it deletes a memory. A gist+pointer memory keeps
+its FULL raw text in the artifact, so deleting only the row leaves the content
+readable on disk (issue #366).
+
+Removal cannot be unconditional. `store_artifact` is content-addressed with
+dedup: two captures of byte-identical output map to ONE file. Deleting that
+file when the first of two referring memories is forgotten would leave the
+second memory's pointer dangling at a path that no longer exists. So this
+module answers "is anyone still referring to this artifact?" before unlinking.
+
+Separate module from `artifact_store` on purpose: that one owns filesystem
+writes and has no database dependency. Garbage collection needs both the store
+and the filesystem, and mixing them into the writer would give it two reasons
+to change (SRP).
+"""
+
+from __future__ import annotations
+
+import logging
+from pathlib import Path
+from typing import TYPE_CHECKING
+
+from mcp_server.infrastructure.row_factory import DICT_ROW
+
+if TYPE_CHECKING:
+    from mcp_server.infrastructure.db_types import StoreConnection
+
+logger = logging.getLogger(__name__)
+
+
+def count_artifact_references(conn: StoreConnection, artifact_path: str) -> int:
+    """Number of memories whose body still points at ``artifact_path``.
+
+    Pre: ``conn`` is a live store connection (PgMemoryStore's psycopg
+    connection or the SQLite `PsycopgCompatConnection`, which translates the
+    ``%s`` placeholder); ``artifact_path`` is a non-empty path string.
+    Post: returns the count of rows in ``memories`` whose ``content`` contains
+    the path. Substring match is the correct test here because the pointer line
+    embeds the path verbatim and is the only place a memory body carries it.
+
+    Uses ``COUNT(*) AS c`` + ``row["c"]`` with ``row_factory=DICT_ROW``, the
+    shared cross-backend scalar convention (cf. count_relationships /
+    count_entities): the SQLite compat cursor yields mapping rows, so reading
+    by position raises KeyError there.
+
+    Raises whatever the driver raises — a failure to COUNT must NOT be read as
+    "zero references", which would delete a still-referenced artifact. The
+    caller decides how to degrade; see ``delete_artifact_if_unreferenced``.
+    """
+    with conn.cursor(row_factory=DICT_ROW) as cur:
+        cur.execute(
+            "SELECT COUNT(*) AS c FROM memories WHERE content LIKE %s",
+            (f"%{artifact_path}%",),
+        )
+        row = cur.fetchone()
+    return int(row["c"]) if row else 0
+
+
+def delete_artifact_if_unreferenced(
+    conn: StoreConnection, artifact_path: str | None
+) -> bool:
+    """Unlink ``artifact_path`` when no memory refers to it any more.
+
+    Pre: the referring memory row has ALREADY been deleted — otherwise it
+    counts itself and the artifact is never collected. ``artifact_path`` may be
+    None (memory had no artifact), in which case this is a no-op.
+    Post: returns True only when the file was actually unlinked. Returns False
+    when there was no artifact, when another memory still refers to it, when
+    the file is already gone, or when the reference count could not be
+    established. Never raises: a failure to collect an artifact must not fail
+    the deletion of the memory the user asked to forget, and every non-removal
+    path logs why.
+    """
+    if not artifact_path:
+        return False
+
+    try:
+        references = count_artifact_references(conn, artifact_path)
+    except Exception as exc:  # noqa: BLE001 — mechanism boundary, see docstring
+        # Fail CLOSED: an unknown reference count must not authorize a delete.
+        logger.warning(
+            "artifact reference count failed for %s — keeping the file: %s",
+            artifact_path,
+            exc,
+        )
+        return False
+
+    if references > 0:
+        logger.info(
+            "artifact %s kept — still referenced by %d memory/memories",
+            artifact_path,
+            references,
+        )
+        return False
+
+    path = Path(artifact_path)
+    try:
+        path.unlink()
+    except FileNotFoundError:
+        logger.info("artifact %s already absent — nothing to remove", artifact_path)
+        return False
+    except OSError as exc:
+        logger.warning("artifact %s could not be removed: %s", artifact_path, exc)
+        return False
+    logger.info("artifact %s removed (last referrer forgotten)", artifact_path)
+    return True