fix(desktop): pre-merge review fixes for the bound executor

- Terminal approvals carry their command at args.args.command, so the
  inbox summary read nothing; it now reads the terminal call's real shape,
  and the tests use it.
- A turn budget shorter than the pickup window left a bound call pending; an
  unclaimed call is now settled as never started through the pending-only
  path, as the chat-view wait does.
- Ringing the device is best effort: a publish that throws is logged and can
  no longer break Stop or any other caller.
- Running claimed calls are no longer fetched for the inbox, and offered or
  awaiting calls and cancel items are fetched with separate caps, so neither
  can starve the other.
- No pickup window runs while the user decides: an offer refuses a call still
  awaiting approval, and recording the decision clears any pickup deadline,
  so an allowed call's window starts when it is offered after the answer.
- The per-user rate limit refills every second instead of in one lump a
  minute, so a device that spent its burst can still renew its leases.
- The integration suites delete the audit rows they create.

Author: waleedlatif1

apps/sim/lib/api/server/routes/desktop-executor.ts

@@ -26,9 +26,11 @@ export const desktopExecutorErrorPolicy = extendInternalErrorPolicy(
 
 /**
  * One busy device renews a lease per running call every 20 s and pulls its inbox on every
- * doorbell, so the bucket allows a sustained 10 requests a second per user.
+ * doorbell, so the bucket allows a sustained 10 requests a second per user. It refills every
+ * second rather than once a minute: a device that spent its burst must still renew its leases
+ * well before they lapse.
  */
 export const desktopExecutorRateLimit = internalRateLimits.user({
   bucketName: 'desktop-executor',
-  config: { maxTokens: 600, refillRate: 600, refillIntervalMs: 60_000 },
+  config: { maxTokens: 600, refillRate: 10, refillIntervalMs: 1_000 },
 })

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

@@ -244,8 +244,10 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
 
   afterAll(async () => {
     if (chatIds.length) await db.delete(copilotChats).where(inArray(copilotChats.id, chatIds))
-    if (deviceIds.length)
+    if (deviceIds.length) {
+      await db.delete(auditLog).where(inArray(auditLog.resourceId, deviceIds))
       await db.delete(desktopDevices).where(inArray(desktopDevices.id, deviceIds))
+    }
     const workspaces = [...workspaceIds.values()]
     if (workspaces.length) await db.delete(workspace).where(inArray(workspace.id, workspaces))
     if (userIds.length) await db.delete(user).where(inArray(user.id, userIds))
@@ -481,7 +483,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
       const toolCallId = await pendingCall(
         run.runId,
         'terminal',
-        { operation: 'run', command: 'npm test' },
+        { operation: 'run', args: { command: 'npm test' } },
         { awaitingApproval: true }
       )
       expect((await inbox(desktop)).items).toEqual([
@@ -512,7 +514,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
       expect((await inbox(desktop)).items).toMatchObject([{ kind: 'call', toolCallId }])
       await expect(claim(desktop.principal, desktop.deviceId, toolCallId)).resolves.toMatchObject({
         toolName: 'terminal',
-        args: { operation: 'run', command: 'npm test' },
+        args: { operation: 'run', args: { command: 'npm test' } },
       })
     })
 
@@ -521,7 +523,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
       const run = await boundRun(desktop)
       const toolCallId = await pendingCall(run.runId, 'terminal', {
         operation: 'run',
-        command: 'ls',
+        args: { command: 'ls' },
       })
       expect((await inbox(desktop)).items).toMatchObject([{ kind: 'call', toolCallId }])
     })
@@ -532,7 +534,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
       const toolCallId = await pendingCall(
         run.runId,
         'terminal',
-        { operation: 'run', command: 'rm -rf build' },
+        { operation: 'run', args: { command: 'rm -rf build' } },
         { awaitingApproval: true }
       )
       await db
@@ -820,6 +822,49 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
       }
     })
 
