feat(cloud login): emit machine-readable login_url event in --json mode (BE-3365)

comfy_cli/cloud/command.py

@@ -20,6 +20,7 @@
 from typing import Annotated
 
 import typer
+from rich import markup as rich_markup
 
 from comfy_cli import tracking
 from comfy_cli.auth import store
@@ -39,7 +40,14 @@
 # ---------------------------------------------------------------------------
 
 
-@app.command("login", help="Sign in to Comfy Cloud via your browser (OAuth + PKCE).")
+@app.command(
+    "login",
+    help=(
+        "Sign in to Comfy Cloud via your browser (OAuth + PKCE). Under --json/--json-stream "
+        "the authorize URL is emitted as a `login_url` event before the command blocks on the "
+        "browser callback, so an agent/MCP parent can open it; see docs/json-output.md."
+    ),
+)
 @tracking.track_command("cloud")
 def login_cmd(
     no_browser: Annotated[
@@ -65,14 +73,27 @@ def login_cmd(
         rprint(f"Signing in to [bold cyan]Comfy Cloud[/bold cyan] ([dim]{base_url}[/dim])")
 
     def _on_url(url: str) -> None:
-        if not renderer.is_pretty():
+        if renderer.is_pretty():
+            # Escape the URL for Rich: an IPv6 base_url (e.g. `http://[::1]:8188`)
+            # would otherwise make Rich parse `[::1]` as markup and raise.
+            safe_url = rich_markup.escape(url)
+            if no_browser:
+                rprint("\nOpen this URL in your browser to sign in:")
+                rprint(f"  [cyan]{safe_url}[/cyan]\n")
+            else:
+                rprint("[dim]Opening browser… (if it doesn't appear, copy this URL)[/dim]")
+                rprint(f"[dim]  {safe_url}[/dim]")
             return
-        if no_browser:
-            rprint("\nOpen this URL in your browser to sign in:")
-            rprint(f"  [cyan]{url}[/cyan]\n")
-        else:
-            rprint("[dim]Opening browser… (if it doesn't appear, copy this URL)[/dim]")
-            rprint(f"[dim]  {url}[/dim]")
+        # Machine mode: surface the authorize URL as a `login_url` event so an
+        # agent/MCP parent driving `comfy --json cloud login` can open (or hand
+        # off) the URL. run_login then blocks up to `timeout` seconds on the
+        # loopback callback, so upgrade to the NDJSON stream and emit the line
+        # now — `event()` writes + flushes, so the parent sees the URL *before*
+        # we block on the wait. If the write raises (the piped parent hung up),
+        # let it propagate: run_login re-raises OSError from the callback rather
+        # than blocking the full timeout on a stream nobody is reading.
+        renderer.force_stream()
+        renderer.event("login_url", url=url, timeout_s=timeout)
 
     try:
         result = run_login(
@@ -93,6 +114,11 @@ def _on_url(url: str) -> None:
             details=e.details,
         )
         raise typer.Exit(code=1)
+    except OSError:
+        # The machine stream broke while emitting `login_url` (the piped parent
+        # hung up), so run_login bailed instead of blocking the full timeout.
+        # We can't write an envelope to a dead stream — just fail fast.
+        raise typer.Exit(code=1)
 
     session = store.save_cloud_session(
         base_url=result.base_url,

comfy_cli/cloud/oauth.py

@@ -450,6 +450,13 @@ def run_login(
         if on_url_ready is not None:
             try:
                 on_url_ready(authorize_url)
+            except OSError:
+                # A broken output stream (e.g. a piped `--json` parent hung up)
+                # means we can no longer surface the URL — don't block the full
+                # `timeout_s` on a callback nobody will complete. Propagate so
+                # the caller can fail fast. Non-I/O callback errors (rendering,
+                # etc.) stay isolated below and must not break login.
+                raise
             except Exception:  # noqa: BLE001 — callback errors must not break login
                 pass
 

comfy_cli/command/setup.py

@@ -336,6 +336,12 @@ def _on_url(url: str) -> None:
         if e.hint:
             pprint(f"  [dim]{e.hint}[/dim]")
         return False
+    except OSError:
+        # A broken output stream (run_login re-raises OSError from the URL
+        # callback rather than blocking the full timeout on a dead terminal).
+        # In this interactive wizard the session is already unusable — degrade
+        # to a failed sign-in instead of an unhandled traceback.
+        return False
 
     store.save_cloud_session(
         base_url=result.base_url,

docs/json-output.md

@@ -110,6 +110,7 @@ line, ending the stream early.
 | `executed`         | Node finished and reported its outputs               |
 | `output`           | One file-like output became available (`url`)        |
 | `execution_error`  | Server reported a node exception (error envelope follows) |
+| `login_url`        | `comfy cloud login`: OAuth authorize URL, before the browser-callback wait |
 
 Agents must ignore events whose `type` they do not recognise — new event
 kinds may be added in a backward-compatible manner. Agents must ignore
@@ -266,6 +267,21 @@ envelope alone.
 {"schema": "event/1", "type": "execution_error", "prompt_id": "9b1c…", "details": {"node_id": "1", "exception_message": "API key invalid", "...": "..."}}
 ```
 
+### `login_url`
+
+Emitted by `comfy cloud login` under `--json`/`--json-stream` — this event is
+part of the sign-in stream, not the `run` stream. It carries the OAuth
+authorize URL as soon as it is built, and is flushed **before** the command
+blocks (up to `timeout_s` seconds) waiting for the loopback browser callback,
+so a parent process driving login headlessly can open (or forward) `url` in
+time. If the callback never arrives, the terminal envelope is an
+`oauth_timeout` error; on success it is the login envelope (`data.action:
+"login"`, `data.session` with tokens redacted).
+
+```json
+{"schema": "event/1", "type": "login_url", "url": "https://api.comfy.org/oauth/authorize?...", "timeout_s": 300}
+```
+
 ## Success envelope
 
 On `--wait` success, `data` carries:

tests/comfy_cli/auth/test_oauth.py

@@ -354,6 +354,35 @@ def test_run_login_times_out_when_browser_never_returns(monkeypatch: pytest.Monk
     assert exc.value.code == "oauth_timeout"
 
 
+def test_run_login_propagates_oserror_from_url_callback(monkeypatch: pytest.MonkeyPatch):
+    """A broken output stream in `on_url_ready` (e.g. a piped `--json` parent
+    that hung up) must fail fast — run_login re-raises the OSError instead of
+    swallowing it and blocking the full `timeout_s` on a callback wait nobody
+    will complete. Non-I/O callback errors stay isolated (tested elsewhere)."""
+    monkeypatch.setattr(
+        oauth,
+        "_post_json",
+        lambda url, body: {"client_id": "mcp-dyn-X", "redirect_uris": body["redirect_uris"]},
+    )
+
+    def broken_callback(_url: str) -> None:
+        raise BrokenPipeError("parent hung up")
+
+    start = time.monotonic()
+    with patch.object(oauth.webbrowser, "open", return_value=True):
+        with pytest.raises(OSError):
+            oauth.run_login(
+                base_url="https://testcloud.comfy.org",
+                resource="https://testcloud.comfy.org/mcp",
+                # A long timeout would be a multi-second hang if the OSError were
+                # swallowed; failing fast means we never reach the wait.
+                timeout_s=30.0,
+                open_browser=False,
+                on_url_ready=broken_callback,
+            )
+    assert time.monotonic() - start < 5.0  # never blocked on the callback wait
+
+
 # ---------------------------------------------------------------------------
 # Store round-trip
 # ---------------------------------------------------------------------------

tests/comfy_cli/cloud/test_login_command.py

@@ -0,0 +1,188 @@
+"""``comfy cloud login`` machine-output tests.
+
+The pretty renderer surfaces the OAuth authorize URL for headless users; these
+tests pin the machine-mode contract added for agents/MCP: under ``--json`` the
+URL is emitted as a ``login_url`` ``event/1`` line and flushed *before*
+``run_login`` blocks on the loopback callback, so a pipe-reading parent can open
+it. The final envelope (success or ``oauth_timeout`` error) still renders, with
+the session redacted.
+
+Pattern: install a renderer in the mode under test, mock
+``comfy_cli.cloud.command.run_login`` (so no browser / network / callback wait),
+and capture stdout.
+"""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+import typer
+
+from comfy_cli.cloud import command, oauth
+from comfy_cli.output import Renderer, set_renderer
+from comfy_cli.output.renderer import OutputMode
+
+_AUTHORIZE_URL = "https://example/oauth/authorize?response_type=code&code_challenge=abc123"
+_ACCESS_TOKEN = "super-secret-access-token-do-not-leak"
+_REFRESH_TOKEN = "super-secret-refresh-token-do-not-leak"
+
+
+def _login_result() -> oauth.LoginResult:
+    tokens = oauth.TokenSet(
+        access_token=_ACCESS_TOKEN,
+        refresh_token=_REFRESH_TOKEN,
+        token_type="Bearer",
+        expires_in=3600,
+        expires_at=9999999999,
+        scope="mcp:tools:read mcp:tools:call",
+    )
+    return oauth.LoginResult(
+        tokens=tokens,
+        client_id="comfy-cli",
+        base_url="https://testcloud.comfy.org",
+        resource="https://testcloud.comfy.org/mcp",
+        scope="mcp:tools:read mcp:tools:call",
+        redirect_uri="http://127.0.0.1:51234/callback",
+    )
+
+
+def _parse_lines(out: str) -> list[dict]:
+    return [json.loads(line) for line in out.splitlines() if line.strip()]
+
+
+@pytest.fixture(autouse=True)
+def _no_tracking(monkeypatch):
+    """Keep the ``@track_command`` decorator inert (no network / consent I/O)."""
+    monkeypatch.setattr("comfy_cli.tracking.track_event", lambda *a, **k: None)
+
+
+def test_json_login_emits_login_url_event_before_envelope(monkeypatch, capsys):
+    """`comfy --json cloud login --no-browser` streams a `login_url` event
+    (carrying the authorize URL) ahead of the final, session-redacted envelope."""
+
+    def fake_run_login(**kwargs):
+        # run_login fires on_url_ready once the authorize URL is built, before
+        # it blocks on the loopback callback. Emulate exactly that ordering.
+        kwargs["on_url_ready"](_AUTHORIZE_URL)
+        return _login_result()
+
+    monkeypatch.setattr(command, "run_login", fake_run_login)
+    set_renderer(Renderer(mode=OutputMode.JSON, command="cloud login", version="test"))
+
+    command.login_cmd(no_browser=True, timeout=300)
+
+    out = capsys.readouterr().out
+    lines = _parse_lines(out)
+
+    # First the event, then the envelope — the parent must see the URL first.
+    assert lines[0]["schema"] == "event/1"
+    assert lines[0]["type"] == "login_url"
+    assert lines[0]["url"] == _AUTHORIZE_URL
+    assert lines[0]["timeout_s"] == 300
+
+    envelope = lines[-1]
+    assert envelope["type"] == "envelope"
+    assert envelope["ok"] is True
+    assert envelope["data"]["action"] == "login"
+
+    # Ordering is explicit, not incidental.
+    types = [ln.get("type") for ln in lines]
+    assert types.index("login_url") < types.index("envelope")
+
+    # Session is redacted: the raw tokens never reach stdout.
+    session = envelope["data"]["session"]
+    assert session["tokens_redacted"] is True
+    assert _ACCESS_TOKEN not in out
+    assert _REFRESH_TOKEN not in out
+
+
+def test_pretty_login_prints_url_without_event_line(monkeypatch, capsys):
+    """Pretty mode is unchanged: the URL is printed for the human, and no
+    machine `login_url` event line is emitted."""
+
+    def fake_run_login(**kwargs):
+        kwargs["on_url_ready"](_AUTHORIZE_URL)
+        return _login_result()
+
+    monkeypatch.setattr(command, "run_login", fake_run_login)
+    # Default renderer is pretty (fixture in conftest resets the singleton).
+    set_renderer(Renderer(mode=OutputMode.PRETTY, command="cloud login", version="test"))
+
+    command.login_cmd(no_browser=True, timeout=300)
+
+    out = capsys.readouterr().out
+    assert _AUTHORIZE_URL in out
+    # No machine event/envelope leaked onto stdout in pretty mode.
+    assert '"type": "login_url"' not in out
+    assert "event/1" not in out
+
+
+def test_json_login_timeout_still_emits_login_url_before_error_envelope(monkeypatch, capsys):
+    """On timeout, the real `run_login` has already fired `on_url_ready` (before
+    it blocks on the callback wait), so a `--json` timeout emits the `login_url`
+    line *then* the `oauth_timeout` error envelope — the feature's headline
+    guarantee (URL surfaced before the block) holds even when the block times
+    out. Emulate that ordering, not an impossible URL-less timeout."""
+
+    def fake_run_login(**kwargs):
+        # Real run_login surfaces the URL, then the wait times out.
+        kwargs["on_url_ready"](_AUTHORIZE_URL)
+        raise oauth.OAuthTimeout(
+            "timed out waiting for browser callback after 300s",
+            hint="re-run `comfy cloud login` and complete the sign-in in your browser",
+        )
+
+    monkeypatch.setattr(command, "run_login", fake_run_login)
+    set_renderer(Renderer(mode=OutputMode.JSON, command="cloud login", version="test"))
+
+    with pytest.raises(typer.Exit) as exc:
+        command.login_cmd(no_browser=True, timeout=300)
+    assert exc.value.exit_code == 1
+
+    out = capsys.readouterr().out
+    lines = _parse_lines(out)
+
+    # The URL is surfaced first, then the error envelope closes the stream.
+    assert lines[0]["type"] == "login_url"
+    assert lines[0]["url"] == _AUTHORIZE_URL
+    envelope = lines[-1]
+    assert envelope["type"] == "envelope"
+    assert envelope["ok"] is False
+    assert envelope["error"]["code"] == "oauth_timeout"
+    types = [ln.get("type") for ln in lines]
+    assert types.index("login_url") < types.index("envelope")
+
+
+def test_json_login_fails_fast_when_login_url_write_breaks(monkeypatch, capsys):
+    """If the machine stream breaks while emitting `login_url` (a piped parent
+    hung up), `run_login` re-raises the `OSError` from the callback instead of
+    blocking the full timeout, and `login_cmd` fails fast with exit code 1
+    rather than a silent 300s hang."""
+
+    class _BrokenStream:
+        def write(self, _s):
+            raise BrokenPipeError("parent hung up")
+
+        def flush(self):
+            raise BrokenPipeError("parent hung up")
+
+    blocked = {"waited": False}
+
+    def real_ish_run_login(**kwargs):
+        # Mirror run_login's contract: fire the URL callback (which now writes to
+        # the broken stream) *before* the callback wait. A correct implementation
+        # never reaches the wait because the OSError propagates.
+        kwargs["on_url_ready"](_AUTHORIZE_URL)
+        blocked["waited"] = True  # only reached if the OSError was swallowed
+        raise oauth.OAuthTimeout("timed out", hint="")
+
+    monkeypatch.setattr(command, "run_login", real_ish_run_login)
+    renderer = Renderer(mode=OutputMode.JSON, command="cloud login", version="test")
+    renderer.machine_stream = _BrokenStream()
+    set_renderer(renderer)
+
+    with pytest.raises(typer.Exit) as exc:
+        command.login_cmd(no_browser=True, timeout=300)
+    assert exc.value.exit_code == 1
+    assert blocked["waited"] is False  # never blocked on the wait