fix: ComfyUI timeout/resume + websocket wait + multi-server + music tool - #475
Open
nbsumbana-pixel wants to merge 5 commits into
Open
fix: ComfyUI timeout/resume + websocket wait + multi-server + music tool#475nbsumbana-pixel wants to merge 5 commits into
nbsumbana-pixel wants to merge 5 commits into
Conversation
Non-accelerated local-GPU workflows (e.g. Wan 1.3B at 832x480/81-97 frames) routinely took ~1360-1630s, so the old 900s default false-failed real renders that were still completing server-side. Timeout is now a configurable timeout_seconds input (default 3600s), and ComfyUIError carries the prompt_id on error/timeout so a timed-out-but-still-running job can be resumed via resume_prompt_id instead of resubmitted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves the "async generation" open question from the adapter plan. generate() now watches ComfyUI's websocket events (executing/progress/ execution_error) and reacts immediately instead of sleeping between REST polls, with an optional on_progress callback that comfyui_video uses to print step progress on long renders. websocket-client is an optional import; _wait() falls back to the original poll() loop (with the remaining time budget, not a fresh one) when it's unavailable or the connection drops, so resume_prompt_id recovery is unaffected either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nbsumbana-pixel
changed the title
Resolves the "multi-server" open question from the adapter plan. ComfyUIClient(capability="image"|"video") now resolves its server URL from COMFYUI_IMAGE_SERVER_URL / COMFYUI_VIDEO_SERVER_URL first, falling back to the shared COMFYUI_SERVER_URL and then the localhost default — so comfyui_image and comfyui_video can point at separate ComfyUI instances (different GPUs, different model sets) with zero extra config for single-server setups. is_default_url/unavailable_reason() and the setup_offer metadata account for the override. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nbsumbana-pixel
changed the title
Resolves the "music generation" open question from the adapter plan.
Unlike comfyui_image/comfyui_video there is no bundled workflow: ACE-Step's
ComfyUI node interface isn't standardized across custom node packs
(AceStepModelLoader vs native TextEncodeAceStepAudio, etc.), so instead of
picking one pack and breaking for everyone else, comfyui_music always
requires a caller-supplied workflow_json/workflow_path + output_node --
the same override contract image/video offer as an alternative, just
mandatory here. prompt is provenance-only, never injected into the graph.
Routed through the existing registry.get_by_capability("music_generation")
path alongside suno_music/music_gen -- no dedicated selector needed.
ComfyUIClient.generate() now also reads the "audio" output key (what
ComfyUI's native SaveAudio node writes), and gets timeout/resume/websocket-
wait/multi-server support for free via the shared client. Duration is a
best-effort ffprobe probe of the downloaded file since a custom workflow
gives no other way to know it ahead of time.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nbsumbana-pixel
changed the title
ACE-Step v1's node-pack fragmentation turns out to be moot: ComfyUI ships TextEncodeAceStepAudio/EmptyAceStepLatentAudio as native core nodes (comfy_extras/nodes_ace.py), not a third-party pack, and Comfy-Org's own workflow_templates repo has an official ACE-Step-v1 template built from those plus long-stable core nodes. tools/_comfyui/workflows/ace-step-1-t2a.json was built by cross-checking every node's class_type and input names against ComfyUI's own source (nodes_ace.py, nodes_audio.py, nodes_latent.py, nodes.py) rather than trusting the UI-format export directly. comfyui_music now defaults to this bundled workflow: prompt maps to ACE-Step's tags field (matching suno_music's "prompt = music description" convention), lyrics/duration_seconds/steps/cfg/lyrics_strength/seed are all patchable, and missing ace_step_v1_3.5b.safetensors surfaces through the same missing_models contract as image/video. workflow_json/workflow_path + output_node remains available for ACE-Step 1.5, other node packs, or different audio models entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes out all four open questions in
docs/comfyui-adapter-plan.md:timeout_seconds) — non-accelerated local-GPU workflows (e.g. Wan 1.3B at 832x480/81-97 frames) were observed taking ~1360-1630s and false-failing under the old default even though the job was still completing server-side.ComfyUIErrornow carriesprompt_idon both execution errors and timeouts;resume_prompt_idlets a timed-out-but-still-running job be resumed instead of resubmitted.generate()now waits via ComfyUI's websocket feed by default (wait_ws()), reacting toexecuting/execution_errorevents immediately instead of sleeping between REST polls. Optionalon_progresscallback;comfyui_video/comfyui_musicprint step progress on long renders.websocket-clientis an optional import —_wait()falls back to RESTpoll()(with the remaining time budget, not a fresh one) when unavailable.ComfyUIClient(capability="image"|"video"|"music")resolves its server URL from a per-capability env var (COMFYUI_IMAGE_SERVER_URL/COMFYUI_VIDEO_SERVER_URL/COMFYUI_MUSIC_SERVER_URL) first, then the sharedCOMFYUI_SERVER_URL, then the localhost default — so each capability can point at a different ComfyUI instance with zero extra config for single-server setups.comfyui_music(new tool): ships with a bundled ACE-Step v1 (3.5B) text-to-audio workflow. The node-pack fragmentation that originally blocked this turned out to be moot for v1 — ComfyUI shipsTextEncodeAceStepAudio/EmptyAceStepLatentAudioas native core nodes, not a third-party pack.tools/_comfyui/workflows/ace-step-1-t2a.jsonwas built by cross-checking every node'sclass_type/inputs against ComfyUI's own source (nodes_ace.py,nodes_audio.py,nodes_latent.py,nodes.py), not trusting the UI-format export directly.promptmaps to ACE-Step'stagsfield,lyrics/duration_seconds/steps/cfg/lyrics_strength/seedare all patchable, andworkflow_json/workflow_path+output_noderemains available for ACE-Step 1.5 or other node packs. Routed through the existingregistry.get_by_capability("music_generation")path alongsidesuno_music/music_gen.generate()now also reads the"audio"output key (whatSaveAudioMP3/SaveAudiowrite). Gets timeout/resume/websocket-wait/multi-server for free via the shared client.Also updates
docs/comfyui-adapter-plan.mdand.agents/skills/comfyui/SKILL.mdthroughout.Test plan
pytest tests/contracts/test_comfyui_tools.py— 117 passed, covering timeout/resume, websocket completion/error/timeout/fallback paths, multi-server priority/isolation, and the fullcomfyui_musiccontract (bundled-path generation/missing-models/status plus custom-workflow override, sharedTestContractsuite, and a workflow-JSON validity check).pytest tests/ -k "comfy or registry or selector or music"— 183 passed, 1 skipped.🤖 Generated with Claude Code