Skip to content

Commit 7308f99

Browse files
committed
fix(desktop): a result that keeps failing to reach Sim stops keeping the machine awake
Delivery still retries it, but after ten minutes of failures it no longer counts as work the machine must stay awake for.
1 parent 78e9300 commit 7308f99

2 files changed

Lines changed: 65 additions & 6 deletions

File tree

‎apps/desktop/src/main/desktop-executor/executor.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ class FakeRunner implements DesktopToolRunner {
155155
}
156156
}
157157

158-
function setup(options: { leaseRenewMs?: number; maxHeldCalls?: number } = {}) {
158+
function setup(
159+
options: { leaseRenewMs?: number; maxHeldCalls?: number; deliveryAwakeLimitMs?: number } = {}
160+
) {
159161
/** Approval items the executor handed to the notifier, one array per inbox read. */
160162
const approvals: DesktopApprovalItem[][] = []
161163
const sim = new FakeSim()
@@ -173,6 +175,9 @@ function setup(options: { leaseRenewMs?: number; maxHeldCalls?: number } = {}) {
173175
onBusyChange: (value) => busy.push(value),
174176
onApprovals: (items) => approvals.push(items),
175177
...(options.maxHeldCalls ? { maxHeldCalls: options.maxHeldCalls } : {}),
178+
...(options.deliveryAwakeLimitMs !== undefined
179+
? { deliveryAwakeLimitMs: options.deliveryAwakeLimitMs }
180+
: {}),
176181
})
177182
return { sim, journal, runner, executor, onUnregistered, busy, approvals }
178183
}
@@ -581,6 +586,24 @@ describe('keeping the machine awake', () => {
581586
expect(busy).toEqual([true, false])
582587
})
583588

