commit fb17b672efcab5d1dd3e7b247f83ce172fa42e7b
parent 94a9aa620d54b2ac7a9fc3f77c0a75bdfcf1cbab
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sat, 26 Sep 2026 03:50:33 -0400
plans: L2 review fixes — hung-mount wording, rate-limited prefetch exit, availability-check stop with the pre-clean guard
release-10.md, slice L2: item 7's "when it fires" text corrected (a hung
mount holds the re-queue behind it), found and left gains the hung-mount
re-queue, the old-yt-dlp listing fallback, manual batches stopping at a
persistently soft-blocked video and two pre-existing observations, and a
"Review fixes" paragraph with its commit table, the tests and the proof they
bite, and the re-gate (tsc; common 1,933, editor unit 85, scripts 162 + 1
skip, mcp 219; editor build; e2e 46 specs 256/256, 16.2 min).
FACTS: the item 6 bullet names the prefetch exit, the check's stop and the
verifyBeforeClean guard. editor/CHANGELOG.md [Unreleased]: the soft-block
bullet says a rate limit now ends a download and the availability check a
request sooner and the cleanup keeps what the check did not reach; the boot
bullet says a re-queue on the hung drive still waits for it.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Diffstat:
3 files changed, 103 insertions(+), 15 deletions(-)
diff --git a/editor/CHANGELOG.md b/editor/CHANGELOG.md
@@ -1,8 +1,8 @@
# Changelog
## [Unreleased]
-- **YouTube's "try again later" block now backs off instead of reading as a deleted video.** When YouTube rate-limits a session it answers "This content isn't available, try again later." The editor read that as a removed video: the download moved straight on to the next video (into the same block), no cooldown was recorded, and the video was set aside as deleted, so later download runs skipped it. It is now handled like an HTTP 429. A download stops its batch and records the platform cooldown that the auto-download runner and Sync honour, and the runner defers the video for 6 hours. The availability check records a temporary error instead of "deleted". A metadata scan stops (retrying once with cookies when the channel has them) instead of recording the video as gone. Videos that really are gone ("Video unavailable", removed by the uploader, a terminated account, Rumble's 410) are still recorded as deleted.
-- **Jobs a restart left queued are settled even when a drive hangs.** At boot the editor settles those jobs after it has checked where its storage locations are. A hung network mount could stall that check forever, and the jobs then stayed "queued" on `/jobs`. The settling now waits at most 60 seconds, logs `[boot] storage pass still running after 60 s …` and carries on. The storage check keeps running and logs when it ends.
+- **YouTube's "try again later" block now backs off instead of reading as a deleted video.** When YouTube rate-limits a session it answers "This content isn't available, try again later." The editor read that as a removed video: the download moved straight on to the next video (into the same block), no cooldown was recorded, and the video was set aside as deleted, so later download runs skipped it. It is now handled like an HTTP 429. A download stops its batch and records the platform cooldown that the auto-download runner and Sync honour, and the runner defers the video for 6 hours. A metadata scan stops (retrying once with cookies when the channel has them) instead of recording the video as gone. The availability check records a temporary error instead of "deleted", and stops probing for the rest of that run. Every rate limit also ends things sooner now: a download whose metadata request is refused no longer goes on to try the download itself, and the availability check stops at the first refused probe instead of asking about every remaining video. When the check that runs before cleaning audio is cut short this way, the cleanup keeps the audio of every video it did not reach, rather than trusting what an older check said about it. Videos that really are gone ("Video unavailable", removed by the uploader, a terminated account, Rumble's 410) are still recorded as deleted.
+- **Jobs a restart left queued are settled even when a drive hangs.** At boot the editor settles those jobs after it has checked where its storage locations are. A hung network mount could stall that check forever, and the jobs then stayed "queued" on `/jobs`. The settling now waits at most 60 seconds, logs `[boot] storage pass still running after 60 s …` and carries on. The storage check keeps running and logs when it ends. A job re-queued for a channel on the hung drive itself still waits for that drive, and any re-queued after it wait too.
- **`/jobs` says why a job was cancelled at boot.** A job the boot settled shows its reason under its status on `/jobs` and as *Cancelled because* on its own page: for example "server restarted; the scheduler re-derives syncs" or "superseded by a newer queued job (…)". The reason used to be only in the job's log.
- **The server log says how often a queued job skips its page refresh.** When a queued job finishes outside any request, the editor skips its page refresh and notes it in the log. The note used to appear once and never again. Now the first one after a quiet spell is logged at once, any more in the next 10 minutes are counted, and one line at the end gives the count, with a running total.
diff --git a/plans/FACTS.md b/plans/FACTS.md
@@ -6432,7 +6432,12 @@ Line numbers are `plans/FACTS.md` lines at `e172749b`, before this record's in-p
try again later", either apostrophe) is checked first: `parseUnavailableFromStderr` → `error`,
`classifyDownloadFailure` → `rate_limit` even over a per-video class. The batch records the
platform cooldown and aborts; the metadata scan stops as a `soft-block` block. Every failure
- still sleeps. No live sidecar held the string (2026-09-26 read-only grep of 136,401
+ still sleeps. **L2 review fixes:** a metadata prefetch that classifies `rate_limit` ends
+ `downloadOneManaged` there (no attempt 1; `writeOutcome` writes the sidecar for both exits), and
+ `runAvailabilityCheck` stops at its first `rate_limit` probe (`blocked`, `probedIds`, one
+ `recordDownloadBackoff`). **`verifyBeforeClean` leaves every tier C suspect a blocked check did not
+ probe `unverified`** — never judged on its old `availability.json`, which is what would clear an
+ irreversible delete. No live sidecar held the string (2026-09-26 read-only grep of 136,401
`availability.json`/`download-outcome.json` and every `metadata-scan.json`), but an availability
check stores no text for `deleted`, so an earlier misread there cannot be found after the fact.
- **A job record carries `progressAt`** (live-only, stamped by the one progress writer and by task
diff --git a/plans/release-10.md b/plans/release-10.md
@@ -271,11 +271,17 @@ then settles anyway; `instrumentation.ts` calls it in place of `storagePass.then
covers several locations at their worst; past it the pass is stuck on a syscall, and waiting buys
nothing.
- **When it fires:** `[boot] storage pass still running after 60 s; settling queued jobs without it
- (a hung mount? a job re-queued for a channel it has not re-pointed yet will be refused as
- unreachable, and /jobs will say so)`, and, when the pass finally ends, `[boot] storage pass
- finished N s after the queued-job pass began waiting (it stopped waiting at 60 s)`. The race is
- over a derived promise, so the storage pass is never cancelled and a re-point it enqueued runs to
- the end. A pass that throws counts as finished.
+ (a hung mount? a job re-queued for a channel on it will wait on the same mount, and the re-queues
+ after it with it)`, and, when the pass finally ends, `[boot] storage pass finished N s after the
+ queued-job pass began waiting (it stopped waiting at 60 s)`. The race is over a derived promise,
+ so the storage pass is never cancelled and a re-point it enqueued runs to the end. A pass that
+ throws counts as finished.
+- **What a re-queue then meets** (corrected in review; as first shipped this said every such job is
+ refused at once). For an UNMOUNTED drive, the media guard's `stat` gets ENOENT at once, so the job
+ is refused — at submission, closing the old meta `cancelled` with the error, or when it starts —
+ and `/jobs` says so. For a HUNG mount, the case the bound exists for, the guard's own `stat`
+ (`lib/channelMedia.ts`) hangs on the same syscall; re-queues run one at a time, so the ones after
+ it stay `queued` until the mount answers. Every cancel has run by then. Left: see below.
**8 — `/jobs` shows `cancelReason`.** `JobListEntry.cancelReason` is read from the meta only when its
status is `cancelled`; `fromEntry` copies it to `JobRowView.cancelReason`. `JobRow.tsx` draws it
@@ -303,7 +309,7 @@ costs, and nothing reads that log.
| `904ffc4e` | 8: `cancelReason` through `listJobs` → `fromEntry` → `JobRow` and the job page; `listJobs.test.ts` +1, `jobRows.test.ts` +1, `jobs-filters.spec.ts` +1 |
| `11446fba` | 9: `SkipReporter`, one per process; `safeRevalidate.test.ts` 3 → 9 |
| `fc2ce63c` | 8: a card row's reason at the end of its heading, not beside the pill |
-| _this_ | `plans:` this record, FACTS (three release 9 bullets amended), the editor `[Unreleased]` bullets |
+| `5b657aeb` | `plans:` this record, FACTS (three release 9 bullets amended), the editor `[Unreleased]` bullets |
**Gates**, all from the worktree root.
- **tsc** (`pnpm -r --no-bail --workspace-concurrency=1 exec tsc --noEmit`) clean before every code
@@ -345,12 +351,20 @@ costs, and nothing reads that log.
- **Numbers tools: none.**
**Found and left.**
-- **A soft-blocked prefetch still makes the primary attempt.** `downloadOneManaged` goes on to attempt
- 1 after any failed metadata prefetch, whatever the class (a 429 and a deleted video alike): one more
- request into the block before the batch stops. Stopping there would back off more, but it changes
- the per-video flow for every class, so it is not in this slice.
-- **The availability check has no rate-limit stop.** It now records the soft block as `error` (was
- `deleted`), but it keeps probing the rest of its list. It has no cooldown path to plug into.
+- ~~A soft-blocked prefetch still makes the primary attempt~~ and ~~the availability check has no
+ rate-limit stop~~: both done in the review fixes, below.
+- **A hung mount can hold the boot pass's re-queues.** After the 60 s timeout, a job re-queued for a
+ channel on a hung mount waits on the media guard's `stat`, and the re-queues after it wait with it
+ (above). Cancels are unaffected, and re-queues are few (one at the first live boot). A bound on the
+ guard's `stat`, or re-queues that do not wait on each other, would fix it.
+- **An old yt-dlp's soft-block line on a channel listing.** A flat-playlist listing that fails with
+ the bare pre-#12958 line now classifies `rate_limit`, so `enumeratePlaylistUrls` throws
+ `EnumerationIncompleteError` (`runYtdlp.ts:420`) and a full sweep falls back to its paged walk (one
+ more page request). The current yt-dlp's longer line took that path before this slice (it matches
+ `/rate-limit/`), and a flat listing never reaches the playability check that prints it.
+- **Manual batches have no per-video deferral.** A video that answers "try again later" every time
+ stops every manual download-missing at itself, on every run; the auto runner defers it 6 h
+ instead. It errs toward backing off.
- **Earlier misreads cannot be found.** An availability check stores no stderr for a `deleted`
result, so a soft block it read before this release is indistinguishable from a real removal. None
shows in the download outcomes or scan stores (above).
@@ -359,6 +373,75 @@ costs, and nothing reads that log.
`managedDownloadsSleep.test.ts`, which feeds the real line through the classifier into the batch
loop.
- `plans/STATE.md` still lists items 6–9 as open; it is shared with L1, so the merge updates it.
+- **Seen while fixing, pre-existing, not changed:** an unblocked tier C check skips a suspect with no
+ `webpage_url` in its metadata, and `verifyBeforeClean` then judges it on its old record (the
+ blocked case is now guarded; this one is as before). And `resolveMaybeMissingState` reads an
+ `error` probe newer than the scan as `available` (`stateFromAvailability`'s default), so a
+ maybe-missing video whose confirm probe failed stops being flagged.
+
+**Review fixes (review verdict SHIP AFTER FIXES, `l2-review.md`).**
+
+- **Item 7's wording (should-fix).** The comment, the timeout line and this record promised that a job
+ re-queued for a channel the storage pass had not reached is refused at once. True for an unmounted
+ drive; not for a hung mount, where the media guard's own `stat` hangs too and the sequential
+ re-queues behind it stay `queued`. Corrected in all three; listed under found and left. No
+ behaviour change.
+- **A rate-limited prefetch ends the download (question a).** In `runManagedDownload`, after the
+ prefetch's auth retry: when the last prefetch attempt classifies `rate_limit`, one log line
+ (`Metadata prefetch for <id> was rate-limited by the source; not attempting the download …`) and
+ the record `failed` / `rate_limit` with only the prefetch attempt(s). The sidecar and
+ availability-history tail is one helper, `writeOutcome`, which both exits call. Every other failure
+ class keeps today's flow. The batch's cooldown and abort, and the runner's deferral, follow from the
+ record unchanged.
+- **The availability check stops on a rate limit (question b), with the data-loss guard in the same
+ commit.**
+ - `runAvailabilityCheck`: the first probe that classifies `rate_limit` records its own `error`
+ and sets `blocked`. No further probe starts; the rest count as `skipped`, and a `STOPPED: …` line
+ says how many. After the pass the platform cooldown is recorded once, best-effort, with
+ `recordDownloadBackoff(detectPlatform(config?.url ?? url) ?? "unknown", paths)`, the key the batch
+ and the Sync gate use (`onPlatformBackoff` is the test seam). The result gains `blocked`,
+ `blockMessage` and `probedIds`.
+ - **`verifyBeforeClean`: when the tier C check was blocked, every suspect it did not probe is
+ `unverified`.** Judged on its old `availability.json` (a months-old `public`), it would have been
+ cleared for an irreversible delete. An unblocked check is judged exactly as before.
+ - The other callers are unaffected, as the review said. `checkKeptDeleted` only pins, and an unprobed
+ id keeps its old verdict (pinning is the safe direction). The full-sweep confirm leaves an
+ unprobed id `maybe_missing`, because its probe time is older than the scan's.
+- **Two found-and-left lines** (above): an old yt-dlp's soft-block line on a flat listing, and manual
+ batches stopping at a persistently soft-blocked video.
+
+| sha | what |
+|---|---|
+| `61c03ac1` | (a) the rate-limited prefetch exit, `writeOutcome`; `prefetchRateLimit.test.ts` (4) |
+| `24e0f7d5` | item 7's comment and timeout line say what a hung mount does |
+| `b8053655` | (b) the availability check's stop + cooldown + `probedIds`; `verifyBeforeClean` leaves unprobed suspects unverified; `checkAvailability.test.ts` (3), `verifyBeforeClean.test.ts` +2 |
+| _this_ | `plans:` this paragraph, the item 7 record text, found and left, FACTS, the `[Unreleased]` bullets |
+
+**Tests, and the proof they bite.**
+- `prefetchRateLimit.test.ts` drives the real `downloadOneManaged` against a temp yt-dlp script that
+ counts its spawns. A soft block gives 1 spawn, 1 attempt and a `rate_limit` sidecar, and a 429 is
+ the same. A 403 still reaches the primary (2 spawns, `network`), and so does a removed video (2,
+ `per_video`). With the exit disabled, the soft-block and 429 cases fail.
+- `checkAvailability.test.ts` covers four probeable videos:
+ - a soft block gives 1 spawn, 3 skipped, one `error` written and one youtube cooldown;
+ - a 429 and the bot check behave the same;
+ - a 403 and a removed video probe all four, with no cooldown.
+- `verifyBeforeClean.test.ts` runs through the real quick check and tier C:
+ - a blocked confirmation leaves all 3 suspects `unverified`, although two carry a stale `public`;
+ the listed video stays cleanable, and the cooldown lands in the state file;
+ - an unblocked one probes all 3 and judges `public` / `deleted` / 403 as before;
+ - with the guard disabled, the blocked case fails.
+
+**Re-gate on `b8053655`:**
+- tsc clean before each fix commit (`l2-tsc-6..8.log`).
+- Unit tests (`l2-units-2.log`): common **1,933/1,933** (1,924 + 9), editor unit **85/85**, `test:scripts`
+ **162 + 1 skip of 163**, mcp **219/219**.
+- Editor `next build` ok, 43 s (`l2-build-3.log`).
+- EDITOR e2e (`l2-e2e-2.log`): the 37 specs plus 9 cleanup / availability specs (`cleanup-actionable
+ cleanup-holds cleanup-page do-not-clean shard cookies-mode whisper channels-actions
+ auto-subs-replace`; `pre-clean-availability`, `availability`, `availability-backfill` and
+ `maybe-missing` were already in the list). **256 passed, 0 failed, 16.2 min**, no queue wait.
+- The primary's `export/public/sw.js` was untouched again.
## Rollout
