commit cd756a8f03451f50b851e92e0893d5fcb93c4306
parent 8b7173d3a5d8bc228eb437082e792077a8e06c4a
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Tue, 22 Sep 2026 16:33:52 -0400
merge main into export/tags-followups
Diffstat:
17 files changed, 834 insertions(+), 107 deletions(-)
diff --git a/common/controller/autoRunner.test.ts b/common/controller/autoRunner.test.ts
@@ -4,6 +4,7 @@ import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import {
+ computeLeafPending,
focusHoldLine,
laneDispatchRoot,
makeFocusHoldReporter,
@@ -19,6 +20,7 @@ import {
type FocusSummary,
} from "../lib/channelPriority";
import { LANES } from "../lib/autoQueueTypes";
+import { emptyAutoQueueState } from "../jobs/autoQueueState";
import type {
AutoQueueGroup,
AutoQueuePolicy,
@@ -479,3 +481,65 @@ test("a focus holds the scan the same way it holds every download pick", () => {
);
});
+
+// --- computeLeafPending's `shared`: the four-lane poll reads once -----------
+//
+// /operations asks all four lanes for their pending work on a ~3 s poll, and
+// each call used to list every channel's config off disk and re-parse the
+// auto-queue state — both of which the caller had already read for the rest of
+// the payload, and neither of which is per-lane.
+
+function pendingFixture(): Paths {
+ const dir = mkdtempSync(path.join(tmpdir(), "leaf-pending-"));
+ const channelsDir = path.join(dir, "channels");
+ // A channel that IS on disk. An injected listing has to be believed over it.
+ mkdirSync(path.join(channelsDir, "on-disk"), { recursive: true });
+ writeFileSync(
+ path.join(channelsDir, "on-disk", "config.json"),
+ JSON.stringify({ url: "https://www.youtube.com/@ondisk" }),
+ );
+ return {
+ channelsDir,
+ sitesDir: path.join(dir, "sites"),
+ autoQueueStateFile: path.join(dir, "auto-queue.json"),
+ } as Paths;
+}
+
+test("computeLeafPending takes its channel listing from `shared`", async () => {
+ const paths = pendingFixture();
+ const result = await computeLeafPending(LANES[0], paths, {
+ configs: [],
+ state: emptyAutoQueueState(),
+ });
+ // The leaves the compiled root defines, all at zero: an empty listing is no
+ // channels, so there is no work to attribute and nothing to pick.
+ assert.deepEqual(Object.values(result.counts), [0]);
+ assert.deepEqual(Object.values(result.head), [[]]);
+ assert.deepEqual(result.owner, {});
+ assert.equal(result.nextUp, null);
+});
+
+test("an injected auto-queue state changes nothing about the answer", async () => {
+ const paths = pendingFixture();
+ // The state carries SWRR weights, which only matter once there is work to
+ // pick between — so what `shared.state` must never do is change the result.
+ // It saves a read; it is not an input to the numbers.
+ const state = emptyAutoQueueState();
+ state[LANES[0]].runtime.currentWeights = { "prio-all": 17 };
+ const injected = await computeLeafPending(LANES[0], paths, {
+ configs: [],
+ state,
+ });
+ const read = await computeLeafPending(LANES[0], paths, { configs: [] });
+ assert.deepEqual(injected.counts, read.counts);
+ assert.deepEqual(injected.head, read.head);
+ assert.deepEqual(injected.owner, read.owner);
+ assert.equal(injected.nextUp, read.nextUp);
+
+ // AND THE CALLER'S OBJECT IS NOT TOUCHED. selectNextWork advances fairness by
+ // mutating currentWeights in place, and this runs on a 3-second poll: the
+ // deep clone that stops merely HAVING the page open from skewing a
+ // round-robin group now has to protect a state the caller still holds and
+ // will hand to the next lane.
+ assert.deepEqual(state[LANES[0]].runtime.currentWeights, { "prio-all": 17 });
+});
diff --git a/common/controller/autoRunner.ts b/common/controller/autoRunner.ts
@@ -354,8 +354,12 @@ async function listChannelMeta(
paths: Paths,
kind: AutoQueueKind,
priority: ChannelPriority,
+ // A listing the CALLER already has. The status poll reads every channel's
+ // config once and then asked four lanes for their pending work, each of which
+ // re-read the whole directory — see computeLeafPending's `shared`.
+ sharedConfigs?: readonly { slug: string; config: ChannelConfig }[],
): Promise<{ meta: ChannelMeta[]; slugs: string[] }> {
- const configs = await listChannelConfigs(paths);
+ const configs = sharedConfigs ?? (await listChannelConfigs(paths));
return {
meta: configs
.filter(({ slug }) => !isChannelPaused(priority, slug, kind))
@@ -904,9 +908,21 @@ export type LeafPending = {
// operator sees as "cornbreadman: 12 pending". Uses the same matching AND the
// same ordering as the runner, so the numbers and the drill-down line up with
// what would actually be picked.
+//
+// `shared` IS THE FOUR-LANE POLL'S WAY OUT OF READING EVERYTHING FOUR TIMES.
+// /operations asks all four lanes on a ~3 s poll, and each call listed every
+// channel's config (a readdir plus a read per channel — 30 on Jeralyzer) and
+// parsed the auto-queue state document again, having already read both itself
+// for the rest of the payload. Nothing about either read is per-lane, so a
+// caller that holds them passes them in and the poll pays once. Omitted, each
+// is read here exactly as before, so every other caller is unchanged.
export async function computeLeafPending(
kind: AutoQueueKind,
paths: Paths = getPaths(),
+ shared?: {
+ configs?: readonly { slug: string; config: ChannelConfig }[];
+ state?: AutoQueueState;
+ },
): Promise<LeafPending> {
const settings = getSettings();
const policy = settings.autoQueue[kind];
@@ -920,6 +936,7 @@ export async function computeLeafPending(
paths,
kind,
settings.channelPriority,
+ shared?.configs,
);
const ctx = priorityContextFor(paths, settings, slugs);
const root = laneDispatchRoot(kind, policy, ctx, meta.map((m) => m.slug));
@@ -974,7 +991,7 @@ export async function computeLeafPending(
// runtime.currentWeights in place. This runs on a 3-second status poll, so
// asking the live runtime would let merely HAVING the page open skew a
// round-robin group's rotation. Deep-clone first; the clone is discarded.
- const state = await readAutoQueueState(paths);
+ const state = shared?.state ?? (await readAutoQueueState(paths));
const runtime = {
currentWeights: { ...state[kind].runtime.currentWeights },
};
diff --git a/common/lib/archive/contract.test.ts b/common/lib/archive/contract.test.ts
@@ -261,3 +261,24 @@ test("the search index worker's private pageFileName matches CONTRACT.pagePad",
assert.ok(src.includes("`/transcripts/${slug}/${pageFileName(p)}`"));
assert.equal(pageUrl("transcripts", "alpha", 12), "/transcripts/alpha/page-0012.json");
});
+
+// shipsPwa is the one function in this module that reads the ambient
+// environment, and the guard in front of that read is not observable from its
+// return value — `typeof process` is true in every runtime the test suite has.
+// So the assertion is on the SOURCE, the same way the service-worker checks
+// above are: what is being pinned is that the read cannot throw at import time
+// in a browser, not what it answers.
+test("shipsPwa guards its process.env read", () => {
+ const src = readSource("common/lib/archive/contract.ts");
+ const fn = /export function shipsPwa\([\s\S]*?\n}/.exec(src);
+ assert.ok(fn, "shipsPwa not found — did it move or get renamed?");
+ assert.match(fn[0], /typeof process !== "undefined"/);
+ // The guard must come BEFORE the dereference, which is the only arrangement
+ // that stops `ReferenceError: process is not defined`.
+ assert.ok(
+ fn[0].indexOf('typeof process !== "undefined"') <
+ fn[0].indexOf("process.env.INSTANCE_MODE"),
+ "the guard must precede the read",
+ );
+ // What it ANSWERS is unchanged, and is pinned by the behaviour test above.
+});
diff --git a/common/lib/archive/contract.ts b/common/lib/archive/contract.ts
@@ -180,18 +180,30 @@ export function pageUrl(
// Was two copies with "keep in sync" comments on each (compose-site.ts and
// export/app/lib/mode.ts); S2c deleted both, and this is the one.
//
-// `process.env.INSTANCE_MODE` is read bare, exactly as both copies read it, and
-// the reason that is safe is NOT that Next inlines it — it does not:
-// INSTANCE_MODE is neither `NEXT_PUBLIC_` nor listed in a next.config `env:`
-// block, so a client bundle would read `undefined` here. (An earlier draft of
-// this comment claimed the opposite; the S1 review checked.) It is safe because
-// no browser evaluates it: the only caller in the export app is
-// export/app/lib/mode.ts, whose only caller is export/app/layout.tsx, a SERVER
-// component, and the other caller is compose-site.ts, a build script. If this
-// ever becomes reachable from a client module it needs a
-// `typeof process !== "undefined"` guard AND an env var the client can see.
+// THE `typeof process` GUARD STOPS A CRASH. IT DOES NOT MAKE HUB DETECTION
+// WORK CLIENT-SIDE, and that distinction is the whole comment.
+//
+// `INSTANCE_MODE` is neither `NEXT_PUBLIC_` nor listed in a next.config `env:`
+// block, so Next does not inline it and a client bundle reads `undefined` here.
+// (An earlier draft of this comment claimed the opposite; the S1 review
+// checked.) Hub detection is therefore SERVER-ONLY, guard or no guard, until
+// somebody ships the value to the client deliberately — which would need an env
+// var the client can actually see, not a change to this line.
+//
+// Today that costs nothing: the export app reaches this only through
+// export/app/lib/mode.ts ← export/app/layout.tsx, a SERVER component, and the
+// other caller is compose-site.ts, a build script. What the guard buys is the
+// failure mode when that stops being true. An unguarded `process.env` in a
+// module some client component pulls in throws `ReferenceError: process is not
+// defined` at import time and takes the page down with it; guarded, the same
+// import yields `false` — no PWA, which is the right answer for every site but
+// a hub and a recoverable one for a hub. A wrong answer nobody notices beats a
+// white screen.
export function shipsPwa(site: { pwa?: boolean }): boolean {
- return site.pwa === true || process.env.INSTANCE_MODE === "hub";
+ return (
+ site.pwa === true ||
+ (typeof process !== "undefined" && process.env.INSTANCE_MODE === "hub")
+ );
}
// ─── The hub's member entry, once ───
diff --git a/common/lib/curatedTags.test.ts b/common/lib/curatedTags.test.ts
@@ -226,7 +226,40 @@ test("sanitizeTagsConfig is idempotent", () => {
// ─── mergeTagDefs ───
-test("mergeTagDefs overlays presentation fields and appends site rules", () => {
+test("the site layer carries no rules, on read and on write", () => {
+ const raw = {
+ tags: [
+ {
+ id: "eva-collab",
+ rules: [{ id: "site", kind: "metadata", pattern: "extra" }],
+ },
+ {
+ id: "site-only",
+ label: "Site only",
+ rules: [{ id: "r1", kind: "caption", pattern: "x" }],
+ },
+ ],
+ };
+ const site = sanitizeTagsConfig(raw, { layer: "site" });
+ // Not emptied — DROPPED: no `rules` key at all, so what a site writes has
+ // none and nothing downstream is ever handed a rule that cannot fire.
+ for (const def of site.tags) {
+ assert.equal(Object.prototype.hasOwnProperty.call(def, "rules"), false);
+ }
+ // The same coercion runs on read, so a hand-edited site file cannot smuggle
+ // one back in.
+ assert.deepEqual(
+ sanitizeTagsConfig(JSON.parse(JSON.stringify(site)), { layer: "site" }),
+ site,
+ );
+ // And the corpus layer is untouched by this: the identical input keeps them.
+ assert.deepEqual(
+ (sanitizeTagsConfig(raw).tags[0].rules ?? []).map((r) => r.id),
+ ["site"],
+ );
+});
+
+test("mergeTagDefs overlays presentation fields and cannot add a rule", () => {
const merged = mergeTagDefs(DEFS, [
{
id: "eva-collab",
@@ -235,6 +268,9 @@ test("mergeTagDefs overlays presentation fields and appends site rules", () => {
color: "#b48ead",
order: 9,
hidden: true,
+ // A rule reaching the merge at all means someone bypassed the site
+ // coercion; it is still not appended, because a site rule can never tag
+ // a record and "harmless on a published def" is how it looks like it can.
rules: [{ id: "site", kind: "metadata", pattern: "extra", enabled: true }],
},
]);
@@ -246,7 +282,7 @@ test("mergeTagDefs overlays presentation fields and appends site rules", () => {
assert.equal(collab.hidden, true);
assert.deepEqual(
(collab.rules ?? []).map((r) => r.id),
- ["meta", "site"],
+ ["meta"],
);
// Untouched fields survive, and the global input is not mutated.
assert.equal(collab.group, "eva");
diff --git a/common/lib/curatedTags.ts b/common/lib/curatedTags.ts
@@ -222,11 +222,21 @@ function coerceTagDef(raw: unknown, layer: TagLayer): CuratedTagDef | null {
// override", and inventing one here would rename the tag on that site, since
// mergeTagDefs applies every field the overlay carries.
const label = trimmedString(r.label) ?? (layer === "site" ? undefined : id);
- const rules = Array.isArray(r.rules)
- ? r.rules
- .map((rule, i) => coerceRule(rule, i))
- .filter((rule): rule is CuratedTagRule => rule !== null)
- : [];
+ // THE SECOND FIELD THE LAYERS TREAT DIFFERENTLY: a site layer carries no
+ // rules at all, and one written there is dropped — on read AND on write, so
+ // it can never reach a file, a merge or a published def.
+ //
+ // A rule is evaluated once at index time, over records SHARED by every site
+ // that carries the channel. There is no per-site record for a per-site hit to
+ // live in, so a site rule could only ever LOOK like it worked. Dropping it in
+ // the coercion is the one place that covers every writer. Rules bind from
+ // transcripts/tags.json alone; promote one there to make it fire.
+ const rules =
+ layer === "site" || !Array.isArray(r.rules)
+ ? []
+ : r.rules
+ .map((rule, i) => coerceRule(rule, i))
+ .filter((rule): rule is CuratedTagRule => rule !== null);
const order =
typeof r.order === "number" && Number.isFinite(r.order)
? r.order
@@ -348,10 +358,13 @@ export function sanitizeTagsConfig(
// Layer a site's tags.json over the corpus one.
//
// For an id the corpus already defines, the site entry is a FIELD-WISE OVERLAY
-// of label/groupLabel/color/order/hidden and may APPEND rules. It can never
-// delete a corpus rule (and assignments do not live at the site layer at all —
-// an assignment is a fact about a video, not a presentation choice). A site id
-// the corpus does not define becomes a full site-only tag.
+// of label/groupLabel/color/order/hidden — presentation, and nothing else. It
+// can never delete a corpus rule, and it cannot add one: coerceTagDef drops
+// rules from a site layer entirely (see there for why a per-site rule cannot
+// tag anything), so there is nothing here to append. Assignments do not live at
+// the site layer at all — an assignment is a fact about a video, not a
+// presentation choice. A site id the corpus does not define becomes a full
+// site-only tag, rule-less like every other site row.
//
// Deliberately NOT mergeAliases' wholesale replacement: aliases are suggestions,
// tags are a shared vocabulary that a site may dress up but not gut.
@@ -375,9 +388,8 @@ export function mergeTagDefs(
if (overlay.color !== undefined) base.color = overlay.color;
if (overlay.order !== undefined) base.order = overlay.order;
if (overlay.hidden !== undefined) base.hidden = overlay.hidden;
- if (overlay.rules && overlay.rules.length > 0) {
- base.rules = [...(base.rules ?? []), ...overlay.rules];
- }
+ // No rule branch: a sanitized site def has none, and the corpus rules on
+ // `base` are left exactly as the corpus wrote them.
}
return out;
}
diff --git a/common/lib/curatedTagsStore.ts b/common/lib/curatedTagsStore.ts
@@ -7,12 +7,15 @@
// site-only tags. See common/lib/curatedTags.ts for the pure model, the
// coercion and mergeTagDefs.
//
-// **A site-layer RULE never tags a record.** mergeTagDefs will append one (it
-// is harmless on a published /tags.json def), but rules are evaluated once at
-// index time over records SHARED by every site that carries the channel —
-// there is no per-site record for a per-site hit to live in. Rules bind only
-// from transcripts/tags.json; promote one there to make it fire. This is why
-// the editor offers no site-side rule editor.
+// **A site-layer RULE never tags a record, so a site layer carries none.**
+// Rules are evaluated once at index time over records SHARED by every site that
+// carries the channel — there is no per-site record for a per-site hit to live
+// in, so a rule written at the site layer could only ever LOOK like it worked.
+// The site coercion therefore DROPS rules outright: on READ, so a hand-edited
+// sites/<id>/tags.json cannot smuggle one in, and on WRITE, so neither can a
+// caller — and mergeTagDefs has no rule branch left to append through. Rules
+// bind from transcripts/tags.json alone; promote one there to make it fire.
+// This is why the editor offers no site-side rule editor.
//
// Unlike aliasesStore there are NO seeded defaults: a fresh install has no
// tags, and an absent global file reads as an empty config.
diff --git a/editor/app/api/ops/_lib.ts b/editor/app/api/ops/_lib.ts
@@ -1,6 +1,7 @@
import { NextResponse } from "next/server";
import { authorizeWorkerRequest } from "yt-dlp-transcript-common/lib/workerToken";
import { isValidChannelSlug } from "yt-dlp-transcript-common/controller/channels";
+import { isValidSiteId } from "yt-dlp-transcript-common/lib/site";
import type { StreamActionResult } from "yt-dlp-transcript-common/jobs/streamCommand";
import type { QueueOutcome } from "../../channels/lib/queueForSlugs";
@@ -158,6 +159,47 @@ export function reqStringArray(body: OpsBody, key: string): string[] {
return (v as string[]).map((s) => s.trim());
}
+// ONE SITE OR SEVERAL, SPELLED EITHER WAY. The two build routes disagreed —
+// build-site took `siteIds` (a list), build-deploy took `siteId` (one) — so the
+// same body worked on one and 400'd on the other, and the fix people reached
+// for was to guess, which costs a round trip every time. Both routes now accept
+// both keys, and this reader is the single place that says what that means.
+//
+// Both keys at once is still a 400 rather than a merge: a caller that sent both
+// has two ideas about what it wants, and picking one on its behalf is how a
+// deploy of the wrong site becomes somebody's afternoon. Neither key is a 400
+// naming both, because "which did you mean" is the actual question.
+//
+// A SITE ID, NOT MERELY A STRING, and checked BEFORE any job starts — the same
+// reason reqSlug exists, with a worse failure behind it. `getSite` throws a
+// plain Error on an id failing SITE_ID_RE and `ops()` maps that to a 500, so
+// ["good", "BAD"] used to queue the first build, throw on the second and answer
+// 500 with no jobs and no ids: a real build running that nothing was watching,
+// and a `--wait` exiting 1 about it.
+export function reqSiteIds(body: OpsBody): string[] {
+ const one = body.siteId;
+ const many = body.siteIds;
+ if (one !== undefined && many !== undefined) {
+ throw new OpsInputError('send either "siteId" or "siteIds", not both');
+ }
+ let ids: string[];
+ if (one !== undefined) ids = [reqString(body, "siteId")];
+ else if (many !== undefined) ids = reqStringArray(body, "siteIds");
+ else {
+ throw new OpsInputError(
+ '"siteId" (a string) or "siteIds" (a non-empty array of strings) is required',
+ );
+ }
+ for (const id of ids) {
+ if (!isValidSiteId(id)) {
+ throw new OpsInputError(
+ `"${id}" is not a valid site id (lowercase letters, digits and "-"; must start with a letter or digit)`,
+ );
+ }
+ }
+ return ids;
+}
+
export function oneOf<T extends string>(
body: OpsBody,
key: string,
diff --git a/editor/app/api/ops/build-deploy/route.ts b/editor/app/api/ops/build-deploy/route.ts
@@ -3,29 +3,93 @@ import {
buildAndDeployAction,
buildAndDeployAllSitesAction,
} from "../../../sites/lib/buildAction";
-import { jobResponse, OpsInputError, ops, optBool, optString } from "../_lib";
+import {
+ jobResponse,
+ OpsInputError,
+ ops,
+ opsFail,
+ optBool,
+ reqSiteIds,
+} from "../_lib";
export const dynamic = "force-dynamic";
-// POST { siteId: string, skipArchives? } | { all: true, skipArchives? }
-// -> { ok: true, jobId }
+// POST { siteId: string | siteIds: string[], skipArchives? }
+// | { all: true, skipArchives? }
+// -> { ok: true, jobs: [{ siteId, jobId }], skipped: [{ siteId, reason }],
+// jobId? }
+//
+// Build THEN deploy: one managed job per site (one log, one Cancel each), so
+// the caller polls /api/jobs/<jobId>/log exactly as it does for build-site.
+//
+// `siteId` and `siteIds` are the same key (reqSiteIds), and the response is
+// build-site's: this route took one id and that one a list, so the two halves
+// of the same sentence in a runbook needed different JSON. `jobId` is still
+// there when exactly one job started, so every existing single-site caller —
+// and jobResponse's own shape — is unchanged; `jobs`/`skipped` are additive,
+// and `pnpm ops --wait` already reads `jobs[]`.
//
-// One managed job either way (build then deploy, one log, one Cancel), so the
-// caller polls /api/jobs/<jobId>/log exactly as for a single site.
+// Asking for builds and getting NONE is still a 400 carrying the reason, not a
+// cheerful `{ ok: true, jobs: [] }`: --wait would exit 0 on it and report
+// success about a deploy that never started.
export async function POST(request: Request) {
- return ops(request, ["siteId", "all", "skipArchives"], async (body) => {
- const skipArchives = optBool(body, "skipArchives");
- const all = optBool(body, "all");
- const siteId = optString(body, "siteId");
- if (all) {
- if (siteId) throw new OpsInputError('send either "siteId" or "all", not both');
- return jobResponse(await buildAndDeployAllSitesAction(skipArchives));
- }
- if (!siteId?.trim()) {
- throw new OpsInputError('"siteId" is required (or send { "all": true })');
- }
- return jobResponse(await buildAndDeployAction(siteId.trim(), skipArchives));
- });
+ return ops(
+ request,
+ ["siteId", "siteIds", "all", "skipArchives"],
+ async (body) => {
+ const skipArchives = optBool(body, "skipArchives");
+ if (optBool(body, "all")) {
+ if (body.siteId !== undefined || body.siteIds !== undefined) {
+ throw new OpsInputError(
+ 'send either "siteId"/"siteIds" or "all", not both',
+ );
+ }
+ return jobResponse(await buildAndDeployAllSitesAction(skipArchives));
+ }
+ const jobs: { siteId: string; jobId: string }[] = [];
+ const skipped: { siteId: string; reason: string }[] = [];
+ let info = false;
+ for (const siteId of reqSiteIds(body)) {
+ // ONE SITE'S THROW CANNOT COST THE OTHERS THEIR JOB IDS. An exception
+ // out of here becomes a 500 carrying no `jobs` at all, while the builds
+ // already queued run on with nobody holding their ids. reqSiteIds
+ // rejects the malformed-id case before any job starts; this catches
+ // whatever else the action can raise.
+ let result: Awaited<ReturnType<typeof buildAndDeployAction>>;
+ try {
+ result = await buildAndDeployAction(siteId, skipArchives);
+ } catch (e) {
+ skipped.push({ siteId, reason: (e as Error).message });
+ continue;
+ }
+ if (!result.ok) {
+ skipped.push({ siteId, reason: result.error });
+ info = info || result.info === true;
+ continue;
+ }
+ // The stream is cancelled, never returned — see _lib's header.
+ void result.stream.cancel();
+ jobs.push({ siteId, jobId: result.jobId });
+ }
+ if (jobs.length === 0) {
+ // One site asked for, one reason: the bare sentence the action gave,
+ // exactly as jobResponse has always returned it.
+ return opsFail(
+ skipped.length === 1
+ ? skipped[0].reason
+ : skipped.map((s) => `${s.siteId}: ${s.reason}`).join("; "),
+ 400,
+ info ? { info: true } : undefined,
+ );
+ }
+ return NextResponse.json({
+ ok: true,
+ jobs,
+ skipped,
+ ...(jobs.length === 1 ? { jobId: jobs[0].jobId } : {}),
+ });
+ },
+ );
}
export function GET() {
diff --git a/editor/app/api/ops/build-site/route.ts b/editor/app/api/ops/build-site/route.ts
@@ -3,49 +3,59 @@ import {
buildAllSitesAction,
buildExportAction,
} from "../../../sites/lib/buildAction";
-import { jobResponse, OpsInputError, ops, optBool } from "../_lib";
+import {
+ jobResponse,
+ OpsInputError,
+ ops,
+ optBool,
+ reqSiteIds,
+} from "../_lib";
export const dynamic = "force-dynamic";
-// POST { siteIds: string[], skipData?, skipArchives? } | { all: true, skipArchives? }
+// POST { siteIds: string[] | siteId: string, skipData?, skipArchives? }
+// | { all: true, skipArchives? }
+//
+// BUILD WITHOUT DEPLOYING. One build-export job per site on the shared build
+// queue (they run one at a time, as they do from /sites), returning
+// `{ jobs: [{ siteId, jobId }] }` — a list, because there is a job per site and
+// a caller waiting on them needs all the ids. `{ all: true }` is the single
+// build-all job instead, which is the docker fan-out.
//
-// BUILD WITHOUT DEPLOYING. `siteIds` queues one build-export job per site on the
-// shared build queue (they run one at a time, as they do from /sites) and
-// returns `{ jobs: [{ siteId, jobId }] }` — a list, because there is a job per
-// site and a caller waiting on them needs all the ids. `{ all: true }` is the
-// single build-all job instead, which is the docker fan-out.
+// `siteId` and `siteIds` both work, via reqSiteIds: this route and build-deploy
+// used to disagree about the spelling, which made the pair unguessable.
export async function POST(request: Request) {
return ops(
request,
- ["siteIds", "all", "skipData", "skipArchives"],
+ ["siteId", "siteIds", "all", "skipData", "skipArchives"],
async (body) => {
const skipArchives = optBool(body, "skipArchives");
if (optBool(body, "all")) {
- if (body.siteIds !== undefined) {
- throw new OpsInputError('send either "siteIds" or "all", not both');
+ if (body.siteIds !== undefined || body.siteId !== undefined) {
+ throw new OpsInputError(
+ 'send either "siteId"/"siteIds" or "all", not both',
+ );
}
return jobResponse(await buildAllSitesAction(skipArchives));
}
- const raw = body.siteIds;
- if (
- !Array.isArray(raw) ||
- raw.length === 0 ||
- raw.some((s) => typeof s !== "string" || !s.trim())
- ) {
- throw new OpsInputError(
- '"siteIds" is required and must be a non-empty array of strings (or send { "all": true })',
- );
- }
const skipData = optBool(body, "skipData");
const jobs: { siteId: string; jobId: string }[] = [];
const skipped: { siteId: string; reason: string }[] = [];
- for (const siteId of (raw as string[]).map((s) => s.trim())) {
- const result = await buildExportAction(
- siteId,
- undefined,
- skipData,
- skipArchives,
- );
+ for (const siteId of reqSiteIds(body)) {
+ // One site's throw cannot cost the others their job ids — see the same
+ // guard in build-deploy.
+ let result: Awaited<ReturnType<typeof buildExportAction>>;
+ try {
+ result = await buildExportAction(
+ siteId,
+ undefined,
+ skipData,
+ skipArchives,
+ );
+ } catch (e) {
+ skipped.push({ siteId, reason: (e as Error).message });
+ continue;
+ }
if (!result.ok) {
skipped.push({ siteId, reason: result.error });
continue;
diff --git a/editor/app/operations/status.ts b/editor/app/operations/status.ts
@@ -27,7 +27,8 @@ import { readPriorityView } from "./channelPriorityView";
// set costs a channel listing and, for a site focus, a sites read; the
// auto-queue state document is one JSON parse — it was read once PER LANE, four
// times per poll, because `buildKind` did its own reading; and the channel
-// briefs are shared with the lanes builder through the per-request cache.
+// briefs are shared with the lanes builder through the per-request cache, and
+// with the four computeLeafPending calls through their `shared` argument.
export async function buildAutoQueueStatusPayload(): Promise<AutoQueueStatusPayload> {
const paths = getPaths();
const settings = getSettings();
@@ -37,8 +38,14 @@ export async function buildAutoQueueStatusPayload(): Promise<AutoQueueStatusPayl
readAutoQueueState(paths),
getChannelBriefs(paths),
]);
+ // …and the four lanes' pending work re-reads NEITHER. `briefs` is the channel
+ // listing and `state` the auto-queue document, both already in hand one line
+ // up; without them each of the four calls listed every channel's config off
+ // disk again and re-parsed the state, on a ~3 s poll.
const pendingByKind = await Promise.all(
- LANES.map((lane) => computeLeafPending(lane, paths)),
+ LANES.map((lane) =>
+ computeLeafPending(lane, paths, { configs: briefs, state }),
+ ),
);
const byLane = <T>(values: readonly T[]): Record<AutoQueueKind, T> =>
Object.fromEntries(LANES.map((lane, i) => [lane, values[i]])) as Record<
diff --git a/editor/e2e/ops-api.spec.ts b/editor/e2e/ops-api.spec.ts
@@ -25,6 +25,7 @@ import {
resolvePath,
writeChannelConfig,
writeSettings,
+ writeSite,
} from "./helpers";
const TOKEN = "test-worker-token";
@@ -36,7 +37,8 @@ type OpsResponse = {
jobId?: string;
queued?: string[];
jobIds?: string[];
- skipped?: { slug: string; reason: string }[];
+ skipped?: { slug?: string; siteId?: string; reason: string }[];
+ jobs?: { siteId: string; jobId: string }[];
};
async function ops(
@@ -651,3 +653,83 @@ test("tag-videos refuses a traversing slug, a bad op and an unknown key", async
// Nothing was written by any of the three.
expect(await pathExists("test-transcripts/tags.json")).toBe(false);
});
+
+// --- the two build routes speak the same body ---------------------------------
+//
+// They did not. build-site took `siteIds` (a list) and build-deploy took
+// `siteId` (one), so the two halves of the same runbook sentence needed
+// different JSON and the only way to learn which was to get a 400. Both now
+// accept both keys; sending BOTH is still refused, because a caller with two
+// ideas about what to build should not have one picked for it.
+// Job sidecars on disk — the only way to say "and nothing was queued".
+async function listJobIds(): Promise<string[]> {
+ return (await readdir(resolvePath("test-transcripts/.jobs")).catch(() => []))
+ .filter((f) => f.endsWith(".meta.json"))
+ .sort();
+}
+
+test("build-site and build-deploy each take siteId or siteIds, and refuse both or neither", async ({
+ request,
+}) => {
+ await resetData("title-filter-channel");
+ await settings();
+
+ for (const action of ["build-site", "build-deploy"]) {
+ const both = await ops(request, action, {
+ siteId: "a",
+ siteIds: ["a"],
+ });
+ expect(both.status, action).toBe(400);
+ expect(both.body.error, action).toContain("not both");
+
+ const neither = await ops(request, action, {});
+ expect(neither.status, action).toBe(400);
+ expect(neither.body.error, action).toContain("siteId");
+
+ // `all` is still exclusive of either spelling.
+ const withAll = await ops(request, action, { all: true, siteId: "a" });
+ expect(withAll.status, action).toBe(400);
+ expect(withAll.body.error, action).toContain("not both");
+
+ // A MALFORMED ID IS A 400 BEFORE ANY JOB STARTS, and that is the point of
+ // validating in reqSiteIds rather than letting getSite throw: an id list of
+ // ["good", "BAD"] used to queue the first build, throw on the second and
+ // answer 500 with no ids at all — a real build running that nothing was
+ // watching, and a --wait exiting 1 about it.
+ const before = await listJobIds();
+ const bad = await ops(request, action, { siteIds: ["buildsite", "BAD ID"] });
+ expect(bad.status, action).toBe(400);
+ expect(bad.body.error, action).toContain("not a valid site id");
+ expect(bad.body.jobs, action).toBeUndefined();
+ expect(await listJobIds(), action).toEqual(before);
+ }
+});
+
+test("build-site with a bare siteId starts one build-export job", async ({
+ request,
+}) => {
+ test.setTimeout(120_000);
+ await resetData("title-filter-channel");
+ await settings();
+ await writeSite("buildsite", {});
+
+ const { status, body } = await ops(request, "build-site", {
+ siteId: "buildsite",
+ });
+ expect(status).toBe(200);
+ expect(body.ok).toBe(true);
+ // The response is the LIST shape whichever key was used — one job, named.
+ expect(body.jobs?.length).toBe(1);
+ expect(body.jobs?.[0].siteId).toBe("buildsite");
+ expect(body.skipped).toEqual([]);
+
+ const jobId = body.jobs![0].jobId;
+ await expect
+ .poll(async () => {
+ const meta = await readJson<{ kind: string }>(
+ `test-transcripts/.jobs/${jobId}.meta.json`,
+ ).catch(() => null);
+ return meta?.kind ?? null;
+ })
+ .toBe("build-export");
+});
diff --git a/editor/scripts/measure-nav.mjs b/editor/scripts/measure-nav.mjs
@@ -36,7 +36,7 @@ const BASE = argOf("base", "http://localhost:3001").replace(/\/$/, "");
const RUNS = Number(argOf("runs", "3"));
// Routes worth timing, with the segment name used to build a state tree. THE
-// TWELVE SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and
+// THIRTEEN SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and
// only those: stateTree() below encodes ONE segment, so /sites/<id>/charts
// and /operations/<id> cannot be listed — each would need the nested
// [segment, {children: [param, …]}] form, a different tree per route. A
diff --git a/scripts/archilyzer-ops.mjs b/scripts/archilyzer-ops.mjs
@@ -8,7 +8,8 @@
//
// USAGE
//
-// pnpm ops <action> [--json '<body>' | --file <path>] [--wait] [--quiet]
+// pnpm ops <action> [--json '<body>' | --file <path>] [--wait]
+// [--wait-timeout <seconds>] [--quiet]
// pnpm ops get channel <slug> [--counts]
// pnpm ops get tags [<tagId>]
// pnpm ops list
@@ -30,6 +31,8 @@
// pnpm ops lane --json '{"lane":"download","held":true}'
// pnpm ops refresh-report --json '{"all":true}'
// pnpm ops relocate --json '{"slugs":["x"],"locationId":"platter"}'
+// pnpm ops build-site --json '{"siteId":"anilyzer"}' --wait
+// pnpm ops build-deploy --json '{"siteIds":["anilyzer","jeralyzer"]}' --wait
// pnpm ops get channel the-quartering
// pnpm ops tags --json '{"op":"define","tag":{"id":"eva-collab","label":"Collab"}}'
// pnpm ops tag-videos --file ids.json
@@ -42,15 +45,24 @@
// --wait follows /api/jobs/<jobId>/log to the end for a job-starting action and
// exits 0 only if the job finished `done`. Without it the command returns as
// soon as the job is QUEUED, which is the honest answer: the queue may hold it
-// behind other work for hours.
+// behind other work for hours. A poll that fails does NOT end the follow — see
+// followJob — and --wait-timeout <seconds> is there for a caller that cannot
+// wait indefinitely.
//
// The response JSON is printed verbatim on stdout (log lines from --wait go to
// stderr), so `pnpm ops … | jq` works.
import { readFile } from "node:fs/promises";
+import { pathToFileURL } from "node:url";
const DEFAULT_URL = "http://localhost:3001";
+// Consecutive polls where NEITHER the job's log NOR the active list answered,
+// after which --wait gives up. At the 30 s backoff ceiling that is ~5 minutes
+// of an editor saying nothing at all, which is not a busy server — it is a
+// server that is gone.
+const MAX_PROBE_FAILURES = 10;
+
// The read-side routes, reachable as `get <noun> <arg>`. Kept tiny and explicit:
// an ops API that let a caller assemble arbitrary GET paths would be a proxy,
// not an adapter.
@@ -104,10 +116,32 @@ export function parseArgs(argv) {
let wait = false;
let quiet = false;
let counts = false;
+ let waitTimeout = null;
+ const readTimeout = (raw) => {
+ const n = Number(raw);
+ if (!Number.isFinite(n) || n <= 0) {
+ return { error: "--wait-timeout needs a positive number of seconds" };
+ }
+ waitTimeout = n;
+ // A timeout on a wait nobody asked for is not a preference, it is a typo
+ // with no effect — so it IMPLIES --wait rather than being ignored.
+ wait = true;
+ return null;
+ };
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === "--wait") {
wait = true;
+ } else if (arg === "--wait-timeout") {
+ const raw = argv[++i];
+ if (raw === undefined) {
+ return { error: "--wait-timeout needs a number of seconds" };
+ }
+ const err = readTimeout(raw);
+ if (err) return err;
+ } else if (arg.startsWith("--wait-timeout=")) {
+ const err = readTimeout(arg.slice("--wait-timeout=".length));
+ if (err) return err;
} else if (arg === "--quiet") {
quiet = true;
} else if (arg === "--counts") {
@@ -167,6 +201,7 @@ export function parseArgs(argv) {
path: GETTERS[noun](positional[2], counts),
wait: false,
quiet,
+ waitTimeout,
};
}
const action = positional[0];
@@ -194,18 +229,28 @@ export function parseArgs(argv) {
...(action === "tag-videos" ? { defaultSource: agentSource() } : {}),
wait,
quiet,
+ waitTimeout,
};
}
export function usage() {
return [
"Usage: pnpm ops <action> [--json '<body>' | --file <path>] [--wait]",
+ " [--wait-timeout <seconds>] [--quiet]",
" pnpm ops get channel <slug> [--counts]",
" pnpm ops get tags [<tagId>]",
" pnpm ops list",
"",
`Actions: ${ACTIONS.join(", ")}`,
"",
+ "--wait follows the job's log and survives a poll that fails (a busy",
+ " in-process build starves the server): it backs off and, after three",
+ " failures, asks /api/jobs/active whether the job is still there.",
+ "--wait-timeout <seconds> gives up and exits 1 instead of waiting forever.",
+ " Default: no timeout — the queue may legitimately hold a job for hours.",
+ "",
+ 'build-site and build-deploy both take "siteId" (one) or "siteIds" (a list).',
+ "",
"Env: ARCHILYZER_EDITOR_URL (default http://localhost:3001), WORKER_TOKEN,",
" ARCHILYZER_AGENT (provenance of a tag write; default \"cli\")",
].join("\n");
@@ -229,19 +274,127 @@ function authHeaders() {
// the browser polls it same-origin with no token. Sending one here implied a
// gate that does not exist, which is worse than sending nothing: the next
// person to read this would conclude the endpoint was protected.
-async function followJob(jobId, quiet) {
+//
+// A POLL FAILURE IS NOT A JOB FAILURE, and this used to treat them as the same
+// thing. The editor is single-process: a busy in-process build-index starves
+// the event loop for long enough that `fetch` rejects outright, and one
+// rejection ended the follow with a stack trace about a job that was running
+// fine and went on to finish. So every poll is caught, the wait backs off
+// 1→2→4…→30 s instead of hammering a server that is already struggling, and
+// `from` is kept across the failure so not one line of log text is lost.
+//
+// After three consecutive failures the endpoint is no longer trusted to answer
+// at all, and the question becomes a different one — IS THE JOB STILL THERE?
+// `/api/jobs/active` is cheap and is what the editor's own head polls. Listed
+// ⇒ keep waiting, however long that takes. Absent ⇒ it ended while we could not
+// see it, so one last log poll reads the terminal status; if even that fails,
+// the follow gives up rather than claiming an outcome it never read.
+//
+// `--wait-timeout` bounds the whole thing for a caller that cannot hang (CI,
+// an agent). Default none, because the honest default for a queue that may hold
+// a job behind hours of other work is to wait.
+export async function followJob(jobId, quiet, opts = {}) {
+ const doFetch = opts.fetch ?? fetch;
+ const sleep = opts.sleep ?? ((ms) => new Promise((r) => setTimeout(r, ms)));
+ const now = opts.now ?? (() => Date.now());
+ const deadline =
+ opts.timeoutSeconds > 0 ? now() + opts.timeoutSeconds * 1000 : null;
+ const base = opts.baseUrl ?? baseUrl();
+ const id = encodeURIComponent(jobId);
+
let from = 0;
+ let failures = 0;
+ let probeFailures = 0;
+ let backoff = 1000;
+
+ // One log poll. Returns the status, or null when the poll itself failed —
+ // never throws, so a transient fetch rejection cannot end the follow.
+ const pollLog = async () => {
+ try {
+ const res = await doFetch(`${base}/api/jobs/${id}/log?from=${from}`);
+ if (!res.ok) return null;
+ const payload = await res.json();
+ if (payload.content && !quiet) process.stderr.write(payload.content);
+ // Only advance once the chunk is in hand: a poll that failed halfway
+ // must re-ask for the same offset.
+ from = payload.nextOffset ?? from;
+ return payload.status ?? null;
+ } catch {
+ return null;
+ }
+ };
+
+ const stillListed = async () => {
+ try {
+ const res = await doFetch(`${base}/api/jobs/active`);
+ if (!res.ok) return null;
+ const payload = await res.json();
+ const jobs = Array.isArray(payload.jobs) ? payload.jobs : [];
+ return jobs.some((j) => j && j.id === jobId);
+ } catch {
+ return null;
+ }
+ };
+
+ // "queued" and "running" are the two NON-answers. Everything else is the job
+ // having ended, which is the only thing worth returning.
+ const terminal = (s) => s !== null && s !== "queued" && s !== "running";
+
for (;;) {
- const res = await fetch(
- `${baseUrl()}/api/jobs/${encodeURIComponent(jobId)}/log?from=${from}`,
- );
- if (!res.ok) throw new Error(`log poll failed: HTTP ${res.status}`);
- const payload = await res.json();
- if (payload.content && !quiet) process.stderr.write(payload.content);
- from = payload.nextOffset ?? from;
- const status = payload.status;
- if (status !== "queued" && status !== "running") return status;
- await new Promise((r) => setTimeout(r, 1000));
+ const status = await pollLog();
+ if (status !== null) {
+ failures = 0;
+ probeFailures = 0;
+ backoff = 1000;
+ if (terminal(status)) return status;
+ } else {
+ failures++;
+ if (failures >= 3) {
+ const listed = await stillListed();
+ if (listed === false) {
+ // Gone from the active list: it went terminal while the log endpoint
+ // was unreachable. One more try at the status it ended with — and
+ // ONLY a terminal one is an outcome. A log endpoint that came back
+ // answering "running" means the active list was stale, not that the
+ // job finished; returning that printed "running" as the result and
+ // exited 1 for a job that was fine.
+ const final = await pollLog();
+ if (terminal(final)) return final;
+ if (final === null) {
+ throw new Error(
+ `lost contact with job ${jobId}: it is no longer active and its log could not be read`,
+ );
+ }
+ // Readable again and still going: back to waiting, from scratch.
+ failures = 0;
+ probeFailures = 0;
+ backoff = 1000;
+ } else if (listed === true) {
+ // A job we can still see is a job to wait for.
+ failures = 0;
+ probeFailures = 0;
+ backoff = 1000;
+ } else {
+ // The probe failed too, so we now know nothing at all. Without a
+ // bound this waits forever on an editor that has gone away; with one
+ // it says so. Only CONSECUTIVE failures count — a single answer of
+ // either kind resets it.
+ probeFailures++;
+ if (probeFailures >= MAX_PROBE_FAILURES) {
+ throw new Error(
+ `lost contact with the editor at ${base}: ${MAX_PROBE_FAILURES} consecutive failed polls of job ${jobId} and of /api/jobs/active`,
+ );
+ }
+ }
+ }
+ backoff = Math.min(backoff * 2, 30_000);
+ }
+ if (deadline !== null && now() >= deadline) {
+ throw new Error(
+ `--wait-timeout: gave up after ${opts.timeoutSeconds}s waiting for job ${jobId}; it is still running and can be followed on /jobs`,
+ );
+ }
+ await sleep(status !== null ? 1000 : backoff);
}
}
@@ -325,7 +478,9 @@ async function main() {
}
let worst = 0;
for (const jobId of jobIds) {
- const status = await followJob(jobId, parsed.quiet);
+ const status = await followJob(jobId, parsed.quiet, {
+ timeoutSeconds: parsed.waitTimeout ?? 0,
+ });
console.error(`[${jobId}] ${status}`);
if (status !== "done") worst = 1;
}
@@ -333,7 +488,7 @@ async function main() {
}
// Importable for the arg-parsing tests; only the CLI entry point runs main().
-if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
+if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
main().then(
(code) => process.exit(code),
(e) => {
diff --git a/scripts/archilyzer-ops.test.mjs b/scripts/archilyzer-ops.test.mjs
@@ -5,7 +5,7 @@
// Run with: pnpm test:scripts
import assert from "node:assert/strict";
import test from "node:test";
-import { parseArgs, usage } from "./archilyzer-ops.mjs";
+import { followJob, parseArgs, usage } from "./archilyzer-ops.mjs";
test("no arguments prints usage", () => {
assert.equal(parseArgs([]).help, true);
@@ -118,3 +118,138 @@ test("get tags reads the whole vocabulary, or one tag's assignments", () => {
// The optional argument is per-noun: `get channel` still demands a slug.
assert.match(parseArgs(["get", "channel"]).error, /needs an argument/);
});
+
+// ─── --wait, and the poll that fails ───
+
+test("--wait-timeout is parsed, and refuses a non-number", () => {
+ assert.equal(parseArgs(["build-index", "--wait", "--wait-timeout", "90"]).waitTimeout, 90);
+ assert.equal(parseArgs(["build-index", "--wait-timeout=90"]).waitTimeout, 90);
+ // Default is NONE: the queue may legitimately hold a job for hours.
+ assert.equal(parseArgs(["build-index", "--wait"]).waitTimeout, null);
+ assert.match(parseArgs(["build-index", "--wait-timeout"]).error, /needs a number/);
+ assert.match(parseArgs(["build-index", "--wait-timeout", "soon"]).error, /positive number/);
+ assert.match(parseArgs(["build-index", "--wait-timeout", "0"]).error, /positive number/);
+ assert.match(usage(), /--wait-timeout/);
+});
+
+// A fake editor. `log` is the sequence of log-poll outcomes — an Error is
+// thrown at the caller the way a starved server makes `fetch` reject.
+function fakeEditor({ log = [], active = [] } = {}) {
+ const calls = { log: 0, active: 0 };
+ const reply = (body) => ({ ok: true, json: async () => body });
+ const doFetch = async (url) => {
+ if (url.includes("/api/jobs/active")) {
+ const next = active[Math.min(calls.active, active.length - 1)];
+ calls.active++;
+ if (next instanceof Error) throw next;
+ return reply({ jobs: next });
+ }
+ const next = log[Math.min(calls.log, log.length - 1)];
+ calls.log++;
+ if (next instanceof Error) throw next;
+ return reply(next);
+ };
+ return { doFetch, calls };
+}
+
+const follow = (jobId, editor, opts = {}) =>
+ followJob(jobId, true, {
+ fetch: editor.doFetch,
+ sleep: async () => {},
+ baseUrl: "http://editor",
+ ...opts,
+ });
+
+test("a poll that rejects does not end the follow", async () => {
+ // THE BUG THIS FIXES: one `fetch failed` while an in-process build-index
+ // starved the server used to abort --wait and report a job that was fine.
+ const editor = fakeEditor({
+ log: [
+ { content: "a", nextOffset: 1, status: "running" },
+ new Error("fetch failed"),
+ new Error("fetch failed"),
+ { content: "b", nextOffset: 2, status: "done" },
+ ],
+ });
+ assert.equal(await follow("j1", editor), "done");
+ // Two failures is below the probe threshold, so it never asked.
+ assert.equal(editor.calls.active, 0);
+});
+
+test("after three failures it asks whether the job is still there", async () => {
+ const boom = new Error("fetch failed");
+ // Still listed ⇒ keep waiting, and the follow ends on the status it finally
+ // reads rather than on a guess.
+ const waiting = fakeEditor({
+ log: [boom, boom, boom, { content: "", nextOffset: 0, status: "done" }],
+ active: [[{ id: "j1" }]],
+ });
+ assert.equal(await follow("j1", waiting), "done");
+ assert.equal(waiting.calls.active, 1);
+
+ // Absent from the active list ⇒ it ended while the log was unreachable, so
+ // one final poll reads the terminal status. `failed` is reported, not hidden.
+ const gone = fakeEditor({
+ log: [boom, boom, boom, { content: "", nextOffset: 0, status: "failed" }],
+ active: [[]],
+ });
+ assert.equal(await follow("j1", gone), "failed");
+});
+
+test("a job that is gone AND unreadable refuses to claim an outcome", async () => {
+ const boom = new Error("fetch failed");
+ const editor = fakeEditor({ log: [boom], active: [[]] });
+ await assert.rejects(follow("j1", editor), /lost contact with job j1/);
+});
+
+test("--wait-timeout gives up on a job that never ends", async () => {
+ const editor = fakeEditor({
+ log: [{ content: "", nextOffset: 0, status: "running" }],
+ });
+ let clock = 0;
+ await assert.rejects(
+ follow("j1", editor, { timeoutSeconds: 5, now: () => (clock += 3000) }),
+ /--wait-timeout: gave up after 5s/,
+ );
+});
+
+// The two build routes used to disagree about the spelling of their one
+// argument, so the usage text is where a reader finds out they no longer do.
+test("usage says both build routes take siteId or siteIds", () => {
+ assert.match(usage(), /build-site and build-deploy both take "siteId".*"siteIds"/);
+});
+
+test("a recovered log endpoint answering 'running' is not an outcome", async () => {
+ // THE BUG: the absent-from-active branch returned whatever the final poll
+ // said. A stale active list plus a recovered log endpoint therefore printed
+ // "running" as the job's result and exited 1 for a job that was fine.
+ const boom = new Error("fetch failed");
+ const editor = fakeEditor({
+ log: [
+ boom,
+ boom,
+ boom,
+ { content: "", nextOffset: 0, status: "running" },
+ { content: "", nextOffset: 0, status: "done" },
+ ],
+ active: [[]],
+ });
+ assert.equal(await follow("j1", editor), "done");
+});
+
+test("an editor that answers nothing at all is given up on, not waited on", async () => {
+ // Without --wait-timeout the null-probe path used to wait forever: the log
+ // never answers AND the active list never answers, so nothing ever resets
+ // the failure count and nothing ever concludes.
+ const boom = new Error("fetch failed");
+ const editor = fakeEditor({ log: [boom], active: [boom] });
+ await assert.rejects(follow("j1", editor), /lost contact with the editor/);
+});
+
+test("--wait-timeout implies --wait", () => {
+ // A timeout on a wait nobody asked for is a typo with no effect, not a
+ // preference to honour silently.
+ const p = parseArgs(["build-index", "--wait-timeout", "30"]);
+ assert.equal(p.wait, true);
+ assert.equal(p.waitTimeout, 30);
+});
diff --git a/scripts/worktree.mjs b/scripts/worktree.mjs
@@ -9,6 +9,7 @@
import { execFileSync, spawn } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
+import { pathToFileURL } from "node:url";
// Base ports (offset 0 == main worktree). Mirrors the hardcoded defaults in
// editor/export package.json scripts and the Playwright configs.
@@ -67,6 +68,21 @@ function mainRoot(trees = listWorktrees()) {
return trees.length ? trees[0].path : git(["rev-parse", "--show-toplevel"]);
}
+// WHERE A WORKTREE LIVES, asked the same way by `add` and by `rm`.
+//
+// A branch name may contain "/" and a sibling directory name may not, so `add`
+// has always flattened `tags/site` to `tags-site`. `rm` did not, so
+// `pnpm wt rm tags/site` pointed git at a path nothing had ever created — and
+// that string is exactly what somebody who just ran `add` will type. One
+// function, both callers, so the two spellings cannot drift apart again.
+//
+// An absolute path passes through untouched: `rm` accepts one (it is what
+// `list` prints), and sanitizing it would eat its separators.
+export function worktreeDirFor(main, name) {
+ if (path.isAbsolute(name)) return name;
+ return path.join(path.dirname(main), name.replace(/[^A-Za-z0-9._-]/g, "-"));
+}
+
function offsetForIndex(index) {
return index * OFFSET_STEP;
}
@@ -211,8 +227,7 @@ function cmdAdd(args) {
}
const main = mainRoot();
- const name = branch.replace(/[^A-Za-z0-9._-]/g, "-");
- const dir = path.join(path.dirname(main), name);
+ const dir = worktreeDirFor(main, branch);
if (branchExists(branch)) {
git(["worktree", "add", dir, branch], { stdio: "inherit" });
@@ -280,7 +295,9 @@ function cmdRm(args) {
return 1;
}
const main = mainRoot();
- const dir = path.isAbsolute(name) ? name : path.join(path.dirname(main), name);
+ // Sanitized exactly as `add` sanitized it, so `rm <the branch you added>`
+ // finds the directory `add` actually made.
+ const dir = worktreeDirFor(main, name);
// --share-data worktrees always carry an untracked .worktree-env, so git
// refuses a plain remove; --force handles that (and any other local files).
const rmArgs = ["worktree", "remove", ...(force ? ["--force"] : []), dir];
@@ -328,9 +345,13 @@ async function main() {
}
}
-main()
- .then((code) => process.exit(code ?? 0))
- .catch((err) => {
- process.stderr.write(`${err?.stack ?? err}\n`);
- process.exit(1);
- });
+// Importable for the unit tests (worktreeDirFor is pure and needs no repo);
+// only the CLI entry point runs main(), which shells out to git.
+if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
+ main()
+ .then((code) => process.exit(code ?? 0))
+ .catch((err) => {
+ process.stderr.write(`${err?.stack ?? err}\n`);
+ process.exit(1);
+ });
+}
diff --git a/scripts/worktree.test.mjs b/scripts/worktree.test.mjs
@@ -0,0 +1,46 @@
+// Where `pnpm wt` puts a worktree — the one thing `add` and `rm` have to agree
+// about. Pure: no repo, no git, no filesystem.
+//
+// Run with: pnpm test:scripts
+import assert from "node:assert/strict";
+import test from "node:test";
+import path from "node:path";
+import { worktreeDirFor } from "./worktree.mjs";
+
+const MAIN = "/home/u/Projects/yt-dlp-transcript-browser";
+const SIBLING = path.dirname(MAIN);
+
+test("a branch with a slash becomes one sibling directory", () => {
+ // THE BUG: `add` flattened and `rm` did not, so `pnpm wt rm tags/site` —
+ // the exact string the person had just passed to `add` — pointed git at a
+ // path that had never existed.
+ assert.equal(worktreeDirFor(MAIN, "tags/site"), path.join(SIBLING, "tags-site"));
+ assert.equal(
+ worktreeDirFor(MAIN, "one-core/phase-2/gate-a"),
+ path.join(SIBLING, "one-core-phase-2-gate-a"),
+ );
+});
+
+test("a plain name is unchanged, dots and dashes survive", () => {
+ assert.equal(worktreeDirFor(MAIN, "hotfix"), path.join(SIBLING, "hotfix"));
+ assert.equal(
+ worktreeDirFor(MAIN, "release-1.2_rc"),
+ path.join(SIBLING, "release-1.2_rc"),
+ );
+});
+
+test("an absolute path passes through untouched", () => {
+ // `rm` accepts one because it is what `list` prints; sanitizing it would eat
+ // its separators and aim git at a directory that does not exist.
+ const abs = path.join(SIBLING, "tags-site");
+ assert.equal(worktreeDirFor(MAIN, abs), abs);
+ assert.equal(worktreeDirFor(MAIN, "/tmp/wt/some thing"), "/tmp/wt/some thing");
+});
+
+test("nothing escapes the sibling directory", () => {
+ // A relative name cannot climb out: every "/" and "." run that could form a
+ // traversal is flattened to dashes first.
+ const dir = worktreeDirFor(MAIN, "../../etc/passwd");
+ assert.equal(dir, path.join(SIBLING, "..-..-etc-passwd"));
+ assert.equal(path.dirname(dir), SIBLING);
+});
