fix(core): scope episode KG cleanup by namespace

[wshobson]

Summary

• scope SQLite episode cleanup of knowledge-graph passage associations through namespace-owned entities
• add a manually seeded SQLite regression gate for the affected association state
• add supported storage-API parity tests for SQLite and live Postgres with enforced RLS

Measured reachability

The supported storage path was tested with one episode UUID reused across two namespaces. On both SQLite and live Postgres with enforced RLS, deleting observations in namespace A removed only A's observation and preserved namespace B's observation. The cross-namespace effect reproduced only for manually seeded SQLite knowledge-graph associations, so the supported-path security stop did not trigger.

Verification

• RED: the new SQLite knowledge-graph regression failed before the fix because namespace B's association count became 0 instead of remaining 2
• GREEN: all six focused episode-delete tests passed after the fix
• mutation: removing only the new entity-namespace predicate reproduced the same intended regression; restoring it returned the focused tests to green
• cargo test -p pensyve-core
• PENSYVE_TEST_DATABASE_URL=<isolated-test-db> cargo test -p pensyve-core --features postgres
• cargo clippy -p pensyve-core --all-targets --features postgres -- -D warnings
• cargo fmt --all -- --check
• git diff --check origin/main...HEAD

The repo-wide make check additionally reaches an existing unknown-Clippy-lint error in pensyve-mcp-tools/src/server.rs, outside this PR's allowed scope. The changed crate's stricter all-targets Postgres-feature Clippy command above is clean.

Addresses #281. This intentionally avoids an auto-close keyword because the issue's deployment and production-smoke gates remain after merge.

[coderabbitai]

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

• 🔍 Trigger review

---

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

[claude]

Claude finished @wshobson's task in 3m 15s —— View job

---

Claude PR Review

• Read the PR diff (sqlite.rs, live_rls.rs, test_namespace_scoping.rs)
• Analyze the SQLite cleanup fix for correctness & security
• Review the new tests for coverage & correctness
• Check cross-language / Postgres parity concerns
• Post review feedback

The core fix — adding AND entity_id IN (SELECT id FROM kg_entities WHERE namespace_id = ?2) to the kg_passage_entities delete — is correct, and it now matches the entity-namespace join used by the two single-memory delete paths. The Postgres/SQLite parity tests are good additions.

One finding (defense-in-depth completeness), left as an inline comment:

The fix is incomplete. In the same delete_observations_by_episode, the sibling kg_triples delete (sqlite.rs:1665–1670) has the identical cross-namespace bleed you're fixing here — and it's easier to hit because kg_triples has its own namespace_id column:

• kg_triples is scoped only through the passage_id sub-select, never by kg_triples.namespace_id = ?2. A ns-B triple whose passage_id points at ns-A's observation (the same "manually seeded" state this PR targets) is deleted by an A-scoped cleanup.
• Both other KG delete sites already guard this — delete_memory_by_id_with_namespace (sqlite.rs:801) and the capturing delete (sqlite.rs:2961) use WHERE passage_id = ?1 AND namespace_id = ?2. This function is the odd one out.
• The new regression test seeds a kg_triples row in ns-B (seed_kg_triple(&db, ns_b.id, observation.id, …)) but only asserts on kg_passage_entities counts, so the kg_triples bleed passes silently. A kg_triples-count-in-namespace assertion for ns-B would gate the full fix.

Suggested change to the kg_triples delete:

"DELETE FROM kg_triples \
 WHERE namespace_id = ?2 \
   AND passage_id IN (SELECT id FROM observation_memories \
                       WHERE episode_id = ?1 AND namespace_id = ?2)"

Fix this →

Other notes

• Supported-path partitioning (observations keyed by a shared episode UUID) already worked on both backends, as the PR states — the new SQLite/Postgres parity tests confirm and lock that behavior. 👍
• Transaction handling (BEGIN/COMMIT/ROLLBACK) is unchanged and correct; no unwrap/panic paths introduced.
• No SQL-injection or cross-language boundary concerns in this diff — all queries are parameterized and the change is storage-internal.

Not blocking, but I'd close the kg_triples gap in this same PR since it's the direct sibling of the exact bug being fixed and the test scaffolding is already in place.
· fix/issue-281-kg-cleanup

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f15cb74115

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @wshobson's task in 1m 43s —— View job

---

Claude PR Review

• Read the updated diff (new commit 829aad4 "confine episode KG triple cleanup")
• Verify the kg_triples cross-namespace bleed is now fixed
• Confirm the regression test now gates the kg_triples scope
• Check Postgres/SQLite parity is preserved
• Post review feedback

The new commit 829aad4 closes the gap flagged in the prior round. The fix is now complete and correct — LGTM.

What changed since the last review

• delete_observations_by_episode kg_triples delete (sqlite.rs:1666) now carries WHERE namespace_id = ?2 AND passage_id IN (…). A ns-B triple whose passage_id points at ns-A's observation is no longer swept up by an A-scoped cleanup. This matches the sibling delete sites at sqlite.rs:801 and sqlite.rs:2962 (WHERE passage_id = … AND namespace_id = ?2), so all four KG delete paths are now consistent.
• The kg_passage_entities delete (sqlite.rs:1673) keeps its entity_id IN (SELECT id FROM kg_entities WHERE namespace_id = ?2) guard — the right scoping for that table, which has no namespace_id column of its own and matches sites sqlite.rs:805 / sqlite.rs:2966.
• The regression test now gates both siblings: it seeds a ns-B kg_triple (asserts count 1 before the delete) and asserts kg_triples_count_in_namespace(ns_b) == 1 after the A-scoped delete, alongside the existing kg_passage_entities ns-B assertion. The pre-delete assertions correctly guard against a false green.

Correctness sweep of the remaining deletes in this function (all still sound):

• memory_fts delete is keyed by memory_id IN (observations in ns-A for episode) — observation IDs are unique and the sub-select is namespace-scoped, so no cross-namespace bleed.
• observation_memories delete is scoped by episode_id + namespace_id.
• Transaction framing (BEGIN/COMMIT/ROLLBACK) unchanged; no new unwrap/panic paths; all queries parameterized (no injection surface).

Tests

• The SQLite + live-Postgres-RLS parity tests lock the supported-path partitioning of a shared episode UUID across two namespaces. As the PR notes, that path already worked on both backends — the KG bleed only reproduced via the manually-seeded state, which the new SQLite unit test now covers directly. Good coverage.

Note: I could not run cargo test / clippy in this review environment (the command needs approval that isn't available here), so GREEN is asserted from the PR's verification log and code analysis, not a fresh local run. The change is small, self-contained, and the query edits are straightforward, so I'm confident in the analysis.

Nothing blocking — this resolves the P1 raised by both the earlier review and Codex.
· fix/issue-281-kg-cleanup

[wshobson]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 829aad4f99

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".