feat(server): associate connections with entities (union with feeds)#1158
Conversation
Add entity_ids to manage_connections create/connect/update (cross-org validated via assertEntityIdsInOrg, tri-state clear on update); list entity_names + entity_id filter become a UNION of the connection's own entity_ids and its feeds' entity_ids; parse entity_ids to number[] so the API returns a typed array. Bumps owletto pointer for the matching UI. Depends on the connections.entity_ids drift migration (#1157) for prod DBs.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds optional multi-entity tagging to connections: accept entity_ids on create/connect/update, validate they belong to the same org, persist them to connections.entity_ids, support tri-state update semantics (omit preserves, null/[] clears, array sets), aggregate entity_names from connection and feed tags on list, and add integration tests; also repins packages/owletto. ChangesEntity ID tagging for connections
Possibly related PRs
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server/src/tools/admin/manage_connections.ts (1)
195-200: 💤 Low valueSchema description mentions
nullbut schema doesn't accept it.The description says "Pass [] (or null) to clear" but
Type.Optional(Type.Array(...))only acceptsundefinedor an array—nullwould fail TypeBox validation. Consider updating the description to just "Pass [] to clear all links; omit to leave unchanged."🤖 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 `@packages/server/src/tools/admin/manage_connections.ts` around lines 195 - 200, The schema description for the entity_ids property is inconsistent: it claims "Pass [] (or null) to clear" but the schema uses Type.Optional(Type.Array(...)) which does not accept null. Update the description text on the entity_ids field to remove "or null" (e.g., "Pass [] to clear all links; omit to leave unchanged") so it matches the Type.Optional(Type.Array(Type.Number())) validation, or alternatively change the schema to accept null by wrapping with Type.Union([Type.Array(...), Type.Null()]) if you intend to allow null; make this change where entity_ids is defined.
🤖 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 `@packages/server/src/tools/admin/manage_connections.ts`:
- Around line 195-200: The schema description for the entity_ids property is
inconsistent: it claims "Pass [] (or null) to clear" but the schema uses
Type.Optional(Type.Array(...)) which does not accept null. Update the
description text on the entity_ids field to remove "or null" (e.g., "Pass [] to
clear all links; omit to leave unchanged") so it matches the
Type.Optional(Type.Array(Type.Number())) validation, or alternatively change the
schema to accept null by wrapping with Type.Union([Type.Array(...),
Type.Null()]) if you intend to allow null; make this change where entity_ids is
defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a1fb9ab-2640-4a12-ba9c-02fb4fb700b7
📒 Files selected for processing (4)
packages/owlettopackages/server/src/__tests__/integration/connections-cross-org-entity.test.tspackages/server/src/sandbox/namespaces/connections.tspackages/server/src/tools/admin/manage_connections.ts
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
What
Server side of connection↔entity association (mirrors the
feeds.entity_idspattern).entity_idsoncreate/connect/updateactions — cross-org validation viaassertEntityIdsInOrg; tri-state clear on update (undefinedleaves unchanged,[]clears), matching feeds.entity_names+ theentity_idfilter become a UNION of the connection's ownentity_idsand its feeds'entity_ids(so existing feed-derived associations keep working and direct tags are added on top).entity_ids→number[](Postgres returned thebigint[]as the literal string'{2}'; the UI picker needs a typed array).connections-cross-org-entity(cross-org reject, in-org set, tri-state clear, union list both ways).Validation
feeds-cross-org-entity) on Node 22 against real pgvector.tsc --noEmit(packages/server): clean.entity_ids: [2]/entity_names: "Acme Corp"and the entity filter through the real HTTP API; toggled a second entity via the UI and confirmed[2,3]persisted + surfaced under both entities. The serialization bug ('{2}'string →[2]) was caught here and fixed.Dependency / merge order
Prod's serving DB (
owletto) is missingconnections.entity_idsdue to squashed-baseline drift — repaired by #1157, which must land (and deploy) before this ships to prod. Merge the owletto web PR first, then update this pointer to the merged SHA.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Tests