589+
it('lets the machine sleep once a result has kept failing to reach Sim, and keeps retrying', async () => {
590+
const { sim, journal, executor, busy } = setup({ deliveryAwakeLimitMs: 20 })
591+
sim.completeErrors = Array.from({ length: 6 }, () => new DeviceRequestError(503, 'deploying'))
592+
await journal.put({
593+
toolCallId: 'r-1',
594+
state: 'result',
595+
executionToken: 't-1',
596+
completion: DONE,
597+
})
598+
599+
await executor.recover()
600+
601+
await vi.waitFor(() => expect(busy).toEqual([true, false]))
602+
expect(sim.completions).toHaveLength(0)
603+
await vi.waitFor(() => expect(sim.completions).toHaveLength(1))
604+
expect(busy).toEqual([true, false])
605+
})
606+
584607
it('stays busy while a result a previous run left is still on its way to Sim', async () => {
585608
const { sim, journal, executor, busy } = setup()
586609
sim.completeErrors = [new DeviceRequestError(503, 'deploying')]

‎apps/desktop/src/main/desktop-executor/executor.ts‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ const logger = createLogger('DesktopExecutor')
2828
const DEFAULT_MAX_HELD_CALLS = 32
2929
/** Ceiling on the wait between attempts to deliver one result. */
3030
const DELIVERY_RETRY_MAX_MS = 30_000
31+
/**
32+
* How long a result that keeps failing to reach Sim holds the machine awake. Delivery keeps
33+
* retrying after this; it just stops counting as work that needs the machine awake.
34+
*/
35+
const DELIVERY_AWAKE_LIMIT_MS = 10 * 60_000
3136

3237
const NOT_STARTED_AFTER_RESTART =
3338
'Not run: this action never started, because the Sim desktop app restarted before it began. Nothing happened on the user’s computer. Do not retry it in this turn; tell the user, who can ask again.'
@@ -65,6 +70,8 @@ export interface DesktopExecutorOptions {
6570
maxHeldCalls?: number
6671
/** First delivery retry delay; tests shorten it. */
6772
retryBaseMs?: number
73+
/** How long a failing delivery keeps the machine awake; tests shorten it. */
74+
deliveryAwakeLimitMs?: number
6875
}
6976

7077
type HeldPhase = 'queued' | 'running' | 'reporting'
@@ -95,7 +102,9 @@ export class DesktopExecutor {
95102
/** The journal walk after a restart; sign-out waits for it before clearing the journal. */
96103
private recovering: Promise<void> | null = null
97104
/** Results a previous app run left, still on their way to Sim. */
98-
private recoveredInFlight = 0
105+
private readonly recoveringIds = new Set<string>()
106+
/** Results that have failed to reach Sim for longer than {@link DELIVERY_AWAKE_LIMIT_MS}. */
107+
private readonly stalledDeliveries = new Set<string>()
99108
/** Results Sim refused because it no longer recognized this device; sent once it registers again. */
100109
private readonly parked = new Map<
101110
string,
@@ -154,10 +163,10 @@ export class DesktopExecutor {
154163
state: entry.state,
155164
})
156165
// Held awake like a running call: the result exists only on this machine until Sim has it.
157-
this.recoveredInFlight += 1
166+
this.recoveringIds.add(entry.toolCallId)
158167
this.updateBusy()
159168
void this.deliver(entry.toolCallId, entry.executionToken, completion).finally(() => {
160-
this.recoveredInFlight -= 1
169+
this.recoveringIds.delete(entry.toolCallId)
161170
this.updateBusy()
162171
})
163172
}
@@ -357,9 +366,25 @@ export class DesktopExecutor {
357366
completion: DesktopToolCompletion
358367
): Promise<void> {
359368
if (this.disposed) return
360-
let pending = completion
361369
// Best effort: unrecorded, a crash reports the call from its `started` entry as outcome unknown.
362370
await this.record({ toolCallId, state: 'result', executionToken, completion })
371+
const sendingSince = Date.now()
372+
try {
373+
await this.sendResult(toolCallId, executionToken, completion, sendingSince)
374+
} finally {
375+
// Whoever holds the delivery (a held call, a recovered result) updates the busy state as it
376+
// lets go; doing it here first would flash the machine awake again in between.
377+
this.stalledDeliveries.delete(toolCallId)
378+
}
379+
}
380+
381+
private async sendResult(
382+
toolCallId: string,
383+
executionToken: string,
384+
completion: DesktopToolCompletion,
385+
sendingSince: number
386+
): Promise<void> {
387+
let pending = completion
363388
for (let attempt = 1; !this.disposed; attempt++) {
364389
try {
365390
const outcome = await this.options.client.complete({
@@ -407,6 +432,15 @@ export class DesktopExecutor {
407432
})
408433
break
409434
}
435+
if (
436+
Date.now() - sendingSince >
437+
(this.options.deliveryAwakeLimitMs ?? DELIVERY_AWAKE_LIMIT_MS) &&
438+
!this.stalledDeliveries.has(toolCallId)
439+
) {
440+
// Still retried, but no longer a reason to keep the machine awake.
441+
this.stalledDeliveries.add(toolCallId)
442+
this.updateBusy()
443+
}
410444
await sleep(
411445
backoffWithJitter(attempt, error.retryAfterMs, {
412446
baseMs: this.options.retryBaseMs,
@@ -478,7 +512,9 @@ export class DesktopExecutor {
478512
private updateBusy(): void {
479513
// A disposed executor reports idle once, at dispose; a delivery that settles later must not
480514
// speak for the executor that replaced it.
481-
const busy = !this.disposed && (this.held.size > 0 || this.recoveredInFlight > 0)
515+
const awake = (toolCallId: string) => !this.stalledDeliveries.has(toolCallId)
516+
const busy =
517+
!this.disposed && ([...this.held.keys()].some(awake) || [...this.recoveringIds].some(awake))
482518
if (busy === this.busy) return
483519
this.busy = busy
484520
this.options.onBusyChange?.(busy)

0 commit comments

Comments
 (0)