Skip to content

Commit 7dfd109

Browse files
feat(host): seamless in-place window-mode transitions + foundational cleanup (#551)
* refactor(main): extract window state and context menu helpers Move window-bounds cache (WindowBounds, flushWindowState, saveWindowBounds, getSavedBounds, getWindowOptions) into src/main/lib/windowState.ts and the context menu attach helper into src/main/lib/contextMenu.ts. Trim historical narration around the Tray docking import, the removed TRAY_ICON, and the chooser host pill text. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * chore: drop plan-step references from comments Per AGENTS.md, comments must not reference plan steps, phases, tracks, or stage IDs. Sweep removes Phase N / Track M-N / Stage W-N / post-unification-code-review.md F-NN / vN.M.K cross-refs from version-resolve, standalone/actions, settings, index, and a handful of renderer composables and tests. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): rewrite suite for WebContentsView chooser host architecture The legacy sidebar/tabs UI assumed by navigation.test.ts and fullscreen.test.ts no longer exists. Delete those plus the orphaned support modules (comfyWindow.ts, navigationHelpers.ts) and re-enable the suite via playwright.config.ts. Add an eval-bridge harness (e2e/support/cdpPages.ts) that runs executeJavaScript against each WebContentsView's webContents, working around Playwright connectOverCDP hanging when the parent BrowserWindow has no DOM. Add chooser/title-bar helpers (chooserHelpers.ts), a new chooser smoke suite (chooser.test.ts), and rewrite launchApp.ts, lifecycle.test.ts, and window-visible.spec.ts against the new harness. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * docs: add CLEANUP_PLAN for index.ts split + popup primitive Roadmap (P0-P4) for the remaining cleanup ahead of the seamless dashboard <-> instance window transitions feature. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * fix(e2e): scope chooser tile match to real install tiles clickInstallTile and the post-install waitFor used .chooser-tile + a textContent substring, which silently matched the New Install tile because its description ('Set up a fresh ComfyUI environment.') contains the install-name substring 'ComfyUI'. Switch both call sites to .chooser-tile:not(.chooser-tile-new):not(.chooser-tile-cloud) .chooser-tile-name so we only consider installed tiles' name labels. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * test(e2e/lifecycle): drive stop via host-window close, tolerate app teardown After launch the chooser host transforms in place to host the install, so the chooser panel's .chooser-tile-cta-close is no longer reachable. Close the install's BrowserWindow directly via main-process evaluate; the close handler tears the comfy process down via the same path the chooser-tile button would use. Closing the last window also quits the app, so the post-close poll now treats an evaluate failure as the terminal stopped state. Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * test(e2e/chooser): cover embedded popup lifecycles ahead of P0.1/P0.2 split Add regression coverage for the three EmbeddedPopupView-shaped subsystems hung off the chooser host so the upcoming src/main/index.ts extractions can be validated end-to-end: * title popup + system modal are pre-warmed when title-bar-ready fires. * title popup opens with menu items on waffle click and closes via __comfyTitlePopup.close(). * title-bar tooltip popup is created on demand via __comfyTitleBar.showTooltip and hides cleanly via hideTooltip. The first-use takeover would race the renderer mount and lock the title bar (consent-lockdown hides the waffle button), so extend the e2e harness with a SeedOptions.settings hook and pre-seed firstUseCompleted: true for the chooser suite. Settings now seed BEFORE Electron launches so the renderer's first read sees them; installations still seed post-launch via app.getPath('userData'). Amp-Thread-ID: https://ampcode.com/threads/T-019e1f94-0238-767d-8770-7db59757c516 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract process error handlers into lib/processErrorHandlers Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract title-tooltip popup into popups/titleTooltip Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract system-modal popup into popups/systemModal Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract asset-download IPC into lib/ipc/registerAssetDownloadHandlers Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * test(e2e/chooser): cover host-registry dedup ahead of P0.5 split Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark Thread B done; add Thread C testing notes Amp-Thread-ID: https://ampcode.com/threads/T-019e1fcf-7754-703e-9441-11cc2816c591 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract host registry into host/registry Pulls ComfyWindowEntry, ComfyPanelKey, BodyMode, VALID_PANELS, comfyWindows, the installationIdToWindowKey secondary index, pendingAttachClaims, lastFocusedInstallationId, and the find/openOrFocus helpers (plus bringToFront) out of index.ts and into src/main/host/registry.ts. Late-bound chooser-host construction via setHostFactories({ createChooser }) so the registry never imports host-construction code (no cycle). index.ts wires the factory at the top of whenReady. index.ts 3720 -> 3125 lines (-595). Amp-Thread-ID: https://ampcode.com/threads/T-019e2090-2750-71d1-a4c9-c5aab842cc37 Co-authored-by: Amp <amp@ampcode.com> * test(host/registry): unit suite for extracted registry helpers Covers nextWindowKey sequencing, computeBodyMode (5 modes incl. install-less + lifecycle gating via _runningSessions), pendingAttachClaims round-trip + overwrite, register/unregister + getEntryByInstallationId (incl. re-index-after-takeover safety), and findPreferredHostByVisibility (visible-beats-minimised, insertion order within bucket, predicate isolation, destroyed-skip). Amp-Thread-ID: https://ampcode.com/threads/T-019e2090-2750-71d1-a4c9-c5aab842cc37 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark Thread C done; record final API + line counts Amp-Thread-ID: https://ampcode.com/threads/T-019e2090-2750-71d1-a4c9-c5aab842cc37 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): cap lifecycle suite at 3 minutes per test (was 10) 10 minutes was overkill — the full lifecycle run including download finishes in under 2 minutes locally. Tighten the per-test cap to 180s so a hung install or a stuck process surfaces fast instead of burning a full 10 minute window. Amp-Thread-ID: https://ampcode.com/threads/T-019e2090-2750-71d1-a4c9-c5aab842cc37 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract title-bar popup into popups/titlePopup Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * test(popups/titlePopup): unit cover menu builder + height calc Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark P0.3 done Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract host construction into host/createHostWindow * test(host/createHostWindow): cover expectedPartitionFor branches Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark P0.6 done Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract attach/detach into host/{attach,detach} * test(host/detach): cover closeAllHostWindows + preClearedClose Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark P0.7 done Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * refactor(main): extract panel routing into host/panelView * test(host/panelView): cover setActivePanel + refreshComfyTabBody guards Amp-Thread-ID: https://ampcode.com/threads/T-019e20b6-b1c6-76cf-9baf-54176cc188d1 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark P0.9 done Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * refactor(popups): introduce EmbeddedPopupView lifecycle primitive Owns the WebContentsView construction + dev/prod URL load + parent-closed teardown + popup-destroyed cleanup + show-fallback timer that all three title-bar / shell popups (titleTooltip, systemModal, titlePopup) currently duplicate inline. Each popup module will keep its own typed entry record (spec, item list, last-synced JSON, ...) and delegate the lifecycle to this class on the next pass. Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * refactor(popups): adopt EmbeddedPopupView in the 3 popup modules systemModal, titleTooltip, and titlePopup now hold an EmbeddedPopupView on each entry record and delegate WebContentsView construction, child-view attachment, dev/prod URL load, parent-closed teardown, popup-destroyed cleanup, isOpen / rendererReady tracking, and the show-fallback timer to the primitive. Each module shrank by ~100 lines net (systemModal 268 -> 174, titleTooltip 440 -> 326, titlePopup 940 -> 808) while behaviour is preserved end-to-end — the existing fast suite covers titlePopup + titleTooltip and the system-modal close confirmation is exercised by the lifecycle suite. Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark Thread E (P1 popup primitive) done Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * docs(agents): allow commit + push on working branches without per-action confirmation Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * fix(popups): clear title-bar reopen guard when popup auto-dismisses The title-bar dropdown could only be opened once per window. The renderer keeps an isMenuOpen flag set on comfy-titlebar:menu-opened and cleared on comfy-titlebar:menu-closed; while the flag is true every click on the trigger button is routed to dismissFileMenu instead of opening. Before the EmbeddedPopupView refactor, hideTitlePopup always sent menu-closed, including from the blur / will-move / move / popup-blur dismiss paths. The refactor moved those auto-dismiss listeners into the primitive's view.hide() — which bypassed hideTitlePopup, leaving menu-closed unsent and the renderer's isMenuOpen permanently stuck true after the first open. Add an onHide callback to EmbeddedPopupView that fires whenever hide() actually transitions out of the open/pending state, regardless of the trigger. titlePopup uses it to send menu-closed; the manual close paths in hideTitlePopup no longer need to send the IPC themselves. Adds: - 1 vitest case covering the onHide manual + auto-dismiss paths. - 1 fast-suite e2e regression test for open -> blur dismiss -> reopen. Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * fix(popups/titlePopup): drop redundant pre-show re-stack so request-size resize sticks The downloads drawer was rendering at the 360px ceiling instead of its natural content height (e.g. ~140px for one entry). Tracing the IPC order: 1. openTitlePopup setBounds(ceiling) + addChildView (re-stack #1) 2. send set-config to renderer 3. renderer measures, sends request-size(natural) 4. main request-size handler setBounds(natural) 5. renderer sends notifyRendered 6. main showTitlePopupNow -> view.showOnTop -> removeChildView + addChildView (re-stack #2) + setVisible(true) Re-attaching the WebContentsView in step 6 appears to reset its bounds back to whatever was in effect before the attach, undoing the natural-height resize from step 4 and leaving the popup stuck at the ceiling. Pre-refactor titlePopup re-stacked only in openTitlePopup (step 1) and showTitlePopupNow just setVisible(true) — so the natural resize was never undone. systemModal and titleTooltip always re-stacked at show time (matching view.showOnTop) and never re-stacked in their open helpers, so they don't have this problem. Drop the pre-show re-stack from openTitlePopup; view.showOnTop's single re-stack at show time is enough to bring the popup above the title-bar / comfy / panel views, and runs after the natural-height resize so bounds stick. Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * fix(downloads-popup): measure children directly so a non-empty list isn't stuck at flex-allocated height The downloads drawer expanded to its ceiling height (~360px) as soon as anything appeared in it, even with one entry that should have been ~140px. The empty state was correctly small. Root cause: \.downloads-list\ is \ lex: 1 1 auto\ so it stretches to fill the popup body. Per the CSS spec, \scrollHeight\ equals \clientHeight\ when content fits without overflow, so \scrollHeight\ reported the flex-allocated 280px instead of the natural ~90px content size. Renderer requested 280 + head + foot, main clamped at the 360 ceiling, popup stayed maxed. The empty state worked because \.downloads-empty\ is a plain block with no \ lex: 1\ rule, so its \offsetHeight\ is the natural rendered size. Sum the children's own \offsetHeight\ plus the list's vertical padding to get the unstretched height. Long lists still cap at the ceiling and scroll internally — main's request-size handler clamps to \DOWNLOADS_POPUP_MAX_HEIGHT_PX\ / 60% of host content height. Amp-Thread-ID: https://ampcode.com/threads/T-019e20f2-8d79-75b6-b024-14279bd95fa7 Co-authored-by: Amp <amp@ampcode.com> * docs(e2e): introduce E2E hardening plan; slot Thread G before Thread F Amp-Thread-ID: https://ampcode.com/threads/T-019e22c5-f346-7205-b7c6-8d4c5200fb91 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): G0 plumbing — test-only globalThis.__e2e bridge + dev-hook helpers Wires a test-only subsystem registered when process.env['E2E'] === '1' so the upcoming G1-G4 fast-suite tests can seed downloads, stub the install-update probe, push fake AppUpdateState through the broadcast pipeline, and read live popup bounds — without driving the production data path (real downloads, real GitHub release fetches, real auto-updater). - Helpers attached to globalThis.__e2e in main; Playwright's app.evaluate(...) bridge calls into them directly. No IPC channel hop is needed since app.evaluate already runs in main. - Implementations live as _test_* exports in their owning modules (comfyDownloadManager, updater, titlePopup) so each surface stays cohesive; lib/e2eHooks.ts is a single small wiring file and lib/e2eOverrides.ts holds the install-update override map. - e2e/support/devHooks.ts wraps each helper in a typed Playwright call. e2e/devhooks-smoke.test.ts exercises every helper end-to-end so a regression in the bridge fails loudly here rather than across G1-G4. - E2E env-var set in electronHarness.ts so the gate flips for every E2E test. Amp-Thread-ID: https://ampcode.com/threads/T-019e22c5-f346-7205-b7c6-8d4c5200fb91 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): G1 — downloads-shelf coverage (sizing + per-status rows + live repaint) Drives the title-bar downloads tray popup against seeded comfyDownloadManager state so the sizing + per-row regressions that have been hitting silent failures during recent refactors fail loudly at PR time. 6 tests, ~3.4s total, all on the chooser host (no install needed): - Empty drawer fits content (popup height < 200px) — catches empty-placeholder padding regressions. - One downloading entry fits content (NOT clipped at the 360 ceiling) — the exact bug fixed in cb43979 where scrollHeight === clientHeight on flex children left the popup stuck at the ceiling height. - Many entries cap at the ceiling and the internal list scrolls — guards against overflow: visible regressions. - Per-status row controls render — pause only on downloading, resume only on paused, cancel hidden on terminal, dismiss only on terminal. - 'Clear finished' header button only renders when there's a terminal entry, AND the open popup updates live when one arrives (covers the live-repaint contract). - Live repaint — empty popup adds rows when seedDownloads fires mid-open (validates the tray-state-changed broadcast → comfy-titlepopup:downloads-changed route). Adds an openDownloadsTray helper to chooserHelpers, a titlePopupPage facade + public WebContentsPage.evaluate to cdpPages, and inline waitForStableBounds / closeTitlePopupIfOpen helpers in the test file. Amp-Thread-ID: https://ampcode.com/threads/T-019e22c5-f346-7205-b7c6-8d4c5200fb91 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): G3 — desktop-update pill coverage + install-update suppression Drives the title-bar app-update pill against seeded AppUpdateState via the G0 setAppUpdateState dev hook, exercising the production _setUpdateState → _broadcastAppUpdateStateToTitleBars path. 6 tests, ~3.2s total, all on the chooser host: - Pill hidden when kind=null (idle state). - Pill renders for each non-null kind: available / downloading / ready, with the right is-* modifier class. - Clicking the ready-state pill opens the embedded system-modal restart-confirm prompt (covers the click-app-update-pill → openSystemModal flow). - Install-update pill stays hidden on the install-less chooser even when the override is flipped on (covers the !isInstallLess gate in showInstallUpdatePill). The 'install-update pill renders + clicks open Settings' path needs an install-backed window (the pill is suppressed on install-less hosts by design) — left for the lifecycle suite per the plan. Adds inline isSystemModalVisible / closeSystemModalIfOpen helpers; the modal is dismissed via Escape since its renderer treats Escape as a cancel ack. Amp-Thread-ID: https://ampcode.com/threads/T-019e22c5-f346-7205-b7c6-8d4c5200fb91 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): G2 — settings panel coverage (open/close + 3 install-less tabs) Drives the unified SettingsModal from the title-bar waffle menu and validates each install-less tab's body renders. The per-install ComfyUI Settings tab is install-backed only (sidebar gates it on hasInstallation) — left for the lifecycle suite per the plan. 6 tests, ~5.4s total, all on the chooser host: - Opens from waffle menu and lands on the install-less default ('global') tab — covers the file-menu Settings → setActivePanel → switchPanel('settings') → openOverlay chain. - Escape dismisses the settings modal. - Reopens cleanly after a close. - Global tab renders SettingsView (at least one .settings-section). - Directories tab renders .directories-panel. - Downloads (settings) tab renders .downloads-tab. Helpers: openSettingsViaWaffle clicks Settings via [role=menuitem] in the popup; selectSettingsTab clicks .settings-sidebar-item by label and waits for the .active modifier to land. Amp-Thread-ID: https://ampcode.com/threads/T-019e22c5-f346-7205-b7c6-8d4c5200fb91 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): G4 — title-bar dropdown + tooltip regression coverage Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * refactor(downloads): extract IPC registration to lib/ipc/registerDownloadHandlers.ts Pure relocation. Mirrors the existing register*Handlers.ts files in src/main/lib/ipc/. comfyDownloadManager.ts no longer reaches for ipcMain / shell directly. Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * refactor(panel): extract app-update prompts and Send Feedback into composables Pure relocation. PanelApp.vue 1203 -> 1118 lines. - useAppUpdatePrompts: showAppUpdateRestartPrompt, showAppUpdateDownloadPrompt - useSendFeedback: app version cache + onOpenFeedback subscription + click handler Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * refactor(panel): extract panel-trigger-overlay deep-link router into composable Pure relocation. PanelApp.vue 1118 -> 1080 lines. useDeepLinkRouter takes the parent's installationId / bootstrapReady gate and the openOverlay + app-update prompt callbacks; the synchronous IPC subscription still installs before any async bootstrap step because all onMounted hooks fire in registration order before any of them yields. Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * refactor(titlebar): extract tooltip coordinator and update-pill state into composables Pure relocation. TitleBarApp.vue 1090 -> 900 lines. - useTitleBarTooltip: macOS hover tooltip dispatcher (issue #514) + tooltipAttrs helper - useUpdatePills: app-update + install-update pill state, labels, click handlers, and bridge subscriptions Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark Thread F partial progress (P3 splits in progress) Amp-Thread-ID: https://ampcode.com/threads/T-019e2311-6aeb-7349-9fd9-560288cfbe38 Co-authored-by: Amp <amp@ampcode.com> * refactor(snapshots): extract SnapshotInspector and shared lib helpers Move the inline inspector body (diff toggle, environment grid, custom-node and pip-package lists) out of SnapshotTab.vue into SnapshotInspector.vue. Promote the pure presentation helpers (triggerClass, formatRelative, copyReasonLabel, changeSummary, diffHasChanges) into lib/snapshots.ts so both files share one source. SnapshotTab.vue 1011 -> 694 lines. Amp-Thread-ID: https://ampcode.com/threads/T-019e2329-47df-7673-94aa-f506eb81d384 Co-authored-by: Amp <amp@ampcode.com> * refactor(chooser): extract ChooserInstallTile and shared tile styles Move the per-install tile rendering (icon, kebab/error cluster, progress block, name, meta pills, update/migrate/version pills, close-instance CTA) out of ChooserView.vue into chooser/ChooserInstallTile.vue. Promote the chooser-tile-* CSS into chooser/chooser-tiles.css and import it from both component scoped style blocks so the New Install / Cloud entry tiles and per-install tiles render identically without duplicating the rules. ChooserView.vue 924 -> 306 lines. Amp-Thread-ID: https://ampcode.com/threads/T-019e2329-47df-7673-94aa-f506eb81d384 Co-authored-by: Amp <amp@ampcode.com> * refactor(titlebar): extract menus + downloads-tray into useTitleBarMenus Pull the file-menu opener, downloads-tray click handler, isMenuOpen / menuClosedAt reopen-guard map, downloadsState ref, and the matching onMenuOpened/onMenuClosed/onDownloadsChanged bridge subscriptions out of TitleBarApp.vue into useTitleBarMenus. Both popups share a single WebContentsView in main, so the dismiss / reopen-guard logic belongs together. TitleBarApp.vue 900 -> 810 lines. Amp-Thread-ID: https://ampcode.com/threads/T-019e2329-47df-7673-94aa-f506eb81d384 Co-authored-by: Amp <amp@ampcode.com> * refactor(titlebar): extract identity / theme / first-use state into useTitleBarIdentity Pull installLabel, sourceCategory, themeBg/themeText, isFullscreen, firstUseMode, the install-type-icon computeds, isLight luminance test, and the matching onTitleChanged / onSourceCategoryChanged / onThemeChanged / onFullscreenChanged / onFirstUseModeChanged bridge subscriptions out of TitleBarApp.vue into useTitleBarIdentity. Parent retains only the install-less flag, hover gate, and panel-changed mirroring. TitleBarApp.vue 810 -> 730 lines. Amp-Thread-ID: https://ampcode.com/threads/T-019e2329-47df-7673-94aa-f506eb81d384 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark Snapshot/Chooser/TitleBar splits done; record P4 sweep findings Flip P3 SnapshotTab.vue and ChooserView.vue rows to done. Record the new TitleBarApp composables (useTitleBarMenus, useTitleBarIdentity) under TitleBarApp's row. Add an initial-sweep status note under P4 explaining that the new modules in host/*, popups/*, renderer composables, and registerDownloadHandlers.ts contain no historical-narration violations and one resolved P0.9 plan reference (host/detach.ts). Also drop the lone P0.9 plan reference from host/detach.ts. Amp-Thread-ID: https://ampcode.com/threads/T-019e2329-47df-7673-94aa-f506eb81d384 Co-authored-by: Amp <amp@ampcode.com> * refactor(panel): extract overlay slot + helpers into usePanelOverlays Amp-Thread-ID: https://ampcode.com/threads/T-019e2394-e4cc-73b9-a95c-23a5a5143a41 Co-authored-by: Amp <amp@ampcode.com> * refactor(panel): extract chooser-tile launch handoff into useChooserHandoff Amp-Thread-ID: https://ampcode.com/threads/T-019e2394-e4cc-73b9-a95c-23a5a5143a41 Co-authored-by: Amp <amp@ampcode.com> * refactor(panel): extract first-use chain into useFirstUseChain Amp-Thread-ID: https://ampcode.com/threads/T-019e2394-e4cc-73b9-a95c-23a5a5143a41 Co-authored-by: Amp <amp@ampcode.com> * docs(cleanup-plan): mark PanelApp split done; record P4 sweep on new composables Amp-Thread-ID: https://ampcode.com/threads/T-019e2394-e4cc-73b9-a95c-23a5a5143a41 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): cover title-bar hover-gate state machine Amp-Thread-ID: https://ampcode.com/threads/T-019e23b2-13b0-726c-9d4f-df64a04c5be7 Co-authored-by: Amp <amp@ampcode.com> * refactor(titlebar): extract hover-gate state machine into useTitleBarHoverGate Amp-Thread-ID: https://ampcode.com/threads/T-019e23b2-13b0-726c-9d4f-df64a04c5be7 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): cover hover-gate state machine on the install-backed comfy window Amp-Thread-ID: https://ampcode.com/threads/T-019e23b2-13b0-726c-9d4f-df64a04c5be7 Co-authored-by: Amp <amp@ampcode.com> * refactor(ipc/shared): trim three long doc-blocks per AGENTS.md Amp-Thread-ID: https://ampcode.com/threads/T-019e23b2-13b0-726c-9d4f-df64a04c5be7 Co-authored-by: Amp <amp@ampcode.com> * test(e2e): retry app.evaluate calls on transient context-destroyed errors Wraps the eval-bridge entry points (cdpPages findWebContentsId / WebContentsPage.evaluate, devHooks seed/setInstall/setApp/getTitlePopupBounds) in a small retry helper. CI runners hit 'Execution context was destroyed' when the host window's child WebContentsViews tear up/down in quick succession; the next tick lands on a fresh evaluation context, so a 50ms backoff retry recovers without masking real failures. Amp-Thread-ID: https://ampcode.com/threads/T-019e2467-d182-720c-afdd-60f91a534f29 Co-authored-by: Amp <amp@ampcode.com> * refactor(host): encapsulate attach claims + add isChooserHost/isInstallHost predicates P2 cleanup for the seamless-transition feature. Registry now owns the attach-claim lifecycle behind three named helpers (claimAttachHost / consumeAttachClaim / dropAttachClaimsForWindow); pendingAttachClaims is module-private so producer / consumer / cleanup sites can't drift apart. consumeAttachClaim folds get+delete into one take-once call so onLaunch can't double-attach. isChooserHost(entry) / isInstallHost(entry) type-guard predicates live next to computeBodyMode in registry.ts. Routed every semantic chooser- vs-install branch through them: title-popup file-menu builder + activate guard, will-prevent-unload, applyChooserHostTheme(ToAll), attach already-attached guard, returnToDashboard + _detachInstallImpl guards, refreshComfyTabBody loop, chooser-pick claim guard, transfer-host- bounds-to-install. Registry-internal sites that ARE the implementation the predicate is built from (the installationIdToWindowKey index in register/unregisterHostEntry, find-preferred predicates inside the registry itself) keep direct installationId comparisons. Tests updated to use the helpers + cover the take-once contract and per- window cleanup. Amp-Thread-ID: https://ampcode.com/threads/T-019e2467-d182-720c-afdd-60f91a534f29 Co-authored-by: Amp <amp@ampcode.com> * refactor(host): split createHostWindow + extract attachInstall wire helpers Two related P2 cleanups for the seamless-transition feature. createHostWindow now composes three module-private helpers: - buildHostSkeleton(opts): BrowserWindow + titleBarView + comfyView + bound layoutViews + resize/move/fullscreen/focus listeners - wireTitleBarHandshake(args): comfy-window:title-bar-ready IPC handler that pushes initial state and pre-warms file-menu / system-modal popups; returns the bound handler so the lifecycle module can ipcMain.off it on closed - installLifecycleListeners(args): close consult-and-destroy flow + closed-time registry/claim cleanup createHostWindow itself is now a 35-line composition. A future swapInstallInPlace can re-run wireTitleBarHandshake + installLifecycleListeners against an existing skeleton without rebuilding the BrowserWindow. attachInstall extracted three independent wire* helpers, each returning a teardown closure that's collected into _installCleanup: - wireKeyboardShortcuts: F5/Ctrl+R reload + Ctrl/Cmd zoom shortcuts - wireFailRetry: backoff retry on did-fail-load; returns { cancel } which doubles as the relaunch-flow interrupt and the teardown - wireRenderProcessGone: Datadog report + automatic reload attachInstall itself shrank from ~330 to ~220 lines. Remaining inline blocks (theme observer, OS title sync, installation-rename listener, dom-ready content script) share closure state and are deferred to the seamless-transition feature, which will need to redesign that shared state to support an in-place install swap. Amp-Thread-ID: https://ampcode.com/threads/T-019e2467-d182-720c-afdd-60f91a534f29 Co-authored-by: Amp <amp@ampcode.com> * docs: remove completed planning docs Five plan/checklist docs whose work has fully landed, with no remaining open items and no references from any other doc or source file. Git history preserves the trail of decisions for anyone who needs to revisit; keeping the live tree clean of completed plans prevents readers from mistaking past work for active scope. - docs/CLEANUP_PLAN.md — pre-seamless-transition cleanup, P0-P4 all shipped on this branch. - docs/E2E_HARDENING_PLAN.md — G0-G4 shipped; G5 was always optional and is not pursued. - docs/357-code-quality-plan.md — issue 357, all phases shipped. - docs/431-view-flattening-plan.md — issue 431, all phases shipped; also referenced files that have since been deleted. - docs/view-flattening-plan.md — superseded by 431-view-flattening-plan and now also retired. Reference / postmortem / ongoing-design docs remain untouched (window-mode-unification-revert.md is referenced from production code; postmortem-reactive-proxy-ipc.md, telemetry.md, theme-colors.md, migration-parity.md, electron-window-parity.md are evergreen references; unified-window-phase3-notes.md, post-phase3-ux-polish.md, post-unification-code-review.md still track open follow-on work). Amp-Thread-ID: https://ampcode.com/threads/T-019e2467-d182-720c-afdd-60f91a534f29 Co-authored-by: Amp <amp@ampcode.com> * refactor(host): inline createHostWindow + attachInstall wire helpers; trim claim-attach-host doc Code-review followup. Reverts the speculative wire/skeleton extractions in createHostWindow and attachInstall back to the original linear structure. The extracted helpers existed only for a future swapInstallInPlace caller and added parameter-passing without removing complexity. The wireFailRetry split also created a real bug: its returned cancel function tore down the did-fail-load listener, but that same function was stored in comfyFailRetryTimerCancels and called mid-session by onModelFolderRelaunch — silently killing retry for the rest of the install's lifetime. Inlining restores the original behavior where cancelFailRetry only clears the timer (safe to call from the relaunch flow) and the listeners are torn down by _installCleanup at end-of-life only. Also trims a multi-paragraph history-narrating doc-block on the claim-attach-host IPC handler down to one sentence + the doc link (per AGENTS.md "don't narrate history"). Kept from the prior P2 work: the registry attach-claim helpers (claimAttachHost / consumeAttachClaim / dropAttachClaimsForWindow) and the isChooserHost / isInstallHost type-guard predicates and their call-site routing. Amp-Thread-ID: https://ampcode.com/threads/T-019e2467-d182-720c-afdd-60f91a534f29 Co-authored-by: Amp <amp@ampcode.com> * feat(host): seamless in-place window-mode transitions Flip a single host window between chooser (dashboard) and install modes in-place instead of closing and reopening the window. - index.ts: claim-attach-host IPC + onLaunch handle identity swaps and reload titlebar with new install id; wire destroyPanelView via HostWindowFactories - createHostWindow.ts: extract loadTitleBarUrl; fix stale comfyView closure in close handler (use registry); replace empty catches with forwardDatadogError; guard webContents before isDestroyed() - panelView.ts: add destroyPanelView helper for transition cleanup - detach.ts: returnToDashboard now flips host in place - e2eHooks/devHooks: programmatic returnToDashboard test hook - lifecycle.test.ts: cover same-window-id flip on return-to-dashboard Drop the now-obsolete window-mode-unification-revert plan doc. Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * fix(host): read comfyView off entry in layoutViews so unique-partition in-place attach paints rebuildComfyViewIfNeeded swaps entry.comfyView when a chooser host is attached in place to a unique-partition install (Standalone / Portable). The captured comfyView in the layoutViews closure pointed at the now-destroyed view, so the freshly-built view kept its default 0x0 invisible bounds and ComfyUI loaded but never painted. Mirrors the same fix already applied to the close handler. Strengthens the lifecycle E2E to assert comfyView is visible with non-zero bounds — the previous coverage only checked the URL, which loaded fine. Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * fix(host): cascade-offset new host windows that would land on top of an existing one File menu -> New Window (and any other createHostWindow caller) used getWindowOptions(boundsKey) which returns the saved bounds verbatim, so a freshly-spawned chooser host opened at the EXACT same x/y/w/h as the most recent host of the same boundsKey. The new window appeared to just re-render the previous one. Walk live host origins and bump (x, y) by 30px per collision (with chain detection) so the new window cascades visibly past every overlap. Skipped when getWindowOptions returned no x/y (no saved bounds, Electron centers). Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * fix(host): identity-aware bounds saving + sibling-default size for spawned hosts Two related fixes for the bounds inheritance weirdness exposed by the in-place chooser->install flip: 1. Save bounds under the LIVE entry identity (chooser slot or installation id) instead of the construction-time key. A chooser host that flips to install-backed in place was saving its install-mode user resizes under the chooser slot — and vice versa via Return to Dashboard. Now each identity slot only persists what the user did while the host was that identity. 2. When opening a new host while another host of the same identity is already alive (most common: File -> New Window with a chooser open), use the canonical 1280x900 default size instead of the saved bounds. Saved-bounds inheritance there made the new window size + place identically to the live one, so it looked like the existing window had simply re-rendered. First-spawn (no live sibling) keeps the saved-bounds restore so app relaunches still land at the user's preferred size. Drive-by cleanups from the seamless-transition investigation: - Drop stranded attach claims when the existing-entry path takes over a launch instead of the in-place flip (otherwise the claim sat in the map until the chooser host closed). - Update the now-stale claim-rejection comment in useChooserHandoff: rebuildComfyViewIfNeeded handles partition mismatches in place; the fallback only fires for genuine sender races. - Document the loadTitleBarUrl handshake contract: any new title-bar renderer state must be re-pushed by onTitleBarReadyHandler since the navigation drops cached messages. Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * fix(host): chooser hosts always open at default size; never persist their bounds The dashboard isn't a workspace the user customizes — it's a launcher surface. Restoring last-session bounds across cold starts (and especially across in-place flips that drifted the slot) made the dashboard feel like it had inherited an unrelated window's shape. Force chooser hosts to use the canonical 1280x900 default at construction (skip getSavedBounds + maximized restore), and skip the persistBounds save when the live entry is install-less. Install-backed hosts continue to restore + persist their per-install bounds normally. Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * refactor(host): extract safeTeardown closure in close handler Replaces 8 repeated try/catch + reportTeardownError blocks with single-line safeTeardown(source, fn) calls. Same functional safety + same explicit telemetry source mapping; the sequence of teardown steps is now readable at a glance. Amp-Thread-ID: https://ampcode.com/threads/T-019e2ce5-2727-71dc-a6a5-a7540e0521a5 Co-authored-by: Amp <amp@ampcode.com> * fix(e2e): write seeded settings.json to macOS userData path On macOS, Electron's app.getPath('userData') resolves to ~/Library/Application Support/<appName>/, but the harness was writing the seeded settings.json to ~/.config/<appName>/ for everything except Windows. The seed never reached the renderer, so firstUseCompleted defaulted to false, the FirstUseTakeover auto-mounted, and pushed firstUseMode='consent-lockdown' which v-ifs the title-bar waffle button out of the DOM. Fix: write to ~/Library/Application Support/<appName>/ on darwin so configDir() in src/main/settings.ts finds the seed. The chooser-test suite happened to dismiss the takeover via clickNewInstallTile + Escape before its first openTitleMenu, masking the bug; the new dropdowns / settings suites call openTitleMenu first and surfaced it. Amp-Thread-ID: https://ampcode.com/threads/T-019e2d4b-3e1c-76da-877e-ddf6110976db Co-authored-by: Amp <amp@ampcode.com> * ci: upload Playwright failure artifacts on E2E job failure Amp-Thread-ID: https://ampcode.com/threads/T-019e2d4b-3e1c-76da-877e-ddf6110976db Co-authored-by: Amp <amp@ampcode.com> * fix(e2e): seed settings.json into both candidate macOS userData dirs Non-packaged Electron may resolve app.name to either the package.json value (comfyui-desktop-2) or its binary name (Electron). The first attempt only wrote to ~/Library/Application Support/comfyui-desktop-2/settings.json; CI's Page Snapshot artifact confirmed firstUseCompleted still came back undefined, so the FirstUseTakeover auto-mounted at the consent step and the consent-lockdown push hid the title-bar waffle button. Seed both paths on darwin so the renderer's first read finds the seed regardless of which app name Electron resolves. Amp-Thread-ID: https://ampcode.com/threads/T-019e2d4b-3e1c-76da-877e-ddf6110976db Co-authored-by: Amp <amp@ampcode.com> * e2e: seed settings via E2E_SETTINGS_SEED env var The harness used to write settings.json into the platform-specific userData dir before launch. On macOS that dir is rooted at the real pw_dir (Application Support), not HOME, so the seed landed outside the test sandbox and main never read it — the FirstUseTakeover opened, set isConsentLockdown, and hid the title-bar waffle button, breaking dropdowns.test.ts and settings.test.ts. Pass the seed JSON via E2E_SETTINGS_SEED instead. Main writes it to dataPath on first settings.load(), bypassing all userData path guesswork. Amp-Thread-ID: https://ampcode.com/threads/T-019e2d74-aba0-70e8-85d2-454c11a251e5 Co-authored-by: Amp <amp@ampcode.com> * Address pass 1+2 code review findings settings.ts: - Add app.isPackaged guard to maybeSeedFromEnv so the E2E settings seed cannot fire in production builds. - Delete process.env['E2E_SETTINGS_SEED'] immediately after reading so the JSON payload doesn't bleed into Python/ComfyUI child processes. host/createHostWindow.ts + host/detach.ts: - Drop the HostWindowFactories.{ensurePanelView,destroyPanelView} indirection and the entire DetachFactories plumbing — both modules now import panelView.ts directly. - Clean up the corresponding setHostWindowFactories / setDetachFactories wiring in main/index.ts. e2e: - Hoist isPopupVisible, closeTitlePopupIfOpen, and TITLE_REOPEN_SUPPRESSION_MS into support/cdpPages.ts and use them from chooser, downloads-shelf, dropdowns, and settings tests. - Bump the title-bar reopen-suppression debounce from 150ms to 300ms (TITLE_REOPEN_SUPPRESSION_MS) to take the timer further off the 100ms guard for CI flake margin. - Move entryCounter = 0 into the downloads-shelf beforeEach for deterministic createdAt ordering across tests. Amp-Thread-ID: https://ampcode.com/threads/T-019e2da9-1d27-73f4-9b13-ad02e60933ac Co-authored-by: Amp <amp@ampcode.com> * Address pass 3 (renderer composables) review findings Bug fixes: - PanelApp.vue: wrap onMounted bootstrap in try/finally so resolveBootstrap always fires; if loadLocale or the store init Promise.all rejected, the bootstrapReady promise stayed pending forever and useDeepLinkRouter would silently wedge any panel-trigger-overlay deep-link the user fired afterwards. - PanelApp.vue: drop the duplicate onFirstUseSkip subscription. useFirstUseChain already owns the same listener, so a single Skip Onboarding click was running completeFirstUseAndDismiss twice. - useChooserHandoff.ts + useFirstUseChain.ts: performChooserLaunch now returns 'focused-running' | 'launched' | 'missing-action'. handleFirstUseComplete dismisses the takeover when the cloud install is already running so the first-use overlay doesn't sit stranded over the focused window. - useChooserHandoff.ts: clear pendingPickUnsub at the START of every prepareChooserHostHandoff call. A previous claimed-by-main launch (or a launch that errored) used to leave its onInstanceStarted listener live; a later unrelated instance-started broadcast could then close this chooser host. - useTitleBarMenus.ts: handleDownloadsTray now calls hideTip() like handleFileMenu does, so the macOS hover tooltip can't outlive the downloads popup opening over it. Cleanup: - usePanelOverlays.ts: drop the unused FirstUseChainHooks.reset field — no implementation, no caller, just misleading docs. Amp-Thread-ID: https://ampcode.com/threads/T-019e2da9-1d27-73f4-9b13-ad02e60933ac Co-authored-by: Amp <amp@ampcode.com> --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 0a7d6e3 commit 7dfd109

