Skip to content

Commit 4fda09e

Browse files
committed
fix(desktop): yield the page only for background calls that act on it
- Only a call from the background executor yields the page to the user. A call from the chat view the user is watching acts exactly as before: there the user steers the agent directly. - A background call that only reads the page (snapshot, read text, list tabs) never waits for the user, since it cannot collide with their input. - Which browser tools only observe the page now lives in @sim/browser-protocol, shared by the chat view's replay policy and the driver. - The prevent-sleep switch shows only when Sim runs chats on this device in the background. - Tests: a chat-view call acts at once under user input; a background call waits, and a background read does not; a user who starts working mid-action stops it before its next input; sign-out releases the sleep blocker while a call is still running.
1 parent b6f2953 commit 4fda09e

7 files changed

Lines changed: 226 additions & 44 deletions

File tree

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,102 @@ describe('executeTool', () => {
7979
driver = freshDriver()
8080
})
8181

82+
/** The user presses a key in the page the agent drives, as the browser reports it. */
83+
function typeInAgentPage(contents: WebContents) {
84+
const calls: ReadonlyArray<readonly unknown[]> = vi.mocked(contents.on).mock.calls
85+
const listener = calls.find(([event]) => event === 'before-input-event')?.[1]
86+
if (typeof listener !== 'function') throw new Error('No input listener on the agent page')
87+
listener({}, { type: 'keyDown', isAutoRepeat: false })
88+
}
89+
90+
it('lets a call from the chat view act at once while the user works in the page', async () => {
91+
await driver.executeTool('chat-test', 'browser_open_tab', {})
92+
typeInAgentPage(session.requireTab().view.webContents)
93+
expect(session.msSinceUserIntervention()).not.toBeNull()
94+
vi.useFakeTimers()
95+
try {
96+
let settled = false
97+
const opening = driver.executeTool('chat-test', 'browser_open_tab', {}).then((result) => {
98+
settled = true
99+
return result
100+
})
101+
await vi.advanceTimersByTimeAsync(100)
102+
103+
expect(settled).toBe(true)
104+
expect((await opening).ok).toBe(true)
105+
} finally {
106+
vi.useRealTimers()
107+
}
108+
})
109+
110+
it('makes a background call wait until the user leaves the page alone', async () => {
111+
await driver.executeTool('chat-test', 'browser_open_tab', {})
112+
typeInAgentPage(session.requireTab().view.webContents)
113+
vi.useFakeTimers()
114+
try {
115+
let settled = false
116+
const opening = driver
117+
.executeTool('chat-test', 'browser_open_tab', {}, 'background-1', undefined, {
118+
background: true,
119+
})
120+
.then((result) => {
121+
settled = true
122+
return result
123+
})
124+
await vi.advanceTimersByTimeAsync(1_000)
125+
expect(settled).toBe(false)
126+
127+
await vi.advanceTimersByTimeAsync(4_000)
128+
expect((await opening).ok).toBe(true)
129+
} finally {
130+
vi.useRealTimers()
131+
}
132+
})
133+
134+
it('stops a background action before its next input once the user starts working mid-action', async () => {
135+
await driver.executeTool('chat-test', 'browser_open_tab', {})
136+
const contents = session.requireTab().view.webContents
137+
// The user presses a key while the action reads the page before acting on it.
138+
const probe = vi.mocked(contents.executeJavaScript).getMockImplementation()
139+
vi.mocked(contents.executeJavaScript).mockImplementation(async (...args) => {
140+
typeInAgentPage(contents)
141+
return probe ? probe(...args) : undefined
142+
})
143+
144+
const result = await driver.executeTool(
145+
'chat-test',
146+
'browser_press_key',
147+
{ key: 'Enter' },
148+
'background-key',
149+
undefined,
150+
{ background: true }
151+
)
152+
153+
expect(result.ok).toBe(false)
154+
expect(result.error).toContain('the user started working in this page')
155+
})
156+
157+
it('never makes a background read wait for the user', async () => {
158+
await driver.executeTool('chat-test', 'browser_open_tab', {})
159+
typeInAgentPage(session.requireTab().view.webContents)
160+
vi.useFakeTimers()
161+
try {
162+
let settled = false
163+
void driver
164+
.executeTool('chat-test', 'browser_list_tabs', {}, 'background-read', undefined, {
165+
background: true,
166+
})
167+
.then(() => {
168+
settled = true
169+
})
170+
await vi.advanceTimersByTimeAsync(100)
171+
172+
expect(settled).toBe(true)
173+
} finally {
174+
vi.useRealTimers()
175+
}
176+
})
177+
82178
it('returns ok:false instead of throwing for tool-level failures', async () => {
83179
// No session exists, so any page-dependent tool fails with guidance.
84180
const result = await driver.executeTool('chat-test', 'browser_click', { elementId: 1 })

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import {
2020
BROWSER_DATA_KINDS,
2121
BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS,
22+
BROWSER_TOOL_OBSERVES_ONLY,
2223
BROWSER_TOOL_QUEUE_WAIT_TIMEOUT_MS,
2324
BROWSER_UPLOAD_MAX_FILES,
2425
type BrowserDataKind,
@@ -5135,12 +5136,23 @@ async function yieldToUser(
51355136
logger.info('Browser automation resumed after the user stopped', { toolCallId })
51365137
}
51375138

5139+
/** How a browser call reached the driver. */
5140+
interface BrowserToolExecutionOptions {
5141+
/**
5142+
* Run by the background executor while the user may be working in the same page, so it yields
5143+
* the page to them. A call from the chat view the user is watching never does: there the user
5144+
* steers the agent directly, as they always have.
5145+
*/
5146+
background?: boolean
5147+
}
5148+
51385149
export async function executeTool(
51395150
scopeId: string,
51405151
tool: BrowserToolName,
51415152
params: Record<string, unknown>,
51425153
toolCallId?: string,
5143-
authorizationBoundary?: BrowserToolQueueBoundary
5154+
authorizationBoundary?: BrowserToolQueueBoundary,
5155+
options: BrowserToolExecutionOptions = {}
51445156
): Promise<{ ok: boolean; result?: unknown; error?: string }> {
51455157
const resolvedScopeId = resolveDriverScopeId(scopeId)
51465158
if (authorizationBoundary) {
@@ -5231,7 +5243,11 @@ export async function executeTool(
52315243
session.setAutomationActive(true)
52325244
}
52335245
try {
5234-
const yieldsToUser = tool !== 'browser_request_takeover'
5246+
// Only a background call acting on the page yields; reading it cannot collide with the user.
5247+
const yieldsToUser =
5248+
options.background === true &&
5249+
tool !== 'browser_request_takeover' &&
5250+
!BROWSER_TOOL_OBSERVES_ONLY[tool]
52355251
const watchdogMs = browserToolWatchdogMs(tool, params)
52365252
if (yieldsToUser && isUserWorkingInPage()) {
52375253
await yieldToUser(

‎apps/desktop/src/main/desktop-executor/service.test.ts‎

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { createDesktopExecutorService, deviceName } from '@/main/desktop-executo
1212
/** Sim's device routes, with registration answers held until the test releases them. */
1313
function fakeSim(protocolVersion = 1) {
1414
const requests: string[] = []
15+
/** Calls the inbox offers; a claim answers for the first one. */
16+
const offered: string[] = []
1517
/**
1618
* Answers to pending registrations: enabled or not, an HTTP status Sim fails with, or
1719
* `'offline'` for a request that never reached Sim.
@@ -37,25 +39,60 @@ function fakeSim(protocolVersion = 1) {
3739
reconcileMs: 10_000,
3840
})
3941
}
40-
if (path === '/api/desktop/inbox') return Response.json({ items: [] })
42+
if (path === '/api/desktop/inbox') {
43+
return Response.json({
44+
items: offered.map((toolCallId) => ({
45+
kind: 'call',
46+
toolCallId,
47+
toolName: 'terminal',
48+
chatId: 'chat-a',
49+
workspaceId: 'ws-1',
50+
createdAt: new Date().toISOString(),
51+
})),
52+
})
53+
}
54+
if (path === '/api/desktop/tool/claim') {
55+
const toolCallId = offered.shift()
56+
if (!toolCallId) return Response.json({ error: 'gone' }, { status: 404 })
57+
return Response.json({
58+
toolName: 'terminal',
59+
args: { operation: 'run', args: { command: 'sleep 600' } },
60+
chatId: 'chat-a',
61+
workspaceId: 'ws-1',
62+
executionToken: `token-${toolCallId}`,
63+
})
64+
}
65+
if (path === '/api/desktop/tool/lease') return Response.json({ renewed: true })
4166
return new Promise<Response>((_resolve, reject) =>
4267
init.signal?.addEventListener('abort', () => reject(new Error('aborted')))
4368
)
4469
})
45-
return { fetch, requests, registrations }
70+
return { fetch, requests, registrations, offered }
4671
}
4772

4873
async function service(protocolVersion = 1, userDataPath?: string) {
4974
const sim = fakeSim(protocolVersion)
75+
/** What the sleep blocker was told, in order. */
76+
const busy: boolean[] = []
5077
const desktopExecutor = createDesktopExecutorService({
5178
userDataPath: userDataPath ?? (await mkdtemp(join(tmpdir(), 'sim-executor-service-'))),
5279
origin: () => 'https://sim.test',
5380
appSession: () => ({ fetch: sim.fetch }),
5481
preferences: () => ({ browserEnabled: true, terminalEnabled: true }),
5582
accountDataAvailable: () => true,
56-
runner: { run: vi.fn(), cancel: vi.fn() },
83+
// A command that runs until it is stopped.
84+
runner: {
85+
run: (_call, signal) =>
86+
new Promise((resolve) =>
87+
signal.addEventListener('abort', () =>
88+
resolve({ status: 'cancelled', message: 'Stopped.' })
89+
)
90+
),
91+
cancel: async () => {},
92+
},
93+
onBusyChange: (value) => busy.push(value),
5794
})
58-
return { sim, desktopExecutor }
95+
return { sim, desktopExecutor, busy }
5996
}
6097

6198
describe('desktop executor registration', () => {
@@ -177,6 +214,19 @@ describe('desktop executor registration', () => {
177214

178215
expect(sim.requests.filter((request) => request.includes('/api/desktop/devices'))).toEqual([])
179216
})
217+
218+
it('releases the sleep blocker at sign-out while a call is still running', async () => {
219+
const { sim, desktopExecutor, busy } = await service()
220+
sim.offered.push('call-1')
221+
desktopExecutor.start()
222+
await vi.waitFor(() => expect(sim.registrations).toHaveLength(1))
223+
sim.registrations[0]?.(true)
224+
await vi.waitFor(() => expect(busy).toEqual([true]))
225+
226+
await desktopExecutor.signOut()
227+
228+
expect(busy).toEqual([true, false])
229+
})
180230
})
181231

182232
describe('device name', () => {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,10 @@ function main(): void {
605605
preferences: () => desktopSettings.getPreferences(),
606606
accountDataAvailable,
607607
browser: {
608-
executeTool: executeAgentBrowserTool,
608+
executeTool: (scopeId, tool, params, toolCallId) =>
609+
executeAgentBrowserTool(scopeId, tool, params, toolCallId, undefined, {
610+
background: true,
611+
}),
609612
cancelTool: cancelAgentBrowserTool,
610613
hasSession: hasBrowserScopeSession,
611614
restoreScope: restoreAgentBrowserScope,

‎apps/sim/app/workspace/[workspaceId]/settings/components/desktop/desktop.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export function Desktop() {
3333
const workspaceId = params.workspaceId as string
3434
const [preferences, setPreferences] = useState<DesktopPreferences | null>(null)
3535
const [pendingPreference, setPendingPreference] = useState<DesktopPreferenceKey | null>(null)
36+
/** Whether Sim runs chats on this device in the background; only then can sleep matter. */
37+
const [runsInBackground, setRunsInBackground] = useState(false)
3638
const updateState = useDesktopUpdateState()
3739
const shellVersion = getDesktopShellVersion()
3840

@@ -46,6 +48,10 @@ export function Desktop() {
4648
.getPreferences()
4749
.then(setPreferences)
4850
.catch(() => toast.error('Could not load desktop settings'))
51+
void bridge.desktopExecutor
52+
?.getDevice()
53+
.then((device) => setRunsInBackground(device !== null))
54+
.catch(() => setRunsInBackground(false))
4955
}, [router, workspaceId])
5056

5157
const updatePreference = async (key: DesktopPreferenceKey, value: boolean) => {
@@ -73,7 +79,8 @@ export function Desktop() {
7379

7480
const notificationsDisabled =
7581
!preferences.notificationsEnabled || pendingPreference === 'notificationsEnabled'
76-
const supportsPreventSleep = Boolean(getDesktopBridge()?.settings.setPreventSleepWhileRunning)
82+
const supportsPreventSleep =
83+
runsInBackground && Boolean(getDesktopBridge()?.settings.setPreventSleepWhileRunning)
7784

7885
return (
7986
<SettingsPanel>

‎apps/sim/lib/mothership/tools/client/browser-tool-execution.ts‎

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
* browser and reports the outcome via the confirm endpoint, which wakes the
88
* server-side waiter.
99
*/
10-
import { type BrowserToolName, browserToolRendererTimeoutMs } from '@sim/browser-protocol'
10+
import {
11+
BROWSER_TOOL_OBSERVES_ONLY,
12+
type BrowserToolName,
13+
browserToolRendererTimeoutMs,
14+
} from '@sim/browser-protocol'
1115
import {
1216
browserSessionClosedCompletion,
1317
browserToolCompletion,
@@ -44,41 +48,7 @@ const logger = createLogger('CopilotBrowserToolExecution')
4448
* reload cannot cause a page or external side effect. Every stateful current
4549
* tool and the retired takeover flow remain fail-closed.
4650
*/
47-
const OBSERVATION_ONLY_BROWSER_TOOLS = {
48-
browser_navigate: false,
49-
browser_open_url: false,
50-
browser_go_back: false,
51-
browser_go_forward: false,
52-
browser_reload: false,
53-
browser_open_tab: false,
54-
browser_switch_tab: false,
55-
browser_close_tab: false,
56-
browser_list_tabs: true,
57-
browser_list_sessions: true,
58-
browser_list_downloads: true,
59-
browser_save_download: false,
60-
browser_wait_for: true,
61-
browser_snapshot: true,
62-
browser_find: true,
63-
browser_read_text: true,
64-
browser_screenshot: true,
65-
browser_extract: true,
66-
browser_click: false,
67-
browser_click_at: false,
68-
browser_type: false,
69-
browser_fill_form: false,
70-
browser_batch: false,
71-
browser_insert_text: false,
72-
browser_press_key: false,
73-
browser_scroll: false,
74-
browser_select_option: false,
75-
browser_set_checked: false,
76-
browser_upload_file: false,
77-
browser_hover: false,
78-
browser_drag: false,
79-
browser_zoom: false,
80-
browser_request_takeover: false,
81-
} as const satisfies Readonly<Record<BrowserToolName, boolean>>
51+
const OBSERVATION_ONLY_BROWSER_TOOLS = BROWSER_TOOL_OBSERVES_ONLY
8252

8353
/** Tool events older than this are replays, not live instructions — never act on them. */
8454
const MAX_EVENT_AGE_MS = 120_000

‎packages/browser-protocol/src/index.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,46 @@ export function isCurrentBrowserToolName(name: string): name is CurrentBrowserTo
150150
return CURRENT_BROWSER_TOOL_NAME_SET.has(name)
151151
}
152152

153+
/**
154+
* Which browser tools only observe the page (read it, list it, wait on it) and never act on it.
155+
* Repeating one cannot cause a side effect, and one can never collide with what the user is doing.
156+
*/
157+
export const BROWSER_TOOL_OBSERVES_ONLY = {
158+
browser_navigate: false,
159+
browser_open_url: false,
160+
browser_go_back: false,
161+
browser_go_forward: false,
162+
browser_reload: false,
163+
browser_open_tab: false,
164+
browser_switch_tab: false,
165+
browser_close_tab: false,
166+
browser_list_tabs: true,
167+
browser_list_sessions: true,
168+
browser_list_downloads: true,
169+
browser_save_download: false,
170+
browser_wait_for: true,
171+
browser_snapshot: true,
172+
browser_find: true,
173+
browser_read_text: true,
174+
browser_screenshot: true,
175+
browser_extract: true,
176+
browser_click: false,
177+
browser_click_at: false,
178+
browser_type: false,
179+
browser_fill_form: false,
180+
browser_batch: false,
181+
browser_insert_text: false,
182+
browser_press_key: false,
183+
browser_scroll: false,
184+
browser_select_option: false,
185+
browser_set_checked: false,
186+
browser_upload_file: false,
187+
browser_hover: false,
188+
browser_drag: false,
189+
browser_zoom: false,
190+
browser_request_takeover: false,
191+
} as const satisfies Readonly<Record<BrowserToolName, boolean>>
192+
153193
export function isBrowserTheme(value: unknown): value is BrowserTheme {
154194
return typeof value === 'string' && BROWSER_THEME_SET.has(value)
155195
}

0 commit comments

Comments
 (0)