diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f5a3d0f..02dec480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,9 +29,24 @@ history. release, or one of the build's releases). The builder's message is carried whole up to 8 KiB, so the blocking deployment ids it names are no longer lost to the 1000-byte cap on the raw body. +- `comfy knowledge pick CAPABILITY --check-local` checks each `oss` pick's + template against the local ComfyUI's model folders, the same check + `comfy templates check` runs. A pick whose model files are missing gets + `available_locally: false`, an `unavailable_reason` and a `missing_models` + count, and a template absent from a fresh gallery index is flagged too. A pick + that could not be checked carries its own `local_check` naming why. The + payload's `local_check` is `ok` when the check ran. When the server is down, a + model folder listing is over the size cap, or the gallery cannot load, it + carries that error code and no pick is marked. The flag is off by default + because it fetches uncached template workflows and calls the local server. ### Fixed +- `comfy templates check` returns an error envelope instead of a traceback when + the gallery or workflow fetch gets a non-200 status or an over-cap body, or + when a model folder listing is over the size cap. It also percent-encodes the + model folder name it asks the local server for, accepts a folder name with + `..` inside it, and refreshes a stale gallery index before looking up the name. - A failed blob upload during `comfy build push` no longer writes the presigned PUT URL's query string to stdout, into the JSON envelope, or into a CI log. Both a rejected upload and a dropped connection quote the URL they were talking diff --git a/comfy_cli/command/knowledge.py b/comfy_cli/command/knowledge.py index d5449206..135a8078 100644 --- a/comfy_cli/command/knowledge.py +++ b/comfy_cli/command/knowledge.py @@ -2,7 +2,7 @@ comfy knowledge status [--refresh] comfy knowledge resolve - comfy knowledge pick [capability] + comfy knowledge pick [capability] [--check-local] Backed by :mod:`comfy_cli.knowledge`. JSON mode is the contract; pretty mode is a short courtesy view. @@ -181,6 +181,63 @@ def _emit_capabilities(renderer, bundle: knowledge.Bundle, *, query: str | None renderer.emit(payload, command="knowledge pick") +def _check_picks_locally(picks: list[dict[str, Any]]) -> str: + """Flag each ``oss`` pick whose template the local ComfyUI cannot run. + + Returns ``ok`` when the check ran. A pick that could not be checked carries + its own ``local_check`` instead of a flag. A failure that would repeat for + every pick (server down, no gallery) returns its error code instead and + marks nothing. + """ + from comfy_cli.command import templates as templates_cmd + + marks: list[tuple[dict[str, Any], dict[str, Any]]] = [] + listings: dict[str, list[str] | None] = {} + rows: list[dict[str, Any]] | None = None + index_fresh = False + try: + for pick in picks: + template = pick.get("template") + if pick.get("route") != "oss" or not template: + continue + if rows is None: + rows = templates_cmd._gallery_rows(None, refresh=False) + # A failed re-fetch serves the stale cache, which cannot show that + # a template is absent upstream. + index_fresh = not templates_cmd._cache_is_stale(templates_cmd._cache_path()) + try: + _row, wf = templates_cmd._template_workflow(template, rows, refresh=False) + except templates_cmd.TemplateCheckError as e: + if e.code == "template_not_found" and index_fresh: + flag = {"available_locally": False, "unavailable_reason": knowledge.UNAVAILABLE_TEMPLATE_NOT_FOUND} + marks.append((pick, flag)) + else: + marks.append((pick, {"local_check": e.code})) + continue + required = templates_cmd._collect_model_requirements(wf) + _present, missing, _warnings = templates_cmd._match_local_models(required, listings) + verdict = templates_cmd._compute_verdict( + api_dependent=False, + missing=missing, + required_count=len(required), + node_types=templates_cmd._collect_node_class_types(wf), + ) + if verdict == "unknown": + marks.append((pick, {"local_check": "unknown"})) + elif missing: + if all(listings.get(m["directory"]) is None for m in missing): + reason = knowledge.UNAVAILABLE_MODEL_FOLDER_NOT_FOUND + else: + reason = knowledge.UNAVAILABLE_MISSING_MODELS + flag = {"available_locally": False, "unavailable_reason": reason, "missing_models": len(missing)} + marks.append((pick, flag)) + except templates_cmd.TemplateCheckError as e: + return e.code + for pick, mark in marks: + pick.update(mark) + return "ok" + + @app.command("pick", help="Ranked model picks for a capability; omit it to list every capability.") @tracking.track_command("knowledge") def pick_cmd( @@ -188,6 +245,16 @@ def pick_cmd( str | None, typer.Argument(help="Capability id (e.g. lipsync, text-to-video). Omit to list every capability."), ] = None, + check_local: Annotated[ + bool, + typer.Option( + "--check-local", + help=( + "Check each oss pick's template against the local ComfyUI's model folders and flag " + "the picks it cannot run. Fetches uncached template workflows and calls the local server." + ), + ), + ] = False, ): renderer = get_renderer() bundle = _require_bundle(renderer) @@ -218,15 +285,21 @@ def pick_cmd( "compiled_at": bundle.compiled_at, "stale": bundle.stale, } + if check_local: + payload["local_check"] = _check_picks_locally(picks) if renderer.is_pretty(): from rich.table import Table columns = ("rank", "model", "route", "template", "status", "caveat", "best_for") + if check_local: + columns += ("unavailable_reason", "local_check") tbl = Table(show_header=True, header_style="bold") for col in columns: tbl.add_column(col) for p in picks: cells = {**p, "best_for": ", ".join(p.get("best_for") or [])} - tbl.add_row(*(sanitize_markup("" if cells[c] is None else cells[c]) for c in columns)) + tbl.add_row(*(sanitize_markup("" if cells.get(c) is None else cells[c]) for c in columns)) renderer.console().print(tbl) + if check_local and payload["local_check"] != "ok": + rprint(f"[yellow]local check did not finish:[/yellow] {payload['local_check']}") renderer.emit(payload, command="knowledge pick") diff --git a/comfy_cli/command/templates.py b/comfy_cli/command/templates.py index 578ff58f..fe9ecf8b 100644 --- a/comfy_cli/command/templates.py +++ b/comfy_cli/command/templates.py @@ -1322,7 +1322,7 @@ def _list_local_folder(target, folder: str) -> list[str] | None: # Reuse the exact target/URL plumbing `comfy models list-folder` uses. from comfy_cli.command.models.search import _http_get_json, _models_path_parts - url = target.url(*_models_path_parts(target), folder) + url = target.url(*_models_path_parts(target), urllib.parse.quote(folder, safe="")) try: data = _http_get_json(url, target) except urllib.error.HTTPError as e: @@ -1365,53 +1365,41 @@ def _compute_verdict(*, api_dependent: bool, missing: list, required_count: int, return "unknown" if loaderish else "runnable" -@app.command( - "check", - help=( - "Report whether a gallery template is runnable on THIS install: which of " - "its models are present vs missing locally, whether it needs partner-API " - "access, and any custom nodes it declares. Resolves the name against the " - "gallery, fetches the workflow, and intersects its model refs with the " - "local server's model folders." - ), -) -@tracking.track_command("templates") -def check_cmd( - name: Annotated[str, typer.Argument(help="Template name (matches `comfy templates ls` rows).")], - gallery_path: Annotated[ - str | None, - typer.Option("--gallery", show_default=False, help="Path to a local index.json (skips the cache + fetch)."), - ] = None, - refresh: Annotated[ - bool, - typer.Option("--refresh", help="Re-fetch the gallery index AND the template workflow before checking."), - ] = False, -): - from comfy_cli.target import resolve_target +class TemplateCheckError(Exception): + """A template-check failure, carrying the fields of its error envelope.""" + + def __init__(self, code: str, message: str, *, hint: str | None = None, details: dict[str, Any] | None = None): + super().__init__(message) + self.code = code + self.message = message + self.hint = hint + self.details = details - renderer = get_renderer() - # 1. Resolve the name against the gallery index (same affordance as `fetch`). +def _gallery_rows(gallery_path: str | None, *, refresh: bool) -> list[dict[str, Any]]: try: - cats = _load_gallery(gallery_path, refresh=refresh) - except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: - renderer.error(code="gallery_load_failed", message=str(e)) - raise typer.Exit(code=1) from e + cats = _load_gallery(gallery_path, refresh=refresh, background_ok=False) + except _GALLERY_LOAD_ERRORS as e: + raise TemplateCheckError("gallery_load_failed", str(e)) from e + return _flatten_templates(cats) - rows = _flatten_templates(cats) + +def _template_workflow( + name: str, rows: list[dict[str, Any]], *, refresh: bool +) -> tuple[dict[str, Any], dict[str, Any]]: + """Resolve ``name`` against the gallery ``rows`` and return ``(row, workflow)``, + reading the workflow JSON from the per-template cache or fetching it.""" match = next((r for r in rows if r["name"] == name), None) if match is None: lower = name.lower() close = [r["name"] for r in rows if lower in r["name"].lower()][:5] - renderer.error( - code="template_not_found", - message=f"no template named {name!r} in the gallery", + raise TemplateCheckError( + "template_not_found", + f"no template named {name!r} in the gallery", hint="try `comfy templates ls --name ` to search", details={"close_matches": close}, ) - raise typer.Exit(code=1) - # 2. Fetch (or read from cache) the per-template workflow JSON. cache_path = _template_workflow_cache_path(name) body: bytes | None = None if not refresh and cache_path.exists(): @@ -1422,11 +1410,11 @@ def check_cmd( if body is None: try: body = _fetch_template_workflow(name) - except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e: + except (urllib.error.URLError, OSError, RuntimeError, ResponseTooLarge) as e: status = getattr(e, "code", None) - renderer.error( - code="template_fetch_failed", - message=f"failed to fetch workflow for {name!r}: {e}", + raise TemplateCheckError( + "template_fetch_failed", + f"failed to fetch workflow for {name!r}: {e}", hint=( "the gallery index references a template whose workflow JSON " "is missing upstream — report at " @@ -1435,8 +1423,7 @@ def check_cmd( else "check network connectivity" ), details={"status": status} if status else None, - ) - raise typer.Exit(code=1) from e + ) from e # Write atomically: a truncated file (interrupted write / full disk) would # otherwise be trusted by the read path above on the next non-refresh run # and fail `template_workflow_invalid_json` until the user passed --refresh. @@ -1454,20 +1441,113 @@ def check_cmd( try: wf = json.loads(body) - except (json.JSONDecodeError, UnicodeDecodeError) as e: - renderer.error( - code="template_workflow_invalid_json", - message=f"template workflow for {name!r} is not valid JSON: {e}", + except (json.JSONDecodeError, UnicodeDecodeError, RecursionError) as e: + raise TemplateCheckError( + "template_workflow_invalid_json", + f"template workflow for {name!r} is not valid JSON: {e}", hint="re-run with --refresh to re-fetch, or report upstream", - ) - raise typer.Exit(code=1) from e + ) from e if not isinstance(wf, dict): - renderer.error( - code="template_workflow_invalid_json", - message=f"template workflow for {name!r} is not a JSON object", + raise TemplateCheckError( + "template_workflow_invalid_json", + f"template workflow for {name!r} is not a JSON object", hint="re-run with --refresh to re-fetch, or report upstream", ) - raise typer.Exit(code=1) + return match, wf + + +def _match_local_models( + required: list[dict[str, str]], listings: dict[str, list[str] | None] +) -> tuple[list[str], list[dict[str, str]], list[str]]: + """Split ``required`` into ``(present, missing, warnings)`` against the local + server's model folders, matching by basename. + + ``listings`` caches each folder's listing by directory, so a caller checking + several templates lists a shared folder once. + """ + warnings: list[str] = [] + present: list[str] = [] + missing: list[dict[str, str]] = [] + if not required: + return present, missing, warnings + + from comfy_cli.command.models.search import _is_walkable_folder_name + from comfy_cli.target import resolve_target + + target = resolve_target(where="local") + try: + for directory in dict.fromkeys(req["directory"] for req in required): + if not _is_walkable_folder_name(directory): + # Not addressable as a `/models/` segment — treat as absent. + listings[directory] = None + warnings.append( + f"model directory {directory!r} isn't a valid model folder — its files are reported missing" + ) + continue + if directory not in listings: + listings[directory] = _list_local_folder(target, directory) + if listings[directory] is None: + warnings.append( + f"model folder {directory!r} not found on the local server " + f"(custom-node folder?) — its files are reported missing" + ) + except ResponseTooLarge as e: + raise TemplateCheckError( + code="model_listing_too_large", + message=f"a local model folder listing is over the response size cap: {e}", + hint="check that the server on this host:port is ComfyUI", + ) from e + except (urllib.error.URLError, OSError, ValueError) as e: + raise TemplateCheckError( + "server_not_running", + f"local ComfyUI server is unreachable, cannot check installed models: {e}", + hint="run `comfy launch` to start a local server", + ) from e + + for req in required: + listing = listings.get(req["directory"]) + # Normalize BOTH sides: a model ref may itself carry a subfolder + # (e.g. ``SDXL/model.safetensors``, as ComfyUI loader widgets emit), + # so compare basenames on the required side too. + req_base = _basename(req["name"]) + if listing and any(_basename(entry) == req_base for entry in listing): + present.append(req["name"]) + else: + missing.append(dict(req)) + return present, missing, warnings + + +@app.command( + "check", + help=( + "Report whether a gallery template is runnable on THIS install: which of " + "its models are present vs missing locally, whether it needs partner-API " + "access, and any custom nodes it declares. Resolves the name against the " + "gallery, fetches the workflow, and intersects its model refs with the " + "local server's model folders." + ), +) +@tracking.track_command("templates") +def check_cmd( + name: Annotated[str, typer.Argument(help="Template name (matches `comfy templates ls` rows).")], + gallery_path: Annotated[ + str | None, + typer.Option("--gallery", show_default=False, help="Path to a local index.json (skips the cache + fetch)."), + ] = None, + refresh: Annotated[ + bool, + typer.Option("--refresh", help="Re-fetch the gallery index AND the template workflow before checking."), + ] = False, +): + renderer = get_renderer() + + # 1-2. Resolve the name against the gallery index (same affordance as `fetch`) + # and read the per-template workflow JSON from cache or fetch it. + try: + match, wf = _template_workflow(name, _gallery_rows(gallery_path, refresh=refresh), refresh=refresh) + except TemplateCheckError as e: + renderer.error(code=e.code, message=e.message, hint=e.hint, details=e.details) + raise typer.Exit(code=1) from e # 3. Model requirements (top-level + subgraph walk) and node class types. required = _collect_model_requirements(wf) @@ -1498,47 +1578,11 @@ def check_cmd( # 5. Installed intersection: list each distinct folder once on the local server # and match required files by basename. - warnings: list[str] = [] - present: list[str] = [] - missing: list[dict[str, str]] = [] - distinct_dirs = list(dict.fromkeys(req["directory"] for req in required)) - if required: - target = resolve_target(where="local") - listings: dict[str, list[str] | None] = {} - try: - for directory in distinct_dirs: - if not directory or ".." in directory or "/" in directory or "\\" in directory: - # Not addressable as a `/models/` segment — treat as absent. - listings[directory] = None - warnings.append( - f"model directory {directory!r} isn't a valid model folder — its files are reported missing" - ) - continue - folder_files = _list_local_folder(target, directory) - listings[directory] = folder_files - if folder_files is None: - warnings.append( - f"model folder {directory!r} not found on the local server " - f"(custom-node folder?) — its files are reported missing" - ) - except (urllib.error.URLError, OSError, ValueError, json.JSONDecodeError) as e: - renderer.error( - code="server_not_running", - message=f"local ComfyUI server is unreachable, cannot check installed models: {e}", - hint="run `comfy launch` to start a local server", - ) - raise typer.Exit(code=1) from e - - for req in required: - listing = listings.get(req["directory"]) - # Normalize BOTH sides: a model ref may itself carry a subfolder - # (e.g. ``SDXL/model.safetensors``, as ComfyUI loader widgets emit), - # so compare basenames on the required side too. - req_base = _basename(req["name"]) - if listing and any(_basename(entry) == req_base for entry in listing): - present.append(req["name"]) - else: - missing.append(dict(req)) + try: + present, missing, warnings = _match_local_models(required, {}) + except TemplateCheckError as e: + renderer.error(code=e.code, message=e.message, hint=e.hint, details=e.details) + raise typer.Exit(code=1) from e # 6. Custom nodes are report-only in v1 (surfaced verbatim, not verified). custom_nodes_required = list(match.get("requires_custom_nodes") or []) diff --git a/comfy_cli/error_codes.py b/comfy_cli/error_codes.py index 0756b69b..bbfd8d91 100644 --- a/comfy_cli/error_codes.py +++ b/comfy_cli/error_codes.py @@ -355,6 +355,12 @@ class ErrorCode: "of `/` and `\\`; for `model download`, choose the destination directory with " "`--relative-path` instead", ), + ErrorCode( + "model_listing_too_large", + "A local model folder listing was over the response size cap, so `templates check` or " + "`knowledge pick --check-local` could not check which model files are installed.", + "check that the server on this host:port is ComfyUI", + ), ErrorCode( "folder_not_found", "Cloud or local server returned 404 for the requested model folder.", diff --git a/comfy_cli/knowledge.py b/comfy_cli/knowledge.py index 950c9bdc..8f9ee7a5 100644 --- a/comfy_cli/knowledge.py +++ b/comfy_cli/knowledge.py @@ -59,6 +59,10 @@ MAX_VERSION_CHARS = 64 UNAVAILABLE_LOCALLY = "the templates or nodes this row resolves to are absent from this install" +# `knowledge pick --check-local` reasons, kept short: each flagged pick repeats one. +UNAVAILABLE_MISSING_MODELS = "model files are missing locally" +UNAVAILABLE_TEMPLATE_NOT_FOUND = "template is not in the gallery" +UNAVAILABLE_MODEL_FOLDER_NOT_FOUND = "model folder not found locally" # Grammatical filler dropped before subset matching. Deliberately generic English: # naming a capability, model or gallery tag here would be a second copy of the diff --git a/comfy_cli/schemas/knowledge.json b/comfy_cli/schemas/knowledge.json index ae9875a3..bc089938 100644 --- a/comfy_cli/schemas/knowledge.json +++ b/comfy_cli/schemas/knowledge.json @@ -56,9 +56,14 @@ "status": { "type": ["string", "null"], "description": "Copied from the pick's model row when that row exists." }, "superseded_by": { "type": ["string", "null"], "description": "From the deprecations list or the pick's model row." }, "best_for": { "type": "array", "items": { "type": "string" }, "maxItems": 1, "description": "Head of the model row's best_for list, present only when the row has one. The caveat is per capability; this is the row's own routing opinion." }, - "fits": { "type": "object", "description": "The model row's fits block (vram_gb per variant, a credit rate, max_refs, source), present only when the row carries one." } + "fits": { "type": "object", "description": "The model row's fits block (vram_gb per variant, a credit rate, max_refs, source), present only when the row carries one." }, + "available_locally": { "type": "boolean", "const": false, "description": "Present only under pick --check-local, on an oss pick the local ComfyUI cannot run." }, + "unavailable_reason": { "type": "string", "description": "Why available_locally is false (pick --check-local): model files are missing, their model folder is not on the local server, or the template is not in a gallery index confirmed fresh." }, + "missing_models": { "type": "integer", "minimum": 1, "description": "How many of the pick template's model files the local ComfyUI lacks (pick --check-local). `comfy templates check