commit 35d1a420b0aca46a79269c44dd3aafd52c47a8a2
parent ca6bc9c50b2ae853c17ab4b88153ef21da10c7dd
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 28 Sep 2026 13:40:15 -0400
jobs: serialWriter — a job's sidecar chain survives a rejecting write (W1 review L1)
metaWriter's chain is now serialWriter(write): `last.then(write, write)`
so a writer that rejects cannot stall every later write, plus a no-op
catch so the last one rejecting is not an unhandled rejection.
writeJobMeta never rejects, so this is defence only, and the chain
itself is described as defensive: writes are issued in order, and the
review's 300-job probe never reproduced an out-of-order landing without
it.
streamCommand.test.ts +1: four writes (the first slow, the second and
fourth rejecting) run one at a time, in order, with no unhandled
rejection. It fails on `.then(write)`, on a missing catch
(unhandledRejection), and on unchained writes.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Diffstat:
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/common/jobs/streamCommand.test.ts b/common/jobs/streamCommand.test.ts
@@ -9,6 +9,7 @@ import { getRegistry, newJobId } from "./registry";
import {
runManagedCommand,
runManagedFunction,
+ serialWriter,
type StreamActionResult,
} from "./streamCommand";
@@ -140,6 +141,33 @@ test("a command job cancelled while queued ends with a cancelled sidecar", async
}
});
+// The sidecar writer behind every job (metaWriter): each write starts only when
+// the previous one has settled, a write that rejects does not stall the ones
+// after it, and the last one rejecting is not an unhandled rejection (the test
+// runner would fail this test on one).
+test("serialWriter runs writes one at a time, in order, past a rejection", async () => {
+ const events: string[] = [];
+ let n = 0;
+ const write = serialWriter(async () => {
+ const i = ++n;
+ events.push(`start ${i}`);
+ // The first write is the slow one; unchained, the later ones would start
+ // (and end) while it is still in flight.
+ await new Promise((r) => setTimeout(r, i === 1 ? 40 : 1));
+ events.push(`end ${i}`);
+ if (i === 2 || i === 4) throw new Error("a writer that rejects");
+ });
+ write();
+ write();
+ write();
+ write();
+ await new Promise((r) => setTimeout(r, 150));
+ assert.deepEqual(events, [
+ "start 1", "end 1", "start 2", "end 2",
+ "start 3", "end 3", "start 4", "end 4",
+ ]);
+});
+
// THE ONE CANCEL THAT MUST NOT: the graceful-shutdown reaper cancels every
// queued job only so the exit cannot promote one into a child. Nobody cancelled
// it, and its `queued` sidecar is what the boot pass (bootQueuedJobs.ts)
diff --git a/common/jobs/streamCommand.ts b/common/jobs/streamCommand.ts
@@ -164,16 +164,29 @@ function makeDoneDeferred(jobId: string): {
return { done, settle };
}
-// ONE WRITER PER JOB, IN ORDER. A job's sidecar is written at least twice —
-// at enqueue, then with its terminal state — and each write is async, so two
-// in flight at once could land in either order: a cancel a moment after the
-// enqueue could leave "queued" on disk, or a shorter JSON over a longer one's
-// tail. Chained, each write starts when the previous one has ended and
-// serializes the record as it is THEN. writeJobMeta never rejects.
+// ONE WRITER PER JOB, IN ORDER — DEFENSIVE. A job's sidecar is written at
+// least twice (at enqueue, then with its terminal state) and each write is
+// async. writeJobMeta snapshots the record before its first await, so two
+// writes are ISSUED in order; only the fs threadpool could complete them out
+// of order, leaving "queued" over "cancelled" or a shorter JSON over a longer
+// one's tail. A 300-job probe (release 13 W1 review) never saw that happen
+// without the chain. Chained, each write starts when the previous one has
+// settled and serializes the record as it is THEN.
function metaWriter(paths: Paths, record: JobRecord): () => void {
+ return serialWriter(() => writeJobMeta(paths, record));
+}
+
+// Run `write` once per call, each call starting only after the previous one
+// SETTLED — resolved or rejected. writeJobMeta never rejects today; a writer
+// that did must not silently stall every later write, hence
+// `then(write, write)`, nor raise an unhandled rejection from the last one,
+// hence the no-op catch (the writer owns its errors, as writeJobMeta does).
+// Exported for its unit test.
+export function serialWriter(write: () => Promise<void>): () => void {
let last: Promise<void> = Promise.resolve();
return () => {
- last = last.then(() => writeJobMeta(paths, record));
+ last = last.then(write, write);
+ last.catch(() => {});
};
}