Repository navigation
feat(import): skip connector-synced content in the legacy import - #246
Conversation
The workspace import logic was reorganised into a sections module with separate files for graph, memory docs, profile, and connector handling, plus a dedicated schema module. This keeps each section's parsing and validation self-contained and makes the connector tests easier to locate. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
A new `LegacyWorkspace::skip_connector_syncs(bool)` option, off by default, leaves out everything v1 synced from outside services through Composio and the older connector path, since those connectors re-sync on their own. The skipped rows yield no item while the checkpoint still advances over them, and counts exclude exactly the same rows, so resumption stays exact. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 2 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Before mergeNone. Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughLegacy imports now support opt-in filtering of connector-synced documents, chunks, profile facets, and namespaced graph rows. The same filters apply to item counts. Filtering defaults to off. ChangesConnector-sync import filtering
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to This adds an opt-in import filter that is off by default, so existing behavior is unchanged. No merge-blocking risk was identified in the supplied context. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The option narrows imported content and leaves default behavior unchanged. Changing the setting during a resumed migration can produce inconsistent exclusions, and the calling application's rollout behavior remains unverified. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
A rabbit checks the import queue Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 439cce2a7c
ℹ️ 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".
|
|
||
| /// Whether a `memory_docs` logical namespace is a connector sync. | ||
| pub(crate) fn is_connector_namespace(logical: &str) -> bool { | ||
| logical.starts_with("skill-") || logical.starts_with("source:") |
There was a problem hiding this comment.
When skip_connector_syncs(true) is enabled for a workspace containing folder, file, RSS, web, or GitHub-repository sources, this predicate drops their memory_docs too. The legacy MemorySourceSink::accept_source_items used source:{source_id} for every source kind, not only Composio, so the namespace prefix alone cannot identify connector content; inspect the persisted source kind/metadata before skipping these rows or the migration silently omits user source documents.
Useful? React with 👍 / 👎.
| OR substr(COALESCE(namespace, ''), 1, 7) = 'source:' \ | ||
| OR substr(COALESCE(namespace, ''), 1, 7) = 'source_')"; |
There was a problem hiding this comment.
When the option is enabled after a non-Composio source was ingested, this filter also removes that source's graph relations. The legacy ingestion path extracted graphs under the same sanitized namespace as its document, so generic source:{source_id} namespaces appear here as source_<source_id> for folders, files, RSS, web pages, and repository sources as well as connectors; blanket-filtering both prefixes contradicts the promise that those sources remain migrated.
Useful? React with 👍 / 👎.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0400 · 411,579 in / 22,455 out · 71,888 cached (17%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0218 · 208,926 in / 11,618 out · 51,806 cached (25%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0177 · 154,636 in / 6,646 out · 20,082 cached (13%) · gpt-5.6-luna
tests: $0.0001 · 11,737 in / 80 out · 0 cached (0%) · glm-5.3-flash
description: $0.0001 · 11,842 in / 154 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0001 · 12,827 in / 809 out · 0 cached (0%) · glm-5.3-flash
|
|
||
| /// SQL condition true for a `user_profile` row that is NOT a Composio | ||
| /// identity facet (`facet_id` starting `skill-`). | ||
| pub(crate) const PROFILE_KEPT: &str = "substr(facet_id, 1, 6) != 'skill-'"; |
There was a problem hiding this comment.
Keep profiles whose facet ID is NULL
In SQL, substr(NULL, 1, 6) != 'skill-' evaluates to NULL, not TRUE, so a WHERE clause using this condition drops every user_profile row with a NULL facet_id even though it is not a Composio identity facet. I could not verify the schema nullability from the supplied context; if facet_id is nullable, this loses profiles during import. Use COALESCE (or an explicit IS NULL branch) so only IDs beginning with skill- are excluded.
| pub(crate) const PROFILE_KEPT: &str = "substr(facet_id, 1, 6) != 'skill-'"; | |
| pub(crate) const PROFILE_KEPT: &str = "substr(COALESCE(facet_id, ''), 1, 6) != 'skill-'"; |
[RULE] null-filtering ·
| /// / `source_*` graph namespaces. [`Self::counts`] excludes the same rows. | ||
| /// Off by default. See the module README for the exact rules. | ||
| #[must_use] | ||
| pub fn skip_connector_syncs(mut self, skip: bool) -> Self { |
There was a problem hiding this comment.
No end-to-end test drives the skip_connector_syncs import behaviour
skip_connector_syncs is a new public flag on LegacyWorkspace that changes what a legacy import yields across documents, chunks, profile and graph sections. No end-to-end harness reaches it: the repository's e2e harness (integration/cortexdb) runs the cortexdb service with flag environments and a mock inference server, and never opens a legacy v1 workspace or runs an import; the candidate coverage lines above are lexical matches on unrelated words (object, document, items()). An end-to-end test would have to boot the host with a real v1 workspace containing connector-synced rows, run the import with the flag set, and observe the resulting store contents/counts — none does. Coverage currently rests entirely on crates/tinymemory-integrations/tests/legacy_import.rs, which is an integration test of the library API, not the e2e lane.
[RULE] e2e-uncovered ·
tinysweeper
Bot
added
the
priority: p2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
Adds
LegacyWorkspace::skip_connector_syncs(bool), an opt-in that leaves content synced from Composio connectors (Gmail, Slack, Notion, Linear, GitHub, ClickUp) out of the v1 import. Everything else still migrates: conversations, folder and file memory-source documents, learnings,global, profile, events, lessons, graph and the goals/persona files. The default is off, so current behavior is unchanged.OpenHuman no longer syncs connectors into memory (#245) and should not carry a user's old connector data into the new engine.
Related issue
None. Follow-up to #245; the OpenHuman host PR turns the option on.
API or behavior changes
New builder method
LegacyWorkspace::skip_connector_syncs(self, skip: bool) -> Self. No change unless a caller turns it on. With it on, bothcounts()anditems()skip the same rows, so counts still equal whatitems()yields, and checkpoints still advance over skipped rows. The rules live insections/connector.rs, and every one comes from how v1 actually wrote connector data:memory_docs)skill-(v1 SkillDoc Composio sync) orsource:(the v1 connector path, stored sanitised assource_…)source_kind = 'email'; orsource_idstarts withgmail:,slack:,notion:,linear:,github:orclickup:; or, whenownerexists, any chunk'sowner LIKE '%-sync:%'({toolkit}-sync:{conn})facet_idstarts withskill-(connector identity facets)graph_namespacerow whose namespace starts withskill-,source:orsource_(graph_globalis untouched)Taint is deliberately not used. v1 also marked the agent's own
globalnotes and flow memory asexternal_sync, so filtering on taint would drop user data. Folder and file sources (mem_src:*), agent chat (conversations:agent), meetings and vault files are kept.Validation
cargo fmt --all -- --check: passcargo clippy --workspace --all-targets --all-features -- -D warnings: passcargo build --all-targets --all-features: covered by the test buildcargo test --workspace --all-features: 1259 passed, 0 failedRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features: passTests
tests/legacy_import.rs:connector_syncs_are_imported_unless_skippedskip_connector_syncs_drops_exactly_the_connector_rows: every rule drops its row, the kept counterparts stay (aglobalrow withexternal_synctaint, a normal facet,graph_global,mem_src:,conversations:agent), andcounts() == items().count()skipping_connector_syncs_keeps_resumption_exact: page size 1, resumed from every checkpointthe_owner_rule_needs_the_owner_columnsections/connector_tests.rs.Documentation
src/import/README.mdgets a new "Connector syncs" section. It also corrects the old claim thatchatchunks are only host-channel transcripts: Composio Slack wrotechatchunks underslack:.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the descriptionSummary by CodeRabbit