ENG-2219 Add an asset copy stage to Roam node materialization

[maparent]

Reviewer brief

This finally ties in the components together, calling importedNodeAssets in materializeSharedNode.ts.
Title collision detection is hoisted above asset creation.

Verification

Some new unit tests, but here is where all the behaviour test videos are landing.

Loom video

Code walkthrough:
https://www.loom.com/share/713b68611c90472b9306f02ad39bc6b5

Show an Obsidian node imported in Roam and Obsidian
https://www.loom.com/share/dfbc2b34f33d46d0827f4b471f6c5183

Show a Roam node imported in Roam and Obsidian
https://www.loom.com/share/40d6571412ac4c92b14a6427e058dc9e

Some more loom videos are needed to show that asset failures do not block the page sharing, those will be added, but the main path is shown to work.

Scope check

• Ran $scope-check against ENG-2219 and the final diff.
• Scope beyond Done When: One addition. The title-collision check was extracted into titleCollisionFailure and is now also made in materializeSharedNode before the asset stage runs, in addition to the checks the two import paths still make. Done When requires no ordering guarantee between the collision check and the copy.
• Required now: Yes. A node rejected for a title collision imports nothing and reads to the user as a clean no-op, but without the reordering it would already have made permanent uploads. That case becomes reachable the moment this stage ships. Undoing an upload is not an option: roamAlphaAPI.file.delete exists and takes a URL, but every user of a graph shares one asset registry keyed by content hash, so deleting a blob this import uploaded can strand another user's import that already resolved to it. Roam also exposes no way to list a graph's files, so an orphan cannot be found afterwards. The addition is ordering only: the failure message and the reported stage are unchanged.
• Anyone affected or consulted: No.
• Decision: Not documented. Proposal in Linear.

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.

https://linear.app/discourse-graphs/issue/ENG-2219/add-an-asset-copy-stage-to-roam-node-materialization

[linear-code]

ENG-2219

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated
discourse-graph	Skipped		Sep 10, 2026 10:07pm UTC

[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 ↗︎.

[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

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 3 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

[devin-ai-integration]

🟡 Encoded aliases create unused uploads

When rows exist for both an encoded token and its decoded spelling, resolvable selects both. rewriteAssetLinks uses only the raw match, leaving the other upload permanently unused.

Learn more

The resolvable Set flattens every lookup candidate, so it cannot preserve rewriteAssetLinks' ordered choice. For markdown containing my%20folder/d.png and reference rows named both my%20folder/d.png and my folder/d.png, importNodeAssets mirrors both rows. rewriteAssetLinks then chooses the raw candidate first and never references the decoded row's upload. Select at most the first matching reference row per parsed token using the same candidate priority as rewriteAssetLinks, while still supporting multiple distinct tokens and deduplicating repeated references.

---

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

[maparent]

The mechanism is right, but the case a real note produces costs nothing.
Two rows in one spelling family do both pass the filter, so mirrorAssetToRoamStorage is called twice. What that costs depends on whether the rows share a content hash.
Same hash is what one file referenced under two spellings gives you. The second call hits the asset registry and comes back reused, so there is one upload and one blob, and handledHashes counts it once. Covered by "counts one upload when two locators name identical bytes" in importNodeAssets.test.ts.
Different hashes would mean two genuinely different files whose paths differ only by percent-encoding, such as a file literally named my%20folder/h.png alongside my folder/h.png. That does leave one upload unreferenced, and it is not a data state we intend to design for.
No change.

[devin-ai-integration]

Devin Review found 1 new potential issue.

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

[devin-ai-integration]

🟡 Failed asset copies never retry

When importNodeAssets reports a transient failure, materializeSharedNode still succeeds and records the source as current. isImportUpToDate skips later normal imports while that timestamp remains unchanged. The asset link stays broken until a forced refresh or source edit.

Learn more

Asset failures live in report.failed, so they do not make the materialization fail. The create and update paths therefore write the current source timestamp before this success result returns. On the next normal import, the freshness guard exits before importNodeAssets runs. The same guard also bypasses the new stage for unchanged pages imported before this feature shipped.

Example: An Obsidian node imports while one image download times out. Its vault link remains unresolved, but the node records the current source timestamp. Retrying the group import after connectivity recovers reports the node as skipped and never retries the image.

Recommended fix: Persist asset-materialization state or a stage version with the imported source identity. Only take the freshness shortcut when both content and assets are current; leave transient asset failures retryable while relying on the asset registry to reuse successful copies.

---

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