Skip to content

Commit 0a5f255

Browse files
committed
fix(desktop): never fail an awake device's call as offline because a presence write was lost
Presence writes are best effort, so a pull whose write failed left the key missing and the next read took the device for offline, failing its pending call early. A call now fails early as offline only when presence is absent and the device has not pulled for longer than the presence TTL plus the interval at which a pull writes last_seen_at; otherwise its pickup window decides. The audit test no longer depends on the insertion order of audit writes, which are not awaited.
1 parent b611a38 commit 0a5f255

5 files changed

Lines changed: 59 additions & 9 deletions

File tree

‎apps/sim/lib/desktop/application/executor.integration.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,6 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
681681
})
682682
.from(auditLog)
683683
.where(eq(auditLog.resourceId, desktop.deviceId))
684-
.orderBy(auditLog.createdAt)
685684
await expect.poll(async () => (await audited()).length).toBe(2)
686685
const entry = (action: string) => ({
687686
action,
@@ -695,10 +694,13 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
695694
chatId: run.chatId,
696695
}),
697696
})
698-
expect(await audited()).toEqual([
699-
entry('desktop_tool_call.claimed'),
700-
entry('desktop_tool_call.completed'),
701-
])
697+
/** Audit writes are not awaited, so their insertion order is not the call order. */
698+
expect(await audited()).toEqual(
699+
expect.arrayContaining([
700+
entry('desktop_tool_call.claimed'),
701+
entry('desktop_tool_call.completed'),
702+
])
703+
)
702704
expect(JSON.stringify(await audited())).not.toContain('Clicked Save')
703705
})
704706

