commit 78a25c89cfbe0ee48dd462bace8d87d0c74aac72
parent 838cff4a97f9c3a279827b20004344f2f6cefc5f
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Tue, 22 Sep 2026 16:11:39 -0400
ops: both build routes take siteId or siteIds
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 find out which was to get a 400 — a note in plans/curated-tags.md exists to
warn the next reader, which is the tell. `reqSiteIds` in _lib.ts is now the one
reader for both: either key works, both at once is still a 400 (a caller with
two ideas about what to build should not have one picked for it), neither is a
400 naming both spellings.
build-deploy loops the way build-site does and returns the same
{ok, jobs, skipped} plus `jobId` when exactly one job started — additive, so
every single-site caller and `pnpm ops --wait` (which already reads jobs[]) is
unchanged. Queueing NOTHING stays a 400 carrying the reason rather than a
cheerful {ok:true, jobs:[]}: --wait would exit 0 on that and report success
about a deploy that never began, which is the bug queueResponse's jobIds were
added to kill.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
6 files changed, 191 insertions(+), 40 deletions(-)
diff --git a/editor/app/api/ops/_lib.ts b/editor/app/api/ops/_lib.ts
@@ -158,6 +158,29 @@ 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.
+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');
+ }
+ if (one !== undefined) return [reqString(body, "siteId")];
+ if (many !== undefined) return reqStringArray(body, "siteIds");
+ throw new OpsInputError(
+ '"siteId" (a string) or "siteIds" (a non-empty array of strings) is required',
+ );
+}
+
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,82 @@ 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)) {
+ const result = await buildAndDeployAction(siteId, skipArchives);
+ 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,43 +3,45 @@ 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())) {
+ for (const siteId of reqSiteIds(body)) {
const result = await buildExportAction(
siteId,
undefined,
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,64 @@ 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.
+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");
+ }
+});
+
+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/scripts/archilyzer-ops.mjs b/scripts/archilyzer-ops.mjs
@@ -31,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
@@ -237,6 +239,8 @@ export function usage() {
"--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");
diff --git a/scripts/archilyzer-ops.test.mjs b/scripts/archilyzer-ops.test.mjs
@@ -212,3 +212,9 @@ test("--wait-timeout gives up on a job that never ends", async () => {
/--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"/);
+});
