fix(generate): fetch spec from /openapi not /openapi.yml (BE-2982)

[mattmillerai]

ELI-5

comfy generate refresh downloads the latest list of API models. It was
knocking on the wrong door — /openapi.yml — which the server answers with
"404 Not Found", so the command always failed and exited 1. The spec actually
lives at /openapi (served as JSON). This changes the one URL so the door
opens.

What changed

• comfy_cli/command/generate/app.py — _refresh() now fetches
  <base_url>/openapi instead of <base_url>/openapi.yml.
• Added a regression test asserting _refresh() requests <base_url>/openapi.

Nothing else needed changing: the server returns JSON, and JSON is a subset of
YAML, so write_cache() (verbatim write) + load_raw_spec() (yaml.load)
parse it fine. base_url(), the vendored spec, and the /proxy/ allowlist are
untouched.

Why it's safe

• base_url() rstrips the trailing slash, so the URL resolves to
  https://api.comfy.org/openapi.
• _refresh() is the only spec-fetch call site (grepped — all other
  openapi.yml references are file paths: the bundled spec and the
  ~/.comfy/openapi-cache.yml cache filename).

Verification (live, 2026-07-14)

• GET https://api.comfy.org/openapi → 200; body parses as YAML,
  openapi: 3.0.2, servers: [{url: https://api.comfy.org}] (matches the
  vendored spec so base_url() resolves identically), 314 paths.
• GET https://api.comfy.org/openapi.yml → 404.
• Full test suite: 2574 passed, 37 skipped. ruff format --check +
  ruff check clean.

Notes

• While writing the regression test I found (and fixed) a test-isolation trap:
  reading base_url() after refresh repopulates the lru_cached spec from
  the freshly-written minimal tmp cache, which leaked into later tests. The
  test now resolves the expected URL before invoking refresh.

[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: 25 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: 35876919-aa98-4330-8fc6-becf11b10e76

📥 Commits

Reviewing files that changed from the base of the PR and between 8613835 and f0aa096.

📒 Files selected for processing (3)

• comfy_cli/command/generate/spec.py
• tests/comfy_cli/command/generate/test_app.py
• tests/comfy_cli/command/generate/test_spec.py

📝 Walkthrough

Walkthrough

The refresh command now fetches /openapi, validates the response as an OpenAPI mapping, and caches it only when valid. YAML parsing handles exponent-only floats, with tests covering parsing, validation, endpoint selection, and cache rejection.

Changes

OpenAPI refresh validation

Layer / File(s)	Summary
Spec parsing and validation comfy_cli/command/generate/spec.py, tests/comfy_cli/command/generate/test_spec.py	The YAML loader parses exponent-only floats, while validate_spec_text accepts valid OpenAPI mappings and rejects invalid bodies.
Refresh fetch and cache flow comfy_cli/command/generate/app.py, tests/comfy_cli/command/generate/test_app.py	Refresh requests /openapi, validates the response before writing the cache, and exits with an error without caching invalid content.

Sequence Diagram(s)

sequenceDiagram
  participant GenerateRefresh
  participant OpenAPIEndpoint
  participant validate_spec_text
  participant OpenAPICache
  GenerateRefresh->>OpenAPIEndpoint: GET /openapi
  OpenAPIEndpoint-->>GenerateRefresh: Spec response text
  GenerateRefresh->>validate_spec_text: Validate response text
  validate_spec_text-->>GenerateRefresh: Parsed mapping or SpecError
  GenerateRefresh->>OpenAPICache: Write valid response

Loading

Possibly related PRs

• Comfy-Org/comfy-cli#452: Adds the generate implementation touched by this refresh and spec validation work.

Suggested reviewers: robinjhuang

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-2982-refresh-openapi-path

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-2982-refresh-openapi-path

---

_{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
🟠 High	1
🟢 Low	2

Panel: 6/8 reviewers contributed findings.

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

[mattmillerai]

Automated resolver note: the only failing check ("Windows Specific Commands" / `test`) is a repo-wide infra flake, not caused by this PR. It fails in the Install Dependencies step (before any test runs) with:

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

a Windows .pyd file-lock during comfy install that reproduces on every recent PR across all branches. All real test suites pass (Ubuntu / macOS / windows-latest 3.10 "Run Tests on Multiple Platforms", GPU runners, ruff, CodeQL). Branch protection is disabled, so this non-required check does not block merge. All 3 CodeRabbit review threads are resolved. Ready to merge.

[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-3278|BE-3278 — Fix repo-wide Windows CI flake: set UV_LINK_MODE=copy in test-windows.yml

[mattmillerai]

Resolver pass — pushed 8613835 (test-only), no source changes needed.

Review threads: all 3 were already addressed in 633220f (spec validation + JSON float resolver) and are resolved. The one deferral — origin-pinning (r3582325058) — is now recorded as a tracked follow-up rather than left in a reply; it needs a human call on whether COMFY_API_BASE_URL must be https, since that could break local/self-hosted http setups.

Gap found and fixed: validate_spec_text() and the point-less-exponent float resolver were verified by hand but had no regression test — either could have been deleted with CI staying green. 8613835 adds direct coverage in test_spec.py, confirmed to fail against main's spec.py and pass with the fix. Suite: 186 passed.

Verified the PR's premise empirically against the live API rather than trusting the description:

• GET /openapi.yml → 404
• GET /openapi → 200, application/json, ~973KB, parses to a valid OpenAPI mapping with servers[0].url = https://api.comfy.org (matches base_url(), so no host redirection today)
• Float resolver: 1e+16/1e-07 → float, while 3.0.2, on/off, v1e5 stay strings

CI: the red test check is the repo-wide Windows flake, not this diff — uv fails to hardlink over a locked pydantic_core/_pydantic_core.cp312-win_amd64.pyd (Access is denied, os error 5) during Install Dependencies, before any test runs. It's currently failing on nearly every open branch; reran it and it reproduced identically. A separate follow-up (set UV_LINK_MODE=copy in test-windows.yml) is already filed. Every other check is green.

No merge conflict — branch is 0 behind main.

[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-3322|BE-3322 — Pin/validate the OpenAPI spec origin before trusting and caching the refreshed spec

[mattmillerai]

Resolver pass — no code changes needed; nothing in this diff is broken. Recording why the two red checks are not this PR.

build (new since the last pass) — repo-wide dependency regression, not this diff.
9 tests fail with ValueError: Comment cannot contain line breaks (8 in test_config_parser.py, plus test_node_init.py::test_node_init_strips_credentials surfacing the same error as assert 1 == 0). The identical 9 failures reproduce on PRs #545, #544, #543 and #541, which share none of this diff.

Root cause: tomlkit 0.15.x now rejects newlines in comments, and comfy_cli/registry/config_parser.py:75 / :250 pass triple-quoted multi-line strings to .comment(). pyproject.toml:49 declares tomlkit unpinned, so the build job resolves 0.15.1 and fails, while uv.lock-based jobs pin 0.13.3 and pass — which is why it is green locally and in every locked check. Reproduced directly:

$ uv run --with tomlkit --no-project python -c "import tomlkit; d=tomlkit.document(); d[chr(120)]=tomlkit.array(); d[chr(120)].comment(chr(10)+chr(10))"
version: 0.15.1
ValueError: Comment cannot contain line breaks

Out of scope here (this PR is a one-line spec-fetch path fix in generate/, and the fix belongs with the tomlkit pin + registry/ call sites), so it is recorded as a tracked follow-up rather than dropped: replace the two multi-line .comment() calls with per-line comments and bound tomlkit in pyproject.toml.

test (Windows) — the known repo-wide infra flake, already tracked as BE-3278 (UV_LINK_MODE=copy). Fails in Install Dependencies before any test runs (uv pip install exit 2 → ImportError: cannot import name __version__ from pydantic_core), same .pyd file-lock signature as before.

This PR: all 3 review threads resolved and answered. No merge conflict (MERGEABLE, 0 behind main). tests/comfy_cli/command/generate/ → 186 passed; ruff format --check clean. Every check other than the two repo-wide failures above is green. Ready to merge.

[mattmillerai]

Correction to my previous comment: the tomlkit 0.15 build breakage is already tracked, with fixes in flight — no new follow-up needed, and I have withdrawn the one I was about to file rather than add a third duplicate:

• fix(registry): emit pyproject hint blocks as single-line tomlkit comments #533 (BE-3285) — "fix(registry): emit pyproject hint blocks as single-line tomlkit comments"
• fix(registry): emit pyproject hints as standalone comments for tomlkit 0.15 #536 (BE-3290) — "fix(registry): emit pyproject hints as standalone comments for tomlkit 0.15"
• ci: install test dependencies from uv.lock instead of resolving fresh #537 (BE-3292) — "ci: install test dependencies from uv.lock instead of resolving fresh" (addresses the pinned-vs-unpinned resolution skew that lets 0.15.1 into the build job at all)

Those two overlapping fixes (#533/#536) probably want de-duplicating against each other, but that is a call for their authors, not this PR.

Everything else in my previous comment stands: #517 needs no code changes — 3/3 review threads resolved, no merge conflict, generate tests 186 passed, ruff clean, and both red checks are repo-wide issues tracked elsewhere.

[bigcat88]

This PR currently conflicts with main — GitHub reports mergeable: CONFLICTING, so it needs a rebase before I can review it and I'm skipping it in the current review sweep.

Please rebase (or merge main in) and I'll pick it up on the next pass. main moved a fair bit in the last day, including #614 (ANSI sanitisation across the pretty-print call sites) and #628 (the duplicate server_died error-code fix that had main red), so a refresh may also clear unrelated CI noise on this branch.