‎apps/sim/lib/desktop/executor/bound-turn.integration.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,11 @@ describe.runIf(Boolean(redisUrl))("a turn bound to a desktop's background execut
438438
async () => {
439439
const desktop = await signedInDesktop()
440440
const run = await boundRun(desktop)
441+
/** Its last pull was minutes ago, and its presence lapsed long since. */
442+
await db
443+
.update(desktopDevices)
444+
.set({ lastSeenAt: sql`now() - interval '10 minutes'` })
445+
.where(eq(desktopDevices.id, desktop.deviceId))
441446

442447
const startedAt = Date.now()
443448
const { toolCallId, answer, context } = await agentCalls(run, 'terminal', {
@@ -622,6 +627,34 @@ describe.runIf(Boolean(redisUrl))("a turn bound to a desktop's background execut
622627
TURN_WAIT_MS
623628
)
624629

630+
it(
631+
'never fails an awake device’s call early because its presence write was lost',
632+
async () => {
633+
const desktop = await signedInDesktop()
634+
const run = await boundRun(desktop)
635+
const redis = getRedisClient()
636+
if (!redis) throw new Error('Redis is required')
637+
const writes = vi.spyOn(redis, 'set').mockRejectedValue(new Error('Redis unavailable'))
638+
try {
639+
/** The pull succeeds, but its presence write fails: the key reads as absent. */
640+
await desktop.pull()
641+
} finally {
642+
writes.mockRestore()
643+
}
644+
645+
const { toolCallId, answer, context } = await agentCalls(run, 'browser_click', { ref: 'e1' })
646+
await offered(toolCallId)
647+
await sleep(POLL_SETTLES_MS)
648+
expect((await storedCall(toolCallId)).status).toBe('pending')
649+
const { executionToken } = await desktop.claim(toolCallId)
650+
await desktop.complete(toolCallId, executionToken, { clicked: true })
651+
await answer
652+
653+
expect(resultOf(context, toolCallId)).toMatchObject({ success: true })
654+
},
655+
TURN_WAIT_MS
656+
)
657+
625658
it(
626659
'enforces the pickup window when presence cannot be read, without failing a call early',
627660
async () => {

‎apps/sim/lib/desktop/executor/constants.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export const DESKTOP_INBOX_RECONCILE_MS = 10_000
1818
*/
1919
export const DESKTOP_PRESENCE_TTL_SECONDS = 45
2020

21+
/** How often an inbox pull writes the device's `last_seen_at`; at most once per interval. */
22+
export const DESKTOP_LAST_SEEN_WRITE_SECONDS = 60
23+
2124
/**
2225
* Only runs started this recently are scanned for inbox items. A turn, including a terminal
2326
* handoff, ends well inside it, and the bound keeps the inbox query on a narrow index range.

‎apps/sim/lib/desktop/executor/repository.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ import {
1818
type SQL,
1919
sql,
2020
} from 'drizzle-orm'
21-
import { DESKTOP_INBOX_HORIZON_HOURS } from '@/lib/desktop/executor/constants'
21+
import {
22+
DESKTOP_INBOX_HORIZON_HOURS,
23+
DESKTOP_LAST_SEEN_WRITE_SECONDS,
24+
DESKTOP_PRESENCE_TTL_SECONDS,
25+
} from '@/lib/desktop/executor/constants'
2226
import {
2327
ASYNC_TOOL_STATUS,
2428
DESKTOP_TOOL_CLAIM_OWNER,
@@ -142,7 +146,7 @@ export async function touchDesktopDevice(deviceId: string): Promise<void> {
142146
.where(
143147
and(
144148
eq(desktopDevices.id, deviceId),
145-
sql`${desktopDevices.lastSeenAt} < now() - interval '1 minute'`
149+
sql`${desktopDevices.lastSeenAt} < now() - ${DESKTOP_LAST_SEEN_WRITE_SECONDS} * interval '1 second'`
146150
)
147151
)
148152
}
@@ -333,9 +337,16 @@ export async function getDesktopToolCallDeadlines(toolCallId: string) {
333337
result: copilotAsyncToolCalls.result,
334338
pickupOverdue: sql<boolean>`coalesce(${pickupOverdueAt(sql`clock_timestamp()`)}, false)`,
335339
leaseLapsed: sql<boolean>`coalesce(${copilotAsyncToolCalls.executionLeaseExpiresAt} <= clock_timestamp(), false)`,
340+
/**
341+
* Whether the device's pulls show it awake, independently of Redis presence: a pull writes
342+
* `last_seen_at` at most once per write interval, so a device seen within the presence TTL
343+
* plus that interval may still be pulling even when its presence key is missing.
344+
*/
345+
recentlySeen: sql<boolean>`coalesce(${desktopDevices.lastSeenAt} > clock_timestamp() - ${DESKTOP_PRESENCE_TTL_SECONDS + DESKTOP_LAST_SEEN_WRITE_SECONDS} * interval '1 second', false)`,
336346
})
337347
.from(copilotAsyncToolCalls)
338348
.innerJoin(copilotRuns, eq(copilotRuns.id, copilotAsyncToolCalls.runId))
349+
.leftJoin(desktopDevices, eq(desktopDevices.id, copilotRuns.desktopDeviceId))
339350
.where(
340351
and(eq(copilotAsyncToolCalls.toolCallId, toolCallId), isNotNull(copilotRuns.desktopDeviceId))
341352
)

‎apps/sim/lib/mothership/request/tools/desktop-wait.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,9 @@ async function settleOverdueDesktopToolCall(toolCallId: string): Promise<boolean
217217
if (!call) return false
218218
if (call.status === ASYNC_TOOL_STATUS.pending) {
219219
const present = await readDesktopPresence(call.deviceId)
220-
// Before its pickup window closes, a call fails early only when its device is known to be away.
221-
if (!call.pickupOverdue && present !== false) return false
220+
// Before its pickup window closes, a call fails early only when its device is known to be away:
221+
// no presence, and no pull recent enough that a lost presence write could explain the gap.
222+
if (!call.pickupOverdue && (present !== false || call.recentlySeen)) return false
222223
const reason = present === false ? 'offline' : 'not_responding'
223224
const outcome = await settleBoundDesktopToolCall(call, desktopToolNotStarted(reason), {
224225
kind: 'pending',

0 commit comments

Comments
 (0)