Skip to content

Commit d3f4739

Browse files
authored
improvement(desktop): install updates on one click, asking only about real in-flight work (#9505)
* improvement(desktop): install updates on one click, asking only about real in-flight work - Restart to update installs immediately; the generic Later/Restart dialog is gone - Prompt only when a terminal command, agent action, computer use, or unsaved edit would be interrupted, naming it, with restart as the default - The minimum-version gate never offers to wait - Reopen every app window after an update relaunch - Fix the menu's downloading copy, which promised a prompt that never came * fix(desktop): count unresponsive windows and browser actions before an update restart - A window that doesn't answer the unsaved-edits check counts as possibly unsaved instead of clean - A running browser-agent action counts as work a restart would interrupt - The queued-send claim is cleared on pagehide only, so the check (or a cancelled unload) can't strand a queued message * fix(desktop): snapshot running work after the window probes and count queued browser actions - Read terminal, computer-use, browser and agent activity after the unsaved-edits probes finish, so work started meanwhile still prompts - A browser action waiting in the queue counts as running - A window whose probe errors counts as possibly unsaved, like one that times out * fix(desktop): ask the OS about terminals without shell integration before an update restart Shells that announce nothing (fish, sh) now count as busy when their tty's foreground process group is not the shell's own. Live windows are recorded for relaunch without the cap, which only bounds persisted routes on read. * chore(desktop): drop the unreachable foreground-read catch
1 parent 2db94a5 commit d3f4739

18 files changed

Lines changed: 581 additions & 81 deletions

File tree

‎apps/desktop/e2e/fixtures/updater.ts‎

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,54 @@ import { writeFileSync } from 'node:fs'
22
import { homedir } from 'node:os'
33
import { join, relative } from 'node:path'
44
import type { DesktopUpdateState } from '@sim/desktop-bridge'
5-
import { app, autoUpdater as nativeUpdater, net } from 'electron'
5+
import { app, BrowserWindow, autoUpdater as nativeUpdater, net } from 'electron'
66
import { MacUpdater } from 'electron-updater'
7+
import { countWindowsWithUnsavedChanges, describeInFlightWork } from '@/main/in-flight-work'
78
import { initUpdater } from '@/main/updater'
89

10+
/** What a window's page does on `beforeunload`, shaped like the web app's own listeners. */
11+
type FixtureUnloadListener = 'none' | 'passive' | 'unsaved-edits'
12+
13+
type FixtureDialog = Pick<
14+
Electron.MessageBoxOptions,
15+
'title' | 'message' | 'buttons' | 'defaultId' | 'cancelId'
16+
>
17+
918
declare global {
1019
var desktopUpdaterFixture: {
1120
check(): void
1221
install(): void
1322
finishStaging(): void
1423
failStaging(): void
24+
setWork(work: {
25+
terminalCommands: string[]
26+
browserToolRunning: boolean
27+
unloadListener: FixtureUnloadListener
28+
/** Adds a second window whose renderer is blocked past the shell's probe deadline. */
29+
hungWindow?: boolean
30+
}): Promise<void>
1531
read(): {
1632
state: DesktopUpdateState
1733
nativeArchive: string | null
1834
installed: string[]
1935
events: { name: string; data: unknown }[]
36+
dialogs: FixtureDialog[]
2037
}
2138
}
39+
/** Filled by the spec's `showShellDialog` stub, which answers with `updaterFixtureDialogResponse`. */
40+
var updaterFixtureDialogs: FixtureDialog[]
41+
var updaterFixtureDialogResponse: number
2242
}
2343

2444
const directory = process.env.SIM_UPDATER_FIXTURE_DIR
2545
const origin = process.env.SIM_UPDATER_FIXTURE_ORIGIN
2646
if (!directory || !origin) throw new Error('Updater fixture configuration is missing')
2747
app.setPath('userData', join(directory, 'user-data'))
2848

29-
void app.whenReady().then(() => {
49+
globalThis.updaterFixtureDialogs = []
50+
globalThis.updaterFixtureDialogResponse = 0
51+
52+
void app.whenReady().then(async () => {
3053
const configPath = join(directory, 'updater.yml')
3154
const cache = relative(join(homedir(), 'Library', 'Caches'), join(directory, 'cache'))
3255
writeFileSync(configPath, `updaterCacheDirName: ${JSON.stringify(cache)}\n`)
@@ -55,6 +78,14 @@ void app.whenReady().then(() => {
5578
installed.push(nativeArchive)
5679
}
5780

81+
// A real renderer, so the unsaved-edits probe runs against Chromium's own event dispatch.
82+
const page = new BrowserWindow({ show: false })
83+
page.webContents.on('will-prevent-unload', (event) => event.preventDefault())
84+
await page.loadURL('about:blank')
85+
let terminalCommands: string[] = []
86+
let browserToolRunning = false
87+
let hungPage: BrowserWindow | null = null
88+
5889
const updater = new MacUpdater()
5990
updater.forceDevUpdateConfig = true
6091
updater.disableDifferentialDownload = true
@@ -68,6 +99,16 @@ void app.whenReady().then(() => {
6899
canSelfUpdate: async () => true,
69100
probeOriginFeed: async () => false,
70101
installStatePath: join(directory, 'update-install.json'),
102+
describeInFlightWork: async () =>
103+
describeInFlightWork({
104+
terminalCommands,
105+
computerUseApp: null,
106+
browserToolRunning,
107+
agentWorkRunning: false,
108+
...(await countWindowsWithUnsavedChanges(
109+
hungPage ? [page.webContents, hungPage.webContents] : [page.webContents]
110+
)),
111+
}),
71112
})
72113

73114
globalThis.desktopUpdaterFixture = {
@@ -78,6 +119,43 @@ void app.whenReady().then(() => {
78119
nativeUpdater.emit('update-downloaded', {}, '', nativeArchive, new Date(), '')
79120
},
80121
failStaging: () => nativeUpdater.emit('error', new Error('Native verification failed')),
81-
read: () => ({ state: handle.getState(), nativeArchive, installed, events }),
122+
setWork: async (work) => {
123+
terminalCommands = work.terminalCommands
124+
browserToolRunning = work.browserToolRunning
125+
if (hungPage) {
126+
// Let the blocked renderer finish first; destroying it mid-loop stalls the app's exit.
127+
await hungPage.webContents.executeJavaScript('0')
128+
hungPage.destroy()
129+
hungPage = null
130+
}
131+
if (work.hungWindow) {
132+
hungPage = new BrowserWindow({ show: false })
133+
await hungPage.loadURL('about:blank')
134+
await hungPage.webContents.executeJavaScript(`window.addEventListener('beforeunload', () => {
135+
const until = Date.now() + 1500
136+
while (Date.now() < until) {}
137+
})`)
138+
}
139+
await page.webContents.executeJavaScript(`(() => {
140+
for (const listener of window.fixtureUnloadListeners ?? []) {
141+
window.removeEventListener('beforeunload', listener)
142+
}
143+
window.fixtureUnloadListeners = []
144+
const kind = ${JSON.stringify(work.unloadListener)}
145+
if (kind === 'none') return
146+
// 'passive' runs on unload without holding the page, like the chat's handoff cleanup.
147+
const listener =
148+
kind === 'unsaved-edits' ? (event) => event.preventDefault() : () => {}
149+
window.addEventListener('beforeunload', listener)
150+
window.fixtureUnloadListeners.push(listener)
151+
})()`)
152+
},
153+
read: () => ({
154+
state: handle.getState(),
155+
nativeArchive,
156+
installed,
157+
events,
158+
dialogs: globalThis.updaterFixtureDialogs,
159+
}),
82160
}
83161
})

‎apps/desktop/e2e/updater.spec.ts‎

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { build } from 'esbuild'
1010

1111
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
1212

13-
test('the real MacUpdater waits for native staging, replaces old builds, and retries failed staging', async () => {
13+
test('the real MacUpdater waits for native staging, replaces old builds, retries failed staging, and restarts on one click', async () => {
1414
test.skip(process.platform !== 'darwin', 'Squirrel.Mac lifecycle')
1515
const directory = mkdtempSync(join(tmpdir(), 'sim-updater-e2e-'))
1616
const reportPath =
@@ -40,6 +40,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret
4040
}
4141
}
4242
let snapshot: unknown
43+
let completed = false
4344
const server = createServer((request, response) => {
4445
const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname
4546
requests.push({ path, status: 200 })
@@ -72,15 +73,18 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret
7273
tsconfig: join(DESKTOP_DIR, 'tsconfig.json'),
7374
plugins: [
7475
{
75-
name: 'approve-fixture-restart',
76+
name: 'record-fixture-dialogs',
7677
setup(builder) {
7778
builder.onResolve({ filter: /^@\/main\/dialogs$/ }, () => ({
7879
path: 'dialog',
7980
namespace: 'fixture',
8081
}))
8182
builder.onLoad({ filter: /.*/, namespace: 'fixture' }, () => ({
82-
contents:
83-
'export async function showShellDialog() { return { response: 1, checkboxChecked: false } }',
83+
contents: `export async function showShellDialog(...args) {
84+
const { title, message, buttons, defaultId, cancelId } = args.at(-1)
85+
globalThis.updaterFixtureDialogs.push({ title, message, buttons, defaultId, cancelId })
86+
return { response: globalThis.updaterFixtureDialogResponse, checkboxChecked: false }
87+
}`,
8488
}))
8589
},
8690
},
@@ -129,17 +133,85 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret
129133
expect((await read()).state).toEqual({ status: 'error', version: '2.1.0' })
130134
})
131135

132-
await check('cached retry installs only the verified replacement', async () => {
136+
await check('cached retry stages only the verified replacement', async () => {
133137
await shell.evaluate(() => globalThis.desktopUpdaterFixture.check())
134138
await expect
135139
.poll(async () => (await read()).state)
136140
.toEqual({ status: 'downloading', version: '2.1.0', percent: 100 })
137141
await expect.poll(async () => (await read()).nativeArchive).toBe('2.1.0')
138142
await shell.evaluate(() => globalThis.desktopUpdaterFixture.finishStaging())
139143
expect((await read()).state).toEqual({ status: 'ready', version: '2.1.0' })
140-
await shell.evaluate(() => globalThis.desktopUpdaterFixture.install())
141-
await expect.poll(async () => (await read()).installed).toEqual(['2.1.0'])
142144
})
145+
146+
await check(
147+
'restart asks only about real in-flight work, then installs on one click',
148+
async () => {
149+
/** Sets the work in progress, clicks Restart to update, and answers any prompt with `response`. */
150+
const installWith = async (
151+
work: {
152+
terminalCommands?: string[]
153+
browserToolRunning?: boolean
154+
unloadListener: 'passive' | 'unsaved-edits'
155+
hungWindow?: boolean
156+
},
157+
response: number
158+
) => {
159+
await shell.evaluate(
160+
async (
161+
_electron,
162+
[{ terminalCommands = [], browserToolRunning = false, ...rest }, answer]
163+
) => {
164+
await globalThis.desktopUpdaterFixture.setWork({
165+
terminalCommands,
166+
browserToolRunning,
167+
...rest,
168+
})
169+
globalThis.updaterFixtureDialogResponse = answer
170+
globalThis.desktopUpdaterFixture.install()
171+
},
172+
[work, response] as const
173+
)
174+
}
175+
const expectNotNowPrompt = async (count: number, message: string) => {
176+
await expect.poll(async () => (await read()).dialogs.length).toBe(count)
177+
expect((await read()).dialogs.at(-1)).toEqual({
178+
title: 'Restart to install Sim 2.1.0?',
179+
message,
180+
buttons: ['Restart and Update', 'Not Now'],
181+
defaultId: 0,
182+
cancelId: 1,
183+
})
184+
// The decline settles asynchronously; nothing may install, and the update stays ready.
185+
await expect
186+
.poll(async () => (await read()).state)
187+
.toEqual({ status: 'ready', version: '2.1.0' })
188+
expect((await read()).installed).toEqual([])
189+
}
190+
191+
await installWith(
192+
{ terminalCommands: ['cd /repo && claude "fix the build"'], unloadListener: 'passive' },
193+
1
194+
)
195+
await expectNotNowPrompt(1, 'claude is still running in a terminal.')
196+
197+
await installWith({ unloadListener: 'unsaved-edits' }, 1)
198+
await expectNotNowPrompt(2, 'A window has unsaved changes.')
199+
200+
await installWith(
201+
{ browserToolRunning: true, unloadListener: 'passive', hungWindow: true },
202+
1
203+
)
204+
await expectNotNowPrompt(
205+
3,
206+
"An agent is using the browser. A window isn't responding and may have unsaved changes."
207+
)
208+
209+
// Answering Not Now again would leave nothing installed if a prompt appeared.
210+
await installWith({ unloadListener: 'passive' }, 1)
211+
await expect.poll(async () => (await read()).installed).toEqual(['2.1.0'])
212+
expect((await read()).dialogs).toHaveLength(3)
213+
}
214+
)
143215
snapshot = await read()
144216
await check(
145217
'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
182254
})
183255
}
184256
)
257+
completed = true
185258
} finally {
186259
if (!snapshot && app)
187260
snapshot = await app
@@ -192,7 +265,7 @@ test('the real MacUpdater waits for native staging, replaces old builds, and ret
192265
reportPath,
193266
JSON.stringify(
194267
{
195-
passed: checks.length === 4 && checks.every((check) => check.status === 'passed'),
268+
passed: completed && checks.every((check) => check.status === 'passed'),
196269
checks,
197270
requests,
198271
snapshot,

‎apps/desktop/src/main/browser-agent/driver.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,31 @@ describe('executeTool', () => {
11501150
}
11511151
})
11521152

1153+
it('reports a browser action as running from queueing until it settles', async () => {
1154+
vi.useFakeTimers()
1155+
try {
1156+
await driver.executeTool('chat-test', 'browser_open_tab', {})
1157+
expect(driver.isBrowserToolRunning()).toBe(false)
1158+
const queued = driver.captureBrowserToolQueueBoundary('chat-test')
1159+
expect(driver.isBrowserToolRunning()).toBe(true)
1160+
if (queued) driver.releaseBrowserToolQueueBoundary(queued)
1161+
expect(driver.isBrowserToolRunning()).toBe(false)
1162+
const contents = session.requireTab().view.webContents
1163+
vi.mocked(contents.getURL).mockReturnValue('https://example.com/')
1164+
vi.mocked(contents.executeJavaScript).mockImplementation(() => new Promise<never>(() => {}))
1165+
1166+
const held = driver.executeTool('chat-test', 'browser_snapshot', {}, 'held-running')
1167+
await vi.advanceTimersByTimeAsync(0)
1168+
expect(driver.isBrowserToolRunning()).toBe(true)
1169+
1170+
driver.cancelTool('chat-test', 'held-running')
1171+
await held
1172+
expect(driver.isBrowserToolRunning()).toBe(false)
1173+
} finally {
1174+
vi.useRealTimers()
1175+
}
1176+
})
1177+
11531178
it('sanitizes hostile tab titles before returning them across the tool boundary', async () => {
11541179
await driver.executeTool('chat-test', 'browser_open_tab', {})
11551180
const contents = session.requireTab().view.webContents

‎apps/desktop/src/main/browser-agent/driver.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,11 @@ function releaseBrowserToolAdmission(state: DriverScopeState, admission: symbol)
457457
activeBrowserToolAdmissions.delete(admission)
458458
}
459459

460+
/** Whether any chat has a browser action queued or running in this process. */
461+
export function isBrowserToolRunning(): boolean {
462+
return activeBrowserToolAdmissions.size > 0 || pendingBrowserToolQueueBoundaries.size > 0
463+
}
464+
460465
function retireDriverScopeState(state: DriverScopeState): void {
461466
state.disposed = true
462467
state.toolQueueCancellationEpoch++

‎apps/desktop/src/main/config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ export interface DesktopSettings {
9292
windowBounds?: WindowBounds
9393
zoomLevel?: number
9494
lastRoute?: string
95+
/** Routes of the other app windows open when an update relaunch was committed. */
96+
updateRelaunchRoutes?: string[]
9597
themeBackground?: 'dark' | 'light'
9698
blockThirdPartyAnalytics?: boolean
9799
trayEnabled?: boolean
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { WebContents } from 'electron'
2+
import { describe, expect, it, vi } from 'vitest'
3+
import { countWindowsWithUnsavedChanges } from '@/main/in-flight-work'
4+
5+
vi.mock('electron', () => import('@/test/electron-mock'))
6+
7+
function windowAnswering(answer: () => Promise<unknown>, state = { destroyed: false }) {
8+
return {
9+
isDestroyed: () => state.destroyed,
10+
isCrashed: () => false,
11+
executeJavaScript: vi.fn(answer),
12+
} as unknown as WebContents
13+
}
14+
15+
describe('countWindowsWithUnsavedChanges', () => {
16+
it('never reports a window it could not ask as clean', async () => {
17+
const counts = await countWindowsWithUnsavedChanges([
18+
windowAnswering(() => Promise.resolve(true)),
19+
windowAnswering(() => Promise.resolve(false)),
20+
windowAnswering(() => Promise.reject(new Error('Script failed to execute'))),
21+
windowAnswering(() => Promise.resolve(true), { destroyed: true }),
22+
])
23+
24+
expect(counts).toEqual({ unsavedWindowCount: 1, unresponsiveWindowCount: 1 })
25+
})
26+
})

0 commit comments

Comments
 (0)