Skip to content

Commit 5156261

Browse files
authored
feat(desktop): bind turns to a desktop and enforce its deadlines from the row (#8650)
* feat(desktop): bind turns to a desktop and enforce its deadlines from the row A turn sent from a desktop whose background executor is registered to the same session (and with mothership-desktop-background-executor on) is bound to that device at admission: copilot_runs.desktop_device_id. Its desktop calls are persisted pending, offered to the device and claimed through the executor's own fenced routes; the chat view's authorize and confirm answer 409 for them. There is no supervisor. The single desktop wait (waitForDesktopToolCall) branches on the binding: a bound call is offered (pickup_deadline_at, a new nullable column) and the device's doorbell rung, then the existing durable wait enforces every deadline from the row on each 5 s check, beside the lease revocation that already lives there: - an unclaimed call whose device is offline fails at once as not started (reason offline); one still unclaimed past its pickup window fails the same way (reason not_responding), as the inverse CAS of the claim; - a claimed call whose lease lapsed fails as outcome unknown, revoking the device's token so its late result is superseded, and rings it to cancel. Every settlement is sealed like the device's own result. The stale-execution cron settles, the same way, bound calls whose waiter died with its process. One not-started builder carries a reason (chat_not_open, offline, not_responding), and the server-owned failure helper now settles through the shared client settlement. The device is rung when a bound call needs approval, when the user answers, and on Stop. * fix(desktop): refuse a claim past its pickup window and leave executor leases to their own settlement A device could claim an offered call after its pickup deadline and before the wait's next check settled it, running an action the turn was about to report as not started. The claim and the inbox now treat a closed window as no longer offered. The generic Sim lease sweep no longer settles a desktop executor's lapsed lease with the Sim interrupted result: the bound wait and the stale-execution cron settle it as outcome unknown, so the model is told the action may already have taken effect. * fix(desktop): settle Stop without the device, survive Redis errors, and only run offered calls Stop on a call the background executor held never settled the turn's tool executions unless the device acknowledged: the executor's claim marked a Sim execution as started, and Stop does not settle that execution. A device that was asleep, offline or signed out left abortRun unsettled and the next turn's workbench pending. The executor's claim now takes only its owner token and lease; no Sim handler runs for it, so Sim-execution quiescence ignores it, as it already ignores the chat view's desktop claims. An overdue unclaimed call now settles from its row when presence cannot be read, and never fails early as offline on a failed read; each call in the cron's sweep settles in its own try/catch; presence writes are best effort, so a Redis error no longer fails a pull or a renewal. The executor takes only a call Sim offered it, within its pickup window, and the inbox lists unclaimed calls only while offered or waiting for the user. A call Sim never got to offer gets an implicit deadline one pickup window after it could first run, so the cron still settles it. A revoked install id stays revoked on re-registration, a claim racing the device binding answers "no longer waiting", Stop rings the device only after its chat is validated, the two device lookups are one, and the desktop inbox E2E runs in the http-e2e CI job against its own app with Redis. * test(desktop): assert outcomes instead of mock calls, and drive Stop through abortRun The desktop tests asserted that mocks were or were not called. They now assert what the caller sees: the 409, the sweep's settled count, and the device's own doorbell, which Stop now rings through the real abortRun against a stand-in worker. * 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. * 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.
1 parent d8ba4bf commit 5156261

43 files changed

Lines changed: 32192 additions & 178 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/test-build.yml‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ jobs:
123123
http-e2e:
124124
name: End-to-end over real HTTP
125125
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-8vcpu-ubuntu-2404' || 'ubuntu-latest' }}
126-
timeout-minutes: 20
126+
# Four suites, each starting its own dev app, run in sequence.
127+
timeout-minutes: 30
127128
services:
128129
postgres:
129130
image: pgvector/pgvector:pg17
@@ -138,6 +139,16 @@ jobs:
138139
--health-interval 5s
139140
--health-timeout 5s
140141
--health-retries 10
142+
# Only the desktop executor's app is given REDIS_URL: its doorbell and presence live there.
143+
redis:
144+
image: redis:7-alpine
145+
ports:
146+
- 6379:6379
147+
options: >-
148+
--health-cmd "redis-cli ping"
149+
--health-interval 5s
150+
--health-timeout 5s
151+
--health-retries 10
141152
env:
142153
DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_test
143154
BETTER_AUTH_SECRET: http-e2e-ci-secret-at-least-32-characters
@@ -298,6 +309,54 @@ jobs:
298309
STOP_AFTER_E2E_REPORT_PATH="$report_dir/stop-after-http-report.json" \
299310
bun run test:workflow-stop-after:e2e
300311
312+
# The desktop background executor's protocol: device registration, the SSE doorbell over
313+
# Redis pub/sub, presence, leased claims, Stop and isolation, against its own app with Redis.
314+
- name: Verify the desktop background executor's inbox over real HTTP
315+
working-directory: apps/sim
316+
env:
317+
NEXT_PUBLIC_APP_URL: http://127.0.0.1:3019
318+
BETTER_AUTH_URL: http://127.0.0.1:3019
319+
REDIS_URL: redis://127.0.0.1:6379
320+
NEXT_PUBLIC_FORCE_HOSTED: 'false'
321+
MSHIP_DESKTOP_BACKGROUND_EXECUTOR: 'true'
322+
COPILOT_TOOL_PERMISSIONS_ENABLED: 'true'
323+
INTERNAL_API_SECRET: desktop-inbox-http-ci-local-secret-at-least-32-characters
324+
DB_TX_TRIPWIRE: throw
325+
DISABLE_TELEMETRY: 'true'
326+
NEXT_TELEMETRY_DISABLED: '1'
327+
READY_TIMEOUT_SECONDS: 300
328+
run: |
329+
report_dir="$RUNNER_TEMP/e2e"
330+
server_log="$report_dir/desktop-inbox-next.log"
331+
mkdir -p "$report_dir"
332+
node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3019 > "$server_log" 2>&1 &
333+
server_pid=$!
334+
finish() {
335+
kill "$server_pid" 2>/dev/null || true
336+
wait "$server_pid" 2>/dev/null || true
337+
awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/desktop-inbox-http-status.log"
338+
}
339+
trap finish EXIT
340+
fail_startup() {
341+
echo "::error::$1"
342+
tail -n 200 "$server_log"
343+
exit 1
344+
}
345+
started=$SECONDS
346+
until curl --fail --silent --max-time 10 http://127.0.0.1:3019/api/health > /dev/null; do
347+
kill -0 "$server_pid" 2>/dev/null || fail_startup 'Local desktop executor app exited during startup.'
348+
[ $((SECONDS - started)) -lt "$READY_TIMEOUT_SECONDS" ] ||
349+
fail_startup "Local desktop executor app did not become ready within $READY_TIMEOUT_SECONDS seconds."
350+
sleep 2
351+
done
352+
echo "Local desktop executor app ready after $((SECONDS - started))s"
353+
DESKTOP_INBOX_E2E_BASE_URL="$NEXT_PUBLIC_APP_URL" \
354+
DESKTOP_INBOX_E2E_DATABASE_URL="$DATABASE_URL" \
355+
DESKTOP_INBOX_E2E_REDIS_URL="$REDIS_URL" \
356+
DESKTOP_INBOX_E2E_AUTH_SECRET="$BETTER_AUTH_SECRET" \
357+
DESKTOP_INBOX_E2E_REPORT_PATH="$report_dir/desktop-inbox-http-report.json" \
358+
bun run test:desktop-inbox:e2e
359+
301360
- name: Upload end-to-end reports and server logs
302361
if: failure()
303362
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4

‎apps/sim/app/api/copilot/confirm/route.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,26 @@ describe('Copilot Confirm API Route', () => {
131131
expect(JSON.stringify(publishToolConfirmation.mock.calls)).not.toContain('resolved-secret')
132132
})
133133

134+
it("refuses a chat view's report for a call a desktop's background executor owns", async () => {
135+
getAsyncToolCall.mockResolvedValue({
136+
...existingRow,
137+
toolName: 'browser_click',
138+
status: 'pending',
139+
claimedBy: null,
140+
})
141+
getRunSegment.mockResolvedValue({ id: 'run-1', userId: 'user-1', desktopDeviceId: 'device-1' })
142+
143+
const response = await POST(
144+
createMockPostRequest({
145+
toolCallId: 'tool-call-123',
146+
status: 'error',
147+
message: 'The desktop refused this claim',
148+
})
149+
)
150+
151+
expect(response.status).toBe(409)
152+
})
153+
134154
it('atomically detaches a live background confirmation', async () => {
135155
const response = await POST(
136156
createMockPostRequest({

‎apps/sim/app/api/copilot/confirm/route.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Span } from '@opentelemetry/api'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage, toError } from '@sim/utils/errors'
4-
import { isPlainRecord } from '@sim/utils/object'
4+
import { isPlainRecord, toRecord } from '@sim/utils/object'
55
import { type NextRequest, NextResponse } from 'next/server'
66
import { copilotConfirmContract } from '@/lib/api/contracts/copilot'
77
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
@@ -40,7 +40,11 @@ import {
4040
settleClientToolCall,
4141
} from '@/lib/mothership/request/tools/client-settlement.server'
4242
import { isWorkflowToolName } from '@/lib/mothership/tools/client-executed-tools'
43-
import { getDesktopToolClaimOwner, isNativeDesktopTool } from '@/lib/mothership/tools/desktop-tools'
43+
import {
44+
getDesktopToolClaimOwner,
45+
isDesktopToolCall,
46+
isNativeDesktopTool,
47+
} from '@/lib/mothership/tools/desktop-tools'
4448
import {
4549
createStructuralWorkflowToolCompletionData,
4650
getWorkflowToolCompletionExecutionId,
@@ -206,6 +210,17 @@ export const POST = withRouteHandler((req: NextRequest) => {
206210
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
207211
}
208212

213+
if (run.desktopDeviceId && isDesktopToolCall(existing.toolName, toRecord(existing.args))) {
214+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.Forbidden)
215+
return NextResponse.json(
216+
{
217+
error:
218+
"This chat's desktop actions report through the desktop app's background executor",
219+
},
220+
{ status: 409 }
221+
)
222+
}
223+
209224
const isWorkflowTool = isWorkflowToolName(existing.toolName || '')
210225
const workflowId = isWorkflowTool
211226
? resolveWorkflowToolTargetId(existing.args, run.workflowId)

‎apps/sim/app/api/copilot/tool-permission/route.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { copilotToolPermissionContract } from '@/lib/api/contracts/copilot'
55
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
66
import { isCopilotToolPermissionsEnabled } from '@/lib/core/config/env-flags'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8+
import { ringDesktopInbox } from '@/lib/desktop/executor/doorbell'
89
import {
910
getAsyncToolCall,
1011
getRunSegment,
@@ -136,6 +137,8 @@ async function applyDecision(
136137
toolName: claimed.toolName,
137138
decidedAt: claimed.permissionDecidedAt?.toISOString(),
138139
})
140+
// A bound device lists the call for approval; the answer turns it into a call or drops it.
141+
if (run.desktopDeviceId) ringDesktopInbox(run.desktopDeviceId, 'approval')
139142

140143
return { toolCallId, decision, applied: true }
141144
}

‎apps/sim/app/api/desktop/tool/authorize/route.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,26 @@ describe('desktop tool authorization', () => {
7272
})
7373
})
7474

75+
it("refuses a chat view's claim on a run bound to a desktop's background executor", async () => {
76+
getAsyncToolCall.mockResolvedValueOnce({
77+
toolCallId: 'bound-click',
78+
runId: 'run-1',
79+
status: 'pending',
80+
toolName: 'browser_click',
81+
args: { ref: 'e1' },
82+
})
83+
getRunSegment.mockResolvedValueOnce({
84+
id: 'run-1',
85+
chatId: 'chat-1',
86+
userId: 'user-1',
87+
status: 'active',
88+
desktopDeviceId: 'device-1',
89+
})
90+
91+
const response = await POST(request('bound-click'))
92+
expect(response.status).toBe(409)
93+
})
94+
7595
it('rejects retired browser tools retained only for history', async () => {
7696
getAsyncToolCall.mockResolvedValueOnce({
7797
toolCallId: 'retired-browser-tool',

‎apps/sim/app/api/desktop/tool/authorize/route.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7474
if (run.status === 'complete' || run.status === 'error' || run.status === 'cancelled') {
7575
return createNotFoundResponse('Pending client tool call not found')
7676
}
77+
// The device's background executor claims a bound run's calls through its own fenced route.
78+
if (run.desktopDeviceId)
79+
return NextResponse.json(
80+
{ error: "This chat's desktop actions run in the desktop app's background executor" },
81+
{ status: 409 }
82+
)
7783

7884
const args = isRecordLike(toolCall.args) ? (toolCall.args as Record<string, unknown>) : {}
7985
if (!isDesktopToolCall(toolCall.toolName, args)) {

‎apps/sim/background/cleanup-stale-executions.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
type StaleSweepableExecutionStatus,
3232
} from '@/lib/logs/types'
3333
import { sweepOrphanedRuns } from '@/lib/mothership/async-runs/orphaned-runs'
34+
import { settleAbandonedDesktopToolCalls } from '@/lib/mothership/request/tools/desktop-wait'
3435
import { cancelStaleDispatches } from '@/lib/table/dispatcher'
3536
import { deleteFile } from '@/lib/uploads/core/storage-service'
3637
import {
@@ -743,6 +744,19 @@ export async function runCleanupStaleExecutions() {
743744
})
744745
}
745746

747+
/**
748+
* Settle desktop calls on device-bound runs whose waiter died with its process: an offered call
749+
* nobody claimed, or a claimed one whose device stopped renewing its lease.
750+
*/
751+
let abandonedDesktopCallsSettled = 0
752+
try {
753+
abandonedDesktopCallsSettled = await settleAbandonedDesktopToolCalls()
754+
} catch (error) {
755+
logger.error('Failed to settle abandoned desktop tool calls:', {
756+
error: toError(error).message,
757+
})
758+
}
759+
746760
return {
747761
executions: {
748762
found: staleExecutionsFound,
@@ -774,6 +788,7 @@ export async function runCleanupStaleExecutions() {
774788
},
775789
chatRuns: {
776790
orphanedSettled: orphanedRunsSettled,
791+
abandonedDesktopCallsSettled,
777792
},
778793
}
779794
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ export const desktopExecutorErrorPolicy = extendInternalErrorPolicy(
2626

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

0 commit comments

Comments
 (0)