From 50fd0a4381ccc7a46466bf2b56208faf5e2aeb18 Mon Sep 17 00:00:00 2001 From: kishore Date: Thu, 27 Aug 2026 19:29:10 -0700 Subject: [PATCH 1/6] fix(cql): name every frontend widget slot in the widget order (BE-10283) The widget catalog, set-widget indexing and the UI->API converter only named schema-declared widget inputs plus control_after_generate. The frontend serializes more slots: - Comfy.UploadImage / Comfy.UploadAudio inject a required `upload` input on every media loader (LoadImage, LoadImageMask, LoadVideo, LoadAudio, ...). Older frontends wrote its value ("image"); current ones mark it serialize:false. It lands after every declared input, optional ones included (getOrderedInputSpecs appends unlisted inputs last). - Comfy.AudioWidget injects `audioUI` on the audio load/save/preview family; Comfy.Preview3D / Comfy.SaveGLB inject a PREVIEW_3D `image`. - Server-declared DOM widget inputs under uppercase custom types (Load3D.image is LOAD_3D, LoadAudioUI.audioUI is AUDIO_UI) were read as links. - Inputs whose `widgetType` option overrides a link-shaped socket type (LTXVEmptyLatentAudio.frame_rate, the "Basic data handling" math nodes) were read as links; the frontend picks the widget via inputSpec.widgetType ?? inputSpec.type. The cloud doc host builds the CRDT document's name-keyed widget map from this order and refuses a widgets_values longer than it, so every workflow with a Load Image node failed to mint (`createNodeMap(LoadImage): widgets_values has 2 entries but widget_order names only 1`) and silently fell back to the v0 path. The converter and set-widget also read every value after such a slot one position off. frontend_extra_widget_names() encodes the injection rules; the DOM widget types and widgetType are honored by _is_link and the converter's _is_widget_input. widget_defaults emits "" for a non-trailing DOM slot (Load3D.image sits before width) and nothing for the trailing markers. Real-catalog diff: 60 classes change, all in the families above. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 16 + comfy_cli/cql/engine.py | 103 ++++- comfy_cli/cql/widget_catalog.py | 11 + comfy_cli/schemas/widget_catalog.json | 2 +- comfy_cli/workflow_ops.py | 3 +- comfy_cli/workflow_to_api.py | 11 + .../cql/test_frontend_widget_slots.py | 363 ++++++++++++++++++ 7 files changed, 505 insertions(+), 4 deletions(-) create mode 100644 tests/comfy_cli/cql/test_frontend_widget_slots.py diff --git a/CHANGELOG.md b/CHANGELOG.md index efdde466f..592ef7c77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,22 @@ history. ## [Unreleased] +### Fixed + +- The widget order (`comfy nodes widget-catalog`, `set-widget` indexing, the + UI→API converter) now names every slot the frontend serializes: the + `upload` button frontend extensions inject on media loaders (`LoadImage`, + `LoadImageMask`, `LoadVideo`, `LoadAudio`, ...), the `audioUI` player on + the audio family, the `PREVIEW_3D` `image` on `SaveGLB`/`Preview3D`, DOM + widgets declared under an uppercase custom type (`Load3D.image`), and + inputs whose `widgetType` overrides a link-shaped socket type + (`LTXVEmptyLatentAudio.frame_rate`, the "Basic data handling" math nodes). + Before, a workflow with any of these nodes carried more `widgets_values` + than the catalog could name, so the cloud doc host refused to mint it + (`createNodeMap(LoadImage): widgets_values has 2 entries but widget_order + names only 1`) and `set-widget`/conversion read the values after such a + slot one position off. + ### Added - `comfy workflow add-node` and an `add_node` op in `comfy workflow apply` diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 567864676..381643540 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -30,6 +30,42 @@ _IMPLICIT_WIDGET_TYPES = frozenset({"STRING", "INT", "FLOAT", "NUMBER", "BOOLEAN", "COMBO"}) +# Uppercase custom types the frontend renders as a DOM widget that SERIALIZES +# into ``widgets_values`` (``ComponentWidgetImpl`` / ``DOMWidget``), so they +# occupy a positional slot exactly like an INT. Every other uppercase custom +# type is a link. Kept to types verified against saved workflows: +# ``Load3D.image`` / ``Load3DAdvanced.viewport_state`` (``LOAD_3D``), +# ``SaveGLB``/``Preview3D``'s injected ``image`` (``PREVIEW_3D``), +# ``LoadAudioUI.audioUI`` (``AUDIO_UI``). ``LOAD3D_CAMERA`` is deliberately +# absent: ``camera_info`` is ``serialize: false`` and writes no slot. +_FRONTEND_DOM_WIDGET_TYPES = frozenset( + {"LOAD_3D", "LOAD_3D_ADVANCED", "PREVIEW_3D", "AUDIO_UI", "IMAGEUPLOAD", "AUDIOUPLOAD"} +) + +# What a fresh node serializes in a DOM-widget slot. ``add_node`` must emit +# these for NON-trailing slots (``Load3D.image`` sits before ``width``), or the +# frontend reads ``width`` into the viewport slot. +_FRONTEND_DOM_WIDGET_DEFAULTS: dict[str, Any] = {"LOAD_3D": "", "LOAD_3D_ADVANCED": "", "PREVIEW_3D": ""} + +# Widget names the FRONTEND injects into a node's inputs after object_info +# (``beforeRegisterNodeDef`` in ``uploadImage.ts``/``uploadAudio.ts``/ +# ``load3d.ts``/``saveMesh.ts``). They have no schema port and are never +# ``set-widget`` targets. Listed with ``control_after_generate`` because all +# three are marker slots a name<->index consumer must be able to name. +FRONTEND_MARKER_SLOTS = frozenset({"control_after_generate", "upload", "audioUI"}) + +# ``Comfy.AudioWidget`` appends an ``audioUI`` player to exactly these classes. +_AUDIO_UI_CLASSES = frozenset( + {"LoadAudio", "SaveAudio", "PreviewAudio", "SaveAudioMP3", "SaveAudioOpus", "SaveAudioAdvanced"} +) +# ``Comfy.UploadImage`` attaches its upload button to the first required media +# COMBO carrying one of these flags (``isMediaUploadComboInput``). Audio has +# its own extension keyed on the ``audio`` input; ``file_upload`` (3D loaders) +# and ``mesh_upload`` attach nothing. +_IMAGE_UPLOAD_FLAGS = frozenset({"image_upload", "animated_image_upload", "video_upload"}) +# ``Comfy.Preview3D`` / ``Comfy.SaveGLB`` inject a ``PREVIEW_3D`` ``image`` widget. +_PREVIEW_3D_CLASSES = frozenset({"SaveGLB", "Preview3D"}) + # Work budget for ``Graph.search_paths``: the number of frontier states it will # expand before giving up and reporting ``truncated``. A full cloud catalog has # thousands of nodes, so an unreachable target must fail fast rather than walk @@ -58,6 +94,15 @@ class PortOptions: # an upload button and the declared options are the server's *installed input # files*, not an install-time enum. See ``Port.is_upload_backed``. upload: bool = False + # The ``_upload`` flag names that were set (``("image_upload",)``), + # so callers can tell WHICH frontend upload extension claims the input. + upload_flags: tuple[str, ...] = () + # ``widgetType``: the frontend renders the widget for THIS type instead of + # the declared socket type (``inputSpec.widgetType ?? inputSpec.type`` in + # litegraphService), so a ``FLOAT,INT`` or ``STRING,FILE_3D_*`` input with + # ``widgetType`` set is a widget slot even though its own type reads as a + # link. None when the schema does not set it. + widget_type: str | None = None @dataclass @@ -477,14 +522,54 @@ def _has_control_after_generate_slot(port: Port) -> bool: return port.type == "INT" and "seed" in leaf_name.lower() -def _is_link(type_id: str, is_enum: bool, force_input: bool) -> bool: +def frontend_extra_widget_names(m: Morphism) -> list[str]: + """Widget names the frontend injects into ``m``'s inputs AFTER object_info. + + ``getOrderedInputSpecs`` walks ``input_order.required``, then + ``input_order.optional``, then every input not listed there — so an + extension-injected input always serializes after every declared widget, + optional ones included. Registration order decides the order among them: + ``Comfy.AudioWidget`` (``audioUI``) before ``Comfy.UploadAudio`` / + ``Comfy.UploadImage`` (``upload``); the lazily loaded 3D extensions + (``PREVIEW_3D`` ``image``) last. A name the server already declares + (``Load3DAdvanced.viewport_state``) is never injected twice. + """ + declared = {p.name for p in m.inputs} + class_id = getattr(m, "id", "") + extras: list[str] = [] + if class_id in _AUDIO_UI_CLASSES: + extras.append("audioUI") + upload = False + for p in m.inputs: + if not p.required or p.is_link: + continue + flags = set(p.options.upload_flags) + if p.name == "audio" and "audio_upload" in flags: + upload = True + elif p.is_upload_backed and flags & _IMAGE_UPLOAD_FLAGS: + upload = True + if upload: + extras.append("upload") + if class_id in _PREVIEW_3D_CLASSES: + extras.append("image") + return [e for e in extras if e not in declared] + + +def _is_link(type_id: str, is_enum: bool, force_input: bool, widget_type: str | None = None) -> bool: """Determine if an input participates in typed wiring (link) or is inline (widget).""" if is_enum: return False + # ``widgetType`` overrides the socket type for widget selection (a + # ``FLOAT,INT`` math input with ``widgetType: "STRING"``, Preview3D's + # ``STRING,FILE_3D_*`` model_file with ``widgetType: "STRING"``). + if widget_type and not force_input: + return False # A dynamic combo is a widget port even when its options block is missing # or malformed — the frontend always renders the selector inline. if _is_dynamic_combo_type(type_id): return False + if type_id in _FRONTEND_DOM_WIDGET_TYPES and not force_input: + return False if type_id in _IMPLICIT_WIDGET_TYPES and not force_input and type_id != "*": return False return True @@ -530,6 +615,12 @@ def _parse_port_options(opts_raw: dict) -> PortOptions: force_input=bool(opts_raw.get("forceInput", False)), template=template_raw if isinstance(template_raw, dict) else None, upload=_upload_marked(opts_raw), + upload_flags=tuple( + sorted(k for k, v in opts_raw.items() if isinstance(k, str) and k.endswith("_upload") and bool(v)) + ), + widget_type=opts_raw.get("widgetType") + if isinstance(opts_raw.get("widgetType"), str) and opts_raw.get("widgetType") + else None, ) @@ -682,7 +773,7 @@ def _port_from_spec(name: str, spec: Any, required: bool) -> Port: name=name, type=type_id, required=required, - is_link=_is_link(type_id, is_enum, opts.force_input), + is_link=_is_link(type_id, is_enum, opts.force_input, opts.widget_type), enum_values=enum_values, enum_declared=enum_declared, options=opts, @@ -1200,6 +1291,7 @@ def widget_order(self, class_name: str) -> list[str]: order.append(p.name) if _has_control_after_generate_slot(p): order.append("control_after_generate") + order.extend(frontend_extra_widget_names(m)) return order def widget_order_default(self, class_name: str) -> list[str]: @@ -1225,6 +1317,7 @@ def widget_order_default(self, class_name: str) -> list[str]: order.extend(_dynamic_sub_widget_names(p.name, p.dynamic_options)) if _has_control_after_generate_slot(p): order.append("control_after_generate") + order.extend(frontend_extra_widget_names(m)) return order def widget_order_for_node(self, class_name: str, widgets_values: list[Any] | None) -> list[str]: @@ -1256,10 +1349,14 @@ def widget_defaults(self, class_name: str) -> dict[str, Any]: out[p.name] = p.options.default elif p.enum_values: out[p.name] = p.enum_values[0] + elif p.type in _FRONTEND_DOM_WIDGET_TYPES: + out[p.name] = _FRONTEND_DOM_WIDGET_DEFAULTS.get(p.type) else: out[p.name] = None if _has_control_after_generate_slot(p): out["control_after_generate"] = "fixed" + # Frontend-injected marker slots (``upload``, ``audioUI``) are trailing + # and ``serialize: false`` on current frontends: no default, no value. return out # -- Validation -- @@ -2385,6 +2482,8 @@ def emit(name: str, port: Port, owner: str | None, depth: int) -> None: if p.is_link: continue emit(p.name, p, None, 0) + for name in frontend_extra_widget_names(m): + entries.append(_WidgetEntry(name=name, port=None, owner=None)) return entries diff --git a/comfy_cli/cql/widget_catalog.py b/comfy_cli/cql/widget_catalog.py index e9c65f9cf..87466280c 100644 --- a/comfy_cli/cql/widget_catalog.py +++ b/comfy_cli/cql/widget_catalog.py @@ -16,6 +16,17 @@ seed. It occupies a real ``widgets_values`` slot and is in no ``input_order``. * ``COMFY_DYNAMICCOMBO_V3`` — one declared selector that expands into key-dependent sub-widgets (``model`` → ``model``, ``model.resolution``). +* Frontend-injected inputs — ``upload`` (the upload button on every media + loader), ``audioUI`` (the audio player), ``image`` (the ``PREVIEW_3D`` + viewport on ``SaveGLB``/``Preview3D``). Extensions add them to the node + definition after ``object_info``; they serialize AFTER every declared + widget, optional ones included, and are ``serialize: false`` on current + frontends (older ones wrote ``"image"``/``null``). The order names them so + a workflow saved by either frontend decomposes, and a fresh node omits them. +* DOM-widget inputs the schema declares under an uppercase custom type + (``Load3D.image`` is ``LOAD_3D``, ``LoadAudioUI.audioUI`` is ``AUDIO_UI``) + and inputs whose ``widgetType`` option overrides a link-shaped socket type + (``FLOAT,INT`` with ``widgetType: "STRING"``) — both occupy a slot. So the catalog is exported from here rather than recomputed by each consumer: a second implementation of widget order is a second answer, and the two would diff --git a/comfy_cli/schemas/widget_catalog.json b/comfy_cli/schemas/widget_catalog.json index a93fa001f..07853629c 100644 --- a/comfy_cli/schemas/widget_catalog.json +++ b/comfy_cli/schemas/widget_catalog.json @@ -22,7 +22,7 @@ "widget_order": { "type": "array", "items": { "type": "string" }, - "description": "Widget names in the exact positional order of the node's widgets_values array — index i in this list IS index i in widgets_values. Includes the synthetic 'control_after_generate' slot the frontend injects after a seed, and dynamic-combo sub-widgets flattened as '.'. Computed by cql.engine.Graph.widget_order, the same call every set-widget in this CLI resolves against." + "description": "Widget names in the exact positional order of the node's widgets_values array — index i in this list IS index i in widgets_values. Includes the synthetic 'control_after_generate' slot the frontend injects after a seed, dynamic-combo sub-widgets flattened as '.', and the trailing inputs frontend extensions inject ('upload' on media loaders, 'audioUI' on the audio family, 'image' on SaveGLB/Preview3D) so a widgets_values written by any frontend version decomposes. Computed by cql.engine.Graph.widget_order, the same call every set-widget in this CLI resolves against." }, "autogrow_templates": { "type": "object", diff --git a/comfy_cli/workflow_ops.py b/comfy_cli/workflow_ops.py index ce808ca9e..2a843d87b 100644 --- a/comfy_cli/workflow_ops.py +++ b/comfy_cli/workflow_ops.py @@ -59,6 +59,7 @@ from typing import Any from comfy_cli import layout +from comfy_cli.cql.engine import FRONTEND_MARKER_SLOTS # New ids live in [2**40, 2**53): always large (never collides with small # frontend counter ids), always inside JS Number.MAX_SAFE_INTEGER. @@ -2166,7 +2167,7 @@ def _widget_index(graph, class_type: str, widget: str, widgets_values=None) -> i # index stays aligned to ``widgets_values`` for the node's real selection. order = graph.widget_order_for_node(class_type, widgets_values) if widget not in order: - avail = [w for w in order if w != "control_after_generate"] + avail = [w for w in order if w not in FRONTEND_MARKER_SLOTS] raise ValueError( f"widget {widget!r} not found on {class_type}; " f"available: {', '.join(avail) if avail else '(none — all inputs are links)'}" diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index db4a06879..546593a35 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -24,6 +24,8 @@ import re from typing import Any +from comfy_cli.cql.engine import _FRONTEND_DOM_WIDGET_TYPES + logger = logging.getLogger(__name__) # C-style comments stripped from dynamic-prompt strings before group parsing. @@ -1028,6 +1030,11 @@ def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: if options.get("forceInput") or options.get("defaultInput"): return False, False input_type = input_spec[0] + # ``widgetType`` overrides the socket type for widget selection + # (``inputSpec.widgetType ?? inputSpec.type`` in the frontend), so a + # ``FLOAT,INT`` input with ``widgetType: "FLOAT"`` owns a slot. + if isinstance(options.get("widgetType"), str) and options.get("widgetType"): + return True, False if isinstance(input_type, (list, tuple)): return True, False # combo of choices if isinstance(input_type, str): @@ -1041,6 +1048,10 @@ def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: return False, False if input_type in {"INT", "FLOAT", "STRING", "BOOLEAN", "COMBO"}: return True, False + if input_type in _FRONTEND_DOM_WIDGET_TYPES: + # Uppercase DOM widgets that serialize a slot (Load3D.image); + # same set the engine's widget order uses, so the two walks agree. + return True, False if input_type.startswith("COMFY_") and "COMBO" in input_type: return True, True if not input_type.isupper(): diff --git a/tests/comfy_cli/cql/test_frontend_widget_slots.py b/tests/comfy_cli/cql/test_frontend_widget_slots.py new file mode 100644 index 000000000..d398c6e55 --- /dev/null +++ b/tests/comfy_cli/cql/test_frontend_widget_slots.py @@ -0,0 +1,363 @@ +"""Frontend-injected and DOM-widget slots in the widget order. + +The frontend serializes MORE positional ``widgets_values`` than object_info +declares for a handful of node families: + +* ``Comfy.UploadImage`` / ``Comfy.UploadAudio`` append a required ``upload`` + input (the upload button) to every node whose required media combo carries + an ``image_upload`` / ``animated_image_upload`` / ``video_upload`` / + ``audio_upload`` flag. Older frontends serialized its value (``"image"``); + current ones mark it ``serialize: false`` and write nothing. +* ``Comfy.AudioWidget`` appends ``audioUI`` to the audio load/save/preview + family; ``Comfy.Preview3D`` / ``Comfy.SaveGLB`` append ``image`` + (``PREVIEW_3D``). +* Server-declared DOM-widget inputs (``LOAD_3D``, ``AUDIO_UI``, ...) occupy a + slot even though their type is an uppercase custom name the engine would + otherwise read as a link. + +The doc host's applier builds a name-keyed widget map from the pinned +catalog's ``widget_order``; a ``widgets_values`` longer than that order is a +hard refusal (``createNodeMap(LoadImage): widgets_values has 2 entries but +widget_order names only 1``), which put every Load* workflow on the v0 path. +Fewer values than names is tolerated, so the order must carry every slot the +frontend CAN write, in the position it writes it: injected inputs come after +every declared one (``getOrderedInputSpecs`` appends unlisted inputs last). +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from comfy_cli.cql.engine import Graph +from comfy_cli.workflow_to_api import convert_ui_to_api + + +def _object_info() -> dict[str, Any]: + files = [["beach.jpg", "example.png"], {"image_upload": True}] + return { + "LoadImage": { + "input": {"required": {"image": files}}, + "input_order": {"required": ["image"]}, + "output": ["IMAGE", "MASK"], + "output_name": ["IMAGE", "MASK"], + "display_name": "Load Image", + "python_module": "nodes", + }, + "LoadImageMask": { + "input": {"required": {"image": files, "channel": [["alpha", "red", "green", "blue"]]}}, + "input_order": {"required": ["image", "channel"]}, + "output": ["MASK"], + "output_name": ["MASK"], + "display_name": "Load Image (as Mask)", + "python_module": "nodes", + }, + "LoadImageWithExif": { + "input": { + "required": {"image": files}, + "optional": {"default_focal_mm": ["FLOAT", {"default": 50.0}]}, + }, + "input_order": {"required": ["image"], "optional": ["default_focal_mm"]}, + "output": ["IMAGE"], + "output_name": ["IMAGE"], + "display_name": "Load Image With EXIF", + "python_module": "custom_nodes.exif", + }, + "LoadVideo": { + "input": {"required": {"file": [["a.mp4"], {"video_upload": True}]}}, + "input_order": {"required": ["file"]}, + "output": ["VIDEO"], + "output_name": ["VIDEO"], + "display_name": "Load Video", + "python_module": "comfy_extras.nodes_video", + }, + "LoadAudio": { + "input": {"required": {"audio": [["a.wav"], {"audio_upload": True}]}}, + "input_order": {"required": ["audio"]}, + "output": ["AUDIO"], + "output_name": ["AUDIO"], + "display_name": "Load Audio", + "python_module": "comfy_extras.nodes_audio", + }, + "PreviewAudio": { + "input": {"required": {"audio": ["AUDIO", {}]}}, + "input_order": {"required": ["audio"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Preview Audio", + "python_module": "comfy_extras.nodes_audio", + }, + "LoadAudioUI": { + "input": { + "required": {"audio": [["a.wav"], {"audio_upload": True}], "start_time": ["FLOAT", {"default": 0.0}]}, + "optional": {"audioUI": ["AUDIO_UI"]}, + }, + "input_order": {"required": ["audio", "start_time"], "optional": ["audioUI"]}, + "output": ["AUDIO"], + "output_name": ["AUDIO"], + "display_name": "Load Audio UI", + "python_module": "custom_nodes.audio_ui", + }, + "Painter": { + # STRING with an upload flag: NOT a media combo, so the frontend + # does not attach the upload button. + "input": { + "required": { + "mask": ["STRING", {"widgetType": "PAINTER", "image_upload": True, "default": ""}], + "width": ["INT", {"default": 512}], + } + }, + "input_order": {"required": ["mask", "width"]}, + "output": ["MASK"], + "output_name": ["MASK"], + "display_name": "Painter", + "python_module": "custom_nodes.painter", + }, + "Load3D": { + "input": { + "required": { + "model_file": ["COMBO", {"options": ["none"], "file_upload": True}], + "image": ["LOAD_3D", {}], + "width": ["INT", {"default": 1024}], + "height": ["INT", {"default": 1024}], + } + }, + "input_order": {"required": ["model_file", "image", "width", "height"]}, + "output": ["IMAGE", "MASK"], + "output_name": ["image", "mask"], + "display_name": "Load 3D", + "python_module": "comfy_extras.nodes_load_3d", + }, + "Load3DAdvanced": { + "input": { + "required": { + "model_file": ["COMBO", {"options": ["none"], "file_upload": True}], + "viewport_state": ["LOAD_3D", {}], + "width": ["INT", {"default": 1024}], + } + }, + "input_order": {"required": ["model_file", "viewport_state", "width"]}, + "output": ["IMAGE"], + "output_name": ["image"], + "display_name": "Load 3D Advanced", + "python_module": "comfy_extras.nodes_load_3d", + }, + "SaveGLB": { + "input": { + "required": {"mesh": ["MESH,FILE_3D_GLB", {}], "filename_prefix": ["STRING", {"default": "3d/ComfyUI"}]} + }, + "input_order": {"required": ["mesh", "filename_prefix"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Save GLB", + "python_module": "comfy_extras.nodes_hunyuan3d", + }, + "Preview3D": { + "input": { + "required": { + "model_file": ["STRING,FILE_3D_GLB,FILE_3D_GLTF", {"default": "", "widgetType": "STRING"}] + }, + "optional": { + "camera_info": ["LOAD3D_CAMERA", {"advanced": True}], + "bg_image": ["IMAGE", {"advanced": True}], + }, + }, + "input_order": {"required": ["model_file"], "optional": ["camera_info", "bg_image"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Preview 3D", + "python_module": "comfy_extras.nodes_load_3d", + }, + "MathAbs": { + "input": {"required": {"value": ["FLOAT,INT", {"default": 0.0, "widgetType": "STRING"}]}}, + "input_order": {"required": ["value"]}, + "output": ["FLOAT"], + "output_name": ["FLOAT"], + "display_name": "Math Abs", + "python_module": "custom_nodes.basic_data_handling", + }, + "MathAdd": { + "input": {"required": {"a": ["INT,FLOAT", {"default": 0.0}], "b": ["INT,FLOAT", {"default": 0.0}]}}, + "input_order": {"required": ["a", "b"]}, + "output": ["FLOAT"], + "output_name": ["FLOAT"], + "display_name": "Math Add", + "python_module": "custom_nodes.essentials", + }, + "KSampler": { + "input": { + "required": { + "seed": ["INT", {"default": 0, "control_after_generate": True}], + "steps": ["INT", {"default": 20}], + } + }, + "input_order": {"required": ["seed", "steps"]}, + "output": ["LATENT"], + "output_name": ["LATENT"], + "display_name": "KSampler", + "python_module": "nodes", + }, + } + + +@pytest.fixture +def graph() -> Graph: + return Graph.from_object_info(_object_info()) + + +# The frontend's serialized shapes these orders must be able to name, slot for +# slot. Captured from real workflows: cloud smoke fixture (LoadImage), the +# inpaint-nodes example (LoadImageMask), AudioTools example (LoadAudio), the +# hunyuan3d template (SaveGLB), DepthAnythingV3 bas_relief (Preview3D). +_FRONTEND_SHAPES = { + "LoadImage": ["beach.jpg", "image"], + "LoadImageMask": ["mask.png", "red", "image"], + "LoadVideo": ["a.mp4", "image"], + "LoadAudio": ["a.wav", None, None], + "SaveGLB": ["mesh/ComfyUI", ""], + "Preview3D": ["out/mesh.glb", ""], +} + + +class TestInjectedUploadSlot: + @pytest.mark.parametrize( + ("cls", "expected"), + [ + ("LoadImage", ["image", "upload"]), + ("LoadImageMask", ["image", "channel", "upload"]), + ("LoadVideo", ["file", "upload"]), + # Injected inputs come after OPTIONAL declared ones too. + ("LoadImageWithExif", ["image", "default_focal_mm", "upload"]), + ], + ) + def test_upload_button_is_the_last_slot(self, graph: Graph, cls: str, expected: list[str]): + assert graph.widget_order(cls) == expected + assert graph.widget_order_default(cls) == expected + assert graph.widget_order_for_node(cls, _FRONTEND_SHAPES.get(cls)) == expected + + def test_string_input_with_upload_flag_gets_no_button(self, graph: Graph): + # Comfy.UploadImage only attaches to a media COMBO. + assert graph.widget_order("Painter") == ["mask", "width"] + + def test_file_upload_flag_alone_gets_no_button(self, graph: Graph): + # Load3D's model_file carries file_upload; the 3D loader handles its + # own upload and the frontend attaches no IMAGEUPLOAD button. + assert "upload" not in graph.widget_order("Load3D") + + def test_upload_has_no_add_node_default(self, graph: Graph): + # Current frontends mark the button ``serialize: false``; a fresh node + # must not carry a phantom trailing value for it. + assert "upload" not in graph.widget_defaults("LoadImage") + assert graph.widget_defaults("LoadImage") == {"image": "beach.jpg"} + + +class TestAudioFamily: + def test_load_audio_carries_audio_ui_then_upload(self, graph: Graph): + # Comfy.AudioWidget registers before Comfy.UploadAudio, so audioUI is + # injected first. Older frontends serialized both as null. + assert graph.widget_order("LoadAudio") == ["audio", "audioUI", "upload"] + assert graph.widget_order_for_node("LoadAudio", _FRONTEND_SHAPES["LoadAudio"]) == ["audio", "audioUI", "upload"] + + def test_preview_audio_link_input_still_gets_audio_ui(self, graph: Graph): + assert graph.widget_order("PreviewAudio") == ["audioUI"] + + def test_server_declared_audio_ui_is_not_injected_twice(self, graph: Graph): + assert graph.widget_order("LoadAudioUI") == ["audio", "start_time", "audioUI", "upload"] + + def test_marker_slots_have_no_defaults(self, graph: Graph): + assert graph.widget_defaults("LoadAudio") == {"audio": "a.wav"} + + +class TestDomWidgetInputs: + def test_load3d_viewport_is_a_slot_in_declared_position(self, graph: Graph): + assert graph.widget_order("Load3D") == ["model_file", "image", "width", "height"] + + def test_load3d_advanced_server_declared_state_is_not_duplicated(self, graph: Graph): + assert graph.widget_order("Load3DAdvanced") == ["model_file", "viewport_state", "width"] + + def test_dom_widget_default_keeps_later_slots_aligned(self, graph: Graph): + # A fresh Load3D must serialize its viewport slot, or width lands in it. + assert graph.widget_defaults("Load3D") == {"model_file": "none", "image": "", "width": 1024, "height": 1024} + + def test_save_glb_preview_is_injected_last(self, graph: Graph): + assert graph.widget_order("SaveGLB") == ["filename_prefix", "image"] + + def test_preview3d_widget_type_override_is_a_widget(self, graph: Graph): + # ``STRING,FILE_3D_GLB,...`` is a link by type; ``widgetType: STRING`` + # makes the frontend render a text widget for it. The camera state is + # ``serialize: false`` and bg_image is a link. + assert graph.widget_order("Preview3D") == ["model_file", "image"] + + def test_multi_type_without_widget_type_stays_a_link(self, graph: Graph): + assert graph.widget_order("MathAdd") == [] + + def test_widget_type_override_on_multi_type_input(self, graph: Graph): + assert graph.widget_order("MathAbs") == ["value"] + assert graph.widget_defaults("MathAbs") == {"value": 0.0} + + def test_control_after_generate_unaffected(self, graph: Graph): + assert graph.widget_order("KSampler") == ["seed", "control_after_generate", "steps"] + + +class TestFrontendShapesFit: + @pytest.mark.parametrize("cls", sorted(_FRONTEND_SHAPES)) + def test_every_captured_shape_fits_the_order(self, graph: Graph, cls: str): + # The doc host refuses a widgets_values longer than widget_order. + assert len(_FRONTEND_SHAPES[cls]) <= len(graph.widget_order_for_node(cls, _FRONTEND_SHAPES[cls])) + + +class TestConverter: + def test_load3d_viewport_slot_is_consumed_in_place(self): + workflow = { + "nodes": [ + { + "id": 1, + "type": "Load3D", + "inputs": [], + "outputs": [], + "widgets_values": ["m.glb", "", 512, 768], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, _object_info()) + assert result["1"]["inputs"] == {"model_file": "m.glb", "image": "", "width": 512, "height": 768} + + def test_trailing_upload_marker_is_ignored(self): + workflow = { + "nodes": [ + { + "id": 1, + "type": "LoadImageMask", + "inputs": [], + "outputs": [], + "widgets_values": ["mask.png", "red", "image"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, _object_info()) + assert result["1"]["inputs"] == {"image": "mask.png", "channel": "red"} + + def test_preview3d_widget_type_override_is_read_as_a_widget(self): + workflow = { + "nodes": [ + { + "id": 1, + "type": "Preview3D", + "inputs": [], + "outputs": [], + "widgets_values": ["out/mesh.glb", ""], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, _object_info()) + assert result["1"]["inputs"]["model_file"] == "out/mesh.glb" From d2f0bfebf6194bc263cda955d167a49ab4c235eb Mon Sep 17 00:00:00 2001 From: kishore Date: Thu, 27 Aug 2026 22:06:31 -0700 Subject: [PATCH 2/6] fix(generate): emit envelope/1 for generate results in json/ndjson modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The submit / sync / resume result paths of `comfy generate` still wrote a bare partner JSON blob via `output.print_json`, so an agent parsing stdout by the documented contract got a document with no `schema`/`ok`/`error` discriminator — indistinguishable from an envelope except by reading it. `list` and `schema` had already moved onto the renderer envelope. In JSON/NDJSON modes `_emit_result` now wraps the partner payload as `data.result` (verbatim, so provider-specific fields survive) with `data.saved` listing the paths `--download` wrote, and emits it through the renderer. The payload schema ships as `generate_result.json` and is registered under `COMMAND_SCHEMAS["comfy generate"]` so `comfy discover` advertises it. Pretty mode with a tail `--json` keeps the legacy raw blob, so this is additive for machine consumers only. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 7 + comfy_cli/command/generate/app.py | 20 ++- comfy_cli/discovery.py | 5 + comfy_cli/schemas/generate_result.json | 19 +++ .../command/generate/test_result_envelope.py | 141 ++++++++++++++++++ 5 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 comfy_cli/schemas/generate_result.json create mode 100644 tests/comfy_cli/command/generate/test_result_envelope.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 592ef7c77..b3c440294 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,13 @@ history. (`createNodeMap(LoadImage): widgets_values has 2 entries but widget_order names only 1`) and `set-widget`/conversion read the values after such a slot one position off. +- `comfy generate `, `comfy generate resume` and sync-mode creates now + emit the `envelope/1` contract in `--output json` / `ndjson` modes instead + of a bare partner blob: the partner payload is wrapped as `data.result` + (verbatim) with `data.saved` listing `--download` artifacts, and the + payload schema is registered as `comfy generate` → `generate_result.json` + so `comfy discover` advertises it. Pretty mode with a tail `--json` keeps + the legacy raw blob. ### Added diff --git a/comfy_cli/command/generate/app.py b/comfy_cli/command/generate/app.py index e4554aa36..74373507b 100644 --- a/comfy_cli/command/generate/app.py +++ b/comfy_cli/command/generate/app.py @@ -344,10 +344,24 @@ def _emit_result(result: poll.PollResult, *, request_id: str, download: str | No # Honor --download in JSON mode too. Previously this returned before # saving, so `--json --download` printed the URL but wrote no file, # forcing callers to curl the URL by hand. Save first, then surface the - # local path alongside the raw response. + # local path alongside the response. + saved: list[str] = [] if download and result.status == "succeeded" and result.image_urls: - saved = output.save_urls(result.image_urls, download, request_id) - output.print_json({"result": result.raw, "saved": [str(p) for p in saved]}) + saved = [str(p) for p in output.save_urls(result.image_urls, download, request_id)] + renderer = get_renderer() + if renderer.is_json(): + # JSON/NDJSON modes get the envelope/1 contract every other + # machine-readable command speaks: data.result wraps the partner + # payload, data.saved lists --download artifacts. Registered as + # COMMAND_SCHEMAS["comfy generate"] -> generate_result.json. + data: dict[str, Any] = {"result": result.raw} + if saved: + data["saved"] = saved + renderer.emit(data, ok=True, command="generate") + return + # Pretty mode with an explicit tail --json keeps the legacy raw blob. + if saved: + output.print_json({"result": result.raw, "saved": saved}) else: output.print_json(result.raw) return diff --git a/comfy_cli/discovery.py b/comfy_cli/discovery.py index 0e9c12ef5..4b5b442af 100644 --- a/comfy_cli/discovery.py +++ b/comfy_cli/discovery.py @@ -145,6 +145,11 @@ # the help tree; agents resolve them through `command_schemas`). "comfy generate list": "generate_list", "comfy generate schema": "generate_schema", + # Terminal result of `comfy generate ` / `generate resume` / + # sync-mode creates: the partner payload wrapped as + # ``data.result`` (+ ``data.saved`` under --download). These are + # argv-tail paths too, so they have no help-tree node. + "comfy generate": "generate_result", # curated model-knowledge bundle "comfy knowledge status": "knowledge", "comfy knowledge resolve": "knowledge", diff --git a/comfy_cli/schemas/generate_result.json b/comfy_cli/schemas/generate_result.json new file mode 100644 index 000000000..6422fd440 --- /dev/null +++ b/comfy_cli/schemas/generate_result.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://comfy.org/schemas/generate_result.json", + "title": "comfy generate", + "description": "Terminal result of `comfy generate `, `comfy generate resume `, and sync-mode creates, in JSON/NDJSON modes: the partner's final response body wrapped as `result` (verbatim, so provider-specific fields survive), plus `saved` local paths when --download produced files. Failures do not use this schema — they arrive as an ok=false envelope with a registered error.code.", + "type": "object", + "required": ["result"], + "properties": { + "result": { + "type": "object", + "description": "The partner API's final response payload, unmodified." + }, + "saved": { + "type": "array", + "items": { "type": "string" }, + "description": "Local paths written by --download, in URL order. Absent when --download was not passed or produced nothing." + } + } +} diff --git a/tests/comfy_cli/command/generate/test_result_envelope.py b/tests/comfy_cli/command/generate/test_result_envelope.py new file mode 100644 index 000000000..5594d0c0b --- /dev/null +++ b/tests/comfy_cli/command/generate/test_result_envelope.py @@ -0,0 +1,141 @@ +"""``comfy generate`` result paths emit ``envelope/1``. + +The discovery verbs (``list`` / ``schema``) were migrated onto the renderer +envelope earlier. The submit/sync/resume *result* paths were left emitting a +bare partner JSON blob via ``output.print_json`` (``_emit_result``, +``as_json=True``), so an agent parsing stdout by the documented contract got a +document with no ``schema``/``type``/``ok``/``error`` discriminator — +indistinguishable from an envelope except by reading it. + +What is pinned here (unit-level; no network, no API key): + + - JSON and NDJSON modes: the terminal result is one ``envelope/1`` whose + ``data`` validates against the registered ``generate_result.json`` schema; + - ``--download`` surfaces the saved local paths inside ``data.saved``; + - the schema is registered in ``COMMAND_SCHEMAS["comfy generate"]`` so + ``comfy discover`` advertises it; + - pretty mode with a tail ``--json`` keeps the legacy raw blob — this fix is + additive for machine consumers only. +""" + +from __future__ import annotations + +import io +import json +from pathlib import Path + +import jsonschema +import pytest + +from comfy_cli.command.generate import app as generate_app +from comfy_cli.command.generate import poll +from comfy_cli.discovery import COMMAND_SCHEMAS +from comfy_cli.output.renderer import OutputMode, Renderer, reset_renderer_for_testing, set_renderer + +SCHEMAS_DIR = Path(__file__).resolve().parents[4] / "comfy_cli" / "schemas" + + +def _validator_for(schema_name: str) -> jsonschema.protocols.Validator: + schema = json.loads((SCHEMAS_DIR / schema_name).read_text()) + store: dict[str, dict] = {} + for path in SCHEMAS_DIR.glob("*.json"): + s = json.loads(path.read_text()) + if s.get("$id"): + store[s["$id"]] = s + store[path.name] = s + base = SCHEMAS_DIR.absolute().as_uri() + "/" + resolver = jsonschema.RefResolver(base_uri=base, referrer=schema, store=store) + return jsonschema.Draft202012Validator(schema, resolver=resolver) + + +def _succeeded(**overrides) -> poll.PollResult: + fields = { + "status": "succeeded", + "error": None, + "image_urls": ["https://cdn.example/img.png"], + "raw": {"status": "succeeded", "urls": ["https://cdn.example/img.png"]}, + } + fields.update(overrides) + return poll.PollResult(**fields) + + +@pytest.fixture() +def pinned_renderer(): + def make(mode: OutputMode) -> tuple[Renderer, io.StringIO, io.StringIO]: + machine, pretty = io.StringIO(), io.StringIO() + renderer = Renderer( + mode=mode, + command="generate", + version="test", + _machine_stream_override=machine, + _pretty_stream_override=pretty, + ) + set_renderer(renderer) + return renderer, machine, pretty + + yield make + reset_renderer_for_testing() + + +# --------------------------------------------------------------------------- # +# These four failed before the fix — stdout carried a bare partner blob. +# --------------------------------------------------------------------------- # + + +def test_success_emits_envelope_in_json_mode(pinned_renderer): + _, machine, _ = pinned_renderer(OutputMode.JSON) + generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=True) + envelope = json.loads(machine.getvalue().splitlines()[-1]) + assert envelope["schema"] == "envelope/1" + assert envelope["type"] == "envelope" + assert envelope["ok"] is True + assert envelope["command"] == "generate" + assert envelope["error"] is None + + +def test_download_variant_lists_saved_paths_in_data(pinned_renderer, tmp_path): + _, machine, _ = pinned_renderer(OutputMode.JSON) + saved = tmp_path / "out.png" + from comfy_cli.command.generate import output as gen_output + + original = gen_output.save_urls + gen_output.save_urls = lambda urls, d, rid: [saved] + try: + generate_app._emit_result(_succeeded(), request_id="req1", download=str(tmp_path), as_json=True) + finally: + gen_output.save_urls = original + data = json.loads(machine.getvalue().splitlines()[-1])["data"] + assert data["saved"] == [str(saved)] + + +def test_payload_validates_against_registered_schema(pinned_renderer): + assert COMMAND_SCHEMAS.get("comfy generate") == "generate_result", ( + "`comfy generate` must advertise its result schema via `comfy discover`" + ) + _, machine, _ = pinned_renderer(OutputMode.JSON) + generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=True) + envelope = json.loads(machine.getvalue().splitlines()[-1]) + _validator_for("generate_result.json").validate(envelope["data"]) + _validator_for("envelope.json").validate(envelope) + + +def test_ndjson_final_line_is_the_envelope(pinned_renderer): + _, machine, _ = pinned_renderer(OutputMode.NDJSON) + generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=True) + lines = [json.loads(x) for x in machine.getvalue().splitlines()] + assert lines[-1]["schema"] == "envelope/1" + assert all(line.get("type") != "envelope" for line in lines[:-1]) + + +# --------------------------------------------------------------------------- # +# Guard: pretty mode with a tail --json keeps the legacy raw blob. +# --------------------------------------------------------------------------- # + + +def test_pretty_mode_tail_json_keeps_legacy_raw_blob(pinned_renderer, capsys): + # print_json bypasses the renderer and writes builtin sys.stdout, so + # capture via capsys rather than the renderer's stream overrides. + pinned_renderer(OutputMode.PRETTY) + generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=True) + doc = json.loads(capsys.readouterr().out) + assert doc["status"] == "succeeded" From 32f14bcc311954706875d4fa8a9b587584209534 Mon Sep 17 00:00:00 2001 From: kishore Date: Fri, 28 Aug 2026 01:09:11 -0700 Subject: [PATCH 3/6] fix(cql): refuse frontend-injected widget slots as edit targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `frontend_extra_widget_names()` entries (`upload`, `audioUI`, the `PREVIEW_3D` `image` on SaveGLB/Preview3D) own a positional `widgets_values` slot but have no schema port. `comfy workflow slots` already omitted them, yet the name-only lookups in `_widget_index` (set-widget, apply replay) and `_write_widget` (set-slot, vary) still accepted them and wrote the value with no validation — a ghost target `slots` never advertised. Mark those entries `frontend_injected` on `_WidgetEntry`, keep them in the positional order, and refuse them by name on every write surface with one shared error (`frontend_injected_widget_error`) naming the slot as frontend-injected and not editable. The "available widgets" list on both lookups is now derived from schema-backed entries (`Graph.editable_widget_names`) — exactly what `slots` lists — instead of filtering the order by marker name, so `SaveGLB.image` no longer shows up as a suggestion either. `control_after_generate` is also a `port=None` marker, but it carries a real serialized user value (`fixed`/`randomize`/…) and pinning a seed is a legitimate edit; it stays writable (and, like `slots`, unadvertised). Pinned by tests either way. Co-Authored-By: Claude Fable 5 --- comfy_cli/cql/engine.py | 78 +++++++++-- comfy_cli/workflow_ops.py | 15 +- .../cql/test_frontend_widget_slots.py | 132 ++++++++++++++++++ 3 files changed, 209 insertions(+), 16 deletions(-) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 381643540..8b4c66374 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -49,9 +49,13 @@ # Widget names the FRONTEND injects into a node's inputs after object_info # (``beforeRegisterNodeDef`` in ``uploadImage.ts``/``uploadAudio.ts``/ -# ``load3d.ts``/``saveMesh.ts``). They have no schema port and are never -# ``set-widget`` targets. Listed with ``control_after_generate`` because all -# three are marker slots a name<->index consumer must be able to name. +# ``load3d.ts``/``saveMesh.ts``). They have no schema port. Listed with +# ``control_after_generate`` because all three are marker slots a name<->index +# consumer must be able to name — but they differ as EDIT targets: the seed +# companion carries a real serialized user value (``fixed``/``randomize``/…) +# and stays writable, while the injected button/player/viewport slots (these +# two plus the ``PREVIEW_3D`` ``image`` of ``_PREVIEW_3D_CLASSES``) are refused +# by every write surface — see ``_WidgetEntry.frontend_injected``. FRONTEND_MARKER_SLOTS = frozenset({"control_after_generate", "upload", "audioUI"}) # ``Comfy.AudioWidget`` appends an ``audioUI`` player to exactly these classes. @@ -1331,6 +1335,23 @@ def widget_order_for_node(self, class_name: str, widgets_values: list[Any] | Non return [] return [e.name for e in _expand_widget_entries(m, widgets_values or [])] + def editable_widget_names(self, class_name: str, widgets_values: list[Any] | None = None) -> list[str]: + """The subset of :meth:`widget_order_for_node` a write may target — + schema-backed slots only, i.e. what ``comfy workflow slots`` advertises.""" + m = self._nodes.get(class_name) + if m is None: + return [] + return _editable_widget_names(_expand_widget_entries(m, widgets_values or [])) + + def frontend_injected_widget_names(self, class_name: str) -> list[str]: + """Names in the widget order that the frontend injects with no schema + port (``upload``, ``audioUI``, ``PREVIEW_3D`` ``image``). They own a + positional slot but are never an edit target.""" + m = self._nodes.get(class_name) + if m is None: + return [] + return frontend_extra_widget_names(m) + def widget_defaults(self, class_name: str) -> dict[str, Any]: """Default value per widget-order name — including dynamic-combo selectors (first key), their sub-widgets, and control_after_generate. Used by @@ -2418,15 +2439,43 @@ def _widgets_as_positional(widgets_values: Any, graph: Graph | None, class_type: class _WidgetEntry: """One positional ``widgets_values`` slot in a node's value-aware order. - ``port`` is ``None`` for a ``control_after_generate`` marker slot (it has - no schema port). ``owner`` is the dotted name of the dynamic combo whose - selected option contributed this entry (``None`` for top-level inputs) — - used to size a combo's sub-span when its selector changes. + ``port`` is ``None`` for a marker slot with no schema port: the + ``control_after_generate`` seed companion, or a frontend-injected input + (``frontend_extra_widget_names``). ``frontend_injected`` tells the two + apart — the companion carries a real user value and is writable; an + injected ``upload``/``audioUI``/``PREVIEW_3D`` slot is a button, player or + viewport state with nothing to validate against, and every write surface + refuses it by name (``frontend_injected_widget_error``). ``owner`` is the + dotted name of the dynamic combo whose selected option contributed this + entry (``None`` for top-level inputs) — used to size a combo's sub-span + when its selector changes. """ name: str port: Port | None owner: str | None + frontend_injected: bool = False + + +def _editable_widget_names(entries: list[_WidgetEntry]) -> list[str]: + """The names a write surface advertises: every schema-backed slot, in + positional order — exactly what ``comfy workflow slots`` lists. Marker + slots are left out: injected ones are refused, and the writable + ``control_after_generate`` companion is deliberately unadvertised.""" + return [e.name for e in entries if e.port is not None] + + +def frontend_injected_widget_error(node_type: str, widget: str, available: list[str]) -> ValueError: + """The refusal every write surface raises for a frontend-injected slot. + + Worded without ``not found`` on purpose: the address resolved, so the + sibling-suggestion enrichment (``_enrich_resolution_error``) must not fire. + """ + return ValueError( + f"widget {widget!r} on {node_type} is frontend-injected (no schema input; " + f"`comfy workflow slots` never lists it) and is not editable; " + f"available widgets: {', '.join(available) if available else '(none — all inputs are links)'}" + ) def _dynamic_combo_sub_ports(dynamic_options: list[dict], selector: Any, prefix: str) -> list[Port]: @@ -2483,7 +2532,7 @@ def emit(name: str, port: Port, owner: str | None, depth: int) -> None: continue emit(p.name, p, None, 0) for name in frontend_extra_widget_names(m): - entries.append(_WidgetEntry(name=name, port=None, owner=None)) + entries.append(_WidgetEntry(name=name, port=None, owner=None, frontend_injected=True)) return entries @@ -2503,7 +2552,7 @@ def _node_widget_slots(node: dict, prefix: str, graph: Graph) -> list[dict]: widgets = _widgets_as_positional(node.get("widgets_values"), graph, node_type) slots: list[dict] = [] for idx, entry in enumerate(_expand_widget_entries(m, widgets)): - if entry.port is None: # control_after_generate marker — not a slot + if entry.port is None: # control_after_generate / injected marker — not a slot continue current = widgets[idx] if idx < len(widgets) else None slot = { @@ -2681,23 +2730,26 @@ def _write_widget(node: dict, input_name: str, value: Any, graph: Graph, *, exte # values this write is about to index against. node["widgets_values"] = widgets order = graph.widget_order_for_node(node_type, widgets) + entries = _expand_widget_entries(m, widgets) + if any(e.frontend_injected and e.name == input_name for e in entries): + raise frontend_injected_widget_error(node_type, input_name, _editable_widget_names(entries)) try: widget_idx = order.index(input_name) except ValueError: warning = _unknown_dynamic_sub_warning(m, input_name, order, widgets) if warning is not None: return [warning] - avail = [n for n in order if n != "control_after_generate"] + avail = _editable_widget_names(entries) raise ValueError( f"widget {input_name!r} not found on {node_type}; " f"available widgets: {', '.join(avail) if avail else '(none — all inputs are links)'}" ) - entries = _expand_widget_entries(m, widgets) port = next((e.port for e in entries if e.name == input_name), None) if port is None: - # Marker slot or an order override without matching entries (tests - # monkeypatch widget_order_for_node) — fall back to the declared port. + # control_after_generate marker or an order override without matching + # entries (tests monkeypatch widget_order_for_node) — fall back to the + # declared port. port = next((p for p in m.inputs if p.name == input_name), None) if port is not None and _is_dynamic_combo_type(port.type) and port.dynamic_options: diff --git a/comfy_cli/workflow_ops.py b/comfy_cli/workflow_ops.py index 2a843d87b..04721cf13 100644 --- a/comfy_cli/workflow_ops.py +++ b/comfy_cli/workflow_ops.py @@ -59,7 +59,7 @@ from typing import Any from comfy_cli import layout -from comfy_cli.cql.engine import FRONTEND_MARKER_SLOTS +from comfy_cli.cql.engine import frontend_injected_widget_error # New ids live in [2**40, 2**53): always large (never collides with small # frontend counter ids), always inside JS Number.MAX_SAFE_INTEGER. @@ -2166,11 +2166,20 @@ def _widget_index(graph, class_type: str, widget: str, widgets_values=None) -> i # selected key (from ``widgets_values``), not the schema's first key, so the # index stays aligned to ``widgets_values`` for the node's real selection. order = graph.widget_order_for_node(class_type, widgets_values) + # A frontend-injected slot (``upload``/``audioUI``/``PREVIEW_3D`` ``image``) + # sits in ``order`` — it owns a position — but has no schema port to + # validate against and ``slots`` never advertises it; refuse it by name so + # it can't become a ghost target. ``control_after_generate`` is also + # unadvertised but stays writable (it carries a real user value). + if widget in graph.frontend_injected_widget_names(class_type): + raise frontend_injected_widget_error( + class_type, widget, graph.editable_widget_names(class_type, widgets_values) + ) if widget not in order: - avail = [w for w in order if w not in FRONTEND_MARKER_SLOTS] + avail = graph.editable_widget_names(class_type, widgets_values) raise ValueError( f"widget {widget!r} not found on {class_type}; " - f"available: {', '.join(avail) if avail else '(none — all inputs are links)'}" + f"available widgets: {', '.join(avail) if avail else '(none — all inputs are links)'}" ) return order.index(widget) diff --git a/tests/comfy_cli/cql/test_frontend_widget_slots.py b/tests/comfy_cli/cql/test_frontend_widget_slots.py index d398c6e55..ea81ffbbd 100644 --- a/tests/comfy_cli/cql/test_frontend_widget_slots.py +++ b/tests/comfy_cli/cql/test_frontend_widget_slots.py @@ -26,10 +26,13 @@ from __future__ import annotations +import copy from typing import Any import pytest +from comfy_cli import workflow_ops +from comfy_cli.cql import engine from comfy_cli.cql.engine import Graph from comfy_cli.workflow_to_api import convert_ui_to_api @@ -361,3 +364,132 @@ def test_preview3d_widget_type_override_is_read_as_a_widget(self): } result = convert_ui_to_api(workflow, _object_info()) assert result["1"]["inputs"]["model_file"] == "out/mesh.glb" + + +# --------------------------------------------------------------------------- +# Injected slots own a position but are never an edit target. +# --------------------------------------------------------------------------- + + +def _node(node_id: int, cls: str, widgets: list[Any]) -> dict[str, Any]: + return {"id": node_id, "type": cls, "inputs": [], "outputs": [], "widgets_values": list(widgets), "mode": 0} + + +def _workflow(*nodes: dict[str, Any]) -> dict[str, Any]: + return {"nodes": list(nodes), "links": []} + + +def _available(msg: str) -> list[str]: + """The advertised target list out of an edit error, split on the shared + ``available widgets:`` marker every write surface renders.""" + assert "available widgets: " in msg, msg + # The not-found path appends a ". Nodes in this workflow: …" hint; names + # never contain ". ", so the list ends at the first sentence break. + return msg.split("available widgets: ", 1)[1].split(". ", 1)[0].rstrip(".").split(", ") + + +# (class, injected name, the only schema-backed widgets ``slots`` advertises) +_INJECTED_TARGETS = [ + ("SaveGLB", "image", ["filename_prefix"]), + ("Preview3D", "image", ["model_file"]), + ("LoadImage", "upload", ["image"]), + ("LoadAudio", "audioUI", ["audio"]), +] + + +class TestInjectedSlotsAreNotEditable: + """``frontend_extra_widget_names`` entries have ``port=None``: no schema to + validate against and no address ``comfy workflow slots`` ever lists. Every + write surface must refuse them by name — otherwise ``set-widget 1.image`` + on a SaveGLB silently lands an unvalidated value in the viewport slot.""" + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_set_widget_refuses_injected_slot(self, graph: Graph, cls: str, name: str, avail: list[str]): + wf = _workflow(_node(1, cls, _FRONTEND_SHAPES[cls])) + before = copy.deepcopy(wf) + with pytest.raises(ValueError) as ei: + workflow_ops.set_widget(wf, graph, 1, name, "ghost") + msg = str(ei.value) + assert "frontend-injected" in msg and "not editable" in msg + assert _available(msg) == avail + assert wf == before + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_set_slot_refuses_injected_slot(self, graph: Graph, cls: str, name: str, avail: list[str]): + # ``set-slot`` / ``vary`` go through ``_write_widget``. + wf = _workflow(_node(1, cls, _FRONTEND_SHAPES[cls])) + before = copy.deepcopy(wf) + with pytest.raises(ValueError) as ei: + engine._apply_one_slot(wf, f"1.{name}", "ghost", graph) + msg = str(ei.value) + assert "frontend-injected" in msg and "not editable" in msg + assert _available(msg) == avail + assert wf == before + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_apply_replay_refuses_injected_slot(self, graph: Graph, cls: str, name: str, avail: list[str]): + # A hand-built op replayed through ``apply`` must not bypass the check. + wf = _workflow(_node(1, cls, _FRONTEND_SHAPES[cls])) + before = copy.deepcopy(wf["nodes"]) + op = workflow_ops._new_op("set_widget", "cli", 0, node_id=1, widget=name, value="ghost") + with pytest.raises(ValueError) as ei: + workflow_ops.apply_op(wf, op, graph) + assert "frontend-injected" in str(ei.value) + assert _available(str(ei.value)) == avail + assert wf["nodes"] == before + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_not_found_list_omits_injected_slots(self, graph: Graph, cls: str, name: str, avail: list[str]): + # Both lookups' "available" list is what ``slots`` advertises — never + # the injected name — so a typo can't be "corrected" onto a ghost. + wf = _workflow(_node(1, cls, _FRONTEND_SHAPES[cls])) + with pytest.raises(ValueError) as ei: + workflow_ops.set_widget(wf, graph, 1, "bogus", "x") + assert _available(str(ei.value)) == avail + with pytest.raises(ValueError) as ei: + engine._apply_one_slot(wf, "1.bogus", "x", graph) + assert _available(str(ei.value)) == avail + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_positional_order_still_names_the_slot(self, graph: Graph, cls: str, name: str, avail: list[str]): + # Refusing the write must not drop the slot from the name<->index + # contract: it still owns its trailing position. + order = graph.widget_order_for_node(cls, _FRONTEND_SHAPES[cls]) + injected = graph.frontend_injected_widget_names(cls) + assert name in injected + # Editable slots first, every injected slot trailing — unchanged. + assert order == avail + injected + assert graph.editable_widget_names(cls, _FRONTEND_SHAPES[cls]) == avail + + @pytest.mark.parametrize(("cls", "name", "avail"), _INJECTED_TARGETS) + def test_error_list_matches_slots(self, graph: Graph, cls: str, name: str, avail: list[str]): + node = _node(1, cls, _FRONTEND_SHAPES[cls]) + advertised = [s["name"] for s in engine._node_widget_slots(node, "1", graph)] + assert advertised == avail + assert name not in advertised + + +class TestControlAfterGenerateStaysWritable: + """The seed companion is also a ``port=None`` marker, but unlike the + injected button/player/viewport slots it carries a real serialized user + value (``fixed``/``randomize``/...) — pinning a seed is a legitimate edit. + It stays writable on every surface, and — like ``slots`` — unlisted.""" + + def test_set_widget_writes_marker(self, graph: Graph): + wf = _workflow(_node(1, "KSampler", [0, "randomize", 20])) + wf, op = workflow_ops.set_widget(wf, graph, 1, "control_after_generate", "fixed") + assert wf["nodes"][0]["widgets_values"] == [0, "fixed", 20] + assert op["widget"] == "control_after_generate" + + def test_set_slot_writes_marker(self, graph: Graph): + wf = _workflow(_node(1, "KSampler", [0, "randomize", 20])) + assert engine._apply_one_slot(wf, "1.control_after_generate", "fixed", graph) == [] + assert wf["nodes"][0]["widgets_values"] == [0, "fixed", 20] + + def test_marker_is_not_advertised(self, graph: Graph): + wf = _workflow(_node(1, "KSampler", [0, "randomize", 20])) + with pytest.raises(ValueError) as ei: + workflow_ops.set_widget(wf, graph, 1, "bogus", 1) + assert _available(str(ei.value)) == ["seed", "steps"] + assert graph.editable_widget_names("KSampler", [0, "randomize", 20]) == ["seed", "steps"] + assert graph.frontend_injected_widget_names("KSampler") == [] From 7aba70fc05a60a22d28095dd252235d156efc88f Mon Sep 17 00:00:00 2001 From: kishore Date: Fri, 28 Aug 2026 10:20:51 -0700 Subject: [PATCH 4/6] chore(hygiene): drop the internal ticket id from test_deprecated_nodes The public-repo-hygiene check flags ticket-shaped ids; this one arrived on main in #808 and has kept every PR red since. The sentence loses nothing. Co-Authored-By: Claude Fable 5 --- tests/comfy_cli/command/test_deprecated_nodes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/comfy_cli/command/test_deprecated_nodes.py b/tests/comfy_cli/command/test_deprecated_nodes.py index 60524a527..ce87e6b58 100644 --- a/tests/comfy_cli/command/test_deprecated_nodes.py +++ b/tests/comfy_cli/command/test_deprecated_nodes.py @@ -4,7 +4,7 @@ suffixes its display name with "(DEPRECATED)" / "(Legacy)"; the successor is registered under the bare display name (``ImageBatch`` -> ``BatchImagesNode``). The frontend hides such classes from its node library by default. The agent -kept building on ``ImageBatch`` (BE-7684) because ``nodes search`` ranked it +kept building on ``ImageBatch`` because ``nodes search`` ranked it like any live class and ``add-node`` accepted it, so: * ``nodes search`` / ``nodes ls`` drop deprecated rows unless From d7b8522de85b3f27f77b9f8ef0d4336e7490eac1 Mon Sep 17 00:00:00 2001 From: kishore Date: Fri, 28 Aug 2026 17:55:22 -0700 Subject: [PATCH 5/6] fix: generate failures are ok=false in every mode; --output json alone gets the envelope; fresh nodes carry no phantom injected values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review (annehe9) on #809: * generate: the tail --json branch returned before the terminal-status check, so a failed job was emitted as an ok=true envelope with exit 0 — the opposite of what generate_result.json promises. The failure path now runs FIRST for every output mode (ok=false envelope + exit 1 in JSON, red line + raw response + exit 1 in pretty). The envelope is gated on the renderer's mode alone, so the global --output json (or a redirected stdout) gets it without the legacy tail --json; the two pretty-mode generate tests that relied on non-TTY-without-flag pin pretty with --no-json, as the rest of that file does. * add_node materialized every name in widget_order_default, so a fresh node carried a trailing null for each injected slot ("upload", "audioUI") — contrary to the comment and to what the frontend writes. Trailing names with no default are dropped. The injected PREVIEW_3D image (SaveGLB / Preview3D) IS serialized, as "": it now gets the same default a declared PREVIEW_3D port gets, matching the captured ["mesh/ComfyUI", ""]. * The add_node claim is now asserted on the built node, not the defaults dict. * capture_recipe validates --param targets against editable_widget_names, so an injected slot is refused up front rather than at apply. Co-Authored-By: Claude Fable 5 --- comfy_cli/command/generate/app.py | 62 ++++++++++--------- comfy_cli/cql/engine.py | 11 +++- comfy_cli/workflow_ops.py | 14 ++++- tests/comfy_cli/command/generate/test_app.py | 6 +- .../command/generate/test_result_envelope.py | 43 +++++++++++++ .../cql/test_frontend_widget_slots.py | 34 ++++++++-- 6 files changed, 129 insertions(+), 41 deletions(-) diff --git a/comfy_cli/command/generate/app.py b/comfy_cli/command/generate/app.py index 74373507b..f9dd0581d 100644 --- a/comfy_cli/command/generate/app.py +++ b/comfy_cli/command/generate/app.py @@ -340,37 +340,14 @@ def _spinner() -> Progress: def _emit_result(result: poll.PollResult, *, request_id: str, download: str | None, as_json: bool) -> None: - if as_json: - # Honor --download in JSON mode too. Previously this returned before - # saving, so `--json --download` printed the URL but wrote no file, - # forcing callers to curl the URL by hand. Save first, then surface the - # local path alongside the response. - saved: list[str] = [] - if download and result.status == "succeeded" and result.image_urls: - saved = [str(p) for p in output.save_urls(result.image_urls, download, request_id)] - renderer = get_renderer() - if renderer.is_json(): - # JSON/NDJSON modes get the envelope/1 contract every other - # machine-readable command speaks: data.result wraps the partner - # payload, data.saved lists --download artifacts. Registered as - # COMMAND_SCHEMAS["comfy generate"] -> generate_result.json. - data: dict[str, Any] = {"result": result.raw} - if saved: - data["saved"] = saved - renderer.emit(data, ok=True, command="generate") - return - # Pretty mode with an explicit tail --json keeps the legacy raw blob. - if saved: - output.print_json({"result": result.raw, "saved": saved}) - else: - output.print_json(result.raw) - return + renderer = get_renderer() + # One failure path for every output mode, checked FIRST: a terminally + # failed job is never a result. In JSON/NDJSON modes it is an ok=false + # envelope with a registered code (the generate_result schema promises + # exactly that); in pretty mode it is a red line plus the partner's raw + # response. Either way the exit code is 1 — a consumer that trusts ``ok`` + # (or the exit code) must never read a failure as success. if result.status != "succeeded": - # A terminal non-succeeded job is a FAILURE, not a result, so it owes the - # caller an envelope even though the success paths above deliberately - # bypass the renderer. (The `as_json` branch returned already: that is - # the command-local `--json` raw-response contract, left untouched.) - renderer = get_renderer() message = f"Job {result.status}: {result.error or 'unknown error'}" if renderer.is_json(): renderer.error( @@ -386,6 +363,31 @@ def _emit_result(result: poll.PollResult, *, request_id: str, download: str | No rprint(f"[bold red]{sanitize_markup(message)}[/bold red]") output.print_json(result.raw) raise typer.Exit(code=1) + if renderer.is_json() or as_json: + # Honor --download in machine modes too. Previously this returned + # before saving, so `--json --download` printed the URL but wrote no + # file, forcing callers to curl the URL by hand. Save first, then + # surface the local path alongside the response. + saved: list[str] = [] + if download and result.image_urls: + saved = [str(p) for p in output.save_urls(result.image_urls, download, request_id)] + if renderer.is_json(): + # JSON/NDJSON modes (the global ``--output json``, a redirected + # stdout, or the tail ``--json``) get the envelope/1 contract every + # other machine-readable command speaks: data.result wraps the + # partner payload, data.saved lists --download artifacts. + # Registered as COMMAND_SCHEMAS["comfy generate"] -> generate_result.json. + data: dict[str, Any] = {"result": result.raw} + if saved: + data["saved"] = saved + renderer.emit(data, ok=True, command="generate") + return + # Pretty mode with an explicit tail --json keeps the legacy raw blob. + if saved: + output.print_json({"result": result.raw, "saved": saved}) + else: + output.print_json(result.raw) + return if download and result.image_urls: saved = output.save_urls(result.image_urls, download, request_id) output.print_urls(result.image_urls, request_id=request_id) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 8b4c66374..dfc3a5444 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -1376,8 +1376,15 @@ def widget_defaults(self, class_name: str) -> dict[str, Any]: out[p.name] = None if _has_control_after_generate_slot(p): out["control_after_generate"] = "fixed" - # Frontend-injected marker slots (``upload``, ``audioUI``) are trailing - # and ``serialize: false`` on current frontends: no default, no value. + # Frontend-injected slots: the ``upload``/``audioUI`` buttons are + # ``serialize: false`` on current frontends — no default, no value, so + # a fresh node ends before them (``_build_node`` drops trailing names + # with no default). The injected PREVIEW_3D ``image`` (SaveGLB / + # Preview3D) IS a DOM widget the frontend serializes as ``""`` — the + # captured shape is ``["mesh/ComfyUI", ""]`` — so it gets the same + # default a declared PREVIEW_3D port gets. + if m.id in _PREVIEW_3D_CLASSES and "image" not in out and "image" in frontend_extra_widget_names(m): + out["image"] = _FRONTEND_DOM_WIDGET_DEFAULTS["PREVIEW_3D"] return out # -- Validation -- diff --git a/comfy_cli/workflow_ops.py b/comfy_cli/workflow_ops.py index 04721cf13..c89ce2efd 100644 --- a/comfy_cli/workflow_ops.py +++ b/comfy_cli/workflow_ops.py @@ -1293,8 +1293,10 @@ def capture_recipe(workflow: dict, graph, name: str = "captured", lift: dict | N raise RecipeError( f"--param target node {node_id} is a UI-only {node.get('type')} — capture skips it (it never reaches the API)" ) - if widget not in graph.widget_order_default(node.get("type", "")): - raise RecipeError(f"--param target {node_id}.{widget!r}: not a widget on {node.get('type')}") + # Editable names only: an injected slot (``upload``) owns a position + # but no value, and apply would refuse it anyway — say so up front. + if widget not in graph.editable_widget_names(node.get("type", "")): + raise RecipeError(f"--param target {node_id}.{widget!r}: not an editable widget on {node.get('type')}") warnings: list[dict] = [] for n in ui_nodes: @@ -2145,7 +2147,13 @@ def _build_node(node_id: int, class_type: str, m, graph, pos: list, size: list) # Widget values in positional order, including dynamic-combo selectors and # their sub-widgets — sourced from the engine so add-node matches the converter. defaults = graph.widget_defaults(class_type) - widgets = [defaults.get(name) for name in graph.widget_order_default(class_type)] + order = graph.widget_order_default(class_type) + # A trailing name with no default is a frontend-injected ``serialize: + # false`` slot (``upload``, ``audioUI``): the frontend writes nothing for + # it, so neither does a fresh node — no phantom trailing ``null``. + while order and order[-1] not in defaults: + order = order[:-1] + widgets = [defaults.get(name) for name in order] return { "id": node_id, "type": class_type, diff --git a/tests/comfy_cli/command/generate/test_app.py b/tests/comfy_cli/command/generate/test_app.py index b2a6bc18d..9f6d9fcd3 100644 --- a/tests/comfy_cli/command/generate/test_app.py +++ b/tests/comfy_cli/command/generate/test_app.py @@ -292,7 +292,9 @@ def test_generate_sync_with_download(runner, api_key, tmp_path, monkeypatch): monkeypatch.setattr(gen_app.client.httpx, "post", lambda *a, **kw: resp) monkeypatch.setattr("comfy_cli.command.generate.client.download_bytes", lambda *a, **kw: b"png-bytes") download = str(tmp_path / "out.png") - r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--download", download]) + # --no-json pins pretty mode (CliRunner has no TTY, so the renderer would + # otherwise pick JSON and emit the envelope instead of the "Saved" line). + r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--download", download, "--no-json"]) assert r.exit_code == 0, r.stdout assert Path(download).exists() assert Path(download).read_bytes() == b"png-bytes" @@ -312,7 +314,7 @@ def test_generate_json_flag(runner, api_key, monkeypatch): def test_generate_download_no_urls(runner, api_key, monkeypatch): resp = httpx.Response(200, json={"data": []}) monkeypatch.setattr(gen_app.client.httpx, "post", lambda *a, **kw: resp) - r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--download", "/tmp/x.png"]) + r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--no-json", "--download", "/tmp/x.png"]) assert r.exit_code == 0 assert "no image urls" in r.stdout.lower() diff --git a/tests/comfy_cli/command/generate/test_result_envelope.py b/tests/comfy_cli/command/generate/test_result_envelope.py index 5594d0c0b..70fd8a084 100644 --- a/tests/comfy_cli/command/generate/test_result_envelope.py +++ b/tests/comfy_cli/command/generate/test_result_envelope.py @@ -139,3 +139,46 @@ def test_pretty_mode_tail_json_keeps_legacy_raw_blob(pinned_renderer, capsys): generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=True) doc = json.loads(capsys.readouterr().out) assert doc["status"] == "succeeded" + + +# --------------------------------------------------------------------------- # +# Review (annehe9) on #809 +# --------------------------------------------------------------------------- # + + +def _failed(**overrides) -> poll.PollResult: + fields = { + "status": "failed", + "error": "partner blew up", + "image_urls": [], + "raw": {"status": "failed", "error": "partner blew up"}, + } + fields.update(overrides) + return poll.PollResult(**fields) + + +def test_failed_job_is_an_ok_false_envelope_and_exit_1_in_json_mode(pinned_renderer): + """A terminally failed job must never be reported as ``ok: true`` — the + registered ``generate_result`` schema promises failures arrive as an + ok=false envelope with ``error.code``, and machine consumers trust ``ok``.""" + import typer + + _, machine, _ = pinned_renderer(OutputMode.JSON) + with pytest.raises(typer.Exit) as e: + generate_app._emit_result(_failed(), request_id="req1", download=None, as_json=True) + assert e.value.exit_code == 1 + envelope = json.loads(machine.getvalue().splitlines()[-1]) + assert envelope["ok"] is False + assert envelope["error"]["code"] == "generate_job_failed" + assert envelope["data"] is None + + +def test_output_json_alone_gets_the_envelope_not_colored_text(pinned_renderer, capsys): + """``comfy --output json generate …`` (no tail ``--json``) must emit the + envelope on the machine stream, never ANSI-colored URLs on stdout.""" + _, machine, _ = pinned_renderer(OutputMode.JSON) + generate_app._emit_result(_succeeded(), request_id="req1", download=None, as_json=False) + envelope = json.loads(machine.getvalue().splitlines()[-1]) + assert envelope["schema"] == "envelope/1" and envelope["ok"] is True + assert envelope["data"]["result"] == {"status": "succeeded", "urls": ["https://cdn.example/img.png"]} + assert "\x1b[" not in capsys.readouterr().out diff --git a/tests/comfy_cli/cql/test_frontend_widget_slots.py b/tests/comfy_cli/cql/test_frontend_widget_slots.py index ea81ffbbd..ba8520fad 100644 --- a/tests/comfy_cli/cql/test_frontend_widget_slots.py +++ b/tests/comfy_cli/cql/test_frontend_widget_slots.py @@ -251,11 +251,23 @@ def test_file_upload_flag_alone_gets_no_button(self, graph: Graph): # own upload and the frontend attaches no IMAGEUPLOAD button. assert "upload" not in graph.widget_order("Load3D") - def test_upload_has_no_add_node_default(self, graph: Graph): - # Current frontends mark the button ``serialize: false``; a fresh node - # must not carry a phantom trailing value for it. + def test_fresh_node_carries_no_phantom_value_for_the_upload_button(self, graph: Graph): + # Current frontends mark the button ``serialize: false``: a fresh node + # from add_node must end at the last serialized value — not carry a + # trailing ``null`` for the injected slot. Asserted on the BUILT node, + # the layer the claim is about (the defaults dict alone is not). assert "upload" not in graph.widget_defaults("LoadImage") - assert graph.widget_defaults("LoadImage") == {"image": "beach.jpg"} + wf, _ = workflow_ops.add_node(_empty_workflow(), graph, "LoadImage") + assert wf["nodes"][0]["widgets_values"] == ["beach.jpg"] + + def test_fresh_3d_nodes_carry_the_frontends_empty_preview_slot(self, graph: Graph): + # The injected PREVIEW_3D ``image`` IS serialized (as ``""``) — the + # captured shapes are SaveGLB ["mesh/ComfyUI", ""], Preview3D + # ["out/mesh.glb", ""] — so add_node must write it, not ``null``. + glb_prefix = graph.widget_defaults("SaveGLB")["filename_prefix"] + for cls, expected in (("SaveGLB", [glb_prefix, ""]), ("Preview3D", ["", ""])): + wf, _ = workflow_ops.add_node(_empty_workflow(), graph, cls) + assert wf["nodes"][0]["widgets_values"] == expected, cls class TestAudioFamily: @@ -273,6 +285,20 @@ def test_server_declared_audio_ui_is_not_injected_twice(self, graph: Graph): def test_marker_slots_have_no_defaults(self, graph: Graph): assert graph.widget_defaults("LoadAudio") == {"audio": "a.wav"} + wf, _ = workflow_ops.add_node(_empty_workflow(), graph, "LoadAudio") + assert wf["nodes"][0]["widgets_values"] == ["a.wav"] + + +def _empty_workflow() -> dict[str, Any]: + return {"last_node_id": 0, "last_link_id": 0, "nodes": [], "links": [], "groups": [], "version": 0.4} + + +class TestCaptureRefusesInjectedSlots: + def test_capture_recipe_refuses_an_injected_slot_as_a_param_up_front(self, graph: Graph): + wf, op = workflow_ops.add_node(_empty_workflow(), graph, "LoadImage") + nid = op["node_id"] + with pytest.raises(workflow_ops.RecipeError, match="upload"): + workflow_ops.capture_recipe(wf, graph, lift={(nid, "upload"): "up"}) class TestDomWidgetInputs: From 1216e1ddb4b4ff5aea6933634739e405a1419066 Mon Sep 17 00:00:00 2001 From: kishore Date: Fri, 28 Aug 2026 17:58:19 -0700 Subject: [PATCH 6/6] test(generate): pin pretty mode with the global --no-json in the two download tests --no-json is a global flag (before the subcommand), as the rest of this file uses it; a tail --no-json is an unknown generate parameter. Co-Authored-By: Claude Fable 5 --- tests/comfy_cli/command/generate/test_app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/comfy_cli/command/generate/test_app.py b/tests/comfy_cli/command/generate/test_app.py index 9f6d9fcd3..a51f2d6dc 100644 --- a/tests/comfy_cli/command/generate/test_app.py +++ b/tests/comfy_cli/command/generate/test_app.py @@ -294,7 +294,7 @@ def test_generate_sync_with_download(runner, api_key, tmp_path, monkeypatch): download = str(tmp_path / "out.png") # --no-json pins pretty mode (CliRunner has no TTY, so the renderer would # otherwise pick JSON and emit the envelope instead of the "Saved" line). - r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--download", download, "--no-json"]) + r = runner.invoke(cli_app, ["--no-json", "generate", "dalle", "--prompt", "x", "--download", download]) assert r.exit_code == 0, r.stdout assert Path(download).exists() assert Path(download).read_bytes() == b"png-bytes" @@ -314,7 +314,7 @@ def test_generate_json_flag(runner, api_key, monkeypatch): def test_generate_download_no_urls(runner, api_key, monkeypatch): resp = httpx.Response(200, json={"data": []}) monkeypatch.setattr(gen_app.client.httpx, "post", lambda *a, **kw: resp) - r = runner.invoke(cli_app, ["generate", "dalle", "--prompt", "x", "--no-json", "--download", "/tmp/x.png"]) + r = runner.invoke(cli_app, ["--no-json", "generate", "dalle", "--prompt", "x", "--download", "/tmp/x.png"]) assert r.exit_code == 0 assert "no image urls" in r.stdout.lower()