From b037201fa067ae513484eab82dc65a9d93f80c21 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:25:30 +0300 Subject: [PATCH] fix: swap keeps only roles the new node can take, role-fed inputs are not offered for wiring, an unwire and a group move survive reopen and in-flight saves, swap asks first Co-Authored-By: Claude Sonnet 5.5 --- app/legacy/models.js | 8 +++++- app/legacy/pipeline_state.js | 39 ++++++++++++++++++++++-------- app/legacy/pipeline_state.test.js | 17 +++++++++++++ core/graph.py | 16 ++++++++++-- core/tests/test_graph.py | 8 ++++++ core/tests/test_routes_pipeline.py | 11 +++++++++ 6 files changed, 86 insertions(+), 13 deletions(-) diff --git a/app/legacy/models.js b/app/legacy/models.js index da9e82d7..cfa5b703 100644 --- a/app/legacy/models.js +++ b/app/legacy/models.js @@ -135,7 +135,10 @@ const actions = el("div", "eng-structure-actions"); const swap = el("button", "btn ghost tiny", "Swap node…"); swap.title = "Use a different node here. It must still produce what the nodes after it read."; - swap.onclick = () => pickNode(`Swap ${slotDisplayLabel(slot)} for…`, (cls) => structural({ action: "replace", slot: slot.id, node: cls })); + swap.onclick = () => pickNode(`Swap ${slotDisplayLabel(slot)} for…`, (cls) => { + if (!confirm(`Swap ${slotDisplayLabel(slot)} for ${cls}?\n\nEverything set on this node (files, values, connections into it) is cleared, and the new node starts empty.`)) return; + structural({ action: "replace", slot: slot.id, node: cls }); + }); const rm = el("button", "btn ghost tiny danger", "Remove"); rm.title = "Take this node out; what it fed is rewired to what fed it, when that is unambiguous."; rm.onclick = () => { if (confirm(`Remove ${slotDisplayLabel(slot)} from the pipeline?`)) structural({ action: "remove", slot: slot.id }); }; @@ -185,6 +188,9 @@ "Fed by another node: change it there, or pick “A value” to type one here."); } const row = controlForPlainWidget(slot, widget); + // An input the app itself writes (the prompt box, the source picture, the length) is the app's: + // wiring it elsewhere would make every Generate refuse. + if ((slot.roles || []).some((r) => r.input === widget.name)) return row; // Another node can drive this value (one Primitive shared by several inputs is how a "link" is made). if (widget.type !== "COMBO" || (widget.choices || []).length) { const sources = sourcesFor(slot, widget.type); diff --git a/app/legacy/pipeline_state.js b/app/legacy/pipeline_state.js index deef25ae..215472cc 100644 --- a/app/legacy/pipeline_state.js +++ b/app/legacy/pipeline_state.js @@ -67,6 +67,7 @@ let loadError = null; let removed = new Set(); // default slots the person took out of this project's pipeline let adoptGate = null; // a promise while a project's pipeline is being put in + let deferredRemoved; let deferred = null; // a project's saved pipeline waiting for the first successful load let epoch = 0; // bumped whenever a project's pipeline replaces the live one let saving = false; @@ -130,7 +131,7 @@ } loading = false; loadPromise = null; - if (slots !== null && deferred) { const d = deferred; deferred = null; await adopt(d); } + if (slots !== null && deferred) { const d = deferred, r = deferredRemoved; deferred = null; deferredRemoved = undefined; await adopt(d, r); } })(); return loadPromise; } @@ -178,6 +179,7 @@ saving = true; do { pending = false; + applyGroups(); const toSend = { slots, ...pendingBody }; pendingBody = null; const mine = epoch; @@ -187,6 +189,7 @@ // this answer is about the old one and must not overwrite the new. if (mine !== epoch) continue; if (res && res.slots) slots = res.slots; + applyGroups(); incomplete = (res && res.incomplete) || []; refused = (res && res.refused) || []; queueable = !!(res && res.queueable); @@ -300,10 +303,11 @@ saving = true; let refusedNow = []; try { + applyGroups(); const res = await API.editPipeline({ slots, ...body }); if (mine !== epoch) return { refused: ["The project was changed while this was being sent."] }; refusedNow = (res && res.refused) || []; - if (res && res.slots && !refusedNow.length) slots = res.slots; + if (res && res.slots && !refusedNow.length) { slots = res.slots; applyGroups(); } incomplete = (res && res.incomplete) || []; refused = refusedNow; queueable = !!(res && res.queueable); @@ -324,12 +328,20 @@ } // A slot's group is the person's own to name (any node in any group). + // Kept until it has landed: a save already in flight answers with the slots as they were when + // it left, and would otherwise put the old group back. + let groupEdits = {}; + function applyGroups() { + (slots || []).forEach((s) => { + if (!(s.id in groupEdits)) return; + if (groupEdits[s.id]) s.group = groupEdits[s.id]; else delete s.group; + }); + } function setGroup(slotId, group) { - const slot = (slots || []).find((s) => s.id === slotId); - if (!slot) return Promise.resolve(); - const name = String(group || "").trim(); - if (name) slot.group = name; else delete slot.group; - return save({}); + if (!(slots || []).some((s) => s.id === slotId)) return Promise.resolve(); + groupEdits[slotId] = String(group || "").trim(); + applyGroups(); + return save({}).then(() => { if (!saving) groupEdits = {}; }); } // Whoever keeps the pipeline with the project hears every landed edit. @@ -352,12 +364,14 @@ async function _adopt(saved, removedIds) { // Not loaded (ComfyUI unreachable): remember what the project holds and put it in the moment a // load succeeds, so the default is never what the next edit is built on. - if (slots === null) { if (Array.isArray(saved) && saved.length) deferred = saved; return !(Array.isArray(saved) && saved.length); } + if (slots === null) { if (Array.isArray(saved) && saved.length) { deferred = saved; deferredRemoved = removedIds; } return !(Array.isArray(saved) && saved.length); } if (!Array.isArray(saved) || !saved.length) return true; // A saved slot is only what the server would accept: a project file outlives the code that // wrote it (v4 files name the node differently), and one bad slot refuses every later edit. const sound = (s) => s && typeof s.id === "string" && typeof s.node === "string" - && s.inputs && typeof s.inputs === "object" && !Array.isArray(s.inputs); + && s.inputs && typeof s.inputs === "object" && !Array.isArray(s.inputs) + && (s.group === undefined || (typeof s.group === "string" && s.group.trim())) + && (s.roles === undefined || Array.isArray(s.roles)); const byId = new Map(saved.filter(sound).map((s) => [s.id, s])); const have = new Set(offered.map((s) => s.id)); // Slots the person took out stay out; ones whose node they swapped keep their swap. @@ -366,7 +380,11 @@ const mine = byId.get(def.id); if (!mine) return JSON.parse(JSON.stringify(def)); if (mine.node !== def.node) return JSON.parse(JSON.stringify(mine)); - const out = { ...def, inputs: { ...(def.inputs || {}), ...mine.inputs } }; + // A default LINK the saved copy lacks was unwired by the person; a default VALUE it lacks is + // one an update added, and fills in. + const inputs = { ...(def.inputs || {}) }; + Object.keys(inputs).forEach((k) => { if (Array.isArray(inputs[k]) && !(k in mine.inputs)) delete inputs[k]; }); + const out = { ...def, inputs: { ...inputs, ...mine.inputs } }; if (typeof mine.group === "string" && mine.group) out.group = mine.group; if (typeof mine.bypassed === "boolean") out.bypassed = mine.bypassed; return out; @@ -374,6 +392,7 @@ // Replaces whatever pipeline was live: nothing queued for the old one may land on the new. const mine = ++epoch; removed = new Set(gone); + groupEdits = {}; pendingBody = null; pending = false; for (let waited = 0; saving && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20)); for (const extras of [true, false]) { diff --git a/app/legacy/pipeline_state.test.js b/app/legacy/pipeline_state.test.js index 10af4dbd..5473b353 100644 --- a/app/legacy/pipeline_state.test.js +++ b/app/legacy/pipeline_state.test.js @@ -154,3 +154,20 @@ test("a structural edit is sent on its own and its refusal is handed back", asyn assert.strictEqual(posts.at(-1).action, "remove"); assert.deepStrictEqual(PS.removedIds(), []); // refused: nothing is remembered as removed }); + +test("a default link the person unwired stays unwired when the project is opened again", async () => { + const PS = load([]); + await PS.adopt([{ id: "model", node: "Loader", inputs: { file: "x" } }, + { id: "gen", node: "Gen", inputs: { steps: 8 } }]); // gen.model (a link) was unwired + assert.strictEqual("model" in PS.slots().find((s) => s.id === "gen").inputs, false); +}); + +test("a group moved while a save is in flight is still moved when it lands", async () => { + const PS = load([], { delay: 30 }); + await PS.ensureLoaded(); + const inflight = PS.save({ inputs: { model: { file: "z" } } }); + await new Promise((r) => setTimeout(r, 5)); + await PS.setGroup("gen", "Mine"); + await inflight; + assert.strictEqual(PS.slots().find((s) => s.id === "gen").group, "Mine"); +}); diff --git a/core/graph.py b/core/graph.py index 69bfda30..6db4d063 100644 --- a/core/graph.py +++ b/core/graph.py @@ -604,8 +604,20 @@ def replace(slots: Sequence[dict], slot_id: str, class_type: str, # Inputs are NOT carried across: a different node has different inputs, and # silently keeping the ones whose names happen to match is how a value ends # up meaning something else. - changed = [dict(slot, node=class_type, inputs={}) if slot["id"] == slot_id else dict(slot) - for slot in slots] + # The slot's roles say which of its inputs the app writes to (the prompt box, the source image, + # the length). Only the ones the new node actually has can still be written: a role left + # pointing at an input that is gone would make every run refuse. + have = schemas.inputs(class_type) + + def fits(slot): + roles = [r for r in (slot.get("roles") or []) if isinstance(r, dict) and r.get("input") in have] + out = dict(slot, node=class_type, inputs={}) + out.pop("roles", None) + if roles: + out["roles"] = roles + return out + + changed = [fits(slot) if slot["id"] == slot_id else dict(slot) for slot in slots] return changed, [] diff --git a/core/tests/test_graph.py b/core/tests/test_graph.py index 2c83bd7e..8104df61 100644 --- a/core/tests/test_graph.py +++ b/core/tests/test_graph.py @@ -577,3 +577,11 @@ def test_a_new_node_can_be_wired_into_several_inputs_and_later_ones_added_to_the slots, _ = graph.add(slots, "Empty", None, SCHEMAS) slots, problems = graph.wire(slots, "empty_2", "width", "primitiveint", 0, SCHEMAS) assert problems == [] and graph.slots_by_id(slots)["empty_2"]["inputs"]["width"] == ["primitiveint", 0] + +def test_a_swap_keeps_only_the_roles_the_new_node_can_still_take(): + slots = [{"id": "empty", "node": "Empty", "inputs": {"width": 8}, + "roles": [{"at": "project.video", "input": "width"}, {"at": "project.video", "input": "length"}]}, + {"id": "save", "node": "Save", "inputs": {"latent": ["empty", 0]}}] + swapped, problems = graph.replace(slots, "empty", "Empty", SCHEMAS) + assert problems == [] + assert graph.slots_by_id(swapped)["empty"]["roles"] == [{"at": "project.video", "input": "width"}] diff --git a/core/tests/test_routes_pipeline.py b/core/tests/test_routes_pipeline.py index e840e33e..ca7e0cf0 100644 --- a/core/tests/test_routes_pipeline.py +++ b/core/tests/test_routes_pipeline.py @@ -119,6 +119,17 @@ def test_a_built_in_slot_can_be_removed_over_http(server): assert sampler["inputs"]["model"] == ["model", 0], "the sampler was not rewired" +def test_a_node_can_be_added_over_http_and_a_missing_one_comes_back_as_a_reason(server): + status, body = _request(server, "POST", "/funpack/api/pipeline", + {"action": "add", "node": "PrimitiveInt", "group": "Mine"}) + assert status == 200 and body["refused"] == [], body + added = body["slots"][-1] + assert added["node"] == "PrimitiveInt" and added["group"] == "Mine" and added["inputs"] == {} + status, body = _request(server, "POST", "/funpack/api/pipeline", {"action": "add", "node": "NoSuchNode"}) + assert status == 200 and any("NoSuchNode" in r for r in body["refused"]) + assert _request(server, "POST", "/funpack/api/pipeline", {"action": "add", "node": 5})[0] == 400 + + def test_a_removal_that_cannot_work_comes_back_as_a_reason(server): status, body = _request(server, "POST", "/funpack/api/pipeline", {"action": "remove", "slot": "latent"})