ENG-2230 Split filepath and source_path across Obsidian publish and import

[maparent]

Reviewer brief

This touches two things, in separate commits. (Could be split easily if desired.)

1. Allow importing Roam file references correctly, using the source_path as a way to get a proper filename.
2. Publish the Obsidian information in a way that clients like Roam can interpret without deep knowledge of Obsidian file search mechanisms.

• This has a small consequence worth noting: if a node contains the same asset with different paths, we will get multiple FileReference rows. (Scope check point 2)
  While doing that, we discovered and fixed a related bug, see scope check point 3.

Corresponds to these OpenSpec tasks:
https://roamresearch.com/#/app/discourse-graphs/page/fmJkSx36O

Verification

Loom checks:

Importing roam data in Obsidian, collision detection:
https://www.loom.com/share/ae2f845d4b7c48e0a74adfdda12bb5bc

Import legacy asset
https://www.loom.com/share/44cdf4b1ec93428fb3c32cc591b0b057

Loom video

Code walkthrough (long):
https://www.loom.com/share/ba9e6165469747fa8fc876560c60fc96

Scope check

• Ran $scope-check against the ENG ticket and final diff.

• Scope beyond Done When: Three items.

  1. Publish now records one FileReference per link spelling within a single note, where the old keying on the resolved path (attachment.path) collapsed them to one row (syncDgNodesToSupabase.ts:741-757, findEmbeddedAttachments at :693-708). Done When 6 covers spellings across two notes, which per-node keying already gave.
  2. updateMarkdownAssetLinks now rewrites links whose text is a storage URL, consulting pathMapping before the http(s) early return in both the markdown-link and image branches (importNodes.ts:741-751, :774-800).
  3. Two pre-existing importer bugs fixed alongside it: the metadataCache.getFirstLinkpathDest round trip on a just-written file is removed (importNodes.ts:673-678, :798-800), and setPathMapping is hoisted to one call with unsetPathMapping on failure so the reuse branch no longer drops the mapping (importNodes.ts:941-945, :1030-1035, :1125-1127).

• Required now:

1. follows from keying rows on the link, which Done When 5 and 7 require; the same-note case is a side effect of that keying rather than a separate decision, and it is bounded — storage is content-addressed (packages/database/src/lib/files.ts:36-43), and import reuses one vault file per filehash.
2. is required for the co-ship with ENG-1870: source_path ?? filepath satisfies Done When 1 but leaves the note pointing at the storage URL, so the asset would import orphaned.
3. is a correctness fix on the same code path, not required by any criterion on its own. (Actually the manual tests failed until this bug was corrected.)

• Anyone affected or consulted: No
• Decision: Not documented. No

Local delegated full review

• Ran a comprehensive review of the entire final diff in a subagent with a fresh context. Use $dg-delegated-full-review when no other full-review workflow is available.
  Two observations were postponed, written as "Decision taken" in the linear ticket.

https://linear.app/discourse-graphs/issue/ENG-2230/split-filepath-and-source-path-across-obsidian-publish-and-import

[linear-code]

ENG-2230

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
discourse-graph	Ready	Preview	Sep 10, 2026 8:47pm UTC

[graphite-app]

PR size/scope check

This PR is over our review-size guideline.

• Recommended: ~200 lines changed
• Acceptable limit: up to 400 lines when well-scoped/self-contained
• Preferred file count: fewer than 5 files

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:

• What single problem this PR solves
• Why the files/changes are coupled

[mdroidian]

@coderabbitai full review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes.

[coderabbitai]

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: fc13c9b9-979b-4468-b64a-ba9075d892ac

📥 Commits

Reviewing files that changed from the base of the PR and between 9e85cbc and fabd10a.

📒 Files selected for processing (3)

• apps/obsidian/src/utils/importNodes.ts
• apps/obsidian/src/utils/publishNode.ts
• apps/obsidian/src/utils/syncDgNodesToSupabase.ts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[maparent]

https://github.com/coderabbitai full review

[devin-ai-integration]

Devin Review found 2 potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

[devin-ai-integration]

🔴 Full-hash collision overwrites existing asset

When both generated collision paths are occupied, resolveAssetTargetPath returns the full-hash path unchecked. The import then overwrites any file there.

Learn more

The collision resolver checks the candidate and eight-character suffix, but treats the full-hash suffix as available without inspecting the vault or current-run claims. The write path later calls modifyBinary whenever that returned path contains a file, regardless of its hash. A user-created file or stale import can therefore be replaced even though its bytes differ from the incoming asset.

Example: report.png contains hash B and report-1a2b3c4d.png contains hash C. If report-1a2b3c4d…<full hash>.png contains hash D, importing hash A selects that final path and replaces D.

Recommended fix: Check the full-hash path with occupantHash too. If different content occupies it, derive additional stable candidates and probe each before returning, or fail the asset import rather than overwrite.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 Folder collisions abort asset imports

When an asset path names an existing folder, occupantHash treats it as vacant. createBinary then fails, leaving the asset unimported.

Learn more

Obsidian's getAbstractFileByPath returns either a file or a folder. The resolver maps folders to undefined, the same value used for an available path. The later existence check cannot reuse or overwrite a folder, so the code reaches createBinary with an occupied path and catches the resulting failure.

Example: A vault already contains the folder import/team/assets/logo.png/. Importing an asset whose source_path becomes assets/logo.png selects that folder path and fails instead of choosing logo-<hash>.png.

Recommended fix: Distinguish a missing path from a non-file occupant in occupantHash. Treat any TAbstractFile that is not a matching TFile as a collision, then probe the suffixed paths.

---

Was this helpful? React with 👍 or 👎 to provide feedback.