Skip to content

Commit 364bd13

Browse files
committed
feat(registry): resolve PEP 621 dynamic version from [tool.comfy.version].path
Custom nodes using `dynamic = ["version"]` in pyproject.toml were silently getting an empty version in registry publishes. Add a comfy-native `[tool.comfy.version]` section with a `path` key that points at a source file containing `__version__` or `VERSION`; parse it with an anchored regex (no Python execution, scanning-friendly per earlier discussion with @ltdrdata, @Lex-DRL). - `config_parser.py`: add `_VERSION_RE`, `_resolve_dynamic_version`, `_extract_version`. Resolution runs only when `dynamic` contains `"version"` and `project.version` is absent; all failure modes (no config, absolute path, path escaping project dir, missing file, no regex match) warn and fall back to empty so scanning contexts stay graceful. Type-check `project.version` up front — running BEFORE the truthy gate — so both truthy and falsy non-strings (`1`, `0`, `0.0`, `false`, `["1","2"]`, `[]`, `{path="_v.py"}`, `{}`) are rejected with a named warning instead of silently falling through or being coerced to their Python repr and POSTed to the registry. A whitespace-only static `version = " "` also warns (mirrors the dynamic path's `__version__ = " "` warning) so the user sees the root cause at parse time, not only at publish time. The quoted-value regex forces every backslash through `\\.` (exclude `\` from the non-escape class) to prevent catastrophic backtracking on malformed Python sources. Open pyproject.toml with `encoding="utf-8-sig"` (strips Windows editor BOMs that otherwise yield a cryptic `Empty key at line 1 col 0`) and add `UnicodeDecodeError` to the parse except tuple so non-UTF-8 bytes degrade to the same friendly error as other unreadable files. Decode captured string literals via `ast.literal_eval` instead of a naive one-char-strip: the old approach silently produced wrong values because it flattened every backslash-escape pair to its second character, so a newline escape in the literal became the letter `n` and a unicode escape like `\uXXXX` became the literal `uXXXX` — both POSTed to the registry as-is. Now all Python escape semantics (`\n`, `\t`, `\uXXXX`, `\"`, `\\`) decode correctly; malformed input the regex admits but `ast` rejects falls back to the raw capture instead of crashing. Detect the triple-quoted case (`__version__ = """1.2.3"""`) when the regex matches an empty `""` followed by the same quote type, and emit a targeted hint instead of the misleading generic "empty or whitespace-only" warning. Warn about unknown keys in `[tool.comfy.version]` so typos like `file = "..."` or `attr = "..."` (common in setuptools/hatch) surface by name instead of silently yielding "path is not set". - `command.py`: fail fast in `validate_node_for_publishing()` when the resolved version is empty, so we never POST an empty `version` to the registry. - tests: 34 new config_parser cases using real tmp_path files (including ReDoS regression guard; typed-version rejection covering both truthy and falsy non-strings; whitespace-only static warning; empty-string-vs-whitespace distinction; triple-quoted detection for double and single quotes plus a negative-control for plain `__version__ = ""`; unknown-keys warnings including a resolution-still-succeeds variant; Python-escape decoding for `\n`, `\t`, `\uXXXX`), plus a publish-layer guard test. Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
1 parent 96f2ff9 commit 364bd13

4 files changed

Lines changed: 1220 additions & 5 deletions

File tree

comfy_cli/command/custom_nodes/command.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,18 @@ def validate_node_for_publishing():
975975
# Perform some validation logic here
976976
typer.echo("Validating node configuration...")
977977
config = extract_node_configuration()
978+
if config is None:
979+
raise typer.Exit(code=1)
980+
981+
if not config.project.version or not config.project.version.strip():
982+
# `.strip()` guards against a whitespace-only static `version = " "`.
983+
# Escape `[` chars so rich doesn't parse `[tool.comfy.version]` and
984+
# `["version"]` as markup tags; `]` doesn't need escaping.
985+
print(
986+
"[red]Error: project version is empty. Set `project.version` in pyproject.toml, "
987+
r'or configure `\[tool.comfy.version].path` if using `dynamic = \["version"]`.[/red]'
988+
)
989+
raise typer.Exit(code=1)
978990

979991
# Run security checks first
980992
typer.echo("Running security checks...")

comfy_cli/registry/config_parser.py

Lines changed: 318 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import ast
2+
import datetime
13
import os
4+
import pathlib
25
import re
36
import subprocess
47
from urllib.parse import urlparse, urlunparse
@@ -22,6 +25,91 @@
2225
# direct-URL hashes (`#sha256=`) survive.
2326
_inline_comment_re: re.Pattern[str] = re.compile(r"(^|\s+)#.*$")
2427

28+
# For `dynamic = ["version"]`: match a top-level `__version__` or `VERSION` assignment in a source file. Anchored
29+
# to start-of-line (MULTILINE) so single-line comments are skipped. Horizontal whitespace only — no cross-line
30+
# matching. Supports an optional PEP 526 type annotation. Straight quotes only. The `\\.` alternation in the
31+
# quoted-value portion accepts backslash-escape sequences (e.g., `\"` for a literal double quote);
32+
# `_decode_string_literal` below feeds the captured contents through `ast.literal_eval` to recover proper
33+
# Python-semantic values for ALL escape forms (`\\n`, `\\t`, `\\uXXXX`, ...). `\` is excluded from the
34+
# non-escape character class (`[^"\\\n]` / `[^'\\\n]`) so every backslash is forced through `\\.`; the earlier
35+
# `[^"\n]` form had overlapping alternatives that caused catastrophic backtracking on malformed Python
36+
# sources (unclosed literals with many backslashes).
37+
#
38+
# Recommended user layout: a dedicated `_version.py` / `__version__.py` (NOT the package's `__init__.py`), so
39+
# nothing in the file — module docstrings, assignments referenced in text, etc. — can collide with this regex.
40+
# This matches hatch/setuptools convention for dynamic-version source files.
41+
_VERSION_RE: re.Pattern[str] = re.compile(
42+
r"""^(?P<name>__version__|VERSION)
43+
(?:[\t ]*:[\t ]*[^=\n]+)?
44+
[\t ]*=[\t ]*
45+
(?:"(?P<dq>(?:\\.|[^"\\\n])*)"|'(?P<sq>(?:\\.|[^'\\\n])*)')
46+
""",
47+
re.MULTILINE | re.VERBOSE,
48+
)
49+
50+
51+
def _decode_string_literal(raw: str, quote: str) -> str:
52+
"""Decode the captured contents of a Python single-line string literal.
53+
54+
The regex matched a `"..."` or `'...'` literal; wrapping `raw` in its
55+
outer quotes again and handing it to `ast.literal_eval` gives full
56+
Python string-escape semantics (`\\n` → newline, `\\uXXXX` → codepoint,
57+
`\\"` → quote, `\\\\` → backslash). The earlier one-char-strip approach
58+
flattened e.g. `\\n` to the literal letter `n`, silently producing a
59+
wrong version string.
60+
61+
On parse failure (exotic/malformed input the regex admitted but ast
62+
rejects, like `\\z` on strict Python versions), fall back to the raw
63+
captured contents so we degrade to the prior wrong-but-not-crashing
64+
behavior rather than erroring out.
65+
"""
66+
try:
67+
parsed = ast.literal_eval(f"{quote}{raw}{quote}")
68+
except (SyntaxError, ValueError):
69+
return raw
70+
return parsed if isinstance(parsed, str) else raw
71+
72+
73+
def _friendly_type_name(value: object) -> str:
74+
"""Map a (possibly tomlkit-wrapped) value to a Python-builtin-style name.
75+
76+
`type(x).__name__` returns tomlkit's class name (e.g., 'Integer', 'Float',
77+
'DateTime') for parsed TOML values, which confuses users who wrote plain
78+
TOML. Since tomlkit's wrapper classes inherit from the standard Python
79+
types, we can detect the semantic type via isinstance.
80+
"""
81+
if isinstance(value, bool): # must precede int — bool is an int subclass
82+
return "bool"
83+
if isinstance(value, int):
84+
return "int"
85+
if isinstance(value, float):
86+
return "float"
87+
if isinstance(value, str):
88+
return "str"
89+
if isinstance(value, datetime.datetime):
90+
return "datetime"
91+
if isinstance(value, datetime.date):
92+
return "date"
93+
if isinstance(value, datetime.time):
94+
return "time"
95+
if isinstance(value, (list, tuple)):
96+
return "array"
97+
if isinstance(value, dict):
98+
return "table"
99+
return type(value).__name__
100+
101+
102+
def _friendly_value(value: object) -> str:
103+
"""Render a value for inclusion in a user-facing warning.
104+
105+
Uses str() for datetime-ish values (ISO-format is more familiar than
106+
`DateTime(2024, 1, 1, 0, 0)` constructor style) and repr() otherwise so
107+
strings show their quotes and collections show brackets.
108+
"""
109+
if isinstance(value, (datetime.datetime, datetime.date, datetime.time)):
110+
return str(value)
111+
return repr(value)
112+
25113

26114
def create_comfynode_config():
27115
# Create the initial structure of the TOML document
@@ -281,19 +369,241 @@ def initialize_project_config():
281369
raise OSError("Failed to write 'pyproject.toml'") from e
282370

283371

372+
def _resolve_dynamic_version(pyproject_dir: pathlib.Path, rel_path: str) -> str:
373+
"""Read a version from a source file referenced by `[tool.comfy.version].path`.
374+
375+
No Python execution — just text I/O and a regex, matching the contract
376+
agreed in issue #294. Returns empty string on any failure and emits a
377+
user-visible warning so scanning contexts degrade gracefully.
378+
"""
379+
# Reject paths that are absolute under either POSIX or Windows rules —
380+
# `pathlib.Path.is_absolute()` alone is OS-specific (e.g., `/etc/foo` is
381+
# not considered absolute on Windows because it has no drive), and we
382+
# want identical rejection behavior regardless of the host OS.
383+
if pathlib.PurePosixPath(rel_path).is_absolute() or pathlib.PureWindowsPath(rel_path).is_absolute():
384+
typer.echo(
385+
f"Warning: `[tool.comfy.version].path` must be relative to pyproject.toml "
386+
f"(got `{rel_path}`). No version will be populated."
387+
)
388+
return ""
389+
path_obj = pathlib.Path(rel_path)
390+
391+
pyproject_dir = pyproject_dir.resolve()
392+
resolved = (pyproject_dir / path_obj).resolve()
393+
try:
394+
resolved.relative_to(pyproject_dir)
395+
except ValueError:
396+
typer.echo(
397+
f"Warning: `[tool.comfy.version].path` must point inside the project directory "
398+
f"(got `{rel_path}`). No version will be populated."
399+
)
400+
return ""
401+
402+
try:
403+
# `utf-8-sig` transparently strips a leading BOM — some Windows editors
404+
# add one, and it would defeat the `^__version__` anchor otherwise.
405+
text = resolved.read_text(encoding="utf-8-sig")
406+
except (OSError, UnicodeDecodeError) as e:
407+
typer.echo(f"Warning: could not read version file `{rel_path}`: {e}. No version will be populated.")
408+
return ""
409+
410+
match = _VERSION_RE.search(text)
411+
if not match:
412+
typer.echo(
413+
f"Warning: could not find `__version__` or `VERSION` in `{rel_path}`. "
414+
f'The version file must contain a line like `__version__ = "1.2.3"`. '
415+
f"No version will be populated."
416+
)
417+
return ""
418+
419+
# Exactly one of `dq` / `sq` was consumed by the regex. Use `is not None`
420+
# instead of truthy-check because an empty capture (`""` or `''`) is a
421+
# valid match the regex now accepts — and we want to warn about that
422+
# case explicitly rather than crash on `None`. Tracking the quote type
423+
# lets us (a) feed the correct outer quote into `_decode_string_literal`
424+
# and (b) detect the triple-quoted case below.
425+
if match.group("dq") is not None:
426+
raw, quote = match.group("dq"), '"'
427+
else:
428+
raw, quote = match.group("sq"), "'"
429+
430+
# `__version__ = """1.2.3"""` parses as an empty `""` literal followed by
431+
# leftover `"1.2.3"""`, which would otherwise surface as the generic
432+
# "empty or whitespace-only" warning and leave the user staring at a
433+
# clearly-non-empty literal. If the capture is empty AND the byte right
434+
# after the match is the same quote type, it's almost certainly a
435+
# triple-quoted literal — emit a targeted hint instead.
436+
if not raw and match.end() < len(text) and text[match.end()] == quote:
437+
typer.echo(
438+
f"Warning: `{match.group('name')}` in `{rel_path}` uses a triple-quoted string literal, "
439+
f'which is not supported. Use a single-line assignment like `{match.group("name")} = "1.2.3"`. '
440+
f"No version will be populated."
441+
)
442+
return ""
443+
444+
version = _decode_string_literal(raw, quote).strip()
445+
if not version:
446+
typer.echo(
447+
f"Warning: `{match.group('name')}` in `{rel_path}` is empty or whitespace-only. "
448+
f"No version will be populated."
449+
)
450+
return ""
451+
return version
452+
453+
454+
def _parse_dynamic_fields(project_data) -> list[str]:
455+
"""Return the `project.dynamic` field as a list of strings.
456+
457+
Warns and returns `[]` if `dynamic` is present but has the wrong shape
458+
(e.g. a scalar string — a common PEP 621 misconfiguration).
459+
"""
460+
dynamic_raw = project_data.get("dynamic", [])
461+
# tomlkit.Array inherits from list, so valid arrays (including empty `[]`)
462+
# pass through. Everything else is a misconfiguration.
463+
if not isinstance(dynamic_raw, (list, tuple)):
464+
typer.echo(
465+
f"Warning: `project.dynamic` must be an array of strings "
466+
f"(got {_friendly_type_name(dynamic_raw)} `{_friendly_value(dynamic_raw)}`). "
467+
f'Use `dynamic = ["version"]` instead. '
468+
f"No dynamic fields will be honored."
469+
)
470+
return []
471+
return [str(d) for d in dynamic_raw]
472+
473+
474+
def _extract_version(project_data, comfy_data, pyproject_dir: pathlib.Path) -> str:
475+
"""Return the project version, honoring PEP 621 `dynamic = ["version"]`.
476+
477+
- Static `project.version` wins if present.
478+
- If absent and `"version"` is in `project.dynamic`, resolve via
479+
`[tool.comfy.version].path` (text-read + regex, no Python execution).
480+
- Otherwise return empty (existing behavior).
481+
"""
482+
static_version = project_data.get("version", "")
483+
dynamic_fields = _parse_dynamic_fields(project_data)
484+
# Type-check runs BEFORE the truthy check so falsy non-strings (`version = 0`,
485+
# `version = 0.0`, `version = false`, `version = []`, `version = {}`) produce
486+
# the same named "must be a string" warning as truthy non-strings (`version = 1`,
487+
# `version = ["1","2"]`, `version = { path = "_v.py" }`). With the order reversed,
488+
# they would silently fall through to the dynamic branch and the user would
489+
# only see the downstream "project version is empty" error at publish time.
490+
# `static_version != ""` skips the default (key absent) without coercing types.
491+
if static_version != "" and not isinstance(static_version, str):
492+
typer.echo(
493+
f"Warning: `project.version` must be a string "
494+
f"(got {_friendly_type_name(static_version)} `{_friendly_value(static_version)}`). "
495+
f"No version will be populated."
496+
)
497+
return ""
498+
if static_version:
499+
# `static_version` is known to be a str from here.
500+
if "version" in dynamic_fields:
501+
# PEP 621 §Dynamic says a field listed in `dynamic` must NOT also
502+
# appear in `[project]`. Honor the static value (the user wrote it
503+
# explicitly) but surface the conflict.
504+
typer.echo(
505+
'Warning: `project.version` is set but `"version"` also appears in `project.dynamic`. '
506+
"Per PEP 621 these are mutually exclusive; using the static value."
507+
)
508+
stripped = static_version.strip()
509+
if not stripped:
510+
# A whitespace-only static `version = " "` must warn here, matching
511+
# the dynamic path's `__version__ = " "` warning. Otherwise only the
512+
# downstream "project version is empty" error would surface, with no
513+
# hint that the root cause is a whitespace-only literal.
514+
typer.echo("Warning: `project.version` is empty or whitespace-only. No version will be populated.")
515+
return ""
516+
# Strip so a padded static `version = " 1.0.0 "` does not get POSTed
517+
# with surrounding whitespace. Matches the stripped semantics applied to
518+
# dynamically-resolved versions.
519+
return stripped
520+
521+
if "version" not in dynamic_fields:
522+
return ""
523+
524+
version_cfg = comfy_data.get("version")
525+
if version_cfg is None:
526+
typer.echo(
527+
'Warning: `dynamic = ["version"]` declared but `[tool.comfy.version].path` is not set. '
528+
"See https://docs.comfy.org/registry/specifications for dynamic-version setup. "
529+
"No version will be populated."
530+
)
531+
return ""
532+
if not isinstance(version_cfg, dict):
533+
# A non-table value under `[tool.comfy].version` — the user likely
534+
# wrote `version = "x"` scalar (or any other type) instead of a nested
535+
# table. Name the actual type, not "scalar value", so arrays/tables
536+
# are described accurately too.
537+
typer.echo(
538+
f"Warning: `[tool.comfy].version` must be a table with a `path` key "
539+
f"(got {_friendly_type_name(version_cfg)} `{_friendly_value(version_cfg)}`). "
540+
f'Use `[tool.comfy.version]` with `path = "..."` instead. '
541+
f"No version will be populated."
542+
)
543+
return ""
544+
# Flag unknown keys so a typo like `file = "..."` / `attr = "..."` (common
545+
# spellings in setuptools/hatch) doesn't silently surface as a misleading
546+
# "path is not set" warning. Emitted independently of path resolution so
547+
# the user sees BOTH the typo and any downstream path-missing condition.
548+
unknown_keys = sorted(k for k in version_cfg if k != "path")
549+
if unknown_keys:
550+
keys_str = ", ".join(f"`{k}`" for k in unknown_keys)
551+
typer.echo(f"Warning: `[tool.comfy.version]` has unknown key(s): {keys_str}. Only `path` is supported.")
552+
# Order matters: check type BEFORE falsy-ness so that `path = 0` / `false`
553+
# / `[]` / `{}` produce a type warning, not a misleading "not set" warning.
554+
path_value = version_cfg.get("path")
555+
if path_value is not None and not isinstance(path_value, str):
556+
typer.echo(
557+
f"Warning: `[tool.comfy.version].path` must be a string "
558+
f"(got {_friendly_type_name(path_value)} `{_friendly_value(path_value)}`). "
559+
f"No version will be populated."
560+
)
561+
return ""
562+
if not path_value:
563+
typer.echo(
564+
"Warning: `[tool.comfy.version].path` is not set. "
565+
"See https://docs.comfy.org/registry/specifications for dynamic-version setup. "
566+
"No version will be populated."
567+
)
568+
return ""
569+
570+
return _resolve_dynamic_version(pyproject_dir, path_value)
571+
572+
284573
def extract_node_configuration(
285574
path: str = os.path.join(os.getcwd(), "pyproject.toml"),
286575
) -> PyProjectConfig | None:
287576
if not os.path.isfile(path):
288577
ui.display_error_message("No pyproject.toml file found in the current directory.")
289578
return None
290579

291-
with open(path) as file:
292-
data = tomlkit.load(file)
580+
try:
581+
# `utf-8-sig` strips a leading BOM if present — Windows editors sometimes
582+
# write one, and tomlkit would otherwise report `Empty key at line 1 col 0`.
583+
# `UnicodeDecodeError` must be in the except tuple: it is a `ValueError`,
584+
# not an `OSError`, and would otherwise escape and crash the caller.
585+
with open(path, encoding="utf-8-sig") as file:
586+
data = tomlkit.load(file)
587+
except (OSError, UnicodeDecodeError, tomlkit.exceptions.TOMLKitError) as e:
588+
ui.display_error_message(f"Could not parse `{path}`: {e}")
589+
return None
293590

294591
project_data = data.get("project", {})
592+
if not isinstance(project_data, dict):
593+
# Degenerate TOML like `project = "hello"` at the root. Keep scanning
594+
# contexts alive by treating it as "no project metadata".
595+
typer.echo(
596+
f"Warning: `project` in pyproject.toml must be a table "
597+
f"(got {_friendly_type_name(project_data)}). Using defaults."
598+
)
599+
project_data = {}
295600
urls_data = project_data.get("urls", {})
296-
comfy_data = data.get("tool", {}).get("comfy", {})
601+
if not isinstance(urls_data, dict):
602+
urls_data = {}
603+
tool_data = data.get("tool", {})
604+
comfy_data = tool_data.get("comfy", {}) if isinstance(tool_data, dict) else {}
605+
if not isinstance(comfy_data, dict):
606+
comfy_data = {}
297607

298608
dependencies = project_data.get("dependencies", [])
299609
supported_comfyui_frontend_version = ""
@@ -307,7 +617,7 @@ def extract_node_configuration(
307617
dep for dep in dependencies if not (isinstance(dep, str) and dep.startswith("comfyui-frontend-package"))
308618
]
309619

310-
supported_comfyui_version = data.get("tool", {}).get("comfy", {}).get("requires-comfyui", "")
620+
supported_comfyui_version = comfy_data.get("requires-comfyui", "")
311621

312622
classifiers = project_data.get("classifiers", [])
313623
supported_os = validate_and_extract_os_classifiers(classifiers)
@@ -334,10 +644,13 @@ def extract_node_configuration(
334644
'Warning: License should be in one of these two formats: license = {file = "LICENSE"} OR license = {text = "MIT License"}. Please check the documentation: https://docs.comfy.org/registry/specifications.'
335645
)
336646

647+
pyproject_dir = pathlib.Path(path).parent
648+
version = _extract_version(project_data, comfy_data, pyproject_dir)
649+
337650
project = ProjectConfig(
338651
name=project_data.get("name", ""),
339652
description=project_data.get("description", ""),
340-
version=project_data.get("version", ""),
653+
version=version,
341654
requires_python=project_data.get("requires-python", ""),
342655
dependencies=dependencies,
343656
license=license,

0 commit comments

Comments
 (0)