fix: resolve host-relative follow-up links against the origin, not baseUrl#21
Merged
Merged
Conversation
…seUrl
Mirrors ComfyPythonSDK#17: a serverless gateway serves the v2 contract
under /deployment/{id}/api/v2 and returns job.urls.* links that already
include that mount prefix; joining them to baseUrl doubled the prefix and
404'd every poll after submit. Server links (leading slash + containing
/api/) now resolve against scheme+authority; internal shorthand paths and
Cloud / self-hosted behavior are byte-identical to before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KV7wUb2hyzKHsR52ZJTvAm
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Mounted link resolution and validation src/low/transport.ts, src/low/followUpLinks.test.ts |
ComfyLow.urlFor resolves server paths containing /api/ against the origin of baseUrl, while tests verify gateway prefixes, shorthand paths, bare-surface links, and authorization headers. |
Suggested reviewers: wei-hai
🚥 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
sundar/host-relative-follow-up-links
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
sundar/host-relative-follow-up-links
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 @coderabbitai help to get the list of available commands.
wei-hai
approved these changes
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
TypeScript mirror of Comfy-Org/ComfyPythonSDK#17 — the same bug existed verbatim in
urlFor(src/low/transport.ts).The serverless gateway serves the v2 contract under
/deployment/{id}/api/v2and returnsjob.urls.*follow-up links that are host-relative and already include that mount prefix.urlForonly recognized links starting with/api/, so a gateway self link fell into the internal-shorthand branch and was built asbaseUrl + /api/v2 + link→ double prefix → 404 on the first poll after submit.Fix: a leading-slash path containing
/api/is a server-returned link and resolves against the origin — the link is authoritative about its own path. Internal shorthand paths (/jobs/…,/assets…) still resolve underbaseUrl + /api/v2; Cloud / self-hosted construction is byte-identical (baseUrl == origin there); auth attachment is unaffected (same origin, existingisSameOriginlogic).The resolution rule is being pinned in the contract itself: Comfy-Org/cloud#5498 documents it on the
JobUrlsschema.How tested
pnpm test— 134 passed (130 existing + 4 new resolution/auth regression tests insrc/low/followUpLinks.test.ts, driven through a capturingfetchso the full request path is exercised)pnpm lint,pnpm typecheckclean🤖 Generated with Claude Code
https://claude.ai/code/session_01KV7wUb2hyzKHsR52ZJTvAm