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'] }], + }); + }); +}