fix(nodes,jobs): render a where_invalid envelope for a bad --where instead of a traceback or a silent misroute

comfy_cli/cmdline.py

@@ -1132,7 +1132,7 @@ def run(
         try:
             decision = where_module.resolve(flag=where, config_value=config.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")
+            renderer.error(code="where_invalid", message=str(e), hint=where_module.WHERE_INVALID_HINT)
             raise typer.Exit(code=1)
 
         # The routing target is now known, so every downstream error envelope
@@ -1362,7 +1362,7 @@ def upload(
     try:
         decision = where_module.resolve(flag=where, config_value=config.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")
+        renderer.error(code="where_invalid", message=str(e), hint=where_module.WHERE_INVALID_HINT)
         raise typer.Exit(code=1)
 
     effective_where = "cloud" if decision.target is where_module.WhereTarget.CLOUD else "local"
@@ -1428,7 +1428,7 @@ def download(
     try:
         decision = where_module.resolve(flag=where, config_value=config.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")
+        renderer.error(code="where_invalid", message=str(e), hint=where_module.WHERE_INVALID_HINT)
         raise typer.Exit(code=1)
 
     effective_where = "cloud" if decision.target is where_module.WhereTarget.CLOUD else "local"

comfy_cli/command/jobs.py

@@ -2356,9 +2356,23 @@ def _is_cloud(where: str | None) -> bool:
     try:
         decision = where_module.resolve_default(flag=where)
     except ValueError:
-        # Invalid value — fall back to local; the validating command
-        # (cmdline.py top-level option) will surface ``where_invalid``.
-        return False
+        # An invalid persisted ``where_default`` shouldn't be fatal; fall back to
+        # the flag (if valid) or auto-detect with the bad config value dropped.
+        try:
+            decision = where_module.resolve(flag=where, config_value=None)
+        except ValueError as exc:
+            # The flag/env/project value itself is bad, so dropping the config
+            # changed nothing and no further fallback can make it valid. Emit the
+            # shared ``where_invalid`` envelope and exit, exactly as ``nodes``
+            # does — this used to `return False`, on the assumption that
+            # ``cmdline.py``'s top-level ``--where`` had already validated the
+            # value, but that only covers ``comfy --where X jobs ls``. A
+            # per-command ``comfy jobs ls --where bogus`` (or an exported
+            # ``COMFY_WHERE=bogus``) reaches no such validator, so it silently
+            # routed to local and exited 0 with ``ok: true``: a machine consumer
+            # got a successful *local* answer to a question about a target it
+            # never named.
+            raise where_module.where_invalid_exit(exc) from exc
     return decision.target is where_module.WhereTarget.CLOUD
 
 

comfy_cli/command/nodes.py

@@ -59,7 +59,15 @@ def _resolved_where(where: str | None) -> str:
     except ValueError:
         # An invalid persisted where_default shouldn't be fatal; fall back to
         # the flag (if valid) or auto-detect with the bad config value dropped.
-        decision = where_module.resolve(flag=where, config_value=None)
+        try:
+            decision = where_module.resolve(flag=where, config_value=None)
+        except ValueError as exc:
+            # The flag/env/project value itself is bad, so dropping the config
+            # changed nothing and no further fallback can make it valid. Emit
+            # the shared `where_invalid` envelope and exit, exactly as
+            # `resolve_default_or_exit` does for the non-recovering verbs —
+            # a machine consumer must read `error.code`, not a stack trace.
+            raise where_module.where_invalid_exit(exc) from exc
     return decision.target.value  # "local" | "cloud"
 
 

comfy_cli/where.py

@@ -22,7 +22,10 @@
 from collections.abc import Mapping
 from dataclasses import dataclass
 from enum import Enum
-from typing import Any
+from typing import TYPE_CHECKING, Any
+
+if TYPE_CHECKING:
+    import typer
 
 
 class WhereTarget(str, Enum):
@@ -34,6 +37,24 @@ class WhereTarget(str, Enum):
 ENV_DEFAULT = "COMFY_WHERE"
 CONFIG_KEY_WHERE_DEFAULT = "where_default"
 
+# The hint carried by every ``where_invalid`` envelope raised from a *multi-source*
+# routing failure — one where the offending value could have come from the flag,
+# ``COMFY_WHERE``, ``defaults.where``, or the persisted ``where_default``, so the
+# user has to be told where to look. A module constant because several call sites
+# emit it (:func:`resolve_default_or_exit`, the recover-first ``nodes``/``jobs``
+# helpers once their fallback is exhausted, and ``cmdline``'s ``run``/``upload``/
+# ``download``) and a user-facing string that lives in six places drifts.
+#
+# Sites that validate a *single explicit* value deliberately keep a shorter hint:
+# the top-level ``--where`` flag and ``comfy set-default --where`` / ``comfy setup``
+# call :func:`_parse` on exactly the string the user just typed, and ``comfy logs``
+# is local-only, so pointing any of them at ``COMFY_WHERE``/``comfy.yaml`` would
+# send the user hunting in a file that had nothing to do with the failure.
+WHERE_INVALID_HINT = (
+    "use --where local or --where cloud, and check COMFY_WHERE, "
+    "`defaults.where` in comfy.yaml, and `comfy set-default --where`"
+)
+
 
 @dataclass
 class WhereResolution:
@@ -115,22 +136,42 @@ def resolve_default_or_exit(
     JSON-envelope command.
 
     Commands that *can* recover (``nodes``, ``jobs``) keep their own
-    ``except ValueError`` fallback instead of calling this.
+    ``except ValueError`` fallback instead of calling this — but a fallback only
+    recovers the *config* case, so once it too fails they raise
+    :func:`where_invalid_exit`, which is this function's tail.
+    """
+    try:
+        return resolve_default(flag=flag, env=env, project_value=project_value)
+    except ValueError as e:
+        raise where_invalid_exit(e) from e
+
+
+def where_invalid_exit(exc: ValueError) -> typer.Exit:
+    """Render the shared ``where_invalid`` envelope for *exc* and return the
+    ``typer.Exit`` the caller must ``raise``.
+
+    Split out of :func:`resolve_default_or_exit` so a command with its own
+    recovery fallback can reuse the identical envelope (same ``code``, same
+    ``message``, same :data:`WHERE_INVALID_HINT`, same exit code) once that
+    fallback is exhausted, instead of letting the ``ValueError`` escape as a raw
+    traceback with nothing on stdout — the worst possible shape for a machine
+    consumer of a JSON-envelope command.
+
+    It *returns* the exception rather than raising it (and is deliberately not
+    annotated ``NoReturn``) so that every call site reads ``raise
+    where_invalid_exit(e) from e``. Nothing in CI enforces a ``NoReturn``
+    contract — ruff's selected rules don't and there is no type checker in the
+    pipeline — so a caller that merely *called* an exiting helper would fall
+    through to an unbound local the moment this function was stubbed, mocked, or
+    softened. Making the ``raise`` structural at each call site keeps the
+    control flow checkable by eye and by any future linter.
     """
     import typer
 
     from comfy_cli.output import get_renderer
 
-    try:
-        return resolve_default(flag=flag, env=env, project_value=project_value)
-    except ValueError as e:
-        get_renderer().error(
-            code="where_invalid",
-            message=str(e),
-            hint="use --where local or --where cloud, and check COMFY_WHERE, "
-            "`defaults.where` in comfy.yaml, and `comfy set-default --where`",
-        )
-        raise typer.Exit(code=1) from e
+    get_renderer().error(code="where_invalid", message=str(exc), hint=WHERE_INVALID_HINT)
+    return typer.Exit(code=1)
 
 
 def _project_where_default() -> str | None:

tests/comfy_cli/command/test_jobs_where_invalid.py

@@ -0,0 +1,206 @@
+"""``comfy jobs`` renders a ``where_invalid`` envelope for a bad routing value.
+
+The sibling of :mod:`tests.comfy_cli.command.test_nodes_where_invalid`, for the
+other recover-first command. ``jobs._is_cloud`` used to swallow the ``ValueError``
+and ``return False`` on the assumption that ``cmdline.py``'s top-level ``--where``
+had already rejected a bad value. That only holds for ``comfy --where X jobs ls``:
+a **per-command** ``comfy jobs ls --where bogus``, an exported ``COMFY_WHERE``, or a
+stale ``defaults.where`` reaches no such validator, so the verb silently routed to
+local and exited **0 with ``ok: true``** — a machine consumer got a successful
+*local* answer to a question about a target it never named, which is worse than the
+traceback the ``nodes`` half of this change fixed.
+
+These tests pin both halves: the unrecoverable sources now emit the shared
+envelope, and the config-recovery branch still recovers.
+"""
+
+from __future__ import annotations
+
+import json
+from typing import Any
+
+import pytest
+from typer.testing import CliRunner
+
+from comfy_cli import where as where_module
+from comfy_cli.caller import Caller
+from comfy_cli.command import jobs as jobs_cmd
+from comfy_cli.output.renderer import OutputMode, Renderer, reset_renderer_for_testing, set_renderer
+
+# Every verb that stamps routing at entry, i.e. every verb routed through
+# ``_stamp_where`` -> ``_is_cloud``. Each calls it as its first statement, so a
+# bad value exits before any host/port resolution or network call — that is what
+# makes these safe to run offline.
+ROUTED_VERBS: list[list[str]] = [
+    ["ls"],
+    ["status", "abc123"],
+    ["wait", "abc123"],
+    ["cancel", "abc123"],
+    ["watch", "abc123"],
+]
+
+
+@pytest.fixture(autouse=True)
+def reset_singleton():
+    reset_renderer_for_testing()
+    yield
+    reset_renderer_for_testing()
+
+
+@pytest.fixture(autouse=True)
+def isolated_routing_sources(monkeypatch: pytest.MonkeyPatch):
+    """Pin every routing source the tests don't set themselves.
+
+    Without this the assertions would read the developer's real ``COMFY_WHERE``,
+    a ``comfy.yaml`` above the checkout, their persisted ``where_default``, and
+    their cloud credentials.
+    """
+    monkeypatch.delenv(where_module.ENV_DEFAULT, raising=False)
+    monkeypatch.setattr("comfy_cli.project.find_project", lambda *a, **kw: None)
+    monkeypatch.setattr(where_module, "_has_cloud_credentials", lambda: False)
+    _set_persisted_where_default(monkeypatch, None)
+
+
+def _set_persisted_where_default(monkeypatch: pytest.MonkeyPatch, value: str | None):
+    """Make ``ConfigManager().get("where_default")`` answer *value*.
+
+    ``ConfigManager`` is ``@singleton``-wrapped, so the module-level name is a
+    factory, not the class — patch the real class off the instance it hands back.
+    """
+    from comfy_cli.config_manager import ConfigManager
+
+    cls = type(ConfigManager())
+    real_get = cls.get
+
+    def fake_get(self, key, *a, **kw):
+        if key == where_module.CONFIG_KEY_WHERE_DEFAULT:
+            return value
+        return real_get(self, key, *a, **kw)
+
+    monkeypatch.setattr(cls, "get", fake_get)
+
+
+def _force_json_renderer():
+    r = Renderer.resolve(
+        is_stdout_tty=False,
+        env={},
+        caller=Caller(kind="user", agentic=False, source_env=None),
+        json_flag=True,
+    )
+    r.mode = OutputMode.JSON
+    set_renderer(r)
+    return r
+
+
+def _invoke(args: list[str], capsys: pytest.CaptureFixture[str]) -> tuple[int, str, str]:
+    """Run a ``jobs`` verb and return ``(exit_code, stdout, stderr)``.
+
+    ``standalone_mode`` is left ON: the whole claim here is about the process
+    exit code, and only click's own standalone handling turns ``typer.Exit``
+    into one.
+    """
+    _force_json_renderer()
+    result = CliRunner().invoke(jobs_cmd.app, args)
+    captured = capsys.readouterr()
+    out = captured.out or result.stdout or ""
+    return result.exit_code, out, captured.err
+
+
+def _sole_envelope(out: str) -> dict[str, Any]:
+    lines = [ln for ln in out.strip().splitlines() if ln.strip()]
+    assert len(lines) == 1, f"expected exactly one envelope on stdout, got {len(lines)} lines: {lines!r}"
+    return json.loads(lines[0])
+
+
+def _assert_where_invalid(code: int, out: str, err: str, bad_value: str):
+    assert code == 1, f"expected exit 1, got {code} (stdout={out!r})"
+    env = _sole_envelope(out)
+    assert env["ok"] is False
+    assert env["error"]["code"] == "where_invalid"
+    assert bad_value in env["error"]["message"]
+    # The regression this file exists for: the old code exited 0 with a *local*
+    # success envelope, so a consumer keying on `ok` never learned it was
+    # answered about the wrong target.
+    for stream in (out, err):
+        assert "Traceback" not in stream
+        assert "ValueError" not in stream
+
+
+class TestBadFlag:
+    @pytest.mark.parametrize("verb", ROUTED_VERBS, ids=lambda v: "-".join(v))
+    def test_bad_where_flag_renders_envelope(self, verb, capsys):
+        """Every routed verb, not just ``ls`` — they share one resolver."""
+        code, out, err = _invoke([*verb, "--where", "bogus"], capsys)
+        _assert_where_invalid(code, out, err, "bogus")
+
+    def test_bad_where_flag_wins_over_a_valid_config(self, monkeypatch, capsys):
+        """A *valid* persisted default must not paper over an explicit bad flag."""
+        _set_persisted_where_default(monkeypatch, "local")
+        code, out, err = _invoke(["ls", "--where", "bogus"], capsys)
+        _assert_where_invalid(code, out, err, "bogus")
+
+    def test_hint_is_the_shared_constant(self, capsys):
+        """Pin the drift the module constant exists to prevent: this envelope,
+        ``nodes``', and ``resolve_default_or_exit``'s must stay byte-identical."""
+        _, out, _err = _invoke(["ls", "--where", "bogus"], capsys)
+        assert _sole_envelope(out)["error"]["hint"] == where_module.WHERE_INVALID_HINT
+
+
+class TestBadEnvAndProject:
+    def test_bad_comfy_where_env_renders_envelope(self, monkeypatch, capsys):
+        """``COMFY_WHERE`` is also how the top-level ``comfy --where`` reaches a
+        subcommand, so this is the path an exported bad value takes."""
+        monkeypatch.setenv(where_module.ENV_DEFAULT, "bogus")
+        code, out, err = _invoke(["ls"], capsys)
+        _assert_where_invalid(code, out, err, "bogus")
+
+    def test_bad_project_defaults_where_renders_envelope(self, monkeypatch, capsys):
+        """``defaults.where`` in the governing ``comfy.yaml`` is re-read by the
+        fallback too, so it is just as unrecoverable as the flag."""
+        from pathlib import Path
+
+        from comfy_cli.project import Project
+
+        project = Project(root=Path("/nonexistent"), config={"schema": "project/1", "defaults": {"where": "bogus"}})
+        monkeypatch.setattr("comfy_cli.project.find_project", lambda *a, **kw: project)
+        code, out, err = _invoke(["ls"], capsys)
+        _assert_where_invalid(code, out, err, "bogus")
+
+
+class TestConfigRecoveryStillWorks:
+    """Regression control for the branch this change must NOT break: a corrupt
+    persisted ``where_default`` is still non-fatal, exactly as before."""
+
+    def test_corrupt_config_with_valid_flag_still_succeeds(self, monkeypatch, capsys):
+        _set_persisted_where_default(monkeypatch, "garbage")
+        code, out, _err = _invoke(["ls", "--where", "local", "--local-only"], capsys)
+        assert code == 0, f"corrupt config recovery regressed (stdout={out!r})"
+        env = _sole_envelope(out)
+        assert env["ok"] is True
+        assert env["where"] == "local"
+
+    def test_corrupt_config_with_no_flag_still_falls_back(self, monkeypatch, capsys):
+        """No flag, no env, no project: dropping the bad config leaves the
+        auto-detect default, which must still resolve rather than exit."""
+        _set_persisted_where_default(monkeypatch, "garbage")
+        code, out, _err = _invoke(["ls", "--local-only"], capsys)
+        assert code == 0, f"corrupt config recovery regressed (stdout={out!r})"
+        env = _sole_envelope(out)
+        assert env["ok"] is True
+        assert env["where"] == "local"
+
+
+class TestIsCloudUnit:
+    """The resolver itself, without the CLI layer in the way."""
+
+    def test_returns_the_target_for_a_valid_flag(self):
+        assert jobs_cmd._is_cloud("cloud") is True
+        assert jobs_cmd._is_cloud("local") is False
+
+    def test_raises_typer_exit_for_a_bad_flag(self):
+        import typer
+
+        _force_json_renderer()
+        with pytest.raises(typer.Exit) as excinfo:
+            jobs_cmd._is_cloud("bogus")
+        assert excinfo.value.exit_code == 1