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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0cd76b66-9583-4978-8c3b-9a9059a7dfa7

📥 Commits

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

📒 Files selected for processing (4)

• comfy_cli/command/generate/app.py
• 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.