83 files changed

Lines changed: 11253 additions & 8360 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,14 @@ jobs:
8787

8888
- name: Run E2E (${{ matrix.platform }})
8989
run: ${{ matrix.platform == 'linux' && format('xvfb-run --auto-servernum {0}', matrix.command) || matrix.command }}
90+
91+
- name: Upload Playwright failure artifacts
92+
if: failure()
93+
uses: actions/upload-artifact@v4
94+
with:
95+
name: playwright-${{ matrix.platform }}-failure
96+
path: |
97+
test-results/
98+
playwright-report/
99+
retention-days: 7
100+
if-no-files-found: ignore

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ pnpm run test
1313

1414
Typecheck and lint are enforced automatically by a husky pre-commit hook.
1515

16+
## Committing and pushing
17+
18+
When the current branch is a working / feature branch (anything other than `main`), it is fine to `git commit` and `git push` without asking for explicit confirmation each time — keep the branch up to date as you make progress. Run the pre-commit checks above first, fix everything they surface, and only then commit + push.
19+
20+
Do **not** push directly to `main`, force-push (`--force` / `--force-with-lease`), or rewrite already-published history without explicit user instruction.
21+
1622
## Fix all issues found by checks
1723

1824
Any errors or warnings surfaced by typecheck, lint, audit, or tests are **our responsibility to fix** — even if they appear to be pre-existing. Do not skip, ignore, or work around them with `--no-verify`. If a pre-commit hook fails, fix the underlying issues before committing.

docs/357-code-quality-plan.md

Lines changed: 0 additions & 113 deletions
This file was deleted.

docs/431-view-flattening-plan.md

Lines changed: 0 additions & 86 deletions
This file was deleted.

docs/view-flattening-plan.md

Lines changed: 0 additions & 104 deletions
This file was deleted.

0 commit comments

Comments
 (0)