chore: sync vendored Comfy API v2 spec (cloud@4204113) - #42
Conversation
📝 WalkthroughWalkthroughThe OpenAPI specification documents the public generated copy, serverless deployment URLs, asset retention overrides and deadlines, deployment lifecycle errors, UUID examples, and host-relative URL resolution. ChangesAPI contract updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/openapi.yaml`:
- Line 863: Replace the truncated example for Output.id in the OpenAPI schema
with a complete valid UUID, consistent with the Asset UUID description and the
complete UUID examples used elsewhere.
- Line 775: Update _Prepared.url() to identify JobUrls links independently of
whether the path contains "/api/"; every host-relative JobUrls link must resolve
against the request origin (scheme and authority), while ordinary API paths
retain their existing base_url and _API resolution.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 95ffb4b2-7513-4163-9033-cd6b16a013ff
📒 Files selected for processing (1)
spec/openapi.yaml
| JobUrls: | ||
| type: object | ||
| description: Embedded follow-up links — follow these, don't build URLs. | ||
| description: Embedded follow-up links — follow these, don't build URLs. A link is either an absolute URL or a host-relative reference (leading `/`) that already includes any prefix the serving surface is mounted under (e.g. a serverless gateway's `/deployment/{deployment_id}/api/v2`). Clients MUST resolve a host-relative link against the request origin (scheme + authority), never against a configured base URL — joining it to a base URL that carries the same mount prefix duplicates the prefix. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'JobUrls|urljoin|urlparse|base_url|origin|\.urls\b' src --glob '*.py'Repository: Comfy-Org/comfy-python-sdk
Length of output: 19724
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- transport URL resolution ---'
sed -n '86,132p' src/comfy_low/transport.py
printf '%s\n' '--- all JobUrls field consumers ---'
rg -n -C 4 '\.urls\.(self|cancel|events)|JobUrls|urls\s*=' src tests spec --glob '*.py' --glob '*.yaml' --glob '*.yml' 2>/dev/null || true
printf '%s\n' '--- URL helper call sites ---'
rg -n -C 3 'self\._p\.url|_p\.url|\.url\(' src/comfy_low src/comfy_sdk --glob '*.py'Repository: Comfy-Org/comfy-python-sdk
Length of output: 16741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from urllib.parse import urlsplit
def resolved(base_url: str, path: str) -> str:
base_url = base_url.rstrip("/")
parts = urlsplit(base_url)
origin_url = f"{parts.scheme}://{parts.netloc}"
if path.startswith("http"):
return path
if path.startswith("/") and "/api/" in path:
return origin_url + path
return base_url + "/api/v2" + path
cases = [
("https://gateway.example/deployment/abc/api/v2", "/deployment/abc/api/v2/jobs/1"),
("https://gateway.example/deployment/abc/api/v2", "/jobs/1"),
("https://gateway.example/api/v2", "/custom/jobs/1"),
]
for base, link in cases:
print(f"base={base!r} link={link!r} -> {resolved(base, link)!r}")
PYRepository: Comfy-Org/comfy-python-sdk
Length of output: 562
Resolve every host-relative JobUrls link against the request origin
_Prepared.url() uses the request origin only when the path contains "/api/". A valid mounted link without that substring is treated as API-relative, so the client prepends base_url and _API and can duplicate the mount prefix. Separate server-link resolution from ordinary API paths, and resolve every host-relative JobUrls link against the origin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/openapi.yaml` at line 775, Update _Prepared.url() to identify JobUrls
links independently of whether the path contains "/api/"; every host-relative
JobUrls link must resolve against the request origin (scheme and authority),
while ordinary API paths retain their existing base_url and _API resolution.
| type: string | ||
| description: Asset UUID. | ||
| example: asset_01JZV9R4N8... | ||
| example: 9f8a1c0d-2b3e-4f56-... |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a complete UUID for Output.id.
9f8a1c0d-2b3e-4f56-... is not a UUID. It conflicts with the Asset UUID description and the complete UUID examples elsewhere in the schema.
Proposed fix
- example: 9f8a1c0d-2b3e-4f56-...
+ example: 9f8a1c0d-2b3e-4f56-8a7b-1c2d3e4f5a6b📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| example: 9f8a1c0d-2b3e-4f56-... | |
| example: 9f8a1c0d-2b3e-4f56-8a7b-1c2d3e4f5a6b |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/openapi.yaml` at line 863, Replace the truncated example for Output.id
in the OpenAPI schema with a complete valid UUID, consistent with the Asset UUID
description and the complete UUID examples used elsewhere.
Automated sync of the public Comfy API v2 spec, projected from the
canonical contract (internal notes stripped, all component schemas
kept). Source:
cloud@4204113.This PR is on its own per-source-commit branch
(
chore/sync-v2-spec-4204113); a laterspec change opens a separate PR and will not touch this branch, so a
regen commit pushed here is safe.
Action required before merge: regenerate the low layer and commit
the result so the spec-drift check passes —
Summary by CodeRabbit
New Features
Documentation