fix(media-home): repair scrutiny round-1 doc-vs-reality findings

jellyfin: send the access token over exactly ONE channel per request (drop the simultaneous X-Emby-Token header; the MediaBrowser Token= parameter is the sole transport, legacy fallback remains documented as substitute-never- stack and is request-capture tested); remove the dead no-op conditional in JellyfinClient.__init__; make the login error-path test exception-safe with patch.object; document the test-pinned dry-run plan keys and the actual 0/1/2 exit-code mapping in worked-recipes and SKILL.md. peertube: replace the stale dry-run shape prose ('url'/'form') with the test-pinned {dry_run, method, path, params} / form_fields keys; harden cmd_me against a non-dict role (no AttributeError) with regression tests; remove the dead client facade, the unused cmd_channels variable, and the unused List/Tuple imports (ruff F401/F841 clean). ghost: fold the 5 nested with-statements (ruff SIM117) in test_ghost.py into single with-statements. All three skills double-runner + proxy-trap green (25/56/37 tests); validate-evals, paired smoke, quality validator, core gates, and catalog check modes green. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
2026-09-11 19:47:12 +03:00 · 2026-08-29 20:32:06 -04:00
parent a20b66e6c1
commit 08689dfd8f
10 changed files with 164 additions and 95 deletions
@@ -238,16 +238,14 @@ class JwtSigningTests(unittest.TestCase):
    def test_malformed_secret_half_is_graceful_error_not_traceback(self):
        client = self.cli.GhostClient(url="https://example.com", key="5f9d4b1c8e2a43d7b6c0a1e9:not-hex!")
        stderr = io.StringIO()
-        with self.assertRaises(SystemExit):
-            with contextlib.redirect_stderr(stderr):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(stderr):
            client._jwt_token()
        self.assertIn("hexadecimal", stderr.getvalue())

    def test_key_without_colon_names_required_format(self):
        client = self.cli.GhostClient(url="https://example.com", key="justonepart")
        stderr = io.StringIO()
-        with self.assertRaises(SystemExit):
-            with contextlib.redirect_stderr(stderr):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(stderr):
            client._jwt_token()
        self.assertIn("id:secret", stderr.getvalue())

@@ -431,8 +429,7 @@ class MockedClientTests(unittest.TestCase):
        cli.requests.put = Mock(return_value=collision)
        client = cli.GhostClient(url="https://example.com", key=FIXED_KEY)
        stderr = io.StringIO()
-        with self.assertRaises(SystemExit):
-            with contextlib.redirect_stderr(stderr):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(stderr):
            client.update_post("abc123", status="published",
                               updated_at="2026-01-01T00:00:00.000Z")
        message = stderr.getvalue()
@@ -449,8 +446,7 @@ class MockedClientTests(unittest.TestCase):
        cli.requests.get = Mock(return_value=missing)
        client = cli.GhostClient(url="https://example.com", key=FIXED_KEY)
        stderr = io.StringIO()
-        with self.assertRaises(SystemExit):
-            with contextlib.redirect_stderr(stderr):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(stderr):
            client._get("/posts/does-not-exist")
        message = stderr.getvalue()
        self.assertIn("404", message)
@@ -468,8 +464,7 @@ class MockedClientTests(unittest.TestCase):
        cli.requests.get = Mock(return_value=bad_scheme)
        client = cli.GhostClient(url="https://example.com", key=FIXED_KEY)
        stderr = io.StringIO()
-        with self.assertRaises(SystemExit):
-            with contextlib.redirect_stderr(stderr):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(stderr):
            client._get("/posts")
        self.assertIn("Ghost [token]", stderr.getvalue())

@@ -49,7 +49,8 @@ endpoint requires its `Client=..., Device=..., DeviceId=..., Version=...` quarte
 any token exists** — the server rejects `POST /Users/AuthenticateByName` with
 `400 Error processing request.` otherwise. Afterwards the access token (or API key) rides
 the same header as `Token="..."`; the legacy `X-Emby-Token` header means the same thing
