fix(i18n): hold every refusal reason by type, and refuse what the scan cannot read

Review round 1 of the refusal contract: it claimed more than it held. - The other reason carriers get their Literal too (BlockedUrl, ClipError, FolderError, the playlist and sign-in refusal helpers), so the type checker refuses a reason passed by variable or an unknown literal, and the contract reads one shape. - A literal reason in a detail dict built anywhere counts as a reason of its code; a code the scan cannot read, a refused() whose reason it cannot read, and a new reason passed by variable each fail the contract until listed with what holds them. - A sign-in refusal key the backend no longer sends is caught too. - localizeDetail's numbers (download cap in GB, upload cap in bytes) and the shared-sentence fallbacks are tested in both languages. - One Literal parser in the contract; a stale comment fixed.
2026-10-31 01:00:26 +01:00
parent 4379f76747
commit ceadf69e43
8 changed files with 213 additions and 82 deletions
@@ -22,6 +22,7 @@ import json
 import math
 import re
 from pathlib import Path
+from typing import Literal

 from app.downloads.edit import AAC_ARGS, EVEN_FRAME, INPUT_GUARD, X264_ARGS
 from app.plex.sync import HDR_TRANSFERS
@@ -57,11 +58,26 @@ _CROP_ASPECTS = (0.25, 4.0)
 _EDGE_SLACK = 1e-3


+# Every reason a clip is refused; the UI names each one (`clip.errors.<reason>`, held by the frontend's
+# backendIds contract, which reads this Literal).
+ClipReason = Literal[
+    "bad_source",
+    "bad_range",
+    "too_short",
+    "too_long",
+    "bad_audio",
+    "bad_crop",
+    "not_found",
+    "media_missing",
+    "past_end",
+]
+
+
 class ClipError(Exception):
    """A clip request that cannot be cut; `reason` is an i18n-mappable key, `params` the numbers its
    message names (the cap itself, so the client never says a stale one)."""

-    def __init__(self, reason: str, **params: float):
+    def __init__(self, reason: ClipReason, **params: float):
        self.reason = reason
        self.params = params
        super().__init__(reason)
@@ -24,6 +24,7 @@ kept it — and its `ready_at`, the moment it finished (`stamp_done`).

 from datetime import datetime, timedelta, timezone
 from enum import StrEnum
+from typing import Literal
 from urllib.parse import urlsplit

 from sqlalchemy import select
@@ -181,10 +182,20 @@ def stamp_done(db: Session, job: DownloadJob) -> None:
    job.expiry_warned = False


+# Every folder change the rules refuse, answered as a generic refusal; the UI names each one
+# (`errors.refused.<reason>`, held by the frontend's backendIds contract, which reads this Literal).
+FolderReason = Literal[
+    "file_folder_gone",
+    "file_folder_name_taken",
+    "file_folder_not_its_source",
+    "file_folder_not_own",
+]
+
+
 class FolderError(Exception):
    """A folder change the rules refuse; `reason` is the refusal the route answers with."""

-    def __init__(self, reason: str) -> None:
+    def __init__(self, reason: FolderReason) -> None:
        super().__init__(reason)
        self.reason = reason

@@ -33,22 +33,31 @@ user-controlled destination to guard.

 import ipaddress
 import socket
+from typing import Literal
 from urllib.parse import urlparse

 # Reason codes travel to the client inside the structured 422 detail, so they are part of the API
