diff --git a/packages/app/e2e/helpers/desktop-updates.ts b/packages/app/e2e/helpers/desktop-updates.ts index 1014270d4..c52d042c0 100644 --- a/packages/app/e2e/helpers/desktop-updates.ts +++ b/packages/app/e2e/helpers/desktop-updates.ts @@ -65,6 +65,8 @@ export interface DesktopBridgeConfig { daemonListen?: string; /** Keep start_desktop_daemon pending to hold the desktop startup blocker open. */ hangDaemonStart?: boolean; + /** Delay the desktop settings IPC response to exercise startup ordering. */ + desktopSettingsDelayMs?: number; /** * Controls what dialog.ask returns when the daemon management confirm dialog * fires. True = confirm (proceed with the action), false = cancel. Defaults to @@ -101,6 +103,7 @@ declare global { __capturedDialogCall: ConfirmDialogCall | undefined; __capturedDialogOpenCalls: Array | undefined>; __recordDesktopEditorOpen?: (input: DesktopEditorOpenRecord) => Promise; + __desktopDaemonStartRequested?: boolean; } } @@ -129,6 +132,7 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi let daemonRunning = true; let currentPid: number | null = cfg.daemonPid ?? null; let startCount = 0; + window.__desktopDaemonStartRequested = false; function buildDaemonStatus() { return { @@ -145,6 +149,7 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi } function startDesktopDaemon() { + window.__desktopDaemonStartRequested = true; if (cfg.hangDaemonStart) { return new Promise(() => undefined); } @@ -156,6 +161,13 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi return buildDaemonStatus(); } + async function waitForDesktopSettingsResponse() { + const delayMs = cfg.desktopSettingsDelayMs ?? 0; + if (delayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } + const desktopBridge: { platform: string; invoke: (command: string, args?: Record) => Promise; @@ -212,6 +224,7 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi } if (command === "get_desktop_settings") { + await waitForDesktopSettingsResponse(); return { releaseChannel: "stable", daemon: { manageBuiltInDaemon: manageDaemon, keepRunningAfterQuit: true }, @@ -277,6 +290,17 @@ export async function injectDesktopBridge(page: Page, config: DesktopBridgeConfi }, config); } +export async function waitForDesktopDaemonStartRequest(page: Page): Promise { + await page.waitForFunction(() => window.__desktopDaemonStartRequested === true); + // Give the startup state two paints to expose any app → splash regression. + await page.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())), + ), + ); +} + export async function waitForDirectoryDialog( page: Page, ): Promise | undefined> { diff --git a/packages/app/e2e/workspace-navigation-regression.spec.ts b/packages/app/e2e/workspace-navigation-regression.spec.ts index 62b18f6ee..82b3078fb 100644 --- a/packages/app/e2e/workspace-navigation-regression.spec.ts +++ b/packages/app/e2e/workspace-navigation-regression.spec.ts @@ -31,11 +31,52 @@ import { } from "./helpers/workspace-ui"; import { clickSettingsBackToWorkspace } from "./helpers/settings"; import { getServerId } from "./helpers/server-id"; -import { injectDesktopBridge } from "./helpers/desktop-updates"; +import { injectDesktopBridge, waitForDesktopDaemonStartRequest } from "./helpers/desktop-updates"; import { expectAppRoute } from "./helpers/route-assertions"; import { installDaemonWebSocketGate } from "./helpers/daemon-websocket-gate"; const LOADING_WORKSPACE_TEXT_PATTERN = /Loading workspace/i; +type StartupPresentation = "splash" | "app"; + +declare global { + interface Window { + __paseoStartupPresentationTrace?: StartupPresentation[]; + } +} + +async function observeStartupPresentation(page: Page): Promise { + await page.addInitScript(() => { + const trace: StartupPresentation[] = []; + window.__paseoStartupPresentationTrace = trace; + + document.addEventListener("DOMContentLoaded", () => { + const recordPresentation = () => { + let presentation: StartupPresentation | null = null; + if (document.querySelector('[data-testid="startup-splash"]')) { + presentation = "splash"; + } else if ( + document.querySelector( + '[data-testid="workspace-header-title"], [data-testid="sidebar-settings"]', + ) + ) { + presentation = "app"; + } + if (presentation && trace.at(-1) !== presentation) { + trace.push(presentation); + } + }; + new MutationObserver(recordPresentation).observe(document.documentElement, { + childList: true, + subtree: true, + }); + recordPresentation(); + }); + }); +} + +async function getStartupPresentation(page: Page): Promise { + return page.evaluate(() => window.__paseoStartupPresentationTrace?.slice() ?? []); +} async function expectNoLoadingWorkspacePane( page: Page, @@ -239,7 +280,9 @@ test.describe("Workspace navigation regression", () => { } }); - test("refresh keeps the user on the same workspace route", async ({ page }) => { + test("refresh keeps one continuous splash before restoring the workspace route", async ({ + page, + }) => { const serverId = getServerId(); const daemonGate = await installDaemonWebSocketGate(page); const workspace = await seedWorkspace({ repoPrefix: "workspace-refresh-route-" }); @@ -254,6 +297,7 @@ test.describe("Workspace navigation regression", () => { serverId, manageBuiltInDaemon: true, hangDaemonStart: true, + desktopSettingsDelayMs: 250, daemonListen: `127.0.0.1:${getE2EDaemonPort()}`, }); await openWorkspaceThroughApp(page, { serverId, workspace }); @@ -261,14 +305,16 @@ test.describe("Workspace navigation regression", () => { await expectWorkspaceTabVisible(page, agent.id); await expectWorkspaceLocation(page, { serverId, workspace }); + await observeStartupPresentation(page); await daemonGate.drop(); await page.reload(); - await expect(page.getByTestId("startup-splash")).toBeVisible({ timeout: 30_000 }); + await waitForDesktopDaemonStartRequest(page); daemonGate.restore(); await waitForSidebarHydration(page); await expectWorkspaceLocation(page, { serverId, workspace }); await waitForWorkspaceTabsVisible(page); + expect(await getStartupPresentation(page)).toEqual(["splash", "app"]); } finally { daemonGate.restore(); await workspace.cleanup(); diff --git a/packages/app/src/app/_layout.tsx b/packages/app/src/app/_layout.tsx index 2f7fcd2fd..5c3909798 100644 --- a/packages/app/src/app/_layout.tsx +++ b/packages/app/src/app/_layout.tsx @@ -60,7 +60,6 @@ import { resolveStartupBlocker, resolveStartupNavigationReady, shouldRunStartupGiveUpTimer, - startDaemonIfGateAllows, startHostRuntimeBootstrap, type StartupBlocker, } from "@/navigation/host-runtime-bootstrap"; @@ -337,7 +336,6 @@ function HostRuntimeBootstrapProvider({ children }: { children: ReactNode }) { store, daemonStartService, shouldStartDaemon: shouldStartBuiltInDaemon, - onGateError: (message) => daemonStartService.recordError(message), }); }, []); @@ -376,11 +374,7 @@ function HostRuntimeBootstrapProvider({ children }: { children: ReactNode }) { const retry = useCallback(() => { const daemonStartService = getDaemonStartService({ store: getHostRuntimeStore() }); - startDaemonIfGateAllows({ - daemonStartService, - shouldStartDaemon: shouldStartBuiltInDaemon, - onGateError: (message) => daemonStartService.recordError(message), - }); + void daemonStartService.startIfEnabled({ shouldStart: shouldStartBuiltInDaemon }); }, []); const splashError = diff --git a/packages/app/src/navigation/host-runtime-bootstrap.test.ts b/packages/app/src/navigation/host-runtime-bootstrap.test.ts index 3e82a0b07..abe7aa04d 100644 --- a/packages/app/src/navigation/host-runtime-bootstrap.test.ts +++ b/packages/app/src/navigation/host-runtime-bootstrap.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { resolveStartupBlocker, resolveStartupNavigationReady, @@ -7,115 +7,37 @@ import { shouldRunStartupGiveUpTimer, startHostRuntimeBootstrap, } from "./host-runtime-bootstrap"; - -function createFakeStore() { - return { boot: vi.fn() }; -} - -function createFakeDaemonStartService() { - return { - start: vi.fn(async () => ({ ok: true as const })), - }; -} +import type { + DaemonStartCondition, + StartDaemonIfEnabledInput, +} from "@/runtime/daemon-start-service"; describe("startHostRuntimeBootstrap", () => { - it("fires boot and daemon-start without awaiting the daemon-start promise", () => { + it("boots the host registry and starts the managed-daemon decision as one operation", () => { const events: string[] = []; + const shouldStartDaemon = async () => true; const store = { - boot: vi.fn(() => { + boot: () => { events.push("boot"); - }), - }; - const daemonStartService = { - start: vi.fn(async () => { - events.push("daemon-start"); - return { ok: true as const }; - }), - }; - - startHostRuntimeBootstrap({ - store, - daemonStartService, - shouldStartDaemon: true, - }); - - expect(store.boot).toHaveBeenCalledTimes(1); - expect(daemonStartService.start).toHaveBeenCalledTimes(1); - expect(events).toEqual(["boot", "daemon-start"]); - }); - - it("skips daemon-start when shouldStartDaemon is false", () => { - const store = createFakeStore(); - const daemonStartService = createFakeDaemonStartService(); - - startHostRuntimeBootstrap({ - store, - daemonStartService, - shouldStartDaemon: false, - }); - - expect(store.boot).toHaveBeenCalledTimes(1); - expect(daemonStartService.start).not.toHaveBeenCalled(); - }); - - it("skips daemon-start when the startup gate resolves false", async () => { - const store = createFakeStore(); - const daemonStartService = createFakeDaemonStartService(); - - startHostRuntimeBootstrap({ - store, - daemonStartService, - shouldStartDaemon: async () => false, - }); - await Promise.resolve(); - - expect(store.boot).toHaveBeenCalledTimes(1); - expect(daemonStartService.start).not.toHaveBeenCalled(); - }); - - it("surfaces gate rejection to onGateError without starting the daemon", async () => { - const store = createFakeStore(); - const daemonStartService = createFakeDaemonStartService(); - const onGateError = vi.fn(); - - startHostRuntimeBootstrap({ - store, - daemonStartService, - shouldStartDaemon: async () => { - throw new Error("settings file unreadable"); }, - onGateError, - }); - await vi.waitFor(() => { - expect(onGateError).toHaveBeenCalledTimes(1); - }); - - expect(daemonStartService.start).not.toHaveBeenCalled(); - expect(onGateError).toHaveBeenCalledWith(expect.stringContaining("settings file unreadable")); - }); - - it("does not await the daemon-start promise", () => { - const store = createFakeStore(); - let resolveStart: ((value: { ok: true }) => void) | undefined; + }; + let receivedCondition: DaemonStartCondition | null = null; const daemonStartService = { - start: vi.fn( - () => - new Promise<{ ok: true }>((resolve) => { - resolveStart = resolve; - }), - ), + startIfEnabled: async (input: StartDaemonIfEnabledInput) => { + receivedCondition = input.shouldStart; + events.push("daemon-start-decision"); + return { ok: true as const }; + }, }; startHostRuntimeBootstrap({ store, daemonStartService, - shouldStartDaemon: true, + shouldStartDaemon, }); - expect(store.boot).toHaveBeenCalledTimes(1); - expect(daemonStartService.start).toHaveBeenCalledTimes(1); - - resolveStart?.({ ok: true }); + expect(events).toEqual(["boot", "daemon-start-decision"]); + expect(receivedCondition).toBe(shouldStartDaemon); }); }); diff --git a/packages/app/src/navigation/host-runtime-bootstrap.ts b/packages/app/src/navigation/host-runtime-bootstrap.ts index cf3ea0344..eb09a88db 100644 --- a/packages/app/src/navigation/host-runtime-bootstrap.ts +++ b/packages/app/src/navigation/host-runtime-bootstrap.ts @@ -1,5 +1,9 @@ import type { ActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; -import type { DaemonStartResult } from "@/runtime/daemon-start-service"; +import type { + DaemonStartCondition, + DaemonStartResult, + StartDaemonIfEnabledInput, +} from "@/runtime/daemon-start-service"; import type { Href } from "expo-router"; import { buildHostRootRoute, @@ -12,54 +16,22 @@ export interface HostRuntimeBootstrapStore { } export interface HostRuntimeBootstrapDaemonStartService { - start: () => Promise; + startIfEnabled: (input: StartDaemonIfEnabledInput) => Promise; } -type HostRuntimeBootstrapStartGate = boolean | (() => boolean | Promise); - export interface StartHostRuntimeBootstrapInput { store: HostRuntimeBootstrapStore; daemonStartService: HostRuntimeBootstrapDaemonStartService; - shouldStartDaemon: HostRuntimeBootstrapStartGate; - onGateError?: (message: string) => void; + shouldStartDaemon: DaemonStartCondition; } export function startHostRuntimeBootstrap(input: StartHostRuntimeBootstrapInput): void { input.store.boot(); - startDaemonIfGateAllows({ - daemonStartService: input.daemonStartService, - shouldStartDaemon: input.shouldStartDaemon, - onGateError: input.onGateError, + void input.daemonStartService.startIfEnabled({ + shouldStart: input.shouldStartDaemon, }); } -export function startDaemonIfGateAllows(input: { - daemonStartService: HostRuntimeBootstrapDaemonStartService; - shouldStartDaemon: HostRuntimeBootstrapStartGate; - onGateError?: (message: string) => void; -}): void { - const gate = input.shouldStartDaemon; - if (typeof gate === "boolean") { - if (gate) { - void input.daemonStartService.start(); - } - return; - } - - void Promise.resolve() - .then(() => gate()) - .then((shouldStartDaemon) => { - if (shouldStartDaemon) { - void input.daemonStartService.start(); - } - return null; - }) - .catch((error) => { - const message = error instanceof Error ? error.message : String(error); - input.onGateError?.(`Failed to evaluate desktop daemon settings: ${message}`); - }); -} - const WELCOME_ROUTE: Href = "/welcome"; export type StartupBlocker = diff --git a/packages/app/src/runtime/daemon-start-service.test.ts b/packages/app/src/runtime/daemon-start-service.test.ts index 4793fb8af..e2c82ddf8 100644 --- a/packages/app/src/runtime/daemon-start-service.test.ts +++ b/packages/app/src/runtime/daemon-start-service.test.ts @@ -164,6 +164,53 @@ describe("DaemonStartService", () => { expect(runningSnapshots).toEqual([true, false]); }); + it("stays running while deciding whether the managed daemon should start", async () => { + const fake = createFakeStore(); + let resolveCondition: ((value: boolean) => void) | undefined; + let daemonStartCount = 0; + const service = new DaemonStartService({ + store: fake.store, + startDesktopDaemon: async () => { + daemonStartCount += 1; + return makeStatus(); + }, + }); + + const startPromise = service.startIfEnabled({ + shouldStart: () => + new Promise((resolve) => { + resolveCondition = resolve; + }), + }); + + expect(service.isRunning()).toBe(true); + expect(daemonStartCount).toBe(0); + + resolveCondition?.(true); + await startPromise; + + expect(service.isRunning()).toBe(false); + expect(daemonStartCount).toBe(1); + }); + + it("finishes without starting the daemon when management is disabled", async () => { + const fake = createFakeStore(); + let daemonStartCount = 0; + const service = new DaemonStartService({ + store: fake.store, + startDesktopDaemon: async () => { + daemonStartCount += 1; + return makeStatus(); + }, + }); + + const result = await service.startIfEnabled({ shouldStart: false }); + + expect(result).toEqual({ ok: true }); + expect(service.isRunning()).toBe(false); + expect(daemonStartCount).toBe(0); + }); + it("clears the error and notifies subscribers when retry begins", async () => { const fake = createFakeStore(); const startMock = vi @@ -188,19 +235,27 @@ describe("DaemonStartService", () => { expect(service.getLastError()).toBeNull(); }); - it("recordError surfaces an external error and notifies subscribers", () => { + it("surfaces settings errors through the daemon startup state", async () => { const fake = createFakeStore(); const service = new DaemonStartService({ store: fake.store, startDesktopDaemon: async () => makeStatus(), }); - const notifications = vi.fn(); - service.subscribe(notifications); - service.recordError("settings file unreadable"); + const result = await service.startIfEnabled({ + shouldStart: async () => { + throw new Error("settings file unreadable"); + }, + }); - expect(service.getLastError()).toBe("settings file unreadable"); - expect(notifications).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + ok: false, + error: "Failed to evaluate desktop daemon settings: settings file unreadable", + }); + expect(service.getLastError()).toBe( + "Failed to evaluate desktop daemon settings: settings file unreadable", + ); + expect(service.isRunning()).toBe(false); }); it("stops notifying after a subscriber unsubscribes", async () => { diff --git a/packages/app/src/runtime/daemon-start-service.ts b/packages/app/src/runtime/daemon-start-service.ts index e3c178bde..7a254c4e3 100644 --- a/packages/app/src/runtime/daemon-start-service.ts +++ b/packages/app/src/runtime/daemon-start-service.ts @@ -3,6 +3,11 @@ import { connectionFromListen } from "@/types/host-connection"; import type { HostRuntimeStore } from "@/runtime/host-runtime"; export type DaemonStartResult = { ok: true } | { ok: false; error: string }; +export type DaemonStartCondition = boolean | (() => boolean | Promise); + +export interface StartDaemonIfEnabledInput { + shouldStart: DaemonStartCondition; +} type DaemonConnectionStore = Pick; @@ -50,8 +55,27 @@ export class DaemonStartService { } async start(): Promise { + return this.startIfEnabled({ shouldStart: true }); + } + + async startIfEnabled(input: StartDaemonIfEnabledInput): Promise { + // Settings evaluation is part of startup. Publish the running state before + // its first await so restored app chrome cannot appear between these phases. this.beginRequest(); try { + let shouldStart: boolean; + try { + shouldStart = + typeof input.shouldStart === "boolean" ? input.shouldStart : await input.shouldStart(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return this.fail(`Failed to evaluate desktop daemon settings: ${message}`); + } + + if (!shouldStart) { + return { ok: true }; + } + const daemon = await this.invokeStartDesktopDaemon(); const result = await upsertDesktopDaemonConnection(this.store, daemon); return result.ok ? result : this.fail(result.error); @@ -66,10 +90,6 @@ export class DaemonStartService { return this.lastError; } - recordError(message: string): void { - this.setLastError(message); - } - isRunning(): boolean { return this.inFlightCount > 0; }