feat(models): add --host/--port to search/list-folders/list-folder/show

comfy_cli/command/models/search.py

@@ -1,6 +1,8 @@
 """``comfy models`` — live model discovery against local or cloud.
 
-Four subcommands, all routed by ``--where`` (cloud auto-detect by default):
+Four subcommands, all routed by ``--where`` (cloud auto-detect by default);
+each also accepts ``--host``/``--port`` to aim a *local* query at a specific
+ComfyUI (rejected against a cloud target), mirroring ``comfy upload``:
 
     comfy models list-folders           # GET /api/experiment/models  | /models
     comfy models list-folder <folder>   # GET /api/experiment/models/<folder> | /models/<folder>
@@ -178,18 +180,91 @@ def _emit_http_error(e: urllib.error.HTTPError, *, renderer, target, message: st
     raise typer.Exit(code=1) from e
 
 
-def _resolve_and_stamp(renderer, where: str | None):
+# How a `cloud` routing decision was reached, in words, for the --host/--port
+# rejection message. Keys are `where.WhereResolution.source` values. Mirrors the
+# copy `cmdline.upload` uses (BE-5662) — kept local rather than imported so this
+# module stays free of a `cmdline` import cycle.
+_WHERE_SOURCE_PHRASES = {
+    "flag": "targeting cloud via --where cloud",
+    "env": "targeting cloud via the COMFY_WHERE environment variable",
+    "project": "targeting cloud via this project's configured default",
+    "config": "targeting cloud via your saved `where_default` setting",
+    "auto": "targeting cloud because you're signed in (no explicit --where)",
+}
+
+
+def _resolve_and_stamp(renderer, where: str | None, *, host: str | None = None, port: int | None = None):
     """Resolve the routing Target for a ``models`` verb and stamp it on the renderer.
 
     Every verb here calls this at the point it decides local-vs-cloud, so the
     error envelopes raised downstream carry ``where`` instead of ``null``.
     Errors raised *before* this (an unsafe path segment) keep ``where: null``,
     which is correct — nothing had routed yet. Explicit
     ``emit(..., where=...)`` arguments still take precedence over the stamp.
