Skip to content

Commit 05fd5e0

Browse files
committed
fix(install): make API fallback query the cloned repo, not always upstream
The fallback path inside checkout_stable_comfyui hardcoded get_latest_release("comfyanonymous", "ComfyUI") regardless of which repo was actually cloned. For `comfy install --url <fork> --version latest`, that picks a tag the fork may not have, then checkout fails on it. Plumb the install URL down to checkout_stable_comfyui and parse it into (owner, repo) before the API call. Non-GitHub URLs (local paths, GitLab, self-hosted) and missing URLs fall back to comfyanonymous/ComfyUI to preserve the prior behavior on edge cases. The local-resolver path is unchanged — it already reads tags from the cloned repo regardless of URL, which is the right thing. Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
1 parent dae8a42 commit 05fd5e0

2 files changed

Lines changed: 142 additions & 3 deletions

File tree

comfy_cli/command/install.py

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import os
22
import platform
3+
import re
34
import subprocess
45
import sys
56
from typing import TypedDict
@@ -189,7 +190,7 @@ def execute(
189190

190191
if version != "nightly":
191192
try:
192-
checkout_stable_comfyui(version=version, repo_dir=repo_dir)
193+
checkout_stable_comfyui(version=version, repo_dir=repo_dir, url=url)
193194
except GitHubRateLimitError as e:
194195
rprint(f"[bold red]Error checking out ComfyUI version: {e}[/bold red]")
195196
sys.exit(1)
@@ -498,14 +499,47 @@ def _resolve_latest_tag_from_local(repo_dir: str) -> tuple[str | None, bool]:
498499
return (best[1] if best else None), fetch_ok
499500

500501

501-
def checkout_stable_comfyui(version: str, repo_dir: str):
502+
_GITHUB_REPO_RE = re.compile(
503+
# `github.com[:/]<owner>/<repo>` with optional `.git` and optional setuptools-style
504+
# `@branch` suffix (matching what ``clone_comfyui`` accepts via ``rsplit("@", 1)``).
505+
# Branch names may contain slashes (`release/1.0`), so the `@<branch>` group is greedy
506+
# to end-of-string. The repo segment forbids `@` and `/` to avoid eating those parts.
507+
r"github\.com[/:]([^/\s]+)/([^/@\s]+?)(?:\.git)?(?:@.+)?/?$",
508+
)
509+
510+
511+
def _parse_github_owner_repo(url: str | None) -> tuple[str, str] | None:
512+
"""Parse a GitHub repo URL into ``(owner, repo)``.
513+
514+
Handles the URL forms ``clone_comfyui`` accepts:
515+
- ``https://github.com/owner/repo``
516+
- ``https://github.com/owner/repo.git``
517+
- ``https://github.com/owner/repo@branch`` (setuptools-style branch suffix)
518+
- ``git@github.com:owner/repo`` (SSH form)
519+
520+
Returns ``None`` for empty input, local paths, or non-GitHub URLs (GitLab,
521+
self-hosted, etc.) — the caller decides what to do with that.
522+
"""
523+
if not url:
524+
return None
525+
match = _GITHUB_REPO_RE.search(url)
526+
return (match.group(1), match.group(2)) if match else None
527+
528+
529+
def checkout_stable_comfyui(version: str, repo_dir: str, url: str | None = None):
502530
"""
503531
Supports installing stable releases of Comfy (semantic versioning) or the 'latest' version.
504532
505533
For ``version="latest"`` we resolve the highest stable semver tag from the
506534
local clone first to avoid burning the unauthenticated GitHub API budget
507535
(60 req/hr per IP). The ``releases/latest`` API is only consulted when local
508536
resolution turns up nothing.
537+
538+
The optional ``url`` is the install URL forwarded from ``execute``; it lets
539+
the API fallback query the same repo we cloned from (forks included)
540+
instead of always asking upstream. Non-GitHub URLs and missing URLs
541+
fall back to ``comfyanonymous/ComfyUI`` so the prior behavior is preserved
542+
for users who pass a local path or a non-GitHub remote.
509543
"""
510544
rprint(f"Looking for ComfyUI version '{version}'...")
511545
if version == "latest":
@@ -518,7 +552,8 @@ def checkout_stable_comfyui(version: str, repo_dir: str):
518552
)
519553
else:
520554
rprint("[yellow]No stable release tags found locally; querying GitHub API.[/yellow]")
521-
selected_release = get_latest_release("comfyanonymous", "ComfyUI")
555+
owner, repo = _parse_github_owner_repo(url) or ("comfyanonymous", "ComfyUI")
556+
selected_release = get_latest_release(owner, repo)
522557
if selected_release is None:
523558
rprint(f"Error: No release found for version '{version}'.")
524559
sys.exit(1)

