From 382a72cfd763a6d6182bb247574aa8e5aaa3959d Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:15:45 +0300 Subject: [PATCH] fix: a project opened while ComfyUI is down is owned once it answers; the Modules page keeps its rows under Disable all; settings visibility is computed once per list Co-Authored-By: Claude Opus 5.5 --- app/ui/features/sampler/hub.test.js | 4 +-- app/ui/features/sampler/index.js | 2 +- app/ui/features/settings/engine.js | 2 +- app/ui/features/settings/modules.js | 2 +- app/ui/shell/pipeline_link.js | 9 ++++++- app/ui/shell/pipeline_state.js | 30 ++++++++++++++--------- app/ui/shell/tests/pipeline_link.test.js | 18 ++++++++++++++ app/ui/shell/tests/pipeline_state.test.js | 12 +++++++++ 8 files changed, 61 insertions(+), 18 deletions(-) diff --git a/app/ui/features/sampler/hub.test.js b/app/ui/features/sampler/hub.test.js index 1fdf65d4..21d16140 100644 --- a/app/ui/features/sampler/hub.test.js +++ b/app/ui/features/sampler/hub.test.js @@ -10,7 +10,7 @@ test("the Sampler window shows the pipeline's steps and sampler, and a change sa const saved = []; const slot = { id: "sampler", node: "S", inputs: { steps: 20, sampler_name: "euler", seed: 0 }, roles: [{ at: "generation.sampling", input: "steps", label: "Steps" }, { at: "generation.sampling", input: "sampler_name", label: "Sampler" }, { at: "generation.seed", input: "seed" }] }; - const ps = { slots: () => [slot], activeModules: () => [], useful: () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, + const ps = { slots: () => [slot], activeModules: () => [], useful: () => true, usefulness: () => () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, subscribe: () => () => {}, ensureLoaded: async () => {}, save: async (b) => { saved.push(b); } }; const api = { describeNodes: async () => ({ nodes: { S: { widgets: [{ name: "steps", type: "INT", min: 1, max: 100 }, { name: "sampler_name", type: "COMBO", choices: ["euler", "dpmpp_2m"] }, { name: "seed", type: "INT" }] } } }) }; const host = document.createElement("div"); @@ -33,7 +33,7 @@ test("the Sampler window shows the pipeline's steps and sampler, and a change sa test("when the sampler node's options cannot be read, the window says where to change them", async () => { const slot = { id: "sampler", node: "S", inputs: { steps: 20 }, roles: [{ at: "generation.sampling", input: "steps", label: "Steps" }] }; - const ps = { slots: () => [slot], activeModules: () => [], useful: () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, + const ps = { slots: () => [slot], activeModules: () => [], useful: () => true, usefulness: () => () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, subscribe: () => () => {}, ensureLoaded: async () => {}, save: async () => {} }; const host = document.createElement("div"); document.body.append(host); diff --git a/app/ui/features/sampler/index.js b/app/ui/features/sampler/index.js index dbf2d2a5..8af62430 100644 --- a/app/ui/features/sampler/index.js +++ b/app/ui/features/sampler/index.js @@ -13,7 +13,7 @@ export default { setup({ host, app }) { const ps = app.pipeline; let pop = null, specs = {}, unreadable = false; - const sampling = () => ps.activeModules().filter((m) => (m.category || "") === "sampling" && Object.keys(m.settings || {}).length && ps.useful(m)); + const sampling = () => { const ok = ps.usefulness(); return ps.activeModules().filter((m) => (m.category || "") === "sampling" && Object.keys(m.settings || {}).length && ok(m)); }; const marked = () => (ps.slots() || []).flatMap((slot) => (slot.roles || []).filter((r) => r.at === "generation.sampling" && r.input && !Array.isArray((slot.inputs || {})[r.input])).map((role) => ({ slot, role }))); const describe = async () => { const missing = [...new Set(marked().map(({ slot }) => slot.node))].filter((n) => !(n in specs)); diff --git a/app/ui/features/settings/engine.js b/app/ui/features/settings/engine.js index 84cfed5b..416e1a77 100644 --- a/app/ui/features/settings/engine.js +++ b/app/ui/features/settings/engine.js @@ -13,7 +13,7 @@ export const engine = (app) => function mount() { if (ps.loading()) return page.set([c.hint.default({ text: "Loading…" })]); if (ps.loadError()) return page.set([c.banner.warn({ text: `Could not load Engine settings: ${ps.loadError()}` })]); if (ps.allOff()) return page.set([c.banner.info({ text: "All enhancements are disabled for this project, so there is nothing to set. Turn them back on in Settings ▸ Modules." })]); - const withSettings = ps.activeModules().filter((m) => Object.keys(m.settings || {}).length && ps.useful(m)); // nothing would read the rest + const ok = ps.usefulness(), withSettings = ps.activeModules().filter((m) => Object.keys(m.settings || {}).length && ok(m)); // nothing would read the rest const cats = CATEGORIES.filter(([k]) => withSettings.some((m) => (m.category || "") === k)); if (!cats.length) return page.set([c.emptyState.default({ icon: "⚡", title: "Nothing to set", hint: "No installed module exposes a setting for this pipeline." })]); if (!cats.some(([k]) => k === category)) category = cats[0][0]; diff --git a/app/ui/features/settings/modules.js b/app/ui/features/settings/modules.js index 46fecb5a..d189c2a8 100644 --- a/app/ui/features/settings/modules.js +++ b/app/ui/features/settings/modules.js @@ -11,7 +11,7 @@ export const modules = (app) => function mount() { if (ps.loading()) return page.set([c.hint.default({ text: "Loading…" })]); if (ps.loadError()) return page.set([c.banner.warn({ text: `Could not load: ${ps.loadError()}` })]); const control = ps.control(); - const mine = Object.values(ps.modulesById()).filter((m) => control[m.id] && control[m.id].controllable && (control[m.id].quarantine || ps.useful(m))); + const ok = ps.usefulness(), mine = Object.values(ps.modulesById()).filter((m) => control[m.id] && control[m.id].controllable && (control[m.id].quarantine || ok(m))); if (!mine.length) return page.set([c.emptyState.default({ icon: "☷", title: "Nothing to switch", hint: "No installed module can be switched off." })]); const name = (m) => m.title || m.id; const row = (m) => { diff --git a/app/ui/shell/pipeline_link.js b/app/ui/shell/pipeline_link.js index 34feb79e..bb963e9e 100644 --- a/app/ui/shell/pipeline_link.js +++ b/app/ui/shell/pipeline_link.js @@ -5,7 +5,7 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { // it is written. A project opened while an open or a save is still in flight must never receive the other's pipeline. // An id, not the object: Undo puts a copy of the same project in place. let owner = null; - let retries = 0, opens = 0, retry = null; + let retries = 0, opens = 0, retry = null, waiting = false; // waiting: gave up because ComfyUI did not answer // Only a real change is written: opening a project must not rewrite its file. const live = (base, slots) => ({ ...base, slots: JSON.parse(JSON.stringify(slots || [])), removed: pipeline.removedIds(), unwired: pipeline.unwiredMap(), whole: pipeline.whole() }); const store = (base, slots) => { @@ -15,6 +15,7 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { async function adopt() { clearTimeout(retry); // a retry still waiting is for a project that may no longer be open + waiting = false; if (pipeline.settled) await pipeline.settled(); // an edit on its way lands in the project it was made in first const here = project.project && project.project.models; // Undo / Redo of something else: the same project, its pipeline already live, nothing to put in (or to lose). @@ -36,12 +37,18 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { if (project.fresh) store(saved, pipeline.slots()); } else if (retries++ < 3) { retry = setTimeout(adopt, 5000); + } else if (!pipeline.slots()) { + // ComfyUI is down: keep asking, slowly; and if a panel's own load gets there first, its announcement puts this project's in. + waiting = true; + retry = setTimeout(adopt, 15000); + if (retries === 4) say("ComfyUI is not answering, so this project's pipeline is not loaded yet: it goes in as soon as ComfyUI answers."); } else { say("This project's saved pipeline could not be loaded; the default is in use."); } } pipeline.subscribe((slots) => { + if (waiting && owner === null) { retries = 0; adopt(); return; } if (!project.project || project.project.id !== owner) return; store(project.project.models || {}, slots); }); diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index 5d327f72..1397da57 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -303,20 +303,26 @@ export function createPipelineState(API) { } function isQuarantined(id) { return !!(controlState[id] && controlState[id].quarantine); } // Whether anything would act on a module's settings now: its own node is in the pipeline (when it has one), - // and what it serves (the modifier loader, the taste store) has a user that is on, not set to a manual value, - // has every node it needs in the pipeline, and is itself acted on (a taste learner the loader never installs is not). - function useful(m, values = currentValues(), seen = new Set([m.id])) { + // and every capability it offers (the modifier loader, the taste store) reaches a user that is switched on for this + // project, not set to a manual value, has every node it needs in the pipeline, and is itself acted on (a taste learner + // the loader never installs is not). "Disable all enhancements" is not counted: it sits on top, and the Modules page + // still lists what is kept underneath it. -> a predicate; the values are read once for a whole list. + function usefulness() { + const values = currentValues(), off = new Set(((values._off || {}).modules) || []); + const users = Object.values(modulesById).filter((u) => (u.uses || []).length && !off.has(u.id) && !isQuarantined(u.id)); const has = (n) => !slots || slots.some((s) => s.node === n); - const working = (u) => { - const v = values[u.id] || {}; - return !seen.has(u.id) && (!("enabled" in (u.settings || {})) || v.enabled) && v.mode !== "manual" - && (u.nodes || []).every(has) && useful(u, values, new Set([...seen, u.id])); + const check = (m, seen) => { + const working = (u) => { + const v = values[u.id] || {}; + return !seen.has(u.id) && (!("enabled" in (u.settings || {})) || v.enabled) && v.mode !== "manual" + && (u.nodes || []).every(has) && check(u, new Set([...seen, u.id])); + }; + return (!(m.nodes || []).length || m.nodes.some(has)) + && (m.serves || []).every((cap) => users.some((u) => u.uses.includes(cap) && working(u))); }; - const inPipeline = !(m.nodes || []).length || m.nodes.some(has); - // Every capability it offers must reach a working user: the taste key is installed by the loader AND read by a learner. - const served = (m.serves || []).every((cap) => activeModules().some((u) => (u.uses || []).includes(cap) && working(u))); - return inPipeline && served; + return (m) => check(m, new Set([m.id])); } + const useful = (m) => usefulness()(m); function activeModules() { return allOff() ? [] : Object.values(modulesById).filter((m) => !isOff(m.id) && !isQuarantined(m.id)); @@ -582,7 +588,7 @@ export function createPipelineState(API) { frozenInputs, ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, settled, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, modulesById: () => modulesById, - activeModules, useful, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, + activeModules, useful, usefulness, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, removedIds: () => [...removed], whole: () => whole, unwiredMap: () => JSON.parse(JSON.stringify(unwired)), diff --git a/app/ui/shell/tests/pipeline_link.test.js b/app/ui/shell/tests/pipeline_link.test.js index d15f95cd..a2636d4b 100644 --- a/app/ui/shell/tests/pipeline_link.test.js +++ b/app/ui/shell/tests/pipeline_link.test.js @@ -139,3 +139,21 @@ test("opening an older project whose pipeline an update's defaults fill in does await opened(); assert.equal(writes.length, 0); }); + +test("a project opened while ComfyUI is down is owned once it answers, so later edits are saved, and the toast is said once", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const writes = [], said = []; + let opened, heard, up = false; + const project = { project: { id: "a", models: { slots, removed: [], unwired: {}, whole: false } }, setField: (k, v) => { writes.push(v.slots[0].inputs.ckpt_name); project.project[k] = v; } }; + const pipeline = { adopt: async () => up, slots: () => (up ? slots : null), removedIds: () => [], unwiredMap: () => ({}), whole: () => false, subscribe: (fn) => { heard = fn; } }; + linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; }, say: (x) => said.push(x) }); + const flush = async () => { for (let i = 0; i < 5; i++) await new Promise((r) => setImmediate(r)); }; + await opened(); + for (let i = 0; i < 5; i++) { t.mock.timers.tick(16000); await flush(); } + assert.equal(said.length, 1); + up = true; + heard(slots); // a panel's own load got the pipeline in and announced it + await flush(); + heard([{ ...slots[0], inputs: { ckpt_name: "edited" } }]); + assert.deepStrictEqual(writes, ["edited"]); +}); diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index c6e28a06..6c7fb1a9 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -434,3 +434,15 @@ test("a project retried after the first load failed goes in, and nothing waits f assert.strictEqual(PS.slots().find((s) => s.id === "model").inputs.file, "mine"); await Promise.race([PS.save({ inputs: { model: { file: "next" } } }), new Promise((_, no) => setTimeout(() => no(new Error("save hung")), 1000))]); }); + +test("with every enhancement disabled, the modules kept underneath still count, so the Modules page can list them", async () => { + const loader = { id: "mods", nodes: ["FunPackModifierSettings", "FunPackLoadModifiers"], uses: ["modifier"] }; + const sharpen = { id: "sharpen", serves: ["modifier"], settings: { amount: { default: 0.5 } } }; + const PS = load([]); + lastApi.pipeline = async () => ({ slots: [{ id: "a", node: "FunPackModifierSettings", inputs: {} }, { id: "b", node: "FunPackLoadModifiers", inputs: {} }], incomplete: [], refused: [], queueable: true }); + lastApi.modules = async () => ({ modules: [loader, sharpen], control: {} }); + await PS.ensureLoaded(); + await PS.setAllOff(true); + assert.strictEqual(PS.allOff(), true); + assert.strictEqual(PS.usefulness()(PS.modulesById().sharpen), true); +});