-and is scheduled for removal from Jellyfin 12.0. The bundled CLI sends the modern form.
+and is scheduled for removal from Jellyfin 12.0. The bundled CLI sends the modern form and
+puts the token in exactly that one channel per request (never co-sends `X-Emby-Token`).
 See [references/auth-and-sessions.md](references/auth-and-sessions.md).

 ## Essential Commands
@@ -156,10 +157,11 @@ scripts/jellyfin login --username alice --prompt --json | jq -r '"\(.user_id) \(

 Put `--json` before or after the subcommand. Output keys are stable snake_case: `items`
 (with `id`, `name`, `type`, `year`, `series`, `season_number`, `episode_number`),
-`results`, `libraries`, `total_record_count`, `start_index`. `--dry-run` emits the exact
-`path` + `params` (or `authorization_header` for `login`) that would be sent, so jq can
-verify a chain before running it live. Use `jq -r '.items[] | [.name, .year] | @tsv'` for
-tabular handoff.
+`results`, `libraries`, `total_record_count`, `start_index`. `--dry-run` emits a plan
+carrying `dry_run`, `path`, and `params` (`login` adds `authorization_header`; `info`
+composes a `requests` list), matching what would be sent, so jq can verify a chain before
+running it live. Exit codes: `0` success (including dry-run), `1` CLI/API errors, `2`
+argument errors. Use `jq -r '.items[] | [.name, .year] | @tsv'` for tabular handoff.

 ## Known Gotchas

@@ -123,7 +123,10 @@ It is wire-equivalent to the legacy `X-Emby-Token` header on every server that s
 legacy auth, and it keeps working when legacy channels are switched off.

 Never send two different tokens in one request — server precedence across channels is
-unspecified at the contract level and the value used becomes uncertain.
+unspecified at the contract level and the value used becomes uncertain. The bundled CLI
+takes this literally: after login it puts the token in exactly ONE channel per request
+(the `Token=` parameter of the modern header) and never attaches `X-Emby-Token` alongside
+it; the offline suite pins that single-channel contract with a request-capture test.

 ### Legacy kill-switch and deprecation timeline

@@ -133,8 +136,9 @@ unspecified at the contract level and the value used becomes uncertain.
  `X-Emby-Authorization` stop resolving — a client that "worked yesterday" and now gets
  uniform 401s has almost certainly met this toggle.
 - Maintainers have targeted disabling the deprecated options starting with the 12.0
-  release. Speak the modern `Authorization` header natively; keep `X-Emby-Token` only as a
-  compat fallback for old servers.
+  release. Speak the modern `Authorization` header natively; if you must support a server
+  with legacy auth locked off, substitute the legacy header for the modern one on that
+  server's requests — never send both on the same request.

 ## Error signatures worth mocking

@@ -52,7 +52,10 @@ proxy, not Jellyfin.
 - One access token per `(DeviceId, user)` pair: re-logging-in the same pair revokes the
  pair's previous token. Multi-profile CLIs must vary the DeviceId per profile or they will
  keep logging each other out.
- Never send two token channels in one request; which one wins is not contractual.
+- Never send two token channels in one request; which one wins is not contractual. The
+  bundled CLI honors this literally: the token rides only the MediaBrowser `Token=`
+  parameter and the legacy `X-Emby-Token` header is never attached (request-capture-tested
+  in the offline suite).

 ## User-scoping pitfalls

