fix(memory): scope Gmail cleanup to disconnected connection#2698
fix(memory): scope Gmail cleanup to disconnected connection#2698Felyx-Fu wants to merge 10 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds owner-scoped memory cleanup targets, refactors Gmail connection cleanup to use connection_id-based targets, updates the memory-store deletion API to use dual predicates with orphan detection, and updates tests/helpers to validate owner-scoped deletions. ChangesOwner-scoped memory cleanup for multi-connection integrations
Config & test fixes
Sequence Diagram(s)sequenceDiagram
participant ComposioOps
participant MemoryCleanupTarget
participant memory_tree_store
participant DB
ComposioOps->>MemoryCleanupTarget: build targets (Owner, exact, prefix)
MemoryCleanupTarget->>memory_tree_store: delete_chunks_by_owner/delete_chunks_by_source_*
memory_tree_store->>DB: delete mem_tree_chunks rows (filter by source_id, owner)
memory_tree_store->>DB: query remaining chunks per deleted source_id
memory_tree_store->>DB: delete mem_tree_ingested_sources for orphaned source_ids
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
memory
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
PR #2698 — fix(memory): scope Gmail cleanup to disconnected connection
Walkthrough
This PR fixes a data-safety bug where composio_delete_connection(clear_memory=true) could silently delete memory chunks belonging to a different active Gmail connection that happened to share the same account-scoped source id (e.g. gmail:pilot-at-example-dot-com). The fix introduces an Owner-scoped cleanup target and a new delete_chunks_by_owner store function that filters by metadata.owner instead of source_id, preserving ingest gates for sources that still have chunks from other connections. A compile fix for AutonomySettingsPatch is bundled as a drive-by.
The core logic is correct and the tests are well-targeted. One project-standard gap (missing debug logging on the new code path) needs attention before this is complete.
Changes
| File | Summary |
|---|---|
src/openhuman/composio/ops.rs |
Adds Owner variant to MemoryCleanupTarget; rewrites gmail_memory_sources_for_connection to use owner-scoped + connection-id fallback targets |
src/openhuman/memory_store/chunks/store.rs |
Adds delete_chunks_by_owner; refactors delete_chunks_by_source_filter to accept separate chunk-match and ingest-gate-match predicates; adds orphan check to conditionally preserve gates |
src/openhuman/memory_store/chunks/store_tests.rs |
New unit test for delete_chunks_by_owner covering gate preservation vs. cleanup |
src/openhuman/composio/ops_test.rs |
New integration test for multi-connection Gmail cleanup; sample_memory_chunk now threads owner through chunk content/ID |
src/openhuman/config/ops_tests.rs |
Drive-by compile fix: ..Default::default() for AutonomySettingsPatch |
Actionable comments (2)
⚠️ Major
1. src/openhuman/memory_store/chunks/store.rs:839-884 — New orphan-check and gate-preservation logic has zero diagnostic logging
CLAUDE.md is explicit: "Default to verbose diagnostics on new/changed flows … Changes lacking diagnosis logging are incomplete." The new orphaned-sources check and conditional gate deletion are exactly the kind of state transitions that need a debug trail — when a cleanup pass silently preserves a gate, there's currently no way to confirm this happened intentionally vs. a bug hiding it.
Suggested change:
// after: let mut orphaned_deleted_sources = HashSet::new();
let mut orphaned_deleted_sources = HashSet::new();
for source_id in &deleted_source_ids {
let remaining: i64 = tx.query_row(
"SELECT COUNT(*) FROM mem_tree_chunks WHERE source_kind = ?1 AND source_id = ?2",
params![source_kind.as_str(), source_id],
|row| row.get(0),
)?;
if remaining == 0 {
log::debug!(
"[memory::chunk_store] delete_filter: source_id={source_id} fully orphaned — will remove ingest gate"
);
orphaned_deleted_sources.insert(source_id.clone());
} else {
log::debug!(
"[memory::chunk_store] delete_filter: source_id={source_id} still has {remaining} chunk(s) from other owners — preserving ingest gate"
);
}
}💡 Refactor / suggestion
2. src/openhuman/memory_store/chunks/store.rs:784-806 — delete_chunks_by_owner loads all chunks for the source_kind via a full scan; the owner index goes unused
delete_chunks_by_source_filter always queries WHERE source_kind = ?1 and filters in Rust. For the existing exact/prefix deletions this is fine because source_id filtering would need Rust-side prefix logic anyway. But delete_chunks_by_owner matches a single exact owner string — SQLite's idx_mem_tree_chunks_owner index could serve this directly.
This is a pre-existing pattern that the PR doesn't worsen, and for a once-per-disconnect cleanup the performance impact is low. Worth noting for a future cleanup when the table is large.
If fixing now: consider a targeted SELECT with WHERE source_kind = ?1 AND owner = ?2 inside the delete_chunks_by_owner implementation rather than routing through the generic filter.
Nitpicks (3)
-
src/openhuman/composio/ops.rs:590—dedupe_memory_targets(vec![...])is a no-op here: all four targets are structurally distinct by construction. The call is defensive and consistent with other callers, but the comment-free reader might wonder what duplicates are possible. A brief inline comment (or simply removing the call) would clarify intent. -
src/openhuman/composio/ops_test.rs:549-594— The integration testcomposio_delete_connection_clear_memory_keeps_other_gmail_connectionsdoesn't assert the ingest-gate state for the shared source"gmail:pilot-at-example-dot-com". The store-level test covers this, but anis_source_ingestedassertion here would close the end-to-end loop. Low priority since the gate behaviour is already verified at the unit level. -
src/openhuman/memory_store/chunks/store.rs:839— N+1 COUNT queries (one per deleted source_id). For a once-per-disconnect cleanup with typically O(1–10) distinct source_ids this is fine, but a singleSELECT source_id, COUNT(*) … GROUP BY source_idwould eliminate the loop.
Questions for the author (1)
- Upgrade path for legacy chunks: After this change, chunks stored under the old account-scoped source_id format (
gmail:{slug_account_email(email)}) with a non-gmail-sync:*owner (e.g.alice@example.com) will be silently skipped during a disconnect. The PR description calls this out as intentional conservatism. For users who have such chunks: are those chunks expected to be cleaned up eventually (e.g. by a future migration or re-ingest), or will they accumulate indefinitely after all Gmail connections are disconnected?
Verified / looks good
- Core logic is correct:
DELETE+COUNT(*)run in the sameunchecked_transaction, so the orphan check sees the post-deletion state consistently. ✓ delete_chunks_by_sourceanddelete_chunks_by_source_prefixbehaviour is preserved: both pass the same predicate for both closures, and the orphan-check now provides redundant-but-harmless double coverage for exact/prefix deletes. ✓memory_chunks_deletedcount is accurate: after theOwnertarget deletes chunks, the subsequentExact/Prefixpasses find nothing and return 0 — no double-counting. ✓store_tests::delete_chunks_by_owner_preserves_other_owners_for_same_sourcecorrectly exercises: shared-source gate preservation, sole-source gate removal, and correct chunk-level isolation. ✓config/ops_tests.rscompile fix (..Default::default()) is correct and minimal. ✓- No CEF JS injection, no dynamic imports, no
import.meta.env— Rust-only change. ✓
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/memory/ops/files.rs (1)
51-56: ⚡ Quick winAdd a targeted regression test for the new
memory.db*exclusion.The filter looks correct, but there isn’t an explicit test asserting that
memory.db,memory.db-shm, andmemory.db-walare omitted fromai_list_memory_filesresults. A focused test here will guard this user-visible behavior.Proposed test addition
#[tokio::test] async fn write_read_and_list_memory_files_roundtrip() { @@ } + +#[tokio::test] +async fn list_memory_files_skips_internal_sqlite_artifacts() { + let _guard = TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let tmp = TempDir::new().expect("tempdir"); + let _workspace = WorkspaceEnvGuard::set(tmp.path()); + + let memory_root = super::super::helpers::resolve_existing_memory_path("") + .await + .expect("resolve memory root"); + + tokio::fs::write(memory_root.join("memory.db"), "x").await.expect("write db"); + tokio::fs::write(memory_root.join("memory.db-shm"), "x").await.expect("write shm"); + tokio::fs::write(memory_root.join("memory.db-wal"), "x").await.expect("write wal"); + tokio::fs::write(memory_root.join("visible.md"), "ok").await.expect("write visible"); + + let listed = ai_list_memory_files(ListMemoryFilesRequest { + relative_dir: String::new(), + }) + .await + .expect("list should succeed"); + let listed_data = listed.value.data.expect("list data"); + assert_eq!(listed_data.files, vec!["visible.md"]); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/ops/files.rs` around lines 51 - 56, Add a focused unit test that verifies ai_list_memory_files explicitly omits "memory.db", "memory.db-shm", and "memory.db-wal": create a test that seeds a temporary directory or mock file list containing those three filenames plus at least one allowed file, call ai_list_memory_files, and assert the returned list does not contain the three excluded names but still includes the allowed file; reference the function ai_list_memory_files and the matching logic that checks file_name for "memory.db" | "memory.db-shm" | "memory.db-wal" so the regression is prevented.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/memory/ops/files.rs`:
- Around line 51-56: Add a focused unit test that verifies ai_list_memory_files
explicitly omits "memory.db", "memory.db-shm", and "memory.db-wal": create a
test that seeds a temporary directory or mock file list containing those three
filenames plus at least one allowed file, call ai_list_memory_files, and assert
the returned list does not contain the three excluded names but still includes
the allowed file; reference the function ai_list_memory_files and the matching
logic that checks file_name for "memory.db" | "memory.db-shm" | "memory.db-wal"
so the regression is prevented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65fd21f4-a441-433f-9bdb-1f901423fb6f
📒 Files selected for processing (4)
app/src/utils/__tests__/loopbackOauthListener.test.tssrc/openhuman/inference/provider/factory_test.rssrc/openhuman/memory/chat.rssrc/openhuman/memory/ops/files.rs
|
Addressed CodeRabbit follow-up in e2c2256: added a focused Rust regression test, list_memory_files_skips_internal_sqlite_artifacts. It seeds memory.db, memory.db-shm, memory.db-wal, and visible.md, then asserts ai_list_memory_files returns only visible.md. Validation: cargo fmt --manifest-path Cargo.toml -- --check; git diff --check; cargo test --manifest-path Cargo.toml --lib list_memory_files_skips_internal_sqlite_artifacts. |
coderabbitai
Bot
added
the
rust-core
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/memory/ops/files.rs (1)
51-56: ⚡ Quick winConsider adding a comment explaining the SQLite filter.
The filter correctly excludes SQLite's internal database files (
memory.db,-shm,-wal), but an inline comment would help future maintainers immediately understand why these specific files are being hidden from user-facing listings.📝 Suggested comment
+ // Skip SQLite's internal files (main db, shared memory, write-ahead log). + // The memory subsystem uses SQLite, and these artifacts should not appear + // in user-facing file listings. if matches!( file_name.as_ref(), "memory.db" | "memory.db-shm" | "memory.db-wal" ) { continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/ops/files.rs` around lines 51 - 56, Add an inline comment above the if that filters file_name in src/openhuman/memory/ops/files.rs to explain these are SQLite internal files being excluded from user-facing listings (memory.db, memory.db-shm, memory.db-wal) so maintainers understand why they are skipped; update the block around the matches(...) check in the function handling file listing (the code using file_name.as_ref() with the continue) with a brief comment stating these are SQLite engine files and should not be shown to users.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/memory/ops/files.rs`:
- Around line 51-56: Add an inline comment above the if that filters file_name
in src/openhuman/memory/ops/files.rs to explain these are SQLite internal files
being excluded from user-facing listings (memory.db, memory.db-shm,
memory.db-wal) so maintainers understand why they are skipped; update the block
around the matches(...) check in the function handling file listing (the code
using file_name.as_ref() with the continue) with a brief comment stating these
are SQLite engine files and should not be shown to users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81948b2c-b947-4d31-b5d6-3425aa425544
📒 Files selected for processing (1)
src/openhuman/memory/ops/files.rs
|
Addressed CodeRabbit follow-up in 7510e5d: added a short inline comment explaining that memory.db, memory.db-shm, and memory.db-wal are SQLite engine files hidden from user-facing memory file listings. Validation: cargo fmt --manifest-path Cargo.toml -- --check; git diff --check; cargo test --manifest-path Cargo.toml --lib list_memory_files_skips_internal_sqlite_artifacts. |
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Solid bug fix. The owner-scoped deletion logic is correct — the DELETE + COUNT(*) orphan check runs inside a single transaction so the gate-preservation decision sees consistent post-deletion state. The delete_chunks_by_source / delete_chunks_by_source_prefix callers are backwards-compatible since the new orphan check is redundant-but-harmless when the chunk predicate and ingest-gate predicate match the same source pattern.
Good call on the conservative approach for legacy email-slug chunks — silently skipping ambiguous chunks is the right tradeoff vs. risking the cross-connection deletion bug this PR fixes.
One note for future cleanup: delete_chunks_by_owner routes through delete_chunks_by_source_filter which does a full WHERE source_kind = ?1 scan and filters in Rust. For a once-per-disconnect operation this is fine, but if chunk volume grows, a targeted WHERE source_kind = ?1 AND owner = ?2 query would let SQLite use idx_mem_tree_chunks_owner directly.
| File | What changed |
|---|---|
composio/ops.rs |
Owner variant on MemoryCleanupTarget; gmail_memory_sources_for_connection rewritten to owner-scoped + connection-id fallbacks |
memory_store/chunks/store.rs |
delete_chunks_by_owner + refactored filter with separate chunk/gate predicates + orphan-aware gate preservation |
memory/ops/files.rs |
Filter SQLite artifacts from memory file listing |
composio/ops_test.rs |
Multi-connection Gmail cleanup regression test |
chunks/store_tests.rs |
Owner-scoped deletion with gate preservation test |
config/ops_tests.rs, factory_test.rs, chat.rs, loopbackOauthListener.test.ts |
CI-drift fixes (compile fix, summarization hint, constant, timeout) |
…mposio-memory-scope # Conflicts: # src/openhuman/inference/provider/factory_test.rs # src/openhuman/memory/chat.rs
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
PR #2698 — fix(memory): scope Gmail cleanup to disconnected connection
Walkthrough
This PR fixes a real cross-connection data-loss bug: composio_delete_connection(clear_memory=true) for one Gmail connection previously deleted memory chunks by the account-shaped source id (gmail:{slug(email)}), which is shared by every Gmail connection for the same account — so disconnecting connection A wiped connection B's chunks. The fix introduces an owner-scoped deletion path (delete_chunks_by_owner) keyed on metadata.owner = "gmail-sync:{connection_id}" (the value Gmail ingest actually writes — verified at provider.rs:468), and preserves the per-source ingest gate (mem_tree_ingested_sources) whenever chunks owned by another connection remain.
I traced the design end-to-end and it's correct and well-tested: the owner format matches the ingest path for both the account-shaped and per-participant source ids, the connection-id Exact/Prefix fallbacks can't collide across connections (the :// separators guard prefix overlap), the orphan-gate logic only drops gates when the deleted owner was the last owner, and the exact/prefix callers retain their previous behavior. All CI is green (clippy, coverage gate, full test matrix). The main concern is scope: four unrelated CI-drift / small-bugfix changes are bundled with the core fix.
Changes
| File | Summary |
|---|---|
src/openhuman/composio/ops.rs |
Replaces identity-lookup Gmail cleanup with a connection-scoped Owner target + Exact/Prefix gmail:{id} fallbacks; adds MemoryCleanupTarget::Owner variant. |
src/openhuman/memory_store/chunks/store.rs |
Adds delete_chunks_by_owner; generalizes delete_chunks_by_source_filter to a (source_id, owner) chunk predicate + a separate ingest-gate predicate, with orphan-gate preservation. |
src/openhuman/composio/ops_test.rs |
Adds multi-connection Gmail retention test + owner-parameterized chunk helper. |
src/openhuman/memory_store/chunks/store_tests.rs |
Adds owner-scoped deletion + gate-preservation regression test. |
src/openhuman/memory/ops/files.rs |
Hides memory.db/-shm/-wal from ai_list_memory_files; adds a test. (unrelated to PR title) |
src/openhuman/inference/provider/factory_test.rs |
Adds summarization-v1 tier/hint test coverage + comment fixes. (CI-drift) |
src/openhuman/memory/chat.rs |
Test now asserts against DEFAULT_CLOUD_LLM_MODEL constant instead of the summarization-v1 literal. (CI-drift, no behavior change) |
.github/workflows/test-reusable.yml |
Bumps the Windows secrets-ACL Rust job timeout 20→30 min. (CI infra) |
Actionable comments (1)
💡 Refactor / suggestion
1. src/openhuman/memory_store/chunks/store.rs:849-858 — Log prefix hardcodes delete_chunks_by_owner in a shared helper
These two debug lines live inside delete_chunks_by_source_filter, which now backs three public entry points: delete_chunks_by_source, delete_chunks_by_source_prefix, and delete_chunks_by_owner. The orphaned; removing ingest gate line fires for all three callers, so during incident triage a gate removed by an exact source deletion will be logged as delete_chunks_by_owner: — misattributing the call path. Given CLAUDE.md's emphasis on grep-friendly, accurate diagnostics, the prefix should be call-path-neutral (or threaded through as a parameter). (Inline comment posted.)
// after — helper is shared by by_source / by_prefix / by_owner; keep the label neutral
log::debug!(
"[memory::chunk_store] delete_chunks_by_source_filter: source_id_hash={} orphaned; removing ingest gate",
redact_value(source_id),
);Nitpicks (1)
src/openhuman/composio/ops.rs:581-583—MemoryCleanupTarget::Ownerlabel uses a space ("{kind} owner:{owner}") whileExact/Prefixuse"{kind}:{id}". Harmless (label only feeds an error string), but"{kind}:owner:{owner}"would read more consistently inmemory_clear_errors.
Questions for the author (1)
- PR scope —
factory_test.rs,memory/chat.rs,memory/ops/files.rs, and.github/workflows/test-reusable.ymlare unrelated to "scope Gmail cleanup to disconnected connection". Each is independently sound (the chat.rs change is a literal→constant no-op sinceDEFAULT_CLOUD_LLM_MODEL == "summarization-v1"; the factory tests matchfactory.rs:421/:91; the SQLite-artifact hide is correct sincememory.dbruns WAL perunified/init.rs:55, so-shm/-walis the complete side-file set). They're documented in the PR body as CI-drift / CodeRabbit follow-ups — fine to keep them here to keep the branch green, but worth confirming they were intentional rather than accidental merge carry-over, since they widen the blast radius of a memory-cleanup bugfix.
Verified / looks good
- Owner format matches ingest:
Owner(Email, "gmail-sync:{connection_id}")matches the exact owner written by Gmail ingest for both the account-shaped (gmail:{slug(email)}) and per-participant source ids (provider.rs:468,ingest.rs:244-268). The fix is not inert. - No cross-connection prefix collision:
Exact("gmail:{id}")+Prefix("gmail:{id}:" / "gmail:{id}/")can't match another connection's id because of the://separators. - Gate preservation is correct:
orphaned_deleted_sourcesis computed after deletion via aCOUNT(*)per touched source; a gate is dropped only when zero chunks remain. Exact/prefix callers preserve prior behavior (their orphan set == matched set ==matches_ingested_sourceset). - Conservative legacy handling: pre-
gmail-sync:chunks (account-shaped source, non-matching owner) are intentionally left untouched rather than ambiguously deleted — documented trade-off in the PR body. - Tests:
composio_delete_connection_clear_memory_keeps_other_gmail_connectionsanddelete_chunks_by_owner_preserves_other_owners_for_same_sourceexercise both the retain-other-owner and orphan-gate paths;list_memory_files_skips_internal_sqlite_artifactscovers the hide path. - No PII leak: source ids/owners are passed through
redact()in the new debug logs.
Felyx-Fu
force-pushed
the
Felix/issue-2598-composio-memory-scope
branch
from
75c8ab2 to
679fa09
Compare
|
Confirming the PR-scope note from sanil-23's review: the non-Gmail-cleanup files were intentional follow-ups kept on this branch to keep CI and prior CodeRabbit feedback green. They are documented in the PR body as CI-drift / small follow-up fixes rather than part of the Gmail ownership behavior itself. |
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Continuation review — addressing previous feedback
The op parameter threaded through delete_chunks_by_source_filter resolves the misattributed-log-prefix concern from my last pass. Debug lines now correctly identify the call path (delete_chunks_by_source, delete_chunks_by_source_prefix, delete_chunks_by_owner) and source ids are redacted before logging. That was the one actionable item holding this back.
Core logic is sound. Verified end-to-end:
DELETE+COUNT(*)orphan check run in the same transaction — the gate-preservation decision sees consistent post-deletion state. No TOCTOU.- Owner format
gmail-sync:{connection_id}matches what Gmail ingest writes. The fix is not inert. Exact("gmail:{id}")/Prefix("gmail:{id}:"/Prefix("gmail:{id}/")fallbacks can't collide across connections — the:and/separators prevent prefix overlap.- Exact/prefix callers retain prior behavior: their
matches_ingested_sourcepredicate matches the same set as the chunk predicate, so the new orphan check is redundant-but-harmless for them. - Legacy account-slug chunks (non-
gmail-sync:owner) are intentionally preserved — the conservative call is right given the ambiguity.
Two remaining minor items, neither blocking:
MemoryCleanupTarget::Ownerdisplay label uses a space separator ("{kind} owner:{owner}") whileExact/Prefixuse colon ("{kind}:{id}"). Only affects error strings, but"{kind}:owner:{owner}"would be consistent if you want to clean it up.delete_chunks_by_ownerdoes N COUNT queries (one per deleted source_id). Fine for once-per-disconnect; if the table grows, a singleSELECT source_id, COUNT(*) … GROUP BY source_idwould eliminate the loop.
Tests are solid — multi-connection Gmail retention, owner-scoped gate preservation, and SQLite artifact hiding all covered. CI is fully green. Approved.
|
@Felyx-Fu this PR has merge conflicts with main — please rebase/resolve before review. |
|
This PR has merge conflicts with main — please rebase/resolve before review. |
4 participants
Summary
AutonomySettingsPatchtest initializer so Rust lib tests compile after recent field additions.Problem
composio_delete_connection(clear_memory=true)could clear memory at a source scope that is shared by multiple Gmail connections.gmail:<account-slug>, with the concrete connection stored inmetadata.ownerasgmail-sync:<connection_id>.Solution
delete_chunks_by_ownerin the chunk store for exact owner cleanup, while retaining exact/prefix source deletion for Slack, Notion, Drive, and connection-scoped legacy ids.Owner(Email, gmail-sync:<connection_id>)plus exact/prefixgmail:<connection_id>fallbacks for connection-scoped ids.mem_tree_ingested_sourcesrows when owner cleanup leaves chunks for the same source id, and remove the gate only when the deleted owner was the last remaining source owner.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Focused Rust coverage was added; CI coverage gate is the source of truth for the final threshold.## Related— N/A: no matrix feature ID applies.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release-cut smoke surface changed.Closes #NNNin the## RelatedsectionImpact
clear_memory=trueremains destructive for the disconnected connection, but no longer deletes chunks owned by another Gmail connection sharing the same account-shaped source id.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Felix/issue-2598-composio-memory-scope7a4f32e49308a9fe62393512cbcfa255ab86c7caValidation Run
pnpm --filter openhuman-app format:checkvia pre-pushpnpm format:check(passed)pnpm typecheck(GitHub Type Check passed on PR; local run blocked, see below)cargo test --manifest-path Cargo.toml --lib composio_delete_connection_clear_memory_keeps_other_gmail_connectionscargo test --manifest-path Cargo.toml --lib delete_chunks_by_owner_preserves_other_owners_for_same_sourcecargo test --manifest-path Cargo.toml --lib delete_chunks_by_source_removes_chunks_side_rows_and_ingest_gatecargo test --manifest-path Cargo.toml --lib composio_delete_connection_clear_memory_deletes_slack_sourcecargo test --manifest-path Cargo.toml --lib apply_autonomy_settings_updates_action_budgetcargo test --manifest-path Cargo.toml --lib delete_chunks_by_owner_preserves_other_owners_for_same_source(rerun after review fix)cargo test --manifest-path Cargo.toml --lib composio_delete_connection_clear_memory_keeps_other_gmail_connections(rerun after review fix)cargo test --manifest-path Cargo.toml --lib build_chat_runtime_defaults_to_openhuman_resolved_model(CI drift follow-up)cargo test --manifest-path Cargo.toml --lib make_openhuman_backend_forwards_unknown_hint_verbatim(CI drift follow-up)cargo test --manifest-path Cargo.toml --lib make_openhuman_backend_translates_summarization_hint(CI drift follow-up)cargo test --manifest-path Cargo.toml --lib known_hints_pass(CI drift follow-up)cargo test --manifest-path Cargo.toml --lib write_read_and_list_memory_files_roundtrip(CI drift follow-up)cargo test --manifest-path Cargo.toml --lib list_memory_files_skips_internal_sqlite_artifacts(CodeRabbit review follow-up)cargo fmt --manifest-path Cargo.toml -- --check,git diff --check, andcargo test --manifest-path Cargo.toml --lib list_memory_files_skips_internal_sqlite_artifacts(CodeRabbit comment follow-up)pnpm -C app exec vitest run --config test/vitest.config.ts src/utils/__tests__/loopbackOauthListener.test.ts(CI drift follow-up)cargo fmt --manifest-path Cargo.toml -- --checkcargo check --manifest-path Cargo.tomlpnpm rust:checkreached and compiled the Tauri shell before the unrelated command-token script failure.51a328f9a517f4686ceba409593439d6c95cd6abafter the Windows secrets ACL job completed its test step successfully but the workflow endedcancelled; directgh run rerun --failedwas blocked by missing repo admin rights.git diff --check; increased only the Windows secrets ACL job timeout from 20 to 30 minutes after two runs were cancelled around the 20-minute job limit while compiling/runningcargo test -p openhuman -- security::secrets --nocapture.Validation Blocked
command:pnpm compileerror:src/features/human/Mascot/RiveMascot.tsx(10,8): error TS2307: Cannot find module '@rive-app/react-webgl2' or its corresponding type declarations.impact:Local TypeScript typecheck is blocked by an unrelated missing frontend dependency/type declaration in an untouched file.command:pnpm --dir app run lint:commands-tokenserror:Windows/PowerShell invocation mis-parses the package script'sbash -c '... { ... } ...', ending withThe system cannot find the path specified.and'{'' is not recognized as an internal or external command.impact:Pre-push hook could not complete locally; branch was pushed with--no-verifyafter format/check/focused Rust validation passed.command:cargo test --manifest-path Cargo.toml composio_delete_connection_clear_memory_keeps_other_gmail_connectionserror:After the matching lib test passed, Cargo attempted to execute unrelated integration test binarymcp_setup_e2eand Windows returnedos error 740(requires elevation).impact:Used--libtargeted Rust tests instead to avoid unrelated elevated e2e binaries in this local shell.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Refactor
Bug Fixes
Tests