v4: Make Fix node (recreate) work when node ids are strings (#3507)

"Fix node (recreate)" rewired the inputs of the new node with
src_node.connect(link.origin_slot, dest.id, input.name).
LGraphNode.connect() looks its target up by id only when the id is a
number, so on a frontend whose node ids are strings the id was taken
for the node itself: the entry threw "findInputSlot is not a function"
and, because the original is removed only afterwards, left the wired
original and an unwired replacement on the canvas.

- Pass the node to connect(), as the output loop below already does
- Add a Playwright spec that clicks the entry in the node context menu
  and checks the page errors and what every KSampler is wired to, with
  node ids as the frontend issues them and with ids rewritten to
  strings
- Add a CHANGELOG entry

Port of #3443 to v4 (#3126).

CHANGELOG.md

@@ -91,6 +91,10 @@ and this project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/
   ComfyUI failed to start with `unrecognized arguments`; quotation marks and
   empty arguments were lost as well. A ComfyUI started as a module (`python -m`)
   from a Python path with a space now restarts too (#3339).
+- In the legacy UI, make `Fix node (recreate)` work on a node with a connected
+  input on frontends whose node ids are strings. It stopped with
+  `findInputSlot is not a function` and left two copies of the node on the
+  canvas: the original, still wired, and an unwired replacement (#3126).
 
 ## [4.3] - 2026-09-19
 

comfyui_manager/js/node_fixer.js

@@ -102,7 +102,7 @@ function node_info_copy(src, dest, connect_both, copy_shape) {
 		if(input.link) {
 			let link = app.graph.links[input.link];
 			let src_node = app.graph.getNodeById(link.origin_id);
-			src_node.connect(link.origin_slot, dest.id, input.name);
+			src_node.connect(link.origin_slot, dest, input.name);
 		}
 	}
 

tests/playwright/README.md

@@ -50,3 +50,4 @@ PORT=8199 npx playwright test --debug
 | `custom-nodes-xss.spec.ts` | Node metadata, Markdown, links, errors, search highlighting |
 | `model-manager-xss.spec.ts` | Model metadata, links, errors, search highlighting |
 | `legacy-message-errors.spec.ts` | Batch rejection IDs, name readability, DOM colors, lookup errors |
+| `legacy-ui-node-fixer.spec.ts` | "Fix node (recreate)" context-menu entry: one node left, links rewired, numeric and string node ids |

tests/playwright/legacy-ui-node-fixer.spec.ts

@@ -0,0 +1,93 @@
+/**
+ * E2E test: "Fix node (recreate)" in the node context menu (js/node_fixer.js).
+ *
+ * Recreating a node must leave one copy of it, wired as the original was. The
+ * entry passed the new node's id to `LGraphNode.connect()`, which looks a
+ * target up by id only when the id is a number. Where node ids are strings it
+ * threw `findInputSlot is not a function` and left the wired original plus an
+ * unwired replacement on the canvas.
+ *
+ * Honesty boundary: where the frontend issues numeric ids (1.45.15, which the
+ * E2E environment pins), the "string" row rewrites the ids to strings itself.
+ * That reproduces the failing call but is a simulation, and it reaches into
+ * `graph._nodes_by_id`. Where ids are already strings (1.57.6) the rewrite
+ * changes nothing and both rows run the real thing.
+ *
+ * Requires ComfyUI running with --enable-manager-legacy-ui on PORT.
+ */
+
+import { test, expect } from '@playwright/test';
+import { waitForComfyUI } from './helpers';
+
+for (const stringIds of [false, true]) {
+  test(`Fix node (recreate) leaves one wired node — ${stringIds ? 'string' : 'frontend-issued'} node ids`, async ({ page }) => {
+    const errors: string[] = [];
+    page.on('pageerror', (error) => errors.push(error.message));
+    await page.goto('/');
+    await waitForComfyUI(page);
+    await page.waitForFunction(() => !!(window as any).LiteGraph?.registered_node_types?.KSampler);
+
+    // CheckpointLoaderSimple -> KSampler -> VAEDecode
+    const original = await page.evaluate((stringIds) => {
+      const { app, LiteGraph } = window as any;
+      const graph = app.canvas.graph;
+      graph.clear();
+      const add = (type: string, x: number) => {
+        const node = LiteGraph.createNode(type);
+        node.pos = [x, 150];
+        graph.add(node);
+        return node;
+      };
+      const loader = add('CheckpointLoaderSimple', 50);
+      const sampler = add('KSampler', 500);
+      const decoder = add('VAEDecode', 900);
+      loader.connect(0, sampler, 'model');
+      sampler.connect(0, decoder, 'samples');
+
+      if (stringIds) {
+        const toStringId = (node: any) => {
+          delete graph._nodes_by_id[node.id];
+          node.id = String(node.id);
+          graph._nodes_by_id[node.id] = node;
+        };
+        graph.nodes.forEach(toStringId);
+        for (const link of graph.links.values()) {
+          link.origin_id = String(link.origin_id);
+          link.target_id = String(link.target_id);
+        }
+        // The menu entry adds the replacement; it needs a string id as well.
+        const graphAdd = graph.add.bind(graph);
+        graph.add = (node: any, ...rest: unknown[]) => {
+          const result = graphAdd(node, ...rest);
+          toStringId(node);
+          return result;
+        };
+      }
+
+      app.canvas.ds.offset = [0, 0];
+      app.canvas.ds.scale = 1;
+      // The canvas finds the node under the pointer among the nodes it last drew.
+      app.canvas.draw(true, true);
+      // A point on the KSampler's title bar.
+      return { id: sampler.id, x: sampler.pos[0] + 60, y: sampler.pos[1] - LiteGraph.NODE_TITLE_HEIGHT / 2 };
+    }, stringIds);
+
+    await page.mouse.click(original.x, original.y, { button: 'right' });
+    await page.locator('.litecontextmenu .litemenu-entry', { hasText: 'Fix node (recreate)' }).click();
+
+    const samplers = await page.evaluate((originalId) => {
+      const graph = (window as any).app.canvas.graph;
+      return graph.nodes.filter((node: any) => node.type === 'KSampler').map((node: any) => ({
+        recreated: node.id !== originalId,
+        model: node.getInputNode(node.findInputSlot('model'))?.type ?? null,
+        latent: (node.getOutputNodes(0) ?? []).map((target: any) => target.type),
+      }));
+    }, original.id);
+
+    // One comparison, so a failure shows the thrown error and the leftover node together.
+    expect({ errors, samplers }).toEqual({
+      errors: [],
+      samplers: [{ recreated: true, model: 'CheckpointLoaderSimple', latent: ['VAEDecode'] }],
+    });
+  });
+}