Prevent disabled LLM nodes from launching Modal

This commit is contained in:
Ken Simpson
2026-08-19 10:33:32 -07:00
parent bd3893d53a
commit 5e910ef272
7 changed files with 132 additions and 5 deletions
+1
View File
@@ -116,6 +116,7 @@ For credential safety, the node accepts only HTTPS `modal.direct` origins, refus
- Leave `Run on Modal` disabled to run the node locally on Apple Silicon through MLX-VLM. This is intended for MacBooks whose unified memory can hold an LLM while heavier image or video nodes run on Modal.
- Enable `Run on Modal` to run the node inside the same Modal GPU worker as its surrounding remote component.
- The visible toggle is authoritative at queue time. A disabled LLM node bypasses Modal setup, deployment, asset sync, and remote dispatch entirely, even if an older saved workflow contains stale remote-marker metadata.
The queue planner treats an unmarked Modal LLM as an ordinary local node, including when it sits between two remote regions. Inputs and outputs cross those region boundaries through the existing transport layer; the model itself stays on the Mac.
+14 -2
View File
@@ -49,6 +49,7 @@ _MODAL_UI_EVENT_LIMIT_PER_CLIENT = 512
_MODAL_UI_EVENTS_LOCK = threading.Lock()
_MODAL_UI_EVENTS_BY_CLIENT: dict[str, deque[dict[str, Any]]] = {}
_MODAL_INTERRUPT_QUEUE_BRIDGE_ATTR = "__comfy_modal_interrupt_queue_bridge_installed"
_REMOTE_TOGGLE_WIDGET_NAME = "Run on Modal"
_TRANSPORTABLE_OUTPUT_TYPES = frozenset(
{
"*",
@@ -1144,8 +1145,7 @@ def extract_remote_node_ids(
marker = (settings or get_settings()).marker_property
remote_node_ids: set[str] = set()
for node, ancestor_node_ids in _iter_workflow_nodes_with_ancestors(workflow):
properties = node.get("properties") or {}
if properties.get(marker):
if _workflow_node_remote_enabled(node, marker):
node_id = str(node.get("id"))
if prompt_node_ids is None:
remote_node_ids.add(node_id)
@@ -1167,6 +1167,18 @@ def extract_remote_node_ids(
return remote_node_ids
def _workflow_node_remote_enabled(node: Mapping[str, Any], marker: str) -> bool:
"""Return the visible remote toggle, falling back to the saved marker property."""
named_widget_values = node.get("widgets_values_named")
if isinstance(named_widget_values, Mapping):
visible_toggle_value = named_widget_values.get(_REMOTE_TOGGLE_WIDGET_NAME)
if isinstance(visible_toggle_value, bool):
return visible_toggle_value
properties = node.get("properties")
return isinstance(properties, Mapping) and bool(properties.get(marker))
def _normalize_output_metadata(node_class: type[Any]) -> tuple[tuple[str, ...], tuple[str, ...], tuple[bool, ...]]:
"""Return normalized output metadata for a node class."""
if hasattr(node_class, "GET_SCHEMA"):
+1
View File
@@ -72,6 +72,7 @@
- The Modal status pill should be driven by actual remote work, not by the custom node route being installed. Since the frontend patches every prompt through `/modal/queue_prompt`, the backend must fast-path prompts with zero `Run on Modal` nodes without emitting `modal_status`, and the frontend should ignore any empty Modal status event as a defensive guard.
- A visibility/focus redraw is not enough if the browser missed Modal websocket events while backgrounded. The local ComfyUI side needs to retain recent client-scoped `modal_status` and `modal_progress` payloads, and the frontend should replay that buffer on refocus before redrawing the status pill and node overlays.
- Programmatic context-menu changes to `properties.is_modal_remote` also have to update the live `Run on Modal` widget value. Otherwise the saved workflow state and node border can be correct while the visible toggle still appears disabled.
- ComfyUI restores saved widget values after `nodeCreated`, so the inverse mismatch is also possible: the visible `Run on Modal` widget can be disabled while an older `properties.is_modal_remote` value remains true. Reconcile the property in `loadedGraphNode`/`afterConfigureGraph`, treat `widgets_values_named["Run on Modal"]` as authoritative at queue time, and keep the backend guard so stale metadata cannot launch Modal.
- Graph-wide Modal enablement should operate on live eligible nodes, not just serialized workflow JSON. Scanning the root graph plus nested subgraphs lets the action update both node properties and visible widgets in one pass.
- Once Modal has several node context actions, putting them under one `Modal` submenu keeps labels short and prevents repeated prefixes from cluttering the main node menu.
+1 -1
View File
@@ -1,6 +1,6 @@
[project]
name = "modal-sync"
version = "0.2.2"
version = "0.2.3"
description = "ComfyUI nodes for Modal-backed workflow execution and hosted model endpoints."
readme = "README.md"
requires-python = ">=3.11"
+34
View File
@@ -49,6 +49,8 @@ const transformedSource = `${[
" markPromptTerminal,",
" patchQueuePrompt,",
" setRemoteFlag,",
" synchronizeRemoteFlagFromWidget,",
" extractRemoteNodeIds,",
" setSandwichedLocalNodeIds,",
" isSandwichedLocalNode,",
" setAllEligibleWorkflowNodesRemote,",
@@ -338,6 +340,38 @@ modalToggle.setRemoteFlag(toggleSyncNode, false);
assert.equal(toggleSyncNode.properties.is_modal_remote, false);
assert.equal(toggleSyncNode.__modalToggleWidget.value, false);
const restoredToggleNode = {
comfyClass: "ModalLLM",
properties: { is_modal_remote: true },
__modalToggleWidget: { value: false },
};
modalToggle.synchronizeRemoteFlagFromWidget(restoredToggleNode);
assert.equal(restoredToggleNode.properties.is_modal_remote, false);
assert.deepEqual(
modalToggle.extractRemoteNodeIds({
nodes: [
{
id: 9,
properties: { is_modal_remote: true },
widgets_values_named: { "Run on Modal": false },
},
],
}),
[],
);
assert.deepEqual(
modalToggle.extractRemoteNodeIds({
nodes: [
{
id: 9,
properties: { is_modal_remote: false },
widgets_values_named: { "Run on Modal": true },
},
],
}),
["9"],
);
const eligibleNode = {
id: "eligible",
comfyClass: "KSampler",
+40
View File
@@ -3084,6 +3084,46 @@ def test_extract_remote_node_ids_recurses_into_nested_subgraph_workflows(
) == {"100:11"}
def test_extract_remote_node_ids_prefers_visible_toggle_over_stale_property(
api_intercept_module: Any,
settings_module: Any,
) -> None:
"""A restored disabled widget must prevent stale metadata from starting Modal."""
settings = settings_module.ModalSyncSettings(
app_name="app",
auto_deploy=True,
allow_ephemeral_fallback=False,
enable_memory_snapshot=True,
enable_gpu_memory_snapshot=False,
execution_mode="local",
sync_custom_nodes=False,
volume_name="volume",
route_path="/modal/queue_prompt",
marker_property="is_modal_remote",
local_storage_root=Path("/tmp/storage"),
remote_storage_root="/storage",
custom_nodes_archive_name="custom_nodes_bundle.zip",
comfyui_root=None,
custom_nodes_dir=Path("/tmp/custom_nodes"),
)
workflow = {
"nodes": [
{
"id": 9,
"properties": {"is_modal_remote": True},
"widgets_values_named": {"Run on Modal": False},
},
{
"id": 10,
"properties": {"is_modal_remote": False},
"widgets_values_named": {"Run on Modal": True},
},
]
}
assert api_intercept_module.extract_remote_node_ids(workflow, settings) == {"10"}
def test_extract_remote_node_ids_maps_subgraph_container_to_descendant_prompt_nodes(
api_intercept_module: Any,
settings_module: Any,
+41 -2
View File
@@ -2,6 +2,7 @@ import { app } from "../../scripts/app.js";
import { PromptExecutionError, api } from "../../scripts/api.js";
const REMOTE_PROPERTY = "is_modal_remote";
const REMOTE_WIDGET_NAME = "Run on Modal";
const MODAL_ROUTE = "/modal/queue_prompt";
const MODAL_ANALYZE_ROUTE = MODAL_ROUTE.replace(/\/queue_prompt$/, "/analyze_remote_nodes");
const MODAL_PROGRESS_STATE_ROUTE = MODAL_ROUTE.replace(/\/queue_prompt$/, "/progress_state");
@@ -2692,13 +2693,26 @@ function laneOwnerKey(promptId, nodeIdValue, laneId) {
function extractRemoteNodeIds(workflow) {
const remoteNodeIds = [];
for (const node of workflow?.nodes ?? []) {
if (node?.properties?.[REMOTE_PROPERTY]) {
if (serializedRemoteFlag(node)) {
remoteNodeIds.push(String(node.id));
}
}
return remoteNodeIds;
}
/**
* Return the user-visible Modal toggle value from a serialized workflow node.
* @param {object | undefined} node
* @returns {boolean}
*/
function serializedRemoteFlag(node) {
const namedWidgetValue = node?.widgets_values_named?.[REMOTE_WIDGET_NAME];
if (typeof namedWidgetValue === "boolean") {
return namedWidgetValue;
}
return Boolean(node?.properties?.[REMOTE_PROPERTY]);
}
/**
* Return the root workflow graph, including subgraphs when available.
* @returns {LGraph | null}
@@ -3208,6 +3222,24 @@ function setRemoteFlag(node, value) {
refreshNodeDecorations();
}
/**
* Reconcile a restored node property with the toggle value displayed to the user.
* @param {LGraphNode} node
*/
function synchronizeRemoteFlagFromWidget(node) {
if (!isEligibleNode(node) || !node.__modalToggleWidget) {
return;
}
const enabled = Boolean(node.__modalToggleWidget.value);
node.properties ||= {};
if (node.properties[REMOTE_PROPERTY] === enabled) {
return;
}
node.properties[REMOTE_PROPERTY] = enabled;
clearSandwichedLocalNodeWarnings();
refreshNodeDecorations();
}
/**
* Return the current remote visual state for a node.
* @param {LGraphNode} node
@@ -3896,7 +3928,7 @@ function decorateNode(node) {
const widget = node.addWidget(
"toggle",
"Run on Modal",
REMOTE_WIDGET_NAME,
node.properties[REMOTE_PROPERTY],
(value) => setRemoteFlag(node, value),
{
@@ -4787,7 +4819,14 @@ app.registerExtension({
decorateNode(node);
},
async loadedGraphNode(node) {
synchronizeRemoteFlagFromWidget(node);
},
async afterConfigureGraph() {
for (const node of allWorkflowNodes()) {
synchronizeRemoteFlagFromWidget(node);
}
selectedModalGpu();
},
});