commit a1d030f998762155f9a96d8ebd227121af5bf6b7
parent 68bdda4abfc6a81fe8e4d9445fb5489fa5b96716
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sun, 20 Sep 2026 17:51:22 -0400
Report a misspelled config field before a missing channel
`{ slug: "typo", patch: { downloadFilterExcluded: … } }` answered "Channel not
found" and never mentioned the key that was actually wrong. A field name is a
fact about the REQUEST, so its check is now a pure pass the route runs before it
looks the channel up.
Also harden two races in the spec: the job-meta sidecar is written with a void
promise after enqueue (poll it), and snapshot.json is removed rather than
assumed absent, since a debounced regen from the previous spec can land between
resetData's copy and the assertion.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
3 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/editor/app/api/ops/channel-config/route.ts b/editor/app/api/ops/channel-config/route.ts
@@ -3,6 +3,7 @@ import { readChannelConfig } from "yt-dlp-transcript-common/controller/channels"
import {
applyChannelFormPatch,
channelConfigToFormData,
+ validateChannelFormPatch,
} from "../../../channels/components/channelConfigToForm";
import { updateChannelAction } from "../../../channels/actions";
import { actionResponse, OpsInputError, ops, reqString } from "../_lib";
@@ -29,15 +30,18 @@ export async function POST(request: Request) {
) {
throw new OpsInputError('"patch" must be a JSON object');
}
- const paths = getPaths();
- const existing = await readChannelConfig(paths, slug);
- if (!existing) throw new OpsInputError(`Channel "${slug}" not found`);
- const fd = channelConfigToFormData(existing);
+ // KEYS FIRST, CHANNEL SECOND. A misspelled field is a fact about the
+ // request; reporting "channel not found" for it would hide the real error.
try {
- applyChannelFormPatch(fd, patch as Record<string, unknown>);
+ validateChannelFormPatch(patch as Record<string, unknown>);
} catch (e) {
throw new OpsInputError((e as Error).message);
}
+ const paths = getPaths();
+ const existing = await readChannelConfig(paths, slug);
+ if (!existing) throw new OpsInputError(`Channel "${slug}" not found`);
+ const fd = channelConfigToFormData(existing);
+ applyChannelFormPatch(fd, patch as Record<string, unknown>);
return actionResponse(await updateChannelAction(slug, undefined, fd));
});
}
diff --git a/editor/app/channels/components/channelConfigToForm.ts b/editor/app/channels/components/channelConfigToForm.ts
@@ -134,13 +134,37 @@ export function applyChannelFormPatch(
fd: FormData,
patch: ChannelConfigPatch,
): void {
+ validateChannelFormPatch(patch);
+ for (const [key, value] of Object.entries(patch)) {
+ if ((CHANNEL_FORM_FLAGS as readonly string[]).includes(key)) {
+ if (value) fd.set(key, "on");
+ else fd.delete(key);
+ continue;
+ }
+ if (key === "ytdlpExtraArgs" && Array.isArray(value)) {
+ const joined = (value as string[]).join("\n");
+ if (joined.trim()) fd.set(key, joined);
+ else fd.delete(key);
+ continue;
+ }
+ if (value === null || value === "") {
+ fd.delete(key);
+ continue;
+ }
+ fd.set(key, typeof value === "number" ? String(value) : (value as string));
+ }
+}
+
+// SHAPE ONLY, AND SEPARATE ON PURPOSE: a misspelled field name is a fact about
+// the REQUEST, so the route checks it before it looks the channel up. Otherwise
+// `{ slug: "typo", patch: { downloadFilterExcluded: … } }` reports the channel
+// and never mentions the key that was actually wrong.
+export function validateChannelFormPatch(patch: ChannelConfigPatch): void {
for (const [key, value] of Object.entries(patch)) {
if ((CHANNEL_FORM_FLAGS as readonly string[]).includes(key)) {
if (typeof value !== "boolean") {
throw new Error(`"${key}" must be a boolean`);
}
- if (value) fd.set(key, "on");
- else fd.delete(key);
continue;
}
if (!(CHANNEL_FORM_VALUES as readonly string[]).includes(key)) {
@@ -152,22 +176,11 @@ export function applyChannelFormPatch(
if (value.some((v) => typeof v !== "string")) {
throw new Error(`"${key}" must be an array of strings`);
}
- const joined = (value as string[]).join("\n");
- if (joined.trim()) fd.set(key, joined);
- else fd.delete(key);
continue;
}
- if (value === null || value === "") {
- fd.delete(key);
+ if (value === null || typeof value === "number" || typeof value === "string") {
continue;
}
- if (typeof value === "number") {
- fd.set(key, String(value));
- continue;
- }
- if (typeof value !== "string") {
- throw new Error(`"${key}" must be a string, a number, or "" to clear it`);
- }
- fd.set(key, value);
+ throw new Error(`"${key}" must be a string, a number, or "" to clear it`);
}
}
diff --git a/editor/e2e/ops-api.spec.ts b/editor/e2e/ops-api.spec.ts
@@ -14,6 +14,7 @@
// first branch, shared with /api/worker/* and unit-tested by nothing else
// either. See plans/FACTS.md.
+import { rm } from "node:fs/promises";
import { test, expect, type APIRequestContext } from "@playwright/test";
import { baseUrl } from "./baseUrl";
import {
@@ -21,6 +22,7 @@ import {
pathExists,
readJson,
resetData,
+ resolvePath,
writeSettings,
} from "./helpers";
@@ -247,11 +249,16 @@ test("metadata-scan starts a job, and the job says it is a metadata-scan", async
// THE ROUTE DOES NOT STREAM, so the id is the whole contract: the caller
// follows the same log endpoint the editor's own panel polls.
- const meta = await readJson<{ kind: string; channelSlug: string }>(
- `test-transcripts/.jobs/${body.jobId}.meta.json`,
- );
- expect(meta.kind).toBe("metadata-scan");
- expect(meta.channelSlug).toBe(SLUG);
+ // The sidecar is written with a `void` promise right after enqueue, so poll.
+ const metaPath = `test-transcripts/.jobs/${body.jobId}.meta.json`;
+ await expect
+ .poll(async () => {
+ const meta = await readJson<{ kind: string; channelSlug: string }>(
+ metaPath,
+ ).catch(() => null);
+ return meta ? `${meta.kind}/${meta.channelSlug}` : null;
+ })
+ .toBe(`metadata-scan/${SLUG}`);
const log = await request.get(
`${baseUrl}/api/jobs/${body.jobId}/log?from=0`,
@@ -275,8 +282,10 @@ test("refresh-report regenerates snapshot.json", async ({ request }) => {
await settings();
const SLUG = "test-filter";
const SNAP = `test-transcripts/channels/${SLUG}/snapshot.json`;
- // The fixture ships no report — that is what makes the first call's effect
- // unambiguous rather than a timestamp comparison.
+ // Removed rather than assumed absent: a debounced regen armed by the previous
+ // spec can land between resetData's copy and here, and the point of the
+ // assertion below is the ROUTE's effect, not the scheduler's.
+ await rm(resolvePath(SNAP), { force: true });
expect(await pathExists(SNAP)).toBe(false);
// No page, no click: the route IS the refresh. It regenerates SYNCHRONOUSLY
