From fd3a8b5cf225e3cea2f166e46ed87586f122efbb Mon Sep 17 00:00:00 2001 From: Artokun Date: Sat, 8 Aug 2026 09:57:04 -0700 Subject: [PATCH 1/6] =?UTF-8?q?fix(panel):=20a=20tab=20that=20is=20open=20?= =?UTF-8?q?and=20empty=20must=20SAY=20SO=20=E2=80=94=20the=20#779=20silenc?= =?UTF-8?q?e=20detector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #784 fixed the cause of the blank panel on frontend 1.50.x and #785 gave a THROWING render a visible shell. But a render that is never CALLED — what an actual sidebar-tab contract change in a future frontend would produce — still fails in perfect silence, and that silence is what cost the #779 reporter an hour of reinstalling things that could never have helped. This adds the watchdog that turns the silence into one console line naming the panel version, the frontend version, and what to do. Two evidence-only checks: - STARVATION: our tab is PROVABLY selected (dual-generation marker read, the #784 helper) and neither .cmcp-root nor the #785 failure shell exists, continuously for 3s, re-verified at expiry. An unreadable marker disarms rather than counts; the first successful paint retires it for the page. - APPEARANCE: the rail exists but our tab button never joined it within 10s of the rail being seen — the shape of a frontend that accepts registerSidebarTab() and drops the legacy spec. No rail at all is 'I cannot tell' and stays silent. The false-positive bar is treated as the primary requirement: slow first builds, keep-alive detaches on tab switches, users wandering off mid-window, and unknown markers are all asserted quiet in the tests. The recommended workaround pin in the message tracks VERIFIED_FRONTENDS instead of a hardcoded version, so it cannot age into bad advice. Refs #779 (the diagnostic half; the cause was fixed by #784/#786). Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/sidebar-render-watchdog.test.mjs | 438 ++++++++++++++++++ web/js/comfyui-mcp-panel.js | 20 + web/js/lib/comfyui-dom-deps.js | 8 +- web/js/lib/sidebar-render-watchdog.js | 341 ++++++++++++++ 4 files changed, 803 insertions(+), 4 deletions(-) create mode 100644 browser_tests/unit/sidebar-render-watchdog.test.mjs create mode 100644 web/js/lib/sidebar-render-watchdog.js diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs new file mode 100644 index 00000000..b4301844 --- /dev/null +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -0,0 +1,438 @@ +// panel#779 — the silence detector: a selected Agent tab with nothing painted, +// or a registered tab whose button never appears, must produce ONE console line +// naming both versions and what to do. +// +// The outage this grew from failed in perfect silence: tab registered, +// selectable, black rectangle, `.cmcp-root` absent, nothing attributed to us. +// #784 fixed that cause and #785 gave a THROWING render a visible shell — but a +// render that is never CALLED (what a real sidebar-tab contract change would +// produce) still says nothing. The reporter answered that silence with an hour +// of reinstalls that could never have helped. +// +// The bar for these tests is the false-positive bar: the watchdog will be read +// as "something is broken", so every path where it must stay quiet — slow first +// build, keep-alive detach on tab switch, a user wandering off mid-window, an +// unreadable tab marker — is asserted as hard as the firing path. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + +import { + RENDER_STARVATION_MS, + TAB_APPEAR_DEADLINE_MS, + WATCHDOG_POLL_MS, + WATCHDOG_GIVE_UP_MS, + renderStarvationReport, + tabNeverAppearedReport, + createRenderWatchdog, + installSidebarRenderWatchdog, +} from "../../web/js/lib/sidebar-render-watchdog.js"; +import { VERIFIED_FRONTENDS } from "../../web/js/lib/comfyui-dom-deps.js"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PANEL_JS = join(HERE, "../../web/js/comfyui-mcp-panel.js"); +const OURS = "comfyui-mcp.agent"; + +// --------------------------------------------------------------------------- +// The reports: observed facts, both versions, closed-off dead ends, a remedy. +// --------------------------------------------------------------------------- + +test("#779 the starvation line carries everything a support answer needs", () => { + const line = renderStarvationReport({ + panelVersion: "0.11.44", + frontendVersion: "1.50.3", + waitedMs: 3000, + }); + assert.match(line, /^\[comfyui-mcp-panel\]/, "attributed to us — the outage line was not"); + assert.match(line, /0\.11\.44/, "panel version"); + assert.match(line, /1\.50\.3/, "frontend version — the field this whole issue turned on"); + assert.match(line, /~3s/, "how long it watched before speaking"); + assert.match(line, /\.cmcp-root/, "the observable a reporter can re-check"); + assert.match(line, /never asked to render|removed as soon as/, "names BOTH shapes it cannot distinguish"); + assert.match(line, /NOT a connection problem/i, "dead end #1, closed"); + assert.match(line, /reinstalling.*cannot change it/i, "dead end #2 — the one that cost an hour"); + assert.match(line, /github\.com\/artokun\/comfyui-mcp-panel\/issues/, "where to send it"); + assert.match(line, /--front-end-version comfyanonymous\/ComfyUI@/, "the workaround, in paste-able form"); +}); + +test("#779 the appearance line is distinct and equally complete", () => { + const line = tabNeverAppearedReport({ + panelVersion: "0.11.44", + frontendVersion: "1.53.0", + waitedMs: 10000, + }); + assert.match(line, /^\[comfyui-mcp-panel\]/); + assert.match(line, /button never\s+appeared/); + assert.match(line, /~10s/); + assert.match(line, /0\.11\.44/); + assert.match(line, /1\.53\.0/); + assert.match(line, /NOT a connection problem/i); + assert.match(line, /github\.com\/artokun\/comfyui-mcp-panel\/issues/); + assert.match(line, /--front-end-version comfyanonymous\/ComfyUI@/); +}); + +test("#779 unknown versions say 'unknown' — never a guess", () => { + const line = renderStarvationReport({}); + assert.match(line, /panel unknown/); + assert.match(line, /frontend unknown/); +}); + +test("#779 the workaround pin is a VERIFIED frontend, not a hardcoded relic", () => { + // The pin must track the registry that records what was actually checked + // against shipped bundles — otherwise this string ages into bad advice. + const newest = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1]; + const line = renderStarvationReport({}); + assert.ok( + line.includes(`--front-end-version comfyanonymous/ComfyUI@${newest}`), + `the recommended pin should be ${newest} (the newest verified frontend)`, + ); + for (const v of VERIFIED_FRONTENDS) { + assert.ok(line.includes(v), `every verified frontend is named as known-good (missing ${v})`); + } +}); + +// --------------------------------------------------------------------------- +// The state machine. Times in ms; WINDOW below for readability. +// --------------------------------------------------------------------------- + +const WINDOW = RENDER_STARVATION_MS; +const ours = { state: "id", id: OURS }; +const other = { state: "id", id: "workflows" }; +const unknown = { state: "unknown" }; +const none = { state: "none" }; + +function machine(onStarve = () => {}) { + return createRenderWatchdog({ tabId: OURS, onStarve }); +} + +test("#779 the healthy path retires the watchdog for good", () => { + const m = machine(() => assert.fail("must not fire")); + assert.equal(m.sample(ours, true, 0).state, "satisfied"); + // Retired means RETIRED: even a later selected-and-empty eternity says nothing. + assert.equal(m.sample(ours, false, WINDOW * 100).state, "satisfied"); + assert.equal(m.fired(), false); +}); + +test("#779 selected-and-empty shorter than the window never fires (slow first build)", () => { + const m = machine(() => assert.fail("must not fire")); + assert.equal(m.sample(ours, false, 0).state, "armed"); + assert.equal(m.sample(ours, false, WINDOW - 1).state, "armed"); + // The build lands just inside the deadline — a loaded machine, not a fault. + assert.equal(m.sample(ours, true, WINDOW - 1).state, "satisfied"); +}); + +test("#779 a full continuous window fires exactly once, with the waited time", () => { + let fired = 0; + let waitedMs = null; + const m = machine((w) => { + fired += 1; + waitedMs = w; + }); + m.sample(ours, false, 0); + m.sample(ours, false, 1000); // observer noise mid-window must not reset the clock + assert.equal(m.sample(ours, false, WINDOW).state, "fired"); + assert.equal(fired, 1); + assert.equal(waitedMs, WINDOW); + // Nothing ever fires twice — one line per page load is the contract. + assert.equal(m.sample(ours, false, WINDOW * 10).state, "fired"); + assert.equal(fired, 1); + assert.equal(m.fired(), true); +}); + +test("#779 switching away disarms; the clock restarts from zero on return", () => { + let fired = 0; + const m = machine(() => (fired += 1)); + m.sample(ours, false, 0); + // Keep-alive: user peeks at another tab mid-window. destroy() detached our + // root, but the active tab is theirs — that is not evidence about us. + assert.equal(m.sample(other, false, WINDOW - 500).state, "idle"); + // Back to us: a FRESH window, not the remainder of the old one. + m.sample(ours, false, WINDOW); + assert.equal(m.sample(ours, false, WINDOW * 2 - 1).state, "armed"); + assert.equal(fired, 0); + assert.equal(m.sample(ours, false, WINDOW * 2).state, "fired"); + assert.equal(fired, 1); +}); + +test("#779 an unreadable or absent selection NEVER arms — the #784 rule for diagnostics", () => { + const m = machine(() => assert.fail("must not fire")); + // "unknown" is "I cannot tell which tab is active", not "ours is starving". + // This is deliberate blindness: if the marker moves again, the guard keeps + // the panel alive (#784) and this watchdog stays quiet rather than crying + // wolf on every tab the user opens. + assert.equal(m.sample(unknown, false, 0).state, "idle"); + assert.equal(m.sample(unknown, false, WINDOW * 2).state, "idle"); + assert.equal(m.sample(none, false, WINDOW * 4).state, "idle"); + assert.equal(m.sample(other, false, WINDOW * 6).state, "idle"); + assert.equal(m.sample(null, false, WINDOW * 8).state, "idle"); +}); + +test("#779 stray paint under ANOTHER active tab neither satisfies nor arms", () => { + const m = machine(() => assert.fail("must not fire")); + // Our content lingering while another tab is active is the guard's problem, + // not proof the contract works — satisfaction requires painted WHILE ours. + assert.equal(m.sample(other, true, 0).state, "idle"); + assert.equal(m.sample(ours, false, 1).state, "armed"); +}); + +test("#779 a reporter that throws is swallowed and still counts as fired", () => { + const m = createRenderWatchdog({ + tabId: OURS, + onStarve: () => { + throw new Error("console is broken too"); + }, + }); + m.sample(ours, false, 0); + assert.equal(m.sample(ours, false, WINDOW).state, "fired"); + assert.equal(m.fired(), true); +}); + +// --------------------------------------------------------------------------- +// The installer, against a fake document and a hand-cranked clock. +// --------------------------------------------------------------------------- + +/** A selected rail button double, in the 1.50 (data-testid) shape. */ +function modernButton(id) { + return { + classList: ["side-bar-button", "side-bar-button-selected"], + getAttribute: (k) => (k === "data-testid" ? `${id}-tab-button` : null), + }; +} + +/** The same in the <=1.49 (class) shape. */ +function legacyButton(id) { + return { + classList: ["side-bar-button", "side-bar-button-selected", `${id}-tab-button`], + getAttribute: () => null, + }; +} + +/** + * The whole harness: fake doc, fake timers, captured reports, an observer stub + * whose callback we can pull. `state` is mutated by the tests to move the world. + */ +function harness({ windowMs = 300, appearDeadlineMs = 1000, pollMs = 50, giveUpMs = 6000 } = {}) { + const state = { + rail: null, // truthy once the rail exists + button: false, // our tab button present in the rail? + selected: null, // the selected rail button element, or null + painted: false, + }; + const reports = []; + let clock = 0; + let seq = 0; + let timers = []; // { id, at, fn } + const observers = []; + + const doc = { + querySelector(sel) { + if (sel === ".side-bar-button-selected") return state.selected; + if (sel === ".side-tool-bar-container") return state.rail; + if (sel.startsWith("[data-testid=")) { + return state.button ? { tag: "modern-button" } : null; + } + if (sel.startsWith("button[class~=")) return null; // fake rail is 1.50-shaped + return null; + }, + }; + + const handle = installSidebarRenderWatchdog({ + tabId: OURS, + doc, + isPainted: () => state.painted, + panelVersion: "0.11.44-test", + getFrontendVersion: () => "9.9.9", + report: (line) => reports.push(line), + makeObserver: (cb) => { + const o = { cb, observe() {}, disconnect() {} }; + observers.push(o); + return o; + }, + setTimer: (fn, ms) => { + const id = ++seq; + timers.push({ id, at: clock + ms, fn }); + return id; + }, + clearTimer: (id) => { + timers = timers.filter((t) => t.id !== id); + }, + now: () => clock, + windowMs, + appearDeadlineMs, + pollMs, + giveUpMs, + }); + + /** Advance the clock, running due timers in order (they may schedule more). */ + function advance(ms) { + const until = clock + ms; + for (;;) { + const due = timers.filter((t) => t.at <= until).sort((a, b) => a.at - b.at)[0]; + if (!due) break; + timers = timers.filter((t) => t.id !== due.id); + clock = Math.max(clock, due.at); + due.fn(); + } + clock = until; + } + + /** Fire every live MutationObserver callback, as a rail class change would. */ + function mutate() { + for (const o of observers) o.cb(); + } + + return { state, reports, advance, mutate, handle, timersLeft: () => timers.length }; +} + +test("#779 installer: the healthy first open reports nothing and stands down", () => { + const h = harness(); + h.state.rail = {}; + h.advance(60); // poll finds the rail, attaches the observer + h.state.button = true; + h.advance(60); // poll sees the button — appearance satisfied + h.state.selected = modernButton(OURS); + h.state.painted = true; // render() attached the root, as it should + h.mutate(); // the selection class change + assert.equal(h.reports.length, 0); + assert.equal(h.handle.sample().state, "satisfied"); + h.advance(20000); + assert.equal(h.reports.length, 0, "a satisfied watchdog never speaks"); + assert.equal(h.timersLeft(), 0, "…and holds no timers"); +}); + +test("#779 installer: selected-but-never-painted produces EXACTLY the one line", () => { + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + // The user opens the Agent tab; render never attaches anything. This is the + // reporter's screen on 1.50.3 before #784, and any future contract move. + h.state.selected = modernButton(OURS); + h.mutate(); + assert.equal(h.reports.length, 0, "nothing said inside the window"); + h.advance(500); // windowMs 300 + slack + assert.equal(h.reports.length, 1); + assert.match(h.reports[0], /no panel content exists/); + assert.match(h.reports[0], /0\.11\.44-test/); + assert.match(h.reports[0], /9\.9\.9/, "frontend version read at fire time"); + h.mutate(); + h.advance(20000); + assert.equal(h.reports.length, 1, "one line per page load, ever"); +}); + +test("#779 installer: the pre-1.50 class shape drives the same detection", () => { + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + h.state.selected = legacyButton(OURS); + h.mutate(); + h.advance(500); + assert.equal(h.reports.length, 1, "a 1.47-shaped rail is watched identically"); +}); + +test("#779 installer: switching away inside the window keeps it quiet", () => { + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + h.state.selected = modernButton(OURS); + h.mutate(); // armed + h.advance(150); // half the window + h.state.selected = modernButton("workflows"); // user wanders off; root detached + h.mutate(); + h.advance(20000); + assert.equal(h.reports.length, 0, "an abandoned window is not a failure"); +}); + +test("#779 installer: paint landing inside the window keeps it quiet", () => { + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + h.state.selected = modernButton(OURS); + h.mutate(); // armed — render hasn't run yet, exactly the mid-construction gap + h.advance(100); + h.state.painted = true; // …and now it has + h.advance(20000); + assert.equal(h.reports.length, 0, "the expiry re-check found a healthy panel"); +}); + +test("#779 installer: the #785 failure shell counts as painted — one voice at a time", () => { + // If render() threw, the shell is already saying something better than we + // can. isPainted() covers it at the integration site; here we prove the + // installer trusts whatever isPainted says. + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + h.state.selected = modernButton(OURS); + h.state.painted = true; // the shell IS paint + h.mutate(); + h.advance(20000); + assert.equal(h.reports.length, 0); +}); + +test("#779 installer: a rail with no button for the deadline says the appearance line", () => { + const h = harness(); + h.state.rail = {}; // the rail exists… + // …but our button never joins it, and nothing is ever selectable. + h.advance(1500); // past appearDeadlineMs (1000) + assert.equal(h.reports.length, 1); + assert.match(h.reports[0], /button never\s+appeared/); + assert.match(h.reports[0], /9\.9\.9/); + h.advance(20000); + assert.equal(h.reports.length, 1); + assert.equal(h.timersLeft(), 0, "spoken once, then fully stood down"); +}); + +test("#779 installer: no rail at all is 'I cannot tell' — permanent silence", () => { + const h = harness(); + h.advance(WATCHDOG_GIVE_UP_MS + 20000); + assert.equal(h.reports.length, 0); + assert.equal(h.timersLeft(), 0, "gave up without a word — no rail, no evidence"); +}); + +test("#779 installer: a button that appears late but within the deadline is fine", () => { + const h = harness(); + h.state.rail = {}; + h.advance(600); // rail seen, button still absent — inside the deadline + h.state.button = true; + h.advance(20000); + assert.equal(h.reports.length, 0, "slow rail population is not a contract break"); +}); + +// --------------------------------------------------------------------------- +// Integration: the watchdog is actually wired at the registration site. +// --------------------------------------------------------------------------- + +test("#779 the panel installs the watchdog right after the sidebar guard", () => { + const src = readFileSync(PANEL_JS, "utf8"); + const guardAt = src.indexOf("installSidebarTabGuard("); + const dogAt = src.indexOf("installSidebarRenderWatchdog({"); + assert.ok(guardAt > 0, "the guard is still installed"); + assert.ok(dogAt > guardAt, "the watchdog is installed after (and only with) the guard"); + assert.match(src, /import \{ installSidebarRenderWatchdog \} from "\.\/lib\/sidebar-render-watchdog\.js"/); +}); + +test("#779 isPainted at the integration site counts BOTH the root and the failure shell", () => { + const src = readFileSync(PANEL_JS, "utf8"); + const call = src.slice(src.indexOf("installSidebarRenderWatchdog({")); + const body = call.slice(0, call.indexOf("});") + 3); + assert.match(body, /\.cmcp-root/, "the panel itself"); + assert.match(body, /\.cmcp-failure-shell/, "the #785 shell — already a voice, not a starvation"); + assert.match(body, /getFrontendVersion/, "the version the whole issue turned on is captured"); + assert.match(body, /__COMFYUI_FRONTEND_VERSION__/); +}); + +test("#779 the exported bounds are what the reports promise", () => { + // The report says "~3s"/"~10s" from its inputs; the defaults must match the + // constants so a default-config line never claims a window it did not wait. + assert.equal(RENDER_STARVATION_MS, 3000); + assert.equal(TAB_APPEAR_DEADLINE_MS, 10000); + assert.ok(WATCHDOG_POLL_MS >= 250, "polling is a trickle, not a hot loop"); + assert.ok(WATCHDOG_GIVE_UP_MS >= 30000); +}); diff --git a/web/js/comfyui-mcp-panel.js b/web/js/comfyui-mcp-panel.js index 2cf9f29a..87e4ccef 100644 --- a/web/js/comfyui-mcp-panel.js +++ b/web/js/comfyui-mcp-panel.js @@ -147,6 +147,7 @@ import { readSaveFailureCause } from "./lib/userdata-failure-cause.js"; import { describeScreenshotFraming } from "./lib/screenshot-framing.js"; import { readActiveSidebarTab, shouldDetachPanelRoot, findSidebarTabButton } from "./lib/active-sidebar-tab.js"; import { buildPanelFailureShell } from "./lib/panel-failure-shell.js"; +import { installSidebarRenderWatchdog } from "./lib/sidebar-render-watchdog.js"; import { displayLabel, boundaryInputLabel, widgetLabelMap } from "./lib/slot-labels.js"; import { createObjectInfoHistory, awaitHistoryBaseline } from "./lib/object-info-history.js"; import { makeRefreshCoalescer } from "./lib/refresh-coalesce.js"; @@ -27403,6 +27404,25 @@ function registerExtensionWhenReady(tries = 0) { () => document.querySelector(".cmcp-root"), () => mounted?.onHide?.(), ); + // #779 — the silence detector. If our tab is provably selected and + // neither the panel nor the #785 failure shell is in the document for a + // few continuous seconds — or the tab button never appears in the rail + // at all — say so ONCE in the console, with both version numbers and + // what to do. Both failure shapes are what a future sidebar-tab + // contract change looks like from here, and both were previously + // indistinguishable from "works on my machine" until a reporter lost + // an hour to reinstalls that could never have helped. + installSidebarRenderWatchdog({ + tabId, + isPainted: () => + !!document.querySelector(".cmcp-root") || + !!document.querySelector(".cmcp-failure-shell"), + panelVersion: typeof PANEL_VERSION === "string" ? PANEL_VERSION : undefined, + getFrontendVersion: () => + window.__COMFYUI_FRONTEND_VERSION__ ?? + app?.extensionManager?.frontendVersion ?? + undefined, + }); } else { console.error( "[comfyui-mcp-panel] app.extensionManager.registerSidebarTab is unavailable; " + diff --git a/web/js/lib/comfyui-dom-deps.js b/web/js/lib/comfyui-dom-deps.js index 86170560..3bf3c1db 100644 --- a/web/js/lib/comfyui-dom-deps.js +++ b/web/js/lib/comfyui-dom-deps.js @@ -30,14 +30,14 @@ export const VERIFIED_FRONTENDS = ["1.47.12", "1.50.3"]; export const COMFYUI_DOM_DEPS = [ { selector: ".side-bar-button-selected", - why: "Which sidebar tab is currently selected — the guard that detaches our root when another tab is active.", - fallback: "None. An unreadable selection is treated as UNKNOWN and changes nothing (#784).", + why: "Which sidebar tab is currently selected — read by the guard that detaches our root when another tab is active, and by the render watchdog that reports a selected-but-never-painted tab.", + fallback: "None. An unreadable selection is treated as UNKNOWN and changes nothing (#784); the watchdog likewise never speaks on UNKNOWN.", verified: ["1.47.12", "1.50.3"], }, { selector: ".side-tool-bar-container", - why: "The sidebar rail, observed for tab-selection changes.", - fallback: "Retried for ~10s while the frontend boots; absent means the guard never arms.", + why: "The sidebar rail, observed for tab-selection changes by the guard and by the render watchdog (which also waits on it before judging that our tab button never appeared).", + fallback: "Retried while the frontend boots; absent means the guard never arms and the watchdog stays silent (no rail, no evidence).", verified: ["1.47.12", "1.50.3"], }, { diff --git a/web/js/lib/sidebar-render-watchdog.js b/web/js/lib/sidebar-render-watchdog.js new file mode 100644 index 00000000..56dfe092 --- /dev/null +++ b/web/js/lib/sidebar-render-watchdog.js @@ -0,0 +1,341 @@ +/** + * panel#779 — if the Agent tab is open and nothing of ours is painted, SAY SO. + * + * The outage this grew from: a new user's tab registered, was selectable, and + * stayed a black rectangle — `.cmcp-root` absent, nothing in the console + * attributed to us. The cause (our own guard deleting the root on an + * unidentifiable tab marker, #784) is fixed, and #785 added a visible shell for + * a render() that THROWS. But a render() that is never CALLED — which is what + * an actual sidebar-tab contract change in a future frontend would produce — + * still fails in perfect silence. The reporter's natural response to that + * silence was an hour of reinstalling things that were never the problem. + * + * This watchdog turns that silence into one console line that names the panel + * version, the frontend version, and what to do. It deliberately arrives + * SECONDS after the failure, not instants: its job is a support answer, not a + * race. + * + * TWO CHECKS, BOTH EVIDENCE-ONLY (the #784 lesson applies to diagnostics too: + * "I cannot tell" must never be reported as "it is broken"): + * + * 1. STARVATION — our tab is PROVABLY the selected one (the rail button carries + * our id, read the same dual-generation way the guard reads it) and yet no + * `.cmcp-root` and no `.cmcp-failure-shell` exists, continuously for + * RENDER_STARVATION_MS, re-verified at expiry. When the selected tab is + * another tab, or unidentifiable ("unknown"), the check DISARMS rather than + * counts — an unreadable marker is not evidence of our failure. The first + * successful paint while our tab is active retires the check for the page's + * lifetime: its charter is first-paint failure, the #779 class; content that + * later disappears is a different bug with a different symptom. + * + * 2. APPEARANCE — the sidebar rail exists but our tab button never showed up in + * it within TAB_APPEAR_DEADLINE_MS of the rail being seen. This is what a + * frontend that accepts registerSidebarTab() and silently drops the legacy + * spec shape would look like: no tab to click, so check 1 can never trigger, + * and the panel simply vanishes from the product. If the rail itself cannot + * be found, that is "I cannot tell", and the check gives up silently. + * + * WHY THE BOUNDS ARE WHAT THEY ARE. Render is invoked synchronously when the + * tab mounts, so 3s of selected-and-empty is not "a slow machine", it is a + * no-show — and a machine so loaded that timers stall does not fire early, + * because the deadline itself is a timer. The rail populates in the same + * render pass that creates it, so 10s from first sighting is generous. + */ + +import { readActiveSidebarTab, findSidebarTabButton } from "./active-sidebar-tab.js"; +import { VERIFIED_FRONTENDS } from "./comfyui-dom-deps.js"; + +/** Continuous selected-but-empty time before the starvation line is spoken. */ +export const RENDER_STARVATION_MS = 3000; +/** Rail seen → our button still absent for this long = the appearance line. */ +export const TAB_APPEAR_DEADLINE_MS = 10000; +/** Poll cadence for the appearance check (also re-samples starvation). */ +export const WATCHDOG_POLL_MS = 500; +/** Stop polling entirely this long after install — a page with no rail by then + * is not going to grow one, and an observerless page costs nothing forever. */ +export const WATCHDOG_GIVE_UP_MS = 60000; + +const ISSUES_URL = "https://github.com/artokun/comfyui-mcp-panel/issues"; + +/** "1.47.12 and 1.50.3", however many entries the registry carries. */ +function verifiedFrontendList() { + const list = VERIFIED_FRONTENDS.slice(); + if (list.length === 0) return "a released frontend"; + if (list.length === 1) return list[0]; + return `${list.slice(0, -1).join(", ")} and ${list[list.length - 1]}`; +} + +/** The remedy sentence both reports end with — one wording, one place. */ +function remedyText() { + const pin = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1] || "1.50.3"; + return ( + `This is NOT a connection problem, and reinstalling the pack or ComfyUI cannot change it. ` + + `Please report it at ${ISSUES_URL} and include both version numbers from this message. ` + + `Until it is fixed, relaunching ComfyUI with ` + + `--front-end-version comfyanonymous/ComfyUI@${pin} restores the panel ` + + `(frontends ${verifiedFrontendList()} are verified to render this panel version).` + ); +} + +/** + * The console line for "selected, and nothing painted". + * + * Wording rules, learned the hard way in this issue: report what was OBSERVED, + * never a guessed cause; name both versions, because the frontend version is + * the field a reporter is least likely to think to include; and close off the + * two remedies that cannot work before anyone spends an hour on them. + * + * @param {{ panelVersion?: string, frontendVersion?: string, waitedMs?: number }} [info] + */ +export function renderStarvationReport(info = {}) { + const p = info.panelVersion || "unknown"; + const f = info.frontendVersion || "unknown"; + const s = Math.round((info.waitedMs ?? RENDER_STARVATION_MS) / 1000); + return ( + `[comfyui-mcp-panel] the Agent tab has been selected for ~${s}s but no panel content exists ` + + `(no .cmcp-root in the document). The tab registered and was selected, yet the panel was ` + + `either never asked to render or its content was removed as soon as it was attached. ` + + `That is a compatibility fault between panel ${p} and ComfyUI frontend ${f}. ` + + remedyText() + ); +} + +/** + * The console line for "registered, and the tab button never appeared". + * + * @param {{ panelVersion?: string, frontendVersion?: string, waitedMs?: number }} [info] + */ +export function tabNeverAppearedReport(info = {}) { + const p = info.panelVersion || "unknown"; + const f = info.frontendVersion || "unknown"; + const s = Math.round((info.waitedMs ?? TAB_APPEAR_DEADLINE_MS) / 1000); + return ( + `[comfyui-mcp-panel] registerSidebarTab() accepted the Agent tab, but its button never ` + + `appeared in the sidebar rail (waited ~${s}s after the rail was seen). This frontend most ` + + `likely changed how a custom sidebar tab is declared, in a way panel ${p} does not speak ` + + `yet — ComfyUI frontend here is ${f}. ` + + remedyText() + ); +} + +/** + * The starvation state machine, pure so it can be tested at second-boundaries + * without a DOM or a clock. + * + * Feed it observations; it answers with the state they produce: + * "idle" not our tab / painted / nothing to watch — any timer can drop + * "armed" our tab is active and empty; `waited` ms so far + * "fired" the window elapsed with the condition continuously true — + * onStarve(waitedMs) was invoked exactly once, ever + * "satisfied" our tab painted while active; the watchdog retires for good + * + * @param {{ tabId: string, windowMs?: number, onStarve?: (waitedMs: number) => void }} opts + */ +export function createRenderWatchdog({ tabId, windowMs = RENDER_STARVATION_MS, onStarve } = {}) { + let armedAt = null; + let done = false; // fired OR satisfied — either way, permanently over + let firedEver = false; + + return { + fired: () => firedEver, + done: () => done, + /** + * @param {{state:string, id?:string}|null|undefined} active as returned by + * readActiveSidebarTab — "none" / "unknown" / {state:"id", id}. + * @param {boolean} painted is any of our content connected right now? + * @param {number} at a monotonic-enough clock (Date.now()). + * @returns {{ state: "idle"|"armed"|"fired"|"satisfied", waited?: number }} + */ + sample(active, painted, at) { + if (done) return { state: firedEver ? "fired" : "satisfied" }; + const ours = !!active && active.state === "id" && active.id === tabId; + if (ours && painted) { + // First proven paint: the contract works here. Retire — later blanks + // are different bugs and get different (visible) symptoms. + done = true; + armedAt = null; + return { state: "satisfied" }; + } + if (!ours) { + // Another tab, no tab, or a marker we cannot read. None of these is + // evidence about US — disarm rather than count (#784's rule). + armedAt = null; + return { state: "idle" }; + } + // Ours, and empty. + if (armedAt == null) armedAt = at; + const waited = at - armedAt; + if (waited >= windowMs) { + done = true; + firedEver = true; + try { + onStarve?.(waited); + } catch { + /* a reporter that throws must not take the page down */ + } + return { state: "fired", waited }; + } + return { state: "armed", waited }; + }, + }; +} + +/** + * Wire the watchdog to a live document. + * + * Injection points exist for the tests; every default is the real thing. The + * return value exposes `sample()` (so a hosting page or test can nudge it) and + * `stop()` (detach everything). + * + * @param {object} opts + * @param {string} opts.tabId + * @param {() => boolean} opts.isPainted is our content connected right now? + * @param {string} [opts.panelVersion] + * @param {() => (string|undefined)} [opts.getFrontendVersion] read at fire time. + * @param {Document} [opts.doc] + * @param {(line: string) => void} [opts.report] default console.error. + * @param {(cb: () => void) => { observe: Function, disconnect: Function }|null} [opts.makeObserver] + * @param {(fn: () => void, ms: number) => unknown} [opts.setTimer] + * @param {(h: unknown) => void} [opts.clearTimer] + * @param {() => number} [opts.now] + * @param {number} [opts.windowMs] + * @param {number} [opts.appearDeadlineMs] + * @param {number} [opts.pollMs] + * @param {number} [opts.giveUpMs] + */ +export function installSidebarRenderWatchdog({ + tabId, + isPainted, + panelVersion, + getFrontendVersion = () => undefined, + doc = typeof document !== "undefined" ? document : null, + report = (line) => console.error(line), + makeObserver = (cb) => + typeof MutationObserver === "function" ? new MutationObserver(cb) : null, + setTimer = (fn, ms) => setTimeout(fn, ms), + clearTimer = (h) => clearTimeout(h), + now = () => Date.now(), + windowMs = RENDER_STARVATION_MS, + appearDeadlineMs = TAB_APPEAR_DEADLINE_MS, + pollMs = WATCHDOG_POLL_MS, + giveUpMs = WATCHDOG_GIVE_UP_MS, +} = {}) { + if (!doc || typeof isPainted !== "function" || !tabId) return null; + + const versions = (waitedMs) => ({ + panelVersion, + frontendVersion: (() => { + try { + return getFrontendVersion(); + } catch { + return undefined; + } + })(), + waitedMs, + }); + + let stopped = false; + let observer = null; + let expiryTimer = null; + let pollTimer = null; + const startedAt = now(); + let railSeenAt = null; + let buttonEverSeen = false; + let appearanceSpoken = false; + + const machine = createRenderWatchdog({ + tabId, + windowMs, + onStarve: (waitedMs) => report(renderStarvationReport(versions(waitedMs))), + }); + + const stop = () => { + stopped = true; + if (observer) { + try { + observer.disconnect(); + } catch { /* an observer that cannot disconnect is already gone */ } + observer = null; + } + if (expiryTimer != null) { + clearTimer(expiryTimer); + expiryTimer = null; + } + if (pollTimer != null) { + clearTimer(pollTimer); + pollTimer = null; + } + }; + + const sample = () => { + if (stopped) { + // Report the resting state honestly: how it ended, or that it merely + // gave up ("stopped") without ever having evidence either way. + return { state: machine.done() ? (machine.fired() ? "fired" : "satisfied") : "stopped" }; + } + const active = readActiveSidebarTab(doc.querySelector(".side-bar-button-selected")); + const res = machine.sample(active, !!isPainted(), now()); + if (res.state === "armed") { + if (expiryTimer == null) { + // Re-verify AT the deadline rather than firing blind: everything may + // have changed since arming, and only a fresh look is evidence. + const delay = Math.max(windowMs - (res.waited ?? 0), 0) + 80; + expiryTimer = setTimer(() => { + expiryTimer = null; + sample(); + }, delay); + } + } else { + if (expiryTimer != null) { + clearTimer(expiryTimer); + expiryTimer = null; + } + if (res.state === "fired" || res.state === "satisfied") stop(); + } + return res; + }; + + const pollAppearance = () => { + if (stopped) return; + pollTimer = null; + const t = now(); + const rail = doc.querySelector(".side-tool-bar-container"); + if (rail) { + if (railSeenAt == null) { + railSeenAt = t; + // The rail exists — from here on, selection changes are observable. + // Same subscription the sidebar guard uses: tab selection toggles a + // class on the rail's buttons in every frontend generation we know. + observer = makeObserver(() => sample()); + if (observer) { + try { + observer.observe(rail, { subtree: true, attributes: true, attributeFilter: ["class"] }); + } catch { + observer = null; // fall back to the poll below + } + } + } + if (!buttonEverSeen && findSidebarTabButton(doc, tabId)) buttonEverSeen = true; + if (!buttonEverSeen && !appearanceSpoken && t - railSeenAt >= appearDeadlineMs) { + appearanceSpoken = true; + report(tabNeverAppearedReport(versions(t - railSeenAt))); + // No button means no way to select the tab: the starvation check can + // never trigger, so there is nothing left to watch. + stop(); + return; + } + } + // The poll doubles as a low-rate starvation re-sample, so a frontend whose + // rail stops emitting class mutations does not blind check 1 entirely. + if (railSeenAt != null) sample(); + if (stopped) return; + const buttonPhaseOver = buttonEverSeen || appearanceSpoken; + const observerCarriesOn = observer != null && buttonPhaseOver; + if (t - startedAt >= giveUpMs || observerCarriesOn) return; // observer (or silence) from here + pollTimer = setTimer(pollAppearance, pollMs); + }; + + sample(); + pollAppearance(); + return { sample, stop }; +} From f42cde71515177f51174fe259a3eabd6860983d7 Mon Sep 17 00:00:00 2001 From: Artokun Date: Sat, 8 Aug 2026 10:15:16 -0700 Subject: [PATCH 2/6] fix(#779 watchdog): name the frontend repo the flag actually fetches from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paste-able workaround told the user to run --front-end-version comfyanonymous/ComfyUI@ That flag fetches releases from the repo it names, and the frontend's 1.x tags exist only in Comfy-Org/ComfyUI_frontend. Pointing it at comfyanonymous/ComfyUI does not error — it silently falls back to the installed default, so it appears to work in exactly the case where it did nothing. Verified live against ComfyUI 0.30.2 while working this issue. A remedy that looks like it worked is worse than no remedy: it retires the question. Corrected to Comfy-Org/ComfyUI_frontend@, with the reason recorded beside the assertion so the spelling cannot quietly revert. Refs #779 Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/sidebar-render-watchdog.test.mjs | 84 +++++++++++++++-- web/js/lib/sidebar-render-watchdog.js | 93 ++++++++++++++----- 2 files changed, 145 insertions(+), 32 deletions(-) diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs index b4301844..3f65f3bc 100644 --- a/browser_tests/unit/sidebar-render-watchdog.test.mjs +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -22,6 +22,7 @@ import { dirname, join } from "node:path"; import { RENDER_STARVATION_MS, + SATISFY_CONFIRM_MS, TAB_APPEAR_DEADLINE_MS, WATCHDOG_POLL_MS, WATCHDOG_GIVE_UP_MS, @@ -55,7 +56,12 @@ test("#779 the starvation line carries everything a support answer needs", () => assert.match(line, /NOT a connection problem/i, "dead end #1, closed"); assert.match(line, /reinstalling.*cannot change it/i, "dead end #2 — the one that cost an hour"); assert.match(line, /github\.com\/artokun\/comfyui-mcp-panel\/issues/, "where to send it"); - assert.match(line, /--front-end-version comfyanonymous\/ComfyUI@/, "the workaround, in paste-able form"); + // Comfy-Org/ComfyUI_frontend, NOT comfyanonymous/ComfyUI: the flag fetches + // releases from the named repo, and the frontend's 1.x tags only exist in the + // frontend repo — the other spelling silently falls back to the installed + // default and appears to work exactly when it did nothing (verified live + // against ComfyUI 0.30.2 while fixing this issue). + assert.match(line, /--front-end-version Comfy-Org\/ComfyUI_frontend@/, "the workaround, in paste-able form"); }); test("#779 the appearance line is distinct and equally complete", () => { @@ -71,7 +77,7 @@ test("#779 the appearance line is distinct and equally complete", () => { assert.match(line, /1\.53\.0/); assert.match(line, /NOT a connection problem/i); assert.match(line, /github\.com\/artokun\/comfyui-mcp-panel\/issues/); - assert.match(line, /--front-end-version comfyanonymous\/ComfyUI@/); + assert.match(line, /--front-end-version Comfy-Org\/ComfyUI_frontend@/); }); test("#779 unknown versions say 'unknown' — never a guess", () => { @@ -86,7 +92,7 @@ test("#779 the workaround pin is a VERIFIED frontend, not a hardcoded relic", () const newest = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1]; const line = renderStarvationReport({}); assert.ok( - line.includes(`--front-end-version comfyanonymous/ComfyUI@${newest}`), + line.includes(`--front-end-version Comfy-Org/ComfyUI_frontend@${newest}`), `the recommended pin should be ${newest} (the newest verified frontend)`, ); for (const v of VERIFIED_FRONTENDS) { @@ -99,6 +105,7 @@ test("#779 the workaround pin is a VERIFIED frontend, not a hardcoded relic", () // --------------------------------------------------------------------------- const WINDOW = RENDER_STARVATION_MS; +const CONFIRM = SATISFY_CONFIRM_MS; const ours = { state: "id", id: OURS }; const other = { state: "id", id: "workflows" }; const unknown = { state: "unknown" }; @@ -108,20 +115,52 @@ function machine(onStarve = () => {}) { return createRenderWatchdog({ tabId: OURS, onStarve }); } -test("#779 the healthy path retires the watchdog for good", () => { +test("#779 the healthy path retires the watchdog — after the paint SURVIVES", () => { const m = machine(() => assert.fail("must not fire")); - assert.equal(m.sample(ours, true, 0).state, "satisfied"); + // One glimpse of paint is only "verifying" — the real #779 removed the root + // instants after render() attached it. + assert.equal(m.sample(ours, true, 0).state, "verifying"); + assert.equal(m.sample(ours, true, CONFIRM - 1).state, "verifying"); + assert.equal(m.sample(ours, true, CONFIRM).state, "satisfied"); // Retired means RETIRED: even a later selected-and-empty eternity says nothing. assert.equal(m.sample(ours, false, WINDOW * 100).state, "satisfied"); assert.equal(m.fired(), false); }); +test("#779 paint-then-instant-removal STILL fires — the shape of the actual outage", () => { + // Live-drill regression: a saboteur that reproduced the pre-#784 guard + // (remove .cmcp-root the moment render attaches it) put the first draft of + // this watchdog to sleep, because its rail observer glimpsed the root in the + // instant between attach and removal and retired on that single sample. The + // glimpse must not count: only a paint that survives the confirmation dwell + // retires the watchdog. + let fired = 0; + const m = machine(() => (fired += 1)); + m.sample(ours, false, 0); // armed on selection + assert.equal(m.sample(ours, true, 10).state, "verifying"); // the glimpse + assert.equal(m.sample(ours, false, 20).state, "armed"); // …and it is gone + assert.equal(m.sample(ours, false, 20 + WINDOW).state, "fired"); + assert.equal(fired, 1); +}); + test("#779 selected-and-empty shorter than the window never fires (slow first build)", () => { const m = machine(() => assert.fail("must not fire")); assert.equal(m.sample(ours, false, 0).state, "armed"); assert.equal(m.sample(ours, false, WINDOW - 1).state, "armed"); // The build lands just inside the deadline — a loaded machine, not a fault. - assert.equal(m.sample(ours, true, WINDOW - 1).state, "satisfied"); + assert.equal(m.sample(ours, true, WINDOW - 1).state, "verifying"); + assert.equal(m.sample(ours, true, WINDOW - 1 + CONFIRM).state, "satisfied"); +}); + +test("#779 an interrupted confirmation dwell does not retire — it re-evaluates later", () => { + const m = machine(() => assert.fail("must not fire")); + assert.equal(m.sample(ours, true, 0).state, "verifying"); + // The user wanders off before the dwell completes. The paint was probably + // real, but PROBABLY is not the retirement bar — stay alive, stay quiet. + assert.equal(m.sample(other, false, 100).state, "idle"); + // Next dwell starts the confirmation over and completes it. + assert.equal(m.sample(ours, true, 5000).state, "verifying"); + assert.equal(m.sample(ours, true, 5000 + CONFIRM).state, "satisfied"); }); test("#779 a full continuous window fires exactly once, with the waited time", () => { @@ -214,7 +253,13 @@ function legacyButton(id) { * The whole harness: fake doc, fake timers, captured reports, an observer stub * whose callback we can pull. `state` is mutated by the tests to move the world. */ -function harness({ windowMs = 300, appearDeadlineMs = 1000, pollMs = 50, giveUpMs = 6000 } = {}) { +function harness({ + windowMs = 300, + confirmMs = 200, + appearDeadlineMs = 1000, + pollMs = 50, + giveUpMs = 6000, +} = {}) { const state = { rail: null, // truthy once the rail exists button: false, // our tab button present in the rail? @@ -261,6 +306,7 @@ function harness({ windowMs = 300, appearDeadlineMs = 1000, pollMs = 50, giveUpM }, now: () => clock, windowMs, + confirmMs, appearDeadlineMs, pollMs, giveUpMs, @@ -297,12 +343,36 @@ test("#779 installer: the healthy first open reports nothing and stands down", ( h.state.painted = true; // render() attached the root, as it should h.mutate(); // the selection class change assert.equal(h.reports.length, 0); + assert.equal(h.handle.sample().state, "verifying", "one glimpse is not proof"); + h.advance(400); // the paint survives the confirmation dwell assert.equal(h.handle.sample().state, "satisfied"); h.advance(20000); assert.equal(h.reports.length, 0, "a satisfied watchdog never speaks"); assert.equal(h.timersLeft(), 0, "…and holds no timers"); }); +test("#779 installer: the live drill — root attached then instantly ripped out — fires", () => { + // This exact sequence put the first draft to sleep on a real 1.47.12 page: + // render attaches the root, the watchdog's observer glimpses it painted, and + // a saboteur (standing in for the pre-#784 guard) removes it within the same + // flush. The glimpse must leave the watchdog in "verifying", and the removal + // must re-arm it — ending in the one report. + const h = harness(); + h.state.rail = {}; + h.state.button = true; + h.advance(60); + h.state.selected = modernButton(OURS); + h.state.painted = true; // the attach… + h.mutate(); + h.state.painted = false; // …and the same-flush removal + h.mutate(); + h.advance(500); // past windowMs 300 + slack + assert.equal(h.reports.length, 1); + assert.match(h.reports[0], /no panel content exists/); + h.advance(20000); + assert.equal(h.reports.length, 1, "still exactly one line"); +}); + test("#779 installer: selected-but-never-painted produces EXACTLY the one line", () => { const h = harness(); h.state.rail = {}; diff --git a/web/js/lib/sidebar-render-watchdog.js b/web/js/lib/sidebar-render-watchdog.js index 56dfe092..06e76a7b 100644 --- a/web/js/lib/sidebar-render-watchdog.js +++ b/web/js/lib/sidebar-render-watchdog.js @@ -23,10 +23,13 @@ * `.cmcp-root` and no `.cmcp-failure-shell` exists, continuously for * RENDER_STARVATION_MS, re-verified at expiry. When the selected tab is * another tab, or unidentifiable ("unknown"), the check DISARMS rather than - * counts — an unreadable marker is not evidence of our failure. The first - * successful paint while our tab is active retires the check for the page's - * lifetime: its charter is first-paint failure, the #779 class; content that - * later disappears is a different bug with a different symptom. + * counts — an unreadable marker is not evidence of our failure. A paint that + * SURVIVES SATISFY_CONFIRM_MS while our tab is active retires the check for + * the page's lifetime: its charter is first-paint failure, the #779 class; + * content that disappears after a confirmed dwell is a different bug with a + * different symptom. A mere glimpse of paint retires nothing — the actual + * #779 attached the root and removed it in the same mutation flush, and a + * live drill proved a glimpse-trusting watchdog sleeps through it. * * 2. APPEARANCE — the sidebar rail exists but our tab button never showed up in * it within TAB_APPEAR_DEADLINE_MS of the rail being seen. This is what a @@ -47,6 +50,13 @@ import { VERIFIED_FRONTENDS } from "./comfyui-dom-deps.js"; /** Continuous selected-but-empty time before the starvation line is spoken. */ export const RENDER_STARVATION_MS = 3000; +/** A paint must SURVIVE this long before it retires the watchdog. The actual + * #779 failure attached the root and removed it within the same mutation + * flush — to a single sample that instant is indistinguishable from healthy. + * Found live: a drill that reproduced the historical remove-on-attach retired + * the first draft of this watchdog instead of firing it. Never trust one + * glimpse of paint. */ +export const SATISFY_CONFIRM_MS = 1500; /** Rail seen → our button still absent for this long = the appearance line. */ export const TAB_APPEAR_DEADLINE_MS = 10000; /** Poll cadence for the appearance check (also re-samples starvation). */ @@ -65,14 +75,21 @@ function verifiedFrontendList() { return `${list.slice(0, -1).join(", ")} and ${list[list.length - 1]}`; } -/** The remedy sentence both reports end with — one wording, one place. */ +/** The remedy sentence both reports end with — one wording, one place. + * + * The pin names Comfy-Org/ComfyUI_frontend, NOT comfyanonymous/ComfyUI: the + * flag fetches GitHub releases from the named repo, and the frontend's 1.x + * release tags exist only in the frontend repo. A comfyanonymous/ComfyUI pin + * quietly falls back to whatever frontend package is installed — it appears + * to work exactly when it did nothing. Verified live against ComfyUI 0.30.2's + * frontend_management.py while fixing #779. */ function remedyText() { const pin = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1] || "1.50.3"; return ( `This is NOT a connection problem, and reinstalling the pack or ComfyUI cannot change it. ` + `Please report it at ${ISSUES_URL} and include both version numbers from this message. ` + `Until it is fixed, relaunching ComfyUI with ` + - `--front-end-version comfyanonymous/ComfyUI@${pin} restores the panel ` + + `--front-end-version Comfy-Org/ComfyUI_frontend@${pin} restores the panel ` + `(frontends ${verifiedFrontendList()} are verified to render this panel version).` ); } @@ -123,16 +140,27 @@ export function tabNeverAppearedReport(info = {}) { * without a DOM or a clock. * * Feed it observations; it answers with the state they produce: - * "idle" not our tab / painted / nothing to watch — any timer can drop + * "idle" not our tab / nothing to watch — any timer can drop * "armed" our tab is active and empty; `waited` ms so far - * "fired" the window elapsed with the condition continuously true — + * "verifying" our tab painted, and the paint has not yet SURVIVED + * SATISFY_CONFIRM_MS — the actual #779 bug attached and removed + * the root within one mutation flush, so one glimpse of paint + * proves nothing; `waited` ms of confirmed dwell so far + * "fired" the window elapsed with selected-and-empty continuously true — * onStarve(waitedMs) was invoked exactly once, ever - * "satisfied" our tab painted while active; the watchdog retires for good + * "satisfied" the paint survived the confirmation dwell while our tab was + * active; the watchdog retires for good * - * @param {{ tabId: string, windowMs?: number, onStarve?: (waitedMs: number) => void }} opts + * @param {{ tabId: string, windowMs?: number, confirmMs?: number, onStarve?: (waitedMs: number) => void }} opts */ -export function createRenderWatchdog({ tabId, windowMs = RENDER_STARVATION_MS, onStarve } = {}) { +export function createRenderWatchdog({ + tabId, + windowMs = RENDER_STARVATION_MS, + confirmMs = SATISFY_CONFIRM_MS, + onStarve, +} = {}) { let armedAt = null; + let paintedAt = null; let done = false; // fired OR satisfied — either way, permanently over let firedEver = false; @@ -144,25 +172,35 @@ export function createRenderWatchdog({ tabId, windowMs = RENDER_STARVATION_MS, o * readActiveSidebarTab — "none" / "unknown" / {state:"id", id}. * @param {boolean} painted is any of our content connected right now? * @param {number} at a monotonic-enough clock (Date.now()). - * @returns {{ state: "idle"|"armed"|"fired"|"satisfied", waited?: number }} + * @returns {{ state: "idle"|"armed"|"verifying"|"fired"|"satisfied", waited?: number }} */ sample(active, painted, at) { if (done) return { state: firedEver ? "fired" : "satisfied" }; const ours = !!active && active.state === "id" && active.id === tabId; - if (ours && painted) { - // First proven paint: the contract works here. Retire — later blanks - // are different bugs and get different (visible) symptoms. - done = true; - armedAt = null; - return { state: "satisfied" }; - } if (!ours) { // Another tab, no tab, or a marker we cannot read. None of these is - // evidence about US — disarm rather than count (#784's rule). + // evidence about US — disarm rather than count (#784's rule). An + // interrupted confirmation dwell does NOT retire: stay alive and + // re-evaluate on the next dwell. armedAt = null; + paintedAt = null; return { state: "idle" }; } - // Ours, and empty. + if (painted) { + // Painted — but a paint only counts once it has SURVIVED. The real + // #779 removed the root instants after render() attached it, and a + // watchdog that retired on the glimpse missed the entire outage. + armedAt = null; + if (paintedAt == null) paintedAt = at; + const dwell = at - paintedAt; + if (dwell >= confirmMs) { + done = true; + return { state: "satisfied" }; + } + return { state: "verifying", waited: dwell }; + } + // Ours, and empty. A prior unconfirmed paint is void. + paintedAt = null; if (armedAt == null) armedAt = at; const waited = at - armedAt; if (waited >= windowMs) { @@ -216,6 +254,7 @@ export function installSidebarRenderWatchdog({ clearTimer = (h) => clearTimeout(h), now = () => Date.now(), windowMs = RENDER_STARVATION_MS, + confirmMs = SATISFY_CONFIRM_MS, appearDeadlineMs = TAB_APPEAR_DEADLINE_MS, pollMs = WATCHDOG_POLL_MS, giveUpMs = WATCHDOG_GIVE_UP_MS, @@ -246,6 +285,7 @@ export function installSidebarRenderWatchdog({ const machine = createRenderWatchdog({ tabId, windowMs, + confirmMs, onStarve: (waitedMs) => report(renderStarvationReport(versions(waitedMs))), }); @@ -275,11 +315,14 @@ export function installSidebarRenderWatchdog({ } const active = readActiveSidebarTab(doc.querySelector(".side-bar-button-selected")); const res = machine.sample(active, !!isPainted(), now()); - if (res.state === "armed") { + if (res.state === "armed" || res.state === "verifying") { if (expiryTimer == null) { - // Re-verify AT the deadline rather than firing blind: everything may - // have changed since arming, and only a fresh look is evidence. - const delay = Math.max(windowMs - (res.waited ?? 0), 0) + 80; + // Re-verify AT the deadline rather than deciding blind: everything may + // have changed since this sample, and only a fresh look is evidence. + // "armed" re-checks at the starvation deadline; "verifying" re-checks + // when the paint would have survived long enough to count. + const horizon = res.state === "armed" ? windowMs : confirmMs; + const delay = Math.max(horizon - (res.waited ?? 0), 0) + 80; expiryTimer = setTimer(() => { expiryTimer = null; sample(); From 824ce9ffb1fddb0cff27c16d27f4a3278424d7fb Mon Sep 17 00:00:00 2001 From: Artokun Date: Sat, 8 Aug 2026 10:21:42 -0700 Subject: [PATCH 3/6] =?UTF-8?q?fix(watchdog):=20a=20glimpse=20of=20paint?= =?UTF-8?q?=20proves=20nothing=20=E2=80=94=20confirm=20the=20dwell,=20and?= =?UTF-8?q?=20pin=20a=20frontend=20that=20is=20not=20the=20failing=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found by running the watchdog against a LIVE page rather than trusting the unit harness, plus one wrong fact inherited from the issue thread. 1. GLIMPSE RETIREMENT (live drill, 1.47.12). A saboteur reproducing the actual pre-#784 failure — remove .cmcp-root the instant render() attaches it — put the first draft to SLEEP instead of firing it: the rail observer sampled in the instant between attach and removal, saw painted, and retired for the page's lifetime. The exact historical outage shape defeated the diagnostic built for it. A paint now only retires the watchdog after it SURVIVES SATISFY_CONFIRM_MS (1.5s) while our tab is active; an interrupted dwell keeps watching. Re-drilled live on both 1.47.12 and 1.50.3: exactly one line, correct versions, healthy paths still silent. 2. WRONG PIN REPO (live launch). The remedy inherited the thread's workaround string, --front-end-version comfyanonymous/ComfyUI@ — which CANNOT work: the flag fetches GitHub releases from the named repo, the frontend's 1.x tags exist only in Comfy-Org/ComfyUI_frontend, and the not-found path silently falls back to the installed default package (verified against ComfyUI 0.30.2's frontend_management.py, and by watching the fallback happen in a live boot log). A diagnostic that names a no-op remedy manufactures false confidence; it now names the repo that resolves. 3. SELF-REFERENTIAL PIN (live drill, 1.50.3). With the failing frontend equal to the newest verified one, the remedy told the user to pin the version they were already running. The pin now prefers the newest verified frontend that DIFFERS from the one being reported. Live verification of the whole branch, isolated second ComfyUI (CPU, scratch base dir, this branch via junction): panel renders and keep-alive survives tab switches on REAL 1.50.3 (the check the #784 thread recorded as still owed) and on 1.47.12; watchdog silent on every healthy path; fires exactly once under the drill on both frontends. Owner's live rig (1.47.12, main) also verified rendering, read-only. Refs #779. Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/sidebar-render-watchdog.test.mjs | 14 ++++++++++++++ web/js/lib/sidebar-render-watchdog.js | 17 ++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs index 3f65f3bc..b6ce3565 100644 --- a/browser_tests/unit/sidebar-render-watchdog.test.mjs +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -100,6 +100,20 @@ test("#779 the workaround pin is a VERIFIED frontend, not a hardcoded relic", () } }); +test("#779 the pin never recommends the frontend that is failing right now", () => { + // Live-drill finding: with the failing frontend equal to the newest verified + // one, a newest-only pin told the user to pin the version they were already + // on. Advice-shaped noise — prefer the newest verified frontend that DIFFERS. + const newest = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1]; + const previous = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 2]; + const line = renderStarvationReport({ frontendVersion: newest }); + assert.ok( + line.includes(`--front-end-version Comfy-Org/ComfyUI_frontend@${previous}`), + `running ${newest}, the pin should back off to ${previous}`, + ); + assert.ok(!line.includes(`ComfyUI_frontend@${newest} `), "never the failing version itself"); +}); + // --------------------------------------------------------------------------- // The state machine. Times in ms; WINDOW below for readability. // --------------------------------------------------------------------------- diff --git a/web/js/lib/sidebar-render-watchdog.js b/web/js/lib/sidebar-render-watchdog.js index 06e76a7b..99b832a1 100644 --- a/web/js/lib/sidebar-render-watchdog.js +++ b/web/js/lib/sidebar-render-watchdog.js @@ -82,9 +82,16 @@ function verifiedFrontendList() { * release tags exist only in the frontend repo. A comfyanonymous/ComfyUI pin * quietly falls back to whatever frontend package is installed — it appears * to work exactly when it did nothing. Verified live against ComfyUI 0.30.2's - * frontend_management.py while fixing #779. */ -function remedyText() { - const pin = VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1] || "1.50.3"; + * frontend_management.py while fixing #779. + * + * The pin prefers the newest verified frontend that DIFFERS from the one + * being reported: telling someone whose frontend is failing to pin that very + * version would be advice-shaped noise. + * + * @param {string|undefined} runningFrontend the version being reported on. */ +function remedyText(runningFrontend) { + const known = VERIFIED_FRONTENDS.filter((v) => v !== runningFrontend); + const pin = known[known.length - 1] || VERIFIED_FRONTENDS[VERIFIED_FRONTENDS.length - 1] || "1.50.3"; return ( `This is NOT a connection problem, and reinstalling the pack or ComfyUI cannot change it. ` + `Please report it at ${ISSUES_URL} and include both version numbers from this message. ` + @@ -113,7 +120,7 @@ export function renderStarvationReport(info = {}) { `(no .cmcp-root in the document). The tab registered and was selected, yet the panel was ` + `either never asked to render or its content was removed as soon as it was attached. ` + `That is a compatibility fault between panel ${p} and ComfyUI frontend ${f}. ` + - remedyText() + remedyText(info.frontendVersion) ); } @@ -131,7 +138,7 @@ export function tabNeverAppearedReport(info = {}) { `appeared in the sidebar rail (waited ~${s}s after the rail was seen). This frontend most ` + `likely changed how a custom sidebar tab is declared, in a way panel ${p} does not speak ` + `yet — ComfyUI frontend here is ${f}. ` + - remedyText() + remedyText(info.frontendVersion) ); } From 16019da4071275329844143cfdbfc353fe08577a Mon Sep 17 00:00:00 2001 From: Artokun Date: Sat, 8 Aug 2026 10:29:46 -0700 Subject: [PATCH 4/6] =?UTF-8?q?fix(watchdog):=20no=20rail=20seen=20means?= =?UTF-8?q?=20NOTHING=20arms=20=E2=80=94=20and=20the=20appearance=20latch?= =?UTF-8?q?=20is=20a=20decision,=20pinned?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex gate round 1, both findings resolved: - P1 ACCEPTED: the one pre-rail sample could arm the starvation check, and its expiry timer would then speak on a page whose sidebar we never recognized — violating the no-evidence-no-claim rule the appearance half already followed. sample() is now gated on railSeenAt, making 'no rail seen = permanent silence' a single watchdog-wide invariant. No missed fire on real pages: the rail is discovered by the synchronous initial poll and every arming sample was already driven by the rail observer or the post-rail poll. - P1 DECLINED with reasons: 'appearance misses a button that appeared then vanished'. Deliberate — the line says NEVER appeared, and saying it about a button that demonstrably appeared would be a false statement about a different, user-visible symptom (gone, not never-there). The latch now carries the reasoning in a comment and a test pins the silence. Round 2: PASS (both responses verified sound, no new defects). 29 unit tests on the watchdog, full suite 3045/3045. Refs #779. Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/sidebar-render-watchdog.test.mjs | 27 +++++++++++++++++++ web/js/lib/sidebar-render-watchdog.js | 13 +++++++++ 2 files changed, 40 insertions(+) diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs index b6ce3565..0f99822e 100644 --- a/browser_tests/unit/sidebar-render-watchdog.test.mjs +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -480,6 +480,33 @@ test("#779 installer: no rail at all is 'I cannot tell' — permanent silence", assert.equal(h.timersLeft(), 0, "gave up without a word — no rail, no evidence"); }); +test("#779 installer: a selected-and-empty tab on a rail-LESS page still says nothing", () => { + // Codex-gate case: a page with a readable selected-button marker but no + // recognizable rail container. Starvation evidence exists in isolation, but + // "no rail seen" means this is not a sidebar we understand — the whole + // watchdog holds to no-evidence-no-claim, not just the appearance half. + const h = harness(); + h.state.selected = modernButton(OURS); // marked selected, never painted… + h.advance(WATCHDOG_GIVE_UP_MS + 20000); // …forever, on a page with no rail + assert.equal(h.reports.length, 0, "no rail was ever seen, so nothing may speak"); + assert.equal(h.timersLeft(), 0); +}); + +test("#779 installer: a button that appeared and later VANISHED is out of charter — silent", () => { + // Deliberate: the appearance line says "never appeared", and a button that + // demonstrably appeared makes that statement false. A vanished tab is also a + // different user-visible symptom (gone, not never-there). Pinned so the + // latch in pollAppearance reads as a decision, not an oversight. + const h = harness(); + h.state.rail = {}; + h.advance(120); // rail seen, poll running + h.state.button = true; + h.advance(120); // button seen — appearance satisfied, latched + h.state.button = false; // …and now it is gone + h.advance(20000); + assert.equal(h.reports.length, 0); +}); + test("#779 installer: a button that appears late but within the deadline is fine", () => { const h = harness(); h.state.rail = {}; diff --git a/web/js/lib/sidebar-render-watchdog.js b/web/js/lib/sidebar-render-watchdog.js index 99b832a1..118ca41d 100644 --- a/web/js/lib/sidebar-render-watchdog.js +++ b/web/js/lib/sidebar-render-watchdog.js @@ -320,6 +320,12 @@ export function installSidebarRenderWatchdog({ // gave up ("stopped") without ever having evidence either way. return { state: machine.done() ? (machine.fired() ? "fired" : "satisfied") : "stopped" }; } + // No rail seen yet means the page has not shown us a sidebar we understand, + // and NOTHING may arm or speak on such a page — the same "no evidence, no + // claim" rule the appearance check follows (codex gate: without this, the + // one pre-rail sample could arm starvation and its expiry timer would fire + // on a page the watchdog had otherwise sworn silence about). + if (railSeenAt == null) return { state: "idle" }; const active = readActiveSidebarTab(doc.querySelector(".side-bar-button-selected")); const res = machine.sample(active, !!isPainted(), now()); if (res.state === "armed" || res.state === "verifying") { @@ -365,6 +371,13 @@ export function installSidebarRenderWatchdog({ } } } + // `buttonEverSeen` LATCHES, deliberately: the appearance check's charter + // is "registration was accepted and the button NEVER appeared". A button + // that appeared and later vanished is a different phenomenon — visibly + // different to the user (the tab is gone, not never-there), and firing + // "never appeared" about a button that demonstrably appeared would be a + // false statement. That class stays out of scope (codex gate, declined + // with reasons; pinned by test). if (!buttonEverSeen && findSidebarTabButton(doc, tabId)) buttonEverSeen = true; if (!buttonEverSeen && !appearanceSpoken && t - railSeenAt >= appearDeadlineMs) { appearanceSpoken = true; From 919fadc5a55534f381b44d180514a9696650f41b Mon Sep 17 00:00:00 2001 From: Artokun Date: Fri, 14 Aug 2026 17:00:13 -0700 Subject: [PATCH 5/6] =?UTF-8?q?fix(watchdog):=20a=20filtered=20sidebar=20r?= =?UTF-8?q?ail=20is=20not=20a=20broken=20panel=20=E2=80=94=20retire=20the?= =?UTF-8?q?=20appearance=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The starvation half of the #779 silence detector stands. The APPEARANCE half could tell a user whose panel is perfectly healthy that it is broken, so it is removed rather than narrowed. The evidence it rested on does not exist. "The rail is present and our tab button is not in it" was read as "this frontend accepted registerSidebarTab() and silently dropped the legacy spec". But ComfyUI ships a supported view that renders a deliberately FILTERED rail: src/views/LinearView.vue:120 verified in the frontend repo at BOTH v1.50.3 (the version this branch live-verified against) and v1.51.3, and reachable by any user who turns on linear mode, since GraphView.vue renders . SideToolbar filters at RENDER time only — the frontend's own registry (sidebarTabStore.sidebarTabs) still lists our tab there, so registration succeeded exactly as designed while the button is absent by intent. On that view the check fired a compatibility-fault line naming both versions and advising a relaunch pinned to a different frontend: a false statement, the expensive kind, aimed at the same reader who lost an hour to reinstalls. Filtering and a genuine contract break are indistinguishable from the DOM, so under this module's own rule — the #784 lesson that "I cannot tell" must never be reported as "it is broken" — the honest answer is silence. Narrowing was considered and rejected: a registry check cannot discriminate either, because a frontend that registers a tab and then declines to render it looks identical to a filtered rail. Removing the phase also closes the second review finding, that the 60s give-up bound could cancel the 10s appearance window before it was owed. Kept, unchanged: starvation (provably-selected + empty for 3s, re-read at the deadline), the no-rail-no-claim gate, and the one-line-ever contract. The poll now only waits for the rail and stands down once the observer is watching. Tests: the appearance report and its bounds are gone; two behavioural specs pin the filtered-rail shape as silent, and a structural spec fails if a second report — or any reasoning about our rail button — comes back. 29/29 in the file, 4428 in the suite (one pre-existing wall-clock flake in manager-install, green in isolation). Refs artokun/comfyui-mcp-panel#779, PR #804 Reported-by: Copilot code review on PR #804 Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/sidebar-render-watchdog.test.mjs | 111 +++++++------ web/js/lib/comfyui-dom-deps.js | 4 +- web/js/lib/sidebar-render-watchdog.js | 147 ++++++++---------- 3 files changed, 125 insertions(+), 137 deletions(-) diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs index 0f99822e..e1b2a7ef 100644 --- a/browser_tests/unit/sidebar-render-watchdog.test.mjs +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -1,6 +1,5 @@ -// panel#779 — the silence detector: a selected Agent tab with nothing painted, -// or a registered tab whose button never appears, must produce ONE console line -// naming both versions and what to do. +// panel#779 — the silence detector: a selected Agent tab with nothing painted +// must produce ONE console line naming both versions and what to do. // // The outage this grew from failed in perfect silence: tab registered, // selectable, black rectangle, `.cmcp-root` absent, nothing attributed to us. @@ -23,11 +22,9 @@ import { dirname, join } from "node:path"; import { RENDER_STARVATION_MS, SATISFY_CONFIRM_MS, - TAB_APPEAR_DEADLINE_MS, WATCHDOG_POLL_MS, WATCHDOG_GIVE_UP_MS, renderStarvationReport, - tabNeverAppearedReport, createRenderWatchdog, installSidebarRenderWatchdog, } from "../../web/js/lib/sidebar-render-watchdog.js"; @@ -64,22 +61,6 @@ test("#779 the starvation line carries everything a support answer needs", () => assert.match(line, /--front-end-version Comfy-Org\/ComfyUI_frontend@/, "the workaround, in paste-able form"); }); -test("#779 the appearance line is distinct and equally complete", () => { - const line = tabNeverAppearedReport({ - panelVersion: "0.11.44", - frontendVersion: "1.53.0", - waitedMs: 10000, - }); - assert.match(line, /^\[comfyui-mcp-panel\]/); - assert.match(line, /button never\s+appeared/); - assert.match(line, /~10s/); - assert.match(line, /0\.11\.44/); - assert.match(line, /1\.53\.0/); - assert.match(line, /NOT a connection problem/i); - assert.match(line, /github\.com\/artokun\/comfyui-mcp-panel\/issues/); - assert.match(line, /--front-end-version Comfy-Org\/ComfyUI_frontend@/); -}); - test("#779 unknown versions say 'unknown' — never a guess", () => { const line = renderStarvationReport({}); assert.match(line, /panel unknown/); @@ -270,7 +251,6 @@ function legacyButton(id) { function harness({ windowMs = 300, confirmMs = 200, - appearDeadlineMs = 1000, pollMs = 50, giveUpMs = 6000, } = {}) { @@ -321,7 +301,6 @@ function harness({ now: () => clock, windowMs, confirmMs, - appearDeadlineMs, pollMs, giveUpMs, }); @@ -460,17 +439,42 @@ test("#779 installer: the #785 failure shell counts as painted — one voice at assert.equal(h.reports.length, 0); }); -test("#779 installer: a rail with no button for the deadline says the appearance line", () => { +test("#779 installer: a rail our button is FILTERED out of is never reported (LinearView)", () => { + // The regression this replaces an earlier check for (Copilot review, PR #804). + // A rail that exists without our button in it is NOT evidence of a broken + // contract: ComfyUI ships a supported view that renders a deliberately + // filtered rail — src/views/LinearView.vue mounts + // + // (present at frontend v1.50.3 AND v1.51.3; reached whenever the user turns + // on linear mode, since GraphView.vue renders ). + // There, registration succeeded — the frontend's own sidebarTabStore still + // lists us, `visibleTabIds` filters only at RENDER time — so reporting "your + // panel is broken, relaunch pinned to another frontend" would be a false + // statement aimed at a user whose panel is perfectly healthy. + // + // Filtering and a genuine contract break are indistinguishable from the DOM, + // so the honest answer is silence — the same no-evidence-no-claim rule the + // starvation check follows for an unreadable marker (#784). const h = harness(); h.state.rail = {}; // the rail exists… - // …but our button never joins it, and nothing is ever selectable. - h.advance(1500); // past appearDeadlineMs (1000) - assert.equal(h.reports.length, 1); - assert.match(h.reports[0], /button never\s+appeared/); - assert.match(h.reports[0], /9\.9\.9/); + h.state.button = false; // …and our button is simply not one of the visible ids + h.advance(WATCHDOG_GIVE_UP_MS + 20000); + assert.equal(h.reports.length, 0, "a filtered rail must never be called a fault"); + assert.equal(h.timersLeft(), 0, "…and it stands down rather than watching forever"); +}); + +test("#779 installer: a filtered rail stays silent even while our tab is selectable later", () => { + // The same shape, but the user leaves linear mode: the button turns up late + // and everything paints. Nothing was ever wrong, and nothing is ever said. + const h = harness(); + h.state.rail = {}; + h.advance(600); // rail seen, our button absent throughout + h.state.button = true; // back to the full rail + h.state.selected = modernButton(OURS); + h.state.painted = true; + h.mutate(); h.advance(20000); - assert.equal(h.reports.length, 1); - assert.equal(h.timersLeft(), 0, "spoken once, then fully stood down"); + assert.equal(h.reports.length, 0, "late appearance is not a contract break"); }); test("#779 installer: no rail at all is 'I cannot tell' — permanent silence", () => { @@ -492,30 +496,20 @@ test("#779 installer: a selected-and-empty tab on a rail-LESS page still says no assert.equal(h.timersLeft(), 0); }); -test("#779 installer: a button that appeared and later VANISHED is out of charter — silent", () => { - // Deliberate: the appearance line says "never appeared", and a button that - // demonstrably appeared makes that statement false. A vanished tab is also a - // different user-visible symptom (gone, not never-there). Pinned so the - // latch in pollAppearance reads as a decision, not an oversight. +test("#779 installer: a button that appeared and later VANISHED stays silent", () => { + // A tab that disappears is a different, visibly different symptom (gone, not + // never-there) and the watchdog has no evidence about its cause — a filtered + // rail produces exactly this transition when the user enters linear mode. const h = harness(); h.state.rail = {}; h.advance(120); // rail seen, poll running h.state.button = true; - h.advance(120); // button seen — appearance satisfied, latched + h.advance(120); h.state.button = false; // …and now it is gone h.advance(20000); assert.equal(h.reports.length, 0); }); -test("#779 installer: a button that appears late but within the deadline is fine", () => { - const h = harness(); - h.state.rail = {}; - h.advance(600); // rail seen, button still absent — inside the deadline - h.state.button = true; - h.advance(20000); - assert.equal(h.reports.length, 0, "slow rail population is not a contract break"); -}); - // --------------------------------------------------------------------------- // Integration: the watchdog is actually wired at the registration site. // --------------------------------------------------------------------------- @@ -540,10 +534,29 @@ test("#779 isPainted at the integration site counts BOTH the root and the failur }); test("#779 the exported bounds are what the reports promise", () => { - // The report says "~3s"/"~10s" from its inputs; the defaults must match the - // constants so a default-config line never claims a window it did not wait. + // The report says "~3s" from its inputs; the default must match the constant + // so a default-config line never claims a window it did not wait. assert.equal(RENDER_STARVATION_MS, 3000); - assert.equal(TAB_APPEAR_DEADLINE_MS, 10000); assert.ok(WATCHDOG_POLL_MS >= 250, "polling is a trickle, not a hot loop"); assert.ok(WATCHDOG_GIVE_UP_MS >= 30000); }); + +test("#779 the watchdog reports NOTHING that DOM absence cannot prove", () => { + // A structural guard on the module, not on one code path: the only thing this + // watchdog is allowed to conclude is starvation (provably-selected + empty). + // An earlier draft also concluded "registerSidebarTab was dropped" from a + // missing rail button, which a supported filtered rail (LinearView) produces + // on a completely healthy panel. If a second report ever comes back, it has + // to justify its evidence here first. + const src = readFileSync( + join(HERE, "../../web/js/lib/sidebar-render-watchdog.js"), + "utf8", + ); + const reports = src.match(/^export function \w*[Rr]eport\w*\(/gm) || []; + assert.equal(reports.length, 1, `exactly one report survives, found: ${reports.join(", ")}`); + assert.match(src, /export function renderStarvationReport\(/); + assert.ok( + !/findSidebarTabButton/.test(src), + "the watchdog must not reason about our rail button's presence at all", + ); +}); diff --git a/web/js/lib/comfyui-dom-deps.js b/web/js/lib/comfyui-dom-deps.js index 3bf3c1db..35525fca 100644 --- a/web/js/lib/comfyui-dom-deps.js +++ b/web/js/lib/comfyui-dom-deps.js @@ -36,8 +36,8 @@ export const COMFYUI_DOM_DEPS = [ }, { selector: ".side-tool-bar-container", - why: "The sidebar rail, observed for tab-selection changes by the guard and by the render watchdog (which also waits on it before judging that our tab button never appeared).", - fallback: "Retried while the frontend boots; absent means the guard never arms and the watchdog stays silent (no rail, no evidence).", + why: "The sidebar rail, observed for tab-selection changes by the guard and by the render watchdog (which waits on it before judging anything at all).", + fallback: "Retried while the frontend boots; absent means the guard never arms and the watchdog stays silent (no rail, no evidence). NOTE: the rail's CONTENTS are not evidence — ComfyUI's LinearView renders it filtered via `visible-tab-ids`, so our button being missing from it means nothing.", verified: ["1.47.12", "1.50.3"], }, { diff --git a/web/js/lib/sidebar-render-watchdog.js b/web/js/lib/sidebar-render-watchdog.js index 118ca41d..16013811 100644 --- a/web/js/lib/sidebar-render-watchdog.js +++ b/web/js/lib/sidebar-render-watchdog.js @@ -15,37 +15,52 @@ * SECONDS after the failure, not instants: its job is a support answer, not a * race. * - * TWO CHECKS, BOTH EVIDENCE-ONLY (the #784 lesson applies to diagnostics too: + * ONE CHECK, EVIDENCE-ONLY (the #784 lesson applies to diagnostics too: * "I cannot tell" must never be reported as "it is broken"): * - * 1. STARVATION — our tab is PROVABLY the selected one (the rail button carries - * our id, read the same dual-generation way the guard reads it) and yet no - * `.cmcp-root` and no `.cmcp-failure-shell` exists, continuously for - * RENDER_STARVATION_MS, re-verified at expiry. When the selected tab is - * another tab, or unidentifiable ("unknown"), the check DISARMS rather than - * counts — an unreadable marker is not evidence of our failure. A paint that - * SURVIVES SATISFY_CONFIRM_MS while our tab is active retires the check for - * the page's lifetime: its charter is first-paint failure, the #779 class; - * content that disappears after a confirmed dwell is a different bug with a - * different symptom. A mere glimpse of paint retires nothing — the actual - * #779 attached the root and removed it in the same mutation flush, and a - * live drill proved a glimpse-trusting watchdog sleeps through it. + * STARVATION — our tab is PROVABLY the selected one (the rail button carries + * our id, read the same dual-generation way the guard reads it) and yet no + * `.cmcp-root` and no `.cmcp-failure-shell` exists, continuously for + * RENDER_STARVATION_MS, re-verified at expiry. When the selected tab is + * another tab, or unidentifiable ("unknown"), the check DISARMS rather than + * counts — an unreadable marker is not evidence of our failure. A paint that + * SURVIVES SATISFY_CONFIRM_MS while our tab is active retires the check for + * the page's lifetime: its charter is first-paint failure, the #779 class; + * content that disappears after a confirmed dwell is a different bug with a + * different symptom. A mere glimpse of paint retires nothing — the actual + * #779 attached the root and removed it in the same mutation flush, and a + * live drill proved a glimpse-trusting watchdog sleeps through it. * - * 2. APPEARANCE — the sidebar rail exists but our tab button never showed up in - * it within TAB_APPEAR_DEADLINE_MS of the rail being seen. This is what a - * frontend that accepts registerSidebarTab() and silently drops the legacy - * spec shape would look like: no tab to click, so check 1 can never trigger, - * and the panel simply vanishes from the product. If the rail itself cannot - * be found, that is "I cannot tell", and the check gives up silently. + * WHY THE BOUND IS WHAT IT IS. `render()` attaches `.cmcp-root` (or, if it + * throws, the #785 shell) SYNCHRONOUSLY, in the same task ComfyUI calls it + * from — there is no async gap in our paint path for a slow machine to widen. + * So 3s of selected-and-empty is not "a slow build", it is a no-show; a + * machine so loaded that `buildPanel()` takes seconds blocks the main thread, + * which delays the deadline timer with it rather than firing it early; and a + * paint that lands at any point inside the window disarms the check, because + * the deadline RE-READS the document instead of deciding on the stale sample + * that armed it. * - * WHY THE BOUNDS ARE WHAT THEY ARE. Render is invoked synchronously when the - * tab mounts, so 3s of selected-and-empty is not "a slow machine", it is a - * no-show — and a machine so loaded that timers stall does not fire early, - * because the deadline itself is a timer. The rail populates in the same - * render pass that creates it, so 10s from first sighting is generous. + * WHAT THIS DELIBERATELY DOES NOT CHECK — and why (Copilot review, PR #804). + * An earlier draft also reported "the rail exists but our tab button never + * joined it within 10s", meaning to catch a frontend that accepts + * registerSidebarTab() and silently drops the legacy spec. That check was + * REMOVED: DOM absence cannot support that conclusion, because a shipped, + * supported ComfyUI view renders a deliberately FILTERED rail — + * `src/views/LinearView.vue` mounts `` (verified in the frontend repo at both v1.50.3 and v1.51.3, and + * reached whenever the user turns on linear mode: `GraphView.vue` renders + * ``). In that view `.side-tool-bar-container` + * is present and our button is absent while registration succeeded exactly as + * designed — the frontend's own registry still lists us + * (`sidebarTabStore.sidebarTabs`, which `visibleTabIds` only filters at RENDER + * time). The check would therefore have told a user whose panel is perfectly + * healthy that their panel is broken and to relaunch ComfyUI pinned to another + * frontend. Filtering and a contract break are indistinguishable from the DOM, + * so under this module's own rule the honest answer is silence. */ -import { readActiveSidebarTab, findSidebarTabButton } from "./active-sidebar-tab.js"; +import { readActiveSidebarTab } from "./active-sidebar-tab.js"; import { VERIFIED_FRONTENDS } from "./comfyui-dom-deps.js"; /** Continuous selected-but-empty time before the starvation line is spoken. */ @@ -57,9 +72,7 @@ export const RENDER_STARVATION_MS = 3000; * the first draft of this watchdog instead of firing it. Never trust one * glimpse of paint. */ export const SATISFY_CONFIRM_MS = 1500; -/** Rail seen → our button still absent for this long = the appearance line. */ -export const TAB_APPEAR_DEADLINE_MS = 10000; -/** Poll cadence for the appearance check (also re-samples starvation). */ +/** Poll cadence while waiting for the rail (also re-samples starvation). */ export const WATCHDOG_POLL_MS = 500; /** Stop polling entirely this long after install — a page with no rail by then * is not going to grow one, and an observerless page costs nothing forever. */ @@ -124,24 +137,6 @@ export function renderStarvationReport(info = {}) { ); } -/** - * The console line for "registered, and the tab button never appeared". - * - * @param {{ panelVersion?: string, frontendVersion?: string, waitedMs?: number }} [info] - */ -export function tabNeverAppearedReport(info = {}) { - const p = info.panelVersion || "unknown"; - const f = info.frontendVersion || "unknown"; - const s = Math.round((info.waitedMs ?? TAB_APPEAR_DEADLINE_MS) / 1000); - return ( - `[comfyui-mcp-panel] registerSidebarTab() accepted the Agent tab, but its button never ` + - `appeared in the sidebar rail (waited ~${s}s after the rail was seen). This frontend most ` + - `likely changed how a custom sidebar tab is declared, in a way panel ${p} does not speak ` + - `yet — ComfyUI frontend here is ${f}. ` + - remedyText(info.frontendVersion) - ); -} - /** * The starvation state machine, pure so it can be tested at second-boundaries * without a DOM or a clock. @@ -244,7 +239,6 @@ export function createRenderWatchdog({ * @param {(h: unknown) => void} [opts.clearTimer] * @param {() => number} [opts.now] * @param {number} [opts.windowMs] - * @param {number} [opts.appearDeadlineMs] * @param {number} [opts.pollMs] * @param {number} [opts.giveUpMs] */ @@ -262,7 +256,6 @@ export function installSidebarRenderWatchdog({ now = () => Date.now(), windowMs = RENDER_STARVATION_MS, confirmMs = SATISFY_CONFIRM_MS, - appearDeadlineMs = TAB_APPEAR_DEADLINE_MS, pollMs = WATCHDOG_POLL_MS, giveUpMs = WATCHDOG_GIVE_UP_MS, } = {}) { @@ -286,8 +279,6 @@ export function installSidebarRenderWatchdog({ let pollTimer = null; const startedAt = now(); let railSeenAt = null; - let buttonEverSeen = false; - let appearanceSpoken = false; const machine = createRenderWatchdog({ tabId, @@ -351,54 +342,38 @@ export function installSidebarRenderWatchdog({ return res; }; - const pollAppearance = () => { + const pollForRail = () => { if (stopped) return; pollTimer = null; const t = now(); const rail = doc.querySelector(".side-tool-bar-container"); - if (rail) { - if (railSeenAt == null) { - railSeenAt = t; - // The rail exists — from here on, selection changes are observable. - // Same subscription the sidebar guard uses: tab selection toggles a - // class on the rail's buttons in every frontend generation we know. - observer = makeObserver(() => sample()); - if (observer) { - try { - observer.observe(rail, { subtree: true, attributes: true, attributeFilter: ["class"] }); - } catch { - observer = null; // fall back to the poll below - } + if (rail && railSeenAt == null) { + railSeenAt = t; + // The rail exists — from here on, selection changes are observable. + // Same subscription the sidebar guard uses: tab selection toggles a + // class on the rail's buttons in every frontend generation we know. + observer = makeObserver(() => sample()); + if (observer) { + try { + observer.observe(rail, { subtree: true, attributes: true, attributeFilter: ["class"] }); + } catch { + observer = null; // fall back to the poll below } } - // `buttonEverSeen` LATCHES, deliberately: the appearance check's charter - // is "registration was accepted and the button NEVER appeared". A button - // that appeared and later vanished is a different phenomenon — visibly - // different to the user (the tab is gone, not never-there), and firing - // "never appeared" about a button that demonstrably appeared would be a - // false statement. That class stays out of scope (codex gate, declined - // with reasons; pinned by test). - if (!buttonEverSeen && findSidebarTabButton(doc, tabId)) buttonEverSeen = true; - if (!buttonEverSeen && !appearanceSpoken && t - railSeenAt >= appearDeadlineMs) { - appearanceSpoken = true; - report(tabNeverAppearedReport(versions(t - railSeenAt))); - // No button means no way to select the tab: the starvation check can - // never trigger, so there is nothing left to watch. - stop(); - return; - } } // The poll doubles as a low-rate starvation re-sample, so a frontend whose - // rail stops emitting class mutations does not blind check 1 entirely. + // rail stops emitting class mutations does not blind the check entirely. if (railSeenAt != null) sample(); if (stopped) return; - const buttonPhaseOver = buttonEverSeen || appearanceSpoken; - const observerCarriesOn = observer != null && buttonPhaseOver; - if (t - startedAt >= giveUpMs || observerCarriesOn) return; // observer (or silence) from here - pollTimer = setTimer(pollAppearance, pollMs); + // Once the observer is watching the rail, it drives sampling and the poll + // has nothing left to discover. Without one, keep trickling until the + // give-up bound. A page that never grows a rail we recognize simply goes + // quiet forever — no rail, no evidence, no claim. + if (observer != null || t - startedAt >= giveUpMs) return; + pollTimer = setTimer(pollForRail, pollMs); }; sample(); - pollAppearance(); + pollForRail(); return { sample, stop }; } From eaec0fa81af3995909d4817b02ec5516e5ebd3b9 Mon Sep 17 00:00:00 2001 From: Artokun Date: Fri, 14 Aug 2026 22:05:35 -0700 Subject: [PATCH 6/6] fix(779): the render watchdog survives a rail remount instead of going deaf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-merge review of PR #804 found the watchdog could not RUN in production, the failure shape this project pays for most often: green tests proving the code matches the tests, never that the code is reachable. The subscription was pinned to the FIRST `.side-tool-bar-container` element, and the poll stopped for good once it attached. ComfyUI does not keep one rail element for the life of the page — it is `v-if`-gated, so it is destroyed and recreated rather than hidden. Verified in the shipped frontend at 1.47.12, 1.48.7, 1.50.3 and 1.51.5: src/components/graph/GraphCanvas.vue const showUI = computed(() => !workspaceStore.focusMode && betaMenuEnabled.value) and linear mode mounts a second, separate instance of its own (src/views/LinearView.vue). So a focus-, linear- or builder-mode toggle leaves the subscription pointing at a node that has left the document, with nothing left to call sample(). Selections on the replacement rail were never sampled and the watchdog fell silent in exactly the way it exists to prevent. The rail is now a GATE, never a handle: its existence still proves "this page has a sidebar we understand" (no rail, no evidence, no claim), and the subscription that drives sampling watches the document, which cannot be replaced. sample() already read the selected button at document scope, so it never cared which rail it was looking at — only the subscription did. Nothing about a rail is retained, so no reference can go stale, and there is no re-binding step to get wrong. childList joins the class filter because a rail rebuilt while our tab is active comes back with its button already selected: no attribute transitions on it, and the only observable is that it appeared. Still exactly one line per genuine starvation for the life of the page — the once-ever latch is untouched, so a remount cannot turn this into the re-arming-notice flood of #1489. Tests: the three P1 tests fail on the parent commit. The observer stub now models both WHERE it is registered and WHAT it subscribed to; the old stub ignored observe()'s target and fired every observer unconditionally, which made a rail-bound subscription and a document-bound one indistinguishable and is why the defect shipped green. Four mutations, each proven applied and each killed: rail-pinned observe, childList dropped, stop() leaking the observer, and the once-ever latch removed. Refs artokun/comfyui-mcp-panel#779 --- .../unit/sidebar-render-watchdog.test.mjs | 226 +++++++++++++++++- web/js/lib/comfyui-dom-deps.js | 4 +- web/js/lib/sidebar-render-watchdog.js | 71 +++++- 3 files changed, 284 insertions(+), 17 deletions(-) diff --git a/browser_tests/unit/sidebar-render-watchdog.test.mjs b/browser_tests/unit/sidebar-render-watchdog.test.mjs index e1b2a7ef..fad5f5f6 100644 --- a/browser_tests/unit/sidebar-render-watchdog.test.mjs +++ b/browser_tests/unit/sidebar-render-watchdog.test.mjs @@ -286,7 +286,25 @@ function harness({ getFrontendVersion: () => "9.9.9", report: (line) => reports.push(line), makeObserver: (cb) => { - const o = { cb, observe() {}, disconnect() {} }; + // A real MutationObserver is REGISTERED ON A NODE. It hears mutations in + // that node's tree and nowhere else, and it keeps hearing nothing at all + // once that node is detached from the document. The first version of this + // stub ignored `observe()`'s target and `mutate()` fired every observer + // unconditionally — which made a rail-bound subscription and a + // document-bound one indistinguishable, and hid the remount defect below. + const o = { + cb, + target: null, + opts: {}, + observe(node, opts) { + this.target = node; + this.opts = opts || {}; + }, + disconnect() { + this.target = null; + this.opts = {}; + }, + }; observers.push(o); return o; }, @@ -318,12 +336,61 @@ function harness({ clock = until; } - /** Fire every live MutationObserver callback, as a rail class change would. */ - function mutate() { - for (const o of observers) o.cb(); + /** + * A DOM mutation happens. Two things decide whether an observer hears it, and + * the stub models both because the defect below hides if either is faked: + * + * - WHERE it is registered. A real MutationObserver only hears mutations in + * the tree of the node it was given. A node the frontend has unmounted is + * detached and mutates no more, so a subscription left on a replaced rail + * hears nothing ever again. + * - WHAT it subscribed to. `{attributes:true}` does not deliver childList + * records, so an element that is BORN with the class already set produces + * no attribute record at all — only the insertion is observable. + * + * @param {"attributes"|"childList"} kind + */ + function mutate(kind = "attributes") { + for (const o of observers) { + const reachable = o.target === doc || (state.rail != null && o.target === state.rail); + if (reachable && o.opts[kind]) o.cb(); + } + } + + /** + * ComfyUI unmounts the rail and mounts a fresh one. Verified in the shipped + * frontend at 1.47.12 / 1.48.7 / 1.50.3 / 1.51.5: the rail is `v-if`-gated, + * `` in + * src/components/graph/GraphCanvas.vue, where `showUI` is + * `!workspaceStore.focusMode && betaMenuEnabled`. Linear mode is a SECOND, + * separate instance (src/views/LinearView.vue). So focus mode, linear mode + * and builder mode each replace the element with a brand-new