tests/comfy_cli/command/github/test_pr.py

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from comfy_cli.command.install import (
1212
GitHubRateLimitError,
1313
PRInfo,
14+
_parse_github_owner_repo,
1415
_resolve_latest_tag_from_local,
1516
checkout_stable_comfyui,
1617
fetch_pr_info,
@@ -626,6 +627,57 @@ def test_tag_with_v_prefix_normalized(self, tmp_path):
626627
assert tag == "0.20.1"
627628

628629

630+
class TestParseGithubOwnerRepo:
631+
"""Cover the URL parser used by the API fallback to query the same repo
632+
we cloned from (forks included), instead of always asking upstream."""
633+
634+
@pytest.mark.parametrize(
635+
"url,expected",
636+
[
637+
# The default URL the install command uses
638+
("https://github.com/comfyanonymous/ComfyUI", ("comfyanonymous", "ComfyUI")),
639+
# With .git suffix
640+
("https://github.com/comfyanonymous/ComfyUI.git", ("comfyanonymous", "ComfyUI")),
641+
# With trailing slash
642+
("https://github.com/comfyanonymous/ComfyUI/", ("comfyanonymous", "ComfyUI")),
643+
# setuptools-style @branch suffix that clone_comfyui supports
644+
("https://github.com/comfyanonymous/ComfyUI@master", ("comfyanonymous", "ComfyUI")),
645+
("https://github.com/comfyanonymous/ComfyUI.git@release/1.0", ("comfyanonymous", "ComfyUI")),
646+
# Forks
647+
("https://github.com/myfork/ComfyUI", ("myfork", "ComfyUI")),
648+
("https://github.com/some-user/some-repo.git", ("some-user", "some-repo")),
649+
# SSH forms
650+
("git@github.com:comfyanonymous/ComfyUI", ("comfyanonymous", "ComfyUI")),
651+
("git@github.com:comfyanonymous/ComfyUI.git", ("comfyanonymous", "ComfyUI")),
652+
],
653+
)
654+
def test_parses_github_urls(self, url, expected):
655+
assert _parse_github_owner_repo(url) == expected
656+
657+
@pytest.mark.parametrize(
658+
"url",
659+
[
660+
None,
661+
"",
662+
"/local/path/to/comfyui", # local path
663+
"https://gitlab.com/foo/bar", # not GitHub
664+
"https://example.com/owner/repo", # not GitHub
665+
"https://github.com/owner/repo/pull/123", # not a repo URL
666+
"ftp://github.com/owner/repo", # exotic scheme — still parses since regex matches `github.com/...`
667+
],
668+
)
669+
def test_returns_none_for_non_github_urls(self, url):
670+
# The PR URL form (last case) intentionally doesn't match — `[^/@]+?` excludes `/`
671+
# so `repo/pull/123` cannot be the second capture; we want this to fall through
672+
# to the upstream default in the caller.
673+
if url == "ftp://github.com/owner/repo":
674+
# Edge-case: this DOES match because we don't anchor on the scheme.
675+
# That's fine — owner/repo is what matters; the API call uses HTTPS regardless.
676+
assert _parse_github_owner_repo(url) == ("owner", "repo")
677+
else:
678+
assert _parse_github_owner_repo(url) is None
679+
680+
629681
class TestCheckoutStableComfyUI:
630682
"""Verify checkout_stable_comfyui prefers local tag resolution over the
631683
GitHub API for `--version latest` (issue #440), and falls back when local
@@ -685,6 +737,58 @@ def test_latest_falls_back_to_api_when_local_empty(self, mock_local, mock_api, m
685737
mock_api.assert_called_once_with("comfyanonymous", "ComfyUI")
686738
mock_co.assert_called_once_with("/repo", "v0.20.1")
687739

740+
@patch("comfy_cli.command.install.git_checkout_tag", return_value=True)
741+
@patch("comfy_cli.command.install.get_latest_release")
742+
@patch("comfy_cli.command.install._resolve_latest_tag_from_local", return_value=(None, True))
743+
def test_latest_fallback_uses_fork_owner_repo_from_url(self, mock_local, mock_api, mock_co):
744+
"""Fork case: API fallback must query the FORK's releases/latest, not upstream's.
745+
746+
Otherwise we'd ask GitHub for `comfyanonymous/ComfyUI`'s latest tag and
747+
try to check it out in a fork that may not have it.
748+
"""
749+
mock_api.return_value = {"tag": "v0.20.1-myfork", "version": None, "download_url": "u"}
750+
751+
checkout_stable_comfyui("latest", "/repo", url="https://github.com/myfork/ComfyUI")
752+
753+
mock_api.assert_called_once_with("myfork", "ComfyUI")
754+
mock_co.assert_called_once_with("/repo", "v0.20.1-myfork")
755+
756+
@patch("comfy_cli.command.install.git_checkout_tag", return_value=True)
757+
@patch("comfy_cli.command.install.get_latest_release")
758+
@patch("comfy_cli.command.install._resolve_latest_tag_from_local", return_value=(None, True))
759+
def test_latest_fallback_strips_branch_suffix_from_url(self, mock_local, mock_api, mock_co):
760+
"""The setuptools-style `@branch` suffix in the install URL must not leak
761+
into the API call. `clone_comfyui` already strips it before cloning."""
762+
mock_api.return_value = {"tag": "v0.20.1", "version": None, "download_url": "u"}
763+
764+
checkout_stable_comfyui("latest", "/repo", url="https://github.com/myfork/ComfyUI.git@some-branch")
765+
766+
mock_api.assert_called_once_with("myfork", "ComfyUI")
767+
768+
@patch("comfy_cli.command.install.git_checkout_tag", return_value=True)
769+
@patch("comfy_cli.command.install.get_latest_release")
770+
@patch("comfy_cli.command.install._resolve_latest_tag_from_local", return_value=(None, True))
771+
def test_latest_fallback_defaults_to_upstream_for_non_github_url(self, mock_local, mock_api, mock_co):
772+
"""Non-GitHub URLs (local paths, GitLab, etc.) fall back to upstream defaults
773+
— preserves prior behavior for users whose URL we can't parse."""
774+
mock_api.return_value = {"tag": "v0.20.1", "version": None, "download_url": "u"}
775+
776+
checkout_stable_comfyui("latest", "/repo", url="/local/path/to/comfyui")
777+
778+
mock_api.assert_called_once_with("comfyanonymous", "ComfyUI")
779+
780+
@patch("comfy_cli.command.install.git_checkout_tag", return_value=True)
781+
@patch("comfy_cli.command.install.get_latest_release")
782+
@patch("comfy_cli.command.install._resolve_latest_tag_from_local", return_value=(None, True))
783+
def test_latest_fallback_defaults_to_upstream_when_url_omitted(self, mock_local, mock_api, mock_co):
784+
"""Backward compat: omitting the new `url` kwarg yields the prior behavior
785+
(querying upstream)."""
786+
mock_api.return_value = {"tag": "v0.20.1", "version": None, "download_url": "u"}
787+
788+
checkout_stable_comfyui("latest", "/repo") # no url=
789+
790+
mock_api.assert_called_once_with("comfyanonymous", "ComfyUI")
791+
688792
@patch("comfy_cli.command.install.git_checkout_tag", return_value=True)
689793
@patch("comfy_cli.command.install.get_latest_release")
690794
@patch("comfy_cli.command.install._resolve_latest_tag_from_local", return_value=(None, False))

0 commit comments

Comments
 (0)