diff --git a/apps/desktop/e2e/fixtures/updater.ts b/apps/desktop/e2e/fixtures/updater.ts index 33fea2ba5dd..ff2b4703838 100644 --- a/apps/desktop/e2e/fixtures/updater.ts +++ b/apps/desktop/e2e/fixtures/updater.ts @@ -2,23 +2,43 @@ import { writeFileSync } from 'node:fs' import { homedir } from 'node:os' import { join, relative } from 'node:path' import type { DesktopUpdateState } from '@sim/desktop-bridge' -import { app, autoUpdater as nativeUpdater, net } from 'electron' +import { app, BrowserWindow, autoUpdater as nativeUpdater, net } from 'electron' import { MacUpdater } from 'electron-updater' +import { countWindowsWithUnsavedChanges, describeInFlightWork } from '@/main/in-flight-work' import { initUpdater } from '@/main/updater' +/** What a window's page does on `beforeunload`, shaped like the web app's own listeners. */ +type FixtureUnloadListener = 'none' | 'passive' | 'unsaved-edits' + +type FixtureDialog = Pick< + Electron.MessageBoxOptions, + 'title' | 'message' | 'buttons' | 'defaultId' | 'cancelId' +> + declare global { var desktopUpdaterFixture: { check(): void install(): void finishStaging(): void failStaging(): void + setWork(work: { + terminalCommands: string[] + browserToolRunning: boolean + unloadListener: FixtureUnloadListener + /** Adds a second window whose renderer is blocked past the shell's probe deadline. */ + hungWindow?: boolean + }): Promise read(): { state: DesktopUpdateState nativeArchive: string | null installed: string[] events: { name: string; data: unknown }[] + dialogs: FixtureDialog[] } } + /** Filled by the spec's `showShellDialog` stub, which answers with `updaterFixtureDialogResponse`. */ + var updaterFixtureDialogs: FixtureDialog[] + var updaterFixtureDialogResponse: number } const directory = process.env.SIM_UPDATER_FIXTURE_DIR @@ -26,7 +46,10 @@ const origin = process.env.SIM_UPDATER_FIXTURE_ORIGIN if (!directory || !origin) throw new Error('Updater fixture configuration is missing') app.setPath('userData', join(directory, 'user-data')) -void app.whenReady().then(() => { +globalThis.updaterFixtureDialogs = [] +globalThis.updaterFixtureDialogResponse = 0 + +void app.whenReady().then(async () => { const configPath = join(directory, 'updater.yml') const cache = relative(join(homedir(), 'Library', 'Caches'), join(directory, 'cache')) writeFileSync(configPath, `updaterCacheDirName: ${JSON.stringify(cache)}\n`) @@ -55,6 +78,14 @@ void app.whenReady().then(() => { installed.push(nativeArchive) } + // A real renderer, so the unsaved-edits probe runs against Chromium's own event dispatch. + const page = new BrowserWindow({ show: false }) + page.webContents.on('will-prevent-unload', (event) => event.preventDefault()) + await page.loadURL('about:blank') + let terminalCommands: string[] = [] + let browserToolRunning = false + let hungPage: BrowserWindow | null = null + const updater = new MacUpdater() updater.forceDevUpdateConfig = true updater.disableDifferentialDownload = true @@ -68,6 +99,16 @@ void app.whenReady().then(() => { canSelfUpdate: async () => true, probeOriginFeed: async () => false, installStatePath: join(directory, 'update-install.json'), + describeInFlightWork: async () => + describeInFlightWork({ + terminalCommands, + computerUseApp: null, + browserToolRunning, + agentWorkRunning: false, + ...(await countWindowsWithUnsavedChanges( + hungPage ? [page.webContents, hungPage.webContents] : [page.webContents] + )), + }), }) globalThis.desktopUpdaterFixture = { @@ -78,6 +119,43 @@ void app.whenReady().then(() => { nativeUpdater.emit('update-downloaded', {}, '', nativeArchive, new Date(), '') }, failStaging: () => nativeUpdater.emit('error', new Error('Native verification failed')), - read: () => ({ state: handle.getState(), nativeArchive, installed, events }), + setWork: async (work) => { + terminalCommands = work.terminalCommands + browserToolRunning = work.browserToolRunning + if (hungPage) { + // Let the blocked renderer finish first; destroying it mid-loop stalls the app's exit. + await hungPage.webContents.executeJavaScript('0') + hungPage.destroy() + hungPage = null + } + if (work.hungWindow) { + hungPage = new BrowserWindow({ show: false }) + await hungPage.loadURL('about:blank') + await hungPage.webContents.executeJavaScript(`window.addEventListener('beforeunload', () => { + const until = Date.now() + 1500 + while (Date.now() < until) {} + })`) + } + await page.webContents.executeJavaScript(`(() => { + for (const listener of window.fixtureUnloadListeners ?? []) { + window.removeEventListener('beforeunload', listener) + } + window.fixtureUnloadListeners = [] + const kind = ${JSON.stringify(work.unloadListener)} + if (kind === 'none') return + // 'passive' runs on unload without holding the page, like the chat's handoff cleanup. + const listener = + kind === 'unsaved-edits' ? (event) => event.preventDefault() : () => {} + window.addEventListener('beforeunload', listener) + window.fixtureUnloadListeners.push(listener) + })()`) + }, + read: () => ({ + state: handle.getState(), + nativeArchive, + installed, + events, + dialogs: globalThis.updaterFixtureDialogs, + }), } }) diff --git a/apps/desktop/e2e/updater.spec.ts b/apps/desktop/e2e/updater.spec.ts index ac0be66e98a..65bdaabffcf 100644 --- a/apps/desktop/e2e/updater.spec.ts +++ b/apps/desktop/e2e/updater.spec.ts @@ -10,7 +10,7 @@ import { build } from 'esbuild' const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) -test('the real MacUpdater waits for native staging, replaces old builds, and retries failed staging', async () => { +test('the real MacUpdater waits for native staging, replaces old builds, retries failed staging, and restarts on one click', async () => { test.skip(process.platform !== 'darwin', 'Squirrel.Mac lifecycle') const directory = mkdtempSync(join(tmpdir(), 'sim-updater-e2e-')) const reportPath = @@ -40,6 +40,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret } } let snapshot: unknown + let completed = false const server = createServer((request, response) => { const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname requests.push({ path, status: 200 }) @@ -72,15 +73,18 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret tsconfig: join(DESKTOP_DIR, 'tsconfig.json'), plugins: [ { - name: 'approve-fixture-restart', + name: 'record-fixture-dialogs', setup(builder) { builder.onResolve({ filter: /^@\/main\/dialogs$/ }, () => ({ path: 'dialog', namespace: 'fixture', })) builder.onLoad({ filter: /.*/, namespace: 'fixture' }, () => ({ - contents: - 'export async function showShellDialog() { return { response: 1, checkboxChecked: false } }', + contents: `export async function showShellDialog(...args) { + const { title, message, buttons, defaultId, cancelId } = args.at(-1) + globalThis.updaterFixtureDialogs.push({ title, message, buttons, defaultId, cancelId }) + return { response: globalThis.updaterFixtureDialogResponse, checkboxChecked: false } + }`, })) }, }, @@ -129,7 +133,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret expect((await read()).state).toEqual({ status: 'error', version: '2.1.0' }) }) - await check('cached retry installs only the verified replacement', async () => { + await check('cached retry stages only the verified replacement', async () => { await shell.evaluate(() => globalThis.desktopUpdaterFixture.check()) await expect .poll(async () => (await read()).state) @@ -137,9 +141,77 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret await expect.poll(async () => (await read()).nativeArchive).toBe('2.1.0') await shell.evaluate(() => globalThis.desktopUpdaterFixture.finishStaging()) expect((await read()).state).toEqual({ status: 'ready', version: '2.1.0' }) - await shell.evaluate(() => globalThis.desktopUpdaterFixture.install()) - await expect.poll(async () => (await read()).installed).toEqual(['2.1.0']) }) + + await check( + 'restart asks only about real in-flight work, then installs on one click', + async () => { + /** Sets the work in progress, clicks Restart to update, and answers any prompt with `response`. */ + const installWith = async ( + work: { + terminalCommands?: string[] + browserToolRunning?: boolean + unloadListener: 'passive' | 'unsaved-edits' + hungWindow?: boolean + }, + response: number + ) => { + await shell.evaluate( + async ( + _electron, + [{ terminalCommands = [], browserToolRunning = false, ...rest }, answer] + ) => { + await globalThis.desktopUpdaterFixture.setWork({ + terminalCommands, + browserToolRunning, + ...rest, + }) + globalThis.updaterFixtureDialogResponse = answer + globalThis.desktopUpdaterFixture.install() + }, + [work, response] as const + ) + } + const expectNotNowPrompt = async (count: number, message: string) => { + await expect.poll(async () => (await read()).dialogs.length).toBe(count) + expect((await read()).dialogs.at(-1)).toEqual({ + title: 'Restart to install Sim 2.1.0?', + message, + buttons: ['Restart and Update', 'Not Now'], + defaultId: 0, + cancelId: 1, + }) + // The decline settles asynchronously; nothing may install, and the update stays ready. + await expect + .poll(async () => (await read()).state) + .toEqual({ status: 'ready', version: '2.1.0' }) + expect((await read()).installed).toEqual([]) + } + + await installWith( + { terminalCommands: ['cd /repo && claude "fix the build"'], unloadListener: 'passive' }, + 1 + ) + await expectNotNowPrompt(1, 'claude is still running in a terminal.') + + await installWith({ unloadListener: 'unsaved-edits' }, 1) + await expectNotNowPrompt(2, 'A window has unsaved changes.') + + await installWith( + { browserToolRunning: true, unloadListener: 'passive', hungWindow: true }, + 1 + ) + await expectNotNowPrompt( + 3, + "An agent is using the browser. A window isn't responding and may have unsaved changes." + ) + + // Answering Not Now again would leave nothing installed if a prompt appeared. + await installWith({ unloadListener: 'passive' }, 1) + await expect.poll(async () => (await read()).installed).toEqual(['2.1.0']) + expect((await read()).dialogs).toHaveLength(3) + } + ) snapshot = await read() await check( 'the next process distinguishes an incomplete update from a successful install', @@ -182,6 +254,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret }) } ) + completed = true } finally { if (!snapshot && app) snapshot = await app @@ -192,7 +265,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret reportPath, JSON.stringify( { - passed: checks.length === 4 && checks.every((check) => check.status === 'passed'), + passed: completed && checks.every((check) => check.status === 'passed'), checks, requests, snapshot, diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index a1594bf89af..fccd280a9ec 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -1150,6 +1150,31 @@ describe('executeTool', () => { } }) + it('reports a browser action as running from queueing until it settles', async () => { + vi.useFakeTimers() + try { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + expect(driver.isBrowserToolRunning()).toBe(false) + const queued = driver.captureBrowserToolQueueBoundary('chat-test') + expect(driver.isBrowserToolRunning()).toBe(true) + if (queued) driver.releaseBrowserToolQueueBoundary(queued) + expect(driver.isBrowserToolRunning()).toBe(false) + const contents = session.requireTab().view.webContents + vi.mocked(contents.getURL).mockReturnValue('https://example.com/') + vi.mocked(contents.executeJavaScript).mockImplementation(() => new Promise(() => {})) + + const held = driver.executeTool('chat-test', 'browser_snapshot', {}, 'held-running') + await vi.advanceTimersByTimeAsync(0) + expect(driver.isBrowserToolRunning()).toBe(true) + + driver.cancelTool('chat-test', 'held-running') + await held + expect(driver.isBrowserToolRunning()).toBe(false) + } finally { + vi.useRealTimers() + } + }) + it('sanitizes hostile tab titles before returning them across the tool boundary', async () => { await driver.executeTool('chat-test', 'browser_open_tab', {}) const contents = session.requireTab().view.webContents diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 595d7369c05..846b05e5e99 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -457,6 +457,11 @@ function releaseBrowserToolAdmission(state: DriverScopeState, admission: symbol) activeBrowserToolAdmissions.delete(admission) } +/** Whether any chat has a browser action queued or running in this process. */ +export function isBrowserToolRunning(): boolean { + return activeBrowserToolAdmissions.size > 0 || pendingBrowserToolQueueBoundaries.size > 0 +} + function retireDriverScopeState(state: DriverScopeState): void { state.disposed = true state.toolQueueCancellationEpoch++ diff --git a/apps/desktop/src/main/config.ts b/apps/desktop/src/main/config.ts index 3148c992577..28ca953f16f 100644 --- a/apps/desktop/src/main/config.ts +++ b/apps/desktop/src/main/config.ts @@ -92,6 +92,8 @@ export interface DesktopSettings { windowBounds?: WindowBounds zoomLevel?: number lastRoute?: string + /** Routes of the other app windows open when an update relaunch was committed. */ + updateRelaunchRoutes?: string[] themeBackground?: 'dark' | 'light' blockThirdPartyAnalytics?: boolean trayEnabled?: boolean diff --git a/apps/desktop/src/main/in-flight-work.test.ts b/apps/desktop/src/main/in-flight-work.test.ts new file mode 100644 index 00000000000..54862b54fe8 --- /dev/null +++ b/apps/desktop/src/main/in-flight-work.test.ts @@ -0,0 +1,26 @@ +import type { WebContents } from 'electron' +import { describe, expect, it, vi } from 'vitest' +import { countWindowsWithUnsavedChanges } from '@/main/in-flight-work' + +vi.mock('electron', () => import('@/test/electron-mock')) + +function windowAnswering(answer: () => Promise, state = { destroyed: false }) { + return { + isDestroyed: () => state.destroyed, + isCrashed: () => false, + executeJavaScript: vi.fn(answer), + } as unknown as WebContents +} + +describe('countWindowsWithUnsavedChanges', () => { + it('never reports a window it could not ask as clean', async () => { + const counts = await countWindowsWithUnsavedChanges([ + windowAnswering(() => Promise.resolve(true)), + windowAnswering(() => Promise.resolve(false)), + windowAnswering(() => Promise.reject(new Error('Script failed to execute'))), + windowAnswering(() => Promise.resolve(true), { destroyed: true }), + ]) + + expect(counts).toEqual({ unsavedWindowCount: 1, unresponsiveWindowCount: 1 }) + }) +}) diff --git a/apps/desktop/src/main/in-flight-work.ts b/apps/desktop/src/main/in-flight-work.ts new file mode 100644 index 00000000000..c83eb8e8f2e --- /dev/null +++ b/apps/desktop/src/main/in-flight-work.ts @@ -0,0 +1,117 @@ +import { createLogger } from '@sim/logger' +import { describeRunningCommand } from '@sim/terminal-protocol' +import { getErrorMessage } from '@sim/utils/errors' +import { sleep } from '@sim/utils/helpers' +import type { WebContents } from 'electron' + +const logger = createLogger('DesktopInFlightWork') + +/** A renderer that cannot answer this quickly is treated as hung, holding edits it cannot report. */ +const UNSAVED_CHANGES_PROBE_TIMEOUT_MS = 1_000 +const MAX_NAMED_TERMINAL_COMMANDS = 2 + +/** + * Asks the page's own `beforeunload` guards whether they would hold it open, without unloading it, + * so every editor's live dirty state counts and no renderer needs to know about the shell. + */ +const UNSAVED_CHANGES_PROBE = `(() => { + let event + try { + event = document.createEvent('BeforeUnloadEvent') + event.initEvent('beforeunload', false, true) + } catch { + event = new Event('beforeunload', { cancelable: true }) + } + window.dispatchEvent(event) + return event.defaultPrevented || (typeof event.returnValue === 'string' && event.returnValue !== '') +})()` + +type UnloadAnswer = 'clean' | 'unsaved' | 'unresponsive' + +async function askAboutUnsavedChanges(contents: WebContents): Promise { + if (contents.isDestroyed() || contents.isCrashed()) return 'clean' + try { + const answer = await Promise.race([ + contents.executeJavaScript(UNSAVED_CHANGES_PROBE), + sleep(UNSAVED_CHANGES_PROBE_TIMEOUT_MS).then(() => 'unresponsive' as const), + ]) + if (answer === 'unresponsive') return answer + return answer === true ? 'unsaved' : 'clean' + } catch (error) { + logger.warn('Could not ask a window about unsaved changes', { + message: getErrorMessage(error, 'unknown'), + }) + return 'unresponsive' + } +} + +/** + * Counts the windows whose pages would ask before leaving, and those too hung + * to answer, which may be holding edits just the same. + */ +export async function countWindowsWithUnsavedChanges( + windows: readonly WebContents[] +): Promise> { + const answers = await Promise.all(windows.map(askAboutUnsavedChanges)) + return { + unsavedWindowCount: answers.filter((answer) => answer === 'unsaved').length, + unresponsiveWindowCount: answers.filter((answer) => answer === 'unresponsive').length, + } +} + +export interface InFlightWork { + /** The command line holding each busy terminal's foreground (`''` when unannounced). */ + terminalCommands: readonly string[] + /** The app a computer-use action is driving, `''` when it has no name, or null when idle. */ + computerUseApp: string | null + /** Whether an agent has a browser action queued or running. */ + browserToolRunning: boolean + /** Whether a chat has desktop work claimed on this machine. */ + agentWorkRunning: boolean + unsavedWindowCount: number + unresponsiveWindowCount: number +} + +/** + * Names the work a restart would interrupt, one sentence per kind, so a + * restart prompt says exactly what is at stake. Empty when nothing is. + */ +export function describeInFlightWork(work: InFlightWork): string[] { + const sentences: string[] = [] + const commands = work.terminalCommands.map(describeRunningCommand) + if (commands.length > MAX_NAMED_TERMINAL_COMMANDS) { + sentences.push(`${commands.length} terminal commands are still running.`) + } else if (commands.length > 1) { + sentences.push(`${commands.join(' and ')} are still running in terminals.`) + } else if (commands.length === 1) { + sentences.push(`${commands[0]} is still running in a terminal.`) + } + + if (work.computerUseApp !== null) { + sentences.push( + work.computerUseApp + ? `An agent is using ${work.computerUseApp}.` + : 'An agent is using an app on this computer.' + ) + } + if (work.browserToolRunning) { + sentences.push('An agent is using the browser.') + } + if (work.agentWorkRunning && work.computerUseApp === null && !work.browserToolRunning) { + sentences.push('An agent is still working on this computer.') + } + + if (work.unsavedWindowCount === 1) { + sentences.push('A window has unsaved changes.') + } else if (work.unsavedWindowCount > 1) { + sentences.push(`${work.unsavedWindowCount} windows have unsaved changes.`) + } + if (work.unresponsiveWindowCount === 1) { + sentences.push("A window isn't responding and may have unsaved changes.") + } else if (work.unresponsiveWindowCount > 1) { + sentences.push( + `${work.unresponsiveWindowCount} windows aren't responding and may have unsaved changes.` + ) + } + return sentences +} diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 152e5827147..161d4ae8526 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -37,6 +37,7 @@ import { executeTool as executeAgentBrowserTool, hasBrowserScopeSession, initDriver as initBrowserAgentDriver, + isBrowserToolRunning, restoreBrowserScope as restoreAgentBrowserScope, } from '@/main/browser-agent/driver' import { @@ -78,6 +79,7 @@ import { installDocumentationHelpSearch, uninstallDocumentationHelpSearch, } from '@/main/help-search' +import { countWindowsWithUnsavedChanges, describeInFlightWork } from '@/main/in-flight-work' import { registerIpcHandlers } from '@/main/ipc' import { attachLoadHealth, type LoadHealthHandle } from '@/main/load-health' import { LocalFilePermissions } from '@/main/local-file-permissions' @@ -128,6 +130,9 @@ function reportHandoffFailure(error: unknown): void { logger.error('Sign-in handoff failed', { error: getErrorMessage(error) }) } +/** Bounds what a hand-edited settings file can make an update relaunch open. */ +const MAX_RELAUNCH_WINDOWS = 16 + const DOCK_ICON_FOR_CHANNEL = { prod: 'dock-icon.png', staging: 'dock-icon-staging.png', @@ -575,6 +580,41 @@ function main(): void { return win } + /** + * An update relaunch reopens every app window where it was: the active one + * through `lastRoute` like any launch, the rest from this list. Written only + * once the relaunch is committed, and withdrawn if it fails. + */ + function rememberWindowsForRelaunch(committed: boolean): void { + if (!committed) { + if (config.get('updateRelaunchRoutes') === undefined) return + config.set('updateRelaunchRoutes', undefined) + config.flush() + return + } + const active = getMainWindow() + const activeRoute = active ? routeFromAppUrl(active.webContents.getURL()) : null + if (activeRoute) config.set('lastRoute', activeRoute) + const otherRoutes = getWindows() + .filter((win) => win !== active) + .flatMap((win) => routeFromAppUrl(win.webContents.getURL()) ?? []) + config.set('updateRelaunchRoutes', otherRoutes.length > 0 ? otherRoutes : undefined) + config.flush() + } + + /** Reopens the windows an update relaunch remembered, once the active window is up. */ + function restoreWindowsAfterRelaunch(): void { + const routes: unknown = config.get('updateRelaunchRoutes') + if (routes === undefined) return + config.set('updateRelaunchRoutes', undefined) + if (!Array.isArray(routes)) return + for (const route of routes.slice(0, MAX_RELAUNCH_WINDOWS)) { + if (typeof route === 'string' && decideStartRoute(route) === route) { + void createAndLoadAppWindow({ route }) + } + } + } + /** Opens the Sim app's settings page in the active window. */ function openSettings(): void { void openMainWindowAt(settingsRoute(config.get('lastRoute'))) @@ -685,6 +725,7 @@ function main(): void { Notification.isSupported() ? new Notification(options) : null, }) + let agentWorkRunning = false const desktopExecutor = createDesktopExecutorService({ userDataPath, origin: appOrigin, @@ -692,7 +733,10 @@ function main(): void { preferences: () => desktopSettings.getPreferences(), accountDataAvailable, onApprovals: (items) => approvalNotifier.update(items), - onBusyChange: (busy) => sleepBlocker.setBusy(busy), + onBusyChange: (busy) => { + agentWorkRunning = busy + sleepBlocker.setBusy(busy) + }, // A result the model has makes a tmux run it handed back as still going collectable across a // restart. onResultDelivered: (toolCallId) => terminal.markRunDelivered(toolCallId), @@ -1070,7 +1114,7 @@ function main(): void { updates: { getState: () => updater?.getState() ?? { status: 'idle' }, check: () => updater?.check(), - install: () => updater?.install(), + install: (options) => updater?.install(options), }, server: { open: () => serverWindow.open(), @@ -1119,6 +1163,22 @@ function main(): void { autoDownload: () => config.get('autoDownloadUpdates') ?? true, setRelaunchPending: (pending) => { committedRelaunchPending = pending + rememberWindowsForRelaunch(pending) + }, + describeInFlightWork: async () => { + // Native activity is read after the window probes so work started meanwhile still counts. + const windows = await countWindowsWithUnsavedChanges( + getWindows().map((win) => win.webContents) + ) + const terminalCommands = await terminal.runningCommands() + const computerUseActivity = computerUse.getActivity() + return describeInFlightWork({ + terminalCommands, + computerUseApp: computerUseActivity ? (computerUseActivity.appName ?? '') : null, + browserToolRunning: isBrowserToolRunning(), + agentWorkRunning, + ...windows, + }) }, beforeInstall: async () => { if (!prepareAccountDataTeardownForQuit()) { @@ -1135,6 +1195,7 @@ function main(): void { }, }) desktopSettings.applySystemPreferences() + restoreWindowsAfterRelaunch() }) } diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index ebd34d86e1f..68727596dff 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -385,7 +385,7 @@ export interface IpcDeps { updates: { getState: () => DesktopUpdateState check: () => void - install: () => void + install: (options: { mandatory: boolean }) => void } server: { open: () => void @@ -1016,7 +1016,8 @@ export function registerIpcHandlers(deps: IpcDeps): void { kind: 'send', gate: 'app-origin', needsUserActivation: true, - handler: () => deps.updates.install(), + handler: (options) => + deps.updates.install({ mandatory: isRecordLike(options) && options.mandatory === true }), }, 'browser-agent:execute-tool': { kind: 'invoke', diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index c9334dab154..a11d90157b1 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -303,6 +303,24 @@ export class TerminalService { this.cwdTimer = null } + /** + * The command line holding each busy terminal's foreground, `''` when unannounced. A shell + * without integration (fish, sh) announces nothing, so the OS's foreground process group + * answers for it; an unreadable group (null) counts as idle. + */ + async runningCommands(): Promise { + const groups = this.options.processGroups ?? OS_PROCESS_GROUPS + const commands = await Promise.all( + [...this.sessions.values()].map(async (session) => { + if (session.isBusy) return [session.foreground ?? ''] + if (session.hasShellIntegration) return [] + const pgid = await groups.foreground(session.pid) + return pgid !== null && pgid !== session.pid ? [''] : [] + }) + ) + return commands.flat() + } + getTabs(): TerminalTabsState { if (this.sessions.size === 0) return EMPTY_TABS return { diff --git a/apps/desktop/src/main/terminal/registry.ts b/apps/desktop/src/main/terminal/registry.ts index 22c91489830..51b9f8330fb 100644 --- a/apps/desktop/src/main/terminal/registry.ts +++ b/apps/desktop/src/main/terminal/registry.ts @@ -375,6 +375,14 @@ export class TerminalRegistry { return true } + /** The command line holding each busy terminal's foreground, across every chat. */ + async runningCommands(): Promise { + const commands = await Promise.all( + [...this.entries.values()].map((entry) => entry.service.runningCommands()) + ) + return commands.flat() + } + /** * Stops every command the agent started in any chat's terminals; the user's own are untouched. * That includes tmux runs no live terminal holds any more: a chat put away, or a previous diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 6ff6a0a53af..c44ebb49e7c 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -104,6 +104,9 @@ const { stubSessions } = vi.hoisted(() => ({ clearScrollback(): void /** Whether Ctrl-C ends the running command, as it does for most programs. */ setInterruptible(interruptible: boolean): void + /** Whether the shell announces its commands, as zsh and bash do and fish does not. */ + setShellIntegration(on: boolean): void + readonly pid: number /** Ends the running command the way its process exiting would. */ finishRun(exitCode: number): void /** Returns the run as still going, its command left running, as a wait that ran out does. */ @@ -142,6 +145,7 @@ vi.mock('@/main/terminal/session', async () => { disposed: false, busy: false, interruptible: true, + shellIntegration: true, toolCallId: null as string | null, resolveRun: null as ((result: Record) => void) | null, } @@ -159,6 +163,9 @@ vi.mock('@/main/terminal/session', async () => { setInterruptible: (interruptible: boolean) => { state.interruptible = interruptible }, + setShellIntegration: (on: boolean) => { + state.shellIntegration = on + }, finishRun, handBack: () => { const resolve = state.resolveRun @@ -204,7 +211,9 @@ vi.mock('@/main/terminal/session', async () => { get isBusy() { return state.busy }, - hasShellIntegration: true, + get hasShellIntegration() { + return state.shellIntegration + }, /** Real sessions poll the pty here; a stub's cwd only ever changes on open. */ refreshCwd: async () => {}, dispose: () => { @@ -477,6 +486,38 @@ describe('a shell that ends by itself', () => { }) }) +describe('running work before a restart', () => { + function serviceWithForeground(read: (shellPid: number) => Promise) { + const terminal = new TerminalService({ + loadCwd: () => '/tmp', + processGroups: { foreground: vi.fn(read), signal: vi.fn() }, + }) + const { activeTerminalId } = terminal.start({ cols: 80, rows: 24 }) + const session = stubSessions.get(activeTerminalId as string) + if (!session) throw new Error('No stub session') + return { terminal, session } + } + + it('asks the OS about a shell that announces nothing, and trusts one that does', async () => { + const { terminal, session } = serviceWithForeground(async () => 4242) + session.setShellIntegration(false) + await expect(terminal.runningCommands()).resolves.toEqual(['']) + + session.setShellIntegration(true) + await expect(terminal.runningCommands()).resolves.toEqual([]) + }) + + it('counts a shell at its own prompt, or an unreadable foreground, as idle', async () => { + let read: () => Promise = async () => null + const { terminal, session } = serviceWithForeground(() => read()) + session.setShellIntegration(false) + await expect(terminal.runningCommands()).resolves.toEqual([]) + + read = async () => session.pid + await expect(terminal.runningCommands()).resolves.toEqual([]) + }) +}) + describe('stopping a tool call', () => { const COMMAND_GROUP = 4242 diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index 5ed6241ccfb..5909fa3824a 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -112,6 +112,8 @@ describe('initUpdater state machine', () => { probeOriginFeed?: (feedUrl: string) => Promise beforeInstall?: () => Promise setRelaunchPending?: (pending: boolean) => void + /** Work the restart would interrupt, as the in-flight check names it. */ + withRunningWork?: string }) { const states: DesktopUpdateState[] = [] const handle = initUpdater({ @@ -127,6 +129,7 @@ describe('initUpdater state machine', () => { platform: 'darwin', beforeInstall: options?.beforeInstall, setRelaunchPending: options?.setRelaunchPending, + describeInFlightWork: async () => (options?.withRunningWork ? [options.withRunningWork] : []), }) // Engine selection (signature detection) resolves asynchronously. await vi.advanceTimersByTimeAsync(0) @@ -151,7 +154,7 @@ describe('initUpdater state machine', () => { vi.useRealTimers() }) - it('walks check -> validated download -> ready and installs only after confirmation', async () => { + it('walks check -> validated download -> ready and installs on one click when nothing is running', async () => { const { handle, states } = await createUpdater() expect(handle.getState()).toEqual({ status: 'idle' }) @@ -183,21 +186,47 @@ describe('initUpdater state machine', () => { handle.install() await vi.advanceTimersByTimeAsync(0) + expect(dialog.showMessageBox).not.toHaveBeenCalled() + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + }) + + it('asks once, naming the running work, and keeps the update ready on Not Now', async () => { + const setRelaunchPending = vi.fn() + const { handle } = await createUpdater({ + setRelaunchPending, + withRunningWork: 'claude is still running in a terminal.', + }) + await stageUpdate(handle, '2.0.0') + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ response: 1, checkboxChecked: false }) + + handle.install() + await vi.advanceTimersByTimeAsync(0) + + expect(dialog.showMessageBox).toHaveBeenCalledTimes(1) expect(dialog.showMessageBox).toHaveBeenCalledWith( expect.objectContaining({ - buttons: ['Later', 'Restart and update'], + message: expect.stringContaining('claude is still running in a terminal.'), + buttons: ['Restart and Update', 'Not Now'], defaultId: 0, - cancelId: 0, + cancelId: 1, }) ) expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(setRelaunchPending).not.toHaveBeenCalled() + expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' }) + expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(true) + }) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, + it('installs a required update without offering to wait for running work', async () => { + const { handle } = await createUpdater({ + withRunningWork: 'claude is still running in a terminal.', }) - handle.install() + await stageUpdate(handle, '2.0.0') + + handle.install({ mandatory: true }) await vi.advanceTimersByTimeAsync(0) + + expect(dialog.showMessageBox).not.toHaveBeenCalled() expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) @@ -212,7 +241,9 @@ describe('initUpdater state machine', () => { resolveConfirmation = resolve }) ) - const { handle } = await createUpdater() + const { handle } = await createUpdater({ + withRunningWork: 'An agent is still working on this computer.', + }) handle.check() await vi.advanceTimersByTimeAsync(0) @@ -222,11 +253,12 @@ describe('initUpdater state machine', () => { vi.mocked(dialog.showMessageBox).mockClear() handle.install() handle.install() + await vi.advanceTimersByTimeAsync(0) expect(dialog.showMessageBox).toHaveBeenCalledTimes(1) expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() - resolveConfirmation({ response: 1, checkboxChecked: false }) + resolveConfirmation({ response: 0, checkboxChecked: false }) await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) @@ -260,10 +292,6 @@ describe('initUpdater state machine', () => { handle.check() emit('update-downloaded', { version: '2.0.0' }) emitSquirrel('update-downloaded') - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, - }) handle.install() await vi.advanceTimersByTimeAsync(0) @@ -292,10 +320,6 @@ describe('initUpdater state machine', () => { emit('update-downloaded', { version: '2.0.0' }) emitSquirrel('update-downloaded') await vi.advanceTimersByTimeAsync(10_000) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, - }) handle.install() await vi.advanceTimersByTimeAsync(0) @@ -328,7 +352,6 @@ describe('initUpdater state machine', () => { }) ) await vi.advanceTimersByTimeAsync(10_000) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ response: 1, checkboxChecked: false }) handle.install() await vi.advanceTimersByTimeAsync(0) @@ -343,10 +366,6 @@ describe('initUpdater state machine', () => { it('bypasses renderer unload guards only after teardown succeeds', async () => { const setRelaunchPending = vi.fn() - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, - }) const { handle } = await createUpdater({ beforeInstall: async () => {}, setRelaunchPending, @@ -446,7 +465,9 @@ describe('initUpdater state machine', () => { () => { throw new Error('Restart confirmation did not initialize') } - const { handle } = await createUpdater() + const { handle } = await createUpdater({ + withRunningWork: 'A window has unsaved changes.', + }) await stageUpdate(handle, '2.0.0') await vi.advanceTimersByTimeAsync(10_000) @@ -457,11 +478,12 @@ describe('initUpdater state machine', () => { }) ) handle.install() + await vi.advanceTimersByTimeAsync(0) emit('update-available', { version: '2.1.0' }) emit('update-downloaded', { version: '2.1.0' }) emitSquirrel('update-downloaded') expect(handle.getState()).toEqual({ status: 'ready', version: '2.1.0' }) - resolveConfirmation({ response: 1, checkboxChecked: false }) + resolveConfirmation({ response: 0, checkboxChecked: false }) await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) @@ -477,7 +499,6 @@ describe('initUpdater state machine', () => { }) await stageUpdate(handle, '2.0.0') await vi.advanceTimersByTimeAsync(10_000) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ response: 1, checkboxChecked: false }) handle.install() await vi.advanceTimersByTimeAsync(0) diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index 05e786dd541..b84071d3da3 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -251,12 +251,22 @@ export interface UpdaterDeps { platform?: NodeJS.Platform /** Flushes desktop-owned state before Squirrel terminates the process. */ beforeInstall?: () => Promise - /** Bypasses renderer unload guards only after the user confirms a relaunch. */ + /** Bypasses renderer unload guards once a relaunch is committed. */ setRelaunchPending?: (pending: boolean) => void + /** + * Names the work a restart would interrupt (running commands, agent + * actions, unsaved edits). A restart asks first only when this is non-empty. + */ + describeInFlightWork?: () => Promise /** Persists the natively staged version so the next process can verify installation. */ installStatePath?: string } +interface UpdateInstallOptions { + /** The shell is below the server's minimum version, so waiting is not an option. */ + mandatory?: boolean +} + export interface UpdaterHandle { setAutoDownload(enabled: boolean): void /** Current pipeline state for the renderer update UI. */ @@ -270,7 +280,7 @@ export interface UpdaterHandle { * Quit and install a downloaded update (`ready`), or open the manual * download for an `available` update on a shell that can't self-update. */ - install(): void + install(options?: UpdateInstallOptions): void /** Main-process state subscription (menu feedback). Returns unsubscribe. */ onState(callback: (state: DesktopUpdateState) => void): () => void } @@ -338,7 +348,7 @@ interface UpdateEngine { /** `interactive` requests must always publish a terminal state for their waiting UI. */ check(interactive?: boolean): void advance(): void - install(): void + install(options: { mandatory: boolean }): void setAutoDownload(enabled: boolean): void } @@ -470,35 +480,47 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { }) } - const confirmAndInstall = () => { + /** Installs at once unless the restart would interrupt nameable work; then asks once. */ + const installUnlessInterrupting = async ({ mandatory }: { mandatory: boolean }) => { if (installInFlight || installConfirmationInFlight || state.status !== 'ready') return installConfirmationInFlight = true - const version = state.version - const options: Electron.MessageBoxOptions = { - type: 'question', - buttons: ['Later', 'Restart and update'], - defaultId: 0, - cancelId: 0, - message: version ? `Restart to install Sim ${version}?` : 'Restart to update Sim?', - detail: - 'Sim will close all app windows while it updates. Running terminal commands, browser activity, downloads, uploads, and unsaved edits may be interrupted. Choose Later to install the update the next time you quit Sim.', - } - const win = deps.getWindow() - const confirmation = win ? showShellDialog(win, options) : showShellDialog(options) - void confirmation - .then(({ response }) => { - if (response === 1 && state.status === 'ready') { - quitAndInstall() + try { + let work: string[] = [] + if (!mandatory) { + try { + work = (await deps.describeInFlightWork?.()) ?? [] + } catch (error) { + logger.warn('Could not check for work a restart would interrupt', { + message: getErrorMessage(error, 'unknown'), + }) } + } + if (work.length > 0) { + const version = state.version + const options: Electron.MessageBoxOptions = { + type: 'warning', + title: version ? `Restart to install Sim ${version}?` : 'Restart to update Sim?', + message: work.join(' '), + detail: + 'Restarting now interrupts this work. Choose Not Now to finish first; the update also installs the next time you quit Sim.', + buttons: ['Restart and Update', 'Not Now'], + defaultId: 0, + cancelId: 1, + } + const win = deps.getWindow() + const { response } = await (win + ? showShellDialog(win, options) + : showShellDialog(options)) + if (response !== 0) return + } + if (state.status === 'ready') quitAndInstall() + } catch (error) { + logger.warn('Could not show the restart prompt for work in progress', { + message: getErrorMessage(error, 'unknown'), }) - .catch((error) => { - logger.warn('Could not show update restart confirmation', { - message: getErrorMessage(error, 'unknown'), - }) - }) - .finally(() => { - installConfirmationInFlight = false - }) + } finally { + installConfirmationInFlight = false + } } let activeProbeId: number | null = null @@ -838,8 +860,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { advance() { if (acceptedUpdateVersion !== null) download(acceptedUpdateVersion) }, - install() { - confirmAndInstall() + install(options) { + void installUnlessInterrupting(options) }, setAutoDownload(enabled) { autoDownloadEnabled = enabled @@ -1017,12 +1039,12 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { } engine.check(true) }, - install() { + install({ mandatory = false } = {}) { if (!engine) { return } if (state.status === 'ready') { - engine.install() + engine.install({ mandatory }) return } if (state.status === 'available' && state.manual) { @@ -1094,7 +1116,7 @@ export function checkForUpdatesInteractive( void showDialog({ type: 'info', message: state.version ? `Downloading Sim ${state.version}…` : 'Downloading update…', - detail: 'You will be prompted to restart when it is ready.', + detail: 'When it finishes, choose Restart to update in Sim, or quit Sim to install it.', }) return case 'ready': diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 811b07561f4..03acbf2a5be 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -238,8 +238,8 @@ const api: SimDesktopApi = { check: (): void => { ipcRenderer.send('desktop:updates:check') }, - install: (): void => { - ipcRenderer.send('desktop:updates:install') + install: (options?: { mandatory?: boolean }): void => { + ipcRenderer.send('desktop:updates:install', { mandatory: options?.mandatory === true }) }, onState: (callback: (state: DesktopUpdateState) => void): (() => void) => { const listener = (_event: unknown, state: DesktopUpdateState) => callback(state) diff --git a/apps/sim/app/_shell/desktop-update-gate.tsx b/apps/sim/app/_shell/desktop-update-gate.tsx index ed231c04534..27f47c3f244 100644 --- a/apps/sim/app/_shell/desktop-update-gate.tsx +++ b/apps/sim/app/_shell/desktop-update-gate.tsx @@ -51,7 +51,7 @@ function gateActionFor(state: DesktopUpdateState): GateAction { onClick: () => {}, } case 'ready': - return { label: 'Restart to update', onClick: () => updates.install() } + return { label: 'Restart to update', onClick: () => updates.install({ mandatory: true }) } default: // idle, available, and error all advance the same way: check() checks // for an update or starts the download of one already found. diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index cd256878d47..5577b79af38 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -4539,10 +4539,8 @@ export function useChat( } window.addEventListener('pagehide', clearClaim) - window.addEventListener('beforeunload', clearClaim) return () => { window.removeEventListener('pagehide', clearClaim) - window.removeEventListener('beforeunload', clearClaim) } }, []) useEffect(() => { diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index 248c84ce8c6..48b3cf01e3c 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -1100,8 +1100,12 @@ interface SimDesktopUpdatesApi { * self-update, or opens an available manual installer. */ check(): void - /** Installs a ready update or opens the installer for an available manual update. */ - install(): void + /** + * Installs a ready update or opens the installer for an available manual + * update. A `mandatory` install never offers to wait, even for running work. + * Shells that predate the option ignore it. + */ + install(options?: { mandatory?: boolean }): void /** Subscribe to pipeline state changes. Returns an unsubscribe function. */ onState(callback: (state: DesktopUpdateState) => void): () => void }