Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions comfy_cli/command/custom_nodes/bisect_custom_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,22 @@ def __str__(self):
{active_list}"""


def parse_cm_output(cm_output: str, pinned_nodes: set[str] | None = None) -> list[str]:
"""Parse cm_cli simple-show output into a list of node names.

cm_cli simple-show always formats node entries as ``name@version``
(see ComfyUI-Manager cm_cli show_list). We whitelist on the ``@``
separator so any progress/status lines are ignored regardless of
their prefix.
"""
pinned = pinned_nodes or set()
return [
stripped
for line in cm_output.strip().split("\n")
if (stripped := line.strip()) and "@" in stripped and stripped not in pinned
]
Comment on lines +125 to +138
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Harden parsing by whitelisting node-shaped lines (name@version) to avoid future ghost entries.

Nice fix for FETCH leaks, but this is still blacklist-based. If cm_cli emits other status lines later, phantom nodes can sneak back in. A tiny whitelist keeps this imp from coming back to rhyme.

♻️ Proposed parser hardening
 def parse_cm_output(cm_output: str, pinned_nodes: set[str] | None = None) -> list[str]:
@@
-    return [
-        stripped
-        for line in cm_output.strip().split("\n")
-        if (stripped := line.strip()) and not stripped.startswith("FETCH") and stripped not in pinned
-    ]
+    return [
+        stripped
+        for raw in cm_output.splitlines()
+        if (stripped := raw.strip())
+        and "@" in stripped
+        and not stripped.startswith("FETCH")
+        and stripped not in pinned
+    ]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@comfy_cli/command/custom_nodes/bisect_custom_nodes.py` around lines 125 -
136, The parser parse_cm_output currently filters by blacklist but should
strictly whitelist node-shaped lines to avoid future ghost entries; update
parse_cm_output to only accept lines matching a node pattern (e.g.,
"name@version") using a regex like r'^[^@\s]+@[^@\s]+$' (or equivalent) and then
apply the existing pinned_nodes exclusion, so only regex-matching lines that are
not in pinned are returned; keep trimming and handle None pinned_nodes as
before.



@bisect_app.command(
help="Start a new bisect session with optionally pinned nodes to always enable, and optional ComfyUI launch args."
+ "?[--pinned-nodes PINNED_NODES]"
Expand All @@ -145,11 +161,7 @@ def start(
typer.echo("Failed to fetch the list of nodes.")
raise typer.Exit()

nodes_list = [
line.strip()
for line in cm_output.strip().split("\n")
if not line.startswith("FETCH DATA") and line.strip() not in pinned_nodes
]
nodes_list = parse_cm_output(cm_output, pinned_nodes)
state = BisectState(
status="running",
all=nodes_list,
Expand Down
142 changes: 142 additions & 0 deletions tests/comfy_cli/command/test_bisect_parse.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
from comfy_cli.command.custom_nodes.bisect_custom_nodes import parse_cm_output

CM_OUTPUT_REAL = """\
FETCH ComfyRegistry Data: 5/85
FETCH ComfyRegistry Data: 10/85
FETCH ComfyRegistry Data: 15/85
FETCH ComfyRegistry Data: 20/85
FETCH ComfyRegistry Data: 25/85
FETCH ComfyRegistry Data: 30/85
FETCH ComfyRegistry Data: 35/85
FETCH ComfyRegistry Data: 40/85
FETCH ComfyRegistry Data: 45/85
FETCH ComfyRegistry Data: 50/85
FETCH ComfyRegistry Data: 55/85
FETCH ComfyRegistry Data: 60/85
FETCH ComfyRegistry Data: 65/85
FETCH ComfyRegistry Data: 70/85
FETCH ComfyRegistry Data: 75/85
FETCH ComfyRegistry Data: 80/85
FETCH ComfyRegistry Data: 85/85
FETCH ComfyRegistry Data [DONE]
FETCH DATA from: https://raw.githubusercontent.com/ltdrdata/ComfyUI-Manager/main/custom-node-list.json [DONE]
FETCH DATA from: https://raw.githubusercontent.com/ltdrdata/ComfyUI-Manager/main/custom-node-list.json [DONE]
A3D ComfyUI Integration@1.0.2
ComfyUI_ACE-Step@1.1.2
Bjornulf_custom_nodes@1.1.1
cg-use-everywhere@6.1.0
ComfyUI-0246@1.1.3
ComfyUI ArtVenture@nightly
comfyui-auto-nodes-layout@0.0.1
ComfyUI-ConDelta@nightly
ComfyUI-Custom-Scripts@nightly
ComfyUI-DiaTTS@0.3.0
ComfyUI-Easy-Use@1.3.0
ComfyUI F5-TTS@nightly
ComfyUI-Florence2@1.0.3
ComfyUI@nightly
ComfyUI-GGUF@nightly
ComfyUI-Image-Filters@nightly
ComfyUI-KJNodes@nightly
comfyui-lmstudio-image-to-text-node@1.1.14
ComfyUI-LogicUtils@1.7.2
ComfyUI-LTXVideo@nightly
ComfyUI-Manager@nightly
ComfyUI-MMAudio@1.0.2
ComfyUI-mxToolkit@0.9.92
ComfyUI-VideoHelperSuite@1.6.1
ComfyUI Web Viewer@1.0.32
comfyui_controlnet_aux@1.0.7
ComfyUI_IPAdapter_plus@2.0.0
Prompt Stash@1.2.0
efficiency-nodes-comfyui@1.0.6
gguf@2.1.0
comfyui_HiDream-Sampler@1.0.0
LF Nodes@0.7.0
lora-info@1.0.2
Masquerade Nodes@nightly
rgthree-comfy@nightly
ComfyUI-TeaCache@1.5.1
WAS Node Suite@1.0.2
ComfyUI-ultimate-openpose-editor@nightly
ComfyUI-Dia@unknown
ComfyUI-Orpheus@unknown
"""

EXPECTED_NODES = [
"A3D ComfyUI Integration@1.0.2",
"ComfyUI_ACE-Step@1.1.2",
"Bjornulf_custom_nodes@1.1.1",
"cg-use-everywhere@6.1.0",
"ComfyUI-0246@1.1.3",
"ComfyUI ArtVenture@nightly",
"comfyui-auto-nodes-layout@0.0.1",
"ComfyUI-ConDelta@nightly",
"ComfyUI-Custom-Scripts@nightly",
"ComfyUI-DiaTTS@0.3.0",
"ComfyUI-Easy-Use@1.3.0",
"ComfyUI F5-TTS@nightly",
"ComfyUI-Florence2@1.0.3",
"ComfyUI@nightly",
"ComfyUI-GGUF@nightly",
"ComfyUI-Image-Filters@nightly",
"ComfyUI-KJNodes@nightly",
"comfyui-lmstudio-image-to-text-node@1.1.14",
"ComfyUI-LogicUtils@1.7.2",
"ComfyUI-LTXVideo@nightly",
"ComfyUI-Manager@nightly",
"ComfyUI-MMAudio@1.0.2",
"ComfyUI-mxToolkit@0.9.92",
"ComfyUI-VideoHelperSuite@1.6.1",
"ComfyUI Web Viewer@1.0.32",
"comfyui_controlnet_aux@1.0.7",
"ComfyUI_IPAdapter_plus@2.0.0",
"Prompt Stash@1.2.0",
"efficiency-nodes-comfyui@1.0.6",
"gguf@2.1.0",
"comfyui_HiDream-Sampler@1.0.0",
"LF Nodes@0.7.0",
"lora-info@1.0.2",
"Masquerade Nodes@nightly",
"rgthree-comfy@nightly",
"ComfyUI-TeaCache@1.5.1",
"WAS Node Suite@1.0.2",
"ComfyUI-ultimate-openpose-editor@nightly",
"ComfyUI-Dia@unknown",
"ComfyUI-Orpheus@unknown",
]


class TestParseCmOutput:
def test_real_output_filters_fetch_lines(self):
result = parse_cm_output(CM_OUTPUT_REAL)
assert result == EXPECTED_NODES
assert len(result) == 40

def test_no_fetch_lines_in_result(self):
result = parse_cm_output(CM_OUTPUT_REAL)
for node in result:
assert not node.startswith("FETCH"), f"FETCH line leaked: {node}"

def test_pinned_nodes_excluded(self):
pinned = {"ComfyUI-Manager@nightly", "ComfyUI@nightly"}
result = parse_cm_output(CM_OUTPUT_REAL, pinned)
assert "ComfyUI-Manager@nightly" not in result
assert "ComfyUI@nightly" not in result
assert len(result) == 38

def test_empty_output(self):
assert parse_cm_output("") == []
assert parse_cm_output(" \n \n ") == []

def test_only_fetch_lines(self):
output = "FETCH ComfyRegistry Data: 5/85\nFETCH DATA from: foo [DONE]\n"
assert parse_cm_output(output) == []

def test_no_fetch_lines(self):
output = "NodeA@1.0\nNodeB@nightly\n"
assert parse_cm_output(output) == ["NodeA@1.0", "NodeB@nightly"]

def test_arbitrary_status_lines_filtered(self):
output = "some random status line\nINFO: loading\nNodeA@1.0\nDone.\n"
assert parse_cm_output(output) == ["NodeA@1.0"]
Comment on lines +140 to +142
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add a regression case for status lines that contain @.

Nice coverage overall, but this suite should also assert that non-node lines with @ are rejected (e.g., INFO: user@host) so the filter doesn’t get tricked by imposter lines.

🧪 Suggested test addition
 class TestParseCmOutput:
@@
     def test_arbitrary_status_lines_filtered(self):
         output = "some random status line\nINFO: loading\nNodeA@1.0\nDone.\n"
         assert parse_cm_output(output) == ["NodeA@1.0"]
+
+    def test_status_lines_with_at_symbol_filtered(self):
+        output = "INFO: user@host\nNodeA@1.0\n"
+        assert parse_cm_output(output) == ["NodeA@1.0"]
📝 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.

Suggested change
def test_arbitrary_status_lines_filtered(self):
output = "some random status line\nINFO: loading\nNodeA@1.0\nDone.\n"
assert parse_cm_output(output) == ["NodeA@1.0"]
def test_arbitrary_status_lines_filtered(self):
output = "some random status line\nINFO: loading\nNodeA@1.0\nDone.\n"
assert parse_cm_output(output) == ["NodeA@1.0"]
def test_status_lines_with_at_symbol_filtered(self):
output = "INFO: user@host\nNodeA@1.0\n"
assert parse_cm_output(output) == ["NodeA@1.0"]
🧰 Tools
🪛 Pylint (4.0.5)

[convention] 140-140: Missing function or method docstring

(C0116)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/comfy_cli/command/test_bisect_parse.py` around lines 140 - 142, Add a
regression test to ensure parse_cm_output ignores status lines that contain "@"
but are not node entries: update or add to test_arbitrary_status_lines_filtered
to include a line like "INFO: user@host" (or similar imposter lines) in the
output string and assert that parse_cm_output(output) still returns only the
legitimate node token(s) (e.g., ["NodeA@1.0"]). This ensures parse_cm_output's
filtering logic (used in test_arbitrary_status_lines_filtered) rejects non-node
lines containing "@" and prevents false positives.

Loading