Skip to content

Commit cb3ff96

Browse files
committed
fix(desktop): hand a device its calls in the order they were persisted
The inbox ordered calls by created_at, then tool_call_id. Calls of one turn can be persisted in the same millisecond, and then the tie broke on random ids: a device could run a click before the type the model emitted first. Calls now carry persist_seq, a strictly increasing number assigned on insert (pre-persist writes them in emission order), and the inbox and the overdue sweep order by it. The migration adds the column without a default and then sets the default, so existing rows are not rewritten; rows persisted before it have no position and sort first, as the oldest.
1 parent ea6cd54 commit cb3ff96

7 files changed

Lines changed: 30287 additions & 7 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,28 @@ describe.runIf(Boolean(redisUrl))('desktop background executor protocol', () =>
865865
])
866866
})
867867

868+
it('lists calls persisted in the same millisecond in the order they were persisted', async () => {
869+
const desktop = await signedInDesktop()
870+
const run = await boundRun(desktop)
871+
const sameMillisecond = new Date()
872+
/** Ids that sort in reverse, so neither the clock nor the id can produce this order. */
873+
const persisted = ['type-zz', 'click-mm', 'submit-aa'].map(
874+
(name) => `${name}-${generateId()}`
875+
)
876+
for (const toolCallId of persisted) {
877+
await db.insert(copilotAsyncToolCalls).values({
878+
runId: run.runId,
879+
toolCallId,
880+
toolName: 'browser_click',
881+
args: { ref: 'e1' },
882+
createdAt: sameMillisecond,
883+
pickupDeadlineAt: sql`now() + interval '1 minute'`,
884+
})
885+
}
886+
887+
expect((await inbox(desktop)).items.map((item) => item.toolCallId)).toEqual(persisted)
888+
})
889+
868890
it('lists pending calls in persistence order even when their doorbell was never heard', async () => {
869891
const desktop = await signedInDesktop()
870892
const run = await boundRun(desktop)

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,29 @@ const isDesktopToolCallRow = or(
8484
)
8585
const INBOX_ROW_LIMIT = 500
8686

87+
/**
88+
* Persistence order, which follows the order the model emitted the calls in: calls of one turn
89+
* can share a millisecond, so the timestamp alone cannot order them. Rows persisted before the
90+
* sequence existed have none and come first, as they are the oldest.
91+
*/
92+
const persistOrder = [
93+
sql`${copilotAsyncToolCalls.persistSeq} ASC NULLS FIRST`,
94+
asc(copilotAsyncToolCalls.createdAt),
95+
asc(copilotAsyncToolCalls.toolCallId),
96+
]
97+
98+
function comparePersistOrder(
99+
a: { persistSeq: number | null; createdAt: Date; toolCallId: string },
100+
b: { persistSeq: number | null; createdAt: Date; toolCallId: string }
101+
): number {
102+
if (a.persistSeq !== b.persistSeq) {
103+
if (a.persistSeq === null) return -1
104+
if (b.persistSeq === null) return 1
105+
return a.persistSeq - b.persistSeq
106+
}
107+
return a.createdAt.getTime() - b.createdAt.getTime() || a.toolCallId.localeCompare(b.toolCallId)
108+
}
109+
87110
export interface DesktopDeviceRegistration {
88111
id: string
89112
userId: string
@@ -178,7 +201,7 @@ export async function touchDesktopDevice(deviceId: string): Promise<void> {
178201
* out the other: unclaimed calls on its recent open runs that are offered or waiting for the
179202
* user's decision, and calls it claimed that Sim settled without its result and it has not yet
180203
* acknowledged (cancel items). A call the device is still running is never listed: it already
181-
* holds it. Ordered by persistence time, the order the device claims in.
204+
* holds it. Ordered by persistence, the order the device claims in.
182205
*/
183206
export async function listDesktopInboxRows(identity: Omit<DesktopDeviceIdentity, 'sessionId'>) {
184207
const rowsWhere = (state: SQL | undefined) =>
@@ -192,6 +215,7 @@ export async function listDesktopInboxRows(identity: Omit<DesktopDeviceIdentity,
192215
permissionDecision: copilotAsyncToolCalls.permissionDecision,
193216
claimed: sql<boolean>`${copilotAsyncToolCalls.executionOwnerToken} IS NOT NULL`,
194217
createdAt: copilotAsyncToolCalls.createdAt,
218+
persistSeq: copilotAsyncToolCalls.persistSeq,
195219
chatId: copilotRuns.chatId,
196220
chatTitle: copilotChats.title,
197221
workspaceId: copilotRuns.workspaceId,
@@ -208,7 +232,7 @@ export async function listDesktopInboxRows(identity: Omit<DesktopDeviceIdentity,
208232
state
209233
)
210234
)
211-
.orderBy(asc(copilotAsyncToolCalls.createdAt), asc(copilotAsyncToolCalls.toolCallId))
235+
.orderBy(...persistOrder)
212236
.limit(INBOX_ROW_LIMIT)
213237
const [waiting, cancelled] = await Promise.all([
214238
rowsWhere(
@@ -228,10 +252,7 @@ export async function listDesktopInboxRows(identity: Omit<DesktopDeviceIdentity,
228252
)
229253
),
230254
])
231-
return [...waiting, ...cancelled].sort(
232-
(a, b) =>
233-
a.createdAt.getTime() - b.createdAt.getTime() || a.toolCallId.localeCompare(b.toolCallId)
234-
)
255+
return [...waiting, ...cancelled].sort(comparePersistOrder)
235256
}
236257

237258
export type DesktopInboxRow = Awaited<ReturnType<typeof listDesktopInboxRows>>[number]
@@ -428,7 +449,7 @@ export async function listOverdueDesktopToolCalls(input: { slackMs: number; limi
428449
)
429450
)
430451
)
431-
.orderBy(asc(copilotAsyncToolCalls.createdAt), asc(copilotAsyncToolCalls.toolCallId))
452+
.orderBy(...persistOrder)
432453
.limit(input.limit)
433454
return rows.map((row) => row.toolCallId)
434455
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
CREATE SEQUENCE IF NOT EXISTS "public"."copilot_async_tool_calls_persist_seq" INCREMENT BY 1 MINVALUE 1 MAXVALUE 9223372036854775807 START WITH 1 CACHE 1;--> statement-breakpoint
2+
-- Added without a default, then given one: a volatile default on ADD COLUMN would rewrite every
3+
-- existing row, while SET DEFAULT applies only to new rows. Existing rows keep a NULL position.
4+
ALTER TABLE "copilot_async_tool_calls" ADD COLUMN IF NOT EXISTS "persist_seq" bigint;--> statement-breakpoint
5+
ALTER TABLE "copilot_async_tool_calls" ALTER COLUMN "persist_seq" SET DEFAULT nextval('copilot_async_tool_calls_persist_seq');--> statement-breakpoint
6+
ALTER SEQUENCE "public"."copilot_async_tool_calls_persist_seq" OWNED BY "copilot_async_tool_calls"."persist_seq";

0 commit comments

Comments
 (0)