From 084bb55eb331d5cc0e99a0a183d81fd1ad17b421 Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Fri, 17 Apr 2026 20:09:36 -0700 Subject: [PATCH] fix: clear stale modal setup badge state --- README.md | 1 + docs/LEARNINGS.md | 1 + tests/test_web_modal_toggle.py | 9 +++++++++ web/modal_toggle.js | 17 +++++++++++++++++ 4 files changed, 28 insertions(+) diff --git a/README.md b/README.md index 08b1fd2..9b3a132 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,7 @@ When the full workflow finishes successfully, all Modal-marked nodes return imme Because the Modal queue route can spend noticeable time hashing, syncing, and creating a remote runtime before the prompt is formally queued, the frontend also emits a temporary synthetic running state into ComfyUI's normal queue/execution UI. Those events now mirror ComfyUI's native websocket payload shapes, including the temporary "Waiting for a machine" initialization notification, so the built-in queue indicators stay alive during the preparatory phase instead of looking idle until the backend finally returns the queued prompt id. The extension also now renders its own global activity badge in the ComfyUI UI during Modal setup and remote execution. That badge stays visible even when ComfyUI's stock queue state does not fully reflect the pre-queue setup phase. +The badge now also prunes orphaned prompt entries when a workflow finishes, so stale orange "setup running" indicators do not linger after the remote work is done. For headless remote execution, the worker also installs a minimal `PromptServer.instance` shim before it imports external custom nodes. That keeps route-registering custom node packs from crashing at import time just because the remote worker is not running ComfyUI's full HTTP server. diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 17350dd..d684a40 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -42,6 +42,7 @@ - For remote node-level UI feedback, a streamed Modal generator method is cleaner than a side-channel file or volume poller. Yielding `progress` envelopes directly from the remote execution method lets the local client forward active-node updates into ComfyUI's websocket stream in real time without extra storage commits, reloads, or cleanup logic. - Some custom-node packs register HTTP routes or prompt handlers at import time through `PromptServer.instance`. A headless remote worker still needs to provide a minimal `PromptServer.instance` shim with `routes`, `app`, `supports`, and no-op send methods, or those packs fail during external custom-node initialization before any workflow execution begins. - For good remote UX, one generic "executing" node state is not enough. The frontend needs separate visual states for "setup", "ready in Modal", "currently executing remotely", "finished for this run", and "workflow fully done", with whole-workflow success clearing the temporary remote states back to the normal idle outline. +- The global Modal activity badge needs the same cleanup discipline as the per-node overlays. If a prompt entry survives in badge state after the prompt itself is gone, the UI can get stuck showing an orange "setup running" badge even though the workflow finished; pruning orphaned prompt ids fixes that. - Warm-container caches and loader reuse can conflict with `modal.Volume.reload()`. If a request truly needs a fresh volume reload after uploading new assets, the worker may have to clear warm loader caches, drop reusable prompt executors, unload ComfyUI models, and retry the reload once to get past Modal's "open files preventing the operation" error. - The synthetic frontend events have to match ComfyUI's websocket adapter shapes exactly, not just semantically. In practice that means `status` detail must be the inner status payload, `executing` detail should be the display node id string, and queue-list initialization state benefits from a matching `"Waiting for a machine"` notification payload keyed by prompt id. - Even with correctly-shaped synthetic events, ComfyUI's built-in queue UI may still not present an obvious "workflow running" state during long pre-queue Modal setup. The extension should own a separate global activity badge so users have explicit feedback while the remote runtime is still being prepared. diff --git a/tests/test_web_modal_toggle.py b/tests/test_web_modal_toggle.py index 37bd083..a31a163 100644 --- a/tests/test_web_modal_toggle.py +++ b/tests/test_web_modal_toggle.py @@ -37,6 +37,7 @@ def test_global_modal_status_badge_is_installed() -> None: assert "Modal setup running for" in source assert "Modal workflow running on" in source assert "installGlobalStatusStyles()" in source + assert "function pruneGlobalStatusStates()" in source def test_remote_modal_status_tracks_active_node_ids() -> None: @@ -68,3 +69,11 @@ def test_streamed_modal_progress_takes_precedence_over_proxy_events() -> None: assert "promptState.hasStreamedProgress = true;" in source assert "if (promptState.hasStreamedProgress && phase !== STATE_ERROR)" in source + + +def test_prompt_cleanup_prunes_orphaned_global_status_entries() -> None: + """Workflow cleanup should remove stale global badge states once prompt state is gone.""" + source = _modal_toggle_source() + + assert "pruneGlobalStatusStates();" in source + assert "refreshGlobalStatusElement();" in source diff --git a/web/modal_toggle.js b/web/modal_toggle.js index 38266f6..ae074e7 100644 --- a/web/modal_toggle.js +++ b/web/modal_toggle.js @@ -122,11 +122,24 @@ function ensureGlobalStatusElement() { return element; } +/** + * Remove orphaned global status entries that no longer have any live prompt state. + */ +function pruneGlobalStatusStates() { + for (const promptId of Array.from(modalGlobalStatusStates.keys())) { + if (modalPromptStates.has(promptId) || syntheticPromptUiStates.has(promptId)) { + continue; + } + modalGlobalStatusStates.delete(promptId); + } +} + /** * Return the most important active global Modal state. * @returns {{ phase: string, promptId: string, nodeCount: number } | null} */ function currentGlobalStatus() { + pruneGlobalStatusStates(); if (modalGlobalStatusStates.size === 0) { return null; } @@ -671,6 +684,8 @@ function handleExecutionPhase(event, phase) { function clearPromptRemoteStates(promptId) { const promptState = modalPromptStates.get(promptId); if (!promptState) { + pruneGlobalStatusStates(); + refreshGlobalStatusElement(); return; } for (const remoteNodeId of promptState.remoteNodeIds) { @@ -681,6 +696,8 @@ function clearPromptRemoteStates(promptId) { } } modalPromptStates.delete(promptId); + pruneGlobalStatusStates(); + refreshGlobalStatusElement(); app.graph?.setDirtyCanvas(true, true); }