fix: add HTTP timeouts to eight bare requests.* calls (BE-3272)

[mattmillerai]

ELI-5

Some of the commands in this tool reach out over the network (downloading model
files, uploading a published node, fetching the standalone Python release info,
talking to the node registry). Eight of those calls forgot to set a timeout.
Python's requests library, given no timeout, will wait forever if the server
on the other end goes silent mid-connection — so a single stalled peer could
freeze the CLI indefinitely with no error and no way out but Ctrl-C.

This PR gives every one of those eight calls a timeout, using two shared
constants so we stop sprinkling magic numbers around. Now, if a peer stalls, the
call fails fast with a normal requests.Timeout error instead of hanging.

What changed

Added two constants to the existing comfy_cli/http.py module:

Constant	Value	Used for
DEFAULT_HTTP_TIMEOUT	30.0	plain, non-streaming API calls
DOWNLOAD_TIMEOUT	(10.0, 60.0) — (connect, read)	streaming downloads and large uploads

For streaming transfers the (connect, read) tuple form matters: requests
applies the read timeout per socket read, not to the whole transfer — so it
caps how long we wait to start connecting/receiving without capping a
legitimately long download/upload.

The eight previously-bare call sites, now with timeouts:

Non-streaming → DEFAULT_HTTP_TIMEOUT

• comfy_cli/file_utils.py:61 — check_unauthorized GET (opened with stream=True, but only the
  status code is read and the body is never consumed, so the flat timeout applies)
• comfy_cli/standalone.py:38 — SHA256SUMS fetch
• comfy_cli/standalone.py:76 — latest-release JSON fetch
• comfy_cli/registry/api.py:82 — publish POST
• comfy_cli/registry/api.py:101 — list GET
• comfy_cli/registry/api.py:124 — install GET

Streaming / transfer → DOWNLOAD_TIMEOUT

• comfy_cli/file_utils.py:467 — signed-URL PUT upload
• comfy_cli/utils.py:123 — download_url streaming GET

Behavior change

Only the failure mode changes: an indefinite hang becomes a typed
requests.Timeout (a subclass of requests.RequestException). Happy paths are
unaffected. check_unauthorized already catches requests.RequestException, so
a timeout there degrades to its existing "can't determine → False" behavior; the
other sites now raise instead of hanging, which is strictly better than the prior
infinite wait.

Tests

• Added regression tests asserting the timeout kwarg is passed at every touched
  site (test_http.py, test_utils.py, test_standalone.py,
  registry/test_api.py, test_file_utils_network.py), plus sanity checks on
  the two new constants.
• Full suite green: 2581 passed, 37 skipped. ruff check + ruff format
  clean.

Judgment calls

• comfy_cli/http.py already existed (it holds a NoRedirectHandler); the
  ticket phrased the fix as "add to comfy_cli/http.py". I appended the two
  constants to that module rather than creating a new file, keeping all shared
  HTTP defaults in one place.
• comfy_cli/command/models/models.py:156 was excluded as the ticket
  directed — it already passes timeout=10 on a continuation line. Grep
  confirmed the other pre-existing requests.* calls in the tree already carry
  timeouts.
• This PR intentionally does not touch the httpx-based download_file
  path, which already has its own generous timeout + retry handling — out of
  scope for this ticket.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 42 minutes

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).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 693e6d56-955d-4a8f-acfb-affd6d8394cd

📥 Commits

Reviewing files that changed from the base of the PR and between a732f5c and b292cc8.

📒 Files selected for processing (10)

• comfy_cli/file_utils.py
• comfy_cli/http.py
• comfy_cli/registry/api.py
• comfy_cli/standalone.py
• comfy_cli/utils.py
• tests/comfy_cli/registry/test_api.py
• tests/comfy_cli/test_http.py
• tests/comfy_cli/test_standalone.py
• tests/comfy_cli/test_utils.py
• tests/test_file_utils_network.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3272-http-timeouts

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3272-http-timeouts

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 3 finding(s).

Severity	Count
🟢 Low	2
⚪ Nit	1

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[mattmillerai]

CI status — both failures are pre-existing, neither is from this PR

build (Run pytest) — upstream tomlkit break, already fixed by #533.
The 9 failures are all ValueError: Comment cannot contain line breaks in test_config_parser.py / test_node_init.py — files this PR does not touch (diff is file_utils.py, http.py, registry/api.py, standalone.py, utils.py + tests). I reproduced them locally on this branch:

• tomlkit==0.13.3 (what uv.lock pins) → all 9 pass
• tomlkit==0.15.1 (what CI resolves) → exactly those 9 fail

