diff --git a/.agents/skills/verify-in-obsidian/SKILL.md b/.agents/skills/verify-in-obsidian/SKILL.md new file mode 100644 index 00000000..a22a57ad --- /dev/null +++ b/.agents/skills/verify-in-obsidian/SKILL.md @@ -0,0 +1,97 @@ +--- +name: verify-in-obsidian +description: >- + Verify an Obsidian plugin change/fix actually works by driving the REAL Obsidian app in + a dev vault via the `obsidian` CLI (eval / dev:console / dev:errors / dev:screenshot / + command / plugin:reload) and asserting on persisted state — instead of trusting unit + tests with mocks (jsdom can't load real Obsidian). Use when asked to verify/QA a fix, + confirm a feature works end-to-end, check that something persists, reproduce a + user-reported bug in the real app, or validate before merging — especially stateful UI + (settings, modals, drag-reorder, lists). FIRST read the repo's AGENTS.md/CLAUDE.md for + the vault name, plugin id, dev commands, and any QA matrix. +when_to_use: >- + "verify this works", "QA the fix", "does it persist / save", "reproduce the bug in the + real app", "check end-to-end before merge", "drive the GUI", "test in the dev vault". +--- + +# Verify Obsidian plugin changes in the real app + +Vitest/jsdom can't load real Obsidian, so unit tests run against mocks/stubs — they **pass +while the real flow is broken**. For anything stateful (settings, modals, drag-reorder, +persistence), "the unit test passes" is necessary but **not sufficient**. Verify the real +app and assert on what actually persisted. + +## Step 0 — read the repo's own guide first +`AGENTS.md` / `CLAUDE.md` hold the project specifics: **vault name, plugin id, dev-vault +path, build/test commands, and the QA matrix.** Use those, not guesses. Many Obsidian repos +already mandate CLI-verifiable development — follow it. + +## The `obsidian` CLI (the house tool for this) +Anything you can do in Obsidian you can do from the CLI, including dev tools. + +- **`vault=` is a PREFIX argument**, always first: `obsidian vault=dev …`. + The suffix form (`obsidian vault=dev`) can resolve to the wrong vault — don't use it. +- Useful commands: `plugin:reload id=`, `command id=`, `eval code=…`, + `dev:debug on|off`, `dev:console`, `dev:errors`, `dev:screenshot`, `dev:dom`, `dev:css`, + `dev:cdp`, `dev:mobile`, `devtools`, `tabs`, `workspace` (the repo's guide lists the full set). +- `dev:console` / `dev:errors` are only reliable while debugger capture is attached + (`obsidian vault=dev dev:debug on`). +- For non-trivial `eval` code, pass it via a **heredoc/file to `code=…`** to avoid + shell-quoting corruption. + +## The loop (evidence-first) +1. **Rebuild** the bundle (watch: `bun run dev`; one-off per the repo's build script). Note the + plugin `main.js` is often **symlinked** into the dev vault, so a rebuild updates it in place. +2. **Reload the plugin** — `obsidian vault=dev plugin:reload id=`. A rebuilt `main.js` + does NOT auto-reload in a running Obsidian. **This is the #1 false-negative**: never conclude + a fix "doesn't work" without confirming the running app loaded the fresh build. +3. **Reproduce / seed** deterministic state (edit `data.json` or use the plugin's data API), + then reload so it takes effect. +4. **Trigger** the behavior — `obsidian vault=dev command id=<…>`, or drive the UI via `eval` + (see gotchas). Test BOTH hotkey and direct-command paths when relevant. +5. **Gather evidence** — `dev:debug on` → `dev:console clear`/`dev:errors clear` → act → + `dev:console limit=200`, `dev:errors`, `dev:screenshot`, and `tabs`/`workspace ids` for layout. +6. **Assert on persisted state** — read the on-disk `data.json` (source of truth), not just the + UI. Confirm it's plain JSON with no framework artifacts (e.g. Svelte `$state` Proxy leakage). +7. **Add/adjust a regression test** around a CLI-native seam so it's caught without manual + steps. Structure code so Obsidian deps are injected behind interfaces; unit tests then swap + in adapters / the obsidian stub for the pure logic. +8. **Clean up** — remove seeded data + temp files; `dev:debug off`. + +## Evidence-first triage (for bug reports) +- **Don't assume the reported bug still exists** — it may have been fixed by unrelated changes. + Confirm current behavior in the dev vault before touching code. +- **Reproduce under real user conditions**, not synthetic ones: the actual plugin settings, + workspace/tab layout, and platform — and test BOTH the hotkey and direct-command + (`command id=…`) paths. +- **Capture evidence before AND after** the action (`dev:console`, `dev:errors`, + `dev:screenshot`, `tabs`, `workspace`). For pane/tab layout, `workspace … ids` is the + authoritative evidence; `tabs` is a quick summary. +- **If you can't reproduce** after solid evidence-gathering, report the exact setup you tested + and ask for a fresh issue with versions, config, and repro artifacts — don't guess-fix. + +## Driving UI via `eval` — gotchas (apply to raw `eval` and obsidian-e2e alike) +- `eval` may **return before a long async body finishes**. Don't chain `await sleep(...)` + inside one big eval and trust the return. Instead: **fast evals** (one DOM action + immediate + return), and **sequence timing outside** (shell/Node `sleep` between calls). +- **Return JSON-serializable values** (a string), never a live DOM node/object. +- **Selectors:** reuse components' `aria-label`s — they're stable and intent-named. +- **Stacked modals:** query all matches, act on the **last** (topmost). Count `.modal-container`. +- **State pollutes across runs:** close leftovers first (click all `.modal-close-button`, + close settings). +- **Read results reliably:** from a final UI-stable eval, or have the eval write to a vault + file and read it from disk. + +## Committed regression tests (`obsidian-e2e`, if available) +For flows worth guarding, port the check into the repo's e2e suite: +`createObsidianClient({ vault })`, a plugin handle, `acquireVaultRunLock`/`publishMarker` in +`beforeAll`, and `restoreData()` + lock release in `afterAll`. The handle exposes +`reload`, `data().patch(...)`, `exec(commandId, args)`, and `dev.eval(...)`. +**These are local-only** (CI has no Obsidian) — keep a CI-runnable unit/component test as the +primary guard and use the e2e test as the integration safety net. + +## The two lessons worth internalizing +1. **Stale loaded code** is the most common false negative — reload the plugin before + concluding anything about a fix. +2. **Verify the persisted state, not the UI** — and add the regression test against a + CLI-native seam, because mock-based unit tests will happily pass on a broken real flow. diff --git a/src/download/streaming.test.ts b/src/download/streaming.test.ts index 5aa33e84..1b2cec12 100644 --- a/src/download/streaming.test.ts +++ b/src/download/streaming.test.ts @@ -3,7 +3,11 @@ import { requestUrl } from "obsidian"; import { plugin } from "../store"; import type PodNotes from "../main"; import { + isPartialPath, + moveIntoPlace, + partialPathFor, probeAndFetchFirstChunk, + sweepStalePartials, writeStreamedFile, type RangeProbe, } from "./streaming"; @@ -183,3 +187,99 @@ describe("writeStreamedFile", () => { ).rejects.toThrow(/Range request failed/); }); }); + +describe("partialPathFor / isPartialPath", () => { + it("builds a dot-prefixed sibling temp in the same folder", () => { + const tmp = partialPathFor("Podcasts/Show/Ep 1.mp3"); + // dir + "...podnotes-partial" (token first so it survives the cap) + expect(tmp).toMatch(/^Podcasts\/Show\/\..*\.Ep 1\.mp3\.podnotes-partial$/); + expect(isPartialPath(tmp)).toBe(true); + }); + + it("handles a vault-root (no folder) path", () => { + const tmp = partialPathFor("Ep 1.mp3"); + expect(tmp).toMatch(/^\..*\.Ep 1\.mp3\.podnotes-partial$/); + expect(tmp).not.toContain("/"); + expect(isPartialPath(tmp)).toBe(true); + }); + + it("is unique per call so concurrent downloads to one final path never clash", () => { + const a = partialPathFor("Podcasts/Ep.mp3"); + const b = partialPathFor("Podcasts/Ep.mp3"); + expect(a).not.toBe(b); + }); + + it("keeps the temp name within the filesystem limit for a maxed-out title (#22)", () => { + // The final name segment is already capped to ~255; the dot prefix + suffix + // must not push the temp past ENAMETOOLONG on the new write path. + const longFinal = `Podcasts/${"X".repeat(400)}.mp3`; + const tmp = partialPathFor(longFinal); + const lastSegment = tmp.split("/").pop() ?? ""; + expect(lastSegment.length).toBeLessThanOrEqual(255); + expect(isPartialPath(tmp)).toBe(true); + // The unique token (front of the name) must survive the tail truncation. + expect(partialPathFor(longFinal)).not.toBe(tmp); + }); + + it("rejects non-partial paths", () => { + expect(isPartialPath("Podcasts/Ep.mp3")).toBe(false); + expect(isPartialPath("Podcasts/.Ep.mp3")).toBe(false); // dotfile, wrong suffix + expect(isPartialPath("Podcasts/Ep.mp3.podnotes-partial")).toBe(false); // no dot prefix + }); +}); + +describe("moveIntoPlace", () => { + it("moves the temp into place with a single rename (buffers nothing)", async () => { + const rename = vi.fn(async () => {}); + plugin.set({ + app: { vault: { adapter: { writeBinary: vi.fn(), rename } } }, + } as unknown as PodNotes); + + await moveIntoPlace("dir/.tok.ep.mp3.podnotes-partial", "dir/ep.mp3"); + + expect(rename).toHaveBeenCalledWith( + "dir/.tok.ep.mp3.podnotes-partial", + "dir/ep.mp3", + ); + }); +}); + +describe("sweepStalePartials", () => { + function setupSweepAdapter(files: string[]) { + const remove = vi.fn(async () => {}); + const list = vi.fn(async () => ({ files, folders: [] as string[] })); + plugin.set({ + app: { vault: { adapter: { writeBinary: vi.fn(), list, remove } } }, + } as unknown as PodNotes); + return { remove, list }; + } + + it("removes orphaned partials but never an active one or a real file", async () => { + const s = setupSweepAdapter([ + "Podcasts/.Ep.dead.podnotes-partial", // orphan -> remove + "Podcasts/.Ep.live.podnotes-partial", // in flight -> keep + "Podcasts/Ep.mp3", // real file -> keep + ]); + + await sweepStalePartials( + "Podcasts", + (p) => p === "Podcasts/.Ep.live.podnotes-partial", + ); + + expect(s.remove).toHaveBeenCalledTimes(1); + expect(s.remove).toHaveBeenCalledWith("Podcasts/.Ep.dead.podnotes-partial"); + }); + + it("never throws when listing fails (best-effort)", async () => { + const list = vi.fn(async () => { + throw new Error("list failed"); + }); + plugin.set({ + app: { vault: { adapter: { writeBinary: vi.fn(), list, remove: vi.fn() } } }, + } as unknown as PodNotes); + + await expect( + sweepStalePartials("Podcasts", () => false), + ).resolves.toBeUndefined(); + }); +}); diff --git a/src/download/streaming.ts b/src/download/streaming.ts index 7078327b..8700c2ea 100644 --- a/src/download/streaming.ts +++ b/src/download/streaming.ts @@ -1,7 +1,8 @@ -import { requestUrl } from "obsidian"; +import { type DataAdapter, requestUrl } from "obsidian"; import { get } from "svelte/store"; import { plugin } from "../store"; import { encodeUrlForRequest } from "../utility/encodeUrlForRequest"; +import { enforceMaxPathLength } from "../utility/enforceMaxPathLength"; // ---- Streaming download (issue #113) --------------------------------------- // Mobile WebViews have a tight per-process memory budget. Buffering a whole @@ -9,11 +10,26 @@ import { encodeUrlForRequest } from "../utility/encodeUrlForRequest"; // bridge copy — used to OOM-kill Obsidian on iOS. Instead we pull the file in // bounded HTTP Range chunks and append each straight to disk, so peak heap // stays at roughly one chunk regardless of episode size. +// +// ---- Temp-then-move (Waypoint et al. crash) -------------------------------- +// Appending chunks directly to the final vault path makes the growing, half- +// written media file visible to the vault's file watcher: a single download +// fires ~12 create/modify events. Watcher plugins (Waypoint, Dataview, Obsidian +// Git, MOC/index generators) react to each one and re-scan the partial file, +// and that synchronous re-scan storm — racing the chunked writer — OOM-crashes +// the app on mobile. So we stream every chunk to a dot-prefixed sibling temp +// (which Obsidian keeps out of the file index entirely, so it fires zero events +// while it grows), then move the finished, size-verified file into place as a +// single rename. Watchers then see exactly one create of an already-complete +// file — the same shape the pre-#113 atomic createBinary path produced. export const DOWNLOAD_CHUNK_SIZE = 4 * 1024 * 1024; // 4 MiB per range request -export interface BinaryAppendAdapter { - writeBinary(path: string, data: ArrayBuffer): Promise; +// Obsidian's DataAdapter — writeBinary/rename/remove/list, all used below — is +// fully typed and public. Only `appendBinary` exists at runtime on the desktop and +// mobile Capacitor adapters without appearing in the public typings, so we extend +// the real interface with that single bolt-on and keep one cast (appendableAdapter). +export interface BinaryAppendAdapter extends DataAdapter { appendBinary?(path: string, data: ArrayBuffer): Promise; } @@ -97,18 +113,20 @@ export async function probeAndFetchFirstChunk( } // Write the already-fetched first chunk, then pull the remaining bytes in -// bounded Range requests, appending each straight to disk. Peak memory is one -// chunk, not the whole file. Returns the total number of bytes written. +// bounded Range requests, appending each straight to `destPath`. Peak memory is +// one chunk, not the whole file. Returns the total number of bytes written. +// `destPath` is the temp path (see partialPathFor); the caller moves the result +// into the final vault path once it is complete and size-verified. export async function writeStreamedFile( url: string, - filePath: string, + destPath: string, probe: RangeProbe, onProgress?: (written: number, total: number | null) => void, chunkSize: number = DOWNLOAD_CHUNK_SIZE, ): Promise { const adapter = appendableAdapter(); - await adapter.writeBinary(filePath, probe.firstChunk); + await adapter.writeBinary(destPath, probe.firstChunk); let written = probe.firstChunk.byteLength; onProgress?.(written, probe.totalSize); @@ -144,7 +162,7 @@ export async function writeStreamedFile( const chunk = response.arrayBuffer; if (chunk.byteLength === 0) break; - await adapter.appendBinary(filePath, chunk); + await adapter.appendBinary(destPath, chunk); written += chunk.byteLength; onProgress?.(written, probe.totalSize); @@ -154,3 +172,71 @@ export async function writeStreamedFile( return written; } + +const PARTIAL_SUFFIX = ".podnotes-partial"; + +// A monotonic counter makes every temp name unique within a session even when two +// downloads resolve to the same final path (distinct episodes can collide on one +// path — #107/#183) and start in the same millisecond, so concurrent downloads +// never share a temp file. +let partialCounter = 0; + +// The sibling temp path a download streams to before being moved into place. It is +// dot-prefixed so Obsidian keeps it out of the file index (no watcher events while +// it grows), lives in the same folder as the final file so the move is a cheap +// in-place rename, and carries a unique token so concurrent downloads can't clash. +// The token comes first so it survives the length cap (#22), which trims the tail: +// the final name's own segment is already at the byte limit, and prepending a dot + +// appending the suffix would otherwise push the temp past ENAMETOOLONG. The embedded +// final name is only there to make the temp recognisable while debugging. +export function partialPathFor(filePath: string): string { + const slash = filePath.lastIndexOf("/"); + const dir = slash === -1 ? "" : filePath.slice(0, slash + 1); + const name = slash === -1 ? filePath : filePath.slice(slash + 1); + const token = `${Date.now().toString(36)}-${(partialCounter++).toString(36)}`; + return enforceMaxPathLength(`${dir}.${token}.${name}${PARTIAL_SUFFIX}`, PARTIAL_SUFFIX); +} + +export function isPartialPath(path: string): boolean { + const name = path.slice(path.lastIndexOf("/") + 1); + return name.startsWith(".") && name.endsWith(PARTIAL_SUFFIX); +} + +// Move the completed temp file to its final vault path with a single rename, so +// watchers see one create of an already-complete file. rename is a standard +// DataAdapter op on every platform and, because the temp is a sibling of the final +// file, an in-place metadata move that buffers zero bytes — preserving #113's +// memory win. (We never finalize by reading the temp back into memory and +// re-writing it: that whole-file buffer is exactly the #113 OOM this path avoids.) +export async function moveIntoPlace( + tmpPath: string, + filePath: string, +): Promise { + await appendableAdapter().rename(tmpPath, filePath); +} + +// Remove temp partials orphaned in `folder` by a previous download that was hard- +// killed (OOM, force-quit) before it could move its file into place or clean up — +// otherwise hidden partials accumulate and can replicate via sync. `isActive` +// guards this and any concurrent download's live temp from being swept (temp names +// are unique per attempt, so a live temp would otherwise look like an orphan). +// Best-effort: a listing failure must never block a download. +export async function sweepStalePartials( + folder: string, + isActive: (path: string) => boolean, +): Promise { + const adapter = appendableAdapter(); + try { + const listing = await adapter.list(folder); + for (const entry of listing.files) { + if (isPartialPath(entry) && !isActive(entry)) { + await adapter.remove(entry); + } + } + } catch (error) { + console.error( + `Failed to sweep stale download temp files in "${folder}":`, + error, + ); + } +} diff --git a/src/downloadEpisode.test.ts b/src/downloadEpisode.test.ts index 9e28f975..887176e4 100644 --- a/src/downloadEpisode.test.ts +++ b/src/downloadEpisode.test.ts @@ -55,6 +55,29 @@ function setupVault({ streaming = false }: { streaming?: boolean } = {}) { const appendBinary = vi.fn(async (path: string, data: ArrayBuffer) => { written.set(path, (written.get(path) ?? 0) + data.byteLength); }); + // A raw adapter rename moves bytes on disk AND reconciles the destination into + // the vault index (Obsidian fires a create event for it), so the moved file is + // immediately resolvable for playback — mirroring the on-device behaviour. + const rename = vi.fn(async (from: string, to: string) => { + if (written.has(from)) { + written.set(to, written.get(from) ?? 0); + written.delete(from); + } + disk.delete(from); + disk.add(to); + present.add(to); + }); + // Immediate children of `folder`, mirroring DataAdapter.list (returns full + // vault-relative paths). Lets the stale-partial sweep find orphaned temps. + const list = vi.fn(async (folder: string) => { + const prefix = folder ? `${folder}/` : ""; + const files: string[] = []; + for (const p of disk) { + if (!p.startsWith(prefix)) continue; + if (!p.slice(prefix.length).includes("/")) files.push(p); + } + return { files, folders: [] as string[] }; + }); const adapter: Record = { exists: async (path: string) => disk.has(path) || present.has(path), @@ -63,6 +86,8 @@ function setupVault({ streaming = false }: { streaming?: boolean } = {}) { if (streaming) { adapter.writeBinary = writeBinary; adapter.appendBinary = appendBinary; + adapter.rename = rename; + adapter.list = list; } const app = { @@ -91,6 +116,8 @@ function setupVault({ streaming = false }: { streaming?: boolean } = {}) { removeFile, writeBinary, appendBinary, + rename, + list, written, }; } @@ -1241,6 +1268,99 @@ describe("downloadEpisodeWithNotice (streaming range path)", () => { expect(recorded?.size).toBe(16); }); + it("streams to a dot-prefixed temp the watchers don't see, then renames it into place", async () => { + const v = setupVault({ streaming: true }); + requestUrlMock + .mockResolvedValueOnce( + rangeResponse(206, [1, 2, 3, 4, 5, 6, 7, 8], { + "content-type": "audio/mpeg", + "content-range": "bytes 0-7/16", + }), + ) + .mockResolvedValueOnce( + rangeResponse(206, [9, 10, 11, 12, 13, 14, 15, 16], { + "content-range": "bytes 8-15/16", + }), + ); + + await downloadEpisodeWithNotice(makeEpisode(), "Podcasts/{{title}}"); + + // Every chunk went to a single temp path, and it was a hidden sibling partial + // (dot-prefixed, in the same folder) — never the final media path. + const [writePath] = v.writeBinary.mock.calls[0]; + const [appendPath] = v.appendBinary.mock.calls[0]; + expect(writePath).toBe(appendPath); + expect(writePath).toMatch( + /^Podcasts\/\..*\.My Title\.mp3\.podnotes-partial$/, + ); + expect(writePath).not.toBe("Podcasts/My Title.mp3"); + + // Exactly one move into the real path; the temp does not survive. + expect(v.rename).toHaveBeenCalledTimes(1); + expect(v.rename).toHaveBeenCalledWith(writePath, "Podcasts/My Title.mp3"); + expect(v.disk.has(writePath)).toBe(false); + // The moved file is index-resolvable, so playback (getAbstractFileByPath) + // resolves it rather than silently binding src="". + expect(v.present.has("Podcasts/My Title.mp3")).toBe(true); + }); + + it("cleans up the temp (not the final path) when the move into place fails", async () => { + const v = setupVault({ streaming: true }); + v.rename.mockRejectedValueOnce(new Error("rename boom")); + requestUrlMock + .mockResolvedValueOnce( + rangeResponse(206, [1, 2, 3, 4, 5, 6, 7, 8], { + "content-type": "audio/mpeg", + "content-range": "bytes 0-7/16", + }), + ) + .mockResolvedValueOnce( + rangeResponse(206, [9, 10, 11, 12, 13, 14, 15, 16], { + "content-range": "bytes 8-15/16", + }), + ); + + await expect( + downloadEpisodeWithNotice(makeEpisode(), "Podcasts/{{title}}"), + ).rejects.toThrow(/rename boom/); + + const [tmpPath] = v.writeBinary.mock.calls[0]; + // The failed move removed the temp via the adapter, and the final path was + // never created or registered. + expect(v.removeFile).toHaveBeenCalledWith(tmpPath); + expect(v.disk.has(tmpPath)).toBe(false); + expect(v.present.has("Podcasts/My Title.mp3")).toBe(false); + expect(get(downloadedEpisodes)["Pod"]).toBeUndefined(); + }); + + it("sweeps an orphaned partial from a prior killed download before streaming", async () => { + const v = setupVault({ streaming: true }); + // A partial left behind in the target folder by a download that was killed + // mid-stream (the OOM crash this fix addresses). + v.disk.add("Podcasts/.My Title.mp3.dead-orphan.podnotes-partial"); + // An unrelated real file in the same folder must be left untouched. + v.disk.add("Podcasts/Keep Me.mp3"); + requestUrlMock + .mockResolvedValueOnce( + rangeResponse(206, [1, 2, 3, 4, 5, 6, 7, 8], { + "content-type": "audio/mpeg", + "content-range": "bytes 0-7/16", + }), + ) + .mockResolvedValueOnce( + rangeResponse(206, [9, 10, 11, 12, 13, 14, 15, 16], { + "content-range": "bytes 8-15/16", + }), + ); + + await downloadEpisodeWithNotice(makeEpisode(), "Podcasts/{{title}}"); + + expect(v.removeFile).toHaveBeenCalledWith( + "Podcasts/.My Title.mp3.dead-orphan.podnotes-partial", + ); + expect(v.disk.has("Podcasts/Keep Me.mp3")).toBe(true); + }); + it("writes the whole body in one shot when the server ignores Range (200)", async () => { const v = setupVault({ streaming: true }); requestUrlMock.mockResolvedValue( diff --git a/src/downloadEpisode.ts b/src/downloadEpisode.ts index fdbb02c1..59b16c26 100644 --- a/src/downloadEpisode.ts +++ b/src/downloadEpisode.ts @@ -25,7 +25,10 @@ import { } from "./utility/mediaType"; import { appendableAdapter, + moveIntoPlace, + partialPathFor, probeAndFetchFirstChunk, + sweepStalePartials, writeStreamedFile, } from "./download/streaming"; import { detectAudioFileExtension } from "./download/mediaSignatures"; @@ -81,6 +84,10 @@ function parentFolderPath(filePath: string): string { // into a single transfer so they can't stack N full downloads in memory. const downloadsInFlight = new Map>(); +// Temp paths of downloads currently streaming, so the stale-partial sweep never +// deletes a concurrent download's live temp when it sweeps the shared folder. +const activePartialPaths = new Set(); + // Single source of truth for "what extension and on-disk path does this download // get, and is it even playable". Shared by the streaming and legacy paths so the // playability rule (#DL-07 / #213) and path resolution live in exactly one place. @@ -163,30 +170,49 @@ async function downloadEpisodeToDisk( await ensureFolderExists(parentFolderPath(filePath)); - // Clean up a half-written file on any mid-stream failure so a later attempt - // can't mistake a truncated partial for a complete download — the legacy - // createBinary path was atomic, but chunked appendBinary is not. - let total: number; + // Stream to a sibling temp the vault watchers never see, then move the finished + // file into place as one rename — so watcher plugins (Waypoint, Dataview, + // Obsidian Git, ...) don't get a per-chunk modify storm on the growing media + // file and re-scan it into an OOM crash. See streaming.ts for the mechanism. + const tmpPath = partialPathFor(filePath); + activePartialPaths.add(tmpPath); + let moved = false; try { - total = await writeStreamedFile( + // Reclaim temps orphaned by a previous download killed mid-stream (the very + // crash this fix addresses). The active set protects any concurrent + // download's live temp from being swept. + await sweepStalePartials( + parentFolderPath(filePath), + (path) => activePartialPaths.has(path), + ); + + const total = await writeStreamedFile( episode.streamUrl, - filePath, + tmpPath, probe, onProgress, ); - } catch (error) { - await deleteEpisodeFile(filePath); - throw error; - } - if (probe.totalSize !== null && total !== probe.totalSize) { - await deleteEpisodeFile(filePath); - throw new Error( - `Incomplete download: got ${total} of ${probe.totalSize} bytes.`, - ); - } + if (probe.totalSize !== null && total !== probe.totalSize) { + throw new Error( + `Incomplete download: got ${total} of ${probe.totalSize} bytes.`, + ); + } - downloadedEpisodes.addEpisode(episode, filePath, total); - return filePath; + // Only a fully-written, size-verified file is exposed to the vault — as one + // atomic rename, so a partial never even momentarily occupies the real path. + await moveIntoPlace(tmpPath, filePath); + moved = true; + + downloadedEpisodes.addEpisode(episode, filePath, total); + return filePath; + } finally { + // If the file never made it into place (stream, size or move failure), drop + // the temp so a later attempt can't mistake a truncated partial for a + // complete download — the legacy createBinary path was atomic, chunked + // appendBinary is not. deleteEpisodeFile is a no-op when the temp is absent. + if (!moved) await deleteEpisodeFile(tmpPath); + activePartialPaths.delete(tmpPath); + } } export default async function downloadEpisodeWithNotice(