+    it('lists new work however many calls the device is already running', async () => {
+      const desktop = await signedInDesktop()
+      const run = await boundRun(desktop)
+      await db.insert(copilotAsyncToolCalls).values(
+        Array.from({ length: 501 }, () => ({
+          runId: run.runId,
+          toolCallId: generateId(),
+          toolName: 'browser_click',
+          args: { ref: 'e1' },
+          status: 'running' as const,
+          claimedBy: 'desktop-browser',
+          executionOwnerToken: generateId(),
+          executionLeaseExpiresAt: sql`now() + interval '1 minute'`,
+          createdAt: new Date(Date.now() - 60_000),
+        }))
+      )
+      /** A call Sim settled without the device's result, still to be acknowledged. */
+      const stopped = generateId()
+      await db.insert(copilotAsyncToolCalls).values({
+        runId: run.runId,
+        toolCallId: stopped,
+        toolName: 'browser_click',
+        args: { ref: 'e2' },
+        status: 'cancelled',
+        executionOwnerToken: generateId(),
+        executionRevokedAt: new Date(),
+        createdAt: new Date(Date.now() - 30_000),
+      })
+      const offered = await pendingCall(run.runId)
+      const gated = await pendingCall(
+        run.runId,
+        'terminal',
+        { operation: 'run', args: { command: 'make' } },
+        { awaitingApproval: true }
+      )
+
+      expect((await inbox(desktop)).items).toMatchObject([
+        { kind: 'cancel', toolCallId: stopped },
+        { kind: 'call', toolCallId: offered },
+        { kind: 'approval_needed', toolCallId: gated },
+      ])
+    })
+
     it('lists pending calls in persistence order even when their doorbell was never heard', async () => {
       const desktop = await signedInDesktop()
       const run = await boundRun(desktop)

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

@@ -39,6 +39,7 @@ vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
 import type { SessionPrincipal } from '@sim/auth/principal'
 import { db } from '@sim/db'
 import {
+  auditLog,
   copilotAsyncToolCalls,
   copilotChats,
   copilotRuns,
@@ -363,8 +364,10 @@ describe.runIf(Boolean(redisUrl))("a turn bound to a desktop's background execut
 
   afterAll(async () => {
     if (chatIds.length > 0) await db.delete(copilotChats).where(inArray(copilotChats.id, chatIds))
-    if (deviceIds.length > 0)
+    if (deviceIds.length > 0) {
+      await db.delete(auditLog).where(inArray(auditLog.resourceId, deviceIds))
       await db.delete(desktopDevices).where(inArray(desktopDevices.id, deviceIds))
+    }
     await db.delete(workspace).where(eq(workspace.id, workspaceId))
     await db.delete(user).where(eq(user.id, userId))
   })
@@ -545,7 +548,9 @@ describe.runIf(Boolean(redisUrl))("a turn bound to a desktop's background execut
         (rings) => rings.length > 0
       )
       expect(desktop.rings).toEqual(['approval'])
-      expect((await desktop.pull()).items).toMatchObject([{ kind: 'approval_needed', toolCallId }])
+      expect((await desktop.pull()).items).toMatchObject([
+        { kind: 'approval_needed', toolCallId, summary: 'rm -rf build' },
+      ])
 
       const decided = await post(toolPermissionPOST, '/api/copilot/tool-permission', {
         decisions: [{ toolCallId, decision: 'allow' }],
@@ -566,6 +571,48 @@ describe.runIf(Boolean(redisUrl))("a turn bound to a desktop's background execut
     TURN_WAIT_MS
   )
 
+  it(
+    'runs no pickup window while the user decides, and offers the call afresh once allowed',
+    async () => {
+      const desktop = await signedInDesktop()
+      const run = await boundRun(desktop)
+      await desktop.pull()
+
+      const { toolCallId, answer, context } = await agentCalls(
+        run,
+        'terminal',
+        { operation: 'run', args: { command: 'make release' } },
+        { gated: true }
+      )
+      /** The user takes longer than a pickup window to answer, with a deadline already on the row. */
+      await db
+        .update(copilotAsyncToolCalls)
+        .set({
+          createdAt: new Date(Date.now() - 600_000),
+          pickupDeadlineAt: sql`now() - interval '5 minutes'`,
+        })
+        .where(eq(copilotAsyncToolCalls.toolCallId, toolCallId))
+      await runCleanupStaleExecutions()
+      expect((await storedCall(toolCallId)).status).toBe('pending')
+      expect((await desktop.pull()).items).toMatchObject([{ kind: 'approval_needed', toolCallId }])
+
+      const decided = await post(toolPermissionPOST, '/api/copilot/tool-permission', {
+        decisions: [{ toolCallId, decision: 'allow' }],
+      })
+      expect(decided.status).toBe(200)
+      await until(
+        () => storedCall(toolCallId),
+        (row) => row.pickupDeadlineAt !== null && row.pickupDeadlineAt.getTime() > Date.now()
+      )
+      const { executionToken } = await desktop.claim(toolCallId)
+      await desktop.complete(toolCallId, executionToken, { output: 'released' })
+      await answer
+
+      expect(resultOf(context, toolCallId)).toMatchObject({ success: true })
+    },
+    TURN_WAIT_MS
+  )
+
   it(
     'ends a running call on Stop without the device, and tells the device to cancel it',
     async () => {

apps/sim/lib/desktop/executor/doorbell.test.ts

@@ -0,0 +1,19 @@
+import { describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/events/pubsub', () => ({
+  createPubSubChannel: () => ({
+    publish: () => {
+      throw new Error('Redis connection closed')
+    },
+    subscribe: vi.fn(),
+    dispose: vi.fn(),
+  }),
+}))
+
+import { ringDesktopInbox } from '@/lib/desktop/executor/doorbell'
+
+describe('desktop inbox doorbell', () => {
+  it('never fails its caller when the ring cannot be published', () => {
+    expect(() => ringDesktopInbox('device-1', 'cancel')).not.toThrow()
+  })
+})

apps/sim/lib/desktop/executor/doorbell.ts

@@ -1,5 +1,9 @@
+import { createLogger } from '@sim/logger'
+import { toError } from '@sim/utils/errors'
 import { createPubSubChannel, type PubSubChannel } from '@/lib/events/pubsub'
 
+const logger = createLogger('DesktopInboxDoorbell')
+
 /** Why a device should re-read its inbox. The event is only a hint; the inbox is the record. */
 export type DesktopInboxChangeReason = 'call' | 'approval' | 'cancel'
 
@@ -22,9 +26,21 @@ function channel(): PubSubChannel<DesktopInboxDoorbell> {
   return scope._desktopInboxDoorbell
 }
 
-/** Tells every pod serving this device's inbox stream that the inbox changed. */
+/**
+ * Tells every pod serving this device's inbox stream that the inbox changed. Best effort: the
+ * ring only hurries the device's next pull, and its periodic reconcile reads the same inbox, so a
+ * failed ring is logged and never fails the caller (Stop, a decision, an offer).
+ */
 export function ringDesktopInbox(deviceId: string, reason: DesktopInboxChangeReason): void {
-  channel().publish({ deviceId, reason })
+  try {
+    channel().publish({ deviceId, reason })
+  } catch (error) {
+    logger.warn('Could not ring the desktop inbox', {
+      deviceId,
+      reason,
+      error: toError(error).message,
+    })
+  }
 }
 
 /** Subscribes to one device's doorbell; returns the unsubscribe. */

apps/sim/lib/desktop/executor/inbox.ts

@@ -27,9 +27,11 @@ export type DesktopInboxEntry =
     }
   | { kind: 'cancel'; toolCallId: string }
 
+/** The command a gated terminal run would execute: `{ operation: 'run', args: { command } }`. */
 function summarize(args: Record<string, unknown>): string | null {
-  return typeof args.command === 'string' && args.command.trim()
-    ? truncate(args.command.trim(), SUMMARY_MAX_LENGTH)
+  const command = isPlainRecord(args.args) ? args.args.command : undefined
+  return typeof command === 'string' && command.trim()
+    ? truncate(command.trim(), SUMMARY_MAX_LENGTH)
     : null
 }
 

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

@@ -13,6 +13,7 @@ import {
   inArray,
   isNotNull,
   isNull,
+  ne,
   notInArray,
   or,
   type SQL,
@@ -152,55 +153,64 @@ export async function touchDesktopDevice(deviceId: string): Promise<void> {
 }
 
 /**
- * The rows a device's inbox is built from: every unclaimed pending desktop call on its recent open
- * runs, plus every call it claimed and has not yet acknowledged. Pending calls on stopped or ended
- * runs are left out here, so they cannot crowd actionable rows past the limit. Ordered by
- * persistence time, the order the device claims in.
+ * The rows a device's inbox is built from, in two sets with their own limits so neither can crowd
+ * out the other: unclaimed calls on its recent open runs that are offered or waiting for the
+ * user's decision, and calls it claimed that Sim settled without its result and it has not yet
+ * acknowledged (cancel items). A call the device is still running is never listed: it already
+ * holds it. Ordered by persistence time, the order the device claims in.
  */
 export async function listDesktopInboxRows(identity: Omit<DesktopDeviceIdentity, 'sessionId'>) {
-  return db
-    .select({
-      toolCallId: copilotAsyncToolCalls.toolCallId,
-      toolName: copilotAsyncToolCalls.toolName,
-      args: copilotAsyncToolCalls.args,
-      status: copilotAsyncToolCalls.status,
-      permissionRequestedAt: copilotAsyncToolCalls.permissionRequestedAt,
-      permissionDecision: copilotAsyncToolCalls.permissionDecision,
-      claimed: sql<boolean>`${copilotAsyncToolCalls.executionOwnerToken} IS NOT NULL`,
-      createdAt: copilotAsyncToolCalls.createdAt,
-      chatId: copilotRuns.chatId,
-      chatTitle: copilotChats.title,
-      workspaceId: copilotRuns.workspaceId,
-    })
-    .from(copilotRuns)
-    .innerJoin(copilotAsyncToolCalls, eq(copilotAsyncToolCalls.runId, copilotRuns.id))
-    .innerJoin(copilotChats, eq(copilotChats.id, copilotRuns.chatId))
-    .where(
-      and(
-        eq(copilotRuns.desktopDeviceId, identity.deviceId),
-        eq(copilotRuns.userId, identity.userId),
-        sql`${copilotRuns.startedAt} > now() - make_interval(hours => ${DESKTOP_INBOX_HORIZON_HOURS})`,
-        inArray(copilotAsyncToolCalls.toolName, [...DESKTOP_TOOL_CALL_NAMES]),
-        or(
-          and(
-            eq(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.pending),
-            isNull(copilotAsyncToolCalls.executionOwnerToken),
-            or(
-              sql`${copilotAsyncToolCalls.pickupDeadlineAt} > clock_timestamp()`,
-              awaitingPermission
-            ),
-            inArray(copilotRuns.status, LIVE_RUN_STATUSES),
-            isNull(copilotRuns.toolAdmissionClosedAt)
-          ),
-          and(
-            isNotNull(copilotAsyncToolCalls.executionOwnerToken),
-            isNull(copilotAsyncToolCalls.executionSettledAt)
-          )
+  const rowsWhere = (state: SQL | undefined) =>
+    db
+      .select({
+        toolCallId: copilotAsyncToolCalls.toolCallId,
+        toolName: copilotAsyncToolCalls.toolName,
+        args: copilotAsyncToolCalls.args,
+        status: copilotAsyncToolCalls.status,
+        permissionRequestedAt: copilotAsyncToolCalls.permissionRequestedAt,
+        permissionDecision: copilotAsyncToolCalls.permissionDecision,
+        claimed: sql<boolean>`${copilotAsyncToolCalls.executionOwnerToken} IS NOT NULL`,
+        createdAt: copilotAsyncToolCalls.createdAt,
+        chatId: copilotRuns.chatId,
+        chatTitle: copilotChats.title,
+        workspaceId: copilotRuns.workspaceId,
+      })
+      .from(copilotRuns)
+      .innerJoin(copilotAsyncToolCalls, eq(copilotAsyncToolCalls.runId, copilotRuns.id))
+      .innerJoin(copilotChats, eq(copilotChats.id, copilotRuns.chatId))
+      .where(
+        and(
+          eq(copilotRuns.desktopDeviceId, identity.deviceId),
+          eq(copilotRuns.userId, identity.userId),
+          sql`${copilotRuns.startedAt} > now() - make_interval(hours => ${DESKTOP_INBOX_HORIZON_HOURS})`,
+          inArray(copilotAsyncToolCalls.toolName, [...DESKTOP_TOOL_CALL_NAMES]),
+          state
         )
       )
-    )
-    .orderBy(asc(copilotAsyncToolCalls.createdAt), asc(copilotAsyncToolCalls.toolCallId))
-    .limit(INBOX_ROW_LIMIT)
+      .orderBy(asc(copilotAsyncToolCalls.createdAt), asc(copilotAsyncToolCalls.toolCallId))
+      .limit(INBOX_ROW_LIMIT)
+  const [waiting, cancelled] = await Promise.all([
+    rowsWhere(
+      and(
+        eq(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.pending),
+        isNull(copilotAsyncToolCalls.executionOwnerToken),
+        or(sql`${copilotAsyncToolCalls.pickupDeadlineAt} > clock_timestamp()`, awaitingPermission),
+        inArray(copilotRuns.status, LIVE_RUN_STATUSES),
+        isNull(copilotRuns.toolAdmissionClosedAt)
+      )
+    ),
+    rowsWhere(
+      and(
+        isNotNull(copilotAsyncToolCalls.executionOwnerToken),
+        isNull(copilotAsyncToolCalls.executionSettledAt),
+        ne(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.running)
+      )
+    ),
+  ])
+  return [...waiting, ...cancelled].sort(
+    (a, b) =>
+      a.createdAt.getTime() - b.createdAt.getTime() || a.toolCallId.localeCompare(b.toolCallId)
+  )
 }
 
 export type DesktopInboxRow = Awaited<ReturnType<typeof listDesktopInboxRows>>[number]
@@ -295,7 +305,9 @@ export async function getRunDesktopDeviceId(runId: string): Promise<string | nul
 
 /**
  * Offers a pending call on a bound run to its device by opening its pickup window. Only an
- * unclaimed call can be offered, and only once, so a re-dispatched call keeps its first deadline.
+ * unclaimed call the user is not still deciding on can be offered, and only once, so a
+ * re-dispatched call keeps its first deadline; recording the user's decision clears it, so an
+ * allowed call's window starts when it is offered after the answer.
  */
 export async function offerDesktopToolCall(input: {
   toolCallId: string
@@ -313,6 +325,7 @@ export async function offerDesktopToolCall(input: {
         eq(copilotAsyncToolCalls.runId, input.runId),
         eq(copilotAsyncToolCalls.status, ASYNC_TOOL_STATUS.pending),
         isNull(copilotAsyncToolCalls.pickupDeadlineAt),
+        sql`NOT (${awaitingPermission})`,
         sql`EXISTS (SELECT 1 FROM ${copilotRuns} r WHERE r.id = ${copilotAsyncToolCalls.runId} AND r.desktop_device_id IS NOT NULL)`
       )
     )