From 0a57d0099338f97b8634acfa36cc78e1c95e1e1e Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Fri, 15 May 2026 17:03:49 -0700 Subject: [PATCH 1/2] feat: smooth ComfyUI in-place update flow (#557) Collapse the multi-step in-place update UX into a single confirm and an auto-launch chain: * Title-bar update pill now drives straight into the update confirm dialog by passing autoAction='update-comfyui' on the deep-link overlay open. The Update tab still mounts behind the dialog so a cancel returns to the surface that shows release notes and channel info. * DetailModal.runAction skips the generic actionGuard 'instance is running' prompt for update-comfyui (mirroring the migrate-to-standalone exemption); the running-state notice is folded into the action's existing confirm dialog as a leading 'ComfyUI is currently running and will be restarted to apply this update.' line. * When the install was running at confirm time, the ProgressModal apiCall now chains stopComfyUI -> wait-for-stopped -> update-comfyui -> launch in a single uninterrupted view, mirroring the existing synthetic 'restart' action. triggersInstanceStart is set so PanelApp's chooser-host close-on-instance-started subscription fires for the post-update launch. Amp-Thread-ID: https://ampcode.com/threads/T-019e2e09-b0c6-753f-9b03-567a1c1fa922 Co-authored-by: Amp --- locales/en.json | 1 + .../src/composables/useDeepLinkRouter.ts | 5 ++ src/renderer/src/panel/PanelApp.test.ts | 7 +- src/renderer/src/views/DetailModal.vue | 68 +++++++++++++++---- 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/locales/en.json b/locales/en.json index 3f330f395..f01422e99 100644 --- a/locales/en.json +++ b/locales/en.json @@ -614,6 +614,7 @@ "updateConfirmMessage": "Update from {installed} to {latest}?\n\nLocal changes will be stashed and a backup branch created.", "updateConfirmMessageLatest": "Update from {installed} to latest on master ({latest})?\n\nThis will pull the newest commits from the master branch. Local changes will be stashed and a backup branch created.", "updateConfirmMessageDowngrade": "Roll back from {installed} to stable release {latest}?\n\nYour install is currently ahead of the latest stable release. This will check out the stable tag, removing any newer commits. Local changes will be stashed and a backup branch created.", + "updateRestartNotice": "ComfyUI is currently running and will be restarted to apply this update.", "updatingTitle": "Updating to {version}", "updatePrepare": "Prepare", "updatePrepareSnapshot": "Saving snapshot of current state…", diff --git a/src/renderer/src/composables/useDeepLinkRouter.ts b/src/renderer/src/composables/useDeepLinkRouter.ts index 7ff33c754..17c9f616e 100644 --- a/src/renderer/src/composables/useDeepLinkRouter.ts +++ b/src/renderer/src/composables/useDeepLinkRouter.ts @@ -48,11 +48,16 @@ export function useDeepLinkRouter(opts: DeepLinkRouterOpts): void { await opts.bootstrapReady const inst = installationStore.getById(id) if (!inst) return + // Issue #557 — pill click drives straight to the update + // confirm dialog. The Update tab still opens behind the + // dialog so a cancel returns the user to the surface that + // shows release notes / channel info. await opts.openOverlay({ kind: 'settings', installation: inst, initialTab: 'comfy', initialDetailTab: 'update', + autoAction: 'update-comfyui', }) return } diff --git a/src/renderer/src/panel/PanelApp.test.ts b/src/renderer/src/panel/PanelApp.test.ts index 858ce9dd8..361afe7f1 100644 --- a/src/renderer/src/panel/PanelApp.test.ts +++ b/src/renderer/src/panel/PanelApp.test.ts @@ -34,7 +34,7 @@ vi.mock('../views/DetailModal.vue', () => ({ // parent owns the close behaviour. props: ['installation', 'initialTab', 'autoAction'], template: - '
', + '
', }, })) vi.mock('../views/ProgressModal.vue', () => ({ @@ -722,7 +722,9 @@ describe('PanelApp', () => { // `kind: 'install-update'` and the host's installationId. The // panel renderer routes that into a Tier 1 manage overlay with // initialTab='update' so the DetailModal opens directly on the - // update tab. + // update tab. Issue #557: it also pre-arms the `update-comfyui` + // auto-action so the confirm dialog fires immediately instead of + // making the user click "Update Now" inside the tab. const wrapper = mountPanel() await flushPromises() expect(wrapper.find('[data-testid="detail-modal"]').exists()).toBe(false) @@ -734,6 +736,7 @@ describe('PanelApp', () => { const detail = wrapper.find('[data-testid="detail-modal"]') expect(detail.exists()).toBe(true) expect(detail.attributes('data-installation-id')).toBe('test-id') + expect(detail.attributes('data-auto-action')).toBe('update-comfyui') }) it('shows the "Desktop Update Ready" confirm modal when a restart-prompt event arrives, and installs on confirm', async () => { diff --git a/src/renderer/src/views/DetailModal.vue b/src/renderer/src/views/DetailModal.vue index 64b418507..566980869 100644 --- a/src/renderer/src/views/DetailModal.vue +++ b/src/renderer/src/views/DetailModal.vue @@ -318,12 +318,36 @@ async function runAction(action: ActionDef, btn: HTMLButtonElement | null): Prom // Pre-flight: check if the installation is busy or running // Skip for migrate-to-standalone — the useMigrateAction composable handles its own guard - if (action.id !== 'migrate-to-standalone' && REQUIRES_STOPPED.has(action.id)) { + // Skip for update-comfyui — the smooth update flow folds the + // stop-required prompt into the action's own confirm dialog and + // chains stopComfyUI → update → launch in `apiCall` (issue #557). + if ( + action.id !== 'migrate-to-standalone' && + action.id !== 'update-comfyui' && + REQUIRES_STOPPED.has(action.id) + ) { if (!await actionGuard.checkBeforeAction(props.installation.id, action.label)) return } let mutableAction = { ...action } + // Smooth update flow — when the install is running, the post-update + // auto-launch is implied by the action; tell the user that in the + // single confirm dialog rather than as a separate stop-required + // prompt. The chained stop+launch lives in the showProgress branch. + const updateWillRestart = + mutableAction.id === 'update-comfyui' && sessionStore.isRunning(props.installation.id) + if (updateWillRestart && mutableAction.confirm) { + const baseMsg = mutableAction.confirm.message ?? '' + mutableAction = { + ...mutableAction, + confirm: { + ...mutableAction.confirm, + message: `${t('standalone.updateRestartNotice')}\n\n${baseMsg}`, + }, + } + } + // fieldSelects chain if (mutableAction.fieldSelects) { const selections: Record = {} @@ -512,24 +536,42 @@ async function runAction(action: ActionDef, btn: HTMLButtonElement | null): Prom // continuous "Restarting ComfyUI" view rather than two flashes. // The action's confirm dialog has already run above. const isRestart = mutableAction.id === 'restart' + // Issue #557 — smooth update flow: when the install is running, + // chain stopComfyUI → update → launch in the same ProgressModal + // so the user gets a single uninterrupted view instead of having + // to click Done and then Start ComfyUI afterwards. + const isUpdateAndRestart = mutableAction.id === 'update-comfyui' && updateWillRestart + const waitForStopped = async (): Promise => { + const deadline = Date.now() + 10_000 + while (sessionStore.isRunning(instId) && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 100)) + } + } const apiCall = isRestart ? async () => { await window.api.stopComfyUI(instId) - // Wait for the session to actually leave the running state - // before kicking off the new launch. The session store is - // updated by main via 'session-status' broadcasts. - const deadline = Date.now() + 10_000 - while (sessionStore.isRunning(instId) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 100)) - } + await waitForStopped() + return window.api.runAction(instId, 'launch') + } + : isUpdateAndRestart + ? async () => { + await window.api.stopComfyUI(instId) + await waitForStopped() + const updateResult = await window.api.runAction( + instId, + mutableAction.id, + mutableAction.data ? toRaw(mutableAction.data) : undefined, + ) + if (!updateResult.ok || updateResult.cancelled) return updateResult return window.api.runAction(instId, 'launch') } : () => window.api.runAction(instId, mutableAction.id, mutableAction.data ? toRaw(mutableAction.data) : undefined) - // Tag launch / restart so PanelApp's handleShowProgress installs - // the chooser-host close-on-instance-started subscription. Without - // this, launches kicked off from this Tier-1 modal would leave the - // dashboard window open next to the new comfy window. - const triggersInstanceStart = mutableAction.id === 'launch' || isRestart + // Tag launch / restart / update-and-restart so PanelApp's + // handleShowProgress installs the chooser-host close-on-instance- + // started subscription. Without this, launches kicked off from + // this Tier-1 modal would leave the dashboard window open next to + // the new comfy window. + const triggersInstanceStart = mutableAction.id === 'launch' || isRestart || isUpdateAndRestart emit('show-progress', { installationId: instId, title, From b92360240d552b7d864fd3207989fd19d76f2dd7 Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Fri, 15 May 2026 22:07:59 -0700 Subject: [PATCH 2/2] fix: surface stopComfyUI timeout instead of failing chained action silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit waitForStopped previously returned void after either a successful stop or a 10s deadline expiry. When the deadline expired (slow VRAM teardown, Windows Restart Manager, etc.), the chained restart -> launch and update -> update -> launch calls hit the backend's REQUIRES_STOPPED guard, which returned {ok: false, running: true} and surfaced as a vague 'Operation failed' banner with no message. Make waitForStopped return a boolean and have both branches short-circuit with a typed ActionResult carrying a new errors.stopTimeout message ('ComfyUI is still shutting down. Please wait a moment and try again.'). Also bump the deadline from 10s to 30s — the cost of a false timeout (confusing error) is higher than the cost of a few extra seconds of waiting. Amp-Thread-ID: https://ampcode.com/threads/T-019e2e09-b0c6-753f-9b03-567a1c1fa922 Co-authored-by: Amp --- locales/en.json | 1 + src/renderer/src/views/DetailModal.vue | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/locales/en.json b/locales/en.json index f01422e99..7e3e66ae7 100644 --- a/locales/en.json +++ b/locales/en.json @@ -832,6 +832,7 @@ "stopRequired": "This installation is currently running and must be stopped first.", "stopRequiredConfirm": "This installation is currently running. Would you like to stop it to continue?", "stopRunning": "Stop Running", + "stopTimeout": "ComfyUI is still shutting down. Please wait a moment and try again.", "folderPermissionDenied": "macOS denied access to \"{path}\". Please open System Settings → Privacy & Security → Files and Folders, grant access to ComfyUI Desktop 2.0, then retry." }, diff --git a/src/renderer/src/views/DetailModal.vue b/src/renderer/src/views/DetailModal.vue index 566980869..d1699827d 100644 --- a/src/renderer/src/views/DetailModal.vue +++ b/src/renderer/src/views/DetailModal.vue @@ -541,22 +541,33 @@ async function runAction(action: ActionDef, btn: HTMLButtonElement | null): Prom // so the user gets a single uninterrupted view instead of having // to click Done and then Start ComfyUI afterwards. const isUpdateAndRestart = mutableAction.id === 'update-comfyui' && updateWillRestart - const waitForStopped = async (): Promise => { - const deadline = Date.now() + 10_000 + // Returns true once the session has actually left the running + // state, false if the 30s deadline expired with the session still + // running. Callers must short-circuit on false — the backend + // enforces REQUIRES_STOPPED and would reject the chained call + // with a vague {ok:false, running:true}, surfacing as a generic + // failure banner that doesn't tell the user what happened. + const waitForStopped = async (): Promise => { + const deadline = Date.now() + 30_000 while (sessionStore.isRunning(instId) && Date.now() < deadline) { await new Promise((r) => setTimeout(r, 100)) } + return !sessionStore.isRunning(instId) } + const stopTimeoutResult = (): ActionResult => ({ + ok: false, + message: t('errors.stopTimeout'), + }) const apiCall = isRestart ? async () => { await window.api.stopComfyUI(instId) - await waitForStopped() + if (!(await waitForStopped())) return stopTimeoutResult() return window.api.runAction(instId, 'launch') } : isUpdateAndRestart ? async () => { await window.api.stopComfyUI(instId) - await waitForStopped() + if (!(await waitForStopped())) return stopTimeoutResult() const updateResult = await window.api.runAction( instId, mutableAction.id,