+
+    ``host``/``port`` route a **local** ``models`` query at a specific ComfyUI
+    (the ``--host``/``--port`` flags), mirroring ``comfy upload`` (BE-5662):
+    they are validated the same way, rejected against an effective ``cloud``
+    target, and otherwise handed to ``resolve_target``, which applies the local
+    precedence explicit flag > ``COMFY_LOCAL_URL`` > ``127.0.0.1:8188``. With
+    both ``None`` (no flags), resolution is exactly what it was before the flags
+    existed. The cloud target's address comes from the signed-in account and
+    ignores host/port entirely, so pairing them with a cloud target is a usage
+    error rather than a silently-ignored flag — the root cause of the comfy-mcp
+    bug where ``search_models`` answered from the local machine regardless of
+    the configured remote target.
     """
+    from comfy_cli import where as where_module
+    from comfy_cli.config_manager import ConfigManager
+    from comfy_cli.host_port import report_usage_error, validate_host
     from comfy_cli.target import resolve_target
 
-    target = resolve_target(where=where)
+    # Validate the flags before resolving anything: the host lands verbatim in
+    # ``http://{host}:{port}/...``, so a URL-special or control character is a
+    # usage error (BadParameter, exit 2) regardless of target. Port range is
+    # checked the same way ``comfy upload`` does. ``report_usage_error`` emits
+    # the terminating envelope for that rejection in JSON/NDJSON mode; the
+    # exception still escapes, so click's exit-2 usage contract is unchanged.
+    with report_usage_error(renderer):
+        if host is not None:
+            host = validate_host(host)
+        if port is not None and not (1 <= port <= 65535):
+            raise typer.BadParameter(f"invalid port: {port} is out of range (1-65535)")
+
+    try:
+        decision = where_module.resolve(
+            flag=where, config_value=ConfigManager().get(where_module.CONFIG_KEY_WHERE_DEFAULT)
+        )
+    except ValueError as e:
+        renderer.error(code="where_invalid", message=str(e), hint="use --where local or --where cloud")
+        raise typer.Exit(code=1) from e
+
+    effective_where = "cloud" if decision.target is where_module.WhereTarget.CLOUD else "local"
+    # Routing is decided, so every error envelope from here down can name the
+    # target — including the `host_flag_cloud` rejection immediately below.
+    renderer.where = effective_where
+    # --host/--port address a local ComfyUI; the cloud target's address comes
+    # from the signed-in account and ignores them (``Target.host``/``port`` are
+    # documented local-only). Reject the combination rather than silently
+    # answering from a machine the caller didn't name.
+    if effective_where == "cloud" and (host is not None or port is not None):
+        # The cloud target can come from an explicit --where, but equally from
+        # COMFY_WHERE, a project/config default, or credential auto-detection —
+        # so name the source rather than accusing the user of a flag they may
+        # never have typed.
+        source = _WHERE_SOURCE_PHRASES.get(decision.source, f"resolved to cloud by {decision.source}")
+        renderer.error(
+            code="host_flag_cloud",
+            message=f"--host/--port target a local ComfyUI server, but this run is {source}",
+            hint=(
+                "pass --where local to aim at a local server; to reach a different cloud address "
+                "set COMFY_CLOUD_BASE_URL or run `comfy cloud set-base-url`"
+            ),
+            details={"host": host, "port": port, "where": effective_where, "where_source": decision.source},
+        )
+        raise typer.Exit(code=1)
+
+    target = resolve_target(where=effective_where, host=host, port=port)
     renderer.where = target.kind
     return target
 
@@ -211,9 +286,17 @@ def list_folders_cmd(
         str | None,
         typer.Option("--where", show_default=False, help="Override the resolved routing mode."),
     ] = None,
+    host: Annotated[
+        str | None,
+        typer.Option(help="Server host (defaults to COMFY_LOCAL_URL or 127.0.0.1). Local targets only."),
+    ] = None,
+    port: Annotated[
+        int | None,
+        typer.Option(help="Server port (defaults to COMFY_LOCAL_URL or 8188). Local targets only."),
+    ] = None,
 ):
     renderer = get_renderer()
-    target = _resolve_and_stamp(renderer, where)
+    target = _resolve_and_stamp(renderer, where, host=host, port=port)
     url = target.url(*_models_path_parts(target))
 
     try:
@@ -286,10 +369,18 @@ def list_folder_cmd(
         int | None,
         typer.Option("--limit", show_default=False, help="Cap output to N rows."),
     ] = None,
+    host: Annotated[
+        str | None,
+        typer.Option(help="Server host (defaults to COMFY_LOCAL_URL or 127.0.0.1). Local targets only."),
+    ] = None,
+    port: Annotated[
+        int | None,
+        typer.Option(help="Server port (defaults to COMFY_LOCAL_URL or 8188). Local targets only."),
+    ] = None,
 ):
     renderer = get_renderer()
     _reject_unsafe_path_segment(folder, kind="folder", renderer=renderer)
-    target = _resolve_and_stamp(renderer, where)
+    target = _resolve_and_stamp(renderer, where, host=host, port=port)
     # Percent-encoded for the same reason `_local_folder_matches` does it: the
     # relaxed validation above admits spaces, `?`/`#`, and non-ASCII, none of
     # which may be allowed to alter the request. Error payloads below carry the
@@ -662,11 +753,19 @@ def search_cmd(
         str | None,
         typer.Option("--where", show_default=False, help="Override the resolved routing mode."),
     ] = None,
+    host: Annotated[
+        str | None,
+        typer.Option(help="Server host (defaults to COMFY_LOCAL_URL or 127.0.0.1). Local targets only."),
+    ] = None,
+    port: Annotated[
+        int | None,
+        typer.Option(help="Server port (defaults to COMFY_LOCAL_URL or 8188). Local targets only."),
+    ] = None,
 ):
     renderer = get_renderer()
     if type_ is not None:
         _reject_unsafe_path_segment(type_, kind="type", renderer=renderer)
-    target = _resolve_and_stamp(renderer, where)
+    target = _resolve_and_stamp(renderer, where, host=host, port=port)
 
     try:
         if target.is_cloud:
@@ -738,9 +837,17 @@ def show_cmd(
         str | None,
         typer.Option("--where", show_default=False, help="Override the resolved routing mode."),
     ] = None,
+    host: Annotated[
+        str | None,
+        typer.Option(help="Server host (defaults to COMFY_LOCAL_URL or 127.0.0.1). Local targets only."),
+    ] = None,
+    port: Annotated[
+        int | None,
+        typer.Option(help="Server port (defaults to COMFY_LOCAL_URL or 8188). Local targets only."),
+    ] = None,
 ):
     renderer = get_renderer()
-    target = _resolve_and_stamp(renderer, where)
+    target = _resolve_and_stamp(renderer, where, host=host, port=port)
 
     if not target.is_cloud:
         # On local there's no asset catalog. We can confirm the file exists by

tests/comfy_cli/command/models/test_search.py

@@ -911,3 +911,172 @@ def test_local_is_explicitly_unsupported(self, local_target, monkeypatch, capsys
         env = _run(["show", "anything.safetensors", "--where", "local"], capsys)
         assert env["ok"] is False
         assert env["error"]["code"] == "models_show_local_unsupported"
+
+
+# ---------------------------------------------------------------------------
+# --host/--port routing (BE-5788) — mirrors `comfy upload` (BE-5662)
+# ---------------------------------------------------------------------------
+
+
+class TestHostPortRouting:
+    """``comfy models <verb> --host/--port`` aims a LOCAL query at a specific
+    ComfyUI, closing the gap where these four verbs could only reach the
+    process-wide ``COMFY_LOCAL_URL`` (or the 127.0.0.1:8188 default). The URL
+    the request actually hits is asserted end-to-end through the *real*
+    ``resolve_target`` — no fixture pins it — so the precedence (explicit flag >
+    ``COMFY_LOCAL_URL`` > default), resolved independently for host and port, is
+    exercised, not mocked.
+    """
+
+    def test_host_and_port_reach_the_resolved_url(self, monkeypatch, capsys):
+        monkeypatch.delenv("COMFY_LOCAL_URL", raising=False)
+        _patch_urlopen(monkeypatch, {"10.0.0.5:9999/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local", "--host", "10.0.0.5", "--port", "9999"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://10.0.0.5:9999/models"
+
+    def test_no_flags_keeps_the_loopback_default(self, monkeypatch, capsys):
+        # Acceptance: no flags -> behavior identical to today.
+        monkeypatch.delenv("COMFY_LOCAL_URL", raising=False)
+        _patch_urlopen(monkeypatch, {"127.0.0.1:8188/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://127.0.0.1:8188/models"
+
+    def test_no_flags_still_honors_comfy_local_url(self, monkeypatch, capsys):
+        monkeypatch.setenv("COMFY_LOCAL_URL", "http://192.168.1.50:7777")
+        _patch_urlopen(monkeypatch, {"192.168.1.50:7777/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://192.168.1.50:7777/models"
+
+    def test_flags_beat_comfy_local_url(self, monkeypatch, capsys):
+        monkeypatch.setenv("COMFY_LOCAL_URL", "http://192.168.1.50:7777")
+        _patch_urlopen(monkeypatch, {"10.0.0.5:9999/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local", "--host", "10.0.0.5", "--port", "9999"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://10.0.0.5:9999/models"
+
+    def test_host_only_flag_keeps_the_env_port(self, monkeypatch, capsys):
+        # host and port resolve independently, so --host alone must not drop the
+        # env var's port back to the 8188 default.
+        monkeypatch.setenv("COMFY_LOCAL_URL", "http://192.168.1.50:7777")
+        _patch_urlopen(monkeypatch, {"10.0.0.5:7777/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local", "--host", "10.0.0.5"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://10.0.0.5:7777/models"
+
+    def test_ipv6_host_is_bracketed_in_the_url(self, monkeypatch, capsys):
+        monkeypatch.delenv("COMFY_LOCAL_URL", raising=False)
+        _patch_urlopen(monkeypatch, {"[::1]:8189/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local", "--host", "::1", "--port", "8189"], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == "http://[::1]:8189/models"
+
+    def test_search_threads_host_port_to_resolve_target(self, monkeypatch, capsys):
+        # The other verbs route through the same helper; assert `search` in
+        # particular hands the pair to `resolve_target`.
+        from comfy_cli.target import Target
+
+        seen: dict[str, Any] = {}
+
+        def fake_resolve_target(**kwargs):
+            seen.update(kwargs)
+            return Target(
+                kind="local",
+                base_url="http://10.0.0.5:9999",
+                path_prefix="",
+                history_path="history",
+                host="10.0.0.5",
+                port=9999,
+            )
+
+        monkeypatch.setattr("comfy_cli.target.resolve_target", fake_resolve_target)
+        _patch_urlopen(monkeypatch, {"10.0.0.5:9999/models/loras": _LOCAL_FILES_BY_FOLDER["loras"]})
+        env = _run(
+            ["search", "--text", "ltx", "--type", "lora", "--where", "local", "--host", "10.0.0.5", "--port", "9999"],
+            capsys,
+        )
+        assert env["ok"] is True, env
+        assert seen["host"] == "10.0.0.5"
+        assert seen["port"] == 9999
+        assert seen["where"] == "local"
+
+
+class TestHostPortCloudRejection:
+    """``--host``/``--port`` name a LOCAL server, so pairing them with an
+    effective cloud target is a structured usage error rather than a silently
+    ignored flag — the comfy-mcp root cause this ticket fixes."""
+
+    @pytest.mark.parametrize(
+        "args",
+        [
+            ["list-folders", "--where", "cloud", "--host", "10.0.0.5"],
+            ["list-folders", "--where", "cloud", "--port", "9999"],
+            ["list-folder", "loras", "--where", "cloud", "--host", "10.0.0.5"],
+            ["search", "--where", "cloud", "--host", "10.0.0.5", "--port", "9999"],
+            ["show", "flux1-dev.safetensors", "--where", "cloud", "--port", "9999"],
+        ],
+    )
+    def test_host_or_port_with_cloud_where_flag_is_rejected(self, args, monkeypatch, capsys):
+        # urlopen must never be reached: the rejection fires before routing.
+        _patch_urlopen(monkeypatch, {})
+        env = _run(args, capsys)
+        assert env["ok"] is False, env
+        assert env["error"]["code"] == "host_flag_cloud"
+
+    def test_cloud_where_env_is_rejected_and_names_the_source(self, monkeypatch, capsys):
+        # COMFY_WHERE is how a top-level `comfy --where cloud` arrives, so the
+        # guard keys off the RESOLVED target, not the flag.
+        monkeypatch.setenv("COMFY_WHERE", "cloud")
+        _patch_urlopen(monkeypatch, {})
+        env = _run(["search", "--host", "10.0.0.5"], capsys)
+        assert env["ok"] is False, env
+        assert env["error"]["code"] == "host_flag_cloud"
+        assert "COMFY_WHERE" in env["error"]["message"]
+        assert env["error"]["details"]["where_source"] == "env"
+
+
+class TestHostPortUsageErrors:
+    """A bad ``--host``/``--port`` is a usage error (exit 2), validated the same
+    way ``comfy upload`` validates its flags, before any request is made."""
+
+    @pytest.fixture
+    def runner(self):
+        return CliRunner()
+
+    @pytest.mark.parametrize(
+        "bad",
+        [
+            "evil/host",
+            "user@evil",
+            "host?x",
+            "host#x",
+            "host\rname",
+            "host\nname",
+            "",
+            "   ",
+            "a%0d%0aX-Injected:%201",
+            "host%2fpath",
+            "127.0.0.1:8188",
+            "localhost:8188",
+        ],
+    )
+    def test_invalid_host_is_a_usage_error(self, runner, bad):
+        # typer.BadParameter -> click UsageError -> exit code 2.
+        result = runner.invoke(search_cmd.app, ["list-folders", "--where", "local", "--host", bad])
+        assert result.exit_code == 2, result.output
+
+    @pytest.mark.parametrize("bad", ["0", "65536", "-1"])
+    def test_out_of_range_port_is_a_usage_error(self, runner, bad):
+        result = runner.invoke(search_cmd.app, ["list-folders", "--where", "local", "--port", bad])
+        assert result.exit_code == 2, result.output
+
+    @pytest.mark.parametrize("good", ["::1", "[::1]", "fe80::1"])
+    def test_ipv6_literal_host_is_accepted(self, good, monkeypatch, capsys):
+        monkeypatch.delenv("COMFY_LOCAL_URL", raising=False)
+        bracketed = good if good.startswith("[") else f"[{good}]"
+        _patch_urlopen(monkeypatch, {f"{bracketed}:8188/models": _LOCAL_FOLDERS})
+        env = _run(["list-folders", "--where", "local", "--host", good], capsys)
+        assert env["ok"] is True, env
+        assert env["data"]["url"] == f"http://{bracketed}:8188/models"