Skip to content

Commit 383bce7

Browse files
waleedlatif1waleedlatif1
committed
fix(desktop): name every executor contract's wire types
Each response and the inbox query get named type aliases, and the device E2E consumes them. The Stop race test settles its completion on every path, and the E2E's lost-doorbell check waits for the result, not just the claim.
1 parent b79996f commit 383bce7

3 files changed

Lines changed: 53 additions & 44 deletions

File tree

‎apps/sim/lib/api/contracts/desktop-executor.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export const registerDesktopDeviceResponseSchema = z.object({
3939
pickupGraceMs: z.number().int().positive(),
4040
reconcileMs: z.number().int().positive(),
4141
})
42+
export type RegisterDesktopDeviceResponse = z.output<typeof registerDesktopDeviceResponseSchema>
4243

4344
export const registerDesktopDeviceContract = defineRouteContract({
4445
method: 'POST',
@@ -49,6 +50,7 @@ export const registerDesktopDeviceContract = defineRouteContract({
4950
})
5051

5152
const desktopInboxQuerySchema = z.object({ deviceId: desktopDeviceIdSchema })
53+
export type DesktopInboxQuery = z.input<typeof desktopInboxQuerySchema>
5254

5355
const desktopInboxCallSchema = z.object({
5456
kind: z.literal('call'),
@@ -86,6 +88,7 @@ export type DesktopInboxItem = z.output<typeof desktopInboxItemSchema>
8688
export const desktopInboxResponseSchema = z.object({
8789
items: z.array(desktopInboxItemSchema),
8890
})
91+
export type DesktopInboxResponse = z.output<typeof desktopInboxResponseSchema>
8992

9093
export const listDesktopInboxContract = defineRouteContract({
9194
method: 'GET',
@@ -123,6 +126,7 @@ export const claimDesktopToolResponseSchema = z.object({
123126
executionToken: z.string().min(1),
124127
leaseExpiresAt: z.string(),
125128
})
129+
export type ClaimDesktopToolResponse = z.output<typeof claimDesktopToolResponseSchema>
126130

127131
export const claimDesktopToolContract = defineRouteContract({
128132
method: 'POST',
@@ -140,6 +144,7 @@ const renewDesktopToolLeaseBodySchema = z.object({
140144
export type RenewDesktopToolLeaseBody = z.input<typeof renewDesktopToolLeaseBodySchema>
141145

142146
export const renewDesktopToolLeaseResponseSchema = z.object({ leaseExpiresAt: z.string() })
147+
export type RenewDesktopToolLeaseResponse = z.output<typeof renewDesktopToolLeaseResponseSchema>
143148

144149
/** A 410 means the call was stopped, settled, or its lease lapsed: cancel the local action. */
145150
export const renewDesktopToolLeaseContract = defineRouteContract({
@@ -170,6 +175,7 @@ export const completeDesktopToolResponseSchema = z.object({
170175
outcome: z.enum(['recorded', 'duplicate', 'superseded']),
171176
status: z.enum(['completed', 'failed', 'cancelled']),
172177
})
178+
export type CompleteDesktopToolResponse = z.output<typeof completeDesktopToolResponseSchema>
173179

174180
export const completeDesktopToolContract = defineRouteContract({
175181
method: 'POST',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
581581
} finally {
582582
releaseStop.resolve()
583583
await stopping
584+
await Promise.allSettled([completing])
584585
}
585586

586587
await expect(completing).resolves.toEqual({ outcome: 'superseded', status: 'cancelled' })

‎apps/sim/scripts/test-desktop-inbox-e2e.ts‎

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,19 @@ import postgres from 'postgres'
1010
import type { z } from 'zod'
1111
import {
1212
type ClaimDesktopToolBody,
13+
type ClaimDesktopToolResponse,
1314
type CompleteDesktopToolBody,
15+
type CompleteDesktopToolResponse,
1416
claimDesktopToolResponseSchema,
1517
completeDesktopToolResponseSchema,
1618
type DesktopInboxItem,
19+
type DesktopInboxQuery,
20+
type DesktopInboxResponse,
1721
desktopInboxResponseSchema,
1822
type RegisterDesktopDeviceBody,
23+
type RegisterDesktopDeviceResponse,
1924
type RenewDesktopToolLeaseBody,
25+
type RenewDesktopToolLeaseResponse,
2026
registerDesktopDeviceResponseSchema,
2127
renewDesktopToolLeaseResponseSchema,
2228
} from '@/lib/api/contracts/desktop-executor'
@@ -154,7 +160,7 @@ async function signIn(existing?: Desktop): Promise<Desktop> {
154160
return { userId, workspaceId, sessionId, cookie, deviceId: generateId() }
155161
}
156162

157-
async function register(desktop: Desktop) {
163+
async function register(desktop: Desktop): Promise<RegisterDesktopDeviceResponse> {
158164
const body: RegisterDesktopDeviceBody = {
159165
deviceId: desktop.deviceId,
160166
name: 'E2E MacBook',
@@ -168,6 +174,14 @@ async function register(desktop: Desktop) {
168174
})
169175
}
170176

177+
/** The device's reconciling pull, the same request on every doorbell and timer tick. */
178+
async function pullInbox(desktop: Desktop): Promise<DesktopInboxResponse> {
179+
const query: DesktopInboxQuery = { deviceId: desktop.deviceId }
180+
return request(desktop, 'GET', `/api/desktop/inbox?${new URLSearchParams(query)}`, {
181+
schema: desktopInboxResponseSchema,
182+
})
183+
}
184+
171185
/** A chat whose running turn was bound to the device at admission. */
172186
async function boundChat(desktop: Desktop, title: string) {
173187
const chatId = generateId()
@@ -280,24 +294,22 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
280294
try {
281295
do {
282296
rerun = false
283-
const inbox = await request(
284-
desktop,
285-
'GET',
286-
`/api/desktop/inbox?deviceId=${desktop.deviceId}`,
287-
{
288-
schema: desktopInboxResponseSchema,
289-
}
290-
)
297+
const inbox = await pullInbox(desktop)
291298
for (const item of inbox.items) {
292299
if (item.kind !== 'call' || ran.has(item.toolCallId)) continue
293300
const claimBody: ClaimDesktopToolBody = {
294301
deviceId: desktop.deviceId,
295302
toolCallId: item.toolCallId,
296303
}
297-
const claim = await request(desktop, 'POST', '/api/desktop/tool/claim', {
298-
body: claimBody,
299-
schema: claimDesktopToolResponseSchema,
300-
})
304+
const claim: ClaimDesktopToolResponse = await request(
305+
desktop,
306+
'POST',
307+
'/api/desktop/tool/claim',
308+
{
309+
body: claimBody,
310+
schema: claimDesktopToolResponseSchema,
311+
}
312+
)
301313
ran.set(item.toolCallId, {
302314
toolName: claim.toolName,
303315
chatId: claim.chatId,
@@ -308,10 +320,13 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
308320
toolCallId: item.toolCallId,
309321
executionToken: claim.executionToken,
310322
}
311-
await request(desktop, 'POST', '/api/desktop/tool/lease', {
312-
body: renewBody,
313-
schema: renewDesktopToolLeaseResponseSchema,
314-
})
323+
const renewed: RenewDesktopToolLeaseResponse = await request(
324+
desktop,
325+
'POST',
326+
'/api/desktop/tool/lease',
327+
{ body: renewBody, schema: renewDesktopToolLeaseResponseSchema }
328+
)
329+
assert(Date.parse(renewed.leaseExpiresAt) > Date.now())
315330
const completeBody: CompleteDesktopToolBody = {
316331
deviceId: desktop.deviceId,
317332
toolCallId: item.toolCallId,
@@ -320,10 +335,15 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
320335
message: `${claim.toolName} done`,
321336
data: { ran: claim.toolName, chatId: claim.chatId },
322337
}
323-
const completedCall = await request(desktop, 'POST', '/api/desktop/tool/complete', {
324-
body: completeBody,
325-
schema: completeDesktopToolResponseSchema,
326-
})
338+
const completedCall: CompleteDesktopToolResponse = await request(
339+
desktop,
340+
'POST',
341+
'/api/desktop/tool/complete',
342+
{
343+
body: completeBody,
344+
schema: completeDesktopToolResponseSchema,
345+
}
346+
)
327347
assert.deepEqual(completedCall, { outcome: 'recorded', status: 'completed' })
328348
completed.add(item.toolCallId)
329349
}
@@ -452,7 +472,7 @@ async function run() {
452472
{ silent: true }
453473
)
454474
await waitFor(
455-
() => executor.ran.has(toolCallId),
475+
() => executor.completed.has(toolCallId),
456476
PICKUP_GRACE_SECONDS * 1000,
457477
'the reconcile pull'
458478
)
@@ -486,14 +506,7 @@ async function run() {
486506
{ operation: 'run', command: 'npm run deploy' },
487507
{ offer: false }
488508
)
489-
const inbox = await request(
490-
desktop,
491-
'GET',
492-
`/api/desktop/inbox?deviceId=${desktop.deviceId}`,
493-
{
494-
schema: desktopInboxResponseSchema,
495-
}
496-
)
509+
const inbox = await pullInbox(desktop)
497510
const expected: DesktopInboxItem = {
498511
kind: 'approval_needed',
499512
toolCallId,
@@ -546,17 +559,13 @@ async function run() {
546559
executionToken: claim.executionToken,
547560
}
548561
await request(desktop, 'POST', '/api/desktop/tool/lease', { body: lease, expected: 410 })
549-
const inbox = await request(desktop, 'GET', `/api/desktop/inbox?deviceId=${desktop.deviceId}`, {
550-
schema: desktopInboxResponseSchema,
551-
})
562+
const inbox = await pullInbox(desktop)
552563
assert(inbox.items.some((item) => item.kind === 'cancel' && item.toolCallId === toolCallId))
553564
await request(desktop, 'POST', '/api/desktop/tool/complete', {
554565
body: { ...lease, status: 'cancelled' } satisfies CompleteDesktopToolBody,
555566
schema: completeDesktopToolResponseSchema,
556567
})
557-
const after = await request(desktop, 'GET', `/api/desktop/inbox?deviceId=${desktop.deviceId}`, {
558-
schema: desktopInboxResponseSchema,
559-
})
568+
const after = await pullInbox(desktop)
560569
assert(!after.items.some((item) => item.toolCallId === toolCallId))
561570
})
562571

@@ -572,14 +581,7 @@ async function run() {
572581
{ ref: 'e9' },
573582
{ silent: true }
574583
)
575-
const inbox = await request(
576-
otherMac,
577-
'GET',
578-
`/api/desktop/inbox?deviceId=${otherMac.deviceId}`,
579-
{
580-
schema: desktopInboxResponseSchema,
581-
}
582-
)
584+
const inbox = await pullInbox(otherMac)
583585
assert.deepEqual(inbox.items, [])
584586
await request(otherMac, 'POST', '/api/desktop/tool/claim', {
585587
body: { deviceId: otherMac.deviceId, toolCallId },

0 commit comments

Comments
 (0)