fix(i18n): every refusal the API answers is structured and said in the reader's language

~150 HTTPExceptions carried a hand-written English sentence as their detail,
shown verbatim under the Hungarian UI — in the blocking error dialog and on
every inline surface that printed HttpError.detail (sign-in, setup wizard,
change password, the passphrase gate, live search). Four also quoted an
upstream exception's text back to the user.

- app/refusal.py: `refused(status, reason, **params)` → {code: "refused",
  reason, …params}; localizeDetail translates errors.refused.<reason> with the
  params interpolated. "Unknown X" 404s use the existing not_found(what).
- sysconfig.set_value raises InvalidSetting (reason + bounds) instead of an
  English ValueError; the three writers answer it through setting_refused.
- The Plex connection/write/image failures log what the upstream said and
  answer a reason; the search scraper's failure is logged and answered as
  youtube/unavailable (it crashed refusal_reason before).
- HttpError.detail now only ever holds a translated reason: a string detail
  (only the framework's own can remain) is left out, so reading .detail is
  safe everywhere — including destructured, the hole the raw-detail source
  scan could not see. That scan and its five-file exception list are gone.
- Enforced: test_http_details.py fails on any HTTPException with a sentence
  for a detail; the contract test requires every refused reason and every
  not_found "what" in both languages, and no key for one that is gone.
- local/jsx-comment-text: a // inside JSX children renders as text; the rule
  is ten lines in eslint.config.js rather than eslint-plugin-react's ~1800
  lockfile lines.

frontend/src/lib/actionErrors.contract.test.ts

@@ -1,109 +0,0 @@
-import { readFileSync } from "node:fs";
-import { describe, expect, it } from "vitest";
-
-// One contract, one implementation — `notifyActionError` (see actionErrors.ts).
-//
-// `req` deliberately leaves 403/404/429 to the caller, and the routes behind those statuses
-// hand-write English. Round 5 taught the shared helper to discard a plain-string `detail` in favour
-// of the caller's already-translated fallback. Round 6 then found `ChatThread` carrying a SECOND,
-// hand-written copy of the same contract (`e.detail || t("messages.sendFailed")`) — so the central
-// fix could not reach it and the English sentence stayed in a Hungarian UI. A fix that lands in one
-// place while a copy of the rule lives in another is this repo's recurring smell; this is the check
-// that names the copies.
-//
-// The assertion pins the expected SET, never a count: a guard that counts is green the moment its
-// glob stops matching (E10 review round 4 learned that the hard way).
-
-// Reads a raw `HttpError.detail` and uses it as the message a user sees. Matches `e.detail ||`,
-// `err.detail ? err.detail :`, `(e instanceof HttpError ? e.detail : undefined) ??`, a cast
-// receiver (`(e as HttpError).detail`), an optional chain (`e?.detail`) and a `.detail` passed
-// straight as a call argument (`setErr(e.detail)`).
-//
-// The receiver alternation and the absence of a `)` lookahead are both load-bearing. The first
-// version required a bare identifier before `.detail` and excluded `.detail)` — so `setErr(e.detail)`,
-// `String(e.detail)` and `(e as HttpError).detail` were all invisible, and because the file SET is
-// compared with `toEqual` a file the pattern cannot see is simply never listed: the guard stayed
-// green over a fresh copy of the contract written in any of those shapes (round 7). The `)`
-// exclusion bought nothing — the only `.detail)` in the frontend is inside `lib/api/core.ts`, which
-// `IMPLEMENTATION` skips wholesale — and re-scanning with the wider pattern returns the same five
-// files, so it removes no false positive.
-const RAW_DETAIL = /(?:\)|[A-Za-z_$][\w$]*)\s*\??\.detail\b(?!\s*=[^=])/;
-// KNOWN LIMIT: a `detail` pulled out by destructuring (`const { detail } = err`) is out of reach
-// of a line-level scan — by then there is no receiver left to match, and a pattern loose enough
-// to catch it fires on every unrelated `detail` field too (measured: `const { data: detail } =
-// useQuery(...)` in VideoInfoCard and an options destructure in plexPlayerHooks, neither of which
-// touches an HttpError). Naming the hole beats pinning two innocent files into the list below as
-// though they were copies of the contract.
-
-// Every place that still prefers the server's raw string over a translated fallback. These are
-// INLINE surfaces — a form field's error line, a setup step's banner — not toasts, and each shows a
-// reason the user is mid-flow on, so converting them is a sweep of its own (with the ~60 backend
-// routes that hand-write those strings). Logged on ROADMAP.md under the bug-hunt deferrals. What
-// must not happen is a NEW one, or a toast path growing one again.
-const KNOWN_RAW_DETAIL_CONSUMERS = [
-  "components/Feed.tsx", // the live-YouTube search error line
-  "components/KeyGate.tsx", // the passphrase field's own refusal
-  "components/SettingsPanel.tsx", // the change-password field
-  "components/SetupWizard.tsx", // the first-run wizard's banner
-  "components/Welcome.tsx", // the sign-in card's refusal panel
-];
-
-// Where the rule itself lives, plus the request layer that produces and localizes the value.
-const IMPLEMENTATION = ["lib/actionErrors.ts", "lib/api/core.ts"];
-
-function consumers(): { hits: string[]; scanned: number } {
-  const files = import.meta.glob("../**/*.{ts,tsx}", {
-    query: "?raw",
-    import: "default",
-    eager: true,
-  });
-  const hits: string[] = [];
-  for (const [rel, source] of Object.entries(files)) {
-    // Vite resolves the glob against THIS file, so same-directory hits come back as "./x".
-    const path = rel.startsWith("./") ? `lib/${rel.slice(2)}` : rel.replace(/^\.\.\//, "");
-    if (/\.test\.tsx?$|\.d\.ts$/.test(path)) continue;
-    if (IMPLEMENTATION.includes(path)) continue;
-    const lines = String(source)
-      .split("\n")
-      .filter((l) => !/^\s*(\/\/|\*|\/\*)/.test(l)); // a comment about the rule is not a copy of it
-    if (lines.some((l) => RAW_DETAIL.test(l))) hits.push(path);
-  }
-  return { hits: hits.sort(), scanned: Object.keys(files).length };
-}
-
-describe("the refused-write contract", () => {
-  it("has no copies beyond the ones on the deferral list", () => {
-    const { hits, scanned } = consumers();
-    // The GLOB must have matched something, or this whole file is a green no-op — and the old
-    // version of this guard did not check that: it read `actionErrors.ts` by its own URL, which
-    // resolves whether or not `import.meta.glob` enumerated a single file. That only mattered
-    // because the pinned set was non-empty; the moment the deferral sweep below lands and the list
-    // becomes `[]`, `expect([]).toEqual([])` would pass over a glob that matched nothing (round 7).
-    expect(scanned).toBeGreaterThan(100);
-    expect(readFileSync(new URL("./actionErrors.ts", import.meta.url), "utf8")).toContain(
-      "detailData",
-    );
-    expect(hits).toEqual(KNOWN_RAW_DETAIL_CONSUMERS);
-  });
-
-  it("sees a raw detail written in any of the shapes this codebase uses", () => {
-    // The pattern is the whole guard, so pin the pattern itself: each of these is a real form
-    // found in this frontend or in its sibling PRs, and every one of the first five was invisible
-    // to the original regex.
-    for (const line of [
-      "  setErr(e.detail);",
-      "  reportError(err.detail);",
-      "  notify({ message: (e as HttpError).detail });",
-      "  toast(String(e.detail));",
-      "  notify({ message: (err as HttpError).detail ?? fallback });",
-      '  setError(e.detail ?? t("x"));',
-      '  const msg = err.detail || t("x");',
-      "  setErr(state.err?.detail);",
-    ])
-      expect(RAW_DETAIL.test(line), line).toBe(true);
-    // And it must not fire on the places that PRODUCE or localize the value, which is why
-    // `IMPLEMENTATION` exists rather than a cleverer pattern: an assignment is not a read.
-    for (const line of ["  detail = await r.json();", "  this.detail = detail;"])
-      expect(RAW_DETAIL.test(line), line).toBe(false);
-  });
-});

frontend/src/lib/actionErrors.ts

@@ -9,11 +9,13 @@ const GATEWAY = new Set([502, 503, 504]);
 /**
  * WHY a write failed, in the reader's language — the line under the toast's title.
  *
- * A STRUCTURED server detail (`{code, reason}`) wins: `req` has already run it through
+ * The server's own reason wins: `req` has already run the structured detail through
  * `localizeDetail`, and it is the most specific thing there is to say (quota numbers, which row is
- * gone). A plain-string `detail` never does — it is whatever English the route hand-wrote, and
- * `reportFailure` leaves exactly 403/404/429 to the caller, so those used to put "No user with that
- * email." into a Hungarian UI (deep-review round 5). Otherwise the status says it.
+ * gone). Every refusal the app answers is structured now (backend `app/refusal.py`), and a string
+ * detail never reaches `HttpError.detail` (see there) — the hand-written English that once put
+ * "No user with that email." into a Hungarian UI has nowhere left to come from. `detailData` still
+ * gates it: status-0 errors carry a local sentinel in `detail`, not a reason. Otherwise the status
+ * says it.
  */
 export function failureReason(err: unknown): string {
   const t = i18n.t.bind(i18n);

frontend/src/lib/api/core.test.ts

@@ -123,6 +123,17 @@ describe("uploadReq", () => {
     expect(reportError).toHaveBeenCalledWith("downloads.errors.upload.tooLarge");
   });
 
+  it("offers no string detail as the reason on the upload path either", async () => {
+    const p = uploadReq("/u", form());
+    const x = FakeXHR.last;
+    x.status = 405;
+    x.responseText = '{"detail":"Method Not Allowed"}';
+    x.onload!();
+    const err = (await p.catch((e: unknown) => e)) as HttpError;
+    expect(err).toBeInstanceOf(HttpError);
+    expect(err.detail).toBeUndefined();
+  });
+
   it("reports a connectivity failure on a network error", async () => {
     const p = uploadReq("/u", form());
     FakeXHR.last.onerror!();
@@ -168,6 +179,23 @@ describe("req — a structured refusal reads in the user's language", () => {
     });
   });
 
+  it("never offers a string detail as the reason — only a structured one, translated", async () => {
+    // Every refusal the app answers is structured; a string can only be the framework's own English
+    // ("Method Not Allowed"). Surfaces print `err.detail` — some destructured, out of any source
+    // scan's reach — so the guarantee has to live here, where the value is made.
+    vi.spyOn(globalThis, "fetch").mockResolvedValue(
+      new Response(JSON.stringify({ detail: "Method Not Allowed" }), {
+        status: 405,
+        headers: { "content-type": "application/json" },
+      }),
+    );
+    const err = (await req("/api/x", { method: "POST" }, { quiet: true }).catch(
+      (e: unknown) => e,
+    )) as HttpError;
+    expect(err).toBeInstanceOf(HttpError);
+    expect(err.detail).toBeUndefined();
+  });
+
   it.each(["quota", "reconnect", "unavailable"])(
     "says what to do about an action YouTube refused (%s)",
     async (reason) => {

frontend/src/lib/api/core.ts

@@ -50,7 +50,12 @@ export function isAborted(err: unknown): boolean {
 
 export class HttpError extends Error {
   status: number;
-  detail?: string; // server-provided reason (FastAPI's `detail`), when present
+  // The server's reason IN THE READER'S LANGUAGE, when it gave one — i.e. a structured detail run
+  // through `localizeDetail`, never the raw text of a string detail. Every refusal the app answers is
+  // structured (backend `app/refusal.py`, enforced by its test_http_details.py); a string can only be
+  // the framework's own ("Method Not Allowed"), so it is left out rather than shown. That is what
+  // makes reading `.detail` safe anywhere — including destructured, which no source scan could see.
+  detail?: string;
   detailData?: unknown; // structured detail ({ code, reason, ... }) when the server sent an object
   constructor(status: number, detail?: string, detailData?: unknown) {
     super(detail || `HTTP ${status}`);
@@ -133,6 +138,11 @@ function localizeDetail(d: unknown): string {
   // wait for the budget, reconnect, or retry. An unknown reason reads as the retryable one.
   if (o?.code === "youtube")
     return t(`errors.youtube.${o.reason}`, t("errors.youtube.unavailable"));
+  // Every other refusal (backend `app/refusal.py`): one family, the reason naming the sentence and
+  // the rest of the detail its values (`{{min}}`, `{{name}}`, …). A reason this build does not know
+  // reads as the generic failure rather than a key.
+  if (o?.code === "refused")
+    return t(`errors.refused.${o.reason}`, { ...o, defaultValue: t("errors.generic") });
   // A playlist action the server refused (Watch later can't be deleted, YouTube said no, …).
   if (o?.code === "playlist") return t(`playlists.errors.${o.reason}`, t("errors.generic"));
   // An admin action on an account or an invite that the server refused: activating an address that
@@ -326,8 +336,8 @@ export async function req<T = unknown>(
       let detailData: unknown;
       try {
         const body = await r.json();
-        if (body && typeof body.detail === "string") detail = body.detail;
-        else if (body && body.detail && typeof body.detail === "object") {
+        // A string detail is not the app's (see `HttpError.detail`) and is not shown.
+        if (body && body.detail && typeof body.detail === "object") {
           detailData = body.detail;
           detail = localizeDetail(body.detail); // structured error → localized message
         }
@@ -393,8 +403,8 @@ export function uploadReq<T = unknown>(
       let detailData: unknown;
       try {
         const body = JSON.parse(xhr.responseText);
-        if (body && typeof body.detail === "string") detail = body.detail;
-        else if (body && body.detail && typeof body.detail === "object") {
+        // A string detail is not the app's (see `HttpError.detail`) and is not shown.
+        if (body && body.detail && typeof body.detail === "object") {
           detailData = body.detail;
           detail = localizeDetail(body.detail);
         }

frontend/src/lib/jsxCommentText.contract.test.ts

@@ -0,0 +1,30 @@
+import { fileURLToPath } from "node:url";
+
+import { ESLint } from "eslint";
+import { describe, expect, it } from "vitest";
+
+// `local/jsx-comment-text` (eslint.config.js): a `//` inside JSX children is rendered as text. One
+// reached the Playlists page past tsc, eslint and every test — only a visual check caught it. A rule
+// that matches nothing looks exactly like a clean codebase, so this asserts it fires.
+const RULE = "local/jsx-comment-text";
+const FRONTEND = fileURLToPath(new URL("../../", import.meta.url));
+
+async function hits(source: string): Promise<number> {
+  const eslint = new ESLint({ cwd: FRONTEND });
+  const [result] = await eslint.lintText(source, { filePath: "src/components/__probe.tsx" });
+  return (result?.messages ?? []).filter((m) => m.ruleId === RULE).length;
+}
+
+describe("a comment inside JSX children", () => {
+  it("is refused — it would render as text", async () => {
+    expect(
+      await hits(`export const A = () => (\n  <>\n    // the list\n    <ul />\n  </>\n);\n`),
+    ).toBe(1);
+    expect(await hits(`export const B = () => <div>/* note */</div>;\n`)).toBe(1);
+  });
+
+  it("is fine as a real comment, and text that merely contains a slash pair is text", async () => {
+    expect(await hits(`export const C = () => <div>{/* the list */}</div>;\n`)).toBe(0);
+    expect(await hits(`export const D = () => <p>See https://example.com</p>;\n`)).toBe(0);
+  });
+});