commit e23c174a7bea0728160a77e3250b03d044b0c0ef
parent 0856b35b90bd7e9864517ec1049df99731d131b5
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Thu, 24 Sep 2026 19:21:06 -0400
common: slice W review fixes
The mode-at-creation test intercepts the rename and stats the temp, so
it cannot pass vacuously (it fails with mode dropped from the writer).
Record: 188 orphan temps in the heading; the roster writers' writes
serialise but a load-merge-write is not locked; transcribeOne.ts:173's
direct remote-transcript write listed as out of scope. Changelog: no
"every file" — names the four temp names that stay.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
3 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/common/lib/jsonFile-server.test.ts b/common/lib/jsonFile-server.test.ts
@@ -2,6 +2,7 @@ import { test } from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import { mkdtemp, readFile, readdir, writeFile } from "node:fs/promises";
+import { createRequire, syncBuiltinESMExports } from "node:module";
import os from "node:os";
import path from "node:path";
import {
@@ -17,6 +18,8 @@ import {
writeJsonAtomicSync,
} from "./jsonFile-server";
+const require = createRequire(import.meta.url);
+
async function scratch(): Promise<string> {
return mkdtemp(path.join(os.tmpdir(), "jsonfile-"));
}
@@ -146,22 +149,24 @@ test("writeFileAtomic: mode is on the file from creation (0o600 cookie jar)", as
const file = path.join(dir, "cookies.txt");
await writeFileAtomic(file, "secret\n", { mode: 0o600 });
assert.equal(fs.statSync(file).mode & 0o777, 0o600);
- // The temp itself is created with the mode: watch every entry while a write
- // is in flight behind a held one.
+ // The temp itself carries the mode BEFORE the rename: intercept the rename
+ // (the module calls fs/promises' `rename`, so patch it and sync the builtin
+ // ESM exports) and stat its source — the temp — at that moment.
+ const fsp = require("node:fs/promises") as typeof import("node:fs/promises");
+ const realRename = fsp.rename;
const seen: number[] = [];
- const hold = writeFileAtomic(file, "x".repeat(1 << 20), { mode: 0o600 });
- const second = writeFileAtomic(file, "second\n", { mode: 0o600 });
- const watcher = fs.watch(dir, (_e, name) => {
- if (!name || !name.includes(".tmp-")) return;
- try {
- seen.push(fs.statSync(path.join(dir, name)).mode & 0o777);
- } catch {
- // renamed away already
- }
- });
- await Promise.all([hold, second]);
- watcher.close();
- for (const m of seen) assert.equal(m, 0o600);
+ fsp.rename = (async (from: fs.PathLike, to: fs.PathLike) => {
+ seen.push(fs.statSync(from).mode & 0o777);
+ return realRename(from, to);
+ }) as typeof fsp.rename;
+ syncBuiltinESMExports();
+ try {
+ await writeFileAtomic(file, "second\n", { mode: 0o600 });
+ } finally {
+ fsp.rename = realRename;
+ syncBuiltinESMExports();
+ }
+ assert.deepEqual(seen, [0o600], "the rename saw exactly one temp, already 0o600");
assert.equal(await readFile(file, "utf8"), "second\n");
});
diff --git a/editor/CHANGELOG.md b/editor/CHANGELOG.md
@@ -1,7 +1,7 @@
# Changelog
## [Unreleased]
-- **Every file the editor replaces atomically is now written one way, and a failed write no longer leaves a temp file behind.** Twenty-six places wrote a file by writing `<file>.tmp-<pid>` and renaming it over the original — the channel roster, maybe-missing and metadata-scan records, the scheduler and auto-queue state, worker defaults, widget presets, the homepage config, relocation markers, shard configs, the duplicate and media-scan reports and their review decisions, the saved-video backup manifest, both cue normalizers, the playlist, the failed-transcriptions list, the X cookie jar, a site's CHANGELOG cut, a saved video copied into its store across drives, and the video page's VTT promote and remark. They now all go through one writer (`common/lib/jsonFile-server.ts`), which gives every write its own temp name and queues writes to the same file one behind another, so two jobs touching one channel's roster at once cannot trip over each other's temp file. A write that fails now removes its temp: the live `.auto-queue/` holds 175 `state.json.tmp-…` files (173 of them empty) from the day `/home` filled up (2026-09-11), each one a failed write the old code left behind; nothing deletes those old ones for you — `find transcripts -name '*.tmp-*'` lists them. No file's contents change — every writer puts the same bytes on disk it did before, measured over the live corpus. The cookie jar is still created readable only by you.
+- **The editor's atomic JSON, text and binary writes now go one way, and a failed write no longer leaves a temp file behind.** Nineteen JSON write sites and seven text and binary ones each wrote `<file>.tmp-<pid>` and renamed it over the original — the channel roster, maybe-missing and metadata-scan records, the scheduler and auto-queue state, worker defaults, widget presets, the homepage config, relocation markers, shard configs, the duplicate and media-scan reports and their review decisions, the saved-video backup manifest, both cue normalizers, the playlist, the failed-transcriptions list, the X cookie jar, a site's CHANGELOG cut, a saved video copied into its store across drives, and the video page's VTT promote and remark. They now all go through one writer (`common/lib/jsonFile-server.ts`), which gives every write its own temp name and queues writes to the same file one behind another, so two jobs touching one channel's roster at once cannot trip over each other's temp file. A write that fails now removes its temp: the live `.auto-queue/` holds 175 `state.json.tmp-…` files (173 of them empty) from the day `/home` filled up (2026-09-11), each one a failed write the old code left behind; nothing deletes those old ones for you — `find transcripts -name '*.tmp-*'` lists them. Four temp names stay, on purpose: the export build's two page writers `buildIndex.ts` (a streaming page writer) and `buildStats.ts` (a hand-joined array) — folding them is a restructuring, not a swap — and `transcode.ts` / `transcribeOne.ts` name the output file ffmpeg or the transcription app writes, which is not our write to fold. No file's contents change — every writer puts the same bytes on disk it did before, measured over the live corpus. The cookie jar is still created readable only by you.
- **Every channel table and every job-in-flight line is now drawn one way.** The /channels rack, the dashboard's Channels table and the work tables on the operation pages and /cleanup are one table with a column set per page, over one channel row built on the server (which no longer ships a channel's config to the browser); the dashboard's "Needs work" seed is computed by the same code the widget endpoint serves. On the jobs side, /jobs rows, the "Active jobs" cards on channel/video/operation pages, the monitor widget's Active jobs strip and the operations board's "In flight" list are one job row in three sizes, with one rule for which buttons (Retry / Reorder / Drain / Cancel / Force-release) a job gets. **What you might notice:** a work table's report column reads "stale"/"missing" like the rack's instead of a date; the dashboard's Sync button is the rack's; a lane line on /jobs offers Force-release while its runner is running; widget job lines show who asked for the job; an in-flight download on the operations board links to its job page. Nothing a count says moved.
- **`site.json`, each channel's `config.json` and the per-video sidecars now have one schema each, and the two config files have generated key tables.** **`SITE.md`** and **`CHANNEL.md`** (new, repo root) list every key with its default and meaning, generated by `common/bin/file-schemas-docs.ts` and checked by a test. Nothing an operator has configured reads or saves differently: every live `site.json` and `config.json`, and a 1,763-file sample of sidecars, read and write back byte-for-byte as before. **Fixed:** a social-channel fetch no longer undoes Configure-form edits made while it was running (it used to write back the whole config it read when it started). Every change to a channel's config now re-reads the file at the moment it saves and changes only its own fields, so a sync stamping its time and a form save made at the same moment both land. Two writes to the same file from the editor no longer share one temporary file.
- **`settings.json` has one schema and one writer, and its key table is generated.** Every key, its default, its clamp and its documentation is now one zod schema (`common/lib/settingsSchema.ts`); `getSettings`/`writeSettings` both parse through it, and every settings form saves through one helper (`editor/app/settings/saveSettings.ts`) that merges only what the form changed. **`SETTINGS.md`** (new, repo root) lists every key with its default and what it does, and `settings.json.example` is now the full default object — both generated by `common/bin/settings-example.ts` and checked by a test, so neither can drift. Nothing an operator has configured reads differently. **Fixed:** adding or editing a storage location on `/storage` no longer erases the record of which location the saved-video store is on (`storage.savedVideosLocationId`).
diff --git a/plans/one-core-phase-3.md b/plans/one-core-phase-3.md
@@ -717,7 +717,8 @@ collision. All of them now go through `common/lib/jsonFile-server.ts`, byte-for-
| `9bfd15cd` | Race class: `rosterStore.writeRoster` (mkdir), `maybeMissingStore.writeMaybeMissing` (no mkdir), `metadataScanStore.writeMetadataScan` (no mkdir) on `writeJsonAtomic`. The counters `metadataScanStore.ts:188-191` and `autoQueueState.ts:141` deleted; `autoQueueState` on `writeJsonAtomic` (mkdir). New `autoQueueState.test.ts` "overlapping writes do not collide on the tmp file" (12 concurrent writes, last issued lands, no temp left); `metadataScanStore.test.ts:253` and `rosterStore.test.ts:184-186` unchanged and green |
| `ae6ed9d2` | Process-global + per-video JSON: `syncSchedulerState`, `workerDefaults`, `widgetPresets`, `homepage` (mkdir `homepageDir` = the file's parent), `migrate-channel-priority`, `relocateDir.writeDirMarker`, `shard.saveShardConfig`, `duplicateShorts` ×2, `scanCorruptMedia` ×2, `backupSavedVideos` manifest, `normalizeLiveChat`, `normalizeTranscript`, `videoActions.ts` remark (`'{"transcription":[]}\n'` → `writeJsonAtomic(file, {transcription: []}, {indent: 0})`). Compact without newline (`{indent: 0, newline: false}`) for the two reports and the two cue files. mkdir exactly where a site had one. New `controller/compactJsonWriters.test.ts` (the four compact writers' bytes = `JSON.stringify` of their parse, no newline; passes on the parent too) and the literal pinned in `jsonFile-server.test.ts` |
| `588fd7e9` | Text / binary: `failedTranscriptions` prune + clear, `runYtdlp.writePlaylistFile`, `xSessionBroker.writeCookieJar` (`{mkdir: true, mode: 0o600}`), `savedVideo-server.moveFileCrossDevice` (`copyFileAtomic`), `sites/lib/cutReleaseAction.ts`, `videoActions.ts` VTT promote. One comment on `storageWatch.ts`'s `let timer` (a per-copy singleton, not a temp name, one caller) |
-| (this) | `plans/tools/phase3-writers-numbers.ts`, this record, the changelog bullet |
+| `be4769de` | `plans/tools/phase3-writers-numbers.ts`, this record, the changelog bullet |
+| (review fixes) | record wording (188, writes-not-a-lock, `transcribeOne.ts:173`), changelog scope, the mode test made non-vacuous — see below |
`videoActions.ts` changed at its two write sites and one added import line only — no export
renamed, no signature changed (slice 3b owns its import list).
@@ -742,17 +743,22 @@ and named. The last three hits are the shared writer itself, its history comment
**Out of scope by name:** `buildIndex.ts:971` (the streaming `createWriteStream` page writer)
and `buildStats.ts:220` (a hand-joined array) — restructuring, not a fold;
-`scripts/diarize.mjs`; everything under `umtool/`. The module-level `storageWatch` timer and
+`scripts/diarize.mjs`; everything under `umtool/`. And one the grep cannot see:
+`transcribeOne.ts:173` writes the remote transcript straight to `transcript.json`
+(`writeFile(transcriptPath, bytes)` — no temp, no rename), so a crash mid-write can leave it
+truncated; it never had the tmp idiom. A candidate for `writeFileAtomic` in a later slice. The module-level `storageWatch` timer and
the TTL caches in `autoRunner.ts` / `recencyIndex.ts` are not temp names.
**Behaviour changes (intended).** (1) Every folded write is chained per absolute path with every
other `writeFileAtomic`/`writeJsonAtomic`/`copyFileAtomic` in the process, across module
copies — the roster's writers in `runYtdlp`, `quickAvailabilityCheck` and the
-`pipelineActions` server action now serialise. (2) A failed write removes its temp; the old
+`pipelineActions` server action now have their WRITES serialised. That is not a lock: a
+`load → merge → writeRoster` from two actors can still lose one merge, exactly as before
+(the read-modify-write lock is `withJsonFileLock`, which these callers do not take). (2) A failed write removes its temp; the old
code left it. (3) Temp names changed shape (`<file>.tmp-<pid>-<seq>-<hex>`); the remark's temp
was `transcript.tmp-<pid>.json` and is now `transcript.json.tmp-…`. Nothing reads temp names.
-**Found and left: 173 orphan temps in the live corpus.** `transcripts/.auto-queue/` holds
+**Found and left: 188 orphan temps in the live corpus.** `transcripts/.auto-queue/` holds
**175** `state.json.tmp-2514131-NNNN` files, all dated 2026-09-11 — the day `/home` hit 100 % —
**173 of them 0 bytes** (two are partial, 16–20 KB): each a failed `writeFile` (ENOSPC) the old
code never cleaned. Thirteen more elsewhere: seven `snapshot.json.tmp-<pid>`, three sidecar
@@ -790,6 +796,17 @@ pinned by `jsonText`'s tests) — and the remark literal, pinned by a unit test.
files each.
- e2e, the prompt's 20 specs (all exist): **140 passed, 0 failed, 10.7 min**.
+**Review fixes** (review verdict: ship after fixes; four nits, none blocking). The heading's
+orphan count is corrected from 173 to 188, the body's total. The record now says the roster
+writers' writes are serialised but a load-merge-write is not locked, and lists
+`transcribeOne.ts:173` as out of scope. The changelog no longer says "every file": it names
+the four temp names that stay. The mode test (`jsonFile-server.test.ts`) could pass vacuously,
+because a `fs.watch` could see no temp. It now intercepts `rename` (patched on
+`node:fs/promises` + `syncBuiltinESMExports`) and stats the temp at that moment, asserting
+exactly one temp, already 0o600. With `mode` removed from `writeFileAtomic` it fails.
+Gates after: tsc clean, common **1733/1733**, editor unit **67/67**. No runtime code changed,
+so the builds, e2e and numbers were not re-run.
+
## Next release — slice 3b and Phase 4 (inventory kept from 2026-09-23)
Slices 3a and 4b shipped in the release above (2026-09-24); the slice 3b bullets and the
