fix(panel): the #779 silence detector — a selected tab that never paints must say so, verified live on real 1.50.3 - #804
fix(panel): the #779 silence detector — a selected tab that never paints must say so, verified live on real 1.50.3#804artokun wants to merge 4 commits into
Conversation
…ce detector #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) <noreply@anthropic.com>
…from
The paste-able workaround told the user to run
--front-end-version comfyanonymous/ComfyUI@<version>
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@<version>, with the reason
recorded beside the assertion so the spelling cannot quietly revert.
Refs #779
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a sidebar render watchdog to diagnose blank or missing Agent tabs.
Changes:
- Detects render starvation and missing tab buttons.
- Reports panel/frontend versions with recovery guidance.
- Adds comprehensive watchdog unit tests and DOM dependency documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
web/js/lib/sidebar-render-watchdog.js |
Implements watchdog detection and reporting. |
web/js/lib/comfyui-dom-deps.js |
Documents added DOM dependencies. |
web/js/comfyui-mcp-panel.js |
Installs the watchdog after tab registration. |
browser_tests/unit/sidebar-render-watchdog.test.mjs |
Tests reports, state transitions, and integration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (!buttonEverSeen && findSidebarTabButton(doc, tabId)) buttonEverSeen = true; | ||
| if (!buttonEverSeen && !appearanceSpoken && t - railSeenAt >= appearDeadlineMs) { | ||
| appearanceSpoken = true; | ||
| report(tabNeverAppearedReport(versions(t - railSeenAt))); |
| const buttonPhaseOver = buttonEverSeen || appearanceSpoken; | ||
| const observerCarriesOn = observer != null && buttonPhaseOver; | ||
| if (t - startedAt >= giveUpMs || observerCarriesOn) return; // observer (or silence) from here | ||
| pollTimer = setTimer(pollAppearance, pollMs); |
… and pin a frontend that is not the failing one 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@<v> — 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) <noreply@anthropic.com>
…atch is a decision, pinned 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) <noreply@anthropic.com>
|
Superseding the earlier body: this was never stranded work — the worktree was live and mid-verification when it was surfaced (the same file-heuristic trap as the documented dont-kill-agents-on-file-heuristics incident). The branch is now COMPLETE and fully verified: live on a real 1.50.3 (isolated second instance, correct Comfy-Org/ComfyUI_frontend pin via local dist cache) and on 1.47.12, healthy paths silent, drills fire exactly one correctly-worded line on both frontends, codex gate round 2 PASS, full suite 3045/3045. See the updated PR body for the verification table. Not merging — that call belongs to the merge/release owner. |
What this PR is, and what it is not
The blank-panel-on-1.50.x cause was already fixed on
mainbefore this branch existed: #784 (the sidebar guard must not destroy on an unknown active tab), #785 (a throwingrender()paints a failure shell instead of nothing), #786 (the second reader of the moved tab-button marker). The sidebar-tab contract never moved — 1.50.3 callsrender(); our own guard deleted the content the instant it was attached.This PR is the two halves that were still missing:
Closes #779 (the issue auto-closed when #784 merged; this branch carries the remaining halves discussed in its thread).
1. The watchdog (
web/js/lib/sidebar-render-watchdog.js)#785 covers a
render()that throws. Arender()that is never called — which is exactly what a real sidebar-tab contract change in a future frontend would produce — still failed in perfect silence. Two evidence-only checks close that:data-testidfirst, class fallback) and neither.cmcp-rootnor the fix(panel): a blank tab is never an acceptable failure state #785 failure shell exists, continuously for 3s, re-verified at expiry. Oneconsole.error, ever, naming the panel version, the frontend version, that it is not a connection problem, that reinstalling cannot help, where to report, and a working--front-end-versionpin.registerSidebarTab()and silently drops the legacy spec. Same message structure.The false-positive bar was treated as the primary requirement, because this line will be read as "something is broken":
The remedy line, corrected against reality
The pin names
Comfy-Org/ComfyUI_frontend— notcomfyanonymous/ComfyUI, the spelling the issue thread circulated. The flag fetches GitHub releases from the repo it names; the frontend's 1.x tags exist only in the frontend repo, and the wrong spelling silently falls back to the installed default package (watched happen in a live boot log; confirmed against 0.30.2'sfrontend_management.pyand the GitHub API). The pin also derives fromVERIFIED_FRONTENDS(#787's registry) and prefers the newest verified frontend that differs from the one failing — never a hardcoded relic, never the version the user is already stuck on.The wording (starvation; appearance is analogous)
It reports what was observed (naming both shapes it cannot distinguish), never a guessed cause — the same rule as #785, learned from the hour this reporter spent reinstalling on a guess.
2. Verification — all live, real browsers, real frontends
Isolated second ComfyUI instance (CPU, scratch
--base-directory, this branch via junction, port 8388) — the owner's rig on 8188 was never restarted or repointed..cmcp-rootconnected + visibledata-testid-only buttons)frontend 1.50.3, pin backs off to@1.47.12frontend 1.47.12main's #784–#786 render + keep-alive, no panel console errorsStatic cross-check of the extracted 1.50.3 bundle confirmed every marker this branch depends on:
"data-testid":\${e.id}-tab-button`,side-bar-button-selected,side-tool-bar-container, andmountCustomExtension=(e,t)=>{e.render(t)}` (render IS invoked; no handler).Unit: 29 watchdog tests (state machine, both wordings, installer against a hand-cranked clock and fake rail in both marker generations, the live-drill regression, integration greps). Full suite 3045/3045. The #787 DOM-reach gate passes — the watchdog reads only already-declared selectors, whose registry entries now name it as a reader.
Codex self-gate: round 1 raised two P1s — one accepted and fixed (pre-rail arming hole), one declined with reasons and pinned by a test (appear-then-vanish is out of the "never appeared" charter). Round 2: PASS.
Out of scope, deliberately
mainis unreleased at 0.11.44) belongs to the release owner.TODO(v2): defineSidebarTabmigration stays a TODO: the contract demonstrably did not move, and migrating under a P0 label would be churn, not fix.--workers=1against the live ComfyUI and are not in CI); the checks above were performed directly in a real browser instead.🤖 Generated with Claude Code