Skip to content

fix: preserve attachment files across startup and concurrent saves - #10

Open
KyleAMathews wants to merge 2 commits into
powersync-ja:feat/powersync-attachmentsfrom
TanStack:codex/powersync-attachment-review-fixes
Open

fix: preserve attachment files across startup and concurrent saves#10
KyleAMathews wants to merge 2 commits into
powersync-ja:feat/powersync-attachmentsfrom
TanStack:codex/powersync-attachment-review-fixes

Conversation

@KyleAMathews

Copy link
Copy Markdown

Summary

Fix attachment saves/deletes before their collection has loaded, and protect files from overlapping same-ID saves. Add a real SQLite/file/SDK intent oracle.

This is a companion to TanStack/db#1616, targeting feat/powersync-attachments, not main. It includes the startup fix's intent from Tanner's 3572ec74, adapted to support on-demand collections too.

Approach

  • Acquire a temporary live query for the attachment ID before opening a mutation; retain it through transaction confirmation, then clean up. No preload inside mutationFn and no eager-only restriction.
  • Check SQLite as well as memory before writing a duplicate file.
  • Reject concurrent same-ID saves across queues sharing a PowerSync database object. Release that reservation after success or compensation; distinct IDs remain independent.
  • Preserve the SDK's filename/local-path convention, including storage-root relocation. Partial file writes get best-effort cleanup.
  • Correct docs: attachment metadata and related rows are atomic; filesystem and remote I/O are not.

The in-process reservation is not a cross-handle/tab/process lock. No SDK state machine or completion workaround is added.

Still blocked upstream

SDK upload completion can overwrite a newer QUEUED_DELETE on both success and failure/retry. A detached owner can leave remote bytes behind or cause an obsolete upload to retry. This companion does not make that overlap safe or clear the original PR's SDK merge blocker.

The same independent intent model retains those desired-contract assertions in an opt-in repro suite. Native SDK controls use no TanStack collection or subclass. Normal CI excludes this named upstream overlap; it does not hide it behind expected-failure assertions.

Verification

From packages/powersync-db-collection:

pnpm exec vitest run --coverage.enabled=false --maxWorkers=2
pnpm exec tsc --noEmit -p tsconfig.json
pnpm run build
pnpm run test:upstream-repros
  • Ordinary package gate: 121 passing tests, no type errors; build and changed-file lint pass.
  • Original implementation: five targeted failures covering cold/on-demand duplicate file loss, uncached deletes, and two-queue ownership; fixed implementation passes.
  • Oracle: nine fixed histories plus fixed/random campaigns (12 histories each), asserting accepted bytes, references, SQL/collection convergence, cleanup and idle drain.
  • Upstream command: five failing checks, intentionally separate and unwaived. It covers native and integrated success/retry races plus generated histories; seed 1616, path 3:0:0:1 shrinks to save → start-upload → delete → fail-upload → drain.
  • SDK tested: common 1.57.0, node 0.19.2. Re-run the repro command against the SDK fix before moving in-flight deletion into the ordinary oracle.

See packages/powersync-db-collection/tests/ATTACHMENT-ORACLE.md for scope and replay instructions.

…wnership

Hold a targeted live-query demand through transaction confirmation for eager and on-demand collections. Reject same-ID overlapping saves across queues sharing a database and preserve SDK filename-based relocation. Add intent oracles and separate native/integration repros for the unresolved upstream upload-completion race.
@KyleAMathews

Copy link
Copy Markdown
Author

All four GitHub workflow runs currently report action_required without any jobs having run. Please review/approve the fork workflow runs when ready. Local verification on the published changes passes (121 tests, TypeScript, changed-file lint, build); the separate upstream SDK repro command still fails all five intended contract checks.

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