fix(services): route a service remote from services.<TAG>.dl.remote

Click picks the subcommand before the dl callback applies the service config, so Services.resolve_command now applies remote and server first. A REST API job routes from its request only.

docs/dev/rest-api/remote-sessions.md

@@ -87,18 +87,37 @@ remote_services:
 If you configure exactly one server, unshackle uses it implicitly:
 
 ```bash
-unshackle dl EXAMPLE 0ABCDEF
+unshackle dl --remote EXAMPLE 0ABCDEF
 ```
 
 If you configure **more than one** server, you must pick one with `--server`:
 
 ```bash
-unshackle dl --server my_server EXAMPLE 0ABCDEF
+unshackle dl --remote --server my_server EXAMPLE 0ABCDEF
 ```
 
 With no `remote_services` configured at all, unshackle raises a clear error telling
 you to add the block shown above.
 
+### Making a service remote by default
+
+To run a service on the server without `--remote`, set `remote` in the
+[`services`](../../reference/configuration/services.md#services) block, not in the
+`services` sub-dict of `remote_services`. Add `server` to select one of several servers:
+
+```yaml title="unshackle.yaml (client)"
+services:
+  EXAMPLE:
+    dl:
+      remote: true
+      server: my_server
+```
+
+`unshackle dl EXAMPLE 0ABCDEF` and `unshackle search EXAMPLE "query"` then run on
+`my_server`. A `--server` on the command line overrides the configured server. The global
+`dl: remote: true` makes every `dl` run remote, but it has no effect on `search`.
+The REST API ignores both settings: a job runs remote only when its request asks for it.
+
 ---
 
 ## What happens during a download

docs/reference/configuration/services.md

@@ -13,7 +13,10 @@ Per-service configuration, keyed by the canonical service tag (for example `EXAM
 - **`title_map`**: an exact-match rename map applied to fetched titles (source name →
   desired name), so a service that names a title differently from your library still matches.
 - **`dl`**: per-service download defaults, using the same keys as the global
-  [`dl`](download.md#dl) block.
+  [`dl`](download.md#dl) block. Set `remote: true` to send the `dl` and `search` runs of that
+  service to a [remote server](#remote_services) without `--remote`. Add `server` to select one of
+  several servers. A `--server` on the command line overrides it. A REST API job ignores both keys.
+  See [making a service remote by default](../../dev/rest-api/remote-sessions.md#making-a-service-remote-by-default).
 
 Individual services may read any additional keys they define. unshackle hands the merged result
 (the service's own `config.yaml` with this block layered on top) to the service as `self.config`.

tests/core/test_service_dl_overrides.py

@@ -131,6 +131,15 @@ def test_hand_built_ctx_client_value_wins():
     assert ctx.params["repack"] is False
 
 
+def test_hand_built_ctx_skips_routing_params(caplog):
+    """An API job routes from its request, so the server's service config must not flip it, or warn."""
+    ctx = hand_built_ctx({})
+    with caplog.at_level(logging.WARNING, logger=log.name):
+        apply_service_dl_overrides(ctx, {"remote": True, "server": "box"}, log)
+    assert ctx.params == {"repack": False}
+    assert not caplog.records
+
+
 def test_hand_built_ctx_missing_param_is_skipped():
     """Known dl option absent from a hand-built context must not raise or be injected."""
     ctx = hand_built_ctx({})

tests/core/test_service_remote_config.py

@@ -0,0 +1,106 @@
+"""``services.<TAG>.dl`` ``remote`` and ``server`` route a service without ``--remote``. Explicit CLI values win."""
+
+from __future__ import annotations
+
+from typing import Any, Optional, cast
+
+import click
+import pytest
+
+from unshackle.commands.dl import dl
+from unshackle.commands.search import search
+from unshackle.core.config import config
+from unshackle.core.services import Services
+
+
+def make_ctx(
+    command: click.Command, args: Optional[list[str]] = None, default_map: Optional[dict[str, Any]] = None
+) -> click.Context:
+    """Parse the group options of a real ``Services`` command, without running its callback."""
+    return command.make_context(
+        command.name or "dl", [*(args or []), "EXAMPLE", "title"], default_map=default_map or {}
+    )
+
+
+def group(ctx: click.Context) -> Services:
+    return cast(Services, ctx.command)
+
+
+@pytest.fixture
+def routed(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, Optional[str]]]:
+    """Record the path get_command takes and the server it passes, without loading a service or calling a server."""
+    calls: list[tuple[str, Optional[str]]] = []
+
+    def remote(name: str, server_name: Optional[str]) -> click.Command:
+        calls.append(("remote", server_name))
+        return click.Command(name)
+
+    def local(tag: str) -> Any:
+        calls.append(("local", None))
+        return type("FakeService", (), {"cli": click.Command(tag)})
+
+    monkeypatch.setattr(Services, "make_remote_command", staticmethod(remote))
+    monkeypatch.setattr(Services, "load", staticmethod(local))
+    return calls
+
+
+@pytest.mark.parametrize("command", [dl.cli, search])
+def test_service_remote_routes_to_remote_command(
+    monkeypatch: pytest.MonkeyPatch, routed: list[tuple[str, Optional[str]]], command: click.Command
+) -> None:
+    """The typed name matches the config key in any case, as a service only the server has keeps the typed case."""
+    monkeypatch.setattr(config, "services", {"EXAMPLE": {"dl": {"remote": True, "server": "box"}}})
+    ctx = make_ctx(command)
+    group(ctx).resolve_command(ctx, ["example", "title"])
+    assert routed == [("remote", "box")]
+    assert ctx.params["remote"] is True
+    assert ctx.params["server"] == "box"
+
+
+@pytest.mark.parametrize(
+    ("services", "args", "default_map", "expected"),
+    [
+        ({"OTHER": {"dl": {"remote": True}}}, [], None, ("local", None)),
+        ({"EXAMPLE": {"dl": {"remote": True, "server": "box"}}}, ["--server", "other"], None, ("remote", "other")),
+        ({"EXAMPLE": {"dl": {"remote": False}}}, [], {"remote": True}, ("local", None)),
+        ({"EXAMPLE": {"dl": {"remote": "false"}}}, [], None, ("local", None)),
+    ],
+    ids=["other-service", "cli-server-wins", "service-false-beats-global", "quoted-false-is-cast"],
+)
+def test_routing_precedence(
+    monkeypatch: pytest.MonkeyPatch,
+    routed: list,
+    services: dict,
+    args: list[str],
+    default_map: Optional[dict[str, Any]],
+    expected: tuple[str, Optional[str]],
+) -> None:
+    monkeypatch.setattr(config, "services", services)
+    ctx = make_ctx(dl.cli, args, default_map)
+    group(ctx).get_command(ctx, "EXAMPLE")
+    assert routed == [expected]
+
+
+def test_invalid_value_names_the_config_key(monkeypatch: pytest.MonkeyPatch, routed: list) -> None:
+    monkeypatch.setattr(config, "services", {"EXAMPLE": {"dl": {"remote": "maybe"}}})
+    ctx = make_ctx(dl.cli)
+    with pytest.raises(click.ClickException, match=r"services\.EXAMPLE\.dl\.remote"):
+        group(ctx).get_command(ctx, "EXAMPLE")
+
+
+def test_resolved_service_does_not_route_the_next(monkeypatch: pytest.MonkeyPatch, routed: list) -> None:
+    """Help and shell completion call get_command for every service on the context that resolved one."""
+    monkeypatch.setattr(config, "services", {"EXAMPLE": {"dl": {"remote": True}}})
+    ctx = make_ctx(dl.cli)
+    group(ctx).resolve_command(ctx, ["EXAMPLE", "title"])
+    assert ctx.params["remote"] is True
+    group(ctx).get_command(ctx, "OTHER")
+    assert routed == [("remote", None), ("local", None)]
+
+
+def test_import_ignores_service_remote(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr(config, "services", {"EXAMPLE": {"dl": {"remote": True}}})
+    monkeypatch.setattr(Services, "make_import_command", staticmethod(lambda tag, ctx: click.Command(tag)))
+    ctx = make_ctx(dl.cli, ["--import", "export.json"])
+    group(ctx).resolve_command(ctx, ["EXAMPLE", "title"])
+    assert ctx.params["remote"] is False

tests/remote/unit/test_remote_command_unknown_tag.py

@@ -22,7 +22,7 @@ SERVED = [
 
 
 def _make(name: str) -> click.Command:
-    return Services.make_remote_command(name, ctx=click.Context(click.Command("dl")))
+    return Services.make_remote_command(name, None)
 
 
 def test_unknown_name_raises_with_typed_name_and_available_list() -> None:

unshackle/commands/dl.py

@@ -70,7 +70,7 @@ from unshackle.core.proxies.resolve import (
     split_proxy_query,
 )
 from unshackle.core.service import Service, grow_session_pool
-from unshackle.core.services import Services
+from unshackle.core.services import ROUTING_PARAMS, Services
 from unshackle.core.temp import with_task_temp
 from unshackle.core.title_cacher import get_account_hash
 from unshackle.core.titles import Movie, Movies, Series, Song, Title_T
@@ -461,9 +461,13 @@ def cut_music_sample(title: Any, window: tuple[int, int], log: logging.Logger) -
 
 def apply_service_dl_overrides(ctx: click.Context, service_dl_config: dict[str, Any], log: logging.Logger) -> None:
     """Apply ``services.<TAG>.dl`` config onto ``ctx.params``. Explicit CLI/env values win.
-    The config replaces defaults and global ``dl:`` default_map values."""
+    The config replaces defaults and global ``dl:`` default_map values.
+    ``remote`` and ``server`` are skipped: they pick the code that runs, so ``Services.resolve_command`` sets
+    them before the service loads, and an API job takes them from its request."""
     params_by_name = {param.name: param for param in ctx.command.params if param.name}
     for name, value in normalize_dl_config(service_dl_config).items():
+        if name in ROUTING_PARAMS:
+            continue
         param = params_by_name.get(name)
         if param is None:
             log.warning(f"Ignoring unknown dl option '{name}' in service config")
@@ -1280,7 +1284,7 @@ class dl:
 
         self.service = Services.get_tag(ctx.invoked_subcommand)
         self.vault_service = Services.get_vault_tag(self.service)
-        apply_service_dl_overrides(ctx, config.services.get(self.service, {}).get("dl", {}), self.log)
+        apply_service_dl_overrides(ctx, (ci_get(config.services, self.service) or {}).get("dl") or {}, self.log)
 
         # Refresh locals Click bound before the overrides ran.
         no_proxy = ctx.params.get("no_proxy", no_proxy)
@@ -1458,9 +1462,9 @@ class dl:
                         )
                 except KeyError:
                     pass
-            merge_dict(config.services.get(self.service), self.service_config)
+            merge_dict(ci_get(config.services, self.service), self.service_config)
 
-        service_config = config.services.get(self.service, {})
+        service_config = ci_get(config.services, self.service, {})
         if service_config:
             reserved_keys = {
                 "profiles",

unshackle/commands/search.py

@@ -20,7 +20,7 @@ from unshackle.core.proxies.resolve import REGION, describe_proxy, load_proxy_pr
 from unshackle.core.service import Service
 from unshackle.core.services import Services
 from unshackle.core.utils.click_types import ContextData
-from unshackle.core.utils.collections import merge_dict
+from unshackle.core.utils.collections import ci_get, merge_dict
 from unshackle.core.utils.redact import mask_proxy
 
 
@@ -80,7 +80,7 @@ def search(
             if service_config_path.exists():
                 service_config = yaml.safe_load(service_config_path.read_text(encoding="utf8"))
                 log.info("Service Config loaded")
-    merge_dict(config.services.get(service), service_config)
+    merge_dict(ci_get(config.services, service), service_config)
 
     proxy_providers: list[Proxy] = []
     if no_proxy:

unshackle/core/services.py

@@ -12,6 +12,7 @@ from pathlib import Path
 from typing import Any, Iterable
 
 import click
+from click.core import ParameterSource
 
 from unshackle.core import binaries
 from unshackle.core.config import config
@@ -19,6 +20,7 @@ from unshackle.core.constants import DEFAULT_SAMPLE
 from unshackle.core.service import Service
 from unshackle.core.service_repo import DirtyServiceRepo, head, is_repo_spec, refresh_repo, resolve_service_repo
 from unshackle.core.utilities import import_module_by_path
+from unshackle.core.utils.collections import ci_get
 from unshackle.core.utils.redact import redact_path
 
 log = logging.getLogger("services")
@@ -348,6 +350,9 @@ def check_load_errors() -> None:
         raise click.ClickException(f"Failed to load {len(LOAD_ERRORS)} service(s):\n{joined}")
 
 
+ROUTING_PARAMS = ("remote", "server")
+
+
 class Services(click.Group):
     """Lazy-loaded command group of project services."""
 
@@ -384,7 +389,7 @@ class Services(click.Group):
         """
         remote = ctx.params.get("remote") or (ctx.parent and ctx.parent.params.get("remote"))
         if remote:
-            remote_services = Services.fetch_remote_services(ctx)
+            remote_services = Services.fetch_remote_services(ctx.params.get("server"))
             if remote_services is not None:
                 return [s["tag"] for s in remote_services]
             tags = Services.get_tags()
@@ -401,13 +406,13 @@ class Services(click.Group):
         check_load_errors()
         tag = Services.get_tag(name)
 
-        import_file = ctx.params.get("import_file") or (ctx.parent and ctx.parent.params.get("import_file"))
-        if import_file:
+        if Services.is_import(ctx):
             return Services.make_import_command(tag, ctx)
 
-        remote = ctx.params.get("remote") or (ctx.parent and ctx.parent.params.get("remote"))
+        routing = Services.routing(ctx, tag)
+        remote = routing.get("remote") or (ctx.parent and ctx.parent.params.get("remote"))
         if remote:
-            return Services.make_remote_command(name, ctx)
+            return Services.make_remote_command(name, routing.get("server"))
 
         try:
             service = Services.load(tag)
@@ -429,6 +434,49 @@ class Services(click.Group):
 
         raise click.ClickException(f"Service '{tag}' has no 'cli' method configured.")
 
+    def resolve_command(
+        self, ctx: click.Context, args: list[str]
+    ) -> tuple[str | None, click.Command | None, list[str]]:
+        """Resolve the service, then store its routing in ``ctx.params`` for the callback.
+
+        ``get_command`` has already computed it, but help and shell completion call ``get_command`` on its own,
+        so only this path may store it.
+        """
+        cmd_name, cmd, rest = super().resolve_command(ctx, args)
+        if cmd_name:
+            ctx.params.update(Services.routing(ctx, Services.get_tag(cmd_name)))
+        return cmd_name, cmd, rest
+
+    @staticmethod
+    def is_import(ctx: click.Context) -> bool:
+        return bool(ctx.params.get("import_file") or (ctx.parent and ctx.parent.params.get("import_file")))
+
+    @staticmethod
+    def routing(ctx: click.Context, tag: str) -> dict[str, Any]:
+        """The ``remote`` and ``server`` values for one service: CLI/env, then ``services.<TAG>.dl``, then defaults.
+
+        Built from the parameter sources, never read back from ``ctx.params``: help and shell completion resolve
+        every service on one context, so a value stored for one service must not route the next one.
+        An import never routes.
+        """
+        if Services.is_import(ctx):
+            return {}
+        service_dl = (ci_get(config.services, tag) or {}).get("dl") or {}
+        routing: dict[str, Any] = {}
+        for param in ctx.command.params:
+            name = param.name
+            if name not in ROUTING_PARAMS or name not in ctx.params:
+                continue
+            if ctx.get_parameter_source(name) not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP):
+                routing[name] = ctx.params[name]
+                continue
+            value = service_dl.get(name)
+            try:
+                routing[name] = param.type_cast_value(ctx, param.get_default(ctx) if value is None else value)
+            except click.BadParameter as e:
+                raise click.ClickException(f"services.{tag}.dl.{name}: {e.format_message()}") from e
+        return routing
+
     @staticmethod
     def docstring_help(doc: str) -> str:
         """Format a service docstring for Click help, one \\b per paragraph so Click keeps the layout.
@@ -443,20 +491,19 @@ class Services(click.Group):
         return "\n\n".join([first] + [f"\b\n{p}" for p in rest])
 
     @staticmethod
-    def fetch_remote_services(ctx: click.Context) -> list[dict] | None:
+    def fetch_remote_services(server_name: str | None) -> list[dict] | None:
         """Fetch the service list from the remote server (cached); None when it is unreachable."""
         from unshackle.core.remote_service import remote_service_list, resolve_server
 
-        server_name = ctx.params.get("server")
         resolve_server(server_name)  # a bad --server raises its ClickException, not "unreachable"
         return remote_service_list(server_name)
 
     @staticmethod
-    def make_remote_command(name: str, ctx: click.Context) -> click.Command:
+    def make_remote_command(name: str, server_name: str | None) -> click.Command:
         """Make a Click command for a remote service, with the options the remote server gives."""
         # A failed fetch (None) means the remote server was unreachable, not that the name is
         # invalid - fall through to a bare stub so `-h` keeps working offline.
-        services = Services.fetch_remote_services(ctx)
+        services = Services.fetch_remote_services(server_name)
         if services is None:
             svc_info = None
             tag = Services.get_tag(name)
@@ -492,7 +539,6 @@ class Services(click.Group):
         def remote_cli(ctx: click.Context, title: str, **kwargs: object) -> object:
             from unshackle.core.remote_service import make_remote_service
 
-            server_name = ctx.parent.params.get("server") if ctx.parent else None
             return make_remote_service(ctx, server_name, tag, title, {"service_params": kwargs})
 
         if cli_params: