[ENG-1855] Add Roam shared-node import discovery#1162
Conversation
Read-only Roam command (DG: Import - Discover shared nodes) listing group-visible shared nodes from other spaces — source app, space, title, modified time — via the existing my_contents/my_spaces RLS views and the ENG-1847 cross-app contract. Already-imported marking flows through an empty getImportedSourceRids() seam (ENG-1856 fills it; ENG-1859 wires the join). Gated behind a new "Cross-app node import enabled" feature flag, off by default. Pure assembly logic covered by unit tests.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const dbTimestampToIso = (value: string | null): string | null => { | ||
| if (!value) return null; | ||
| const ms = new Date(`${value}Z`).valueOf(); | ||
| return Number.isNaN(ms) ? null : new Date(ms).toISOString(); | ||
| }; |
There was a problem hiding this comment.
🚩 Timestamp conversion assumes database returns timezone-naive strings
dbTimestampToIso (apps/roam/src/utils/discoverSharedNodes.ts:44) unconditionally appends Z to the raw last_modified value before parsing. If the Supabase view my_contents returns timestamps that already include timezone info (e.g. 2026-06-12T10:00:00+00:00 or 2026-06-12T10:00:00Z), the appended Z would create an invalid date string like 2026-06-12T10:00:00ZZ, causing NaN, which falls back to null and ultimately to epoch-zero (displayed as "Unknown"). The tests all use bare ISO strings without timezone suffixes, so they pass. This is fine if the underlying column is timestamp (without timezone), but would silently discard all modification dates if the column is timestamptz and the client serializes with timezone info.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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: 797e34e7f0
ℹ️ 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".
|
|
||
| const nodes: DiscoveredSharedNode[] = []; | ||
| for (const { spaceId, sourceNodeId, rows } of byNode.values()) { | ||
| if (!rows.some((row) => row.variant === "full")) continue; |
There was a problem hiding this comment.
This filter makes Roam-origin nodes undiscoverable with the current Roam producer: the only call sites for upsertNodesToSupabaseAsContentWithEmbeddings pass no options (for example apps/roam/src/utils/syncDgNodesToSupabase.ts:927 and apps/roam/src/components/settings/DiscourseNodeSuggestiveRules.tsx:59), so includeFullContent stays false and no variant === "full" rows are created. In that setup, a user in one Roam graph will see zero shared nodes from another Roam graph even after sync; either enable publishing full content for shared Roam nodes or avoid using full as the only discoverability marker for Roam rows.
Useful? React with 👍 / 👎.
| const title = (direct?.text ?? full?.text ?? "").trim(); | ||
| if (!title) continue; | ||
|
|
||
| const sourceNodeRid = spaceUriAndLocalIdToRid(meta.url, sourceNodeId); |
There was a problem hiding this comment.
When the source space is Obsidian, this creates orn:obsidian:<vault>/<id> without the note subtype, but the cross-app contract and Obsidian importer persist imported notes with spaceUriAndLocalIdToRid(..., "note") (packages/database/src/crossAppNodeContract.example.ts:53 and apps/obsidian/src/utils/importNodes.ts:1265). Any future importedRids read from Obsidian imports therefore will not match discovered Obsidian nodes, so already-imported notes are shown as new and can be duplicated.
Useful? React with 👍 / 👎.
| const direct = rows.find((row) => row.variant === "direct"); | ||
| const full = rows.find((row) => row.variant === "full"); | ||
| const title = (direct?.text ?? full?.text ?? "").trim(); |
There was a problem hiding this comment.
For Roam block-backed discourse nodes, convertRoamNodeToLocalContent writes variant: "direct_and_description" whenever node.node_title exists (apps/roam/src/utils/upsertNodesAsContentWithEmbeddings.ts:23), while the full row contains the full markdown document. Since discovery only checks for direct, those shared nodes fall back to the full markdown as title, causing the dialog and later import UI to treat the entire document body as the node title.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a read-only Roam command DG: Import - Discover shared nodes that lists group-visible shared nodes from other spaces — source app, source space, title, and upstream modified time — so a Roam user can see what's available to import from Obsidian/Roam. Discovery only; the import action itself is ENG-1859.
Part of the Roam↔Obsidian push-pull project, Milestone 2 (Bidirectional node import). Extends the existing Obsidian↔Obsidian discovery reference rather than building a new path.
What it does
discoverSharedNodesreads the RLS-scopedmy_contentsview, excludes the current space, groups by(space_id, source_local_id), prefers thedirectvariant for the title, and resolves space metadata viamy_spaces.CrossAppNodecontract (sourceApp,sourceSpaceId/Name,sourceNodeId,sourceNodeRid,title,sourceModifiedAt,alreadyImported).Design notes (the why)
fullvariant (the ENG-1848 shared/published marker) so the list shows genuinely importable nodes, not every synced title — a deliberate scoping choice (diverges slightly from Obsidian's list-all discovery).alreadyImportedflows through an emptygetImportedSourceRids()seam. Roam has no imported-node store yet (that's ENG-1856), so nothing is imported and nothing is marked; ENG-1859 wires this to ENG-1856's reader. Keys on source RID.my_*views + own-space exclusion, not app-side access logic.getPublishedNodesForGroups/getMyGroups(app-specific client + return shape); not promoted to a shared package yet (future-extraction candidate).Cross-app node import enabledfeature flag (off by default; AdminPanel toggle "Cross-app node import"), mirroringAdvanced node search enabled— the feature is incomplete (no import until ENG-1859), so it stays hidden from users by default. Later M2 commands reuse the same flag.Testing
assembleDiscoveredNodes(variant merge, full-variant filter, RID round-trip, already-imported marking, sorting).tsxharness (all cases pass). eslint / prettier / tsc green.Linear: ENG-1855