fix(memory): canonicalize memory identifiers symmetrically (#5164)

[M3gA-Mind]

document namespace/key cannot contain personal identifiers reached Sentry 3,055 times from a single user in one day (TAURI-RUST-QWW, openhuman@0.63.0). The literal rejection was already removed by #5171, but that fix left the underlying defect in place — and added a second one. This closes both.

Summary

• Memory identifiers (namespace, document key, KV key) are canonicalized by one strict-gated, idempotent helper instead of being run through the content scrubber.
• Scanner-built identifiers — WhatsApp JIDs, iMessage +1… chat ids, millisecond timestamps, padded counters — keep their identity, so two contacts can no longer collapse onto one (namespace, key) and silently overwrite each other's documents.
• Every path that addresses a row now derives the same address: writes, reads, recall/search, graph, deletes, and the KV shim. A canonicalized write is readable by the identifier the caller passed.
• The rejections that stay deliberate (secret-shaped identifiers, keys that trim to empty) classify as ExpectedErrorKind::MemoryIdentifierRejected, so their retry volume can't flood Sentry again while real failures on the same write path still page.

Problem

The boundary check used the strict predicate has_likely_pii to reject a write. Rejection is deterministic in the caller's own input, so the caller retried, failed identically, and re-reported — 3,055 events, one user, one day. Not 3,055 defects; one defect at retry rate.

#5171 stopped rejecting and started rewriting the namespace/key with redact_pii. Two defects follow.

1. Wrong scrubber. redact_pii is the content scrubber. The crate's own tests spell out why it must not be used on identifiers:

redact_pii (content scrubbing path — NOT the boundary check) … False positives in the content path only blur substring bytes; they do not reject the write — which is the asymmetry this PR preserves vs. the boundary check.

It rewrites bare digit-run shapes that has_likely_pii deliberately tolerates because the scanners build identifiers out of them. So:

12025551234@c.us:2026-05-30  ->  [REDACTED_PII_PHONE]@c.us:2026-05-30
12025559999@c.us:2026-05-30  ->  [REDACTED_PII_PHONE]@c.us:2026-05-30   # same key

Both land on one (namespace, key), where ON CONFLICT(namespace, key) DO UPDATE has one contact's document overwrite the other's. Same for iMessage +1… chat ids, screen_intelligence_…-1747729035001-…, and padded counters.

2. Write-only. Rewriting an identifier changes the row's address, and only the write side was rewriting:

Path	Addressed
upsert_document*	rewritten namespace + key
Memory::get / Memory::forget	raw key, raw namespace (not even sanitize_namespace)
query.rs (recall/search), graph.rs	namespace without the rewrite
KV get_* / delete_* / list_*	raw key

The write lands, the read misses, the caller treats the row as absent and writes again — the same unthrottled loop, now silent instead of erroring. A pre-existing test asserted exactly that miss:

let stored = memory.kv_get_global("ssn-123-45-6789").await.unwrap();
assert!(stored.is_none(), "original PII key should not match after sanitization");

Solution

One helper, strict-gated, idempotent (memory_store/safety/mod.rs):

• canonical_identifier rewrites only what has_likely_pii flags — formatted / keyword-gated national IDs (ssn-123-45-6789, cliente-RFC-VECJ880326XK4, cuit-20-11111111-2). [REDACTED_PII_*] placeholders carry no PII pattern, so the transform is a fixed point on its own output, which is what lets read paths apply it unconditionally.
• canonical_document_key = trim + canonical_identifier, single-sourcing the exact transform the upserts apply to memory_docs.key.

Applied symmetrically:

• The namespace step moves into UnifiedMemory::sanitize_namespace — the one funnel writes, reads, query.rs, graph.rs, deletes and the on-disk namespaces/<ns>/ directory already share. The four hand-rolled redact_pii(namespace) wrappers in documents.rs are removed as redundant.
• The by-key paths — both upserts, Memory::get, Memory::forget, and the kv.rs shim — go through canonical_document_key / canonical_identifier. The KV compensation lives in the host shim (the crate's set_* canonicalizes, its get_* / delete_* do not), so no submodule bump is needed; canonicalizing there is a no-op on the write path and makes the read path symmetric.

Sentry, defence in depth: ExpectedErrorKind::MemoryIdentifierRejected demotes the remaining rejection wordings (document namespace/key cannot contain secrets per #4947, document key cannot be empty, the kv / episodic variants, and the retired PII wording still sent by pre-#5164 cores) to warn. Anchors require the memory-store subject, so upsert memory_docs: database is locked, embedding failures and sidecar IO errors still reach Sentry as errors.

Impact

• Desktop / core (Rust). No RPC or schema change, no migration.
• Behaviour change: a namespace or key containing a formatted national ID is stored canonicalized (as it already was since fix: Sentry triage — resolve 16 actionable issues across tauri-react, tauri-rust, core-rust #5171) and is now retrievable by the original identifier. Scanner-built identifiers are stored verbatim again — a revert of the fix: Sentry triage — resolve 16 actionable issues across tauri-react, tauri-rust, core-rust #5171 regression.
• Known non-action: rows written between fix: Sentry triage — resolve 16 actionable issues across tauri-react, tauri-rust, core-rust #5171 and this fix under lenient-redacted keys become orphans in their namespace; the scanners recreate correct rows on the next sync. Happy to add a cleanup migration if reviewers want it in scope.
• Perf: has_likely_pii now runs per sanitize_namespace call (short strings, candidate-scan fast path that returns early when no class is flagged) — negligible against the SQLite work it precedes.

Submission Checklist

• Tests added or updated (happy path + failure/edge cases) — see below
• Diff coverage ≥ 80% — every changed line is exercised by the new/updated Rust tests (memory_store::safety, namespace_store::init, namespace_store::documents_tests, core::observability)
• N/A: behaviour-only change — coverage matrix unchanged (no feature added/removed/renamed)
• N/A — no matrix feature IDs affected
• No new external network dependencies introduced
• N/A — does not touch release-cut surfaces
• Linked issue closed via Closes #5164

Tests, failing before / passing after:

• pii_like_document_key_round_trips_through_get_and_forget, pii_like_namespace_round_trips_through_get_and_list, metadata_only_write_round_trips_through_pii_like_key — write/read symmetry.
• scanner_built_phone_shaped_keys_stay_distinct_documents, scanner_built_identifiers_are_preserved_verbatim — the collision/overwrite regression.
• canonical_identifier_rewrites_only_strict_pii, canonical_identifier_is_idempotent, canonical_document_key_trims_before_canonicalizing, sanitize_namespace_canonicalizes_pii_and_preserves_scanner_namespaces — the helper contract.
• classifies_memory_identifier_rejections_as_expected and its over-suppression guard does_not_classify_real_memory_write_failures_as_identifier_rejections.
• The three stale KV/document assertions that encoded the read-miss are corrected to assert the round trip.

Related

• Closes: document namespace/key cannot contain personal identifiers — validation error spam #5164
• Context: fix: Sentry triage — resolve 16 actionable issues across tauri-react, tauri-rust, core-rust #5171 (partial fix this completes), Memory Sources: only Gmail appears from Composio (Calendar and Sheets missing) + intermittent 'document namespace/key cannot contain secrets' sync failure #4947 (the sibling secret-identifier rejection), PII check on document namespace/key floods Sentry with 915+ events #2848 / TAURI-RUST-54T (why the boundary predicate is strict)
• Follow-up PR(s)/TODOs: optional cleanup migration for orphaned rows written between fix: Sentry triage — resolve 16 actionable issues across tauri-react, tauri-rust, core-rust #5171 and this PR; the tinycortex-side get_* / delete_* key asymmetry is compensated at the host shim and could be pushed upstream into the crate.

[greptile-apps]

M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e937c942-5a46-42b3-ba90-7d1f191bdb2f

📥 Commits

Reviewing files that changed from the base of the PR and between bb83836 and cabbed5.

📒 Files selected for processing (8)

• src/core/observability.rs
• src/openhuman/memory_store/kv.rs
• src/openhuman/memory_store/memory_trait.rs
• src/openhuman/memory_store/namespace_store/README.md
• src/openhuman/memory_store/namespace_store/documents.rs
• src/openhuman/memory_store/namespace_store/documents_tests.rs
• src/openhuman/memory_store/namespace_store/init.rs
• src/openhuman/memory_store/safety/mod.rs

---

_{Comment @coderabbitai help to get the list of available commands.}