feat: honor COMFY_LOCAL_URL env var for the local ComfyUI address (BE-3959)#571
feat: honor COMFY_LOCAL_URL env var for the local ComfyUI address (BE-3959)#571mattmillerai wants to merge 3 commits into
Conversation
…-3959) Add a process-wide override for the local ComfyUI target so every command (env, run, jobs, upload/download, nodes, workflow, models search, project, CQL) can point at a ComfyUI on a non-8188 address started outside comfy-cli. New module comfy_cli/local_address.py: - parse_local_url: http://host:port | host:port | http://host (port default 8188, scheme optional/http-only, IPv6 [::1]:8189 supported). - resolve_local_host_port: precedence flag > COMFY_LOCAL_URL > config.background > 127.0.0.1:8188, host and port resolved independently. A malformed env var is ignored with a one-line stderr warning (deduped) so it can't hard-break every command. Routed through it at every local resolution site: target.resolve_target (fixes upload/download, models, project, workflow, CQL), host_port.resolve_host_port (run + jobs), job_watcher (flag > state > env > default), and env_checker (comfy env now probes AND reports the resolved address). CQL default-fill sites switched from a literal 127.0.0.1/8188 default to None so they stop shadowing the env var and resolve through resolve_target.
mattmillerai
added
agent-coded
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
dosubot
Bot
added
size:L
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 6 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 4 |
| 🟢 Low | 2 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
…port fall-through (BE-3959) Address cursor-review panel findings on COMFY_LOCAL_URL support: - Bracket bare IPv6 literals at the probe choke point (check_comfy_server_running) and at the `comfy which` / `comfy env` / setup display sites, so an address like COMFY_LOCAL_URL=http://[::1]:8189 yields a valid http://[::1]:8189 URL instead of the malformed http://::1:8189 (which made the server always read as "down"). _bracket_host is idempotent so already-bracketed callers aren't double-bracketed. - Reject '['/']' in _validate_host: a stray-bracket authority (e.g. a[xyz]) is now ignored-with-warning rather than flowing into a malformed URL or being parsed as a Rich markup tag and crashing the command. Well-formed IPv6 is unaffected (brackets are stripped before validation). - parse_local_url returns port=None when the value omits a port, so a host-only COMFY_LOCAL_URL falls through to a recorded background port instead of shadowing it with a defaulted 8188 (honors the documented independent env > background > default). - Redact user:pass@ userinfo from the invalid-value warning (both the echoed value and the parse error message) so credentials in a mistyped var aren't logged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
ELI-5
comfy-cli always assumed your local ComfyUI lives at
127.0.0.1:8188. If youhad one running somewhere else (say deep-comfy's native ComfyUI on
:8189,started outside comfy-cli), your only option was to pass
--host/--portonevery single command — and
comfy env/comfy upload/comfy downloadhad no override at all. This PR adds one environment variable,
COMFY_LOCAL_URL,that points every command at the right address:
A per-command
--host/--portflag still wins over the env var. Fixes thecomfy-local-mcp breakage on boxes where ComfyUI isn't on
:8188(BE-3956).What changed
New module
comfy_cli/local_address.py:parse_local_url(value)— acceptshttp://host:port,host:port, orhttp://host(port defaults to8188; scheme optional and, if present,must be
http; bracketed IPv6 literals like[::1]:8189supported). RaisesValueErroron garbage.resolve_local_host_port(host, port, background)— precedenceflag >
COMFY_LOCAL_URL>config.background>127.0.0.1:8188, withhost and port resolved independently (an explicit
--portwith no--hoststill takes the env var's host, etc.). A malformedCOMFY_LOCAL_URLis ignored with a one-line stderr warning (deduplicated per process) so a
typo can't hard-break every command.
Routed through it at every local resolution site:
target.py::resolve_targetlocal leg — the central chokepoint, so thisalone fixes
upload/download,models search,project,workflow, andthe CQL object_info fetch.
host_port.py::resolve_host_port— the shared resolver forcomfy runandevery
comfy jobssubcommand; env layer inserted between the flag and theconfig.backgroundfallback.command/job_watcher.py— per-job recorded state still wins:flag >
state.port> env > default (so a running watcher keeps pollingthe server it was launched against). IPv6 host now bracketed before
_snapshot(matches thejobsresolver; pre-existing gap forstate.host).env_checker.py—comfy env(and its--json, which comfy-local-mcp'sserver_inforeturns) now probes AND reports the resolved addressinstead of the hardcoded
http://localhost:8188strings.cql/engine.py,cql/loader.py) switched from aliteral
127.0.0.1/8188signature default toNone, plus the leafcallers in
command/workflow.py,command/workflow_fragments.py,command/nodes.py, andcmdline.py, so they stop shadowing the env varwith a literal default and resolve through
resolve_target.command/setup.pycosmetic probe + thecomfy whichpanel probe incmdline.py.Docs:
COMFY_LOCAL_URLdocumented inREADME.md, the agent-facingcomfy_cli/skills/comfy/SKILL.mdrouting section, and the new module'sdocstring.
Testing
tests/comfy_cli/test_local_address.py(39 tests):parse_local_url(full URL /
host:port/ bare host / IPv6[::1]:8189/http://host/invalid inputs raise), precedence (
flag > env > background > default,independent host/port, invalid env ignored + deduped warning), and
integration that
resolve_target(where='local'),jobs._resolve_host_port,the
runresolver, andcomfy env --jsonall honor the env var (flag wins).2612 passed, 37 skipped);ruff check+ruff format --checkclean under the CI-pinnedruff==0.15.15.COMFY_LOCAL_URL=http://127.0.0.1:8189exportedand no flags —
comfy --json jobs status,comfy --json run, andcomfy uploadall target:8189(server_not_running/ connection-refusedon
127.0.0.1:8189); with--port 8188the flag wins (targets:8188).Judgment calls
COMFY_LOCAL_URLhandling inlaunch.py—comfy launchstartsa server on a chosen port; it shouldn't be redirected by the env var. Left as-is.
127.0.0.1/8188→Noneis the only behaviorchange to existing code (everything else is additive). For the no-env case it
resolves to the identical
127.0.0.1:8188; all networked callers already passhost/port explicitly, so the default only ever applied to the no-arg/offline path.
adds a capability, no deny/dead-end path).