@@ -41,10 +41,12 @@ request.`; wrong password → 401; server restarting → 503 + `Retry-After` (re

 With the bundled CLI, steps 2–5 collapse to env vars: export `JELLYFIN_URL`,
 `JELLYFIN_API_KEY` (or `JELLYFIN_TOKEN`), `JELLYFIN_USER_ID`; then
-`scripts/jellyfin recent --json`. The login path itself is available as
-`scripts/jellyfin login --username alice` (reads `JELLYFIN_PASSWORD` interactively or via
-`--password`/`--password-stdin`), which prints the captured `user_id`/`access_token` for
-exporting.
+`scripts/jellyfin recent --json`. Post-login requests carry the token in exactly one
+channel — the `Token=` parameter of the MediaBrowser `Authorization` header (verified by
+the suite's request-capture test); the legacy `X-Emby-Token` header is not co-sent. The
+login path itself is available as `scripts/jellyfin login --username alice` (reads
+`JELLYFIN_PASSWORD` interactively or via `--password`/`--password-stdin`), which prints the
+captured `user_id`/`access_token` for exporting.

 ## Recipe 2 — Libraries → paged browse of one collection

@@ -129,10 +131,56 @@ scripts/jellyfin next-up --limit 5 --json

 `RunTimeTicks` are 100-nanosecond ticks (divide by 600,000,000 for minutes).

+## Bundled CLI `--dry-run` and exit-code contract
+
+The CLI's dry-run plans are pinned by its offline test suite (`scripts/test_jellyfin_cli.py`),
+so jq keys match tested reality exactly. Every plan carries:
+
+```json
+{ "dry_run": true, "path": "/Items/Latest", "params": { "userId": null, "limit": 10 } }
+```
+
+- `dry_run` (bool, always true), `path` (string) and `params` (object) appear on every
+  command plan; `login` instead emits `path: "/Users/AuthenticateByName"`, `server`,
+  `username`, `authorization_header`, and `pre_token_header: true` (its
+  `authorization_header` is the complete pre-token MediaBrowser header, no `Token=`
+  segment); `info` composes a `requests` array of `{path, params}` steps instead of a
+  single `path`/`params` pair.
+- `params` mirrors the exact query the live call would send (`userId` is JSON `null`
+  when not supplied).
+
+Exit codes: `0` on success (including dry-run previews), `1` on CLI errors (missing
+credentials, unreachable server, API 4xx/5xx, missing required `--user-id`), `2` on
+argparse misuse such as `--movies --episodes` together or a missing required flag.
+
+## Bundled CLI `--dry-run` and exit-code contract
+
+The CLI's dry-run plans are pinned by its offline test suite (`scripts/test_jellyfin_cli.py`),
+so jq keys match tested reality exactly. Every plan carries:
+
+```json
+{ "dry_run": true, "path": "/Items/Latest", "params": { "userId": null, "limit": 10 } }
+```
+
+- `dry_run` (bool, always true), `path` (string) and `params` (object) appear on every
+  command plan; `login` instead emits `path: "/Users/AuthenticateByName"`,
+  `server`, `username`, `authorization_header`, and `pre_token_header: true` (its
+  `authorization_header` is the complete pre-token MediaBrowser header, no `Token=`
+  segment); `info` composes a `requests` array of `{path, params}` steps instead of a
+  single `path`/`params` pair.
+- `params` mirrors the exact query the live call would send (`userId` is JSON `null`
+  when not supplied).
+
+Exit codes: `0` on success (including dry-run previews), `1` on CLI errors (missing
+credentials, unreachable server, API 4xx/5xx, missing required `--user-id`), `2` on
+argparse misuse such as `--movies --episodes` together or a missing required flag.
+
 ## Cross-version-safe baseline (derive your own recipes from these rules)

-1. Speak modern auth (`Authorization: MediaBrowser ...`); treat `X-Emby-Token` as a compat
-   fallback for old servers only.
+1. Speak modern auth (`Authorization: MediaBrowser ...`) and put the token in exactly ONE
+   channel per request (its `Token=` parameter). Never co-send the legacy `X-Emby-Token`
+   header with it — precedence across channels is unspecified; if a legacy-only server
+   needs the old header, substitute it there, do not stack channels.
 2. Send a complete pre-token header everywhere — zero cost, avoids the 400-on-login trap.
 3. Always send explicit `userId` on `/Items*`, `/UserViews`, `/Shows/*`, `/Items/Latest`.
 4. Resolve identity once: `USER_ID = AuthenticationResult.User.Id` (fallback `/Users/Me`
@@ -103,20 +103,20 @@ class JellyfinClient:
        self.token = token or ENV_TOKEN
        self.device_id = device_id or ENV_DEVICE_ID or default_device_id()
        self.dry_run = dry_run
-        if not self.key and self.token:
-            # User access tokens are only meaningful together with their user id;
-            # user-scoped commands resolve that separately via --user-id.
-            pass
+        # A user access token is only meaningful together with its user id; the
+        # user-scoped commands below resolve that separately via --user-id
+        # rather than guessing a session user here.

    def _headers(self, with_token=True):
-        headers = {
+        # Exactly ONE token channel per request: the access token (or API key)
+        # rides the MediaBrowser Authorization header's Token= parameter. The
+        # legacy X-Emby-Token header is deliberately NOT also sent — combining
+        # channels is undefined behavior (see references/auth-and-sessions.md).
+        return {
            "Accept": "application/json",
            "Authorization": build_authorization_header(
                self.device_id, token=(self.key or self.token) if with_token else "")
        }
-        if with_token and self.key:
-            headers["X-Emby-Token"] = self.key
-        return headers

    def _get(self, path, params=None):
        url = f"{self.url}{path}"
@@ -10,7 +10,6 @@ import tempfile
 import unittest
 from unittest.mock import Mock, patch

-
 SCRIPT = pathlib.Path(__file__).resolve().parent / "jellyfin"
 LOADER = importlib.machinery.SourceFileLoader("jellyfin_cli", str(SCRIPT))
 SPEC = importlib.util.spec_from_loader(LOADER.name, LOADER)
@@ -347,16 +346,15 @@ class LoginAuthSequenceTests(unittest.TestCase):
    def test_mocked_login_error_paths_do_not_crash(self):
        cli = jellyfin_cli
        cli.GLOBAL_FLAGS = {"json": False, "dry_run": False}
-        original_post = cli.requests.post
        for status, expected_fragment in ((400, "400"), (401, "401"), (403, "403")):
            with self.subTest(status=status):
-                cli.requests.post = Mock(return_value=FakeResponse(status, text="Error processing request."))
-                stderr = io.StringIO()
-                with contextlib.redirect_stderr(stderr), self.assertRaises(SystemExit):
+                response = FakeResponse(status, text="Error processing request.")
+                with patch.object(cli.requests, "post", return_value=response), \
+                        contextlib.redirect_stderr(io.StringIO()) as stderr, \
+                        self.assertRaises(SystemExit):
                    cli.cmd_login(cli.JellyfinClient(url="http://s:8096", device_id="d"),
                                  ["--username", "alice", "--password", "bad"])
                self.assertIn(expected_fragment, stderr.getvalue())
-        cli.requests.post = original_post
        cli.GLOBAL_FLAGS = {"json": False, "dry_run": False}

    def test_reads_require_a_credential_before_network(self):
@@ -375,8 +373,35 @@ class LoginAuthSequenceTests(unittest.TestCase):
        headers = client._headers()
        self.assertIn('Token="k-1"', headers["Authorization"])
        self.assertTrue(headers["Authorization"].startswith("MediaBrowser "))
+        self.assertNotIn("X-Emby-Token", headers)  # one token channel per request
        token_client = cli.JellyfinClient(token="t-1", device_id="dev-1")
-        self.assertIn('Token="t-1"', token_client._headers()["Authorization"])
+        token_headers = token_client._headers()
+        self.assertIn('Token="t-1"', token_headers["Authorization"])
+        self.assertNotIn("X-Emby-Token", token_headers)
+
+    def test_captured_requests_carry_token_in_exactly_one_channel(self):
+        """VAL-JF-009: the access token (or API key) appears in exactly ONE
+        channel per request — the MediaBrowser Token= parameter — and the
+        legacy X-Emby-Token header is never sent alongside it."""
+        cli = jellyfin_cli
+        captured = []
+        response = FakeResponse(200, json_body={"Items": [], "TotalRecordCount": 0})
+        for client in (cli.JellyfinClient(key="k-capture", device_id="dev-cap"),
+                       cli.JellyfinClient(token="t-capture", device_id="dev-cap")):
+            with patch.object(cli.requests, "get",
+                              side_effect=lambda url, **kw: captured.append(kw) or response):
+                client._get("/Items")
+        self.assertEqual(len(captured), 2)
+        for kwargs in captured:
+            self.assertIn("headers", kwargs)
+            headers = kwargs["headers"]
+            self.assertIn("Token=", headers["Authorization"])
+            self.assertNotIn("X-Emby-Token", headers)
+            channel_count = sum(
+                1 for value in headers.values()
+                if "Token=" in value or value.lower() == "x-emby-token"
+            )
+            self.assertEqual(channel_count, 1)


 class TvNavigationCommandTests(unittest.TestCase):
@@ -118,9 +118,11 @@ scripts/peertube comments --id "<UUID>" --json | jq '{total, total_not_deleted:
 scripts/peertube --dry-run --json search --query "test" | jq '{path, params: (.params | keys)}'
 ```

-`--dry-run` output shape: `{"dry_run": true, "method": "GET", "url": "...", "path":
-"/api/v1/...", "params": {...}}` (token requests additionally carry `"form"` keys without
-values). Every handler's dry-run plan uses the same keys, so one jq pattern audits any
+`--dry-run` output shape: `{"dry_run": true, "method": "GET", "path": "/api/v1/...",
+"params": {...}}` — every plan carries exactly `dry_run`, `method`, `path`, and `params`
+(test-pinned in `scripts/test_peertube.py`); composite commands emit a `requests` array of
+those same keyed steps, and the `login` plan is a `POST /api/v1/users/token` whose
+`form_fields` lists the field NAMES only (never values). One jq pattern audits any
 command.

 ## Raw curl equivalents (auth chain end-to-end)
@@ -17,7 +17,7 @@ import os
 import sys
 import time
 import warnings
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, Optional

 warnings.simplefilter("ignore")

@@ -366,47 +366,6 @@ class PeerTubeClient:
            die(f"Non-JSON response from {path} (status {status}); "
                f"is {self.display_server()} a PeerTube instance?")

-    def get_about(self):
-        return self._get("/config/about")
-
-    def get_stats(self):
-        return self._get("/server/stats")
-
-    def list_videos(self, start=0, count=DEFAULT_COUNT, sort="-publishedAt"):
-        return self._get("/videos", {"start": start, "count": count, "sort": sort})
-
-    def get_video(self, video_id):
-        return self._get(f"/videos/{video_id}")
-
-    def search_videos(self, query, start=0, count=DEFAULT_COUNT, search_target="local", sort=None):
-        params: Dict[str, Any] = {"search": query, "searchTarget": search_target,
-                                  "start": start, "count": count}
-        if sort:
-            params["sort"] = sort
-        return self._get("/search/videos", params)
-
-    def list_comment_threads(self, video_id, start=0, count=DEFAULT_COUNT):
-        return self._get(f"/videos/{video_id}/comment-threads",
-                         {"start": start, "count": count, "sort": "-createdAt"})
-
-    def list_channels(self, start=0, count=DEFAULT_COUNT):
-        return self._get("/video-channels", {"start": start, "count": count})
-
-    def get_channel(self, handle):
-        return self._get(f"/video-channels/{handle}")
-
-    def list_channel_videos(self, handle, start=0, count=DEFAULT_COUNT):
-        return self._get(f"/video-channels/{handle}/videos", {"start": start, "count": count})
-
-    def get_account(self, name):
-        return self._get(f"/accounts/{name}")
-
-    def my_profile(self):
-        return self._get("/users/me")
-
-    def my_videos(self, start=0, count=DEFAULT_COUNT):
-        return self._get("/users/me/videos", {"start": start, "count": count})
-

 # ----- command handlers ------------------------------------------------

@@ -425,8 +384,8 @@ def cmd_server(client, args):
            {"method": "GET", "path": f"{API_BASE}/server/stats", "params": {}},
        ]}
        return emit("[dry-run] GET /config/about + /server/stats", plan)
-    about = client.get_about() or {}
-    stats = client.get_stats() or {}
+    about = client._get("/config/about") or {}
+    stats = client._get("/server/stats") or {}
    instance = about.get("instance") or {}
    name = instance.get("name", "?")
    description = instance.get("shortDescription") or ""
@@ -548,7 +507,7 @@ def cmd_channels(client, args):
    total = data.get("total", len(channels))
    if not channels:
        return emit("No channels.", {"total": total, "start": parsed.offset, "count": 0, "channels": []})
-    lines, _ = [], []
+    lines = []
    for channel in channels:
        display = channel.get("displayName") or channel.get("name") or "?"
        handle = f"{channel.get('name', '?')}@{channel.get('host', '?')}"
@@ -604,7 +563,9 @@ def cmd_me(client, args):
    data = client._get("/users/me") or {}
    if isinstance(data, list):  # docs render an array sample; live servers return one object
        data = data[0] if data else {}
-    role = data.get("role") or {}
+    role = data.get("role") if isinstance(data, dict) else {}
+    if not isinstance(role, dict):  # some instances/versions may send a scalar role id
+        role = {"id": role, "label": str(role)}
    quota = data.get("videoQuota")
    quota_text = f"{quota} bytes" if quota is not None else "?"
    emit(f"@{data.get('username', '?')}  Role: {role.get('label', '?')}  Quota: {quota_text}", data)
@@ -743,6 +743,35 @@ class HandlerOutputTests(ModuleStateTestCase):
            self.assertEqual(payload["role"]["label"], "User")
        client.clear_token()

+    def test_me_tolerates_scalar_role_without_attribute_error(self):
+        """VAL-PT-010: a non-dict `role` (scalar id from a version-drifted
+        server) must degrade to a readable line, never AttributeError."""
+        client = pt.PeerTubeClient(
+            server="https://inst.example", config_dir=tempfile.mkdtemp(prefix="pt-ho-")
+        )
+        profile = {"username": "bob", "role": 2, "videoQuota": None}
+        client.save_session(TOKEN_RESPONSE)
+        with patch.object(pt.requests, "get", return_value=FakeResponse(200, profile)):
+            out = io.StringIO()
+            err = io.StringIO()
+            with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):
+                pt.cmd_me(client, [])
+        self.assertNotIn("Traceback", err.getvalue())
+        self.assertEqual(json.loads(out.getvalue())["role"], 2)
+        client.clear_token()
+
+    def test_me_tolerates_missing_role(self):
+        client = pt.PeerTubeClient(
+            server="https://inst.example", config_dir=tempfile.mkdtemp(prefix="pt-ho-")
+        )
+        client.save_session(TOKEN_RESPONSE)
+        with patch.object(pt.requests, "get", return_value=FakeResponse(200, {"username": "carol"})):
+            out = io.StringIO()
+            with contextlib.redirect_stdout(out):
+                pt.cmd_me(client, [])
+        self.assertEqual(json.loads(out.getvalue())["username"], "carol")
+        client.clear_token()
+
    def test_comments_output_exposes_thread_counts(self):
        client = pt.PeerTubeClient(
            server="https://inst.example", config_dir=tempfile.mkdtemp(prefix="pt-ho-")