fix: comfy generate refresh fetches /openapi with /openapi.yml fallback (BE-3391)#560
fix: comfy generate refresh fetches /openapi with /openapi.yml fallback (BE-3391)#560mattmillerai wants to merge 1 commit into
Conversation
…ck (BE-3391) api.comfy.org/openapi.yml returns 404; the live spec is served at /openapi (JSON body). _refresh() now fetches /openapi first and falls back to /openapi.yml on a 404 (preserving custom COMFY_API_BASE_URL deployments that serve the yml path). Validate the fetched body with the same loader load_raw_spec uses and require a top-level 'paths' mapping before caching, so a 200-with-garbage response can no longer poison ~/.comfy/openapi-cache.yml for the 7-day TTL.
dosubot
Bot
added
size:M
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 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 ignored due to path filters (1)
📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
mattmillerai
added
agent-coded
![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.
Judge call failed (status=parse_error): Could not parse JSON findings from output. First 500 chars:
Based on my review of the actual code, I've verified each finding. Key checks: `_YamlLoader` **does** inherit from `yaml.SafeLoader` (spec.py:24), so Gemini's RCE claim is a false positive — I've dropped that and kept only the real YAML-bomb DoS. `base_url()` (spec.py:280-286) reads `servers[0]["url"]` from the cached spec and is called outside `_refresh`'s try block; `_registry()` (spec.py:339) does `next(iter(node.keys()))` which crashes on string path values. All confirmed.
[
{
"file":
Re-trigger by removing and re-adding the cursor-review label.
1 participant
ELI-5
comfy generate refreshdownloads the latest list of partner AI models so the CLI knows about brand-new ones. It was asking the server for a file at/openapi.yml— but that address now returns 404 Not Found. The real, up-to-date spec lives at/openapi(no extension, served as JSON). Sorefresh— the only way to update the model catalog — was completely broken. This points it at the working address (and keeps the old one as a fallback), and refuses to save anything that isn't a real spec so a bad download can't silently breakcomfy generatefor the next 7 days.What changed
_refresh()(app.py) now fetchesspec.base_url() + "/openapi"first, and falls back to/openapi.ymlon a 404 — preserving any customCOMFY_API_BASE_URLdeployment that still serves the.ymlpath.spec.load_raw_spec()uses (validate_spec_text(), new helper inspec.py) and must contain a top-levelpathsmapping. JSON is a subset of YAML 1.2 and the custom loader only restricts bool resolution, so the JSON body served at/openapiparses cleanly. If the body doesn't parse or has nopaths,refreshexits 1 without writing the cache — previously a200-with-garbage response would have been cached into~/.comfy/openapi-cache.ymland poisoned every subsequentcomfy generateforCACHE_TTL_SECONDS(7 days).spec.write_cache()is unchanged — the cache file staying.ymlis fine since the loader reads JSON.Verification
api.comfy.org(the acceptance criterion):comfy generate refresh→ fetched/openapi(HTTP 200, ~973 KB JSON), wrote~/.comfy/openapi-cache.yml, printedRefreshed model catalog.comfy generate listafterwards read the refreshed cache and rendered the model table (includingflux-2, which is ahead of the vendoredspec/openapi.yml).curl -so /dev/null -w '%{http_code}' https://api.comfy.org/openapi.yml→404;/openapi→200 application/json.tests/comfy_cli/command/generate/):/openapiand writes the cache on a valid JSON spec body;/openapifalls back to/openapi.yml;paths) exits 1 and leaves a pre-existing cache byte-for-byte untouched;spec.load_raw_spec()andspec.get_endpoint()resolves an endpoint from it.ruff format --check+ruff checkclean on the changed files; full suite green (2573 passed, 37 skipped).Notes / judgment calls
/openapi.yml) is empirically 404 today; the change is authorized by the ticket and the old path is retained as a 404 fallback, so custom deployments serving the.ymlpath keep working._isolate_spec_cachesfixture totest_app.pythat clears the module-levelload_raw_spec/_registrylru_caches around each test. The new refresh tests monkeypatchspec._USER_CACHEto a temp file, andbase_url()loads that temp spec into the process-global caches; without the clear, a temp spec leaked into later tests (empty registry → spurious "No models match" failures). The fixture adds a small re-parse cost per test but keeps the suite deterministic.