fix(download): stream to a temp file then move into place

.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=<name>` is a PREFIX argument**, always first: `obsidian vault=dev <command> …`.
+  The suffix form (`obsidian <command> vault=dev`) can resolve to the wrong vault — don't use it.
+- Useful commands: `plugin:reload id=<plugin>`, `command id=<commandId>`, `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=<plugin>`. 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.

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 + ".<token>.<name>.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();
+	});
+});

src/download/streaming.ts

@@ -1,19 +1,35 @@
-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
 // episode (hundreds of MB) via requestUrl().arrayBuffer — plus the native->JS
 // 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<void>;
+// 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<void>;
 }
 
@@ -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<number> {
 	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<void> {
+	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<void> {
+	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,
+		);
+	}
+}