From 70472dc945753c745de008d627035b549b150e31 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Jul 2026 10:06:32 +0200 Subject: [PATCH 1/4] Always install the newest eligible desktop update (#2149) * Reapply "Always revalidate desktop updates before install" This reverts commit 623c05aa4d01e824d6381ba5d547a17834fa157f. * fix(desktop): make update revalidation safe on quit Updater-triggered quits now bypass normal quit handling, and manifest revalidation gives up after five seconds without allowing a late install. * fix(desktop): preserve fail-closed update installs Keep cached updates deferred when quit-time validation is offline, and report validation timeouts separately from superseded releases. * fix(desktop): preserve updater quit behavior Automatic installs remain silent without relaunching, updater handoff is bounded, and background preparation failures remain visible without pinning manifest checks. * fix(desktop): keep AppImage updates manual Preserve the AppImage safety exemption by skipping ordinary quit-time installation while retaining the explicit Update now path. * fix(desktop): serialize update preparation * fix(desktop): recognize macOS updater quit handoff --- docs/release.md | 4 +- .../desktop/src/daemon/quit-lifecycle.test.ts | 268 +++++++--- packages/desktop/src/daemon/quit-lifecycle.ts | 97 +++- .../src/features/app-update-service.test.ts | 461 +++++++++++++----- .../src/features/app-update-service.ts | 299 ++++++++---- .../desktop/src/features/auto-updater.test.ts | 14 +- packages/desktop/src/features/auto-updater.ts | 42 +- packages/desktop/src/main.ts | 52 +- 8 files changed, 909 insertions(+), 328 deletions(-) diff --git a/docs/release.md b/docs/release.md index 90dd263e3..8252898ca 100644 --- a/docs/release.md +++ b/docs/release.md @@ -169,13 +169,13 @@ This does **not** apply to fresh releases cut via `npm run release:patch` — th ### Releasing during an active rollout -If you ship N+1 while N is still ramping, N+1 starts a fresh rollout from its own publish timestamp. N's rollout effectively ends — the newer manifest supersedes it. +If you ship N+1 while N is still ramping, N+1 starts a fresh rollout from its own publish timestamp. N's rollout effectively ends — the newer manifest supersedes it. Rollout-aware clients revalidate the manifest for up to five seconds before installing a downloaded update on quit. If N+1 has replaced N but the client is not admitted to N+1 yet, it skips the downloaded N and waits rather than installing two updates in succession. If revalidation times out, the app exits without installing the cached update. If N+1 is a hotfix for a bug in N, dispatch `desktop-rollout.yml -f tag=v0.1. -f rollout_hours=0` after N+1 publishes so the users who already got N reach the fix fast. ### Limitations -- **No pause / kill switch.** Once a stable user is admitted, they will install the update on next quit (`autoInstallOnAppQuit = true`). To stop new admissions, ship a superseding release. To "recall" already-admitted users, ship a hotfix `+1` patch. +- **No pause / kill switch.** To stop new admissions, ship a superseding release. Clients revalidate on quit and will not install the superseded download, but a client that already completed installation cannot be recalled; ship a hotfix `+1` patch. - **No rollback.** `allowDowngrade = false`. Bad release = ship a hotfix. - **Bootstrap caveat.** Clients running a build older than the rollout feature ignore `rolloutHours` and admit immediately. Rollout protection only applies to clients running the rollout-aware version or later. - **Up to ~30 min automatic admission latency.** Renderer polls every 30 minutes, so a stable user may take up to that long to be evaluated against the rollout window. Clicking **Check** is manual and bypasses rollout admission. diff --git a/packages/desktop/src/daemon/quit-lifecycle.test.ts b/packages/desktop/src/daemon/quit-lifecycle.test.ts index d9af82b65..1d636bd95 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.test.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.test.ts @@ -1,8 +1,8 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { DEFAULT_DESKTOP_SETTINGS } from "../settings/desktop-settings"; import { - createBeforeQuitHandler, + createQuitLifecycle, shouldStopDesktopManagedDaemonOnQuit, stopDesktopManagedDaemonOnQuitIfNeeded, } from "./quit-lifecycle"; @@ -16,6 +16,18 @@ const SETTINGS_STOP_ON_QUIT = { }, }; +function deferred(): { promise: Promise; resolve(value: T): void } { + let resolve!: (value: T) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +function waitForQuitLifecycle(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + describe("quit-lifecycle", () => { it("only stops when keepRunningAfterQuit is explicitly disabled", () => { expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_STOP_ON_QUIT)).toBe(true); @@ -23,96 +35,238 @@ describe("quit-lifecycle", () => { }); it("short-circuits without inspecting the daemon when keep-running is on", async () => { - const isDesktopManagedDaemonRunning = vi.fn(() => true); - const stopDaemon = vi.fn(async () => undefined); - const showShutdownFeedback = vi.fn(); + const events: string[] = []; const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ settingsStore: { get: async () => SETTINGS_KEEP_RUNNING }, - isDesktopManagedDaemonRunning, - stopDaemon, - showShutdownFeedback, + isDesktopManagedDaemonRunning: () => { + events.push("inspect"); + return true; + }, + stopDaemon: async () => { + events.push("stop"); + }, + showShutdownFeedback: () => { + events.push("feedback"); + }, }); expect(stopped).toBe(false); - expect(isDesktopManagedDaemonRunning).not.toHaveBeenCalled(); - expect(stopDaemon).not.toHaveBeenCalled(); - expect(showShutdownFeedback).not.toHaveBeenCalled(); + expect(events).toEqual([]); }); it("does not stop a manually started daemon on quit", async () => { - const stopDaemon = vi.fn(async () => undefined); - const showShutdownFeedback = vi.fn(); + const events: string[] = []; const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT }, isDesktopManagedDaemonRunning: () => false, - stopDaemon, - showShutdownFeedback, + stopDaemon: async () => { + events.push("stop"); + }, + showShutdownFeedback: () => { + events.push("feedback"); + }, }); expect(stopped).toBe(false); - expect(stopDaemon).not.toHaveBeenCalled(); - expect(showShutdownFeedback).not.toHaveBeenCalled(); + expect(events).toEqual([]); }); it("shows feedback then stops a desktop-managed daemon", async () => { - const stopDaemon = vi.fn(async () => undefined); - const showShutdownFeedback = vi.fn(); + const events: string[] = []; const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT }, isDesktopManagedDaemonRunning: () => true, - stopDaemon, - showShutdownFeedback, + stopDaemon: async () => { + events.push("stop"); + }, + showShutdownFeedback: () => { + events.push("feedback"); + }, }); expect(stopped).toBe(true); - expect(showShutdownFeedback).toHaveBeenCalledTimes(1); - expect(stopDaemon).toHaveBeenCalledTimes(1); - expect(showShutdownFeedback.mock.invocationCallOrder[0]).toBeLessThan( - stopDaemon.mock.invocationCallOrder[0], - ); + expect(events).toEqual(["feedback", "stop"]); }); - it("preventDefaults the first quit, runs the async stop decision, then exits hard", async () => { - let resolveStopDecision: (() => void) | null = null; - const app = { exit: vi.fn() }; - const closeTransportSessions = vi.fn(); - const onStopError = vi.fn(); - const preventDefault = vi.fn(); - const secondPreventDefault = vi.fn(); + it("revalidates updates after daemon shutdown before exiting", async () => { + const stopDecision = deferred(); + const updateDecision = deferred(); + const events: string[] = []; - const handleBeforeQuit = createBeforeQuitHandler({ - app, - closeTransportSessions, - stopDesktopManagedDaemonIfNeeded: vi.fn( - () => - new Promise((resolve) => { - resolveStopDecision = () => resolve(false); - }), - ), - onStopError, + const quitLifecycle = createQuitLifecycle({ + app: { + exit: (code) => { + events.push(`exit:${code}`); + }, + }, + closeTransportSessions: () => { + events.push("close-transports"); + }, + stopDesktopManagedDaemonIfNeeded: () => stopDecision.promise, + installAppUpdateOnQuit: () => updateDecision.promise, + createUpdateDeadlineSignal: () => new AbortController().signal, + onStopError: () => { + events.push("stop-error"); + }, + onUpdateError: () => { + events.push("update-error"); + }, }); - handleBeforeQuit({ preventDefault }); + quitLifecycle.handleBeforeQuit({ + preventDefault: () => { + events.push("prevent-default"); + }, + }); - expect(preventDefault).toHaveBeenCalledTimes(1); - expect(closeTransportSessions).toHaveBeenCalledTimes(1); - expect(app.exit).not.toHaveBeenCalled(); - expect(resolveStopDecision).not.toBeNull(); + expect(events).toEqual(["close-transports", "prevent-default"]); - resolveStopDecision?.(); - await Promise.resolve(); - await Promise.resolve(); + events.push("daemon-stopped"); + stopDecision.resolve(false); + await waitForQuitLifecycle(); - expect(app.exit).toHaveBeenCalledWith(0); - expect(onStopError).not.toHaveBeenCalled(); + expect(events).toEqual(["close-transports", "prevent-default", "daemon-stopped"]); - handleBeforeQuit({ preventDefault: secondPreventDefault }); + events.push("update-checked"); + updateDecision.resolve(false); + await waitForQuitLifecycle(); - expect(secondPreventDefault).not.toHaveBeenCalled(); - expect(closeTransportSessions).toHaveBeenCalledTimes(2); - expect(app.exit).toHaveBeenCalledTimes(1); + expect(events).toEqual([ + "close-transports", + "prevent-default", + "daemon-stopped", + "update-checked", + "exit:0", + ]); + + quitLifecycle.handleBeforeQuit({ + preventDefault: () => { + events.push("second-prevent-default"); + }, + }); + + expect(events.at(-1)).toBe("close-transports"); + expect(events).not.toContain("second-prevent-default"); + }); + + it("lets the updater own process exit when a validated update is installing", async () => { + const exits: number[] = []; + const quitLifecycle = createQuitLifecycle({ + app: { exit: (code) => exits.push(code) }, + closeTransportSessions: () => {}, + stopDesktopManagedDaemonIfNeeded: async () => false, + installAppUpdateOnQuit: async () => true, + createUpdateDeadlineSignal: () => new AbortController().signal, + onStopError: () => {}, + onUpdateError: () => {}, + }); + + quitLifecycle.handleBeforeQuit({ preventDefault: () => {} }); + await waitForQuitLifecycle(); + quitLifecycle.handleBeforeQuitForUpdate(); + await waitForQuitLifecycle(); + + expect(exits).toEqual([]); + }); + + it("recognizes a repeated quit as updater handoff", async () => { + const exits: number[] = []; + let preventedQuitCount = 0; + const quitLifecycle = createQuitLifecycle({ + app: { exit: (code) => exits.push(code) }, + closeTransportSessions: () => {}, + stopDesktopManagedDaemonIfNeeded: async () => false, + installAppUpdateOnQuit: async () => true, + createUpdateDeadlineSignal: () => new AbortController().signal, + onStopError: () => {}, + onUpdateError: () => {}, + }); + + quitLifecycle.handleBeforeQuit({ preventDefault: () => preventedQuitCount++ }); + await waitForQuitLifecycle(); + quitLifecycle.handleBeforeQuit({ preventDefault: () => preventedQuitCount++ }); + await waitForQuitLifecycle(); + + expect(preventedQuitCount).toBe(1); + expect(exits).toEqual([]); + }); + + it("exits when the updater does not take ownership before its deadline", async () => { + const revalidationDeadline = new AbortController(); + const handoffDeadline = new AbortController(); + let deadlineCount = 0; + const exits: number[] = []; + const quitLifecycle = createQuitLifecycle({ + app: { exit: (code) => exits.push(code) }, + closeTransportSessions: () => {}, + stopDesktopManagedDaemonIfNeeded: async () => false, + installAppUpdateOnQuit: async () => true, + createUpdateDeadlineSignal: () => + deadlineCount++ === 0 ? revalidationDeadline.signal : handoffDeadline.signal, + onStopError: () => {}, + onUpdateError: () => {}, + }); + + quitLifecycle.handleBeforeQuit({ preventDefault: () => {} }); + await waitForQuitLifecycle(); + handoffDeadline.abort(); + await waitForQuitLifecycle(); + + expect(exits).toEqual([0]); + }); + + it("does not intercept a quit started by a manual update", () => { + const events: string[] = []; + const quitLifecycle = createQuitLifecycle({ + app: { exit: (code) => events.push(`exit:${code}`) }, + closeTransportSessions: () => events.push("close-transports"), + stopDesktopManagedDaemonIfNeeded: async () => { + events.push("stop-daemon"); + return false; + }, + installAppUpdateOnQuit: async () => { + events.push("revalidate-update"); + return false; + }, + createUpdateDeadlineSignal: () => new AbortController().signal, + onStopError: () => events.push("stop-error"), + onUpdateError: () => events.push("update-error"), + }); + + quitLifecycle.handleBeforeQuitForUpdate(); + quitLifecycle.handleBeforeQuit({ + preventDefault: () => events.push("prevent-default"), + }); + + expect(events).toEqual(["close-transports"]); + }); + + it("exits when update revalidation reaches its deadline", async () => { + const deadline = new AbortController(); + const updateDecision = deferred(); + const exits: number[] = []; + const quitLifecycle = createQuitLifecycle({ + app: { exit: (code) => exits.push(code) }, + closeTransportSessions: () => {}, + stopDesktopManagedDaemonIfNeeded: async () => false, + installAppUpdateOnQuit: () => updateDecision.promise, + createUpdateDeadlineSignal: () => deadline.signal, + onStopError: () => {}, + onUpdateError: () => {}, + }); + + quitLifecycle.handleBeforeQuit({ preventDefault: () => {} }); + await waitForQuitLifecycle(); + deadline.abort(); + await waitForQuitLifecycle(); + + expect(exits).toEqual([0]); + + updateDecision.resolve(true); + await waitForQuitLifecycle(); + expect(exits).toEqual([0]); }); }); diff --git a/packages/desktop/src/daemon/quit-lifecycle.ts b/packages/desktop/src/daemon/quit-lifecycle.ts index 879f59778..1998985f5 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.ts @@ -14,6 +14,16 @@ interface BeforeQuitApp { exit(code: number): void; } +interface QuitLifecycle { + handleBeforeQuit(event: BeforeQuitEvent): void; + handleBeforeQuitForUpdate(): void; +} + +interface DeferredUpdateQuit { + promise: Promise; + resolve(): void; +} + export interface StopOnQuitDeps { settingsStore: Pick; isDesktopManagedDaemonRunning: () => boolean; @@ -42,36 +52,95 @@ export async function stopDesktopManagedDaemonOnQuitIfNeeded( return true; } -export function createBeforeQuitHandler({ +function waitForUpdateDeadline(signal: AbortSignal): Promise { + if (signal.aborted) { + return Promise.resolve(false); + } + + return new Promise((resolve) => { + signal.addEventListener("abort", () => resolve(false), { once: true }); + }); +} + +function createDeferredUpdateQuit(): DeferredUpdateQuit { + let resolvePromise!: (started: boolean) => void; + const promise = new Promise((resolve) => { + resolvePromise = resolve; + }); + return { promise, resolve: () => resolvePromise(true) }; +} + +export function createQuitLifecycle({ app, closeTransportSessions, stopDesktopManagedDaemonIfNeeded, + installAppUpdateOnQuit, + createUpdateDeadlineSignal, onStopError, + onUpdateError, }: { app: BeforeQuitApp; closeTransportSessions: () => void; stopDesktopManagedDaemonIfNeeded: () => Promise; + installAppUpdateOnQuit: (signal: AbortSignal) => Promise; + createUpdateDeadlineSignal: () => AbortSignal; onStopError: (error: unknown) => void; -}): (event: BeforeQuitEvent) => void { - // We always preventDefault on first quit so we can run the async stop - // decision, then call app.exit(0) — which bypasses Electron's - // close → window-all-closed → will-quit chain. The window-all-closed - // listener is a darwin no-op (macOS convention) and would otherwise - // veto a re-fired app.quit(). + onUpdateError: (error: unknown) => void; +}): QuitLifecycle { + // The first quit waits for daemon shutdown and update revalidation. A validated + // update re-fires app.quit(); otherwise app.exit(0) bypasses Electron's macOS + // window-all-closed handler, which would veto that second quit. let quitting = false; + let quittingForUpdate = false; + const updateQuit = createDeferredUpdateQuit(); - return (event) => { + function handleBeforeQuit(event: BeforeQuitEvent): void { closeTransportSessions(); - if (quitting) return; + if (quittingForUpdate) return; + if (quitting) { + // MacUpdater's no-relaunch path calls app.quit() without emitting + // before-quit-for-update. A second quit is equivalent handoff evidence. + updateQuit.resolve(); + return; + } quitting = true; event.preventDefault(); - void stopDesktopManagedDaemonIfNeeded() - .catch((error) => { + void (async () => { + try { + await stopDesktopManagedDaemonIfNeeded(); + } catch (error) { onStopError(error); - }) - .finally(() => { - app.exit(0); + } + + const signal = createUpdateDeadlineSignal(); + const updateInstallation = installAppUpdateOnQuit(signal).catch((error) => { + onUpdateError(error); + return false; }); + const installingUpdate = await Promise.race([ + updateInstallation, + waitForUpdateDeadline(signal), + ]); + if (installingUpdate) { + const handoffStarted = await Promise.race([ + updateQuit.promise, + waitForUpdateDeadline(createUpdateDeadlineSignal()), + ]); + if (handoffStarted) { + return; + } + } + + app.exit(0); + })(); + } + + return { + handleBeforeQuit, + handleBeforeQuitForUpdate() { + quittingForUpdate = true; + updateQuit.resolve(); + }, }; } diff --git a/packages/desktop/src/features/app-update-service.test.ts b/packages/desktop/src/features/app-update-service.test.ts index f243a7c28..1e8154602 100644 --- a/packages/desktop/src/features/app-update-service.test.ts +++ b/packages/desktop/src/features/app-update-service.test.ts @@ -17,7 +17,19 @@ class FakeAppUpdateRuntime implements AppUpdateRuntime { > = []; private gate: ((info: RuntimeUpdateInfo) => boolean | Promise) | null = null; private configuration: AppUpdateRuntimeConfiguration | null = null; + private downloadableUpdate: RuntimeUpdateInfo | null = null; + private downloadedUpdate: RuntimeUpdateInfo | null = null; + private activeDownload: { + info: RuntimeUpdateInfo; + promise: Promise; + resolve(): void; + reject(error: Error): void; + } | null = null; checkCount = 0; + downloadCallCount = 0; + downloadedVersions: string[] = []; + installedVersions: string[] = []; + installModes: Array<{ isSilent: boolean; isForceRunAfter: boolean }> = []; configure(input: AppUpdateRuntimeConfiguration): void { this.configuration = input; @@ -59,9 +71,42 @@ class FakeAppUpdateRuntime implements AppUpdateRuntime { } finishUpdateDownload(info: RuntimeUpdateInfo): void { + this.downloadedUpdate = info; + this.downloadedVersions.push(info.version); this.configuration?.onUpdateDownloaded(info); } + beginUpdateDownload(info: RuntimeUpdateInfo): { + resolve(): void; + reject(error: Error): void; + } { + this.downloadableUpdate = info; + this.prepareUpdate(info); + let resolvePromise!: () => void; + let rejectPromise!: (error: Error) => void; + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve; + rejectPromise = reject; + }); + void promise.catch(() => undefined); + const activeDownload = { + info, + promise, + resolve: () => { + this.finishUpdateDownload(info); + this.activeDownload = null; + resolvePromise(); + }, + reject: (error: Error) => { + this.configuration?.onError(error); + this.activeDownload = null; + rejectPromise(error); + }, + }; + this.activeDownload = activeDownload; + return { resolve: activeDownload.resolve, reject: activeDownload.reject }; + } + async checkForUpdates(): Promise<{ isUpdateAvailable: boolean; updateInfo: RuntimeUpdateInfo; @@ -80,12 +125,27 @@ class FakeAppUpdateRuntime implements AppUpdateRuntime { } if (!result || !this.gate) return result; const admitted = await this.gate(result.updateInfo); - return { ...result, isUpdateAvailable: result.isUpdateAvailable && admitted }; + const isUpdateAvailable = result.isUpdateAvailable && admitted; + this.downloadableUpdate = isUpdateAvailable ? result.updateInfo : null; + return { ...result, isUpdateAvailable }; } - async downloadUpdate(): Promise {} + async downloadUpdate(): Promise { + this.downloadCallCount += 1; + if (this.activeDownload) { + return this.activeDownload.promise; + } + if (this.downloadableUpdate) { + this.finishUpdateDownload(this.downloadableUpdate); + } + } - quitAndInstall(): void {} + quitAndInstall(isSilent: boolean, isForceRunAfter: boolean): void { + if (this.downloadedUpdate) { + this.installedVersions.push(this.downloadedUpdate.version); + this.installModes.push({ isSilent, isForceRunAfter }); + } + } } function createService(input?: { now?: () => number; bucket?: () => Promise }) { @@ -148,6 +208,33 @@ describe("app update service", () => { }); }); + it("waits for an automatic poll before starting a manual rollout-bypassing check", async () => { + const { runtime, service } = createService(); + const automaticCheck = runtime.deferNextCheck(); + const automaticPending = service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + const manualPending = service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "manual", + }); + + await Promise.resolve(); + expect(runtime.checkCount).toBe(1); + + automaticCheck.resolve({ isUpdateAvailable: false, updateInfo: rolledOutUpdate }); + await automaticPending; + const manualResult = await manualPending; + + expect(runtime.checkCount).toBe(2); + expect(manualResult.hasUpdate).toBe(true); + expect(manualResult.latestVersion).toBe("1.2.4"); + }); + it("performs a fresh manual check when an update is already cached", async () => { const { runtime, service } = createService({ bucket: async () => 0 }); runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); @@ -179,6 +266,218 @@ describe("app update service", () => { }); }); + it("replaces a downloaded update when a newer release is admitted", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.finishUpdateDownload(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const result = await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + + expect(result).toEqual({ + hasUpdate: true, + readyToInstall: false, + currentVersion: "1.2.3", + latestVersion: "1.2.5", + body: null, + date: "2026-04-28T00:00:00.000Z", + errorMessage: null, + }); + }); + + it("installs the newest admitted release when quitting with an older download", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.finishUpdateDownload(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const installed = await service.installUpdateOnQuit({ + currentVersion: "1.2.3", + releaseChannel: "stable", + signal: new AbortController().signal, + }); + + expect(installed).toBe(true); + expect(runtime.installedVersions).toEqual(["1.2.5"]); + expect(runtime.installModes).toEqual([{ isSilent: true, isForceRunAfter: false }]); + }); + + it("does not install an older download while its replacement is still rolling out", async () => { + const now = Date.parse("2026-04-28T12:00:00.000Z"); + const { runtime, service } = createService({ now: () => now, bucket: async () => 0.4 }); + const olderUpdate = { + ...rolledOutUpdate, + releaseDate: "2026-04-27T00:00:00.000Z", + }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: olderUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.finishUpdateDownload(olderUpdate); + + const newerUpdate = { + ...rolledOutUpdate, + version: "1.2.5", + releaseDate: "2026-04-28T12:00:00.000Z", + }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const installed = await service.installUpdateOnQuit({ + currentVersion: "1.2.3", + releaseChannel: "stable", + signal: new AbortController().signal, + }); + + expect(installed).toBe(false); + expect(runtime.installedVersions).toEqual([]); + }); + + it("does not install after quit-time revalidation expires", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.finishUpdateDownload(rolledOutUpdate); + + const deadline = new AbortController(); + deadline.abort(); + runtime.nextCheck({ + isUpdateAvailable: true, + updateInfo: { ...rolledOutUpdate, version: "1.2.5" }, + }); + const installed = await service.installUpdateOnQuit({ + currentVersion: "1.2.3", + releaseChannel: "stable", + signal: deadline.signal, + }); + + expect(installed).toBe(false); + expect(runtime.installedVersions).toEqual([]); + }); + + it("does not install an unvalidated download when the quit-time check fails", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.finishUpdateDownload(rolledOutUpdate); + + runtime.failNextCheck(new Error("offline")); + const installed = await service.installUpdateOnQuit({ + currentVersion: "1.2.3", + releaseChannel: "stable", + signal: new AbortController().signal, + }); + + expect(installed).toBe(false); + expect(runtime.installedVersions).toEqual([]); + }); + + it("rechecks for the newest release before a manual install", async () => { + const { runtime, service } = createService({ bucket: async () => 0.99 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "manual", + }); + runtime.finishUpdateDownload(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const result = await service.downloadAndInstallUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + }); + + expect(result.installed).toBe(true); + expect(runtime.installedVersions).toEqual(["1.2.5"]); + expect(runtime.installModes).toEqual([{ isSilent: false, isForceRunAfter: true }]); + }); + + it("waits for a stale active download before downloading and installing the rechecked version", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + const staleDownload = runtime.beginUpdateDownload(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const installPending = service.downloadAndInstallUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + }); + await Promise.resolve(); + expect(runtime.installedVersions).toEqual([]); + + staleDownload.resolve(); + const result = await installPending; + + expect(result.installed).toBe(true); + expect(runtime.downloadedVersions).toEqual(["1.2.4", "1.2.5"]); + expect(runtime.installedVersions).toEqual(["1.2.5"]); + }); + + it("installs the rechecked version when the stale active download fails", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + const staleDownload = runtime.beginUpdateDownload(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const installPending = service.downloadAndInstallUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + }); + await new Promise((resolve) => setImmediate(resolve)); + expect(runtime.downloadCallCount).toBe(1); + + staleDownload.reject(new Error("old download failed")); + const result = await installPending; + + expect(result.installed).toBe(true); + expect(runtime.downloadedVersions).toEqual(["1.2.5"]); + expect(runtime.installedVersions).toEqual(["1.2.5"]); + }); + it("trusts the runtime availability decision before comparing versions", async () => { const { runtime, service } = createService({ bucket: async () => 0 }); runtime.nextCheck({ isUpdateAvailable: false, updateInfo: rolledOutUpdate }); @@ -322,28 +621,26 @@ describe("app update service", () => { }); }); - it("keeps preparation errors emitted before the update check rejects", async () => { - const { runtime, service } = createService(); - const deferredCheck = runtime.deferNextCheck(); - const pending = service.checkForAppUpdate({ + it("surfaces preparation errors without blocking newer releases", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + + await service.checkForAppUpdate({ currentVersion: "1.2.3", releaseChannel: "stable", - intent: "automatic", + intent: "manual", }); - runtime.prepareUpdate(rolledOutUpdate); runtime.failRuntime(new Error("sha512 checksum mismatch")); - deferredCheck.reject(new Error("sha512 checksum mismatch")); - const checkResult = await pending; - expect(checkResult.errorMessage).toBe("sha512 checksum mismatch"); - const automaticResult = await service.checkForAppUpdate({ + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + const failedPreparation = await service.checkForAppUpdate({ currentVersion: "1.2.3", releaseChannel: "stable", intent: "automatic", }); - expect(automaticResult).toEqual({ + expect(failedPreparation).toEqual({ hasUpdate: true, readyToInstall: false, currentVersion: "1.2.3", @@ -352,19 +649,9 @@ describe("app update service", () => { date: "2026-04-28T00:00:00.000Z", errorMessage: "sha512 checksum mismatch", }); - }); - - it("returns runtime update errors after an update fails to prepare", async () => { - const { runtime, service } = createService(); - runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); - - await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "manual", - }); - runtime.failRuntime(new Error("sha512 checksum mismatch")); + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); const result = await service.checkForAppUpdate({ currentVersion: "1.2.3", releaseChannel: "stable", @@ -375,13 +662,43 @@ describe("app update service", () => { hasUpdate: true, readyToInstall: false, currentVersion: "1.2.3", - latestVersion: "1.2.4", + latestVersion: "1.2.5", body: null, date: "2026-04-28T00:00:00.000Z", - errorMessage: "sha512 checksum mismatch", + errorMessage: null, }); }); + it("attributes a late preparation failure to the download that started it", async () => { + const { runtime, service } = createService({ bucket: async () => 0 }); + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.prepareUpdate(rolledOutUpdate); + + const newerUpdate = { ...rolledOutUpdate, version: "1.2.5" }; + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + runtime.failRuntime(new Error("old download failed")); + + runtime.nextCheck({ isUpdateAvailable: true, updateInfo: newerUpdate }); + const result = await service.checkForAppUpdate({ + currentVersion: "1.2.3", + releaseChannel: "stable", + intent: "automatic", + }); + + expect(result.latestVersion).toBe("1.2.5"); + expect(result.errorMessage).toBeNull(); + }); + it("performs a fresh manual check after an update preparation error", async () => { const { runtime, service } = createService(); runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); @@ -391,6 +708,7 @@ describe("app update service", () => { releaseChannel: "stable", intent: "manual", }); + runtime.prepareUpdate(rolledOutUpdate); runtime.failRuntime(new Error("sha512 checksum mismatch")); runtime.nextCheck({ @@ -445,91 +763,4 @@ describe("app update service", () => { errorMessage: null, }); }); - - it("returns runtime update errors to multiple automatic checks before a manual retry clears them", async () => { - const { runtime, service } = createService(); - runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); - - await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "manual", - }); - runtime.failRuntime(new Error("sha512 checksum mismatch")); - - const firstAutomaticResult = await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "automatic", - }); - const secondAutomaticResult = await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "automatic", - }); - - expect(firstAutomaticResult).toEqual({ - hasUpdate: true, - readyToInstall: false, - currentVersion: "1.2.3", - latestVersion: "1.2.4", - body: null, - date: "2026-04-28T00:00:00.000Z", - errorMessage: "sha512 checksum mismatch", - }); - expect(secondAutomaticResult).toEqual(firstAutomaticResult); - - runtime.nextCheck(null); - const retryResult = await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "manual", - }); - - expect(runtime.checkCount).toBe(2); - expect(retryResult).toEqual({ - hasUpdate: false, - readyToInstall: false, - currentVersion: "1.2.3", - latestVersion: "1.2.3", - body: null, - date: null, - errorMessage: null, - }); - }); - - it("keeps runtime update errors visible after a manual retry fails", async () => { - const { runtime, service } = createService(); - runtime.nextCheck({ isUpdateAvailable: true, updateInfo: rolledOutUpdate }); - - await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "manual", - }); - runtime.failRuntime(new Error("sha512 checksum mismatch")); - - runtime.failNextCheck(new Error("network down")); - const retryResult = await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "manual", - }); - const automaticResult = await service.checkForAppUpdate({ - currentVersion: "1.2.3", - releaseChannel: "stable", - intent: "automatic", - }); - - expect(retryResult.errorMessage).toBe("network down"); - expect(automaticResult).toEqual({ - hasUpdate: true, - readyToInstall: false, - currentVersion: "1.2.3", - latestVersion: "1.2.4", - body: null, - date: "2026-04-28T00:00:00.000Z", - errorMessage: "sha512 checksum mismatch", - }); - }); }); diff --git a/packages/desktop/src/features/app-update-service.ts b/packages/desktop/src/features/app-update-service.ts index f62dd8167..d5dc513ef 100644 --- a/packages/desktop/src/features/app-update-service.ts +++ b/packages/desktop/src/features/app-update-service.ts @@ -62,6 +62,11 @@ export interface AppUpdateService { }, onBeforeQuit?: () => Promise, ): Promise; + installUpdateOnQuit(input: { + currentVersion: string; + releaseChannel: AppReleaseChannel; + signal: AbortSignal; + }): Promise; } export interface AppUpdateServiceDeps { @@ -96,10 +101,16 @@ function buildCheckResult(input: { async function performQuitAndInstall( runtime: AppUpdateRuntime, - onBeforeQuit?: () => Promise, + { + onBeforeQuit, + restart, + }: { + onBeforeQuit?: () => Promise; + restart: boolean; + }, ): Promise { if (onBeforeQuit) await onBeforeQuit(); - runtime.quitAndInstall(/* isSilent */ false, /* isForceRunAfter */ true); + runtime.quitAndInstall(/* isSilent */ !restart, /* isForceRunAfter */ restart); } function getErrorMessage(error: unknown): string { @@ -109,13 +120,21 @@ function getErrorMessage(error: unknown): string { return String(error); } +function buildDeferredInstallResult(currentVersion: string): AppUpdateInstallResult { + return { + installed: false, + version: currentVersion, + message: "Update validation timed out. The update will be installed later.", + }; +} + export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateService { let cachedUpdateInfo: RuntimeUpdateInfo | null = null; let downloadedUpdateVersion: string | null = null; - let downloading = false; let configuredReleaseChannel: AppReleaseChannel | null = null; - let runtimeErrorMessage: string | null = null; - let inFlightUpdateCheckCount = 0; + let preparationError: { version: string; message: string } | null = null; + let preparingUpdateVersion: string | null = null; + let checkQueue: Promise = Promise.resolve(); function isReadyToInstallVersion(version: string): boolean { return downloadedUpdateVersion === version; @@ -124,23 +143,8 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer function clearUpdateState(): void { cachedUpdateInfo = null; downloadedUpdateVersion = null; - downloading = false; - runtimeErrorMessage = null; - } - - function buildRuntimeErrorResult(currentVersion: string): AppUpdateCheckResult | null { - if (!runtimeErrorMessage) { - return null; - } - - const info = cachedUpdateInfo; - return buildCheckResult({ - currentVersion, - hasUpdate: info?.version !== undefined && info.version !== currentVersion, - readyToInstall: false, - info, - errorMessage: runtimeErrorMessage, - }); + preparationError = null; + preparingUpdateVersion = null; } function configureRuntime(releaseChannel: AppReleaseChannel, intent: AppUpdateCheckIntent): void { @@ -166,28 +170,47 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer const alreadyReady = downloadedUpdateVersion === info.version; cachedUpdateInfo = info; downloadedUpdateVersion = alreadyReady ? info.version : null; - downloading = !alreadyReady; - runtimeErrorMessage = null; + if (!alreadyReady && preparingUpdateVersion === null) { + preparingUpdateVersion = info.version; + } }, onUpdateDownloaded(info) { - cachedUpdateInfo = info; + // A superseded download can finish after a newer manifest check. Keep + // the validated manifest as the install target in that case. + cachedUpdateInfo ??= info; downloadedUpdateVersion = info.version; - downloading = false; - runtimeErrorMessage = null; + if (preparingUpdateVersion === info.version) { + preparingUpdateVersion = null; + } + if (preparationError?.version === info.version) { + preparationError = null; + } }, onUpdateNotAvailable() { clearUpdateState(); }, onError(error) { - downloading = false; - if (inFlightUpdateCheckCount === 0 || cachedUpdateInfo) { - runtimeErrorMessage = getErrorMessage(error); + if (preparingUpdateVersion) { + preparationError = { + version: preparingUpdateVersion, + message: getErrorMessage(error), + }; + preparingUpdateVersion = null; } deps.reportRuntimeError?.(error); }, }); } + function runCheckExclusively(check: () => Promise): Promise { + const result = checkQueue.then(check, check); + checkQueue = result.then( + () => undefined, + () => undefined, + ); + return result; + } + async function checkForAppUpdate({ currentVersion, releaseChannel, @@ -205,73 +228,56 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer }); } - configureRuntime(releaseChannel, intent); + return runCheckExclusively(async () => { + configureRuntime(releaseChannel, intent); - const runtimeErrorResult = buildRuntimeErrorResult(currentVersion); - if (runtimeErrorResult && intent === "automatic") { - return runtimeErrorResult; - } + try { + const result = await deps.runtime.checkForUpdates(); + if (!result || !result.updateInfo || !result.isUpdateAvailable) { + clearUpdateState(); + return buildCheckResult({ + currentVersion, + hasUpdate: false, + readyToInstall: false, + }); + } - const cachedVersion = cachedUpdateInfo?.version ?? null; - if ( - !runtimeErrorResult && - intent === "automatic" && - cachedVersion && - cachedVersion !== currentVersion - ) { - return buildCheckResult({ - currentVersion, - hasUpdate: true, - readyToInstall: isReadyToInstallVersion(cachedVersion), - info: cachedUpdateInfo, - }); - } + const info = result.updateInfo; + const latestVersion = info.version; + const hasUpdate = latestVersion !== currentVersion; + + if (hasUpdate) { + cachedUpdateInfo = info; + const errorMessage = + preparationError?.version === latestVersion ? preparationError.message : null; + if (!errorMessage) { + preparationError = null; + } + return buildCheckResult({ + currentVersion, + hasUpdate: true, + readyToInstall: isReadyToInstallVersion(latestVersion), + info, + errorMessage, + }); + } - try { - inFlightUpdateCheckCount += 1; - const result = await deps.runtime.checkForUpdates(); - if (!result || !result.updateInfo || !result.isUpdateAvailable) { clearUpdateState(); return buildCheckResult({ currentVersion, hasUpdate: false, readyToInstall: false, }); - } - - const info = result.updateInfo; - const latestVersion = info.version; - const hasUpdate = latestVersion !== currentVersion; - - if (hasUpdate) { - cachedUpdateInfo = info; - downloading = !isReadyToInstallVersion(latestVersion); - runtimeErrorMessage = null; + } catch (error) { + deps.reportCheckError?.(error); return buildCheckResult({ currentVersion, - hasUpdate: true, - readyToInstall: isReadyToInstallVersion(latestVersion), - info, + hasUpdate: false, + readyToInstall: false, + errorMessage: getErrorMessage(error), }); } - - clearUpdateState(); - return buildCheckResult({ - currentVersion, - hasUpdate: false, - readyToInstall: false, - }); - } catch (error) { - deps.reportCheckError?.(error); - return buildCheckResult({ - currentVersion, - hasUpdate: false, - readyToInstall: false, - errorMessage: getErrorMessage(error), - }); - } finally { - inFlightUpdateCheckCount -= 1; - } + }); } async function downloadAndInstallUpdate( @@ -292,6 +298,69 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer }; } + const check = await checkForAppUpdate({ + currentVersion, + releaseChannel, + intent: "manual", + }); + if (!check.hasUpdate) { + return { + installed: false, + version: currentVersion, + message: check.errorMessage ?? "No update available.", + }; + } + + return installCachedUpdate(currentVersion, { onBeforeQuit, restart: true }); + } + + async function ensureUpdateDownloaded( + readyVersion: string, + signal?: AbortSignal, + ): Promise<"ready" | "aborted" | "superseded"> { + while (!isReadyToInstallVersion(readyVersion)) { + if (signal?.aborted) return "aborted"; + if (cachedUpdateInfo?.version !== readyVersion) return "superseded"; + + const attemptedVersion: string = preparingUpdateVersion ?? readyVersion; + preparingUpdateVersion ??= readyVersion; + try { + await deps.runtime.downloadUpdate(); + } catch (error) { + if ( + attemptedVersion !== readyVersion && + cachedUpdateInfo?.version === readyVersion && + !signal?.aborted + ) { + continue; + } + throw error; + } + + // electron-updater can return an older, already-running download. Its + // event clears that version, then the next iteration starts the newly + // validated release instead of treating the stale artifact as ready. + if (attemptedVersion === readyVersion && !isReadyToInstallVersion(readyVersion)) { + downloadedUpdateVersion = readyVersion; + preparingUpdateVersion = null; + } + } + + return signal?.aborted ? "aborted" : "ready"; + } + + async function installCachedUpdate( + currentVersion: string, + { + onBeforeQuit, + signal, + restart, + }: { + onBeforeQuit?: () => Promise; + signal?: AbortSignal; + restart: boolean; + }, + ): Promise { if (!cachedUpdateInfo) { return { installed: false, @@ -300,11 +369,13 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer }; } - configureRuntime(releaseChannel, "manual"); - const readyVersion = cachedUpdateInfo.version; + if (signal?.aborted) { + return buildDeferredInstallResult(currentVersion); + } + if (isReadyToInstallVersion(readyVersion)) { - await performQuitAndInstall(deps.runtime, onBeforeQuit); + await performQuitAndInstall(deps.runtime, { onBeforeQuit, restart }); return { installed: true, version: readyVersion, @@ -312,21 +383,19 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer }; } - if (downloading) { - return { - installed: false, - version: currentVersion, - message: "Update is still being prepared. Try again in a moment.", - }; - } - - downloading = true; - try { - await deps.runtime.downloadUpdate(); - downloadedUpdateVersion = readyVersion; - downloading = false; - await performQuitAndInstall(deps.runtime, onBeforeQuit); + const preparation = await ensureUpdateDownloaded(readyVersion, signal); + if (preparation === "aborted") { + return buildDeferredInstallResult(currentVersion); + } + if (preparation === "superseded") { + return { + installed: false, + version: currentVersion, + message: "A newer update was found and will be installed later.", + }; + } + await performQuitAndInstall(deps.runtime, { onBeforeQuit, restart }); return { installed: true, @@ -334,7 +403,6 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer message: "Update downloaded. The app will restart shortly.", }; } catch (error) { - downloading = false; const message = error instanceof Error ? error.message : String(error); deps.reportInstallError?.(message); return { @@ -345,8 +413,35 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer } } + async function installUpdateOnQuit({ + currentVersion, + releaseChannel, + signal, + }: { + currentVersion: string; + releaseChannel: AppReleaseChannel; + signal: AbortSignal; + }): Promise { + if (!deps.isPackaged() || !downloadedUpdateVersion) { + return false; + } + + const check = await checkForAppUpdate({ + currentVersion, + releaseChannel, + intent: "automatic", + }); + if (signal.aborted || !check.hasUpdate) { + return false; + } + + const result = await installCachedUpdate(currentVersion, { signal, restart: false }); + return result.installed; + } + return { checkForAppUpdate, downloadAndInstallUpdate, + installUpdateOnQuit, }; } diff --git a/packages/desktop/src/features/auto-updater.test.ts b/packages/desktop/src/features/auto-updater.test.ts index 4de12d87d..3c28e61c1 100644 --- a/packages/desktop/src/features/auto-updater.test.ts +++ b/packages/desktop/src/features/auto-updater.test.ts @@ -19,15 +19,15 @@ import { resolveStagingUserId, rolloutManifestSchema, shouldAdmitToRollout, - shouldAutoInstallOnQuit, + shouldInstallAppUpdateOnQuit, } from "./auto-updater"; -describe("shouldAutoInstallOnQuit", () => { - it("auto-installs on quit everywhere except Linux AppImage", () => { - expect(shouldAutoInstallOnQuit({ platform: "linux", isAppImage: true })).toBe(false); - expect(shouldAutoInstallOnQuit({ platform: "linux", isAppImage: false })).toBe(true); - expect(shouldAutoInstallOnQuit({ platform: "darwin", isAppImage: false })).toBe(true); - expect(shouldAutoInstallOnQuit({ platform: "win32", isAppImage: false })).toBe(true); +describe("shouldInstallAppUpdateOnQuit", () => { + it("keeps Linux AppImage updates on the manual install path", () => { + expect(shouldInstallAppUpdateOnQuit({ platform: "linux", isAppImage: true })).toBe(false); + expect(shouldInstallAppUpdateOnQuit({ platform: "linux", isAppImage: false })).toBe(true); + expect(shouldInstallAppUpdateOnQuit({ platform: "darwin", isAppImage: false })).toBe(true); + expect(shouldInstallAppUpdateOnQuit({ platform: "win32", isAppImage: false })).toBe(true); }); }); diff --git a/packages/desktop/src/features/auto-updater.ts b/packages/desktop/src/features/auto-updater.ts index 876114585..c9b9a983b 100644 --- a/packages/desktop/src/features/auto-updater.ts +++ b/packages/desktop/src/features/auto-updater.ts @@ -76,18 +76,12 @@ export function getStagingUserId(): Promise { return cachedStagingUserIdPromise; } -// AppImages have no install step. electron-updater "installs" by unlinking the -// running file and mv-ing the downloaded one into place; on app quit it does this -// via a *blocking* execFileSync(newAppImage, { APPIMAGE_EXIT_AFTER_INSTALL: "true" }). -// That env var is only honored by AppImageLauncher, so without it the freshly -// launched process boots the full app and never exits — the quit hangs forever, -// with the old binary already deleted. We therefore install AppImages only on -// explicit quitAndInstall (the "Update now" button), which takes the non-blocking -// spawn path. Every other target keeps auto-install-on-quit, which works there. -export function shouldAutoInstallOnQuit(input: { +export function shouldInstallAppUpdateOnQuit(input: { platform: NodeJS.Platform; isAppImage: boolean; }): boolean { + // AppImage's no-relaunch install path blocks while launching the replacement + // binary, which can hang after the running file has already been replaced. return !(input.platform === "linux" && input.isAppImage); } @@ -97,10 +91,10 @@ class ElectronAppUpdateRuntime implements AppUpdateRuntime { configure(input: AppUpdateRuntimeConfiguration): void { autoUpdater.autoDownload = true; autoUpdater.autoRunAppAfterInstall = true; - autoUpdater.autoInstallOnAppQuit = shouldAutoInstallOnQuit({ - platform: process.platform, - isAppImage: Boolean(process.env.APPIMAGE), - }); + // Paseo revalidates the current manifest before explicitly installing on quit. + // Electron's built-in handler would install an older download without checking + // whether a newer release has superseded it. + autoUpdater.autoInstallOnAppQuit = false; autoUpdater.allowPrerelease = input.releaseChannel === "beta"; autoUpdater.channel = input.releaseChannel === "beta" ? "beta" : "latest"; autoUpdater.allowDowngrade = false; @@ -143,6 +137,7 @@ class ElectronAppUpdateRuntime implements AppUpdateRuntime { } quitAndInstall(isSilent: boolean, isForceRunAfter: boolean): void { + autoUpdater.autoRunAppAfterInstall = isForceRunAfter; autoUpdater.quitAndInstall(isSilent, isForceRunAfter); } } @@ -194,3 +189,24 @@ export async function downloadAndInstallUpdate( onBeforeQuit, ); } + +export async function installAppUpdateOnQuit({ + currentVersion, + releaseChannel, + signal, +}: { + currentVersion: string; + releaseChannel: AppReleaseChannel; + signal: AbortSignal; +}): Promise { + if ( + !shouldInstallAppUpdateOnQuit({ + platform: process.platform, + isAppImage: Boolean(process.env.APPIMAGE), + }) + ) { + return false; + } + + return appUpdateService.installUpdateOnQuit({ currentVersion, releaseChannel, signal }); +} diff --git a/packages/desktop/src/main.ts b/packages/desktop/src/main.ts index a75eb7106..ef4e8a82e 100644 --- a/packages/desktop/src/main.ts +++ b/packages/desktop/src/main.ts @@ -12,6 +12,7 @@ import { existsSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { app, + autoUpdater as electronAutoUpdater, BrowserWindow, clipboard, Menu, @@ -81,19 +82,21 @@ import { stopDesktopDaemonViaCli, } from "./daemon/daemon-manager.js"; import { - createBeforeQuitHandler, + createQuitLifecycle, stopDesktopManagedDaemonOnQuitIfNeeded, } from "./daemon/quit-lifecycle.js"; import { runDesktopStartup } from "./desktop-startup.js"; import { autoUpdateInstalledSkills } from "./integrations/skills/index.js"; import { registerBrowserAutomationIpc } from "./features/browser-automation/ipc.js"; import { BrowserKeyboard } from "./features/browser-keyboard/index.js"; +import { installAppUpdateOnQuit } from "./features/auto-updater.js"; const DEV_SERVER_URL = process.env.EXPO_DEV_URL ?? "http://localhost:8081"; const APP_SCHEME = "paseo"; const PASEO_DEBUG = process.env.PASEO_DEBUG === "1"; const DISABLE_SINGLE_INSTANCE_LOCK = process.env.PASEO_DISABLE_SINGLE_INSTANCE_LOCK === "1"; const APP_NAME = process.env.PASEO_TEST_APP_NAME?.trim() || "Paseo"; +const UPDATE_QUIT_DEADLINE_MS = 5_000; const pendingBrowserWindowOpenRequests = new PendingBrowserWindowOpenRequests(); app.setName(APP_NAME); @@ -928,23 +931,36 @@ function showDaemonShutdownDialog(): void { } } -app.on( - "before-quit", - createBeforeQuitHandler({ - app, - closeTransportSessions: closeAllTransportSessions, - stopDesktopManagedDaemonIfNeeded: () => - stopDesktopManagedDaemonOnQuitIfNeeded({ - settingsStore: getDesktopSettingsStore(), - isDesktopManagedDaemonRunning: isDesktopManagedDaemonRunningSync, - stopDaemon: () => stopDesktopDaemonViaCli("quit"), - showShutdownFeedback: showDaemonShutdownDialog, - }), - onStopError: (error) => { - log.error("[desktop daemon] failed to stop managed daemon on quit", error); - }, - }), -); +const quitLifecycle = createQuitLifecycle({ + app, + closeTransportSessions: closeAllTransportSessions, + stopDesktopManagedDaemonIfNeeded: () => + stopDesktopManagedDaemonOnQuitIfNeeded({ + settingsStore: getDesktopSettingsStore(), + isDesktopManagedDaemonRunning: isDesktopManagedDaemonRunningSync, + stopDaemon: () => stopDesktopDaemonViaCli("quit"), + showShutdownFeedback: showDaemonShutdownDialog, + }), + installAppUpdateOnQuit: async (signal) => { + const settings = await getDesktopSettingsStore().get(); + return installAppUpdateOnQuit({ + currentVersion: app.getVersion(), + releaseChannel: settings.releaseChannel, + signal, + }); + }, + createUpdateDeadlineSignal: () => AbortSignal.timeout(UPDATE_QUIT_DEADLINE_MS), + onStopError: (error) => { + log.error("[desktop daemon] failed to stop managed daemon on quit", error); + }, + onUpdateError: (error) => { + log.error("[auto-updater] failed to validate downloaded update on quit", error); + }, +}); + +// electron-updater forwards this event through Electron's built-in autoUpdater. +electronAutoUpdater.on("before-quit-for-update", quitLifecycle.handleBeforeQuitForUpdate); +app.on("before-quit", quitLifecycle.handleBeforeQuit); app.on("window-all-closed", () => { if (process.platform !== "darwin") { From bce2c50b9e16831ab395e1ea313698ba4dfa0b22 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Jul 2026 10:07:50 +0200 Subject: [PATCH 2/4] Keep terminal sizing reliable through focus changes (#2154) * fix(terminal): retry size claims until sent A focus resize could consume its once-per-focus latch before the renderer handed the size to the client. Keep the claim pending across visibility, connection, and renderer readiness changes, and only commit it after the resize is sent. * fix(terminal): keep disconnected claims retryable A retained runtime client can outlive its active connection. Require the connection at delivery time so a delayed resize callback cannot commit a claim that was never sent. * test(terminal): await PTY resize observation Prove the terminal starts at 80x24 while blurred, then probe through the daemon until its own reported size matches xterm after focus returns. This avoids racing a one-shot stty sample against the asynchronous resize claim. --- packages/app/e2e/terminal-stuck-size.spec.ts | 210 +++--------------- .../terminal-pane-focus-claim.test.ts | 134 +++++++++++ .../components/terminal-pane-focus-claim.ts | 73 ++++++ .../terminal-pane-focus-latch.test.ts | 92 -------- .../components/terminal-pane-focus-latch.ts | 33 --- packages/app/src/components/terminal-pane.tsx | 98 +++++--- .../terminal-emulator-runtime.browser.test.ts | 15 +- .../terminal-mount-claim.browser.test.ts | 159 ------------- 8 files changed, 320 insertions(+), 494 deletions(-) create mode 100644 packages/app/src/components/terminal-pane-focus-claim.test.ts create mode 100644 packages/app/src/components/terminal-pane-focus-claim.ts delete mode 100644 packages/app/src/components/terminal-pane-focus-latch.test.ts delete mode 100644 packages/app/src/components/terminal-pane-focus-latch.ts delete mode 100644 packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts diff --git a/packages/app/e2e/terminal-stuck-size.spec.ts b/packages/app/e2e/terminal-stuck-size.spec.ts index ebdfd0110..9c498e44a 100644 --- a/packages/app/e2e/terminal-stuck-size.spec.ts +++ b/packages/app/e2e/terminal-stuck-size.spec.ts @@ -1,5 +1,6 @@ import { test, expect, type Page } from "./fixtures"; import { TerminalE2EHarness } from "./helpers/terminal-dsl"; +import { getTerminalBufferText } from "./helpers/terminal-perf"; import { buildHostWorkspaceRoute } from "../src/utils/host-routes"; import { getServerId } from "./helpers/server-id"; @@ -8,18 +9,14 @@ import { getServerId } from "./helpers/server-id"; * once focus returns. * * The PTY is only ever resized by an explicit client claim (`terminal_input` resize). A freshly - * mounted terminal produces exactly one claim, from terminal-pane's pane-focus reflow effect. If - * that claim is emitted while the app is not actively visible (window blurred / document hidden), - * `handleTerminalResize` drops it — and nothing used to re-send it, leaving the PTY at 80x24 - * while xterm renders the real pane size: vim/top squeezed into a corner until the user resizes - * the window. + * mounted terminal starts its claim from terminal-pane's pane-focus reflow effect. Previously, if + * that claim was emitted while the app was not actively visible, `handleTerminalResize` dropped + * it without a retry, leaving the PTY at 80x24 while xterm rendered the real pane size. * * The repro is the mundane user flow: with a workspace already showing a terminal, open another * one and switch to a different app while it spawns. We stage the blur deterministically by * stubbing `document.hasFocus()` (which `useAppVisible` consults on window focus/blur events) - * instead of racing real OS focus. The first terminal matters: `useAppVisible` only observes - * focus events while a consumer is mounted, so it is what makes the blur visible to the app — - * exactly as in the real flow. + * instead of racing real OS focus. * * The assertion reads the PTY's own opinion of its size (`stty size`) with input written * daemon-side — clicking or typing in the pane would fire the focus-claim path and mask the bug. @@ -58,31 +55,6 @@ async function readRenderedTerminalSize(page: Page): Promise { - return await page.evaluate(() => { - const terminal = ( - window as Window & { - __paseoTerminal?: { - buffer: { - active: { - length: number; - getLine(index: number): { translateToString(trim: boolean): string } | undefined; - }; - }; - }; - } - ).__paseoTerminal; - if (!terminal) { - return ""; - } - const lines: string[] = []; - for (let index = 0; index < terminal.buffer.active.length; index += 1) { - lines.push(terminal.buffer.active.getLine(index)?.translateToString(true) ?? ""); - } - return lines.join("\n"); - }); -} - async function createTerminalViaMenu(page: Page): Promise { await page.getByTestId("workspace-new-tab-menu-trigger").click(); await page.getByTestId("workspace-new-tab-menu-terminal").click(); @@ -112,116 +84,10 @@ function requireTerminalSize(size: RenderedTerminalSize | null): RenderedTermina return size; } -function claimsFor(frames: ResizeClaim[], terminalId: string): ResizeClaim[] { - return frames.filter((frame) => frame.terminalId === terminalId); -} - -function parseSttySize(bufferText: string): RenderedTerminalSize { - const match = /S=(\d+) (\d+)=/.exec(bufferText); - if (!match?.[1] || !match[2]) { - throw new Error(`stty size did not print in the terminal buffer:\n${bufferText}`); - } - return { rows: Number(match[1]), cols: Number(match[2]) }; -} - -interface ResizeClaim { - terminalId: string; - rows: number; - cols: number; -} - -/** - * Records every resize claim the app puts on the wire. Claims travel two ways: as a JSON - * `terminal_input` before the stream has a binary slot, and as a binary frame - * ([opcode 0x03][slot][JSON {rows, cols}]) once subscribed. The slot -> terminal mapping comes - * from subscribe_terminal_response on the receive side. - */ -function captureResizeClaims(page: Page): ResizeClaim[] { - const resizeFrames: ResizeClaim[] = []; - const slotTerminals = new Map(); - const unmappedBinaryResizes: Array<{ slot: number; rows: number; cols: number }> = []; - - const recordBinaryResize = (slot: number, size: { rows: number; cols: number }) => { - const terminalId = slotTerminals.get(slot); - if (terminalId) { - resizeFrames.push({ terminalId, ...size }); - } else { - unmappedBinaryResizes.push({ slot, ...size }); - } - }; - - const handleSentFrame = (frame: { payload: string | Buffer }) => { - const payload = frame.payload; - if (typeof payload !== "string") { - if (payload.length >= 2 && payload[0] === 0x03) { - try { - const parsed = JSON.parse(payload.subarray(2).toString("utf8")) as { - rows?: number; - cols?: number; - }; - if (parsed.rows !== undefined && parsed.cols !== undefined) { - recordBinaryResize(payload[1] ?? -1, { rows: parsed.rows, cols: parsed.cols }); - } - } catch { - // not a terminal resize frame — ignore - } - } - return; - } - if (!payload.includes("resize")) { - return; - } - try { - const outer = JSON.parse(payload) as { message?: Record }; - const message = outer.message ?? {}; - if (message.type !== "terminal_input") { - return; - } - const inner = message.message as { type?: string; rows?: number; cols?: number }; - if (inner?.type === "resize" && inner.rows !== undefined && inner.cols !== undefined) { - resizeFrames.push({ - terminalId: String(message.terminalId ?? ""), - rows: inner.rows, - cols: inner.cols, - }); - } - } catch { - // not JSON — ignore - } - }; - - const handleReceivedFrame = (frame: { payload: string | Buffer }) => { - const payload = frame.payload; - if (typeof payload !== "string" || !payload.includes("subscribe_terminal_response")) { - return; - } - try { - const outer = JSON.parse(payload) as { message?: Record }; - const message = outer.message ?? {}; - if (message.type !== "subscribe_terminal_response") { - return; - } - const responsePayload = (message.payload ?? {}) as { terminalId?: string; slot?: number }; - if ( - typeof responsePayload.terminalId === "string" && - typeof responsePayload.slot === "number" - ) { - slotTerminals.set(responsePayload.slot, responsePayload.terminalId); - for (const entry of unmappedBinaryResizes.splice(0)) { - recordBinaryResize(entry.slot, { rows: entry.rows, cols: entry.cols }); - } - } - } catch { - // not JSON — ignore - } - }; - - page.on("websocket", (ws) => { - ws.on("framesent", handleSentFrame); - ws.on("framereceived", handleReceivedFrame); - }); - - return resizeFrames; +function parseLatestSttySize(bufferText: string): RenderedTerminalSize | null { + const matches = [...bufferText.matchAll(/S\d+=(\d+) (\d+)=/g)]; + const match = matches.at(-1); + return match?.[1] && match[2] ? { rows: Number(match[1]), cols: Number(match[2]) } : null; } test.describe("terminal PTY size claim under lost window focus", () => { @@ -239,16 +105,14 @@ test.describe("terminal PTY size claim under lost window focus", () => { page, }) => { await page.setViewportSize({ width: 1280, height: 900 }); - const resizeFrames = captureResizeClaims(page); await page.goto(buildHostWorkspaceRoute(getServerId(), harness.workspaceId)); await expect(page.getByTestId("workspace-new-tab-menu-trigger")).toBeVisible({ timeout: 30_000, }); - // A terminal is already open — this also keeps useAppVisible's listeners mounted so the - // upcoming blur is actually observed by the app. Wait for the daemon to know about it rather - // than sleeping: if it were missing from this snapshot it would be misread as "new" below. + // Wait for the daemon to know about the first terminal rather than sleeping: if it were + // missing from this snapshot it would be misread as "new" below. await createTerminalViaMenu(page); await expect(harness.terminalSurface(page).first()).toBeVisible({ timeout: 30_000 }); const countTerminals = async () => (await listTerminalIds(harness)).length; @@ -265,14 +129,19 @@ test.describe("terminal PTY size claim under lost window focus", () => { await expect.poll(async () => (await findNewTerminalIds()).length, { timeout: 15_000 }).toBe(1); const newTerminalId = exactlyOne(await findNewTerminalIds(), "new terminal"); - // Let every mount-time refit run (the ladder goes out to 2s) while the window stays blurred. - // This one has to be a wait, not a poll: the assertion is that nothing happens. + // Keep the app blurred through the complete mount-time refit ladder, which runs out to 2s. await page.waitForTimeout(2_500); - // Nothing may reach the daemon while the app is hidden — handleTerminalResize drops claims - // that fire while !isAppVisible. Without this, deleting that gate would still leave the test - // green: the claim would simply arrive early instead of after focus returns. - expect(claimsFor(resizeFrames, newTerminalId)).toEqual([]); + await harness.client.subscribeTerminal(newTerminalId); + + // Confirm the PTY is still at its spawn size before focus returns. + harness.client.sendTerminalInput(newTerminalId, { + type: "input", + data: 'echo "S0=$(stty size)="\n', + }); + await expect + .poll(async () => getTerminalBufferText(page), { timeout: 15_000 }) + .toMatch(/S0=24 80=/); // ...and comes back. await setWindowFocused(page, true); @@ -282,29 +151,20 @@ test.describe("terminal PTY size claim under lost window focus", () => { // Sanity: the pane really rendered at a desktop size, not the PTY default. expect(rendered.cols).toBeGreaterThan(80); - // The claim must reach the wire once focus is back. Poll for it rather than sleeping: this - // is the behavior under test, so waiting a fixed duration would trade a real assertion for - // a timing bet. - const claimsForNewTerminal = () => claimsFor(resizeFrames, newTerminalId); - await expect - .poll(claimsForNewTerminal, { timeout: 15_000 }) - .toContainEqual({ terminalId: newTerminalId, rows: rendered.rows, cols: rendered.cols }); - - // And the PTY itself must agree. Ask it via the daemon, never via the page: focusing or + // The PTY itself must agree. Ask it via the daemon, never via the page: focusing or // typing in the pane triggers the focus-claim path and would mask the bug. - await harness.client.subscribeTerminal(newTerminalId); - harness.client.sendTerminalInput(newTerminalId, { - type: "input", - data: 'echo "S=$(stty size)="\n', - }); - + let probe = 1; await expect - .poll(async () => readTerminalBufferText(page), { timeout: 15_000 }) - .toMatch(/S=\d+ \d+=/); - - const ptySize = parseSttySize(await readTerminalBufferText(page)); - - // The PTY must match what xterm rendered — not the daemon's 80x24 spawn default. - expect(ptySize).toEqual({ rows: rendered.rows, cols: rendered.cols }); + .poll( + async () => { + harness.client.sendTerminalInput(newTerminalId, { + type: "input", + data: `echo "S${probe++}=$(stty size)="\n`, + }); + return parseLatestSttySize(await getTerminalBufferText(page)); + }, + { timeout: 15_000, intervals: [100, 250, 500] }, + ) + .toEqual({ rows: rendered.rows, cols: rendered.cols }); }); }); diff --git a/packages/app/src/components/terminal-pane-focus-claim.test.ts b/packages/app/src/components/terminal-pane-focus-claim.test.ts new file mode 100644 index 000000000..67e540f52 --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-claim.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; +import { + EMPTY_FOCUS_CLAIM_STATE, + canRequestFocusClaim, + reconcileFocusClaim, + settleFocusClaim, + type FocusClaimState, +} from "./terminal-pane-focus-claim"; + +function request( + state: FocusClaimState, + input: { key: string | null; canRequest: boolean }, +): FocusClaimState { + return reconcileFocusClaim(state, input).state; +} + +describe("terminal pane focus claim", () => { + it("waits for both the client and renderer before requesting a claim", () => { + const withoutClient = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: false, + isConnected: true, + isRendererReady: true, + }); + const withoutRenderer = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: true, + isConnected: true, + isRendererReady: false, + }); + + expect([withoutClient, withoutRenderer]).toEqual([false, false]); + }); + + it("does not deliver a requested claim after the host disconnects", () => { + const disconnected = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: true, + isConnected: false, + isRendererReady: true, + }); + + expect(disconnected).toBe(false); + }); + + it("claims once per continuous pane-focus period after send", () => { + const firstRequest = reconcileFocusClaim(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const sent = settleFocusClaim(firstRequest.state, { + key: "ws:term-1", + sent: true, + }); + const repeated = reconcileFocusClaim(sent, { + key: "ws:term-1", + canRequest: true, + }); + + expect(firstRequest.shouldRequest).toBe(true); + expect(repeated).toEqual({ + state: { claimedKey: "ws:term-1", requestedKey: null }, + shouldRequest: false, + }); + }); + + it("defers until the claim can be requested", () => { + const unavailable = reconcileFocusClaim(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: false, + }); + const available = reconcileFocusClaim(unavailable.state, { + key: "ws:term-1", + canRequest: true, + }); + + expect(unavailable.shouldRequest).toBe(false); + expect(available.shouldRequest).toBe(true); + }); + + it("retries when readiness changes before the requested claim lands", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const hiddenBeforeDelivery = request(requested, { + key: "ws:term-1", + canRequest: false, + }); + const visibleAgain = reconcileFocusClaim(hiddenBeforeDelivery, { + key: "ws:term-1", + canRequest: true, + }); + + expect(visibleAgain).toEqual({ + state: { claimedKey: null, requestedKey: "ws:term-1" }, + shouldRequest: true, + }); + }); + + it("retries a requested claim that was not sent", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const dropped = settleFocusClaim(requested, { + key: "ws:term-1", + sent: false, + }); + const retry = reconcileFocusClaim(dropped, { + key: "ws:term-1", + canRequest: true, + }); + + expect(retry.shouldRequest).toBe(true); + }); + + it("re-arms after pane blur or terminal change", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const sent = settleFocusClaim(requested, { key: "ws:term-1", sent: true }); + const blurred = request(sent, { key: null, canRequest: true }); + const refocused = reconcileFocusClaim(blurred, { key: "ws:term-1", canRequest: true }); + const changed = reconcileFocusClaim(sent, { key: "ws:term-2", canRequest: true }); + + expect(refocused.shouldRequest).toBe(true); + expect(changed.shouldRequest).toBe(true); + }); +}); diff --git a/packages/app/src/components/terminal-pane-focus-claim.ts b/packages/app/src/components/terminal-pane-focus-claim.ts new file mode 100644 index 000000000..03f9ec2fa --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-claim.ts @@ -0,0 +1,73 @@ +export interface FocusClaimState { + claimedKey: string | null; + requestedKey: string | null; +} + +export interface FocusClaimStep { + state: FocusClaimState; + shouldRequest: boolean; +} + +interface FocusClaimReadiness { + isWorkspaceFocused: boolean; + isAppVisible: boolean; + isClientReady: boolean; + isConnected: boolean; + isRendererReady: boolean; +} + +export const EMPTY_FOCUS_CLAIM_STATE: FocusClaimState = { + claimedKey: null, + requestedKey: null, +}; + +export function canRequestFocusClaim(input: FocusClaimReadiness): boolean { + return ( + input.isWorkspaceFocused && + input.isAppVisible && + input.isClientReady && + input.isConnected && + input.isRendererReady + ); +} + +export function reconcileFocusClaim( + state: FocusClaimState, + input: { key: string | null; canRequest: boolean }, +): FocusClaimStep { + if (input.key === null) { + return { state: EMPTY_FOCUS_CLAIM_STATE, shouldRequest: false }; + } + if (state.claimedKey === input.key) { + return { + state: { claimedKey: input.key, requestedKey: null }, + shouldRequest: false, + }; + } + if (!input.canRequest) { + return { + state: { claimedKey: state.claimedKey, requestedKey: null }, + shouldRequest: false, + }; + } + if (state.requestedKey === input.key) { + return { state, shouldRequest: false }; + } + return { + state: { claimedKey: state.claimedKey, requestedKey: input.key }, + shouldRequest: true, + }; +} + +export function settleFocusClaim( + state: FocusClaimState, + input: { key: string; sent: boolean }, +): FocusClaimState { + if (state.requestedKey !== input.key) { + return state; + } + return { + claimedKey: input.sent ? input.key : state.claimedKey, + requestedKey: null, + }; +} diff --git a/packages/app/src/components/terminal-pane-focus-latch.test.ts b/packages/app/src/components/terminal-pane-focus-latch.test.ts deleted file mode 100644 index 41dd60de2..000000000 --- a/packages/app/src/components/terminal-pane-focus-latch.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { resolveFocusLatchStep, type FocusLatchStep } from "./terminal-pane-focus-latch"; - -/** - * Pins the ordering that caused "terminal stuck at 80x24": the latch must record that the action - * RAN, not that the effect ran. A pane that mounts before its workspace has focus (or while the - * app is hidden) must still fire once readiness arrives — burning the latch on the attempt - * permanently disarms the only resize claim a fresh terminal ever sends. - */ - -function run(steps: Array<{ key: string | null; canFire: boolean }>): FocusLatchStep[] { - const results: FocusLatchStep[] = []; - let latchedKey: string | null = null; - for (const step of steps) { - const result = resolveFocusLatchStep({ ...step, latchedKey }); - latchedKey = result.latchedKey; - results.push(result); - } - return results; -} - -describe("resolveFocusLatchStep", () => { - it("fires immediately when the pane is focused and ready at mount", () => { - const [step] = run([{ key: "ws:term-1", canFire: true }]); - expect(step).toEqual({ latchedKey: "ws:term-1", fire: true }); - }); - - it("defers instead of burning the latch when readiness arrives late", () => { - // The bug: pane focused while the workspace is not (or the app is hidden). The old code - // latched on the first pass and never fired again. - const steps = run([ - { key: "ws:term-1", canFire: false }, // mount: pane focused, workspace not focused yet - { key: "ws:term-1", canFire: true }, // workspace focus / visibility arrives - ]); - expect(steps[0]).toEqual({ latchedKey: null, fire: false }); - expect(steps[1]).toEqual({ latchedKey: "ws:term-1", fire: true }); - }); - - it("fires exactly once per continuous pane-focus period", () => { - const steps = run([ - { key: "ws:term-1", canFire: true }, - { key: "ws:term-1", canFire: true }, // unrelated re-render - { key: "ws:term-1", canFire: false }, // workspace blurs... - { key: "ws:term-1", canFire: true }, // ...and refocuses: not a new pane-focus period - ]); - expect(steps).toEqual([ - { latchedKey: "ws:term-1", fire: true }, - { latchedKey: "ws:term-1", fire: false }, - { latchedKey: "ws:term-1", fire: false }, - { latchedKey: "ws:term-1", fire: false }, - ]); - }); - - it("re-arms when the pane blurs and fires again on refocus", () => { - const steps = run([ - { key: "ws:term-1", canFire: true }, - { key: null, canFire: true }, // pane blurred / terminal gone - { key: "ws:term-1", canFire: true }, - ]); - expect(steps).toEqual([ - { latchedKey: "ws:term-1", fire: true }, - { latchedKey: null, fire: false }, - { latchedKey: "ws:term-1", fire: true }, - ]); - }); - - it("fires for a different terminal in the same pane", () => { - const steps = run([ - { key: "ws:term-1", canFire: true }, - { key: "ws:term-2", canFire: true }, - ]); - expect(steps).toEqual([ - { latchedKey: "ws:term-1", fire: true }, - { latchedKey: "ws:term-2", fire: true }, - ]); - }); - - it("stays deferred across repeated not-ready passes, then fires once", () => { - const steps = run([ - { key: "ws:term-1", canFire: false }, - { key: "ws:term-1", canFire: false }, - { key: "ws:term-1", canFire: true }, - { key: "ws:term-1", canFire: true }, - ]); - expect(steps).toEqual([ - { latchedKey: null, fire: false }, - { latchedKey: null, fire: false }, - { latchedKey: "ws:term-1", fire: true }, - { latchedKey: "ws:term-1", fire: false }, - ]); - }); -}); diff --git a/packages/app/src/components/terminal-pane-focus-latch.ts b/packages/app/src/components/terminal-pane-focus-latch.ts deleted file mode 100644 index 9d3d6196b..000000000 --- a/packages/app/src/components/terminal-pane-focus-latch.ts +++ /dev/null @@ -1,33 +0,0 @@ -/** - * Once-per-pane-focus latch for terminal-pane's focus/reflow effects. - * - * Each effect wants to fire exactly once per continuous pane-focus period per terminal, but only - * while the action can actually land (workspace focused, and for resize claims the app visible — - * `handleTerminalResize` drops claims emitted while hidden and nothing re-sends them). The latch - * must therefore record "the action ran", never "we tried": latching before the readiness gate - * permanently disarms the effect when a pane mounts before its workspace has focus, which is how - * a terminal ends up rendering full-size while its PTY stays at the daemon's 80x24 default. - */ -export interface FocusLatchStep { - latchedKey: string | null; - fire: boolean; -} - -export function resolveFocusLatchStep(input: { - /** Identity of the pane-focus period; null resets the latch (pane blurred, no terminal). */ - key: string | null; - latchedKey: string | null; - /** Whether the action can land right now; while false the latch stays untouched. */ - canFire: boolean; -}): FocusLatchStep { - if (input.key === null) { - return { latchedKey: null, fire: false }; - } - if (input.latchedKey === input.key) { - return { latchedKey: input.latchedKey, fire: false }; - } - if (!input.canFire) { - return { latchedKey: input.latchedKey, fire: false }; - } - return { latchedKey: input.key, fire: true }; -} diff --git a/packages/app/src/components/terminal-pane.tsx b/packages/app/src/components/terminal-pane.tsx index 52955c206..3eb607857 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -21,7 +21,12 @@ import { resolvePendingModifierDataInput, } from "@/utils/terminal-keys"; import { getWorkspaceTerminalSession } from "@/terminal/runtime/workspace-terminal-session"; -import { resolveFocusLatchStep } from "./terminal-pane-focus-latch"; +import { + EMPTY_FOCUS_CLAIM_STATE, + canRequestFocusClaim, + reconcileFocusClaim, + settleFocusClaim, +} from "./terminal-pane-focus-claim"; import { TerminalStreamController, type TerminalStreamControllerStatus, @@ -222,7 +227,7 @@ export function TerminalPane({ const pendingTerminalInputRef = useRef([]); const keyboardRefitTimeoutsRef = useRef>>([]); const lastAutoFocusKeyRef = useRef(null); - const lastPaneFocusResizeKeyRef = useRef(null); + const paneFocusResizeClaimRef = useRef(EMPTY_FOCUS_CLAIM_STATE); const initialSnapshot = workspaceTerminalSession.snapshots.get({ terminalId }); useEffect(() => { @@ -261,40 +266,48 @@ export function TerminalPane({ ); useEffect(() => { - // Latch only when the focus actually ran: latching before the workspace-focus gate would - // permanently disarm this effect for panes that mount before their workspace has focus. - const step = resolveFocusLatchStep({ - key: isMobile || !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, - latchedKey: lastAutoFocusKeyRef.current, - canFire: isWorkspaceFocused, - }); - lastAutoFocusKeyRef.current = step.latchedKey; - if (step.fire) { + if (isMobile || !isPaneFocused || !terminalId) { + lastAutoFocusKeyRef.current = null; + return; + } + if (!isWorkspaceFocused) { + return; + } + const focusKey = `${scopeKey}:${terminalId}`; + if (lastAutoFocusKeyRef.current !== focusKey) { + lastAutoFocusKeyRef.current = focusKey; requestTerminalFocus(); } }, [isMobile, isPaneFocused, isWorkspaceFocused, requestTerminalFocus, scopeKey, terminalId]); useEffect(() => { - // This reflow produces the one resize claim a freshly mounted terminal ever sends; defer it - // (without burning the latch) until the claim can land — handleTerminalResize drops claims - // while the workspace is unfocused or the app is hidden, and nothing re-sends a dropped one. - const step = resolveFocusLatchStep({ - key: !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, - latchedKey: lastPaneFocusResizeKeyRef.current, - canFire: isWorkspaceFocused && isAppVisible, + const canRequest = canRequestFocusClaim({ + isWorkspaceFocused, + isAppVisible, + isClientReady: client !== null, + isConnected, + isRendererReady: rendererReadyStreamKey === terminalStreamKey, }); - lastPaneFocusResizeKeyRef.current = step.latchedKey; - if (step.fire) { + const step = reconcileFocusClaim(paneFocusResizeClaimRef.current, { + key: !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, + canRequest, + }); + paneFocusResizeClaimRef.current = step.state; + if (step.shouldRequest) { lastSentTerminalSizeRef.current = null; requestTerminalReflow(); } }, [ + client, isAppVisible, + isConnected, isPaneFocused, isWorkspaceFocused, + rendererReadyStreamKey, requestTerminalReflow, scopeKey, terminalId, + terminalStreamKey, ]); const handleTerminalFocus = useCallback(() => { @@ -635,23 +648,40 @@ export function TerminalPane({ const normalizedCols = Math.floor(cols); const nextSize = { rows: normalizedRows, cols: normalizedCols }; measuredTerminalSizeRef.current = nextSize; - if (!input.shouldClaim || !client || !terminalId || !isWorkspaceFocused || !isAppVisible) { + if (!input.shouldClaim) { return; } - const previousSent = lastSentTerminalSizeRef.current; - if ( - previousSent && - previousSent.rows === normalizedRows && - previousSent.cols === normalizedCols - ) { - return; - } - lastSentTerminalSizeRef.current = nextSize; - client.sendTerminalInput(terminalId, { - type: "resize", - rows: normalizedRows, - cols: normalizedCols, + let sent = false; + const canSend = canRequestFocusClaim({ + isWorkspaceFocused, + isAppVisible, + isClientReady: client !== null, + isConnected, + isRendererReady: true, }); + if (client && terminalId && canSend) { + const previousSent = lastSentTerminalSizeRef.current; + if ( + !previousSent || + previousSent.rows !== normalizedRows || + previousSent.cols !== normalizedCols + ) { + lastSentTerminalSizeRef.current = nextSize; + client.sendTerminalInput(terminalId, { + type: "resize", + rows: normalizedRows, + cols: normalizedCols, + }); + } + sent = true; + } + const requestedKey = paneFocusResizeClaimRef.current.requestedKey; + if (requestedKey) { + paneFocusResizeClaimRef.current = settleFocusClaim(paneFocusResizeClaimRef.current, { + key: requestedKey, + sent, + }); + } }, ); diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts index baa6ae350..1c191b1b1 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts @@ -66,6 +66,10 @@ async function waitFor(input: { predicate: () => boolean; timeoutMs?: number }): } } +function settleMountRefits(): Promise { + return new Promise((resolve) => setTimeout(resolve, 2_600)); +} + function createTerminalHost(input: { width: number; height: number; @@ -204,8 +208,17 @@ describe("terminal emulator runtime in a real browser", () => { const mounted = createTerminalHost({ width: 720, height: 360 }); await waitFor({ predicate: () => mounted.sizes.length > 0 }); + await settleMountRefits(); - expect(mounted.sizes[0]?.shouldClaim).toBe(false); + expect(mounted.sizes.length).toBeGreaterThan(1); + expect(mounted.sizes.filter((size) => size.shouldClaim)).toEqual([]); + + const settledSize = latestSize(mounted.sizes); + mounted.runtime.resize({ force: true, shouldClaim: true }); + + expect(mounted.sizes.filter((size) => size.shouldClaim)).toEqual([ + { ...settledSize, shouldClaim: true }, + ]); }); it("reports a larger PTY size when the terminal container grows", async () => { diff --git a/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts b/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts deleted file mode 100644 index ff3d36649..000000000 --- a/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { page } from "@vitest/browser/context"; -import { afterEach, describe, expect, it } from "vitest"; -import { TerminalEmulatorRuntime } from "./terminal-emulator-runtime"; - -/** - * Pins the mechanism behind "terminal stuck at 80x24". - * - * The daemon spawns a PTY at 80x24 and only resizes it when the client sends a resize frame. - * `handleTerminalResize` in terminal-pane only forwards a frame when the emitted fit carries - * `shouldClaim: true`. - * - * These tests show that a freshly mounted terminal NEVER emits such a fit. Every mount-time - * refit (the mount fit, the retry ladder, fonts.ready, the WebGL swap, visibility restore) is - * emitted with `shouldClaim: false`. The ResizeObserver's first delivery IS `shouldClaim: true`, - * but it is emitted without `force`, so the runtime's own dedupe drops it: the size has not - * changed since the mount fit. - * - * So after a fresh mount, in a pane whose size never changes, the client sends the daemon - * NOTHING. The PTY keeps whatever size it was created with. The terminal's size then depends - * entirely on either the size carried at subscribe, or an explicit forced reflow - * (`requestTerminalReflow` -> `runtime.resize({force, shouldClaim})`) -- which is exactly the - * path terminal-pane's focus effects can permanently latch off. - */ - -interface EmittedSize { - rows: number; - cols: number; - shouldClaim: boolean; -} - -interface MountedTerminal { - root: HTMLDivElement; - runtime: TerminalEmulatorRuntime; - sizes: EmittedSize[]; -} - -const mounted: MountedTerminal[] = []; - -afterEach(() => { - for (const entry of mounted.splice(0)) { - entry.runtime.unmount(); - entry.root.remove(); - } -}); - -function mountTerminal(input: { width: number; height: number }): MountedTerminal { - const root = document.createElement("div"); - root.style.cssText = `width:${input.width}px;height:${input.height}px;position:fixed;left:0;top:0;overflow:hidden`; - const host = document.createElement("div"); - host.style.cssText = "width:100%;height:100%"; - root.appendChild(host); - document.body.appendChild(root); - - const sizes: EmittedSize[] = []; - const runtime = new TerminalEmulatorRuntime(); - runtime.setCallbacks({ - callbacks: { - onResize: (size) => { - sizes.push(size as EmittedSize); - }, - }, - }); - runtime.mount({ - root, - host, - initialSnapshot: null, - scrollback: 1_000, - theme: { background: "#0b0b0b", foreground: "#e6e6e6", cursor: "#e6e6e6" }, - }); - - const entry = { root, runtime, sizes }; - mounted.push(entry); - return entry; -} - -function nextFrame(): Promise { - return new Promise((resolve) => { - requestAnimationFrame(() => { - resolve(); - }); - }); -} - -async function waitFor(input: { predicate: () => boolean; timeoutMs?: number }): Promise { - const startedAt = performance.now(); - const timeoutMs = input.timeoutMs ?? 2_000; - - while (!input.predicate()) { - if (performance.now() - startedAt > timeoutMs) { - throw new Error("Timed out waiting for terminal browser condition"); - } - await nextFrame(); - } -} - -/** - * Outlast the runtime's whole refit ladder (0/16/48/120/250/500/1000/2000ms) plus fonts and the - * WebGL swap, so nothing is still in flight when we assert. Unlike every other wait in this file - * this one cannot be a predicate: the point is to prove that no claim EVER arrives, and there is - * no event that says "the mount is done emitting". - */ -function settleMountRefits(): Promise { - return new Promise((resolve) => setTimeout(resolve, 2_600)); -} - -describe("a freshly mounted terminal never claims its size", () => { - it("emits many fits on mount, and not one of them claims the PTY size", async () => { - await page.viewport(1400, 900); - const terminal = mountTerminal({ width: 900, height: 600 }); - - await settleMountRefits(); - - // It really did refit repeatedly while mounting... - expect(terminal.sizes.length).toBeGreaterThan(1); - // ...and it measured a real viewport, not the daemon's default. - expect(terminal.sizes.at(-1)?.cols).not.toBe(80); - - // ...but every single one of those fits declined to own the PTY size, so terminal-pane - // forwards NOTHING to the daemon. This is why a terminal can sit at 80x24 while its xterm - // renders perfectly at the real size. - const claims = terminal.sizes.filter((size) => size.shouldClaim); - expect(claims).toEqual([]); - }); - - it("only an explicit forced reflow claims — the path the focus latch can kill", async () => { - await page.viewport(1400, 900); - const terminal = mountTerminal({ width: 900, height: 600 }); - await settleMountRefits(); - expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([]); - const settledSize = terminal.sizes.at(-1); - - // This is what requestTerminalReflow does. It is the ONLY thing that makes a - // stable-sized, freshly mounted terminal tell the daemon how big it is. - terminal.runtime.resize({ force: true, shouldClaim: true }); - - // It claims the size the terminal actually settled at, so the PTY ends up matching what the - // user sees rendered. - expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([ - { rows: settledSize?.rows, cols: settledSize?.cols, shouldClaim: true }, - ]); - }); - - it("a genuine size change does claim — which is why resizing the window unsticks it", async () => { - await page.viewport(1400, 900); - const terminal = mountTerminal({ width: 900, height: 600 }); - await settleMountRefits(); - expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([]); - - // The ResizeObserver fit IS shouldClaim:true; it is only suppressed while the size is - // unchanged. Change the box and it gets through -- the user-visible "resize the window and - // it fixes itself" behaviour. - terminal.root.style.width = "1300px"; - - await waitFor({ predicate: () => terminal.sizes.some((size) => size.shouldClaim) }); - const claims = terminal.sizes.filter((size) => size.shouldClaim); - // The claim carries the NEW width, not the size it mounted at. - expect(claims.at(-1)?.cols).toBeGreaterThan(terminal.sizes[0]?.cols ?? 0); - }); -}); From 557fc42c890b8badcb60249fd0b30a2396f2b112 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Jul 2026 11:17:21 +0200 Subject: [PATCH 3/4] feat(server): include daemon version in logs (#2155) Attach the running release to every entry so log history can show when daemon upgrades took effect. --- packages/server/src/server/logger.test.ts | 19 +++++++++++++++++++ packages/server/src/server/logger.ts | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/server/src/server/logger.test.ts b/packages/server/src/server/logger.test.ts index 04a54b3ee..f7f42a7ef 100644 --- a/packages/server/src/server/logger.test.ts +++ b/packages/server/src/server/logger.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { spawn } from "node:child_process"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; +import { resolveDaemonVersion } from "./daemon-version.js"; import { resolveLogConfig } from "./logger.js"; import { loadConfig } from "./config.js"; import type { PersistedConfig } from "./persisted-config.js"; @@ -153,6 +154,24 @@ describe("loadConfig logger config", () => { }); describe("createRootLogger", () => { + it("includes the daemon version in child logger entries", async () => { + const paseoHome = await mkdtemp(path.join(tmpdir(), "paseo-logger-version-")); + + const { stdout } = await runLoggerFixture(` + const logger = createRootLogger(undefined, { paseoHome: ${JSON.stringify(paseoHome)} }); + logger.child({ name: "fixture" }).info("versioned child logger"); + logger.flush(); + `); + + expect(JSON.parse(stdout)).toMatchObject({ + pid: expect.any(Number), + hostname: expect.any(String), + daemonVersion: resolveDaemonVersion(), + name: "fixture", + msg: "versioned child logger", + }); + }); + it("writes JSON to stdout by default and does not initialize file logging", async () => { const paseoHome = await mkdtemp(path.join(tmpdir(), "paseo-logger-default-")); const missingLogDir = path.join(paseoHome, "logs"); diff --git a/packages/server/src/server/logger.ts b/packages/server/src/server/logger.ts index 5c1c0522b..118ce9bbe 100644 --- a/packages/server/src/server/logger.ts +++ b/packages/server/src/server/logger.ts @@ -2,6 +2,7 @@ import { mkdirSync } from "node:fs"; import path from "node:path"; import pino from "pino"; import pretty from "pino-pretty"; +import { resolveDaemonVersion } from "./daemon-version.js"; import type { PersistedConfig } from "./persisted-config.js"; import { resolvePaseoHome } from "./paseo-home.js"; @@ -180,13 +181,15 @@ export function createRootLogger( }) : pino.destination({ dest: config.file?.path ?? 1, sync: false }); - return pino( + const logger = pino( { level: config.file?.level ?? config.console.level, redact: { paths: REDACT_PATHS, remove: true }, }, stream, ); + + return logger.child({ daemonVersion: resolveDaemonVersion() }); } export function createChildLogger(parent: pino.Logger, name: string): pino.Logger { From 388f1d426c01a9b492d084c207dfd99ff2bc33af Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 17 Jul 2026 13:02:09 +0200 Subject: [PATCH 4/4] Keep agent browser tabs connected across workspace switches (#2156) * fix(desktop): keep browser tabs connected across workspaces Electron can replace a guest WebContents when a retained browser tab is reparented. Re-register each attachment and keep background actionability checks running so agent browser tools retain the tab through workspace eviction.\n\nCover the full app, daemon, Electron, and MCP path in the existing desktop CI job. * fix(desktop): launch Electron E2E reliably on Linux CI Electron must receive --no-sandbox before the app starts because the hosted runner cannot use Electron's bundled SUID sandbox helper. Forward explicit dev-runner arguments and fail readiness waits as soon as a child exits. * fix(desktop): preserve active browser on repeated registration * fix(desktop): keep browser keyboard attachment idempotent * fix(desktop): wait for E2E bridge readiness * fix(desktop): close E2E logs after output drains --- .github/workflows/ci.yml | 21 +- docs/architecture.md | 2 +- docs/testing.md | 10 + .../components/browser-webview-resident.ts | 30 +- packages/desktop/package.json | 1 + .../scripts/browser-tab-bridge.e2e.mjs | 472 ++++++++++++++++++ packages/desktop/scripts/dev-runner.mjs | 3 +- .../browser-automation/actionability.ts | 9 +- .../features/browser-keyboard/index.test.ts | 51 +- .../src/features/browser-keyboard/index.ts | 7 + .../browser-webviews/registry.test.ts | 26 + .../src/features/browser-webviews/registry.ts | 8 + 12 files changed, 601 insertions(+), 39 deletions(-) create mode 100644 packages/desktop/scripts/browser-tab-bridge.e2e.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 234374fcd..bebab954a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -160,10 +160,29 @@ jobs: - name: Run desktop tests run: npm run test --workspace=@getpaseo/desktop + - name: Build app dependencies for desktop E2E + if: matrix.os == 'ubuntu-latest' + run: npm run build:app-deps + - name: Install virtual display - if: matrix.os == 'ubuntu-latest' && steps.desktop_changes.outputs.desktop == 'true' + if: matrix.os == 'ubuntu-latest' run: sudo apt-get update && sudo apt-get install -y xvfb xauth + - name: Run real Electron browser tab bridge E2E + if: matrix.os == 'ubuntu-latest' + run: npm run test:e2e:browser-tab-bridge --workspace=@getpaseo/desktop + env: + PASEO_TAB_BRIDGE_E2E_ARTIFACT_DIR: ${{ runner.temp }}/browser-tab-bridge-e2e + + - name: Upload browser tab bridge diagnostics + uses: actions/upload-artifact@v4 + if: failure() && matrix.os == 'ubuntu-latest' + with: + name: browser-tab-bridge-e2e + path: ${{ runner.temp }}/browser-tab-bridge-e2e + if-no-files-found: ignore + retention-days: 7 + - name: Build and smoke unpacked desktop app if: matrix.os == 'ubuntu-latest' && steps.desktop_changes.outputs.desktop == 'true' run: npm run build:desktop -- --publish never --linux --x64 --dir diff --git a/docs/architecture.md b/docs/architecture.md index 8ba52a3ca..5b0d8b89a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,7 +138,7 @@ Electron wrapper for macOS, Linux, and Windows. > **Window-state v1 limitation:** only the _first_ window of a session restores and persists saved geometry (size/position/maximized). Windows opened via ⌘⇧N / second-instance / "Open in new window" open at the default size, OS-cascaded, and do not persist — this avoids every window stacking on the same restored bounds and fighting over the single window-state store. Lifting this needs per-window state keys. > -> **In-app browser profile.** Every browser guest uses one stable persistent Electron session, so cookies, authentication, cache, and site storage are shared across tabs, workspaces, and desktop windows and survive tab or app closure. Browser identity is independent of that storage partition: after `did-attach`, the renderer explicitly registers its browser id, workspace id, and guest `WebContents` id, and main accepts the registration only when that guest belongs to the calling renderer and the shared profile. Settings > General > Clear browser data is the sole profile-deletion path; it clears the shared session and reloads live guests without deleting saved tabs or URLs. +> **In-app browser profile.** Every browser guest uses one stable persistent Electron session, so cookies, authentication, cache, and site storage are shared across tabs, workspaces, and desktop windows and survive tab or app closure. Browser identity is independent of that storage partition: after every `did-attach`, the renderer explicitly registers its browser id, workspace id, and current guest `WebContents` id, and main accepts the registration only when that guest belongs to the calling renderer and the shared profile. Registration is intentionally repeated because reparenting a retained `` can replace its guest without replacing the DOM element. Settings > General > Clear browser data is the sole profile-deletion path; it clears the shared session and reloads live guests without deleting saved tabs or URLs. > > **In-app browser window opens.** Ordinary link opens, including Shift-clicked links, become Paseo workspace tabs. Script-created opens with popup features or a named window target and POST-backed opens remain secured Electron child windows in the shared browser profile, preserving `window.opener`, `postMessage`, named-window reuse, request bodies, and `window.close()` for OAuth, payment, and similar popup protocols. Unsupported URL schemes are denied before either path. > diff --git a/docs/testing.md b/docs/testing.md index 98e41d635..06a73d34b 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -124,6 +124,16 @@ PASEO_DESKTOP_SMOKE_ARTIFACT_DIR=/tmp/paseo-desktop-smoke \ npm run build:desktop -- --publish never --linux --x64 --dir ``` +### Browser tab bridge regression + +The desktop browser tab bridge E2E launches an isolated real daemon, Metro, and Electron app. It forces workspace LRU eviction to reparent the original tab and replace its guest `WebContents`, then makes one MCP call each for tab listing, snapshot, and click against that original browser id. A final MCP wait proves the real target page received the click. + +Run it locally with the same command owned by the Ubuntu leg of the existing `desktop-tests` CI check: + +```bash +npm run test:e2e:browser-tab-bridge --workspace=@getpaseo/desktop +``` + ## Test organization - Collocate tests with implementation: `thing.ts` + `thing.test.ts` diff --git a/packages/app/src/components/browser-webview-resident.ts b/packages/app/src/components/browser-webview-resident.ts index a97190018..92577f7da 100644 --- a/packages/app/src/components/browser-webview-resident.ts +++ b/packages/app/src/components/browser-webview-resident.ts @@ -54,22 +54,20 @@ function registerBrowserWhenAttached( identity: BrowserWebviewIdentity, browser: BrowserWebviewProfileHost, ): void { - webview.addEventListener( - "did-attach", - () => { - const webContentsId = webview.getWebContentsId(); - void browser - .registerAttachedBrowser({ - browserId: identity.browserId, - workspaceId: identity.workspaceId, - webContentsId, - }) - .catch((error) => { - console.error("[browser-webview] attached registration failed", error); - }); - }, - { once: true }, - ); + // Reparenting a webview can replace its guest WebContents without replacing + // this DOM element, so every attachment needs a fresh main-process registration. + webview.addEventListener("did-attach", () => { + const webContentsId = webview.getWebContentsId(); + void browser + .registerAttachedBrowser({ + browserId: identity.browserId, + workspaceId: identity.workspaceId, + webContentsId, + }) + .catch((error) => { + console.error("[browser-webview] attached registration failed", error); + }); + }); } function trimNonEmpty(value: string | null | undefined): string | null { diff --git a/packages/desktop/package.json b/packages/desktop/package.json index 85e907a67..4173fae13 100644 --- a/packages/desktop/package.json +++ b/packages/desktop/package.json @@ -20,6 +20,7 @@ "capture-harness": "./capture-harness/run.sh", "dev": "./scripts/dev.sh", "dev:win": "powershell ./scripts/dev.ps1", + "test:e2e:browser-tab-bridge": "npm run build:main && node ./scripts/browser-tab-bridge.e2e.mjs", "verify:electron-cdp": "node ./scripts/verify-electron-cdp.mjs", "test": "vitest run", "typecheck": "tsgo --noEmit -p tsconfig.json" diff --git a/packages/desktop/scripts/browser-tab-bridge.e2e.mjs b/packages/desktop/scripts/browser-tab-bridge.e2e.mjs new file mode 100644 index 000000000..139a186e6 --- /dev/null +++ b/packages/desktop/scripts/browser-tab-bridge.e2e.mjs @@ -0,0 +1,472 @@ +#!/usr/bin/env node + +import { spawn } from "node:child_process"; +import fs from "node:fs"; +import { createServer } from "node:http"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; +import process from "node:process"; +import { setTimeout as delay } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; +import { experimental_createMCPClient } from "ai"; +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { chromium } from "playwright"; + +const scriptDir = path.dirname(fileURLToPath(import.meta.url)); +const desktopDir = path.resolve(scriptDir, ".."); +const rootDir = path.resolve(desktopDir, "../.."); +const devRunner = path.join(desktopDir, "scripts", "dev-runner.mjs"); +const workspaceIds = [ + "tab-bridge-original", + "tab-bridge-evict-one", + "tab-bridge-evict-two", + "tab-bridge-evict-three", +]; +const timeoutMs = 90_000; + +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +async function reservePort() { + return await new Promise((resolve, reject) => { + const server = net.createServer(); + server.unref(); + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + server.close((error) => { + if (error) return reject(error); + if (!address || typeof address === "string") { + return reject(new Error("Failed to reserve a local port")); + } + resolve(address.port); + }); + }); + }); +} + +async function waitForPort(port, label, processInfo) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if ( + processInfo && + (processInfo.child.exitCode !== null || processInfo.child.signalCode !== null) + ) { + throw new Error( + `${label} process exited before opening its port; see ${processInfo.logPath}`, + ); + } + const connected = await new Promise((resolve) => { + const socket = net.createConnection({ host: "127.0.0.1", port }); + socket.setTimeout(500); + socket.once("connect", () => { + socket.destroy(); + resolve(true); + }); + socket.once("timeout", () => { + socket.destroy(); + resolve(false); + }); + socket.once("error", () => resolve(false)); + }); + if (connected) return; + await delay(200); + } + throw new Error(`Timed out waiting for ${label} on port ${port}`); +} + +function writeJson(filePath, value) { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, `${JSON.stringify(value, null, 2)}\n`); +} + +function seedPaseoHome(paseoHome, listen, workspaceRoot) { + const timestamp = "2026-01-01T00:00:00.000Z"; + const projects = workspaceIds.map((workspaceId, index) => { + const cwd = path.join(workspaceRoot, `workspace-${index + 1}`); + fs.mkdirSync(cwd, { recursive: true }); + return { + projectId: `project-${workspaceId}`, + rootPath: cwd, + kind: "non_git", + displayName: `Tab bridge project ${index + 1}`, + customName: null, + createdAt: timestamp, + updatedAt: timestamp, + archivedAt: null, + }; + }); + const workspaces = workspaceIds.map((workspaceId, index) => ({ + workspaceId, + projectId: projects[index].projectId, + cwd: projects[index].rootPath, + kind: "directory", + displayName: `Tab bridge workspace ${index + 1}`, + title: `Tab bridge workspace ${index + 1}`, + branch: null, + baseBranch: null, + createdAt: timestamp, + updatedAt: timestamp, + archivedAt: null, + pinnedAt: null, + })); + + writeJson(path.join(paseoHome, "config.json"), { + version: 1, + daemon: { + listen, + relay: { enabled: false }, + mcp: { enabled: true, injectIntoAgents: false }, + browserTools: { enabled: true }, + cors: { allowedOrigins: ["*"] }, + }, + }); + writeJson(path.join(paseoHome, "projects", "projects.json"), projects); + writeJson(path.join(paseoHome, "projects", "workspaces.json"), workspaces); +} + +function spawnLogged(name, command, args, options, logDir) { + const logPath = path.join(logDir, `${name}.log`); + const log = fs.createWriteStream(logPath, { flags: "a" }); + const child = spawn(command, args, { + ...options, + detached: process.platform !== "win32", + stdio: ["ignore", "pipe", "pipe"], + }); + let openStreams = 2; + const closeLogStream = () => { + openStreams -= 1; + if (openStreams === 0) log.end(); + }; + child.stdout.pipe(log, { end: false }); + child.stderr.pipe(log, { end: false }); + child.stdout.once("end", closeLogStream); + child.stderr.once("end", closeLogStream); + return { child, logPath }; +} + +function stopProcess(child) { + if (!child?.pid || child.exitCode !== null) return; + try { + if (process.platform === "win32") child.kill("SIGTERM"); + else process.kill(-child.pid, "SIGTERM"); + } catch { + // The process may exit between the liveness check and signal delivery. + } +} + +async function waitForAppPage(browser, expoPort) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + for (const context of browser.contexts()) { + for (const page of context.pages()) { + if (page.url().includes(`localhost:${expoPort}`)) return page; + } + } + await delay(250); + } + throw new Error("Timed out waiting for the real Electron app renderer"); +} + +async function waitForDesktopStatus(page) { + const deadline = Date.now() + timeoutMs; + let lastError; + while (Date.now() < deadline) { + try { + const status = await page.evaluate(async () => { + if (typeof window.paseoDesktop?.invoke !== "function") return null; + return await window.paseoDesktop.invoke("desktop_daemon_status"); + }); + if (typeof status?.serverId === "string") return status; + } catch (error) { + // Metro may replace the renderer execution context during its initial load. + lastError = error; + } + await delay(250); + } + throw new Error( + `Timed out waiting for the Electron desktop bridge${lastError ? `: ${String(lastError)}` : ""}`, + ); +} + +async function startTargetPage() { + const server = createServer((_request, response) => { + response.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + response.end(` + + Tab bridge target + + + + `); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address(); + assert(address && typeof address !== "string", "Target page did not bind a TCP port"); + return { server, url: `http://127.0.0.1:${address.port}/` }; +} + +async function closeServer(server) { + await new Promise((resolve) => server.close(resolve)); +} + +function mcpPayload(result, command) { + const payload = result.structuredContent; + assert( + payload && typeof payload === "object", + `${command} returned no structured payload: ${JSON.stringify(result)}`, + ); + assert(payload.ok === true, `${command} failed: ${JSON.stringify(payload)}`); + return payload.result; +} + +async function callBrowserTool(client, name, args = {}) { + return mcpPayload(await client.callTool({ name, args }), name); +} + +async function createCallerAgent(daemonPort) { + const transport = new StreamableHTTPClientTransport( + new URL(`http://127.0.0.1:${daemonPort}/mcp/agents`), + ); + const client = await experimental_createMCPClient({ transport }); + try { + const response = await client.callTool({ + name: "create_agent", + args: { + relationship: { kind: "detached" }, + workspace: { kind: "existing", workspaceId: workspaceIds[0] }, + title: "Browser tab bridge E2E caller", + provider: "mock/ten-second-stream", + settings: { modeId: "load-test" }, + initialPrompt: "Remain available while the browser bridge regression runs.", + background: true, + }, + }); + const result = response.structuredContent; + assert( + result && typeof result === "object", + `create_agent returned no structured payload: ${JSON.stringify(response)}`, + ); + assert(typeof result.agentId === "string", "create_agent returned no caller agent id"); + assert( + result.workspaceId === workspaceIds[0], + `MCP caller attached to unexpected workspace ${result.workspaceId}`, + ); + return result.agentId; + } finally { + await client.close(); + } +} + +async function readGuest(page, browserId) { + return await page.evaluate((id) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + if (!(webview instanceof HTMLElement) || typeof webview.getWebContentsId !== "function") { + return null; + } + return { + webContentsId: webview.getWebContentsId(), + parentId: webview.parentElement?.id ?? null, + }; + }, browserId); +} + +async function runRegression({ page, client, serverId, targetUrl }) { + const originalWorkspaceId = workspaceIds[0]; + const originalWorkspaceRow = page.getByTestId( + `sidebar-workspace-row-${serverId}:${originalWorkspaceId}`, + ); + await originalWorkspaceRow.waitFor({ state: "visible", timeout: timeoutMs }); + await originalWorkspaceRow.click(); + + const created = await callBrowserTool(client, "browser_new_tab", { url: targetUrl }); + const browserId = created.browserId; + assert(typeof browserId === "string", "browser_new_tab returned no browserId"); + + const originalDeck = page.getByTestId(`workspace-deck-entry-${serverId}:${originalWorkspaceId}`); + await originalDeck.getByTestId(`workspace-tab-browser_${browserId}`).click(); + await page.waitForFunction( + (id) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + return webview && webview.parentElement?.id !== "paseo-browser-resident-webviews"; + }, + browserId, + { timeout: timeoutMs }, + ); + const firstGuest = await readGuest(page, browserId); + assert(firstGuest, "Original browser guest was not attached to its workspace pane"); + + for (const workspaceId of workspaceIds.slice(1)) { + await page.getByTestId(`sidebar-workspace-row-${serverId}:${workspaceId}`).click(); + await page + .getByTestId(`workspace-deck-entry-${serverId}:${workspaceId}`) + .waitFor({ state: "visible" }); + } + + await page.waitForFunction( + ({ id, previousWebContentsId }) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + return ( + webview?.parentElement?.id === "paseo-browser-resident-webviews" && + typeof webview.getWebContentsId === "function" && + webview.getWebContentsId() !== previousWebContentsId + ); + }, + { id: browserId, previousWebContentsId: firstGuest.webContentsId }, + { timeout: timeoutMs }, + ); + const replacementGuest = await readGuest(page, browserId); + assert(replacementGuest, "Replacement browser guest was not parked after workspace eviction"); + + const listed = await callBrowserTool(client, "browser_list_tabs"); + assert( + listed.tabs.some((tab) => tab.browserId === browserId), + "browser_list_tabs lost the original tab after guest replacement", + ); + + const snapshot = await callBrowserTool(client, "browser_snapshot", { browserId }); + const ref = snapshot.snapshot.match(/button "Bridge target" \[ref=(@e\d+)\]/)?.[1]; + assert(ref, `browser_snapshot did not expose the target button: ${snapshot.snapshot}`); + + const clicked = await callBrowserTool(client, "browser_click", { browserId, ref }); + assert( + clicked.browserId === browserId && clicked.ref === ref, + "browser_click targeted another tab", + ); + await callBrowserTool(client, "browser_wait", { + browserId, + text: "Clicked", + timeoutMs: 5_000, + }); + + return { + browserId, + originalWebContentsId: firstGuest.webContentsId, + replacementWebContentsId: replacementGuest.webContentsId, + list: "passed", + snapshot: "passed", + click: "passed", + }; +} + +async function main() { + const artifactDir = + process.env.PASEO_TAB_BRIDGE_E2E_ARTIFACT_DIR ?? + fs.mkdtempSync(path.join(os.tmpdir(), "paseo-tab-bridge-e2e-artifacts-")); + const runtimeDir = fs.mkdtempSync(path.join(os.tmpdir(), "paseo-tab-bridge-e2e-")); + fs.mkdirSync(artifactDir, { recursive: true }); + const paseoHome = path.join(runtimeDir, "paseo-home"); + const userData = path.join(runtimeDir, "electron-user-data"); + const workspaceRoot = path.join(runtimeDir, "workspaces"); + fs.mkdirSync(paseoHome, { recursive: true }); + + const [daemonPort, expoPort, cdpPort] = await Promise.all([ + reservePort(), + reservePort(), + reservePort(), + ]); + const listen = `127.0.0.1:${daemonPort}`; + seedPaseoHome(paseoHome, listen, workspaceRoot); + const target = await startTargetPage(); + const children = []; + let browser = null; + let client = null; + + try { + const commonEnv = { + ...process.env, + PASEO_HOME: paseoHome, + PASEO_LISTEN: listen, + PASEO_DAEMON_ENDPOINT: `localhost:${daemonPort}`, + PASEO_CORS_ORIGINS: "*", + PASEO_LOCAL_SPEECH_AUTO_DOWNLOAD: "0", + PASEO_DICTATION_ENABLED: "0", + PASEO_VOICE_MODE_ENABLED: "0", + FORCE_COLOR: "0", + NO_COLOR: "1", + }; + const daemon = spawnLogged( + "daemon", + process.execPath, + ["--import", "tsx", path.join(rootDir, "packages/server/scripts/dev-runner.ts")], + { cwd: rootDir, env: { ...commonEnv, PASEO_NODE_ENV: "development" } }, + artifactDir, + ); + children.push(daemon.child); + await waitForPort(daemonPort, "daemon", daemon); + + const desktopArgs = [ + process.execPath, + devRunner, + ...(process.platform === "linux" ? ["--no-sandbox"] : []), + ]; + const desktopCommand = process.platform === "linux" ? "xvfb-run" : desktopArgs.shift(); + const desktopCommandArgs = + process.platform === "linux" + ? ["-a", "--server-args=-screen 0 1280x800x24", ...desktopArgs] + : desktopArgs; + const desktop = spawnLogged( + "desktop", + desktopCommand, + desktopCommandArgs, + { + cwd: rootDir, + env: { + ...commonEnv, + EXPO_PORT: String(expoPort), + EXPO_DEV_URL: `http://localhost:${expoPort}`, + PASEO_ELECTRON_REMOTE_DEBUGGING_PORT: String(cdpPort), + PASEO_ELECTRON_USER_DATA_DIR: userData, + PASEO_ELECTRON_FLAGS: `--remote-debugging-address=127.0.0.1 --remote-debugging-port=${cdpPort}`, + }, + }, + artifactDir, + ); + children.push(desktop.child); + await waitForPort(cdpPort, "Electron CDP", desktop); + + browser = await chromium.connectOverCDP(`http://127.0.0.1:${cdpPort}`); + const page = await waitForAppPage(browser, expoPort); + const status = await waitForDesktopStatus(page); + + const callerAgentId = await createCallerAgent(daemonPort); + const transport = new StreamableHTTPClientTransport( + new URL( + `http://127.0.0.1:${daemonPort}/mcp/agents?callerAgentId=${encodeURIComponent(callerAgentId)}`, + ), + ); + client = await experimental_createMCPClient({ transport }); + const report = await runRegression({ + page, + client, + serverId: status.serverId, + targetUrl: target.url, + }); + writeJson(path.join(artifactDir, "result.json"), report); + console.log( + `Browser tab bridge E2E passed: WebContents ${report.originalWebContentsId} -> ${report.replacementWebContentsId}; list, snapshot, click passed.`, + ); + } catch (error) { + console.error(`Browser tab bridge E2E failed. Artifacts: ${artifactDir}`); + console.error(error); + throw error; + } finally { + await client?.close().catch(() => undefined); + await browser?.close().catch(() => undefined); + for (const child of children.toReversed()) stopProcess(child); + await closeServer(target.server); + await delay(1_000); + try { + fs.rmSync(runtimeDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch (error) { + console.warn(`Failed to remove isolated E2E state ${runtimeDir}`, error); + } + } +} + +await main(); diff --git a/packages/desktop/scripts/dev-runner.mjs b/packages/desktop/scripts/dev-runner.mjs index 51ca14130..f2b16f822 100644 --- a/packages/desktop/scripts/dev-runner.mjs +++ b/packages/desktop/scripts/dev-runner.mjs @@ -19,6 +19,7 @@ if (!Number.isInteger(expoPort) || expoPort <= 0) { } const expoDevUrl = process.env.EXPO_DEV_URL || `http://localhost:${expoPort}`; +const electronArgs = process.argv.slice(2); const colorEnv = { FORCE_COLOR: process.env.FORCE_COLOR || "1", npm_config_color: process.env.npm_config_color || "always", @@ -175,7 +176,7 @@ try { } if (!stopping) { - spawnChild("electron", electron, [desktopDir], { + spawnChild("electron", electron, [...electronArgs, desktopDir], { detached: true, env: { ...process.env, diff --git a/packages/desktop/src/features/browser-automation/actionability.ts b/packages/desktop/src/features/browser-automation/actionability.ts index b5e48db76..0d2039904 100644 --- a/packages/desktop/src/features/browser-automation/actionability.ts +++ b/packages/desktop/src/features/browser-automation/actionability.ts @@ -100,8 +100,11 @@ function buildActionabilityScript(input: { const deadline = performance.now() + ${JSON.stringify(input.timeoutMs)}; const requiresEditable = ${JSON.stringify(input.editable)}; - const nextFrame = () => new Promise((resolve) => requestAnimationFrame(resolve)); const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + // Electron can suspend requestAnimationFrame while a guest is parked after + // workspace LRU eviction even though the guest remains CDP-controllable. + // Timed layout samples keep background browser automation live. + const waitForLayout = () => sleep(16); const nearlyEqual = (a, b) => Math.abs(a - b) < 0.25; const sameRect = (a, b) => nearlyEqual(a.x, b.x) && @@ -174,9 +177,9 @@ function buildActionabilityScript(input: { } element.scrollIntoView?.({ block: 'center', inline: 'center' }); - await nextFrame(); + await waitForLayout(); const firstRect = element.getBoundingClientRect(); - await nextFrame(); + await waitForLayout(); const secondRect = element.getBoundingClientRect(); if (!sameRect(firstRect, secondRect)) { detail = 'moving'; diff --git a/packages/desktop/src/features/browser-keyboard/index.test.ts b/packages/desktop/src/features/browser-keyboard/index.test.ts index 695750d21..2c792266b 100644 --- a/packages/desktop/src/features/browser-keyboard/index.test.ts +++ b/packages/desktop/src/features/browser-keyboard/index.test.ts @@ -21,11 +21,11 @@ class FakeBrowserContents { public readonly sent: SentMessage[] = []; private destroyed = false; private readonly destroyedListeners: Array<() => void> = []; - private domReadyListener: (() => void) | null = null; + private readonly domReadyListeners: Array<() => void> = []; private finishLoadListener: (() => void) | null = null; - private inputListener: - | ((event: { preventDefault(): void }, input: Electron.Input) => void) - | null = null; + private readonly inputListeners: Array< + (event: { preventDefault(): void }, input: Electron.Input) => void + > = []; public constructor(private readonly webContentsId: number) {} @@ -64,13 +64,12 @@ class FakeBrowserContents { return; } if (event === "dom-ready") { - this.domReadyListener = listener as () => void; + this.domReadyListeners.push(listener as () => void); return; } - this.inputListener = listener as ( - event: { preventDefault(): void }, - input: Electron.Input, - ) => void; + this.inputListeners.push( + listener as (event: { preventDefault(): void }, input: Electron.Input) => void, + ); } public send(channel: string, payload: unknown): void { @@ -105,19 +104,23 @@ class FakeBrowserContents { } public domReady(): void { - this.domReadyListener?.(); + for (const listener of this.domReadyListeners) { + listener(); + } } public input(input: Electron.Input): boolean { let wasPrevented = false; - this.inputListener?.( - { - preventDefault: () => { - wasPrevented = true; + for (const listener of this.inputListeners) { + listener( + { + preventDefault: () => { + wasPrevented = true; + }, }, - }, - input, - ); + input, + ); + } return wasPrevented; } } @@ -196,6 +199,20 @@ describe("BrowserKeyboard", () => { ]); }); + test("handles a reserved shortcut once when the same guest attaches again", () => { + const { attach } = createBrowserKeyboard(); + const guest = new FakeBrowserContents(53); + const host = new FakeBrowserContents(54); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); + attach({ browserId: "browser-a", contents: guest, hostContents: host }); + const command = process.platform === "darwin" ? { meta: true } : { control: true }; + + const wasPrevented = guest.input(electronInput({ ...command, code: "KeyR", key: "r" })); + + expect(wasPrevented).toBe(true); + expect(guest.reloads).toEqual(["reload"]); + }); + test("republishes the latest shortcut policy when the next guest document is ready", () => { const { attach, keyboard } = createBrowserKeyboard(); const guest = new FakeBrowserContents(61); diff --git a/packages/desktop/src/features/browser-keyboard/index.ts b/packages/desktop/src/features/browser-keyboard/index.ts index 8084ddb97..0d90f019f 100644 --- a/packages/desktop/src/features/browser-keyboard/index.ts +++ b/packages/desktop/src/features/browser-keyboard/index.ts @@ -87,6 +87,13 @@ export class BrowserKeyboard { if (!registration || registration.hostWebContentsId !== input.hostContents.id) { return; } + const attachedGuest = this.attachedGuestsByWebContentsId.get(webContentsId); + if ( + attachedGuest?.contents === input.contents && + attachedGuest.hostContents === input.hostContents + ) { + return; + } const guest: BrowserKeyboardGuest = input; this.attachedGuestsByWebContentsId.set(webContentsId, guest); diff --git a/packages/desktop/src/features/browser-webviews/registry.test.ts b/packages/desktop/src/features/browser-webviews/registry.test.ts index 93b8c353b..5ef7a6cf5 100644 --- a/packages/desktop/src/features/browser-webviews/registry.test.ts +++ b/packages/desktop/src/features/browser-webviews/registry.test.ts @@ -33,6 +33,32 @@ describe("PaseoBrowserWebviewRegistry", () => { expect(registry.getActiveBrowserIdForHostWindow(101)).toBe("browser-a"); }); + it("keeps the active browser when the same guest registers again", () => { + const registry = new PaseoBrowserWebviewRegistry(); + + registry.registerWebContents({ + webContentsId: 1, + browserId: "browser-a", + hostWebContentsId: 101, + }); + registry.setWorkspaceActiveBrowser({ + hostWebContentsId: 101, + workspaceId: "workspace-a", + browserId: "browser-a", + }); + + registry.registerWebContents({ + webContentsId: 1, + browserId: "browser-a", + hostWebContentsId: 101, + }); + + expect(registry.getActiveBrowserIdForWorkspaceInHostWindow(101, "workspace-a")).toBe( + "browser-a", + ); + expect(registry.getWebContentsIdForBrowserInHostWindow(101, "browser-a")).toBe(1); + }); + it("ignores stale destroy events after a duplicate browserId moved", () => { const registry = new PaseoBrowserWebviewRegistry(); diff --git a/packages/desktop/src/features/browser-webviews/registry.ts b/packages/desktop/src/features/browser-webviews/registry.ts index 4e49fdf6e..4fb8381f3 100644 --- a/packages/desktop/src/features/browser-webviews/registry.ts +++ b/packages/desktop/src/features/browser-webviews/registry.ts @@ -21,6 +21,14 @@ export class PaseoBrowserWebviewRegistry { }): void { const hostBrowserKey = this.hostBrowserKey(input.hostWebContentsId, input.browserId); const replacedWebContentsId = this.webContentsIdsByHostAndBrowserId.get(hostBrowserKey); + const existingRegistration = this.registrationsByWebContentsId.get(input.webContentsId); + if ( + replacedWebContentsId === input.webContentsId && + existingRegistration?.browserId === input.browserId && + existingRegistration.hostWebContentsId === input.hostWebContentsId + ) { + return; + } if (replacedWebContentsId !== undefined && replacedWebContentsId !== input.webContentsId) { this.removeWebContents(replacedWebContentsId, { preserveActiveBrowser: true }); }