Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ history.
(`connect 70/2005.0 70/2011.text`) and emits a `connect` op carrying the
instance `path`. A link that crosses the subgraph boundary is still refused,
and so is a link inside a definition shared by several instances.
- `comfy workflow delete-node` and `delete-nodes` accept the interior paths
`workflow print` shows (`delete-node 70/2012`). They delete the node from its
subgraph definition with its interior links, and emit a `delete_node` op
carrying the instance `path`. A definition shared by several instances, or a
node that backs a promoted widget, is refused without writing. Replaying the
op needs a doc host on comfy-multi-player 0.3.12 or later: older appliers
ignore `path` and delete the top-level node with the same id.
- `comfy workflow disconnect <file> <node>.<input>` removes an input link and
emits a replayable `disconnect` op. Concrete, promoted, widget-converted,
autogrow, and eligible interior-subgraph inputs retain stable last-write-wins
Expand Down
4 changes: 2 additions & 2 deletions comfy_cli/command/workflow_edit.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ def disconnect_cmd(
@tracking.track_command("workflow")
def delete_cmd(
file: Annotated[str, typer.Argument(help="Frontend-format workflow JSON.")],
node: Annotated[str, typer.Argument(help="Node id to delete.")],
node: Annotated[str, typer.Argument(help="Node id or interior path to delete (for example 70/22).")],
actor: ActorOpt = "cli",
base_version: BaseVersionOpt = 0,
stdout: StdoutOpt = False,
Expand Down Expand Up @@ -517,7 +517,7 @@ def delete_cmd(
@tracking.track_command("workflow")
def delete_nodes_cmd(
file: Annotated[str, typer.Argument(help="Frontend-format workflow JSON.")],
nodes: Annotated[list[str], typer.Argument(help="Node ids to delete (one or more).")],
nodes: Annotated[list[str], typer.Argument(help="Node ids or interior paths to delete (one or more).")],
actor: ActorOpt = "cli",
base_version: BaseVersionOpt = 0,
stdout: StdoutOpt = False,
Expand Down
12 changes: 8 additions & 4 deletions comfy_cli/skills/comfy/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -483,10 +483,14 @@ comfy --json download --out-dir ./out < run.json # pull the
in place.
- **`delete-node` ≠ `delete`:** `delete-node` removes a *node from the graph file*;
`comfy workflow delete` deletes a *saved workflow from Comfy Cloud*. Do not confuse them.
- `add-node`/`connect`/`delete-node` operate on **top-level** nodes only.
`set-widget` additionally resolves values **inside a subgraph** directly — use
the flat promoted address `slots` advertises (e.g. `57.text`) or the nested
form (`57/27.text`); no decompose needed.
- `add-node` operates on **top-level** nodes only. `connect` (between two
nodes of the same subgraph) and `delete-node`/`delete-nodes` also accept the
interior addresses `workflow print` shows (e.g. `70/22`), but only inside a
subgraph definition used by a single instance; a shared definition is
refused rather than edited in every instance. `set-widget` additionally
resolves values **inside a subgraph** directly — use the flat promoted
address `slots` advertises (e.g. `57.text`) or the nested form
(`57/27.text`); no decompose needed.
- **`set-node-field` writes one durable, per-node scalar field** — `title`,
`mode`, `flags.collapsed` or `flags.pinned` — (or `--clear`s it back to
absent). It needs no catalog — none of those four is a widget — and is the
Expand Down
177 changes: 172 additions & 5 deletions comfy_cli/workflow_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,11 @@ def __init__(self, node_id: Any, class_type: str, widget: str, *, interior: bool
#: delete_node accepts (not the spelling the caller typed).
self.node_id = node_id
if interior:
# delete_node only removes top-level nodes, and insert_workflow only
# adds at the root, so there is no command route to replace it.
# insert_workflow only adds at the root, so even where delete_node
# can remove an interior note there is no command route to replace it.
self.hint = (
f"{node_id} is a note inside a subgraph definition: delete-node only removes top-level nodes, "
"so its text can only be changed in the ComfyUI editor"
f"{node_id} is a note inside a subgraph definition: no command can insert a replacement inside "
"a subgraph, so its text can only be changed in the ComfyUI editor"
)
else:
self.hint = (
Expand Down Expand Up @@ -1343,6 +1343,27 @@ def _navigate_subgraph_path(workflow: dict, segments: list[str]) -> dict:
return node


def _interior_path_deleted(workflow: dict, segments: list[str]) -> bool:
"""True when ``segments`` stops at an interior node that is absent from an
existing definition — the shape an interior ``delete_node`` leaves behind,
as opposed to a path that was never valid."""
from comfy_cli.cql import engine as _engine

defs_by_id = _engine._subgraph_defs_by_id(workflow)
node = _find_by_str(workflow, segments[0])
for seg in segments[1:]:
sg = defs_by_id.get(str((node or {}).get("type", ""))) if isinstance(node, dict) else None
if sg is None:
return False
node = next(
(n for n in sg.get("nodes") or [] if isinstance(n, dict) and str(n.get("id", "")) == str(seg)),
None,
)
if node is None:
return True
return False


def _subgraph_boundary_error(workflow: dict, node_id: Any) -> ValueError | None:
"""Explain a connect endpoint that addresses a subgraph interior.

Expand Down Expand Up @@ -1771,6 +1792,76 @@ def _interior_disconnect_scope(workflow: dict, to_node: Any) -> dict | None:
return {"path": [str(seg) for seg in path], "definition": definition, "dst": dst}


def _interior_delete_scope(workflow: dict, node_id: Any) -> dict | None:
"""Resolve an interior node delete to its owning single-instance definition."""
from comfy_cli.cql import engine as _engine

text = str(node_id)
if _find_by_str(workflow, text) is not None:
return None
if _engine._SUBGRAPH_PATH_SEP in text:
segments = _engine.split_node_path(workflow, text)
elif ":" in text:
segments = text.split(":")
else:
return None
if len(segments) < 2:
return None
path = segments[:-1]
try:
host = _navigate_subgraph_path(workflow, path)
except ValueError:
return None
definition = _engine._subgraph_defs_by_id(workflow).get(str(host.get("type", "")))
if definition is None:
return None
target = next(
(
node
for node in definition.get("nodes") or []
if isinstance(node, dict) and str(node.get("id")) == str(segments[-1])
),
None,
)
if target is None:
return None
instances = _definition_instance_count(workflow, str(host.get("type", "")))
if instances > 1:
raise ValueError(
f"subgraph {'/'.join(path)} shares its definition {definition.get('id')} with "
f"{instances} instances, so deleting a node inside it would mutate all {instances} at once — "
"delete-node refuses that; edit a single-instance copy (unpack or duplicate the subgraph in the editor)"
)
# A promoted widget's value lives positionally in the host's
# widgets_values (or, for a legacy promotion, in proxyWidgets order).
# Deleting its source would shift every later value onto the wrong input,
# and neither this CLI nor the shared applier realigns the host, so refuse.
from comfy_cli.cql import promoted as _promoted

target_id = str(target.get("id"))
backed = [
p.name
for p in _promoted.promoted_inputs(definition, _engine._subgraph_defs_by_id(workflow))
if p.is_widget and str(p.source_node) == target_id
]
backed += [
str(entry[1])
for entry in (host.get("properties") or {}).get("proxyWidgets") or []
if isinstance(entry, list) and len(entry) >= 2 and str(entry[0]) == target_id
]
if backed:
raise ValueError(
f"node {'/'.join([*path, target_id])} backs the promoted widget(s) {sorted(set(backed))} of subgraph "
f"{'/'.join(path)}; deleting it would shift the subgraph's remaining widget values onto the wrong "
"inputs — delete-node refuses that; remove the promotion in the editor first"
)
return {
"path": [str(seg) for seg in path],
"definition": definition,
"target": target,
}


def _definition_instance_count(workflow: dict, def_id: str) -> int:
"""How many live instances of definition ``def_id`` the workflow holds.

Expand Down Expand Up @@ -2300,6 +2391,24 @@ def _delete_node_impl(
actor: str = "cli",
base_version: int = 0,
) -> tuple[dict, dict]:
interior = _interior_delete_scope(workflow, node_id)
if interior is not None:
target_id = interior["target"]["id"]
removed = [
_interior_link_id(link)
for link in interior["definition"].get("links") or []
if str(_interior_link_endpoints(link)[0]) == str(target_id)
or str(_interior_link_endpoints(link)[1]) == str(target_id)
]
op = _new_op(
"delete_node",
actor,
base_version,
path=interior["path"],
Comment thread
christian-byrne marked this conversation as resolved.
node_id=target_id,
removed_links=removed,
)
return apply_op(workflow, op, graph), op
_require(workflow, node_id)
removed = [ln[0] for ln in workflow.get("links") or [] if ln[1] == node_id or ln[3] == node_id]
op = _new_op("delete_node", actor, base_version, node_id=node_id, removed_links=removed)
Expand Down Expand Up @@ -3056,7 +3165,12 @@ def _apply_set_widget(workflow: dict, op: dict, graph) -> bool | None:
# Validate against the current definition before the mutating resolver
# forks shared ancestors. Imported/replayed malformed ops must be
# atomic: a rejected widget cannot leave an unreachable fork behind.
preflight = _navigate_subgraph_path(workflow, [str(s) for s in path])
try:
preflight = _navigate_subgraph_path(workflow, [str(s) for s in path])
except ValueError:
if _interior_path_deleted(workflow, [str(s) for s in path]):
return # interior node concurrently deleted => no-op (delete wins)
raise
preflight_warnings = _engine._write_widget(
copy.deepcopy(preflight), op["inner_widget"], op["value"], graph, extend=False
)
Expand Down Expand Up @@ -3557,6 +3671,9 @@ def _apply_interior_disconnect(workflow: dict, op: dict) -> None:


def _apply_delete_node(workflow: dict, op: dict) -> None:
if op.get("path"):
_apply_interior_delete_node(workflow, op)
return
node_id = str(op["node_id"]) # node identity is compared as a string (amendment v1.2)
workflow["nodes"] = [n for n in workflow.get("nodes") or [] if str(n.get("id")) != node_id]
removed = set(op.get("removed_links") or [])
Expand All @@ -3583,6 +3700,54 @@ def _apply_delete_node(workflow: dict, op: dict) -> None:
workflow.pop("_pending_dynamic_connects", None)


def _apply_interior_delete_node(workflow: dict, op: dict) -> None:
Comment thread
christian-byrne marked this conversation as resolved.
"""Apply a path-scoped delete inside one single-instance definition."""
from comfy_cli.cql import engine as _engine

try:
host = _navigate_subgraph_path(workflow, [str(seg) for seg in op["path"]])
except ValueError:
return
definition = _engine._subgraph_defs_by_id(workflow).get(str(host.get("type", "")))
if definition is None or _definition_instance_count(workflow, str(host.get("type", ""))) > 1:
Comment thread
christian-byrne marked this conversation as resolved.
return
Comment thread
coderabbitai[bot] marked this conversation as resolved.
node_id = str(op["node_id"])
definition["nodes"] = [
node
for node in definition.get("nodes") or []
if not (isinstance(node, dict) and str(node.get("id")) == node_id)
]
removed = {str(link_id) for link_id in op.get("removed_links") or []}
kept = []
for link in definition.get("links") or []:
origin_id, target_id, _ = _interior_link_endpoints(link)
if str(_interior_link_id(link)) in removed or str(origin_id) == node_id or str(target_id) == node_id:
removed.add(str(_interior_link_id(link)))
continue
kept.append(link)
definition["links"] = kept
kept_ids = {str(_interior_link_id(link)) for link in kept}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# Links from the input proxy / into the output proxy are also listed on the
# definition's boundary slots; a stale id there leaves the slot "connected"
# to a link that no longer exists.
for boundary in [*(definition.get("inputs") or []), *(definition.get("outputs") or [])]:
Comment thread
christian-byrne marked this conversation as resolved.
if isinstance(boundary, dict) and isinstance(boundary.get("linkIds"), list):
boundary["linkIds"] = [link_id for link_id in boundary["linkIds"] if str(link_id) not in removed]
for node in definition.get("nodes") or []:
if not isinstance(node, dict):
continue
for inp in node.get("inputs") or []:
Comment thread
christian-byrne marked this conversation as resolved.
if isinstance(inp, dict) and inp.get("link") is not None and str(inp["link"]) not in kept_ids:
inp["link"] = None
for out in node.get("outputs") or []:
if isinstance(out, dict) and isinstance(out.get("links"), list):
out["links"] = [link_id for link_id in out["links"] if str(link_id) in kept_ids]
if isinstance(definition.get("node_order"), list):
definition["node_order"] = [value for value in definition["node_order"] if str(value) != node_id]
if isinstance(definition.get("link_order"), list):
definition["link_order"] = [value for value in definition["link_order"] if str(value) in kept_ids]


def _apply_clear(workflow: dict, op: dict) -> None:
workflow["nodes"] = []
workflow["links"] = []
Expand Down Expand Up @@ -3649,6 +3814,8 @@ def _write_target(op: dict) -> tuple:
# neither collides with a same-named widget's register.
return ("node_field", str(op["node_id"]), op["field"])
if kind in ("add_node", "delete_node"):
if op.get("path"):
return ("node", tuple(str(s) for s in op["path"]), str(op["node_id"]))
return ("node", str(op["node_id"]))
if kind in ("connect", "disconnect"):
grow = op.get("grow")
Expand Down
26 changes: 21 additions & 5 deletions docs/op-vocabulary-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,19 @@ Spec form:
```

Minted op fields: `node_id`, `removed_links` (ids of every link incident to the
node at mint time). Replay removes the node, drops incident links (both the
recorded ones and any link whose endpoint is the node), and scrubs dangling
input/output references.
node at mint time). An interior address such as `70/22` additionally carries
the owner-instance `path` (`["70"]`); it is refused when the definition is
shared by multiple live instances, or when the node is the source of a
promoted widget (its host value would shift onto the wrong input). Replay
removes the node, drops incident links (both the recorded ones and any link
whose endpoint is the node), and scrubs dangling input/output references in
that same graph scope — for an interior delete, including the definition's
boundary `inputs[].linkIds` and `outputs[].linkIds`.

An interior delete reuses the `delete_node` kind, so a consumer that predates
`path` cannot reject it. comfy-multi-player 0.3.11 and earlier ignore `path`,
remove the top-level node with the same id, and report the op `applied`. Send
interior deletes only to a doc host on comfy-multi-player 0.3.12 or later.

* Idempotency: `op_id` no-op; replaying a delete of an already-absent node is a
no-op.
Expand Down Expand Up @@ -556,8 +566,14 @@ Current contract, pinned:
a promoted-widget target is still rejected with "promoted widget (a value),
not a link input". Both operations refuse a shared definition rather than
silently rewiring every instance that uses it.
* **`add_node` and `delete_node` cannot address interior nodes** at all; an
interior id fails as node-not-found against the top-level inventory.
* **`delete_node` permits an interior node in one supported, single-instance
definition.** Its op carries the owner-instance `path`; its node-presence
identity is scoped by that path, and replay removes incident definition
links plus their endpoint references. It refuses a shared definition rather
than deleting from every instance, and refuses a node that backs a promoted
widget. A later interior `set_widget` on the deleted node is a no-op (delete
wins). `add_node` still cannot address an interior location from the CLI
authoring surface.
* An interior **widget** write to a shared definition forks the definition at
apply time (`engine._isolate_shared_subgraph`): the definition is deep-copied
under `"sg-" + sha256(def_id + "\x00" + instance_id)[:32]` — deterministic,
Expand Down
Loading
Loading