From 175e085fcf15de27a6bed56fd7345911da612d27 Mon Sep 17 00:00:00 2001 From: "Dr.Lt.Data" <128333288+ltdrdata@users.noreply.github.com> Date: Sat, 14 Nov 2026 05:38:57 +0900 Subject: [PATCH] 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 | 4 + comfyui_manager/js/node_fixer.js | 2 +- tests/playwright/README.md | 1 + tests/playwright/legacy-ui-node-fixer.spec.ts | 93 +++++++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/playwright/legacy-ui-node-fixer.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 32f44306..b1b070ee 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/comfyui_manager/js/node_fixer.js b/comfyui_manager/js/node_fixer.js index 5509d1fa..3e251ff4 100644 --- a/comfyui_manager/js/node_fixer.js +++ b/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); } } diff --git a/tests/playwright/README.md b/tests/playwright/README.md index df13f740..b4b6e802 100644 --- a/tests/playwright/README.md +++ b/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 | diff --git a/tests/playwright/legacy-ui-node-fixer.spec.ts b/tests/playwright/legacy-ui-node-fixer.spec.ts new file mode 100644 index 00000000..de319123 --- /dev/null +++ b/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'] }], + }); + }); +}