commit ca9616c8344331150b995ef4fb25aba58766f55b
parent f4cc389be809bfdd8b3354277371ffea76a34415
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sun, 20 Sep 2026 20:09:39 -0400
storage review: the fixes an auto-pause and a store move cannot ship without
BLOCKER — an auto-pause/restore cycle destroyed per-operation overrides.
`sanitizeOverrides` normalises away any override equal to the BASE tier, which
is what keeps the document a list of exceptions; measured against the FORCED
`paused` every `{op:"paused"}` fence the operator set stops being an exception
and is deleted, so the restore hands back a channel with no fence. Reproduced:
`{tier:"normal", overrides:{sync:"paused"}}` came back bare. On this corpus that
is legal-mindset losing both fences and cornbreadman losing its sync fence to
one hiccup of a USB cable. The record is now computed first and the overrides
normalised against `previousTier` — the base the operator actually set.
BLOCKER — nothing stopped a download writing into a store being moved. The
relocation queue key serialises relocations and says nothing about a persist,
and `moveFileCrossDevice`'s unconditional `mkdir -p` makes that a container
lost three ways: a verify refusing at the end of a multi-hour copy, a container
landing in the parked dir that `reclaimParked` then rm -rf's with
`saved-video.json` still pointing at it, and `saved-videos` recreated as a real
dir between the rename and the symlink. So: `lib/savedVideoStore.ts` — in lib
because `savedVideo-server.ts` is lib and may not import controller — holds the
marker's name and reader and `assertSavedVideosStoreWritable`, which
`persistSourceVideo` and `unpersistSavedVideo` call. That is the guard, and it
covers callers that do not exist yet. `savedVideosStoreBusyReason` is the
courtesy half: a sentence before the operator commits to a copy that will be
raced. And the swap now links defensively (an empty recreated dir is removed; a
non-empty one refuses) and records `savedVideosLocationId` only once the link
has been read back.
Also: two consecutive down passes before the watch pauses anything (one blanket-
caught stat is not evidence — EIO, a disk spinning up); restore stays
single-pass, and the asymmetry is argued in the header. The watch calls
`maybeAutoRepoint` for a `mounted-elsewhere` location that armed it, so a drive
that moves while the editor is up no longer waits for a reboot. A resumed copy's
bar is offset by what is already on the far side, so it no longer tops out at
40 %. `rsyncTree` buffers the trailing segment across chunks, so a torn frame
never parses as `bytes=0`. Move-back deletes the target only when this run can
vouch for the local copy — it did the swap, the marker says a previous run got
past it, or the local tree measures at least as large; otherwise it finishes and
says what it did not delete. Deleting a location the store is on is refused, in
the action and on the row. The parked name is timestamped and swept by prefix. A
store that has never existed is created empty instead of failing rsync 23 behind
a stuck marker. And both sides of the containment check are realpath-resolved.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
17 files changed, 1283 insertions(+), 54 deletions(-)
diff --git a/common/controller/relocateChannelMedia.ts b/common/controller/relocateChannelMedia.ts
@@ -480,6 +480,12 @@ async function moveOut(args: {
);
}
await mkdir(target, { recursive: true });
+ // WHAT A PREVIOUS ATTEMPT ALREADY LANDED, so the bar is about the TREE and
+ // not about this process's share of it. rsync counts only what it sends: a
+ // copy resumed at 60 % would otherwise climb 0 → 40 % and stop, having
+ // moved every remaining byte. One walk of the target, which for a fresh
+ // move is an empty directory and costs a readdir.
+ const already = (await measureTree(target)).bytes;
await writeMarker(paths, slug, {
target,
direction: "out",
@@ -496,6 +502,7 @@ async function moveOut(args: {
log,
progress: makeProgressSink({
totalBytes: measured.bytes,
+ alreadyBytes: already,
log,
onProgress: args.onProgress,
}),
@@ -716,6 +723,9 @@ async function moveBack(args: {
log,
progress: makeProgressSink({
totalBytes: measured.bytes,
+ // The same figure the space check above is priced in — what is already
+ // in `data.incoming` from an interrupted run.
+ alreadyBytes: already,
log,
onProgress: args.onProgress,
}),
diff --git a/common/controller/relocateDir.test.ts b/common/controller/relocateDir.test.ts
@@ -0,0 +1,91 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { makeProgressSink, type RelocationProgress } from "./relocateDir";
+
+// Run with:
+// pnpm --filter yt-dlp-transcript-common exec tsx --test controller/relocateDir.test.ts
+//
+// The sink, on its own: no rsync, no disk. What it has to get right is the two
+// things a bar claims — that it is about the TREE, and that it only moves one
+// way.
+
+function frame(bytes: number, percent: number): string {
+ return ` ${bytes.toLocaleString("en-US")} ${percent}% 1.03GB/s 0:00:03`;
+}
+
+function collect(opts: { totalBytes: number; alreadyBytes?: number }) {
+ const seen: RelocationProgress[] = [];
+ const lines: string[] = [];
+ const sink = makeProgressSink({
+ ...opts,
+ log: (m) => lines.push(m),
+ onProgress: (p) => seen.push(p),
+ });
+ return { sink, seen, lines };
+}
+
+test("a fresh copy's fraction is transferred over the measured tree", () => {
+ const { sink, seen } = collect({ totalBytes: 100 });
+ sink(frame(25, 25));
+ sink(frame(100, 100));
+ assert.deepEqual(
+ seen.map((p) => p.fraction),
+ [0.25, 1],
+ );
+ assert.equal(seen[0].bytes, 25);
+});
+
+// RSYNC COUNTS WHAT *IT* SENT, NOT WHAT IS THERE. Resume a copy that died at
+// 60 % and the bar climbs 0 → 40 % and stops, having moved every remaining
+// byte — which an operator reads as a copy that wedged four fifths of the way
+// through something. The caller measures the destination before the copy; this
+// is what that measurement is for.
+test("a resumed copy is offset by what is already on the far side", () => {
+ const { sink, seen } = collect({ totalBytes: 100, alreadyBytes: 60 });
+ sink(frame(0, 0));
+ sink(frame(20, 50));
+ sink(frame(40, 100));
+ assert.deepEqual(
+ seen.map((p) => p.fraction),
+ [0.6, 0.8, 1],
+ );
+ assert.deepEqual(
+ seen.map((p) => p.bytes),
+ [60, 80, 100],
+ );
+ // The detail line says the same thing, so the log and the bar cannot
+ // disagree about the same instant.
+ assert.match(seen[1].detail, /80 B of 100 B · 80 %/);
+});
+
+// A resumed copy re-sends the partial file it died inside, so
+// `already + transferred` can exceed the tree by one file's size. That is a
+// bar reading 112 %, which is a bar nobody believes again.
+test("the fraction is clamped when a partial file is re-sent", () => {
+ const { sink, seen } = collect({ totalBytes: 100, alreadyBytes: 90 });
+ sink(frame(30, 100));
+ assert.equal(seen[0].fraction, 1);
+ assert.equal(seen[0].bytes, 100);
+});
+
+// ONE LINE PER DECILE, and a resumed copy does not re-announce the deciles it
+// did not do — which is why the latch lives in the closure and not in a
+// parameter.
+test("the log gets one line per decile, starting from the resume point", () => {
+ const { sink, lines } = collect({ totalBytes: 100, alreadyBytes: 60 });
+ sink(frame(0, 0));
+ sink(frame(1, 2));
+ sink(frame(20, 50));
+ sink(frame(40, 100));
+ assert.equal(lines.length, 3);
+ assert.match(lines[0], /^Copying… 60 B of 100 B · 60 %/);
+ assert.match(lines[2], /100 B of 100 B · 100 %/);
+});
+
+test("a line that is not a frame is ignored entirely", () => {
+ const { sink, seen, lines } = collect({ totalBytes: 100 });
+ sink("sending incremental file list");
+ sink(".d..t...... 20240101_test1234567/");
+ assert.deepEqual(seen, []);
+ assert.deepEqual(lines, []);
+});
diff --git a/common/controller/relocateDir.ts b/common/controller/relocateDir.ts
@@ -192,17 +192,37 @@ export type RelocationProgress = {
// re-announce the eight deciles it did not do.
export function makeProgressSink(opts: {
totalBytes: number;
+ // BYTES ALREADY ON THE FAR SIDE WHEN THIS RUN STARTED — a resumed copy's
+ // offset, and 0 for a fresh one.
+ //
+ // rsync counts what IT transferred, not what is there: resume a copy that
+ // died at 60 % and the bar climbs 0 → 40 % and stops, having moved every
+ // remaining byte. The operator reads that as a copy that wedged four fifths
+ // of the way through something. Adding the head start makes the fraction a
+ // statement about the TREE, which is what the bar claims to be about, and it
+ // is why the caller measures the destination before the copy rather than
+ // letting rsync's own numbers stand.
+ alreadyBytes?: number;
log: (m: string) => void;
onProgress?: (p: RelocationProgress) => void;
}): (line: string) => void {
let lastDecile = -1;
+ const already = Math.max(0, opts.alreadyBytes ?? 0);
return (line: string) => {
const raw = parseRsyncProgress(line);
if (!raw) return;
- const fraction = rsyncProgressFraction(raw, opts.totalBytes);
- const detail = formatRsyncProgressDetail(raw, opts.totalBytes, formatBytes);
+ // Clamped to the total: a resumed copy can re-send a partial file, so
+ // `already + transferred` may exceed the tree by the size of one file.
+ const done = Math.min(opts.totalBytes || Infinity, already + raw.bytes);
+ const shifted = { ...raw, bytes: done };
+ const fraction = rsyncProgressFraction(shifted, opts.totalBytes);
+ const detail = formatRsyncProgressDetail(
+ shifted,
+ opts.totalBytes,
+ formatBytes,
+ );
opts.onProgress?.({
- bytes: raw.bytes,
+ bytes: done,
totalBytes: opts.totalBytes,
fraction,
rate: raw.rate,
@@ -247,6 +267,17 @@ export async function rsyncTree(opts: {
reject: false,
});
let output = "";
+ // THE TAIL OF THE LAST CHUNK, HELD BACK UNTIL ITS DELIMITER ARRIVES.
+ //
+ // A chunk boundary falls wherever the pipe decides, and it lands INSIDE a
+ // progress frame often enough to matter on a long copy. Parsing the two
+ // halves separately is not merely a dropped frame: ` 30,000,` matches the
+ // number, the percent and the rate of nothing, and the leading ` 30` of
+ // the next chunk can parse as a frame reporting THIRTY BYTES — so the bar
+ // jumps back to 0 % and the decile latch has already spent its line. So the
+ // trailing segment (the one with no delimiter after it) is carried into the
+ // next chunk, which is what every line-oriented stream reader has to do.
+ let pending = "";
child.all?.on("data", (c: Buffer) => {
const text = c.toString("utf8");
// The verify reads this whole buffer back (driftLines), so it is
@@ -258,9 +289,12 @@ export async function rsyncTree(opts: {
}
// rsync rewrites the progress line in place with carriage returns, so one
// chunk carries many frames. Split on both, exactly as taskHooks does for
- // yt-dlp.
+ // yt-dlp — but keep the last segment back unless the chunk ended on a
+ // delimiter, because only then is it a whole line.
+ const segments = (pending + text).split(/[\r\n]+/);
+ pending = /[\r\n]$/.test(text) ? "" : (segments.pop() ?? "");
let passedThrough = "";
- for (const part of text.split(/[\r\n]+/)) {
+ for (const part of segments) {
if (part.trim() === "") continue;
if (parseRsyncProgress(part) === null) {
passedThrough += `${part}\n`;
@@ -271,6 +305,16 @@ export async function rsyncTree(opts: {
if (passedThrough) opts.log(passedThrough);
});
const result = await child;
+ // THE LAST LINE HAS NO DELIMITER AFTER IT. rsync's final `100%` frame is
+ // exactly that, and it is the one the decile latch needs to reach 10.
+ if (pending.trim() !== "") {
+ if (opts.progress && parseRsyncProgress(pending) !== null) {
+ opts.progress(pending);
+ } else {
+ opts.log(`${pending}\n`);
+ }
+ pending = "";
+ }
return { exitCode: result.exitCode ?? 1, output };
}
diff --git a/common/controller/relocateSavedVideos.test.ts b/common/controller/relocateSavedVideos.test.ts
@@ -8,6 +8,7 @@ import {
readFile,
readlink,
rm,
+ symlink,
writeFile,
} from "node:fs/promises";
import { tmpdir } from "node:os";
@@ -313,3 +314,184 @@ async function exists(p: string): Promise<boolean> {
async function readDirMarkerExists(p: string): Promise<boolean> {
return exists(p);
}
+
+// A STORE THAT HAS NEVER EXISTED IS NOT AN RSYNC ERROR. Nothing has ever been
+// persisted on a corpus whose keep-latest window is unset, so there is no
+// `saved-videos` directory at all — and rsync exits 23 on a missing source,
+// which the first cut reported as "rsync failed (exit 23)" AFTER writing the
+// marker, leaving the store stuck in transition over a directory that was never
+// there.
+test("a store that has never existed moves cleanly, leaving an empty one", async () => {
+ await withTmp(async (h) => {
+ // Deliberately no seedStore.
+ const result = await relocateSavedVideos({
+ paths: h.paths,
+ locationId: "cold",
+ io: h.io,
+ onLog: () => {},
+ });
+ assert.equal(result.files, 0);
+ const target = relocatedSavedVideosDir(h.root);
+ assert.equal(await readlink(h.paths.savedVideosDir), target);
+ assert.equal(h.io.read().storage.savedVideosLocationId, "cold");
+ assert.equal(await exists(savedVideosMarkerPath(h.paths)), false);
+ // And the first persist after the move lands on the platter, through the
+ // link, with nothing special asked of it.
+ await mkdir(path.join(h.paths.savedVideosDir, "chan", "vid9"), {
+ recursive: true,
+ });
+ await writeFile(
+ path.join(h.paths.savedVideosDir, "chan", "vid9", "source-media.mp4"),
+ "z",
+ );
+ assert.equal(
+ await readFile(path.join(target, "chan", "vid9", "source-media.mp4"), "utf8"),
+ "z",
+ );
+ });
+});
+
+// THE TARGET IS DELETED ONLY WHEN THIS RUN CAN VOUCH FOR THE COPY STANDING IN
+// FOR IT. "`store` is a real directory" is not evidence: it is also what
+// `rsync --copy-links` of the corpus produces (WORKTREES.md documents that as
+// the way to carry a store into a shard) and what `inconsistent` looks like.
+// The old code rm -rf'd the relocated copy on the strength of it.
+test("move back refuses to delete a target the local copy cannot account for", async () => {
+ await withTmp(async (h) => {
+ // The state: settings say the store is on cold and the target holds two
+ // files, but `saved-videos` is a REAL directory holding only one.
+ const target = relocatedSavedVideosDir(h.root);
+ await mkdir(path.join(target, "chan", "vid1"), { recursive: true });
+ await writeFile(
+ path.join(target, "chan", "vid1", "source-media.mp4"),
+ "x".repeat(4096),
+ );
+ await mkdir(path.join(target, "chan", "vid2"), { recursive: true });
+ await writeFile(
+ path.join(target, "chan", "vid2", "source-media.mkv"),
+ "y".repeat(2048),
+ );
+ await mkdir(path.join(h.paths.savedVideosDir, "chan", "vid1"), {
+ recursive: true,
+ });
+ await writeFile(
+ path.join(h.paths.savedVideosDir, "chan", "vid1", "source-media.mp4"),
+ "x".repeat(4096),
+ );
+ const settings = h.io.read();
+ await h.io.write({
+ ...settings,
+ storage: { ...settings.storage, savedVideosLocationId: "cold" },
+ });
+
+ const lines: string[] = [];
+ const result = await relocateSavedVideos({
+ paths: h.paths,
+ locationId: "",
+ io: h.io,
+ onLog: (l) => lines.push(l),
+ });
+ // The reversible half succeeded: the record is cleared and the store is a
+ // real directory. The DELETE is what was refused.
+ assert.equal(h.io.read().storage.savedVideosLocationId, undefined);
+ assert.equal(await isDir(target), true);
+ assert.match(lines.join("\n"), /was NOT deleted/);
+ assert.equal(result.locationId, "");
+ // No stuck marker over it.
+ assert.equal(await exists(savedVideosMarkerPath(h.paths)), false);
+ });
+});
+
+test("move back does delete the target once the local copy accounts for it", async () => {
+ await withTmp(async (h) => {
+ await seedStore(h.paths);
+ await relocateSavedVideos({
+ paths: h.paths,
+ locationId: "cold",
+ io: h.io,
+ onLog: () => {},
+ });
+ const target = relocatedSavedVideosDir(h.root);
+ await relocateSavedVideos({
+ paths: h.paths,
+ locationId: "",
+ io: h.io,
+ onLog: () => {},
+ });
+ assert.equal(await exists(target), false);
+ });
+});
+
+// A RERUN MUST NOT TRIP OVER THE LAST ATTEMPT'S PARKED COPY. A fixed
+// `saved-videos.relocated` name meant a rename onto a non-empty directory
+// (ENOTEMPTY); the timestamped name plus a prefix sweep is the channel mover's
+// answer, and it is the one that does not orphan an earlier attempt's copy on
+// the volume the move exists to free.
+test("a stale parked copy is swept, not renamed onto", async () => {
+ await withTmp(async (h) => {
+ await seedStore(h.paths);
+ // What an earlier, crashed attempt left beside the store.
+ const stale = path.join(
+ path.dirname(h.paths.savedVideosDir),
+ "saved-videos.relocated-1600000000000",
+ );
+ await mkdir(path.join(stale, "chan", "old"), { recursive: true });
+ await writeFile(path.join(stale, "chan", "old", "source-media.mp4"), "old");
+
+ await relocateSavedVideos({
+ paths: h.paths,
+ locationId: "cold",
+ io: h.io,
+ onLog: () => {},
+ });
+ // Both the stale copy and this run's parked one are gone.
+ const left = (
+ await readdir(path.dirname(h.paths.savedVideosDir))
+ ).filter((n) => n.startsWith("saved-videos."));
+ assert.deepEqual(left, []);
+ });
+});
+
+// A ROOT THAT IS A SYMLINK BACK INTO THE CORPUS is the lexical check's blind
+// spot, and it is the one that copies the store onto itself and then reclaims
+// the only copy.
+test("a root that only RESOLVES inside the corpus is refused", async () => {
+ await withTmp(async (h) => {
+ await seedStore(h.paths);
+ const inside = path.join(h.paths.transcriptsDir, "inside");
+ await mkdir(inside, { recursive: true });
+ // A path outside the corpus that is a link to a path inside it.
+ const sneaky = path.join(path.dirname(h.paths.transcriptsDir), "sneaky");
+ await symlink(inside, sneaky);
+ const settings = h.io.read();
+ await h.io.write({
+ ...settings,
+ storage: {
+ ...settings.storage,
+ locations: [
+ ...settings.storage.locations,
+ { id: "sneaky", label: "Sneaky", root: sneaky, autoRepoint: false },
+ ],
+ },
+ });
+ await assert.rejects(
+ relocateSavedVideos({
+ paths: h.paths,
+ locationId: "sneaky",
+ io: h.io,
+ onLog: () => {},
+ }),
+ /inside the corpus/,
+ );
+ assert.equal(await isDir(h.paths.savedVideosDir), true);
+ assert.equal(await exists(savedVideosMarkerPath(h.paths)), false);
+ });
+});
+
+async function isDir(p: string): Promise<boolean> {
+ try {
+ return (await lstat(p)).isDirectory();
+ } catch {
+ return false;
+ }
+}
diff --git a/common/controller/relocateSavedVideos.ts b/common/controller/relocateSavedVideos.ts
@@ -4,6 +4,7 @@ import {
constants as fsConstants,
mkdir,
readdir,
+ realpath,
rename,
rm,
symlink,
@@ -19,6 +20,10 @@ import {
} from "../lib/settings";
import type { RelocationMarker, RelocationPhase } from "../lib/channelMedia";
import {
+ relocatedSavedVideosDir,
+ savedVideosMarkerPath,
+} from "../lib/savedVideoStore";
+import {
clearDirMarker,
COPY_ARGS,
isDirectory,
@@ -58,19 +63,26 @@ import {
// deliberate — it is an absolute path the operator set, and silently
// re-anchoring it would be this function deciding something it was not asked.
-export const SAVED_VIDEOS_DIRNAME = "saved-videos";
-export const SAVED_VIDEOS_MARKER_FILENAME = ".relocating-saved-videos.json";
-
-export function savedVideosMarkerPath(paths: Paths): string {
- return path.join(paths.transcriptsDir, SAVED_VIDEOS_MARKER_FILENAME);
-}
+// THE MARKER'S NAME AND ITS READER LIVE IN lib/, not here — see
+// lib/savedVideoStore.ts. `savedVideo-server.ts`, which persists a container
+// and is the thing that must be refused while a move is in flight, is lib and
+// may not import controller. Re-exported so every existing importer of this
+// module keeps working; this file WRITES the marker, it does not own it.
+export {
+ SAVED_VIDEOS_DIRNAME,
+ SAVED_VIDEOS_MARKER_FILENAME,
+ relocatedSavedVideosDir,
+ savedVideosMarkerPath,
+} from "../lib/savedVideoStore";
-// The store's home on a location: `<root>/saved-videos`. Flat, beside the
-// channels' `<slug>/data` dirs, and not configurable for the same reason
-// `relocatedDataDir` is not — a mover recognises a target by its shape.
-export function relocatedSavedVideosDir(root: string): string {
- return path.join(root.trim(), SAVED_VIDEOS_DIRNAME);
-}
+// THE PARKED NAME IS TIMESTAMPED AND SWEPT BY PREFIX, exactly as the channel
+// mover's `data.relocated-<ts>` is, and for its reason: a rerun that renamed
+// onto a parked directory left by an earlier attempt gets ENOTEMPTY, and a
+// sweep that took only the name THIS process minted would orphan the earlier
+// one for ever — a full second copy of the store, on the volume the move exists
+// to free.
+const PARKED_PREFIX = "saved-videos.relocated-";
+const INCOMING_NAME = "saved-videos.incoming";
export type SavedVideosStoreStatus =
// The store is a real directory under the corpus.
@@ -275,13 +287,35 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
} catch {
throw new Error(`The destination root ${loc.root} is not writable`);
}
- if (isWithin(paths.transcriptsDir, loc.root)) {
+ // RESOLVED THROUGH EVERY SYMLINK, on BOTH sides. A lexical comparison is
+ // exactly the check the channel mover learned not to make: `/mnt/x` can be a
+ // link back into the corpus, and the corpus dir itself is routinely a link
+ // (the worktrees' `test-transcripts`, a bind mount in a container). Miss it
+ // and the store is copied onto itself, verified against itself, and then the
+ // "source" is reclaimed — which is the only copy.
+ const [realRoot, realCorpus] = await Promise.all([
+ realOrResolved(loc.root),
+ realOrResolved(paths.transcriptsDir),
+ ]);
+ if (isWithin(realCorpus, realRoot)) {
throw new Error(
`The destination root ${loc.root} is inside the corpus at ` +
`${paths.transcriptsDir} — the store would be copied onto itself and ` +
`then reclaimed. Pick a directory on the other drive.`,
);
}
+ // Belt and braces for a root outside the corpus whose `saved-videos` level is
+ // a link back into it: the target resolves separately, because realpath of
+ // the root cannot see through a link one level down.
+ const realTarget = await realOrResolved(relocatedSavedVideosDir(realRoot));
+ const realStore = await realOrResolved(paths.savedVideosDir);
+ if (isWithin(realCorpus, realTarget) || isWithin(realStore, realTarget)) {
+ throw new Error(
+ `The destination ${target} resolves inside the corpus at ` +
+ `${paths.transcriptsDir} — the store would be copied onto itself and ` +
+ `then reclaimed. Pick a directory on the other drive.`,
+ );
+ }
const state = await linkOrDirState(store);
if (!a.resume && state.kind !== "real-dir" && state.kind !== "missing") {
throw new Error(
@@ -289,6 +323,18 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
`already moved, or something else is there.`,
);
}
+ // A STORE THAT HAS NEVER EXISTED IS NOT AN RSYNC ERROR. Nothing has ever been
+ // persisted on a corpus whose keep-latest window is unset, so there is no
+ // `saved-videos` directory at all — and rsync exits 23 on a missing source,
+ // which the old code reported as "rsync failed (exit 23)" AFTER writing the
+ // marker, leaving the store stuck in transition over a directory that was
+ // never there. Create it empty and let the move proceed: the operator asked
+ // for the store to live on the platter, and an empty store on the platter is
+ // exactly that answer, ready for the first persist.
+ if (state.kind === "missing") {
+ await mkdir(store, { recursive: true });
+ log(`${store} did not exist yet — created it empty before the move.`);
+ }
let phase: RelocationPhase = a.resume?.phase ?? "copy";
const measured = await measureTree(store);
@@ -313,6 +359,8 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
);
}
await mkdir(target, { recursive: true });
+ // What a previous attempt already landed — see the channel mover.
+ const already = (await measureTree(target)).bytes;
await stampMarker(markerFile, target, "out", "copy");
const { exitCode } = await rsyncTree({
rsyncBin: paths.rsyncBin,
@@ -322,6 +370,7 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
log,
progress: makeProgressSink({
totalBytes: measured.bytes,
+ alreadyBytes: already,
log,
onProgress: a.onProgress,
}),
@@ -352,7 +401,10 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
// OBSERVE, DO NOT ASSUME — a crash lands between any two syscalls, and the
// marker can only say which phase it was in, never how far through it got.
const now = await linkOrDirState(store);
- const parked = `${store}.relocated`;
+ const parked = path.join(
+ path.dirname(store),
+ `${PARKED_PREFIX}${Date.now()}`,
+ );
if (now.kind === "real-dir") {
// Re-verify: a resumed run did not do the copy in this process and must
// not take the interrupted one's word for it.
@@ -366,14 +418,24 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
signal: a.signal,
})
).retried;
- await rm(parked, { recursive: true, force: true });
await rename(store, parked);
} else if (now.kind === "other") {
throw new Error(
`${store} is neither a directory nor a symlink — refusing to replace it`,
);
}
- const after = await linkOrDirState(store);
+ // THE LINK IS MADE DEFENSIVELY, AND THE RECORD IS WRITTEN ONLY IF IT EXISTS.
+ //
+ // `after.kind === "missing"` was the whole condition, and it is not enough:
+ // `moveFileCrossDevice` (savedVideo-server.ts) does an unconditional
+ // `mkdir -p` of the destination's parent, so a persist firing in the
+ // instant between the rename above and this line RECREATES `saved-videos`
+ // as a real, empty directory. The old code then saw `real-dir`, made no
+ // link, and went on to record the store as living on a location it could
+ // not be reached at — a settings field pointing at bytes nothing follows.
+ // (The guard in lib/savedVideoStore.ts is what closes that window; this is
+ // what makes the outcome safe if it is ever open anyway.)
+ let after = await linkOrDirState(store);
if (
after.kind === "link" &&
path.resolve(after.linkTarget) !== path.resolve(target)
@@ -382,8 +444,42 @@ async function moveStoreOut(a: Inner): Promise<SavedVideosRelocateResult> {
`${store} already points at ${after.linkTarget}, not ${target}`,
);
}
- if (after.kind === "missing") await symlink(target, store);
- // THE RECORD IS WRITTEN ONLY NOW, on success, after the link exists.
+ if (after.kind === "real-dir") {
+ // An empty directory is the mkdir -p above and nothing else — remove it
+ // and link. A NON-empty one holds bytes this move did not copy, and
+ // deleting it is not this function's call to make.
+ const stray = await readdir(store).catch(() => ["keep"] as string[]);
+ if (stray.length > 0) {
+ throw new Error(
+ `${store} is a real directory again and is not empty (${stray.length} ` +
+ `entr(ies)) — something wrote into the store during the move. The ` +
+ `copy at ${target} is complete and untouched; move those entries ` +
+ `aside and rerun.`,
+ );
+ }
+ log(
+ `${store} was recreated as an empty directory during the swap ` +
+ `(a persist raced the move) — removing it and linking.`,
+ );
+ await rm(store, { recursive: false, force: true }).catch(async () => {
+ await rm(store, { recursive: true, force: true });
+ });
+ after = await linkOrDirState(store);
+ }
+ if (after.kind === "missing") {
+ await symlink(target, store);
+ after = await linkOrDirState(store);
+ }
+ if (after.kind !== "link") {
+ throw new Error(
+ `${store} is not a symlink after the swap (it is ${after.kind}) — ` +
+ `refusing to record the store as relocated. The copy at ${target} is ` +
+ `complete; nothing has been deleted.`,
+ );
+ }
+ // THE RECORD IS WRITTEN ONLY NOW: on success, after the link exists and has
+ // been read back. It is a statement about where bytes are, and a statement
+ // nothing can follow is worse than no statement.
const latest = io.read();
await io.write({
...latest,
@@ -433,7 +529,7 @@ async function moveStoreBack(a: Inner): Promise<SavedVideosRelocateResult> {
);
}
- const incoming = `${store}.incoming`;
+ const incoming = path.join(path.dirname(store), INCOMING_NAME);
let phase: RelocationPhase = a.resume?.phase ?? "copy";
if (phase === "copy" && !(await isDirectory(target))) {
throw new Error(
@@ -479,6 +575,8 @@ async function moveStoreBack(a: Inner): Promise<SavedVideosRelocateResult> {
log,
progress: makeProgressSink({
totalBytes: measured.bytes,
+ // The figure the space check above is priced in.
+ alreadyBytes: already,
log,
onProgress: a.onProgress,
}),
@@ -503,6 +601,24 @@ async function moveStoreBack(a: Inner): Promise<SavedVideosRelocateResult> {
phase = "swap";
}
+ // MAY THE TARGET BE DELETED AT THE END? Only when this run can vouch for the
+ // copy that is standing in for it.
+ //
+ // The old code reclaimed the target whenever `store` was a real directory,
+ // and "a real directory" is not evidence of anything: it is ALSO what
+ // `rsync --copy-links` of the corpus produces (WORKTREES.md documents that as
+ // the way to carry a store into a shard), and it is what the `inconsistent`
+ // state looks like — settings naming a location while the disk holds a real
+ // dir. In both cases `rm -rf target` deletes the relocated copy on the
+ // strength of a local directory nobody compared it with.
+ //
+ // Three things count as vouching, and nothing else does: this run did the
+ // swap itself from a verified `incoming`; the marker says a previous run got
+ // past the swap (`phase: "reclaim"`, which is written only after it); or the
+ // local tree measures at least as large as the target's, which is the
+ // cheapest honest answer when neither of the first two applies.
+ let vouched = a.resume?.phase === "reclaim";
+
if (phase === "swap") {
await stampMarker(markerFile, target, "back", "swap");
const now = await linkOrDirState(store);
@@ -523,6 +639,9 @@ async function moveStoreBack(a: Inner): Promise<SavedVideosRelocateResult> {
// recursively would be the one way this design eats the media.
if (now.kind !== "missing") await unlink(store);
await rename(incoming, store);
+ // THIS run moved a verified copy into place. Nothing is more vouched
+ // than that.
+ vouched = true;
log(`Swapped: ${store} is a real directory again`);
}
const latest = io.read();
@@ -533,6 +652,40 @@ async function moveStoreBack(a: Inner): Promise<SavedVideosRelocateResult> {
await stampMarker(markerFile, target, "back", "reclaim");
}
+ // THE LAST MEASUREMENT BEFORE THE ONLY DESTRUCTIVE STEP. Cheap — two walks of
+ // a store that holds one container per pinned video — and it is the answer to
+ // "is what I am about to delete still the only copy".
+ if (!vouched && (await isDirectory(target))) {
+ const [here, there] = await Promise.all([
+ measureTree(store),
+ measureTree(target),
+ ]);
+ vouched = here.bytes >= there.bytes && here.files >= there.files;
+ if (!vouched) {
+ // NOT AN ERROR, AND DELIBERATELY NOT: the move back has succeeded as far
+ // as anything reversible goes — the store is a real directory and the
+ // record is cleared. What is refused is the DELETE, and leaving a second
+ // copy on the platter is the safe half of that decision. The marker is
+ // cleared so the store is not stuck in transition over it.
+ await reclaimParked(paths, log);
+ await clearDirMarker(markerFile);
+ log(
+ `The store is in place, but ${target} was NOT deleted: it holds ` +
+ `${there.files} file(s)/${formatBytes(there.bytes)} against ` +
+ `${here.files}/${formatBytes(here.bytes)} here, so this run cannot ` +
+ `vouch that the local copy is complete. Compare them and remove it ` +
+ `by hand.`,
+ );
+ return {
+ locationId: "",
+ target,
+ bytes: here.bytes,
+ files: here.files,
+ resumed: Boolean(a.resume),
+ retried,
+ };
+ }
+ }
await rm(target, { recursive: true, force: true });
await reclaimParked(paths, log);
await clearDirMarker(markerFile);
@@ -556,16 +709,21 @@ async function reclaimParked(
log: (m: string) => void,
): Promise<void> {
const parent = path.dirname(paths.savedVideosDir);
- const base = path.basename(paths.savedVideosDir);
const names = await readdir(parent).catch(() => [] as string[]);
for (const n of names) {
- if (n !== `${base}.relocated` && n !== `${base}.incoming`) continue;
+ if (!n.startsWith(PARKED_PREFIX) && n !== INCOMING_NAME) continue;
const p = path.join(parent, n);
log(`Reclaiming ${p}`);
await rm(p, { recursive: true, force: true });
}
}
+// Resolved through every symlink when the path exists, lexically when it does
+// not — `relocateChannelMedia.ts`'s helper, same name, same reason.
+async function realOrResolved(p: string): Promise<string> {
+ return await realpath(p).catch(() => path.resolve(p));
+}
+
function isWithin(parent: string, child: string): boolean {
const rel = path.relative(parent, child);
return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel));
diff --git a/common/controller/storageWatch.test.ts b/common/controller/storageWatch.test.ts
@@ -1,4 +1,4 @@
-import { test } from "node:test";
+import { beforeEach, test } from "node:test";
import assert from "node:assert/strict";
import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
@@ -10,7 +10,15 @@ import {
sanitizeChannelPriority,
} from "../lib/channelPriority";
import { LANES } from "../lib/autoQueueTypes";
-import { runStorageWatchPass } from "./storageWatch";
+import {
+ resetStorageWatchSuspicion,
+ runStorageWatchPass,
+} from "./storageWatch";
+
+// THE CONFIRMATION COUNT IS MODULE STATE (see storageWatch.ts rule 3), so each
+// case starts from a clean one — otherwise the second test inherits the first
+// test's suspicions and pauses on what should be its first pass.
+beforeEach(() => resetStorageWatchSuspicion());
// Run with:
// pnpm --filter yt-dlp-transcript-common exec tsx --test controller/storageWatch.test.ts
@@ -103,16 +111,46 @@ function tierOf(h: H, slug: string): string | undefined {
return h.io.read().channelPriority.channels[slug]?.tier;
}
+// ONE BAD READ IS A SUSPICION, TWO IN A ROW IS A FACT — availability is a bare
+// stat with a blanket catch, so an EIO or a spun-down disk reads exactly like
+// "not mounted". Most cases here are about what happens once a drive really is
+// gone, so they run the confirming pair and assert on the second.
+async function twoPasses(h: H) {
+ const first = await runStorageWatchPass({
+ paths: h.paths,
+ io: h.io,
+ bins: h.paths,
+ });
+ const second = await runStorageWatchPass({
+ paths: h.paths,
+ io: h.io,
+ bins: h.paths,
+ });
+ return { first, second };
+}
+
test("a channel whose target is gone is auto-paused, once, in one write", async () => {
await withTmp(async (h) => {
await seedRelocated(h, "gone-a", { targetExists: false });
await seedRelocated(h, "gone-b", { targetExists: false });
await seedRelocated(h, "fine", { targetExists: true });
- const first = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
- assert.deepEqual(first.paused.sort(), ["gone-a", "gone-b"]);
- assert.deepEqual(first.restored, []);
- assert.equal(first.wrote, true);
+ // RUN THE PASSES ONE AT A TIME HERE, not through twoPasses(): the write
+ // count between them is exactly what this case is about.
+ const pass = () =>
+ runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
+ const first = await pass();
+ // The first pass only suspects — and writes NOTHING, which is the point:
+ // one flaky stat must not rewrite the corpus's priority document.
+ assert.deepEqual(first.suspected.sort(), ["gone-a", "gone-b"]);
+ assert.deepEqual(first.paused, []);
+ assert.equal(first.wrote, false);
+ assert.equal(h.writes, 0);
+ // The second confirms.
+ const second = await pass();
+ assert.deepEqual(second.paused.sort(), ["gone-a", "gone-b"]);
+ assert.deepEqual(second.restored, []);
+ assert.equal(second.wrote, true);
// TWO CHANNELS, ONE WRITE. Ten on a drive that vanished must be one pulse
// bump, not ten.
assert.equal(h.writes, 1);
@@ -124,9 +162,9 @@ test("a channel whose target is gone is auto-paused, once, in one write", async
);
// A QUIET PASS WRITES NOTHING. The document already describes the world.
- const second = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
- assert.deepEqual(second.paused, []);
- assert.equal(second.wrote, false);
+ const third = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
+ assert.deepEqual(third.paused, []);
+ assert.equal(third.wrote, false);
assert.equal(h.writes, 1);
});
});
@@ -143,7 +181,7 @@ test("the drive coming back restores the tier it overwrote", async () => {
});
h.writes = 0;
- await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
+ await twoPasses(h);
assert.equal(tierOf(h, "away"), "paused");
await mkdir(path.join(h.root, "away", "data"), { recursive: true });
@@ -171,9 +209,9 @@ test("a manually paused channel is never claimed by the watch", async () => {
}),
});
h.writes = 0;
- const r = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
- assert.deepEqual(r.paused, []);
- assert.equal(r.wrote, false);
+ const { second } = await twoPasses(h);
+ assert.deepEqual(second.paused, []);
+ assert.equal(second.wrote, false);
assert.equal(h.writes, 0);
assert.equal(
h.io.read().channelPriority.channels.off.autoPaused,
@@ -196,9 +234,9 @@ test("a channel mid-relocation is not auto-paused", async () => {
phase: "copy",
}),
);
- const r = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
- assert.deepEqual(r.paused, []);
- assert.equal(r.wrote, false);
+ const { second } = await twoPasses(h);
+ assert.deepEqual(second.paused, []);
+ assert.equal(second.wrote, false);
});
});
@@ -240,12 +278,10 @@ test("a record on an in-place channel is restored", async () => {
test("write: false reports the transition and changes nothing", async () => {
await withTmp(async (h) => {
await seedRelocated(h, "gone", { targetExists: false });
- const r = await runStorageWatchPass({
- paths: h.paths,
- io: h.io,
- bins: h.paths,
- write: false,
- });
+ // The first pass only suspects, whatever `write` says.
+ const opts = { paths: h.paths, io: h.io, bins: h.paths, write: false };
+ assert.deepEqual((await runStorageWatchPass(opts)).suspected, ["gone"]);
+ const r = await runStorageWatchPass(opts);
assert.deepEqual(r.paused, ["gone"]);
assert.equal(r.wrote, false);
assert.equal(h.writes, 0);
@@ -262,6 +298,80 @@ test("no locations and nothing auto-paused is a free pass", async () => {
});
h.writes = 0;
const r = await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths });
- assert.deepEqual(r, { probed: 0, paused: [], restored: [], wrote: false });
+ assert.deepEqual(r, {
+ probed: 0,
+ suspected: [],
+ repointed: [],
+ paused: [],
+ restored: [],
+ wrote: false,
+ });
+ });
+});
+
+// ONE BAD READ MUST NOT PAUSE A TIER. Availability is a bare `stat` with a
+// blanket catch (storageVolumes.ts), so an EIO on a flaky cable or a disk that
+// has spun down and needs a beat to answer is indistinguishable from "not
+// mounted" — and pausing on it rewrites the corpus's priority document for a
+// drive that is fine.
+test("a drive that blips for one pass is never paused", async () => {
+ await withTmp(async (h) => {
+ await seedRelocated(h, "blip", { targetExists: false });
+ const first = await runStorageWatchPass({
+ paths: h.paths,
+ io: h.io,
+ bins: h.paths,
+ });
+ assert.deepEqual(first.suspected, ["blip"]);
+ assert.deepEqual(first.paused, []);
+ assert.equal(h.writes, 0);
+
+ // It answers on the next pass. Nothing was ever paused, and the suspicion
+ // is dropped — so a LATER real outage starts its own two-pass count rather
+ // than pausing immediately on the strength of a blip an hour ago.
+ await mkdir(path.join(h.root, "blip", "data"), { recursive: true });
+ const second = await runStorageWatchPass({
+ paths: h.paths,
+ io: h.io,
+ bins: h.paths,
+ });
+ assert.deepEqual(second.paused, []);
+ assert.deepEqual(second.restored, []);
+ assert.equal(second.wrote, false);
+ assert.equal(h.writes, 0);
+
+ // Prove the suspicion really was dropped: the drive going away again takes
+ // two fresh passes.
+ await rm(path.join(h.root, "blip"), { recursive: true, force: true });
+ assert.deepEqual(
+ (await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths }))
+ .paused,
+ [],
+ );
+ assert.deepEqual(
+ (await runStorageWatchPass({ paths: h.paths, io: h.io, bins: h.paths }))
+ .paused,
+ ["blip"],
+ );
+ });
+});
+
+// RESTORE STAYS SINGLE-PASS, and the asymmetry is the point: being slow to
+// pause costs a few refused units (the start-of-work guards catch those), while
+// being slow to restore leaves a lane off after the operator fixed the cable.
+test("the restore needs only one good pass", async () => {
+ await withTmp(async (h) => {
+ await seedRelocated(h, "back", { targetExists: false });
+ await twoPasses(h);
+ assert.equal(tierOf(h, "back"), "paused");
+ h.writes = 0;
+ await mkdir(path.join(h.root, "back", "data"), { recursive: true });
+ const r = await runStorageWatchPass({
+ paths: h.paths,
+ io: h.io,
+ bins: h.paths,
+ });
+ assert.deepEqual(r.restored, ["back"]);
+ assert.equal(h.writes, 1);
});
});
diff --git a/common/controller/storageWatch.ts b/common/controller/storageWatch.ts
@@ -22,7 +22,7 @@ import { inspectChannelMedia } from "../lib/channelMedia";
import { locationOfDataDir } from "../lib/storageLocations";
import type { VolumeBins } from "../lib/storageVolumes";
import { listChannelConfigs } from "./channels";
-import { probeAllLocations } from "./storageLocations";
+import { maybeAutoRepoint, probeAllLocations } from "./storageLocations";
// THE DRIVE WENT AWAY WHILE THE CHANNEL WAS ON — now what.
//
@@ -49,6 +49,17 @@ import { probeAllLocations } from "./storageLocations";
// an `autoPaused` record, and the one priority writer clears that record on
// any manual tier change — so a drive coming back can never un-pause a
// channel the operator paused on purpose in the meantime.
+// 3. IT TAKES TWO CONSECUTIVE DOWN PASSES TO PAUSE, AND ONE UP PASS TO
+// RESTORE. Availability is a bare `stat` with a blanket catch
+// (`storageVolumes.ts`) — an EIO on a flaky cable, or a disk that has spun
+// down and needs a beat to answer, reads exactly like "not mounted". One
+// such read would otherwise pause every channel on the drive and rewrite
+// the corpus's priority document. The confirmation is held IN MEMORY, not
+// in settings: a pending suspicion is not a fact worth persisting, and a
+// process restart starting the count again is the safe direction. The
+// asymmetry is deliberate — being slow to pause costs a few refused units
+// (the start-of-work guards catch those), while being slow to RESTORE costs
+// the operator a lane that stays off after they fixed the cable.
//
// IT IS A RUNNER, so `ARCHILYZER_IDLE_BOOT` must not arm it: a container
// pointed at somebody else's corpus for the first time has no business
@@ -60,6 +71,13 @@ import { probeAllLocations } from "./storageLocations";
export type StorageWatchResult = {
// Locations probed this pass.
probed: number;
+ // Channels this pass saw as unreachable for the FIRST time. They are not
+ // paused yet; the next pass decides. Reported so a caller (and the test) can
+ // see the confirmation working rather than infer it from silence.
+ suspected: string[];
+ // Locations whose volume was found at a different mountpoint and for which an
+ // auto re-point was queued. Empty unless a location has `autoRepoint` on.
+ repointed: string[];
// Channels newly auto-paused, and channels restored. Both empty on a quiet
// pass, which is the overwhelming majority.
paused: string[];
@@ -80,6 +98,15 @@ export type StorageWatchOpts = {
const DEFAULT_IO = { read: getSettings, write: writeSettings };
+// Channels seen down on the LAST pass and not yet paused. Module state, and
+// deliberately not settings: see rule 3 in the header.
+const suspected = new Set<string>();
+
+// Test seam, and the escape hatch for a process that wants a clean count.
+export function resetStorageWatchSuspicion(): void {
+ suspected.clear();
+}
+
export async function runStorageWatchPass(
opts: StorageWatchOpts = {},
): Promise<StorageWatchResult> {
@@ -90,6 +117,8 @@ export async function runStorageWatchPass(
const locations = settings.storage.locations;
const out: StorageWatchResult = {
probed: 0,
+ suspected: [],
+ repointed: [],
paused: [],
restored: [],
wrote: false,
@@ -111,6 +140,49 @@ export async function runStorageWatchPass(
});
out.probed = Object.keys(probes).length;
+ // THE DRIVE CAME UP SOMEWHERE ELSE — FOLLOW IT, IF THE OPERATOR ARMED THAT.
+ //
+ // `mounted-elsewhere` is the one status with a remedy that moves no bytes:
+ // the volume IS here, under a different mountpoint, and a re-point rewrites
+ // the links. Until now only the BOOT pass took it, so a disk that came back
+ // at a new mountpoint while the editor was up sat there while this pass
+ // dutifully paused every channel on it — and the fix was a restart. Same
+ // opt-in (`autoRepoint`), same preflight, same refusal-with-a-reason; what
+ // changes is that the cadence can reach it.
+ //
+ // It runs BEFORE the per-channel loop and the enqueued job runs after this
+ // pass returns, so this pass still sees (and may still suspect) the channels
+ // on that location — which is correct: nothing has moved yet, and the
+ // two-pass confirmation gives the re-point a whole interval to land before
+ // anything is paused.
+ if (opts.write !== false) {
+ for (const loc of locations) {
+ const probe = probes[loc.id];
+ if (!probe || probe.status !== "mounted-elsewhere" || !loc.autoRepoint) {
+ continue;
+ }
+ const outcome = await maybeAutoRepoint({
+ paths,
+ location: loc,
+ probe,
+ bins: opts.bins ?? paths,
+ io,
+ }).catch((err) => ({
+ started: false as const,
+ reason: (err as Error).message,
+ }));
+ if (outcome.started) {
+ out.repointed.push(loc.id);
+ log(
+ `[storage] "${loc.id}": the volume came up at a different mountpoint ` +
+ `— auto re-point queued (${outcome.newRoot})`,
+ );
+ } else {
+ log(`[storage] "${loc.id}": auto re-point declined — ${outcome.reason}`);
+ }
+ }
+ }
+
const configs = await listChannelConfigs(paths);
let model: ChannelPriority = settings.channelPriority;
@@ -121,6 +193,7 @@ export async function runStorageWatchPass(
// In place. It cannot be on a drive that went away — but it CAN carry a
// record from before it was moved back, and that record has to come off
// or the channel stays paused for ever.
+ suspected.delete(slug);
if (wasAutoPaused) {
model = restoreAfterMedia(model, slug);
out.restored.push(slug);
@@ -145,6 +218,17 @@ export async function runStorageWatchPass(
: locationDown || media.status === "unreachable";
if (down && !wasAutoPaused) {
+ // ONE BAD READ IS A SUSPICION, TWO IN A ROW IS A FACT. See rule 3.
+ if (!suspected.has(slug)) {
+ suspected.add(slug);
+ out.suspected.push(slug);
+ log(
+ `[storage] ${slug}: media unreachable (${
+ loc ? `location "${loc.id}" is ${probe?.status ?? "unprobed"}` : media.status
+ }) — waiting for a second pass to confirm before pausing`,
+ );
+ continue;
+ }
const before = model;
model = autoPauseForMedia(model, slug);
// autoPauseForMedia no-ops on a channel the OPERATOR already paused —
@@ -159,6 +243,7 @@ export async function runStorageWatchPass(
}
continue;
}
+ if (!down) suspected.delete(slug);
if (!down && wasAutoPaused) {
const restoredTo = model.channels[slug]?.autoPaused?.previousTier;
model = restoreAfterMedia(model, slug);
diff --git a/common/jobs/progressParsers.test.ts b/common/jobs/progressParsers.test.ts
@@ -230,3 +230,32 @@ test("rsync progress: the detail line is the one wording", () => {
"57.2 MB of 57.2 MB · 100 % · 1.05GB/s",
);
});
+
+// A TORN FRAME MUST NEVER PARSE AS A FRAME. A chunk boundary falls wherever the
+// pipe decides, and it lands inside a progress line often enough to matter on a
+// long copy. The halves are the hazard, not the loss: ` 30,000,` still
+// matches nothing, but the NEXT chunk's leading ` 30` can parse as a frame
+// reporting thirty bytes — the bar jumps back to 0 % and the decile latch has
+// already spent its line. relocateDir.ts holds the trailing segment back; this
+// pins what each half does on its own so that buffering is provably necessary.
+test("rsync progress: half a frame is not a frame, and two halves are one", () => {
+ const whole = " 30,000,000 85% 1.03GB/s 0:00:03 (xfr#1, to-chk=1/3)";
+ const cut = 12;
+ const head = whole.slice(0, cut);
+ const tail = whole.slice(cut);
+ // The head alone has no percentage, so it cannot parse.
+ assert.equal(parseRsyncProgress(head), null);
+ // The tail alone is the dangerous one: rejoined wrongly it would be read as a
+ // frame about a handful of bytes.
+ const strayTail = parseRsyncProgress(tail);
+ if (strayTail !== null) {
+ assert.notEqual(strayTail.bytes, 30_000_000);
+ }
+ // Rejoined, it is the frame it always was.
+ assert.deepEqual(parseRsyncProgress(head + tail), {
+ bytes: 30_000_000,
+ percent: 85,
+ rate: "1.03GB/s",
+ etaSeconds: 3,
+ });
+});
diff --git a/common/lib/channelPriority.test.ts b/common/lib/channelPriority.test.ts
@@ -1306,3 +1306,73 @@ test("an auto-paused channel keeps its rank across a sanitize", () => {
undefined,
);
});
+
+// THE FULL ROUND TRIP: pause → sanitize → restore → sanitize gives back the
+// entry the operator wrote, in every field.
+//
+// The overrides half is the one that bit. `sanitizeOverrides` normalises away
+// any override equal to the BASE TIER — which is what keeps the document a
+// list of exceptions — and while the machine's pause stands the base on the
+// entry is the forced `paused`. Measured against that, every `{op:"paused"}`
+// fence the operator set stops being an exception and is deleted, so the
+// restore hands back a channel with no fence at all. On this corpus that is
+// legal-mindset losing both of its, and cornbreadman losing its sync fence to
+// one hiccup of a USB cable.
+test("an auto-pause round trip preserves tier, rank AND per-operation overrides", () => {
+ const written = sanitizeChannelPriority({
+ channels: {
+ "legal-mindset": {
+ tier: "normal",
+ rank: 7,
+ overrides: { sync: "paused", download: "paused" },
+ },
+ },
+ });
+ assert.deepEqual(written.channels["legal-mindset"], {
+ tier: "normal",
+ rank: 7,
+ overrides: { sync: "paused", download: "paused" },
+ });
+
+ // The drive goes away. The tier is forced, the fences are NOT touched.
+ const paused = sanitizeChannelPriority(
+ autoPauseForMedia(written, "legal-mindset", new Date("2026-09-20T00:00:00Z")),
+ );
+ assert.equal(paused.channels["legal-mindset"].tier, "paused");
+ assert.equal(paused.channels["legal-mindset"].rank, 7);
+ assert.deepEqual(paused.channels["legal-mindset"].overrides, {
+ sync: "paused",
+ download: "paused",
+ });
+ // Sanitizing twice must not erode it either — the document is written back
+ // to disk on every settings write.
+ assert.deepEqual(sanitizeChannelPriority(paused), paused);
+
+ // And it comes back exactly as it went in.
+ const restored = sanitizeChannelPriority(
+ restoreAfterMedia(paused, "legal-mindset"),
+ );
+ assert.deepEqual(restored.channels["legal-mindset"], {
+ tier: "normal",
+ rank: 7,
+ overrides: { sync: "paused", download: "paused" },
+ });
+});
+
+// The mirror case: an override equal to the RESTORED base is still normalised
+// away while auto-paused, because that is what it will be on restore.
+test("an override equal to the previous tier is still dropped while auto-paused", () => {
+ const paused = sanitizeChannelPriority(
+ autoPauseForMedia(
+ sanitizeChannelPriority({
+ channels: { a: { tier: "low", overrides: { sync: "low" } } },
+ }),
+ "a",
+ ),
+ );
+ assert.equal(paused.channels.a.overrides, undefined);
+ assert.equal(
+ sanitizeChannelPriority(restoreAfterMedia(paused, "a")).channels.a.tier,
+ "low",
+ );
+});
diff --git a/common/lib/channelPriority.ts b/common/lib/channelPriority.ts
@@ -266,10 +266,29 @@ export function sanitizeChannelPriority(value: unknown): ChannelPriority {
? raw.tier
: DEFAULT_CHANNEL_TIER;
const entry: ChannelPriorityEntry = { tier };
- const overrides = sanitizeOverrides(raw.overrides, tier);
- if (overrides) entry.overrides = overrides;
+ // AUTO-PAUSE FIRST, BECAUSE THE OVERRIDES ARE NORMALISED AGAINST THE BASE
+ // TIER — AND WHILE THE MACHINE'S PAUSE STANDS, `tier` IS NOT IT.
+ //
+ // `sanitizeOverrides` drops any override equal to the base, which is what
+ // keeps the document a list of exceptions. Measure that against the FORCED
+ // `paused` and every `{op: "paused"}` fence the operator set is an
+ // "exception" that is no longer an exception, so it is deleted — and the
+ // restore then hands back a channel with the fence gone. Reproduced:
+ // `{tier:"normal", overrides:{sync:"paused"}}` auto-paused and restored
+ // came back as a bare `{tier:"normal"}`. On this corpus that is
+ // legal-mindset losing both its fences, and cornbreadman — which lives on
+ // the platter with `overrides:{sync:"paused"}` — losing its sync fence to
+ // one hiccup of a USB cable.
+ //
+ // `previousTier` is the base the operator actually set, so that is what the
+ // exceptions are exceptions to.
const autoPaused = sanitizeAutoPause(raw.autoPaused, tier);
if (autoPaused) entry.autoPaused = autoPaused;
+ const overrides = sanitizeOverrides(
+ raw.overrides,
+ autoPaused ? autoPaused.previousTier : tier,
+ );
+ if (overrides) entry.overrides = overrides;
if (typeof raw.rank === "number" && Number.isFinite(raw.rank)) {
// RANK IS ONLY MEANINGFUL WHERE SOMETHING IS ORDERED. A channel that is
diff --git a/common/lib/savedVideo-server.ts b/common/lib/savedVideo-server.ts
@@ -15,6 +15,8 @@ import {
type SavedVideoKeepReason,
type SavedVideoPointer,
} from "./savedVideo";
+import { assertSavedVideosStoreWritable } from "./savedVideoStore";
+import type { Paths } from "./paths";
// Filesystem side of the saved-video store. See savedVideo.ts for the layout.
@@ -105,7 +107,18 @@ export async function persistSourceVideo(opts: {
sourceFilename: string;
storeDir: string;
keepReason?: SavedVideoKeepReason;
+ // WHEN THE STORE IS BEING MOVED, THIS DOES NOT RUN. Passed by the download
+ // path, which has them; a caller that omits them opts out, which is right for
+ // a store that is not the corpus one. See lib/savedVideoStore.ts for the
+ // three ways a write landing mid-move loses a container silently — the worst
+ // of them is `moveFileCrossDevice`'s unconditional `mkdir -p` recreating
+ // `saved-videos` as a real directory in the instant between the mover's
+ // rename and its symlink.
+ paths?: Pick<Paths, "transcriptsDir" | "savedVideosDir">;
}): Promise<SavedVideoPointer> {
+ if (opts.paths) {
+ await assertSavedVideosStoreWritable(opts.paths, opts.storeDir);
+ }
const src = path.join(opts.videoDir, opts.sourceFilename);
const dest = path.join(opts.storeDir, opts.sourceFilename);
const st = await stat(src);
@@ -143,9 +156,16 @@ async function pruneEmptyStoreDirs(storeDir: string): Promise<void> {
// Reverse persistSourceVideo: move the stored container back into the data dir
// and remove the pointer. Returns false when there was no pointer to reverse.
-export async function unpersistSavedVideo(videoDir: string): Promise<boolean> {
+export async function unpersistSavedVideo(
+ videoDir: string,
+ // Same opt-in guard as persistSourceVideo, and for the same window: this
+ // READS out of the store and then prunes its empty dirs, both of which race a
+ // move in flight.
+ paths?: Pick<Paths, "transcriptsDir" | "savedVideosDir">,
+): Promise<boolean> {
const pointer = await loadSavedVideo(videoDir);
if (!pointer) return false;
+ if (paths) await assertSavedVideosStoreWritable(paths, pointer.dir);
const src = savedVideoPath(pointer);
const dest = path.join(videoDir, pointer.file);
try {
diff --git a/common/lib/savedVideoStore.test.ts b/common/lib/savedVideoStore.test.ts
@@ -0,0 +1,168 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path from "node:path";
+import type { Paths } from "./paths";
+import {
+ assertSavedVideosStoreWritable,
+ isInCorpusSavedVideoStore,
+ readSavedVideosMarker,
+ savedVideosMarkerPath,
+ SavedVideosStoreInTransitionError,
+} from "./savedVideoStore";
+import { persistSourceVideo } from "./savedVideo-server";
+
+// Run with:
+// pnpm --filter yt-dlp-transcript-common exec tsx --test lib/savedVideoStore.test.ts
+//
+// THE GUARD AT THE MOMENT OF THE WRITE. The relocation queue key serialises
+// relocations against each other and says nothing about a download; this is
+// what actually stops a container landing in a store that is being moved — and
+// it has to live in lib/, because `savedVideo-server.ts` is what does the
+// landing and lib may not import controller.
+
+async function withTmp(
+ fn: (paths: Paths, videoDir: string) => Promise<void>,
+): Promise<void> {
+ const dir = await mkdtemp(path.join(tmpdir(), "ttb-store-guard-"));
+ const transcriptsDir = path.join(dir, "corpus");
+ const paths = {
+ transcriptsDir,
+ channelsDir: path.join(transcriptsDir, "channels"),
+ savedVideosDir: path.join(transcriptsDir, "saved-videos"),
+ } as Paths;
+ const videoDir = path.join(paths.channelsDir, "chan", "data", "vid1");
+ await mkdir(videoDir, { recursive: true });
+ await mkdir(paths.savedVideosDir, { recursive: true });
+ try {
+ await fn(paths, videoDir);
+ } finally {
+ await rm(dir, { recursive: true, force: true });
+ }
+}
+
+async function writeMarker(paths: Paths, phase = "copy"): Promise<void> {
+ await writeFile(
+ savedVideosMarkerPath(paths),
+ JSON.stringify({
+ target: "/mnt/platter/saved-videos",
+ direction: "out",
+ startedAt: new Date().toISOString(),
+ phase,
+ }),
+ );
+}
+
+test("no marker, no refusal", async () => {
+ await withTmp(async (paths) => {
+ assert.equal(await readSavedVideosMarker(paths), null);
+ await assertSavedVideosStoreWritable(paths, paths.savedVideosDir);
+ });
+});
+
+test("a marker refuses a write into the corpus store, by name", async () => {
+ await withTmp(async (paths) => {
+ await writeMarker(paths, "swap");
+ await assert.rejects(
+ assertSavedVideosStoreWritable(
+ paths,
+ path.join(paths.savedVideosDir, "chan", "vid1"),
+ ),
+ (err: Error) => {
+ assert.ok(err instanceof SavedVideosStoreInTransitionError);
+ assert.match(err.message, /being moved/);
+ assert.match(err.message, /phase "swap"/);
+ return true;
+ },
+ );
+ });
+});
+
+// A CHANNEL MAY POINT ITS OWN STORE SOMEWHERE ELSE ENTIRELY
+// (`ChannelConfig.savedVideosDir`), and that store is not the one being moved.
+// Refusing a write to it would decline a persist for a move that has nothing to
+// do with it.
+test("a store outside the corpus is not this move's business", async () => {
+ await withTmp(async (paths) => {
+ await writeMarker(paths);
+ assert.equal(
+ isInCorpusSavedVideoStore(paths, "/somewhere/else/chan/vid1"),
+ false,
+ );
+ await assertSavedVideosStoreWritable(paths, "/somewhere/else/chan/vid1");
+ });
+});
+
+// THE WHOLE POINT, end to end: a persist that fires while the store is being
+// moved does not move the container. The three ways it ends badly are in
+// savedVideoStore.ts's header; the worst is the container landing in the
+// directory the swap is about to park, which `reclaimParked` then rm -rf's
+// while `saved-video.json` still points at it.
+test("persistSourceVideo refuses while the store is in transition, and moves nothing", async () => {
+ await withTmp(async (paths, videoDir) => {
+ const source = "source-media.mp4";
+ await writeFile(path.join(videoDir, source), "the only copy");
+ const storeDir = path.join(paths.savedVideosDir, "chan", "vid1");
+ await writeMarker(paths);
+
+ await assert.rejects(
+ persistSourceVideo({
+ videoDir,
+ sourceFilename: source,
+ storeDir,
+ paths,
+ }),
+ /being moved/,
+ );
+ // The container is still where it was, with no pointer beside it and
+ // nothing created in the store.
+ assert.equal(
+ await readFile(path.join(videoDir, source), "utf8"),
+ "the only copy",
+ );
+ assert.equal(await exists(path.join(videoDir, "saved-video.json")), false);
+ assert.equal(await exists(storeDir), false);
+
+ // Clear the marker and the same call succeeds — the guard is the marker
+ // and nothing else.
+ await rm(savedVideosMarkerPath(paths), { force: true });
+ const pointer = await persistSourceVideo({
+ videoDir,
+ sourceFilename: source,
+ storeDir,
+ paths,
+ });
+ assert.equal(pointer.dir, storeDir);
+ assert.equal(
+ await readFile(path.join(storeDir, source), "utf8"),
+ "the only copy",
+ );
+ });
+});
+
+// A caller that passes no paths opts out. That is right for a store that is not
+// the corpus one, and it is what keeps this change from touching every existing
+// call site.
+test("persistSourceVideo without paths does not consult the marker", async () => {
+ await withTmp(async (paths, videoDir) => {
+ await writeMarker(paths);
+ await writeFile(path.join(videoDir, "source-media.mp4"), "x");
+ const storeDir = path.join(paths.savedVideosDir, "chan", "vid1");
+ const pointer = await persistSourceVideo({
+ videoDir,
+ sourceFilename: "source-media.mp4",
+ storeDir,
+ });
+ assert.equal(pointer.file, "source-media.mp4");
+ });
+});
+
+async function exists(p: string): Promise<boolean> {
+ try {
+ await readFile(p);
+ return true;
+ } catch (err) {
+ return (err as NodeJS.ErrnoException).code === "EISDIR";
+ }
+}
diff --git a/common/lib/savedVideoStore.ts b/common/lib/savedVideoStore.ts
@@ -0,0 +1,115 @@
+import path from "node:path";
+import { readFile } from "node:fs/promises";
+import type { Paths } from "./paths";
+import type { RelocationMarker } from "./channelMedia";
+
+// IS THE SAVED-VIDEO STORE SAFE TO WRITE INTO RIGHT NOW?
+//
+// The store moves to another drive the same way a channel's `data/` does, and
+// it inherits the same hazard: for the length of a multi-hour copy the bytes at
+// `transcripts/saved-videos` are being read, verified and then swapped, and
+// anything that writes into them in the meantime is one of three failures, all
+// of them silent until much later.
+//
+// 1. A container landing mid-copy makes `verifyCopy` refuse AT THE END of the
+// whole transfer — the omnimirror incident, exactly, one directory down.
+// 2. A container landing between the verify and the rename lands in the dir
+// the swap is about to PARK, and `reclaimParked` then `rm -rf`s it while
+// the video's `saved-video.json` still points at it. That is the only copy
+// of a source container, gone, with a pointer that outlives it.
+// 3. `moveFileCrossDevice` does an unconditional `mkdir -p` of the
+// destination's parent, so a persist firing between the `rename` and the
+// `symlink` RECREATES `saved-videos` as a real directory — the symlink is
+// then never made, and the move would have recorded the store as being on
+// a location it cannot be reached at.
+//
+// THIS MODULE IS lib/, AND IT HAS TO BE. `common/lib/savedVideo-server.ts` is
+// what persists a container, it is lib, and lib may not import controller
+// (architecture.test.ts) — so the marker's name and its reader live here, next
+// to `channelMedia.ts`, which is the same module for the same reason about a
+// channel. The controller that WRITES the marker imports these; it does not own
+// them.
+//
+// The marker's shape is deliberately a channel relocation marker
+// (`{target, direction, startedAt, phase}`) — one mover writes both, and a
+// second shape would be a second thing to keep in step.
+
+export const SAVED_VIDEOS_DIRNAME = "saved-videos";
+export const SAVED_VIDEOS_MARKER_FILENAME = ".relocating-saved-videos.json";
+
+export function savedVideosMarkerPath(
+ paths: Pick<Paths, "transcriptsDir">,
+): string {
+ return path.join(paths.transcriptsDir, SAVED_VIDEOS_MARKER_FILENAME);
+}
+
+// The store's home on a location: `<root>/saved-videos`. Flat, beside the
+// channels' `<slug>/data` dirs, and not configurable for the same reason
+// `relocatedDataDir` is not — a mover recognises a target by its shape.
+export function relocatedSavedVideosDir(root: string): string {
+ return path.join(root.trim(), SAVED_VIDEOS_DIRNAME);
+}
+
+export async function readSavedVideosMarker(
+ paths: Pick<Paths, "transcriptsDir">,
+): Promise<RelocationMarker | null> {
+ try {
+ const raw = JSON.parse(
+ await readFile(savedVideosMarkerPath(paths), "utf8"),
+ ) as Partial<RelocationMarker>;
+ if (typeof raw.target !== "string" || raw.target.trim() === "") return null;
+ return {
+ target: raw.target,
+ direction: raw.direction === "back" ? "back" : "out",
+ startedAt: typeof raw.startedAt === "string" ? raw.startedAt : "",
+ phase:
+ raw.phase === "swap" || raw.phase === "reclaim" ? raw.phase : "copy",
+ };
+ } catch {
+ return null;
+ }
+}
+
+// Thrown by assertSavedVideosStoreWritable. A distinct class so a caller can
+// tell "the store is being moved" from any other I/O failure and decline the
+// persist rather than failing the whole download.
+export class SavedVideosStoreInTransitionError extends Error {
+ readonly marker: RelocationMarker;
+ constructor(marker: RelocationMarker) {
+ super(
+ `The saved-video store is being moved (${marker.direction} to ` +
+ `${marker.target}, phase "${marker.phase}") — nothing may be written ` +
+ `into it until that finishes or its marker is cleared on /storage.`,
+ );
+ this.name = "SavedVideosStoreInTransitionError";
+ this.marker = marker;
+ }
+}
+
+// Is `dir` the corpus store, or inside it? A channel may point its own store
+// somewhere else entirely (`ChannelConfig.savedVideosDir`), and that store is
+// not the one being moved — guarding a write to it would refuse a persist for a
+// move that has nothing to do with it.
+export function isInCorpusSavedVideoStore(
+ paths: Pick<Paths, "savedVideosDir">,
+ dir: string,
+): boolean {
+ const rel = path.relative(paths.savedVideosDir, dir);
+ return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel));
+}
+
+// THE GUARD, at the moment of the write. One `readFile` of a small JSON file
+// per persisted container — the same cost `inspectChannelMedia` pays per lane
+// tick, and a persist happens once per kept video, not once per tick.
+//
+// It is checked AT THE WRITE and not only at the start of the download because
+// the window is the whole copy: a download that began before the move started
+// is exactly the one that would land a container in the parked directory.
+export async function assertSavedVideosStoreWritable(
+ paths: Pick<Paths, "transcriptsDir" | "savedVideosDir">,
+ storeDir: string,
+): Promise<void> {
+ if (!isInCorpusSavedVideoStore(paths, storeDir)) return;
+ const marker = await readSavedVideosMarker(paths);
+ if (marker) throw new SavedVideosStoreInTransitionError(marker);
+}
diff --git a/common/views/storage.ts b/common/views/storage.ts
@@ -325,7 +325,7 @@ export function buildStorageRows(i: StorageRowsInputs): StorageRowsPayload {
offered: !busy,
...(busy ? { withheld: busy } : {}),
},
- deleteAction(loc, counts, busy),
+ deleteAction(loc, counts, busy, i.savedVideos?.locationId === loc.id),
];
const roll = i.rollups[loc.id];
@@ -561,10 +561,23 @@ function deleteAction(
loc: StorageLocation,
counts: StorageChannelCounts,
busy: string | null,
+ // The saved-video store lives here. A resident like any other, and orphaned
+ // by exactly the same delete — see the action's own refusal.
+ hasStore: boolean,
): StorageActionView {
if (busy) {
return { kind: "delete", label: "Delete", offered: false, withheld: busy };
}
+ if (hasStore) {
+ return {
+ kind: "delete",
+ label: "Delete",
+ offered: false,
+ withheld:
+ `The saved-video store is under ${loc.root}. Move it back in place, ` +
+ `or onto another location, first.`,
+ };
+ }
if (counts.total > 0) {
return {
kind: "delete",
diff --git a/common/ytdlp/downloadOneManaged.ts b/common/ytdlp/downloadOneManaged.ts
@@ -262,6 +262,12 @@ async function finalizeAppExtraction(opts: {
sourceFilename: source,
storeDir,
keepReason: opts.category === "none" ? undefined : opts.category,
+ // The store-in-transition guard. A move of the saved-video store is a
+ // multi-hour copy, and a container landing in the middle of it is lost
+ // three different ways — see lib/savedVideoStore.ts. The catch below is
+ // already the right handling: the container stays in the data dir with a
+ // line in the log, and the next persist (after the move) picks it up.
+ paths: opts.paths,
});
opts.onLog(
`Persisted source video to ${path.join(pointer.dir, pointer.file)} (${pointer.bytes} bytes).\n`,
diff --git a/editor/app/storage/actions.ts b/editor/app/storage/actions.ts
@@ -28,6 +28,7 @@ import { savedVideosMarkerPath } from "yt-dlp-transcript-common/controller/reloc
import { channelMediaBusyReason } from "../channels/lib/mediaBusy";
import { enqueueRepointJob } from "./lib/repointJob";
import { enqueueSavedVideosRelocation } from "./lib/savedVideosJob";
+import { savedVideosStoreBusyReason } from "./lib/storeBusy";
// THE SIX THINGS AN OPERATOR MAY DO TO A STORAGE LOCATION.
//
@@ -179,6 +180,22 @@ export async function deleteStorageLocationAction(
`Move them back in place, or onto another location, first.`,
};
}
+ // AND THE SAVED-VIDEO STORE IS A RESIDENT TOO. The channel check above exists
+ // because deleting the location erases the only record of which disk those
+ // absolute paths belong to; the store is on the location by exactly the same
+ // kind of record (`settings.storage.savedVideosLocationId`) and would be
+ // orphaned by exactly the same delete — reachable only through a symlink
+ // whose target nothing in the corpus can any longer name.
+ if ((settings.storage.savedVideosLocationId ?? "") === id) {
+ return {
+ ok: false,
+ error:
+ `The saved-video store is on ${location.root}. Move it back in place ` +
+ `(or onto another location) on this page first — deleting the location ` +
+ `would leave the store reachable only through a symlink nothing here ` +
+ `remembers the name of.`,
+ };
+ }
const locations = settings.storage.locations.filter((l) => l.id !== id);
await writeSettings({
...settings,
@@ -301,6 +318,16 @@ export async function repointStorageLocationAction(
export async function relocateSavedVideosAction(
locationId: string,
): Promise<StreamActionResult> {
+ // THE RELOCATION QUEUE KEY ONLY SERIALISES RELOCATIONS. It stops a second
+ // move, a channel move and a re-point from running at once, and it says
+ // nothing at all about the download that is about to persist a source
+ // container into the directory this is about to copy, verify, rename and
+ // reclaim. See lib/storeBusy.ts for the three ways that ends badly, and
+ // lib/savedVideoStore.ts for the guard that makes it refuse rather than lose
+ // a container — this is the half that answers before the operator commits to
+ // a multi-hour copy.
+ const busy = savedVideosStoreBusyReason("moving the saved-video store");
+ if (busy) return { ok: false, error: busy };
return enqueueSavedVideosRelocation({ locationId });
}
@@ -321,6 +348,8 @@ export async function resumeSavedVideosRelocationAction(): Promise<StreamActionR
"interrupted move to resume.",
};
}
+ const busy = savedVideosStoreBusyReason("resuming the store's move");
+ if (busy) return { ok: false, error: busy };
if (marker.direction === "back") {
return enqueueSavedVideosRelocation({ locationId: "" });
}
diff --git a/editor/app/storage/lib/storeBusy.ts b/editor/app/storage/lib/storeBusy.ts
@@ -0,0 +1,80 @@
+import { getRegistry } from "yt-dlp-transcript-common/jobs/registry";
+import { getAutoRunnerStatus } from "yt-dlp-transcript-common/controller/autoRunner";
+
+// IS ANYTHING WRITING INTO THE SAVED-VIDEO STORE RIGHT NOW?
+//
+// `channelMediaBusyReason` asks the same question about one channel's `data/`.
+// This is its twin for the store, and the difference is the one that matters:
+// THE STORE HAS NO SLUG. A persist belongs to a video, the video belongs to a
+// channel, and the container lands in the ONE store — so "is this channel
+// busy" answers nothing here and the question has to be asked of the whole
+// machine.
+//
+// WHY A BUSY CHECK AT ALL WHEN THE MARKER ALREADY REFUSES THE WRITE.
+// `assertSavedVideosStoreWritable` (lib/savedVideoStore.ts) is the guard: it
+// runs at the moment of the persist, it covers every caller including ones that
+// do not exist yet, and it is what makes a raced write impossible rather than
+// merely unlikely. What it CANNOT do is give the operator the answer before
+// they commit — a download that starts a persist thirty seconds into a
+// three-hour copy is refused correctly, and the operator finds out from a job
+// log. So this is the courtesy half: a sentence, before the move, naming what
+// is running.
+//
+// THE KIND LIST IS DELIBERATELY NOT EXHAUSTIVE, and that is safe precisely
+// because the marker is the guard. It names the kinds that can reach
+// `persistSourceVideo` / the store today; a kind that escapes it costs a
+// refused persist (logged, container left in the data dir, picked up next
+// time), not a lost container.
+//
+// SERVER-SIDE ONLY: it reaches the controller, which imports execa
+// transitively. `next build` proves nothing client-side imports it.
+
+// Everything that runs `downloadOneManaged` (which persists a kept source
+// container at the end of a download), plus the four kinds whose whole subject
+// is the store.
+const STORE_TOUCHING_KINDS = new Set([
+ // Downloads, every entry point.
+ "auto-download",
+ "auto-download-unit",
+ "download-from-playlist",
+ "download-missing",
+ "download-missing-subs",
+ "import-one",
+ "redownload-archive",
+ "redownload-incomplete-bucket",
+ "retry-bucket",
+ // One kind, channel-scoped: a sync downloads.
+ "sync",
+ // The store's own jobs.
+ "persist-kept",
+ "check-kept-deleted",
+ "backup-saved-videos",
+ "verify-saved-video-backup",
+]);
+
+// A sentence naming what is holding the store, or null. The caller supplies the
+// verb, so the same reason reads as an instruction wherever it appears.
+export function savedVideosStoreBusyReason(what?: string): string | null {
+ const jobs = getRegistry()
+ .list()
+ .filter(
+ (j) =>
+ STORE_TOUCHING_KINDS.has(j.kind) &&
+ (j.status === "running" || j.status === "queued"),
+ );
+ // THE DOWNLOAD LANE'S UNITS MAKE NO JOB RECORD — the omnimirror lesson, and
+ // the reason `channelMediaBusyReason` exists in the shape it does. A unit
+ // that is mid-download is a persist that has not happened yet. No slug
+ // filter: every one of them writes into the same store.
+ const units = getAutoRunnerStatus("download").inFlight.length;
+ if (jobs.length === 0 && units === 0) return null;
+
+ const parts: string[] = [];
+ if (jobs.length > 0) {
+ const kinds = [...new Set(jobs.map((j) => j.kind))].slice(0, 3).join(", ");
+ parts.push(`${jobs.length} running/queued download job(s) (${kinds})`);
+ }
+ if (units > 0) parts.push(`${units} auto-download unit(s) in flight`);
+ const subject = `${parts.join(" and ")} — any of them can persist a source video into the store`;
+ return what ? `Finish or cancel ${subject}, before ${what}.` : subject;
+}