.github/workflows/pytest.yml installs with a bare pip install -e ., so it picks up the newest tomlkit rather than the locked one. main last ran green on 2026-07-13, before 0.15.1 landed. #533 (BE-3285) already fixes the underlying config_parser issue, so I've deliberately not duplicated that fix here — this branch should go green once #533 merges.

test (Windows) — dependency install flake. ImportError: cannot import name '__version__' from 'pydantic_core' (unknown location) during Install Dependencies, i.e. a broken venv before any of this PR's code runs. Re-running.

Review feedback addressed

All three Cursor findings fixed and threads resolved — see the commit above. Full suite green locally: 2583 passed, 37 skipped (+2 new regression tests, both verified to fail without the fix).

[mattmillerai]

CI update after the review-fix push — confirmed both failures are repo-wide, not this PR

build: same 9 failures, and my changes add 2 passing tests.
Passed count went 2573 → 2575 between the two runs (exactly the two regression tests I added), with the failure list byte-identical — all tomlkit ValueError: Comment cannot contain line breaks. So this PR introduces zero new failures. #533 fixes the root cause.

test (Windows): pre-existing infra bug, not a flake. I re-ran it and it failed identically. Root cause is in Install Dependencies, before any PR code runs:

error: failed to remove file ...\venv\Lib\site-packages\pydantic_core/_pydantic_core.cp312-win_amd64.pyd: Access is denied. (os error 5)

test-windows.yml creates one venv, pip install -e . into it, then runs comfy install --fast-deps — which shells out to uv pip install targeting that same venv. comfy is running from it, and its import chain (tracking.py → mixpanel → pydantic → pydantic_core) holds _pydantic_core.*.pyd open. Windows locks loaded DLLs, so the replacement fails and leaves pydantic_core unimportable.

Cross-PR evidence that neither failure is mine:

PR	build	test (Windows)
#527 (this)	fail — tomlkit	fail — pydantic_core lock
#526 (unrelated)	fail — tomlkit	fail
#533 (tomlkit fix)	pass	fail

#533 passing build while still failing Windows isolates these as two independent, pre-existing breakages. Both are logged as follow-ups (the Windows one wants a maintainer call on CI venv topology — the CLI shouldn't be installing over the interpreter it's running from).

This PR is ready to merge on its own merits: review threads all resolved, full suite green locally (2583 passed / 37 skipped), ruff_check green. build goes green once #533 lands.

[mattmillerai]

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

• https://linear.app/comfyorg/issue/BE-3292|BE-3292 — Pin test dependencies in pytest.yml so upstream releases can't break every PR's CI
• https://linear.app/comfyorg/issue/BE-3293|BE-3293 — Windows CI (test-windows.yml) fails for every PR: comfy install clobbers the venv it is running from

[mattmillerai]

Re-verified — still ready to merge, still gated on #533

Independent re-check of this branch at b292cc8:

• Full suite green locally: 2583 passed, 37 skipped.
• Lint green: ruff check + ruff format clean under the version CI pins (0.15.15). (Heads-up for anyone linting locally: older ruff flags 15 UP038 hits across the tree — that rule was removed in ruff 0.15, so they are a local-version artifact, not real findings.)
• Review threads: all 3 resolved.
• Mergeable: no conflicts with main.

build is still red for the same pre-existing reason. I re-confirmed the root cause rather than taking it on faith: config_parser.py:75 passes a multi-line string to tomlkits .comment(), and tomlkit 0.15 rejects it. Pinning locally isolates it exactly — tomlkit==0.13.3 (what uv.lock pins) → all 9 pass; tomlkit==0.15.1 (what CIs bare pip install -e . resolves) → exactly those 9 fail. That code dates to #438 (April) and is untouched by this PR, whose diff is confined to file_utils.py / http.py / registry/api.py / standalone.py / utils.py.

#533 is still open, so main still carries the break. I deliberately have not duplicated its fix here — that would conflict with #533 and pull an unrelated config_parser change into a timeouts PR. This branch goes green once #533 merges. Windows test remains the separate pre-existing venv-clobber bug (BE-3293).

One fix this round: the PR description listed check_unauthorized under DOWNLOAD_TIMEOUT, but the code uses DEFAULT_HTTP_TIMEOUT. The code is right — that call opens with stream=True but only reads the status code and never consumes the body, so a flat timeout is the correct choice. Corrected the description to match; no code change.

Follow-ups already filed: BE-3292 (pin CI test deps), BE-3293 (Windows CI venv clobber).