-# vocabulary (frontend `localizeDetail`) — keep them stable.
-REASON_SCHEME = "scheme"  # not http(s)
-REASON_NO_HOST = "no_host"  # URL carries no hostname
-REASON_MALFORMED = "malformed"  # unparseable authority or port; urlparse raises late, so guard it
-REASON_UNRESOLVABLE = "unresolvable"  # DNS said nothing (honest: not the same as "blocked")
-REASON_PRIVATE = "private"  # resolves into a network the server must not fetch from
+# vocabulary — keep them stable. The UI names each one (`downloads.errors.urlBlocked.<reason>`, held by
+# the frontend's backendIds contract, which reads this Literal).
+UrlBlockedReason = Literal["scheme", "no_host", "malformed", "unresolvable", "private"]
+REASON_SCHEME: UrlBlockedReason = "scheme"  # not http(s)
+REASON_NO_HOST: UrlBlockedReason = "no_host"  # URL carries no hostname
+REASON_MALFORMED: UrlBlockedReason = (
+    "malformed"  # unparseable authority or port; urlparse raises late
+)
+REASON_UNRESOLVABLE: UrlBlockedReason = (
+    "unresolvable"  # DNS said nothing (not the same as "blocked")
+)
+REASON_PRIVATE: UrlBlockedReason = (
+    "private"  # resolves into a network the server must not fetch from
+)


 class BlockedUrl(Exception):
    """A user-supplied URL that must not be fetched server-side. ``reason`` is one of the
    REASON_* codes above."""

-    def __init__(self, reason: str):
+    def __init__(self, reason: UrlBlockedReason):
        super().__init__(reason)
        self.reason = reason

@@ -11,7 +11,8 @@ A test (`test_http_details.py`) reads every `HTTPException` in the app and fails
 and the frontend's contract test (`i18n/backendIds.contract.test.ts`) requires every `code` a detail
 carries to be in one record of `lib/api/refusalCodes.ts`, and every reason it can be sent with — a
 literal, or a `Literal` type when it travels in a variable — to have its sentence in both languages.
-So the rule is enforced, not described.
+A code or reason that scan cannot read fails it until it is listed with what holds it. So the rule is
+enforced, not described.
 """

 from fastapi import HTTPException
@@ -976,7 +976,16 @@ def _resend_verify_if_eligible(email: str) -> None:
            send_verification_link(db, user)


-def _login_refused(status: int, reason: str, user: User | None = None, **extra) -> HTTPException:
+# Every refusal a password sign-in answers with; the UI names each one (`welcome.auth.blocked.<reason>`,
+# held by the frontend's backendIds contract, which reads this Literal).
+LoginRefusal = Literal[
+    "rate_limited", "invalid", "suspended", "email_unverified", "pending_approval"
+]
+
+
+def _login_refused(
+    status: int, reason: LoginRefusal, user: User | None = None, **extra
+) -> HTTPException:
    """Every refusal password_login can answer with, in the codebase's structured `{code, reason}`
    shape so the SPA can say it in the user's own language (the old bare English strings were
    unreadable under a bilingual UI, and unusable as anything but a one-line error).
