From a121b4972c3434cfb163ec7166fa27c0d30cbfee Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 00:24:45 -0500 Subject: [PATCH] fix(desktop): aim the Update Hermes command at the right target The command palette's row called applyBackendUpdate() directly, so in local mode it drove the backend checkout rather than the client, and nothing opened the updates overlay to report either outcome. Route it through the same target selection the statusbar and About panel use, always surface the overlay, and re-check instead of applying when the active target is already current. --- apps/desktop/src/global.d.ts | 2 + apps/desktop/src/store/updates.test.ts | 116 ++++++++++++++++++++++--- apps/desktop/src/store/updates.ts | 20 +++++ 3 files changed, 126 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 336f95b54ae2..a53965a63b59 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -373,6 +373,8 @@ export interface DesktopUpdateStatus { error?: string behind?: number currentSha?: string + /** Backend only: the version string the backend reports for itself. */ + currentVersion?: string targetSha?: string commits?: DesktopUpdateCommit[] dirty?: boolean diff --git a/apps/desktop/src/store/updates.test.ts b/apps/desktop/src/store/updates.test.ts index 7b19ba15f459..9dd1cbed6a38 100644 --- a/apps/desktop/src/store/updates.test.ts +++ b/apps/desktop/src/store/updates.test.ts @@ -51,6 +51,8 @@ const { applyUpdates, $updateApply, $updateOverlayOpen, + $updateOverlayTarget, + requestActiveUpdate, resetUpdateApplyState, startUpdatePoller, stopUpdatePoller, @@ -69,6 +71,18 @@ const status = (over: Partial = {}): DesktopUpdateStatus => const lastToast = () => notifySpy.mock.calls.at(-1)?.[0] as { onDismiss: () => void } +const setRemote = (on: boolean) => + setConnection({ + baseUrl: 'http://box:9119', + isFullscreen: false, + mode: on ? 'remote' : 'local', + nativeOverlayWidth: 0, + token: 't', + wsUrl: 'ws://box:9119', + logs: [], + windowButtonPosition: null + }) + describe('maybeNotifyUpdateAvailable', () => { beforeEach(() => { storage.clear() @@ -175,18 +189,6 @@ describe('checkBackendUpdates', () => { vi.useRealTimers() }) - const setRemote = (on: boolean) => - setConnection({ - baseUrl: 'http://box:9119', - isFullscreen: false, - mode: on ? 'remote' : 'local', - nativeOverlayWidth: 0, - token: 't', - wsUrl: 'ws://box:9119', - logs: [], - windowButtonPosition: null - }) - it('maps the backend /update/check onto the backend status, including commits', async () => { setRemote(true) checkHermesUpdateSpy.mockResolvedValue({ @@ -254,6 +256,96 @@ describe('checkBackendUpdates', () => { }) }) +// The ⌘K "Update Hermes" row. It used to call applyBackendUpdate() flat, which +// in local mode aimed at the backend checkout instead of the client and, with +// no overlay open, showed nothing at all. +describe('requestActiveUpdate', () => { + const applyClientMock = vi.fn() + const checkClientMock = vi.fn() + + beforeEach(() => { + storage.clear() + notifySpy.mockClear() + dismissSpy.mockClear() + applyClientMock.mockReset().mockResolvedValue({ ok: true, handedOff: true }) + checkClientMock.mockReset().mockResolvedValue(status({ behind: 0 })) + updateHermesSpy.mockReset().mockResolvedValue({ ok: true, name: 'update' }) + checkHermesUpdateSpy.mockReset().mockResolvedValue({ + install_method: 'git', + current_version: '0.4.2', + behind: 0, + update_available: false, + can_apply: true, + update_command: null, + message: null + }) + getActionStatusSpy.mockReset().mockResolvedValue({ lines: [], running: false, exit_code: 0 }) + resetUpdateApplyState() + $updateStatus.set(null) + $backendUpdateStatus.set(null) + $updateOverlayOpen.set(false) + ;(globalThis as unknown as { window: unknown }).window = { + hermesDesktop: { updates: { apply: applyClientMock, check: checkClientMock } } + } + vi.useRealTimers() + }) + + afterEach(() => { + setRemote(false) + delete (globalThis as unknown as { window?: unknown }).window + }) + + it('applies the CLIENT update in local mode, never the backend', async () => { + setRemote(false) + $updateStatus.set(status({ behind: 3 })) + + requestActiveUpdate() + await vi.waitFor(() => expect(applyClientMock).toHaveBeenCalled()) + + expect(updateHermesSpy).not.toHaveBeenCalled() + expect($updateOverlayTarget.get()).toBe('client') + }) + + it('applies the BACKEND update in remote mode', async () => { + setRemote(true) + $backendUpdateStatus.set(status({ behind: 3 })) + + requestActiveUpdate() + await vi.waitFor(() => expect(updateHermesSpy).toHaveBeenCalled()) + + expect(applyClientMock).not.toHaveBeenCalled() + expect($updateOverlayTarget.get()).toBe('backend') + }) + + it('always opens the overlay, so selecting the row is never a silent no-op', () => { + setRemote(false) + $updateStatus.set(status({ behind: 3 })) + + requestActiveUpdate() + + expect($updateOverlayOpen.get()).toBe(true) + }) + + it('opens the overlay to re-check instead of applying when already current', () => { + setRemote(false) + $updateStatus.set(status({ behind: 0, updateAvailable: false })) + + requestActiveUpdate() + + expect($updateOverlayOpen.get()).toBe(true) + expect(applyClientMock).not.toHaveBeenCalled() + expect(updateHermesSpy).not.toHaveBeenCalled() + }) + + it('applies on a backend that reports an update it cannot count commits for', async () => { + setRemote(true) + $backendUpdateStatus.set(status({ behind: 0, updateAvailable: true })) + + requestActiveUpdate() + await vi.waitFor(() => expect(updateHermesSpy).toHaveBeenCalled()) + }) +}) + describe('applyUpdates terminal state', () => { const applyMock = vi.fn() diff --git a/apps/desktop/src/store/updates.ts b/apps/desktop/src/store/updates.ts index 1f0998a5e069..0b46920e30d2 100644 --- a/apps/desktop/src/store/updates.ts +++ b/apps/desktop/src/store/updates.ts @@ -254,6 +254,25 @@ export function startActiveUpdate(): void { void (target === 'backend' ? applyBackendUpdate() : applyUpdates()) } +/** + * Command-palette entry point. The About panel's "Update now" only renders once + * we know an update is waiting; this row is always listed, so it also has to + * handle "already current" — open the overlay for the active target and let its + * check answer, and only apply when there's something to install. + */ +export function requestActiveUpdate(): void { + const target: UpdateTarget = isRemoteMode() ? 'backend' : 'client' + const status = target === 'backend' ? $backendUpdateStatus.get() : $updateStatus.get() + + if ((status?.behind ?? 0) > 0 || status?.updateAvailable) { + startActiveUpdate() + + return + } + + openUpdateOverlayFor(target) +} + /** Re-read the running app's version from the Electron main process and * publish it on `$desktopVersion`. Called when the About panel mounts, the * update flow finishes, and the window regains focus, so the About text @@ -294,6 +313,7 @@ function mapBackendCheck(res: BackendUpdateCheckResponse): DesktopUpdateStatus { message: res.message ?? undefined, updateAvailable: res.update_available, behind: behind > 0 ? behind : 0, + currentVersion: res.current_version, targetSha: res.update_available ? `backend:${res.current_version}` : undefined, commits: res.commits, fetchedAt: Date.now()