diff --git a/src/__tests__/attachmentSyncTimeoutRetry.test.ts b/src/__tests__/attachmentSyncTimeoutRetry.test.ts index c6ced8f..a3e8380 100644 --- a/src/__tests__/attachmentSyncTimeoutRetry.test.ts +++ b/src/__tests__/attachmentSyncTimeoutRetry.test.ts @@ -213,6 +213,33 @@ describe('syncToGitHub — attachment push survives a stalled IndexedDB read', ( expect(outcome2.result.attachmentSyncSkipped).toBeFalsy() }) + // Attachments now stream in AFTER the sync (fillAttachmentsInBackground), so + // a push can fire while a fill is half done. Neither half may confuse the + // push: a banked image has localSha === the remote blob sha it came from, so + // 3b skips it, and one still in flight simply isn't in IDB yet (absence is + // never a delete — only an explicit tombstone is). + test('a background-fetched attachment is not re-uploaded, while a genuinely local one still is', async () => { + mockAttachmentState.paths = ['Files/fetched.png', 'Files/mine.png'] + // Banked by the background fill: its sha is the remote blob's sha. + mockAttachmentState.shaByPath.set('Files/fetched.png', 'remote-blob-sha') + mockAttachmentState.blobByPath.set('Files/fetched.png', new Blob([new Uint8Array([1])], { type: 'image/png' })) + // Created locally by the user: not on the remote at all. + mockAttachmentState.shaByPath.set('Files/mine.png', 'local-only-sha') + mockAttachmentState.blobByPath.set('Files/mine.png', new Blob([new Uint8Array([2])], { type: 'image/png' })) + mockGetTreeMap.mockResolvedValue(new Map([['Files/fetched.png', 'remote-blob-sha']])) + + const real = note({ id: 'n1', title: 'Real note', content: 'hello\n' }) + await syncToGitHub({ provider: new GitHubProvider('tok'), repo: REPO, notes: [real], folders: [] }) + + const paths = postedTreeEntries().map(e => e.path) + expect(paths).not.toContain('Files/fetched.png') + expect(paths).toContain('Files/mine.png') + // An image the fill has NOT reached yet is absent from IDB — it must not + // be pushed as a deletion. + expect(paths).not.toContain('Files/notyet.png') + expect(mockCreateBlobBinary).toHaveBeenCalledTimes(1) + }) + test('tombstones are also left unconsumed this cycle (3c skipped alongside 3b)', async () => { mockAttachmentState.listTimesOut = true const real = note({ id: 'n1', title: 'Real note', content: 'hello\n' }) diff --git a/src/__tests__/syncApply.test.ts b/src/__tests__/syncApply.test.ts index cdaba4d..f53a647 100644 --- a/src/__tests__/syncApply.test.ts +++ b/src/__tests__/syncApply.test.ts @@ -602,6 +602,105 @@ test('applyAttachmentClassifications: a single failed fetch is counted, not thro spy.mockRestore() }) +// Regression (83 MiB / 175-image vault, 08/09/2026): the apply used to await +// the ENTIRE fetch batch before writing anything to IDB, so a watchdog abort +// mid-batch banked zero attachments and every retry re-classified all 175 as +// `attachmentCreated` and restarted from nothing — a permanent "Syncing…". +// Each blob must now be persisted as it lands, and a caller abort must reject +// the batch instead of being swallowed as N per-file failures. +test('applyAttachmentClassifications: an AbortError mid-batch keeps the already-fetched blobs and propagates', async () => { + useGitHubStore.setState({ token: 'tok', syncRepo: REPO }) + mockGetBlobBytes.mockImplementation(async (..._a: unknown[]) => { + const sha = _a[3] as string + if (sha === 'sha-b') { + // Macrotask tick: every pending microtask (i.e. a.png's IDB write on the + // fixed code) has drained by the time this rejects. + await new Promise(r => setTimeout(r, 0)) + throw Object.assign(new Error('The operation was aborted'), { name: 'AbortError' }) + } + return new Uint8Array([1, 2, 3]) + }) + + const classifications: PullClassification[] = [ + { kind: 'attachmentCreated', path: 'attachments/a.png', remoteSha: 'sha-a', mime: 'image/png' }, + { kind: 'attachmentCreated', path: 'attachments/b.png', remoteSha: 'sha-b', mime: 'image/png' }, + { kind: 'attachmentCreated', path: 'attachments/c.png', remoteSha: 'sha-c', mime: 'image/png' }, + ] + + await expect(applyAttachmentClassifications(classifications)).rejects.toMatchObject({ + name: 'AbortError', + }) + // The first blob is banked, so the next pull classifies it as present. + expect(mockPutAttachmentAtPath.mock.calls.map(call => call[0])).toContain('attachments/a.png') + // ...and the aborted one never is. + expect(mockPutAttachmentAtPath.mock.calls.map(call => call[0])).not.toContain('attachments/b.png') + + mockGetBlobBytes.mockReset() + mockPutAttachmentAtPath.mockReset().mockResolvedValue(undefined) +}) + +// The fill runs in the background now, so it must be cancellable: a newer pull +// (or the page going away) aborts the controller, and the batch has to stop +// fetching while KEEPING what it already banked — that banked set is exactly +// what shrinks the next pull's `attachmentCreated` list. +test('applyAttachmentClassifications: an aborted signal stops the batch and keeps what was banked', async () => { + useGitHubStore.setState({ token: 'tok', syncRepo: REPO }) + const controller = new AbortController() + // Cancel the moment the first image is banked. + mockPutAttachmentAtPath.mockImplementation(async (path: string) => { + if (path === 'attachments/a.png') controller.abort() + }) + mockGetBlobBytes.mockImplementation(async (..._a: unknown[]) => { + const sha = _a[3] as string + // The signal must actually reach getBlobBytes — that is the wiring this + // asserts. Real githubFetch throws exactly this shape on a caller abort. + const signal = _a[4] as AbortSignal | undefined + if (signal?.aborted) throw Object.assign(new Error('The operation was aborted'), { name: 'AbortError' }) + if (sha !== 'sha-a') { + // Park on a macrotask so the abort (a microtask behind a.png's write) + // has landed by the time these resume. + await new Promise(r => setTimeout(r, 0)) + if (signal?.aborted) throw Object.assign(new Error('The operation was aborted'), { name: 'AbortError' }) + } + return new Uint8Array([1, 2, 3]) + }) + + const classifications: PullClassification[] = [ + { kind: 'attachmentCreated', path: 'attachments/a.png', remoteSha: 'sha-a', mime: 'image/png' }, + { kind: 'attachmentCreated', path: 'attachments/b.png', remoteSha: 'sha-b', mime: 'image/png' }, + { kind: 'attachmentCreated', path: 'attachments/c.png', remoteSha: 'sha-c', mime: 'image/png' }, + ] + + await expect( + applyAttachmentClassifications(classifications, { signal: controller.signal }), + ).rejects.toMatchObject({ name: 'AbortError' }) + + const written = mockPutAttachmentAtPath.mock.calls.map(call => call[0]) + expect(written).toEqual(['attachments/a.png']) + // Every getBlobBytes call carried the signal through. + for (const call of mockGetBlobBytes.mock.calls) expect(call[4]).toBe(controller.signal) + + mockGetBlobBytes.mockReset() + mockPutAttachmentAtPath.mockReset().mockResolvedValue(undefined) +}) + +test('applyAttachmentClassifications: reports progress as it banks each image', async () => { + useGitHubStore.setState({ token: 'tok', syncRepo: REPO }) + mockGetBlobBytes.mockResolvedValue(new Uint8Array([1])) + const phases: string[] = [] + + await applyAttachmentClassifications( + [ + { kind: 'attachmentCreated', path: 'attachments/1.png', remoteSha: 's1', mime: 'image/png' }, + { kind: 'attachmentCreated', path: 'attachments/2.png', remoteSha: 's2', mime: 'image/png' }, + ], + { onPhase: (m) => phases.push(m) }, + ) + + expect(phases[0]).toBe('Downloading images… (0 / 2)') + expect(phases[phases.length - 1]).toBe('Downloading images… (2 / 2)') +}) + test('applyAttachmentClassifications: no attachment classifications → all-zero counts, no fetch', async () => { const counts = await applyAttachmentClassifications([ { kind: 'remoteCreated', path: 'X.md', remoteSha: 's', remoteContent: 'x\n', tags: [], body: 'x\n' }, diff --git a/src/__tests__/useGitHubSync.test.ts b/src/__tests__/useGitHubSync.test.ts index 39d1d65..fc49888 100644 --- a/src/__tests__/useGitHubSync.test.ts +++ b/src/__tests__/useGitHubSync.test.ts @@ -120,6 +120,40 @@ beforeEach(() => { }) describe('useGitHubSync — runPullOnly', () => { + // The 83 MiB / 175-image regression, at the hook level: applying attachments + // used to be awaited INSIDE the watchdog-wrapped sync, so a slow image batch + // timed out the whole sync — notes included. The attachment fill is now fire + // and forget, so the sync must reach its terminal state while the images are + // still downloading. A never-resolving fetch stands in for "83 MiB over a + // slow link"; before the change this test would hit the watchdog instead. + test('resolves to a terminal sync state while the attachment fill is still running', async () => { + let fillStarted = false + applyAttachmentClassificationsMock.mockImplementation(() => { + fillStarted = true + return new Promise(() => {}) // never settles + }) + pullFromGitHubMock.mockResolvedValue({ + classifications: [ + { kind: 'remoteCreated', path: 'a.md', remoteSha: 'sha1', remoteContent: '', tags: [], body: 'hi' }, + { kind: 'attachmentCreated', path: 'Files/big.png', remoteSha: 'img1', mime: 'image/png' }, + ], + latestCommitSha: 'commit-sha', + }) + applyNonConflictsMock.mockReturnValue({ created: 1, updated: 0, deleted: 0, autoMerged: 0 }) + + const { result } = renderHook(() => useGitHubSync()) + await act(async () => { + await result.current.runPullOnly() + }) + + expect(fillStarted).toBe(true) + expect(result.current.syncState.kind).toBe('ok') + if (result.current.syncState.kind === 'ok') { + // Queued, not finished — the count comes from the classifications. + expect(result.current.syncState.message).toMatch(/1 image/) + } + }) + test('pulls and applies, never pushes', async () => { pullFromGitHubMock.mockResolvedValue({ classifications: [ diff --git a/src/hooks/useGitHubSync.ts b/src/hooks/useGitHubSync.ts index 68851e8..30695bd 100644 --- a/src/hooks/useGitHubSync.ts +++ b/src/hooks/useGitHubSync.ts @@ -13,12 +13,12 @@ import { syncToGitHub, pullFromGitHub } from '@/utils/githubSync' import type { PullClassification, SyncResult, GitPathUpdate } from '@/utils/githubSync' import { makeGitHostProvider } from '@/utils/gitHost' import { getValidGitHubToken, withTokenRefresh, ReconnectRequiredError } from '@/utils/tokenRefresh' -import { applyNonConflicts, applyAttachmentClassifications } from '@/utils/syncApply' -import { fillShellsInBackground } from '@/utils/backgroundFill' +import { applyNonConflicts } from '@/utils/syncApply' +import { fillShellsInBackground, fillAttachmentsInBackground } from '@/utils/backgroundFill' import { pendingStoreHydration } from '@/utils/ensureStoresHydrated' import { switchVault } from '@/utils/switchVault' import { notesKey } from '@/utils/repoStorage' -import type { ApplyCounts, AttachmentApplyCounts } from '@/utils/syncApply' +import type { ApplyCounts } from '@/utils/syncApply' import type { ConflictTabData } from '@/stores/workspaceStore' import type { SyncRepo } from '@/types' import { @@ -207,10 +207,19 @@ async function runPull( // are skipped here — the caller opens them in the merge UI instead. async function runApply( classifications: PullClassification[], -): Promise<{ notes: ApplyCounts; attachments: AttachmentApplyCounts }> { + onPhase?: (msg: string) => void, +): Promise<{ notes: ApplyCounts; attachmentsQueued: number }> { const notes = await applyNonConflicts(classifications) - const attachments = await applyAttachmentClassifications(classifications) - return { notes, attachments } + // Attachments are NOT awaited: the binary fetch is the one part of an apply + // that can outlast SYNC_WATCHDOG_MS on a big vault, and awaiting it here used + // to time out the entire sync (notes and push included). Kicked off from + // runApply rather than from each of the four call sites so no path can + // forget it. See fillAttachmentsInBackground for the resume story. + void fillAttachmentsInBackground(classifications, onPhase) + const attachmentsQueued = classifications.filter( + c => c.kind === 'attachmentCreated' || c.kind === 'attachmentUpdated', + ).length + return { notes, attachmentsQueued } } // ── Step 3: PUSH ──────────────────────────────────────────────────────────── @@ -258,20 +267,20 @@ async function runPush( // Compose the human-readable status line shown in the sidebar's sync button. function formatSyncMessage( pulled: ApplyCounts, - attached: AttachmentApplyCounts, + attachmentsQueued: number, pushed: SyncResult, ): string { const totalPulled = - pulled.created + pulled.updated + pulled.deleted + - attached.created + attached.updated + pulled.created + pulled.updated + pulled.deleted + attachmentsQueued if (pushed.unchanged && totalPulled === 0) return 'Up to date' const parts: string[] = [] if (pulled.created) parts.push(`↓${pulled.created} new`) if (pulled.updated) parts.push(`↓${pulled.updated} updated`) if (pulled.deleted) parts.push(`↓${pulled.deleted} removed`) - const attachTotal = attached.created + attached.updated - if (attachTotal) parts.push(`↓${attachTotal} image${attachTotal === 1 ? '' : 's'}`) + // Queued, not finished: the images stream in behind the sync and report + // their own "Downloading images… (n / m)" progress line. + if (attachmentsQueued) parts.push(`↓${attachmentsQueued} image${attachmentsQueued === 1 ? '' : 's'}`) if (pushed.created) parts.push(`↑${pushed.created} new`) if (pushed.updated) parts.push(`↑${pushed.updated} updated`) if (pushed.deleted) parts.push(`↑${pushed.deleted} deleted`) @@ -299,19 +308,17 @@ function addSyncToast(toast: Omit): void { // pretending we uploaded anything. function formatPullMessage( pulled: ApplyCounts, - attached: AttachmentApplyCounts, + attachmentsQueued: number, ): string { const totalPulled = - pulled.created + pulled.updated + pulled.deleted + - attached.created + attached.updated + pulled.created + pulled.updated + pulled.deleted + attachmentsQueued if (totalPulled === 0) return 'Up to date' const parts: string[] = [] if (pulled.created) parts.push(`↓${pulled.created} new`) if (pulled.updated) parts.push(`↓${pulled.updated} updated`) if (pulled.deleted) parts.push(`↓${pulled.deleted} removed`) - const attachTotal = attached.created + attached.updated - if (attachTotal) parts.push(`↓${attachTotal} image${attachTotal === 1 ? '' : 's'}`) + if (attachmentsQueued) parts.push(`↓${attachmentsQueued} image${attachmentsQueued === 1 ? '' : 's'}`) if (pulled.autoMerged) parts.push(`auto-merged ${pulled.autoMerged}`) return `Pulled ${parts.join(' · ')}` } @@ -334,6 +341,14 @@ export function useGitHubSync(): UseGitHubSyncResult { const [syncState, setSyncState] = useState({ kind: 'idle' }) + // Progress line for the two fire-and-forget fills (note bodies, attachments). + // Only surfaces when nothing more important is showing, so a background fill + // can never overwrite a real sync status or an error — the status bar reads + // "Synced" for the notes while the images are still coming down. + const backgroundPhase = useCallback((msg: string) => { + setSyncState((prev) => (prev.kind === 'idle' ? { kind: 'running', message: msg } : prev)) + }, []) + // Defensive: clear any leftover `isSyncing: true` from a sync that never // reached its finally block (e.g. tab crash mid-pull, unmount during // setState). Without this, a wedged flag would silently kill every @@ -401,7 +416,7 @@ export function useGitHubSync(): UseGitHubSyncResult { // Apply everything that isn't in conflict; leave push for the user // to retry after they resolve the merge tabs. setSyncState({ kind: 'running', message: 'Applying changes…' }) - await runApply(classifications) + await runApply(classifications, backgroundPhase) if (conflicts.length >= BATCH_THRESHOLD) { openMergeBatch(conflicts) } else { @@ -414,7 +429,7 @@ export function useGitHubSync(): UseGitHubSyncResult { } setSyncState({ kind: 'running', message: 'Applying changes…' }) - const { notes: pullCounts, attachments: attachCounts } = await runApply(classifications) + const { notes: pullCounts, attachmentsQueued } = await runApply(classifications, backgroundPhase) // progressive-clone: stream shell bodies in the background. Fire AND // FORGET — we don't await, so the push below and the success toast @@ -422,10 +437,7 @@ export function useGitHubSync(): UseGitHubSyncResult { // (syncToGitHub drops contentLoaded===false), so an in-flight fill can // never race the push into an empty-body overwrite. Resumes on reload // via the startup kick-off in useAutoSync. - void fillShellsInBackground((msg) => { - // Only surface fill progress when nothing more important is showing. - setSyncState((prev) => (prev.kind === 'idle' ? { kind: 'running', message: msg } : prev)) - }) + void fillShellsInBackground(backgroundPhase) // AI commit messages: when the user has opted in AND didn't // pass a custom message via the SCM input, ask the model to @@ -471,7 +483,7 @@ export function useGitHubSync(): UseGitHubSyncResult { } recordSync(result.commitSha) - const okMessage = formatSyncMessage(pullCounts, attachCounts, result) + const okMessage = formatSyncMessage(pullCounts, attachmentsQueued, result) setSyncState({ kind: 'ok', message: okMessage, @@ -587,7 +599,7 @@ export function useGitHubSync(): UseGitHubSyncResult { // isn't in conflict, open merge tabs (batch view above // BATCH_THRESHOLD) for the user to resolve. setSyncState({ kind: 'running', message: 'Applying changes…' }) - await runApply(classifications) + await runApply(classifications, backgroundPhase) if (conflicts.length >= BATCH_THRESHOLD) { openMergeBatch(conflicts) } else { @@ -600,14 +612,12 @@ export function useGitHubSync(): UseGitHubSyncResult { } setSyncState({ kind: 'running', message: 'Applying changes…' }) - const { notes: pullCounts, attachments: attachCounts } = await runApply(classifications) + const { notes: pullCounts, attachmentsQueued } = await runApply(classifications, backgroundPhase) // progressive-clone: stream shell bodies in the background (fire and // forget). See runSync for the full rationale — pull-only never pushes, // so there's no race to worry about here at all. - void fillShellsInBackground((msg) => { - setSyncState((prev) => (prev.kind === 'idle' ? { kind: 'running', message: msg } : prev)) - }) + void fillShellsInBackground(backgroundPhase) // Record the pulled HEAD as the new baseline so lastCommitSha tracks // the remote after a pull-only too. Previously only runSync called @@ -616,7 +626,7 @@ export function useGitHubSync(): UseGitHubSyncResult { // refetch both key off it. recordSync(latestCommitSha) - const okMessage = formatPullMessage(pullCounts, attachCounts) + const okMessage = formatPullMessage(pullCounts, attachmentsQueued) setSyncState({ kind: 'ok', message: okMessage, diff --git a/src/utils/backgroundFill.ts b/src/utils/backgroundFill.ts index 6b80caa..8a4fd92 100644 --- a/src/utils/backgroundFill.ts +++ b/src/utils/backgroundFill.ts @@ -17,9 +17,10 @@ import { useNoteStore, useGitHubStore } from '@/stores' import type { Note, SyncRepo } from '@/types' +import type { PullClassification } from './githubSync' import { getBlobContent, gitBlobSha } from './github' import { serializeNote, parseNote } from './githubSync' -import { bodyWithInlineTags } from './syncApply' +import { bodyWithInlineTags, applyAttachmentClassifications } from './syncApply' import { decryptNoteContent, isEncryptedContent } from './vaultCrypto' import { getVaultKey, VaultLockedError } from './vaultKey' import { mapWithConcurrency, DEFAULT_CONCURRENCY } from './concurrency' @@ -196,3 +197,44 @@ export async function fillShellsInBackground( export function _resetFillInFlight(): void { fillInFlight = false } + +// ── Attachments ───────────────────────────────────────────────────────────── +// Same fire-and-forget shape as fillShellsInBackground, for the binary half of +// a pull. Attachments used to be applied INSIDE the watchdog-wrapped runSync: +// a vault with a large image folder (measured: 175 images / 83.3 MiB) could not +// finish the fetch inside SYNC_WATCHDOG_MS, so the whole sync timed out — notes +// included — and every retry started over. Now the sync (tree, notes, +// conflicts, push) completes as it did before attachments existed, and the +// images stream in afterwards. +// +// Resume across reloads needs no extra state: syncPull classifies attachments +// off IDB (listAttachmentPaths + getAttachmentGitSha), so whatever the last +// fill banked is simply absent from the next pull's `attachmentCreated` set. +// The startup auto-pull is therefore the resume, exactly as the startup +// fillShellsInBackground kick-off is for note bodies. +let attachmentFillController: AbortController | null = null + +export async function fillAttachmentsInBackground( + classifications: PullClassification[], + onPhase?: (msg: string) => void, +): Promise { + // A newer pull supersedes the one in flight: abort it so its blob fetches + // stop instead of racing the new batch for the same paths. Anything it had + // already banked stays banked. + attachmentFillController?.abort() + const controller = new AbortController() + attachmentFillController = controller + try { + await applyAttachmentClassifications(classifications, { + signal: controller.signal, + onPhase, + }) + } catch { + // Aborted (superseded, or the page going away) or a hard failure. Never + // rethrow: a background fill must not fail the sync that started it. What + // was banked stays banked; the next pull re-classifies only what is still + // missing. + } finally { + if (attachmentFillController === controller) attachmentFillController = null + } +} diff --git a/src/utils/github.ts b/src/utils/github.ts index de66446..314ff85 100644 --- a/src/utils/github.ts +++ b/src/utils/github.ts @@ -665,20 +665,29 @@ export async function createBlobBinary( return data.sha as string } -// Fetch a blob's raw bytes by SHA. GitHub returns it base64-encoded for -// binary content; we decode straight into a Uint8Array so the caller can -// wrap it as a Blob with the correct MIME. +// Fetch a blob's raw bytes by SHA. We ask for the `raw` media type so GitHub +// streams the bytes as-is: the default JSON reply base64-encodes them, which +// is ~33% more on the wire (a 83 MiB image folder ships as ~111 MB) plus a +// decode pass. Fall back to the base64 JSON shape if the Accept is ignored. +// `signal` lets a caller cancel an in-flight blob (the background attachment +// fill aborts its fetches when a newer pull supersedes it). githubFetch already +// distinguishes a caller abort from its own timeout: the former propagates as +// an AbortError without retrying. export async function getBlobBytes( token: string, owner: string, repo: string, sha: string, + signal?: AbortSignal, ): Promise { const res = await githubFetch( `https://api.github.com/repos/${owner}/${repo}/git/blobs/${sha}`, - { headers: GH_HEADERS(token) }, + { headers: { ...GH_HEADERS(token), 'Accept': 'application/vnd.github.raw' }, signal }, ) await ensureOk(res, `Read binary blob ${sha}`) + if (!res.headers.get('content-type')?.includes('json')) { + return new Uint8Array(await res.arrayBuffer()) + } const data = await res.json() if (data.encoding === 'base64') return base64ToBytes(data.content) // Unexpected — UTF-8 encoding on a binary blob would corrupt non-ASCII diff --git a/src/utils/syncApply.ts b/src/utils/syncApply.ts index 2932ae8..c358f55 100644 --- a/src/utils/syncApply.ts +++ b/src/utils/syncApply.ts @@ -447,6 +447,12 @@ export interface AttachmentApplyCounts { export async function applyAttachmentClassifications( classifications: PullClassification[], + opts?: { + /** Cancels in-flight blob fetches (a superseding pull, or the watchdog). */ + signal?: AbortSignal + /** Progress line, same shape as fillShellsInBackground's. */ + onPhase?: (msg: string) => void + }, ): Promise { const counts: AttachmentApplyCounts = { created: 0, updated: 0, failed: 0 } @@ -463,11 +469,29 @@ export async function applyAttachmentClassifications( // no-vercel-clone: fetch the attachment bytes with bounded concurrency // instead of one blob at a time — on a first clone of a vault with many // images the sequential getBlobBytes walk was a second contributor to the - // 45s watchdog blowout. Behaviour is otherwise identical: a single failed - // attachment is logged + counted as `failed`, never aborting the batch (so - // we catch INSIDE the mapper and return null rather than letting - // mapWithConcurrency reject the whole call on the first error). - const fetched = await mapWithConcurrency(attachments, DEFAULT_CONCURRENCY, async (c) => { + // 45s watchdog blowout. + // + // Each blob is BANKED (written to IDB) as soon as it lands, inside the + // mapper. It used to await the whole batch first and write afterwards, which + // made the apply all-or-nothing: a 175-image / 83 MiB vault never finished + // inside the 45s watchdog, so nothing was ever persisted and every retry + // re-classified all 175 as `attachmentCreated` and restarted from zero. + // syncPull classifies straight off IDB (listAttachmentPaths + + // getAttachmentGitSha, which recomputes the sha from the stored bytes — no + // manifest to keep in step), so a half-applied batch just means the next + // pull has fewer creates to do and the sync converges across retries. + // Write order is no longer input order; nothing depends on it (one write + // per distinct path). + // + // A single failed attachment is still logged + counted as `failed` and never + // aborts the batch. The one exception is a caller abort (AbortError — the + // watchdog or a user cancel): the sync is over, so stop instead of grinding + // through the remaining blobs and reporting the cancellation as N failures. + const total = attachments.length + let done = 0 + if (total > 0) opts?.onPhase?.(`Downloading images… (0 / ${total})`) + + await mapWithConcurrency(attachments, DEFAULT_CONCURRENCY, async (c) => { try { // Prefer the bytes already in memory from a zipball pull. const cached = takeZipballAttachmentBytes(c.path) @@ -478,35 +502,24 @@ export async function applyAttachmentClassifications( mime = cached.mime } else { if (!token || !syncRepo) throw new Error('No token / repo for incremental attachment fetch') - bytes = await getBlobBytes(token, syncRepo.owner, syncRepo.name, c.remoteSha) + bytes = await getBlobBytes(token, syncRepo.owner, syncRepo.name, c.remoteSha, opts?.signal) mime = c.mime } - return { c, bytes, mime } - } catch (err) { - console.error(`Failed to fetch attachment ${c.path}:`, err) - return null - } - }) - - // IDB writes are cheap and must stay deterministic — apply them in order. - for (const item of fetched) { - if (!item) { - counts.failed++ - continue - } - const { c, bytes, mime } = item - try { // `.slice()` detaches from any SharedArrayBuffer typing so the Blob // constructor accepts the bytes as a BlobPart on strict TS configs. - const blob = new Blob([bytes.slice()], { type: mime }) - await putAttachmentAtPath(c.path, blob) + await putAttachmentAtPath(c.path, new Blob([bytes.slice()], { type: mime })) if (c.kind === 'attachmentCreated') counts.created++ else counts.updated++ } catch (err) { + // Cancellation is not a per-file failure — let it reject the batch. + if ((err as Error | undefined)?.name === 'AbortError') throw err console.error(`Failed to apply attachment ${c.path}:`, err) counts.failed++ + } finally { + done++ + opts?.onPhase?.(`Downloading images… (${done} / ${total})`) } - } + }) return counts }