feat!: post a model run to Comfy Router with the model's native body

CHANGELOG.md

@@ -114,6 +114,43 @@ notes for each version.
 
 ### Changed
 
+- **Breaking (wire): `client.models.run` now posts to Comfy Router.** It sends
+  `POST {COMFY_ROUTER_BASE_URL}/v1/models/{provider}/{model}` — the route
+  `spec/router-openapi.yaml` declares as `runRouterModel` — with the partner
+  model's **own native JSON input** as the body, forwarded to the provider
+  unchanged. It previously posted `{COMFY_BASE_URL}/api/v2/models/run` with a
+  `{"model": ..., "arguments": {...}}` envelope, which nothing serves: the
+  `/api/v2` surface is jobs and assets, and the model-ID-addressed invocation
+  routes are Router's. The Python method signature is unchanged
+  (`run(model, arguments, *, idempotency_key=None, timeout=...)`), the result is
+  still the provider's payload returned as-is, and the `Idempotency-Key` and
+  retry behaviour are unchanged — what moved is the URL and the body shape.
+  **Anyone who pointed `COMFY_BASE_URL` at a Router host to make model runs work
+  must now point `COMFY_ROUTER_BASE_URL` there instead**, and set
+  `COMFY_BASE_URL` back at their v2 deployment (or unset it for Comfy Cloud).
+- The `model` argument to `client.models.run` is now the canonical
+  `{provider}/{model}` id, because it *is* the two path segments the route is
+  addressed by. Exactly two non-empty segments are accepted; a one-segment id, a
+  three-segment `{provider}/{model}/{variant}` id (that form is not addressable
+  on this route yet), and any `.`/`..` segment now raise `ValueError` locally
+  before a request is made, and a non-string raises `TypeError`. Each segment is
+  percent-encoded into exactly one path segment. Previously any string was
+  accepted and travelled in the body. Mirrors the TypeScript SDK's
+  `parseModelId`, so the two SDKs accept and reject the same ids.
+- `COMFY_ROUTER_BASE_URL` (default `https://api.comfy.org`) selects the Router
+  deployment, deliberately separate from `COMFY_BASE_URL` — one variable
+  pointed at both surfaces would send jobs to Router or model runs to the v2
+  API. Same validation and read-per-construction rules as `COMFY_BASE_URL`, and
+  the same name as the TypeScript SDK's. `COMFY_ROUTER_BASE_URL` and
+  `ROUTER_BASE_URL_ENV_VAR` are exported from `comfy_sdk`.
+- `client.models.base_url` (and the namespace's `repr`) now reports the Router
+  base URL rather than the client's `COMFY_BASE_URL`, since that is the host
+  model runs actually reach. The client's own `base_url` is unchanged.
+- The client's API key is attached to *both* of its configured origins — the
+  `COMFY_BASE_URL` deployment and the `COMFY_ROUTER_BASE_URL` Router — and to no
+  third origin. Both are set by the caller (a constant or an environment
+  variable), never by a server response, so a server-returned absolute
+  follow-up link pointing anywhere else still receives no credential.
 - A client targeting a deployment named by `COMFY_BASE_URL` is unchanged: with
   no key resolved it is still built without one and still sends no credentials,
   which is what a self-hosted ComfyUI behind the API proxy needs. Only the Comfy

README.md

@@ -185,6 +185,21 @@ export COMFY_BASE_URL="http://127.0.0.1:8189"               # self-hosted proxy
 It is read each time a client is constructed, must be an `http(s)` URL, and an
 unset or blank value (including whitespace-only) means Comfy Cloud.
 
+`COMFY_BASE_URL` selects the **jobs and assets** surface. `client.models` talks
+to a different one — Comfy Router, `https://api.comfy.org` — and follows its own
+variable, `COMFY_ROUTER_BASE_URL`, with the same rules (read per construction,
+`http(s)` only, blank means the default):
+
+```bash
+export COMFY_ROUTER_BASE_URL="https://api.comfy.org"  # the default; set it to redirect model runs
+```
+
+Two variables rather than one because they are genuinely two hosts: the v2
+surface serves `/api/v2/jobs` and `/api/v2/assets`, Router serves
+`/v1/models/{provider}/{model}`, and neither serves the other's routes. Your
+`COMFY_API_KEY` is the credential for both — the SDK attaches it to those two
+configured origins and to no third one.
+
 Upgrading from an earlier version: `Comfy("<url>", "<key>")` becomes
 `Comfy(api_key="<key>")` with `COMFY_BASE_URL` set. `api_key` is keyword-only,
 so the old positional call raises `TypeError` rather than reading a URL as a
@@ -362,27 +377,51 @@ Model operations live in a namespace on the client you already constructed —
 ```python
 client = Comfy(api_key="comfyui-...")
 
-client.models.base_url   # the client's own base URL, where model requests go
+client.models.base_url   # Comfy Router's base URL, where model runs go
 client.models.timeout    # the client's own HTTP timeout
 ```
 
 The namespace is bound to that client's transport, so it uses the client's
-credentials, base URL, connection pool and timeout, and a configuration change
-made on the client afterwards applies through `models` as well — there is no
-second set of settings to keep in sync. `AsyncComfy` carries the same `models`
-namespace, and nothing extra is imported or constructed for it:
+credentials, connection pool and timeout, and a configuration change made on
+the client afterwards applies through `models` as well — there is no second set
+of settings to keep in sync. `AsyncComfy` carries the same `models` namespace,
+and nothing extra is imported or constructed for it:
 `from comfy_sdk import Comfy` stays the only entry point.
 
-`base_url` and `timeout` are a read-only view of that shared configuration;
-model operations are added to this namespace as they land.
+The one setting it does **not** share is the target host: `client.models.base_url`
+reports `COMFY_ROUTER_BASE_URL` (`https://api.comfy.org` by default), not the
+client's `COMFY_BASE_URL`. See
+[Targeting another deployment](#targeting-another-deployment) for why they are
+two variables.
+
+`base_url` and `timeout` are a read-only view of that configuration; model
+operations are added to this namespace as they land.
 
 ### `models.run` — one call, one result
 
 ```python
-result = client.models.run("acme/flux/dev", {"prompt": "a cat", "steps": 4})
+result = client.models.run("fal-ai/flux-pro", {"prompt": "a cat", "steps": 4})
 result["images"][0]["url"]
 ```
 
+That call is `POST https://api.comfy.org/v1/models/fal-ai/flux-pro` with
+`{"prompt": "a cat", "steps": 4}` as the body.
+
+Three things follow from that, and they are the whole contract of this method:
+
+- **It targets Comfy Router** — `https://api.comfy.org`, redirected by
+  `COMFY_ROUTER_BASE_URL`, not by `COMFY_BASE_URL`. Your API key goes with it.
+- **The first argument is the model's canonical id**, `{provider}/{model}` —
+  exactly the two segments that address the route, and exactly what Router's
+  model catalog lists. It must be two non-empty segments: `"fal-ai"` alone,
+  `"fal-ai/flux-pro/fp8"` (the three-segment variant form, which this route does
+  not take yet), and anything containing a `.` or `..` segment all raise
+  `ValueError` locally, before any request. A non-string raises `TypeError`.
+- **The second argument is the model's own native input**, forwarded to the
+  provider unchanged. There is no Comfy-shaped envelope around it: whatever the
+  partner's own API documents as the request body is what you pass here, so you
+  can move between the partner's API and Router by changing the host.
+
 `run` returns when the generation is **complete**. There is no submit step and
 nothing to poll: where the platform has to submit-and-poll an upstream
 provider, that happens server side inside this one call. The value you get back
@@ -393,7 +432,7 @@ The awaitable form is the **async client**, not a differently-named method:
 
 ```python
 async with AsyncComfy(api_key="comfyui-...") as client:
-    result = await client.models.run("acme/flux/dev", {"prompt": "a cat"})
+    result = await client.models.run("fal-ai/flux-pro", {"prompt": "a cat"})
 ```
 
 There is no `run_async()`, and there will not be one — one operation, one name,
@@ -467,10 +506,11 @@ arrives at Comfy's own ten-minute deadline, so a single-window budget would
 already be spent when it lands and the collect attempt it exists for would never
 start. Nothing else pays for that room.
 
-A note on what the default trades: `POST /models/run` is in neither vendored
-spec, so a deployment may apply the v2 rule instead and keep the key claimed
-across the `504`. There the collect resend comes back `422
-idempotency_key_reuse` in place of the real `504`. Set
+A note on what the default trades: the collect rule is
+`spec/router-openapi.yaml`'s, so it binds Comfy Router — but
+`COMFY_ROUTER_BASE_URL` can name a deployment that applies the v2 rule instead
+and keeps the key claimed across the `504`. There the collect resend comes back
+`422 idempotency_key_reuse` in place of the real `504`. Set
 `retry_collectable=False` on such a deployment.
 
 Other 5xx responses and client-side timeouts are the cases left out by default,

scripts/check_drift.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 """Fail if the committed tree drifts from the vendored specs.
 
-Two checks, one job:
+Three checks, one job:
 
 1. **Models vs ``spec/openapi.yaml``.** Regenerates the models into a temp file
    and diffs against the committed one, so a spec edit without a regen (or a
@@ -14,8 +14,16 @@
    for, which would reach callers as an untyped ``RouterError``. This compares
    the spec's ``x-comfy-error-types`` list against ``ROUTER_ERROR_TYPES``,
    which is what makes the next vendored Router sync a real diff review.
+3. **The bound model-run route vs ``spec/router-openapi.yaml``.** Same reason,
+   different artifact: ``comfy_low.transport`` posts a model run to a
+   hand-written path constant and a hand-written host constant. The spec
+   declares both -- the path whose ``post.operationId`` is ``runRouterModel``,
+   and ``servers[0].url``. A sync that *moves* the route (the ``/v1`` -> ``/v2``
+   move already on the roadmap) while those constants stay put would leave the
+   SDK posting to a route the contract no longer declares, with nothing else in
+   CI noticing.
 
-``tests/test_router_spec_contract.py`` asserts the same thing from the test
+``tests/test_router_spec_contract.py`` asserts the same things from the test
 suite. Both exist on purpose: the suite is where a contributor sees it, and
 this script is the job that fails a spec-only PR that never ran pytest.
 """
@@ -117,6 +125,99 @@ def _declared_router_error_types() -> list[str]:
     return values
 
 
+def _declared_run_route() -> tuple[str, str]:
+    """The spec's ``(runRouterModel path, servers[0].url)``.
+
+    Same failure policy as :func:`_declared_router_error_types`: every way the
+    file can be unusable becomes a ``ValueError`` with a sentence someone can
+    act on, rather than a traceback that reads like a bug in the checker.
+    """
+    try:
+        import yaml
+    except ImportError as exc:  # pragma: no cover - depends on the install extra
+        raise ValueError(
+            f"PyYAML is not installed, so {ROUTER_SPEC.name} cannot be read "
+            "(pip install -e '.[dev]')"
+        ) from exc
+
+    try:
+        doc = yaml.safe_load(ROUTER_SPEC.read_text(encoding="utf-8"))
+    except OSError as exc:
+        raise ValueError(f"{ROUTER_SPEC.name} could not be read: {exc}") from exc
+    except yaml.YAMLError as exc:
+        raise ValueError(f"{ROUTER_SPEC.name} is not valid YAML: {exc}") from exc
+
+    if not isinstance(doc, dict):
+        raise ValueError(f"{ROUTER_SPEC.name} is not a mapping at the top level")
+    paths = doc.get("paths")
+    if not isinstance(paths, dict):
+        raise ValueError(f"{ROUTER_SPEC.name} has no paths object")
+    # Searched by operationId rather than looked up by the path we expect: a
+    # lookup would silently find nothing the day the path moves, which is the
+    # one day this check exists for.
+    declared = [
+        path
+        for path, item in paths.items()
+        if isinstance(item, dict)
+        and isinstance(item.get("post"), dict)
+        and item["post"].get("operationId") == "runRouterModel"
+    ]
+    if len(declared) != 1:
+        raise ValueError(
+            f"{ROUTER_SPEC.name} declares {len(declared)} paths with "
+            f"post.operationId 'runRouterModel' (expected exactly 1): {declared}"
+        )
+    servers = doc.get("servers")
+    if not isinstance(servers, list) or not servers or not isinstance(servers[0], dict):
+        raise ValueError(f"{ROUTER_SPEC.name} has no servers[0]")
+    host = servers[0].get("url")
+    if not isinstance(host, str) or not host:
+        raise ValueError(f"{ROUTER_SPEC.name}'s servers[0].url is not a non-empty string")
+    return declared[0], host
+
+
+def _check_router_run_route() -> int:
+    if not ROUTER_SPEC.exists():
+        print(f"ERROR: {ROUTER_SPEC.name} is missing from spec/", file=sys.stderr)
+        return 1
+    sys.path.insert(0, str(ROOT / "src"))
+    try:
+        from comfy_low.transport import _MODEL_RUN_PATH_TEMPLATE, ROUTER_BASE_URL
+    except Exception as exc:
+        print(f"ERROR: comfy_low.transport does not import: {exc!r}", file=sys.stderr)
+        return 1
+
+    try:
+        declared_path, declared_host = _declared_run_route()
+    except ValueError as exc:
+        print(f"ERROR: {exc}", file=sys.stderr)
+        return 1
+
+    failed = False
+    if declared_path != _MODEL_RUN_PATH_TEMPLATE:
+        print(
+            f"ERROR: the bound model-run route has drifted from {ROUTER_SPEC.name}.\n"
+            f"  spec (runRouterModel): {declared_path}\n"
+            f"  sdk  (_MODEL_RUN_PATH_TEMPLATE): {_MODEL_RUN_PATH_TEMPLATE}\n"
+            "  Update comfy_low.transport._MODEL_RUN_PATH_TEMPLATE to the spec's path.",
+            file=sys.stderr,
+        )
+        failed = True
+    if declared_host != ROUTER_BASE_URL:
+        print(
+            f"ERROR: the default router host has drifted from {ROUTER_SPEC.name}.\n"
+            f"  spec (servers[0].url): {declared_host}\n"
+            f"  sdk  (ROUTER_BASE_URL): {ROUTER_BASE_URL}\n"
+            "  Update comfy_low.transport.ROUTER_BASE_URL to the spec's server URL.",
+            file=sys.stderr,
+        )
+        failed = True
+    if failed:
+        return 1
+    print(f"OK: the SDK posts a model run to {declared_host}{declared_path}, as the spec declares")
+    return 0
+
+
 def _check_models() -> int:
     if not COMMITTED.exists():
         print("ERROR: committed models missing; run scripts/gen_models.sh", file=sys.stderr)
@@ -208,12 +309,13 @@ def _run(name: str, check: Callable[[], int]) -> int:
 
 
 def main() -> int:
-    # Both run every time: reporting only the first failure would hide the
-    # second one behind a fix for the first. `max` over both results rather
-    # than a short-circuit for the same reason.
+    # All three run every time: reporting only the first failure would hide the
+    # others behind a fix for it. `max` over the results rather than a
+    # short-circuit for the same reason.
     return max(
         _run("models", _check_models),
         _run("router error types", _check_router_error_types),
+        _run("router run route", _check_router_run_route),
     )
 
 

src/comfy_low/errors.py

@@ -158,7 +158,7 @@ def error_from_envelope(
     well-formed envelope (so a bare ``401`` with no JSON still maps to
     ``Unauthorized``).
 
-    Not every route answers in the envelope shape. ``POST /api/v2/models/run``
+    Not every route answers in the envelope shape. ``POST {router}/v1/models/{provider}/{model}``
     is fronted by Router, whose error body is ``{detail, error_type}`` and which
     repeats the same coarse bucket on the ``X-Comfy-Error-Type`` header
     (``spec/router-openapi.yaml``). Reading only ``error["code"]`` would collapse