@@ -3,7 +3,7 @@ CRUD (create / rename / delete, add / remove / reorder items) plus YouTube two-w
 mirror the user's own YouTube playlists in (`/sync-youtube`) and push/revert local edits back
 out (`/{id}/push-plan`, `/{id}/revert-youtube`), behind the optional write scope."""

-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Literal

 from fastapi import APIRouter, Depends, HTTPException
 from pydantic import BaseModel, Field
@@ -53,7 +53,22 @@ def _check_pull_cooldown(user: User) -> None:
        raise HTTPException(status_code=429, detail={"code": "sync_cooldown"})


-def _refused(status: int, reason: str) -> HTTPException:
+# Every playlist action the server refuses; the UI names each one (`playlists.errors.<reason>`, held by
+# the frontend's backendIds contract, which reads this Literal).
+PlaylistRefusal = Literal[
+    "name_required",
+    "not_on_youtube",
+    "quota_exceeded",
+    "sync_all_busy",
+    "sync_busy",
+    "watch_later_not_pushable",
+    "watch_later_undeletable",
+    "youtube_editing_off",
+    "youtube_read_scope",
+]
+
+
+def _refused(status: int, reason: PlaylistRefusal) -> HTTPException:
    """A playlist action the server refuses, as a reason the UI translates
    (`playlists.errors.<reason>`) — a bare English detail reached the Hungarian UI verbatim."""
    return HTTPException(status_code=status, detail={"code": "playlist", "reason": reason})
@@ -90,9 +90,9 @@ const runEnum = (name: string) =>
 const runStatuses = runEnum("RunStatus");
 const runTriggers = runEnum("RunTrigger");
 const quotaActions = enumValues("quota.py");
-// The admin actions' refusal reasons → `errors.admin.<reason>`. The codes are read from
-// `localizeDetail`'s own ADMIN_REFUSALS, so a code added there is scanned here too; a detail dict
-// ruff wraps over several lines still matches.
+// The admin actions' refusal reasons → `errors.admin.<reason>`. The codes are the refusal records
+// under that prefix (lib/api/refusalCodes), so a code recorded there is scanned here too; a detail
+// dict ruff wraps over several lines still matches.
 const adminCodes = Object.entries(REFUSAL_FAMILIES)
  .filter(([, f]) => f.prefix === "errors.admin")
  .map(([code]) => code);
@@ -172,29 +172,38 @@ const goneWhats = [
    ),
  ),
 ];
-// Every generic refusal → `errors.refused.<reason>`: each `refused(<status>, "<reason>")` anywhere in
-// the backend (not `_refused`, a router's own family), plus the values of the settings map, which
-// `setting_refused` picks from at runtime, and every `FolderError` reason.
-const appSources = (dir = ""): string[] =>
+// A backend `Name = Literal["a", "b"]` alias, by name: the reasons a typed parameter accepts, which
+// basedpyright then holds at every call site — a reason passed by variable included.
+const literalOf = (file: string, name: string) =>
+  [
+    ...(
+      backend(file).match(new RegExp(`^${name} = Literal\\[([^\\]]*)\\]`, "m"))?.[1] ?? ""
+    ).matchAll(/"(\w+)"/g),
+  ].map((m) => m[1]!);
+// Every backend source, by its path under app/.
+const appPaths = (dir = ""): string[] =>
  readdirSync(new URL(`../../../backend/app/${dir}`, import.meta.url), {
    withFileTypes: true,
  }).flatMap((e) =>
    e.isDirectory()
-      ? appSources(`${dir}${e.name}/`)
+      ? e.name === "__pycache__"
+        ? []
+        : appPaths(`${dir}${e.name}/`)
      : e.name.endsWith(".py")
-        ? [backend(`${dir}${e.name}`)]
+        ? [`${dir}${e.name}`]
        : [],
  );
+const appSources = (): string[] => appPaths().map(backend);
+// Every generic refusal → `errors.refused.<reason>`: each `refused(<status>, "<reason>")` anywhere in
+// the backend (not `_refused`, a router's own family), the values of the settings map, which
+// `setting_refused` picks from at runtime, and the folder rules' `FolderReason`, answered with
+// `refused(409, e.reason)`.
 const refusedReasons = [
  ...new Set([
    ...appSources().flatMap((src) =>
      [...src.matchAll(/(?<![\w.])refused\(\s*\d+,\s*"(\w+)"/g)].map((m) => m[1]!),
    ),
-    // A folder change the rules refuse travels as `FolderError("<reason>")` and is answered with
-    // `refused(409, e.reason)`, which the pattern above cannot read.
-    ...appSources().flatMap((src) =>
-      [...src.matchAll(/FolderError\("(\w+)"\)/g)].map((m) => m[1]!),
-    ),
+    ...literalOf("downloads/folders.py", "FolderReason"),
    ...[
      ...(backend("refusal.py").split("_SETTING_REASONS = {")[1]?.split("}")[0] ?? "").matchAll(
        /:\s*"(\w+)"/g,
@@ -202,61 +211,90 @@ const refusedReasons = [
    ].map((m) => m[1]!),
  ]),
 ];
-// A refused YouTube action → `errors.youtube.<reason>`, the reasons read from their Literal.
-const youtubeReasons = [
-  ...(
-    backend("youtube/client.py").match(/^YouTubeRefusal = Literal\[([^\]]*)\]/m)?.[1] ?? ""
-  ).matchAll(/"(\w+)"/g),
-].map((m) => m[1]!);
-// A clip the server will not cut → `clip.errors.<reason>`: every `ClipError("<reason>"…)` in the backend
-// (a reason may carry its numbers after it: `ClipError("too_long", limit_s=…)`).
-const clipReasons = [
-  ...new Set(
-    appSources().flatMap((src) => [...src.matchAll(/ClipError\("(\w+)"[,)]/g)].map((m) => m[1]!)),
-  ),
-];
+// A `refused(…)` whose reason the scan above cannot read. Only two may: a folder rule's `e.reason`
+// (the nearest `except` above it catches `FolderError`, typed `FolderReason`) and the settings map.
+const opaqueRefused = appPaths().flatMap((f) => {
+  const src = backend(f);
+  return [...src.matchAll(/(?<![\w.])refused\(\s*\d+,\s*(?![\s"])([^,)]+)/g)]
+    .filter((m) => {
+      const arg = m[1]!.trim();
+      if (f === "refusal.py" && arg.startsWith("_SETTING_REASONS[")) return false;
+      const caught = [...src.slice(0, m.index).matchAll(/except ([^\n:]*):/g)].pop()?.[1] ?? "";
+      return !(arg === "e.reason" && /FolderError as e$/.test(caught));
+    })
+    .map((m) => `${f}: ${m[0]}`);
+});
+const youtubeReasons = literalOf("youtube/client.py", "YouTubeRefusal");
+const clipReasons = literalOf("downloads/clip.py", "ClipReason");

-// Every refusal code the backend answers: each `"code": "<x>"` of a detail anywhere in the app. A
-// subtitle's status payload (`{"state": …, "code": …}`) is the player's own vocabulary, not a refusal.
-const backendCodes = [
+// Every `"code": …` a backend dict carries, with the `"reason": …` that follows it. Two modules use a
+// `code` key for something that is not a refusal; everything else is one.
+const NOT_REFUSALS: Record<string, string> = {
+  "plex/subtitles.py":
+    "the player's subtitle status vocabulary (`/status`), read by the player itself",
+  "plex/tmdb.py": "a production country's ISO code",
+};
+const detailSites = appPaths()
+  .filter((f) => !(f in NOT_REFUSALS))
+  .flatMap((f) =>
+    [
+      ...backend(f).matchAll(
+        /"code":\s*(?:"(\w+)"|([^\s,}]+))(?:,\s*"reason":\s*(?:"(\w+)"|([^\s,}]+)))?/g,
+      ),
+    ].map((m) => ({ f, code: m[1], codeExpr: m[2], reason: m[3], reasonExpr: m[4] })),
+  );
+const backendCodes = [...new Set(detailSites.flatMap((s) => (s.code ? [s.code] : [])))];
+// A code the scan cannot read (`"code": SOME_CONSTANT`) is a refusal no record is checked for.
+const opaqueCodes = detailSites.filter((s) => !s.code).map((s) => `${s.f}: "code": ${s.codeExpr}`);
+// A reason passed by variable, each pinned to what holds it, so a new one has to say how it is held.
+const OPAQUE_REASONS: Record<string, string> = {
+  "refusal.py: refused ← reason": "a literal at each refused() call, else opaqueRefused",
+  "routes/auth.py: login ← reason": "_login_refused's LoginRefusal",
+  "routes/downloads.py: url_blocked ← urlguard.REASON_MALFORMED": "UrlBlockedReason",
+  "routes/downloads.py: url_blocked ← e.reason": "BlockedUrl's UrlBlockedReason",
+  "routes/downloads.py: cookies ← e.reason": "InvalidCookies' CookieReason",
+  "routes/downloads.py: edit ← e.reason": "EditError's EditReason",
+  "routes/downloads.py: clip ← e.reason": "ClipError's ClipReason",
+  "routes/files.py: quota ← e.reason": "QuotaExceeded's QuotaReason",
+  "routes/playlists.py: playlist ← reason": "_refused's PlaylistRefusal",
+  "routes/playlists.py: playlist_move ← e.reason": "MoveRefused's MoveReason",
+  "routes/plex/playback.py: subtitle ← code": "REFUSAL_UNSHOWN: no screen reads it",
+  "routes/_common.py: not_found ← what": "opaqueWhats",
+  "routes/_common.py: youtube ← reason": "YouTubeRefusal",
+};
+const opaqueReasons = [
  ...new Set(
-    appSources().flatMap((src) =>
-      src
-        .split("\n")
-        .filter((line) => !line.includes('"state":'))
-        .flatMap((line) => [...line.matchAll(/"code":\s*"(\w+)"/g)].map((m) => m[1]!)),
-    ),
+    detailSites
+      .filter((s) => s.code && s.reasonExpr)
+      .map((s) => `${s.f}: ${s.code} ← ${s.reasonExpr}`),
  ),
 ];
-const literalOf = (file: string, name: string) =>
-  [
-    ...(
-      backend(file).match(new RegExp(`^${name} = Literal\\[([^\\]]*)\\]`, "m"))?.[1] ?? ""
-    ).matchAll(/"(\w+)"/g),
-  ].map((m) => m[1]!);
-const literalCalls = (file: string, re: RegExp) => [
-  ...new Set([...backend(file).matchAll(re)].map((m) => m[1]!)),
-];
-const detailReasons = (file: string, code: string) =>
-  literalCalls(file, new RegExp(`"code":\\s*"${code}",\\s*"reason":\\s*"(\\w+)"`, "g"));
-// What each refusal code can carry as its reason, read where it is decided: a Literal when the reason
-// travels in a variable (`e.reason`), the call's own literal otherwise.
-const reasonsOf: Record<string, string[]> = {
-  refused: refusedReasons,
-  youtube: youtubeReasons,
-  not_found: goneWhats,
-  clip: clipReasons,
-  ...Object.fromEntries(adminCodes.map((c) => [c, detailReasons("routes/admin.py", c)])),
-  playlist: literalCalls("routes/playlists.py", /_refused\(\s*\d+,\s*"(\w+)"/g),
-  playlist_move: literalOf("reader/service.py", "MoveReason"),
-  edit: literalOf("downloads/service.py", "EditReason"),
-  cookies: literalOf("downloads/credentials.py", "CookieReason"),
-  url_blocked: literalCalls("downloads/urlguard.py", /^REASON_\w+ = "(\w+)"/gm),
-  login: literalCalls("routes/auth.py", /_login_refused\(\s*\d+,\s*"(\w+)"/g),
-  quota: literalOf("downloads/quota.py", "QuotaReason"),
-  reset: detailReasons("routes/auth.py", "reset"),
-  collection: detailReasons("routes/plex/collections.py", "collection"),
-};
+// A reason written as a literal in a detail dict, wherever that dict is built.
+const literalReasons = (code: string) =>
+  detailSites.filter((s) => s.code === code && s.reason).map((s) => s.reason!);
+// What each refusal code can carry as its reason: the Literal its typed parameter accepts, or the
+// call's own literal — and every literal any detail dict anywhere gives it.
+const reasonsOf: Record<string, string[]> = Object.fromEntries(
+  Object.entries({
+    refused: refusedReasons,
+    youtube: youtubeReasons,
+    not_found: goneWhats,
+    clip: clipReasons,
+    ...Object.fromEntries(adminCodes.map((c) => [c, []])),
+    playlist: literalOf("routes/playlists.py", "PlaylistRefusal"),
+    playlist_move: literalOf("reader/service.py", "MoveReason"),
+    edit: literalOf("downloads/service.py", "EditReason"),
+    cookies: literalOf("downloads/credentials.py", "CookieReason"),
+    url_blocked: literalOf("downloads/urlguard.py", "UrlBlockedReason"),
+    login: literalOf("routes/auth.py", "LoginRefusal"),
+    quota: literalOf("downloads/quota.py", "QuotaReason"),
+    reset: [],
+    collection: [],
+  } as Record<string, string[]>).map(([code, rs]) => [
+    code,
+    [...new Set([...rs, ...literalReasons(code)])],
+  ]),
+);
 const recordedCodes = [
  ...Object.keys(REFUSAL_FAMILIES),
  ...Object.keys(REFUSAL_BY_REASON),
@@ -359,6 +397,14 @@ describe("backend ids have their names in both languages", () => {
    expect(recordedCodes.filter((c) => !backendCodes.includes(c))).toEqual([]);
  });

+  it("every refusal code and reason is one the scans can read, or says what holds it", () => {
+    // The modules set aside as not refusals still exist and still use the key.
+    for (const f of Object.keys(NOT_REFUSALS)) expect(backend(f), f).toContain('"code":');
+    expect(opaqueCodes).toEqual([]);
+    expect(opaqueRefused).toEqual([]);
+    expect(opaqueReasons.sort()).toEqual(Object.keys(OPAQUE_REASONS).sort());
+  });
+
  it("read every refusal's reasons (a scan that matches nothing is green over nothing)", () => {
    const coded = [...Object.keys(REFUSAL_FAMILIES), ...Object.keys(REFUSAL_BY_REASON)];
    expect(Object.keys(reasonsOf).sort()).toEqual(coded.sort());
@@ -542,16 +588,21 @@ describe("backend ids have their names in both languages", () => {
    });

    it(`${lang}: no sentence for a refusal reason the backend cannot send`, () => {
-      // A family whose prefix is its own (the shared `errors.admin`, and the sign-in screens, which
-      // name more than the one-line refusal, are checked elsewhere or not at all).
+      // A family whose prefix is its own (the shared `errors.admin` is checked below). A suffixed
+      // family shares its prefix with more than its refusals — the sign-in screens' `gate`,
+      // `addressLabel` — so only an id with the refusal's own key counts as a reason there.
      for (const [code, f] of Object.entries(REFUSAL_FAMILIES)) {
-        if (f.prefix === "errors.admin" || "suffix" in f) continue;
+        if (f.prefix === "errors.admin") continue;
+        const suffix = "suffix" in f ? f.suffix : "";
        const fallbackId = f.fallback.startsWith(`${f.prefix}.`)
          ? f.fallback.slice(f.prefix.length + 1)
          : null;
        expect(
          [...idsUnder(keys, f.prefix)].filter(
-            (r) => r !== fallbackId && !reasonsOf[code]!.includes(r),
+            (r) =>
+              keys.has(`${f.prefix}.${r}${suffix}`) &&
+              r !== fallbackId &&
+              !reasonsOf[code]!.includes(r),
          ),
          code,
        ).toEqual([]);
@@ -4,6 +4,7 @@
 // one case per record shape, so moving the dispatch into data changed what no reader sees.
 import { afterEach, describe, expect, it } from "vitest";
 import i18n from "../../i18n";
+import { formatBytes } from "../format";
 import { localizeDetail } from "./refusalText";

 afterEach(async () => {
@@ -45,6 +46,24 @@ describe.each(["en", "hu"])("localizeDetail (%s)", (lng) => {
    );
  });

+  it("puts a download cap in GB and an upload cap in bytes, in the reader's number format", async () => {
+    expect(await say({ code: "quota", reason: "max_bytes", limit: 1.5 * 2 ** 30 })).toContain(
+      lng === "hu" ? "1,5" : "1.5",
+    );
+    const upload = await say({ code: "upload_too_large", limit: 5_242_880 });
+    expect(upload).toContain(formatBytes(5_242_880, lng));
+    expect(upload).not.toBe(i18n.t("downloads.errors.upload.tooLarge", { size: "" }));
+  });
+
+  it("says a shared-sentence code's own fallback for a reason it does not know", async () => {
+    expect(await say({ code: "quota", reason: "nope" })).toBe(
+      i18n.t("downloads.errors.quota.generic"),
+    );
+    expect(await say({ code: "collection", reason: "nope" })).toBe(
+      i18n.t("plex.collection.editFailed"),
+    );
+  });
+
  it("says a shared-sentence code's fallback, and a fixed code's one sentence", async () => {
    expect(await say({ code: "reset", reason: "invalid" })).toBe(
      i18n.t("welcome.auth.deadLink.short"),