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

[mattmillerai]

ELI-5

client.models.run("fal-ai/flux-pro", {...}) was posting to a URL nothing serves. It sent POST {COMFY_BASE_URL}/api/v2/models/run with a Comfy-shaped {"model": ..., "arguments": {...}} envelope — but /api/v2 is the jobs-and-assets surface, and running a partner model is Comfy Router's job, on Router's own host. This points the call at the route the vendored contract already declares: POST https://api.comfy.org/v1/models/{provider}/{model}, with the partner model's own native JSON as the body, forwarded to the provider unchanged. The Python you write is unchanged; the request on the wire is what moves.

What changed

• COMFY_ROUTER_BASE_URL (default https://api.comfy.org) selects the Router deployment, deliberately separate from COMFY_BASE_URL and validated by the same rules (http(s) only, no query/fragment, blank means the default, read per construction, trailing slash stripped). COMFY_ROUTER_BASE_URL and ROUTER_BASE_URL_ENV_VAR are exported from comfy_sdk. One variable pointed at both surfaces would send jobs to Router or model runs to the v2 API, and neither serves the other's routes.
• comfy_low.transport.parse_model_id splits the canonical {provider}/{model} id into the two path segments that address the route, mirroring the TypeScript SDK's parseModelId: exactly two non-empty segments, no ./.. segment, a message that names the three-segment variant form specifically (rather than "invalid id", which would send a caller hunting for a typo), and TypeError rather than ValueError for a non-string. Each segment is percent-encoded with safe="", so nothing in an id can add a path segment, a query or a fragment.
• _MODEL_RUN_PATH_TEMPLATE replaces _MODEL_RUN_PATH, verbatim from the spec, and model_run_request returns the filled path with dict(arguments) unwrapped as the body.
• post_model_run (sync and async) builds its URL absolute against the router base URL so it does not pick up the /api/v2 prefix _Prepared.url adds to a relative path. ComfyLow/AsyncComfyLow take a router_base_url= kwarg and expose router_base_url / safe_router_base_url.
• models.base_url and its repr now report the router base URL — the host these calls actually reach — rather than the client's COMFY_BASE_URL.
• Spec-drift gates, in both places the repo already gates the Router contract: tests/test_router_spec_contract.py::test_run_path_matches_vendored_spec and a third check in scripts/check_drift.py. Both find the path by searching for post.operationId == "runRouterModel" rather than looking up the path they expect — a lookup would pass vacuously on the one day it matters — and both also compare servers[0].url against the default host.
• Docs: the README models.run section states the host, the {provider}/{model} id rule and the native-body contract; a ### Changed CHANGELOG entry marks the breaking wire change.

The riskiest line, and why it is safe

_Prepared.headers broadened its same-origin credential check from one origin to two:

if self.api_key and origin(url) in (self._base_origin, self._router_origin):

That check exists to stop a server-returned absolute follow-up link (job.urls.self / cancel / events) from carrying the bearer token to an attacker-chosen host. Both origins in the new tuple come from the client's own construction — a module constant, or an environment variable the operator set — and never from a server response, so the set of hosts a malicious server can aim the credential at is unchanged at exactly zero. Without the second origin every real model run against a correctly configured client would 401, since Router is a different host from Comfy Cloud by default. tests/test_transport_security.py asserts both directions: the router origin gets the token when it is not base_url, and a third origin still gets nothing once the router is trusted.

The second-riskiest is the new ValueError in _Prepared.__init__ for a non-http router_base_url. Without it, url() — which passes a string through only when it starts with http — would fall through to the base_url + /api/v2 + ... branch and send an authenticated run at a mangled URL on the wrong surface. Comfy() never reaches it (the env resolver validates first); it only binds on direct ComfyLow(...) construction.

Verification of the ticket's acceptance criteria

Criterion	Result
pytest green	620 passed, 4 skipped
scripts/check_drift.py green	all three checks OK, exit 0
A run against a fake server receives POST /v1/models/acme/flux-dev with body {"prompt": ...} and an Idempotency-Key	Verified end to end — see below
A run against the live Router raises a typed RouterError (404 model_not_found with X-Comfy-Request-Id) rather than a bare 404	Not exercised — see ## Residual

The fake-server run, spelled out rather than left to the suite:

path            : /v1/models/acme/flux-dev
body            : {'prompt': 'a cat'}
Idempotency-Key : ['8019c74f-bab3-4cf0-9530-5d3eaf682ae3']

The drift gates were verified to actually fire, not merely to pass: with the vendored path temporarily edited /v1 → /v2 (a local edit, reverted), check_drift.py reported spec (runRouterModel): /v2/models/{provider}/{model} / sdk (_MODEL_RUN_PATH_TEMPLATE): /v1/models/{provider}/{model} and exited 1, and test_run_path_matches_vendored_spec failed. That is the exact /v1 → /v2 sync scenario the gate is for.

Negative-claim falsification

This diff adds a capability denial — a three-segment {provider}/{model}/{variant} id, which the previous code accepted, now raises ValueError before any request. I went looking for a path on which that capability actually works.

It cannot be a regression, and this is the decisive point. For a user to be broken, models.run would have had to be reaching Router with a three-segment id. It never sent a path at all: the id travelled in the body of a request to {COMFY_BASE_URL}/api/v2/models/run. So the capability being refused was never functional through this SDK — the refusal is a forward-looking limit on a route being bound for the first time, not the removal of something that worked.

Against the contract the SDK is bound to (read out of spec/router-openapi.yaml, not asserted): it declares exactly three paths — /v1/models, /v1/models/{provider}/{model}, /v1/models/{provider}/{model}/openapi.json — none with a third model segment; RouterModelId's pattern is ^[a-z0-9]+([._-][a-z0-9]+)*/[a-z0-9]+([._-][a-z0-9]+)*$, exactly two segments, and is documented as "exactly the value that addresses the model on POST /v1/models/{provider}/{model}"; and a catalog entry's id is those same two segments joined. Nothing the catalog lists is three-segment.

Against the live host: inconclusive, and I can say precisely why. api.comfy.org does distinguish unknown paths from known ones before auth (POST /v1/definitely/not/a/route → 404, POST /v1/models/acme/flux-dev → 401), which looked like a usable signal — but the auth boundary turns out to be a prefix rule, not a per-route one: POST /v1/models/a/b/c/d, which no declared route matches, also returns 401. So the 401 on a three-segment path carries no information about whether that path routes. Resolving it needs a credential, which this environment does not have.

The dead-end redirects rather than terminates. The ValueError names the working alternative — "that route takes the two-segment {provider}/{model} id. Pass the id the catalog lists" — and GET /v1/models is the discovery endpoint that yields those ids.

Judgment calls

• ROUTER_BASE_URL literal lives in comfy_low.transport, and comfy_sdk.client.COMFY_ROUTER_BASE_URL is bound to it rather than being a second literal. The transport owns the wire and needs a default for its router_base_url= kwarg; two independent literals for one host is precisely the drift the new gate exists to catch.
• _resolve_base_url was factored onto a shared _resolve_env_url(var, default) so the two targets cannot validate differently. The v2 path's behaviour — including its exact error message — is byte-identical; only the variable name in the message varies, which is the point.
• Corrected three now-false statements the diff made stale, rather than leaving them: the README, retry.py and a retry-test comment each said POST /models/run "is in neither vendored spec". It is in spec/router-openapi.yaml, which also states the deadline_exceeded collect rule the default retry policy is built on. The substance of the caveat survives — a deployment COMFY_ROUTER_BASE_URL names may apply the v2 rule instead — but the premise had to be fixed.
• tests/test_sync_async_parity.py's _default_deployment now pops both variables. It exists so a stray export in a developer's shell cannot turn the module into a collection error, and _resolve_router_base_url raises the same way _resolve_base_url does; leaving it popping one would have reopened that hole.

Residual

• The live-Router acceptance criterion was not exercised. The ticket asks that Comfy().models.run("not-a/real-model", {}) against the real host raise a RouterError subclass — a 404 model_not_found carrying X-Comfy-Request-Id — proving the request reached a Router handler. No API key is available in this environment, so the typed-404 half is unverified. What was verified, unauthenticated and non-mutating: POST https://api.comfy.org/v1/models/not-a/real-model returns 401 with an X-Comfy-Request-Id (eb3cb6e0-…), so a Comfy Router handler on the right host did answer the URL this PR now builds; auth is simply checked before model lookup. A reviewer with a key should run the ticket's exact command and confirm the error is a typed RouterError rather than a bare 404. Related and also unverified: whether the 401 body shape ({"message": ...}, no X-Comfy-Error-Type) — which differs from the RouterRequestError {detail, error_type} shape the spec declares — maps to the typed hierarchy correctly, or falls through to an untyped error. That is an edge-gateway response rather than Router's own, and it is worth its own look.
• Whether a three-segment {provider}/{model}/{variant} id is addressable on the live route is unresolved. The vendored contract says no (see the falsification section) and this PR refuses it locally on that basis, which is what the ticket specifies. The live host cannot answer it without a credential, because it auth-gates the whole /v1/models* prefix. If a variant id turns out to be addressable, the fix is to relax parse_model_id and add the third segment to the path template — the refusal is one function and one constant.
• The TypeScript SDK was not read. The ticket names src/sdk/credentials.ts and src/sdk/models.ts (runUrl / parseModelId) as the behaviour this mirrors. That repo is not checked out in this environment, so the parity claim rests on the ticket's description of it plus the shared vendored contract, not on a diff of the two implementations. Worth a read by someone with both repos open, specifically on the id-rejection rules: an id one SDK accepts and the other refuses is exactly the cross-SDK divergence this was meant to close.
• No integration test covers the new route. tests/integration/test_gateway_e2e.py is opt-in and points at a live v2 deployment; it neither exercises models.run nor reads COMFY_ROUTER_BASE_URL. All coverage here is against the stub in tests/conftest.py.
• Existing callers who pointed COMFY_BASE_URL at a Router host to make model runs work must move that value to COMFY_ROUTER_BASE_URL. Flagged in the CHANGELOG as breaking. Nothing in the SDK detects or warns about that configuration; a stale setup will send jobs to Router and get route errors rather than a message naming the variable.

Provenance

• Authored by: agent-work loop
• Verified: pytest: 620 passed, 4 skipped, 0 failed. ruff check .: all checks passed. ruff format --check .: 51 files already formatted. mypy src: no issues in 19 source files. scripts/check_drift.py: 3/3 checks OK, exit 0 (and verified to exit 1 on a simulated /v1→/v2 spec move). scripts/check_public_repo_hygiene.py: no internal-only references. Live unauthenticated probes of api.comfy.org as described above.
• Deviations: the live-Router acceptance criterion (typed RouterError on a 404) was not exercised — no API key in this environment; see ## Residual.

[coderabbitai]

Warning

Review limit reached

Next included review available in 17 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 130 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f6da224b-cfb6-47d4-bbc0-7a3a493e1f63

📥 Commits

Reviewing files that changed from the base of the PR and between 5466cfd and b83bfc0.

📒 Files selected for processing (18)

• CHANGELOG.md
• README.md
• scripts/check_drift.py
• src/comfy_low/errors.py
• src/comfy_low/transport.py
• src/comfy_sdk/__init__.py
• src/comfy_sdk/client.py
• src/comfy_sdk/models.py
• src/comfy_sdk/retry.py
• tests/conftest.py
• tests/test_api_key.py
• tests/test_error_mapping.py
• tests/test_models_namespace.py
• tests/test_models_run.py
• tests/test_models_run_retry.py
• tests/test_router_spec_contract.py
• tests/test_sync_async_parity.py
• tests/test_transport_security.py

---

_{Comment @coderabbitai help to get the list of available commands.}

[robinjhuang]

Approved — full autonomy check passed.

---

Generated by Claude Code

[robinjhuang]

Approved — full autonomy check passed.

---

Generated by Claude Code

[robinjhuang]

Approved — full autonomy check passed.

---

Generated by Claude Code

[robinjhuang]

Auto-approved under the full-autonomy policy.

Gates verified at b83bfc001a937bf1b20110325a2f90fd3e44da82:

• full-autonomy label present
• assigned to, or review requested from, @robinjhuang
• not a draft
• 8 required check(s) green — none failing, none pending

Issued by full-autonomy-approve.yml (run). This approval attests
that the machine gates above passed at this commit. It does not attest that a
human read the diff.