From deecfe84ed16653edf69f43134ae162e50db2740 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 24 Jul 2026 14:17:34 -0700 Subject: [PATCH 1/2] fix(download): resolve SaveGLB 3d/mesh outputs in extract_output_entries (BE-4417) comfy download --where cloud returned download_no_outputs for succeeded partner-3D jobs (Tripo 3.1, Rodin Gen 2.5): the SaveGLB node emits its entries under the "3d" key (ui={"3d": results}), but extract_output_entries only scanned the fixed allowlist (images, gifs, videos, audio, files), so the .glb was never resolved and transfer.py emitted download_no_outputs. Replace the hardcoded allowlist with shape-based detection mirroring ComfyUI core's normalize_outputs and the cloud worker's isFileOutputArray: any node-output key whose value is a list of dicts carrying a filename is treated as file outputs, with "animated" explicitly skipped (it emits (True,) flags). This is a strict superset of the prior five keys and fixes both the job-watcher state-file path and the API fallback, which share the extractor. --- comfy_cli/comfy_client.py | 18 +++++++--- tests/comfy_cli/cloud/test_client.py | 34 ++++++++++++++++++ .../command/test_transfer_download.py | 36 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/comfy_cli/comfy_client.py b/comfy_cli/comfy_client.py index 02b8c8eb..0076245e 100644 --- a/comfy_cli/comfy_client.py +++ b/comfy_cli/comfy_client.py @@ -533,7 +533,18 @@ def extract_output_entries(record: dict) -> list[dict]: (e.g. ``comfy download`` reading a state file) can join them back to producing nodes on the (filename, subfolder, type) triple — the same one :meth:`Client.view_url` encodes as query params. Ordering is stable: - record ``outputs`` insertion order, then media-key order, then item order. + record ``outputs`` insertion order, then node-output key insertion order, + then item order. + + Detection is shape-based, mirroring ComfyUI core's ``normalize_outputs`` + and the cloud worker's ``isFileOutputArray``: any node-output key whose + value is a list of dicts carrying a ``filename`` is treated as file + outputs. This deliberately covers keys beyond the classic + ``images/gifs/videos/audio/files`` — notably SaveGLB's ``"3d"`` key and + the cloud worker's singular ``"video"`` key — so 3D/mesh jobs resolve + instead of returning ``download_no_outputs`` (BE-4417). The ``"animated"`` + key is skipped explicitly to match core semantics (it emits ``(True,)`` + boolean flags, not file entries). """ results: list[dict] = [] outputs = record.get("outputs") or {} @@ -542,9 +553,8 @@ def extract_output_entries(record: dict) -> list[dict]: for node_id, node_output in outputs.items(): if not isinstance(node_output, dict): continue - for key in ("images", "gifs", "videos", "audio", "files"): - items = node_output.get(key) or [] - if not isinstance(items, list): + for key, items in node_output.items(): + if key == "animated" or not isinstance(items, list): continue for item in items: if isinstance(item, dict) and "filename" in item: diff --git a/tests/comfy_cli/cloud/test_client.py b/tests/comfy_cli/cloud/test_client.py index 1f6c59d3..ee6f1775 100644 --- a/tests/comfy_cli/cloud/test_client.py +++ b/tests/comfy_cli/cloud/test_client.py @@ -641,6 +641,40 @@ def test_extract_output_urls_delegates_to_extract_outputs(self): client = comfy_client.Client(CLOUD) assert client.extract_output_urls(record) == [o["url"] for o in client.extract_outputs(record)] + def test_extract_resolves_saveglb_3d_key(self): + """SaveGLB emits entries under the "3d" key, not the classic media + keys — shape-based detection must still resolve them (BE-4417).""" + record = {"outputs": {"10": {"3d": [{"filename": "abc123.glb", "subfolder": "3d", "type": "output"}]}}} + assert comfy_client.extract_output_entries(record) == [ + {"node_id": "10", "filename": "abc123.glb", "subfolder": "3d", "type": "output"} + ] + urls = comfy_client.Client(CLOUD).extract_output_urls(record) + assert urls == ["https://cloud.example.com/api/view?filename=abc123.glb&subfolder=3d&type=output"] + + def test_extract_resolves_singular_video_key(self): + """The cloud worker's synthesizeMediaTypeFromResult emits a singular + "video" key; shape-based detection covers it too.""" + record = {"outputs": {"7": {"video": [{"filename": "clip.webm", "subfolder": "", "type": "output"}]}}} + assert comfy_client.extract_output_entries(record) == [ + {"node_id": "7", "filename": "clip.webm", "subfolder": "", "type": "output"} + ] + + def test_extract_skips_non_file_shaped_keys(self): + """Keys carrying flags/metadata rather than file entries produce + nothing: "animated" (boolean flags — explicitly skipped), "text" + (strings), "dims" (dicts without a filename).""" + record = { + "outputs": { + "3": { + "animated": [True], + "text": ["hello"], + "dims": [{"width": 512}], + } + } + } + assert comfy_client.extract_output_entries(record) == [] + assert comfy_client.Client(CLOUD).extract_output_urls(record) == [] + class TestGroupOutputs: """_group_outputs: pure grouping of extract_outputs entries by node and diff --git a/tests/comfy_cli/command/test_transfer_download.py b/tests/comfy_cli/command/test_transfer_download.py index 251fc6cb..05dee7e7 100644 --- a/tests/comfy_cli/command/test_transfer_download.py +++ b/tests/comfy_cli/command/test_transfer_download.py @@ -232,6 +232,42 @@ def test_client_extract_outputs_delegates(self, fake_target): assert outputs[0]["url"] == "https://cloud.example.com/api/view?filename=ComfyUI_a.png&subfolder=&type=output" +GLB_RECORD = { + "status": {"completed": True, "status_str": "success"}, + "outputs": {"10": {"3d": [{"filename": "abc123.glb", "subfolder": "3d", "type": "output"}]}}, +} + + +class TestSaveGlb3dDownload: + """Regression (BE-4417): a SaveGLB job emits its output under the "3d" + key. Shape-based extraction must resolve it so `comfy download` saves the + `.glb` instead of erroring `download_no_outputs`.""" + + def test_3d_only_record_downloads_glb(self, fake_target, tmp_path, capsys): + urls = comfy_client.Client(fake_target).extract_output_urls(GLB_RECORD) + assert urls == ["https://cloud.example.com/api/view?filename=abc123.glb&subfolder=3d&type=output"] + state = jobs_state.JobState( + prompt_id=PROMPT_ID, + client_id=None, + workflow="/abs/mesh.json", + where="cloud", + base_url=fake_target.base_url, + status="completed", + outputs=urls, + record=GLB_RECORD, + item_map=None, + ) + assert jobs_state.write(state) is not None + + paths, data = _run_download(fake_target, tmp_path, capsys) + + assert len(paths) == 1 + # Extension derives from the URL's filename query param → `.glb`. + assert Path(paths[0]).suffix == ".glb" + assert Path(paths[0]).is_file() + assert data["files"][0]["node_id"] == "10" + + class TestCollisionSafeNaming: """A retry fan-out reusing the same item ids re-downloads into the same out-dir; attempt 1 must never be silently clobbered (fennec friction #3, From ae140b1a9a23c09fda7f042fa65d73f55dc847c2 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 24 Jul 2026 14:36:40 -0700 Subject: [PATCH 2/2] fix(download): de-dup output entries on the full (node,filename,subfolder,type) tuple (BE-4417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A node that surfaces the same artifact under two keys — e.g. the cloud worker's singular "video" alongside the classic plural "videos" — would emit two identical /view URLs and download the same file twice. De-dup on the full entry tuple, first occurrence wins, preserving order. Addresses the Cursor review de-dup finding on #600. Co-Authored-By: Claude Opus 4.8 --- comfy_cli/comfy_client.py | 26 ++++++++++++++++++-------- tests/comfy_cli/cloud/test_client.py | 16 ++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/comfy_cli/comfy_client.py b/comfy_cli/comfy_client.py index 0076245e..69cef1e1 100644 --- a/comfy_cli/comfy_client.py +++ b/comfy_cli/comfy_client.py @@ -545,8 +545,15 @@ def extract_output_entries(record: dict) -> list[dict]: instead of returning ``download_no_outputs`` (BE-4417). The ``"animated"`` key is skipped explicitly to match core semantics (it emits ``(True,)`` boolean flags, not file entries). + + Entries are de-duplicated on the full ``(node_id, filename, subfolder, + type)`` tuple, first occurrence wins. A node that surfaces the same + artifact under two keys — e.g. the cloud worker's singular ``"video"`` + alongside the classic plural ``"videos"`` — would otherwise yield two + identical ``/view`` URLs and download the same file twice. """ results: list[dict] = [] + seen: set[tuple[str, str, str, str]] = set() outputs = record.get("outputs") or {} if not isinstance(outputs, dict): return results @@ -558,14 +565,17 @@ def extract_output_entries(record: dict) -> list[dict]: continue for item in items: if isinstance(item, dict) and "filename" in item: - results.append( - { - "node_id": str(node_id), - "filename": str(item.get("filename", "")), - "subfolder": str(item.get("subfolder", "")), - "type": str(item.get("type", "output")), - } - ) + entry = { + "node_id": str(node_id), + "filename": str(item.get("filename", "")), + "subfolder": str(item.get("subfolder", "")), + "type": str(item.get("type", "output")), + } + dedup_key = (entry["node_id"], entry["filename"], entry["subfolder"], entry["type"]) + if dedup_key in seen: + continue + seen.add(dedup_key) + results.append(entry) return results diff --git a/tests/comfy_cli/cloud/test_client.py b/tests/comfy_cli/cloud/test_client.py index ee6f1775..e0da5b40 100644 --- a/tests/comfy_cli/cloud/test_client.py +++ b/tests/comfy_cli/cloud/test_client.py @@ -659,6 +659,22 @@ def test_extract_resolves_singular_video_key(self): {"node_id": "7", "filename": "clip.webm", "subfolder": "", "type": "output"} ] + def test_extract_dedups_same_artifact_under_two_keys(self): + """A node that surfaces the same artifact under both the singular + "video" and plural "videos" keys must emit one entry, not two — else + the /view URL (and the downloaded file) is duplicated.""" + record = { + "outputs": { + "7": { + "video": [{"filename": "clip.webm", "subfolder": "", "type": "output"}], + "videos": [{"filename": "clip.webm", "subfolder": "", "type": "output"}], + } + } + } + assert comfy_client.extract_output_entries(record) == [ + {"node_id": "7", "filename": "clip.webm", "subfolder": "", "type": "output"} + ] + def test_extract_skips_non_file_shaped_keys(self): """Keys carrying flags/metadata rather than file entries produce nothing: "animated" (boolean flags — explicitly skipped), "text"