Skip to content

ENG-2229 Add a source_path column to FileReference - #1390

Open
maparent wants to merge 1 commit into
mainfrom
eng-2229-add-a-filename-column-to-filereference
Open

ENG-2229 Add a source_path column to FileReference#1390
maparent wants to merge 1 commit into
mainfrom
eng-2229-add-a-filename-column-to-filereference

Conversation

@maparent

@maparent maparent commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Reviewer brief

As defined in linear.
Renamed filename to source_path, since it did not land, and it's closer to the conceptual name.

Verification

Claude generated tests against a mock database; may or may not be valuable.
I don't think this small change warrants an integration test.

Loom video

https://www.loom.com/share/312cdf5d3bd74e1497ee82f9a5c3e673

Scope check

  • Ran $scope-check against the ENG ticket and final diff.
  • Scope beyond Done When: None

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.

What I did

  • Diff under review: main...HEAD (1 commit, 5de67e6 ENG-2229 Add a filename column to FileReference); working tree is clean for packages/database, so nothing extra was in scope.
  • Read packages/database/src/lib/files.ts, supabase/schemas/assets.sql, all prior my_file_references definitions in supabase/migrations/, and both Obsidian consumers (apps/obsidian/src/utils/importNodes.ts, syncDgNodesToSupabase.ts).
  • Ran npx vitest run src/lib/tests/files.test.ts (5/5 pass), npx tsc --noEmit --skipLibCheck (clean), npx tsx scripts/lintSchemas.ts (only pre-existing sqruff noise).
  • Verified the view change against the previous definition: filename is appended last, so CREATE OR REPLACE VIEW is legal and grants/options are preserved. Correct.
  • Queried the running local Postgres and regenerated types to check dbTypes.ts fidelity.

Findings

packages/database/supabase/schemas/assets.sql:5 — low, forward-looking. filename is an unconstrained character varying written by any editor in a space and read by importers in other spaces, and its comment explicitly invites destinations to "name an imported asset" with it. importNodes.ts writes downloaded assets into the vault at paths it builds itself; the first consumer that uses filename for that path inherits a traversal/overwrite hazard from a value that crosses a trust boundary unchecked. No consumer exists in this diff, so nothing misbehaves today — worth a CHECK rejecting path separators, or a note on the ticket that sanitising is the reader's job.

My opinion: that's barely worth a comment.

Everything else in the diff checks out: the view replacement is order-safe, RLS/policies/GC triggers are untouched, the timestamp orders correctly after 20260819023727, ADD COLUMN IF NOT EXISTS is idempotent, the comment is duplicated into both schema and migration per project convention, and both Obsidian call sites still compile and behave identically (select("*") on the view just gains a field).

https://linear.app/discourse-graphs/issue/ENG-2229/add-a-filename-column-to-filereference

@linear-code

linear-code Bot commented Sep 3, 2026

Copy link
Copy Markdown

ENG-2229

@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown

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

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

Request Review

@supabase

supabase Bot commented Sep 3, 2026

Copy link
Copy Markdown

Updates to Preview Branch (eng-2229-add-a-filename-column-to-filereference) ↗︎

Deployments Status Updated
Database Sun, 06 Sep 2026 22:32:58 UTC
Services Sun, 06 Sep 2026 22:32:58 UTC
APIs Sun, 06 Sep 2026 22:32:58 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Sun, 06 Sep 2026 22:32:58 UTC
Migrations Sun, 06 Sep 2026 22:32:58 UTC
Seeding Sun, 06 Sep 2026 22:32:58 UTC
Edge Functions Sun, 06 Sep 2026 22:33:01 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@maparent
maparent force-pushed the eng-2229-add-a-filename-column-to-filereference branch from 5de67e6 to f86f5ee Compare September 3, 2026 13:20
@maparent
maparent force-pushed the eng-2229-add-a-filename-column-to-filereference branch from f86f5ee to d76bd3f Compare September 3, 2026 13:53
@maparent
maparent requested a review from mdroidian September 3, 2026 13:53
@maparent
maparent requested review from mdroidian and removed request for mdroidian September 3, 2026 17:40
@maparent
maparent force-pushed the eng-2229-add-a-filename-column-to-filereference branch from d76bd3f to 20f5362 Compare September 4, 2026 18:22
@maparent maparent changed the title ENG-2229 Add a filename column to FileReference ENG-2229 Add a source_path column to FileReference Sep 4, 2026
@maparent
maparent requested a review from mdroidian September 4, 2026 18:29
@maparent
maparent force-pushed the eng-2229-add-a-filename-column-to-filereference branch from 20f5362 to a34e4cc Compare September 6, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant