Skip to content

Commit 5861b1d

Browse files
committed
fix(desktop): never stop a run the model may hold
1 parent 9025afb commit 5861b1d

5 files changed

Lines changed: 35 additions & 9 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,20 @@ describe('restarting', () => {
536536
expect(runner.started).toEqual([])
537537
await vi.waitFor(() => expect(journal.entries.size).toBe(0))
538538
})
539+
540+
it('leaves open whether the model has a result when Sim already held one for the call', async () => {
541+
const { sim, journal, executor, delivered, notDelivered } = setup()
542+
// The previous run's send of the real result may have landed just before the app went down.
543+
sim.completionOutcome = 'duplicate'
544+
await journal.put({ toolCallId: 'started-1', state: 'started', executionToken: 't-started' })
545+
546+
await executor.recover()
547+
548+
await vi.waitFor(() => expect(journal.entries.size).toBe(0))
549+
expect(sim.completions).toHaveLength(1)
550+
expect(delivered).toEqual([])
551+
expect(notDelivered).toEqual([])
552+
})
539553
})
540554

541555
describe('delivery', () => {

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export interface DesktopExecutorOptions {
8989
onResultDelivered?: (toolCallId: string) => void
9090
/**
9191
* Called once Sim is done with a call without taking its real result: it settled the call
92-
* first, refused the result, or was sent one that stands in for it. The model never learns of
92+
* first, refused the result, or recorded one that stands in for it. The model never learns of
9393
* anything the action handed back as still going, so nothing will come back to it.
9494
*/
9595
onResultNotDelivered?: (toolCallId: string) => void
@@ -419,7 +419,7 @@ export class DesktopExecutor {
419419
sendingSince: number
420420
): Promise<void> {
421421
let pending = completion
422-
let delivered = false
422+
let notDelivered = true
423423
for (let attempt = 1; !this.disposed; attempt++) {
424424
try {
425425
const outcome = await this.options.client.complete({
@@ -429,8 +429,11 @@ export class DesktopExecutor {
429429
})
430430
logger.info('Desktop call result acknowledged', { toolCallId, outcome })
431431
// Superseded: Sim settled the call first, so this result never reached the model.
432-
delivered = outcome !== 'superseded' && isDeliveredResult(pending)
432+
const delivered = outcome !== 'superseded' && isDeliveredResult(pending)
433433
if (delivered) this.options.onResultDelivered?.(toolCallId)
434+
// A stand-in Sim already holds leaves open which result it took first: a send from before
435+
// a restart may have carried the real one.
436+
notDelivered = !delivered && outcome !== 'duplicate'
434437
break
435438
} catch (error) {
436439
// Encoding failed on this machine, so nothing was sent; the same data would fail again.
@@ -488,7 +491,7 @@ export class DesktopExecutor {
488491
}
489492
}
490493
if (this.disposed) return
491-
if (!delivered) this.options.onResultNotDelivered?.(toolCallId)
494+
if (notDelivered) this.options.onResultNotDelivered?.(toolCallId)
492495
await this.forget(toolCallId)
493496
}
494497

‎apps/desktop/src/main/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,8 +610,8 @@ function main(): void {
610610
// A result the model has makes a tmux run it handed back as still going collectable across a
611611
// restart.
612612
onResultDelivered: (toolCallId) => terminal.markRunDelivered(toolCallId),
613-
// One the model never got leaves a run it handed back as still going with no one to come back
614-
// to it, the rule the launch sweep applies to a previous process's runs.
613+
// A result the model never got leaves such a run with no one to come back to it, so it is
614+
// stopped, as the launch sweep stops a previous process's.
615615
onResultNotDelivered: (toolCallId) => void terminal.stopUndeliveredRun(toolCallId),
616616
runner: createDesktopToolRunner({
617617
preferences: () => desktopSettings.getPreferences(),

‎apps/desktop/src/main/terminal/registry-run-ledger.test.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,24 @@ describe('stopping recorded tmux runs', () => {
125125
const ledger = createRunLedger(dir)
126126
ledger.record(run('current', '%2'))
127127
ledger.record(run('other', '%3'))
128+
// Recorded as handed back: the model has its pane.
129+
ledger.record(run('handed-back', '%4', 'delivered'))
128130
const registry = new TerminalRegistry(undefined, undefined, ledger)
129131

130132
await registry.stopUndeliveredRun('call-current')
131133
await registry.stopUndeliveredRun('call-previous')
134+
await registry.stopUndeliveredRun('call-handed-back')
132135

133136
expect(panes.get('current')).toBe('stopped')
134137
expect(panes.get('previous')).toBe('stopped')
135138
expect(panes.get('other')).toBeUndefined()
136-
expect(ledger.list().map((record) => record.runId)).toEqual(['other'])
139+
expect(panes.get('handed-back')).toBeUndefined()
140+
expect(
141+
ledger
142+
.list()
143+
.map((record) => record.runId)
144+
.sort()
145+
).toEqual(['handed-back', 'other'])
137146
})
138147

139148
it('notes the run of a call whose result reached the model as handed back', () => {

‎apps/desktop/src/main/terminal/registry.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,13 +413,13 @@ export class TerminalRegistry {
413413
/**
414414
* Stops what a call's `run` handed back as still going once Sim is done with the call without
415415
* its result reaching the model: a plain shell's command, or a tmux run, this process's or a
416-
* previous one's.
416+
* previous one's. A run recorded as handed back is the model's, and is left going.
417417
*/
418418
async stopUndeliveredRun(callId: string): Promise<void> {
419419
await Promise.allSettled(
420420
[...this.entries.values()].map((entry) => entry.service.stopAgentCommand(callId))
421421
)
422-
await this.stopRecordedRuns({ callId })
422+
await this.stopRecordedRuns({ callId, keep: (run) => run.state === 'delivered' })
423423
}
424424

425425
/**

0 commit comments

Comments
 (0)