Skip to content

Commit b39a245

Browse files
committed
fix(desktop): measure a shell's startup bound from when its startup files began
The 30 s bound ran from spawn, so a shell whose startup marker arrived late on a busy machine got less than the promised time for its startup files. The deadline now starts at the marker.
1 parent f2be555 commit b39a245

3 files changed

Lines changed: 38 additions & 15 deletions

File tree

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -261,11 +261,11 @@ interface PendingCommand {
261261
resolve(result: TerminalRunResult): void
262262
}
263263

264-
/** How long a new shell gets to reach its first prompt, each measured from its spawn. */
264+
/** How long a new shell gets to reach its first prompt. */
265265
export interface ShellStartupBounds {
266-
/** To begin the startup files we generated; a shell that has not by then never will. */
266+
/** From spawn, to begin the startup files we generated; a shell that has not by then never will. */
267267
unstartedMs: number
268-
/** For startup files that did begin to reach a prompt. */
268+
/** From the moment they began, for those startup files to reach a prompt. */
269269
startingMs: number
270270
}
271271

@@ -329,7 +329,8 @@ export class TerminalSession {
329329
private lines: number
330330
private shellIntegration = false
331331
/** The shell has begun the startup files we generated, so its integration is on the way. */
332-
private startupBegun = false
332+
/** When the shell began the startup files we generated, or null while it has not. */
333+
private startupBegunAt: number | null = null
333334
private readonly spawnedAt = Date.now()
334335
private altScreen = false
335336
private foregroundCommand: string | null = null
@@ -516,7 +517,7 @@ export class TerminalSession {
516517
/**
517518
* Resolves once the shell reaches its first prompt with integration live, or once it is clear
518519
* it will not: it never began our startup files within `bounds.unstartedMs` of spawning, it is
519-
* still running them `bounds.startingMs` after spawning, it exited, or `signal` stopped the
520+
* still running them `bounds.startingMs` after they began, it exited, or `signal` stopped the
520521
* wait. A shell's startup files take a while, and longer on a busy machine, so a command issued
521522
* soon after spawn would otherwise be refused when the shell is merely early.
522523
*/
@@ -531,8 +532,11 @@ export class TerminalSession {
531532
timer = null
532533
const readiness = signal?.aborted ? 'stopped' : this.readiness(bounds)
533534
if (readiness === null) {
534-
const limit = this.startupBegun ? bounds.startingMs : bounds.unstartedMs
535-
timer = setTimeout(check, Math.max(0, this.spawnedAt + limit - Date.now()))
535+
const deadline =
536+
this.startupBegunAt === null
537+
? this.spawnedAt + bounds.unstartedMs
538+
: this.startupBegunAt + bounds.startingMs
539+
timer = setTimeout(check, Math.max(0, deadline - Date.now()))
536540
return
537541
}
538542
this.readinessWaiters.delete(check)
@@ -549,11 +553,13 @@ export class TerminalSession {
549553
private readiness(bounds: ShellStartupBounds): ShellReadiness | null {
550554
if (this.shellIntegration) return 'ready'
551555
if (this.disposed) return 'exited'
552-
const elapsed = Date.now() - this.spawnedAt
553-
if (!this.startupBegun) {
554-
return !this.instrumented || elapsed >= bounds.unstartedMs ? 'not-instrumented' : null
556+
const now = Date.now()
557+
if (this.startupBegunAt === null) {
558+
return !this.instrumented || now - this.spawnedAt >= bounds.unstartedMs
559+
? 'not-instrumented'
560+
: null
555561
}
556-
return elapsed >= bounds.startingMs ? 'starting' : null
562+
return now - this.startupBegunAt >= bounds.startingMs ? 'starting' : null
557563
}
558564

559565
private notifyReadinessWaiters(): void {
@@ -849,8 +855,8 @@ export class TerminalSession {
849855
): void {
850856
switch (marker.kind) {
851857
case 'startup':
852-
if (!this.startupBegun) {
853-
this.startupBegun = true
858+
if (this.startupBegunAt === null) {
859+
this.startupBegunAt = Date.now()
854860
this.notifyReadinessWaiters()
855861
}
856862
break

‎apps/desktop/src/main/terminal/shell-startup.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,23 @@ describe('a shell that is still starting', () => {
9595
terminal.dispose()
9696
})
9797

98+
it('gives startup files their full bound from when they began, however late that was', async () => {
99+
// A machine so busy our files began 7 s after spawn, and the user's then took 25 s more.
100+
const terminal = new TerminalService({ loadCwd: () => '/tmp' })
101+
const running = terminal.executeTool('call-late', 'run', { command: 'echo hi' })
102+
await vi.advanceTimersByTimeAsync(7_000)
103+
shell(STARTUP)
104+
await vi.advanceTimersByTimeAsync(25_000)
105+
expect(pty.writes).toEqual([])
106+
107+
shell(PROMPT)
108+
await answerCommand('hi')
109+
const response = await running
110+
111+
expect(response).toMatchObject({ ok: true, result: { exitCode: 0 } })
112+
terminal.dispose()
113+
})
114+
98115
it('refuses with the screen when startup files stall on a question', async () => {
99116
const terminal = new TerminalService({ loadCwd: () => '/tmp' })
100117
const running = terminal.executeTool('call-stalled', 'run', { command: 'echo hi' })

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ const registerDesktopDeviceBodySchema = z.object({
3030
export type RegisterDesktopDeviceBody = z.input<typeof registerDesktopDeviceBodySchema>
3131

3232
/**
33-
* `enabled: false` means this install cannot run the executor: the device keeps finishing calls on
34-
* runs already bound to it, but no new turn binds to it.
33+
* `enabled: false` means no new turn binds to the device, as on an install that cannot track
34+
* desktop presence; the device still finishes calls on runs already bound to it.
3535
*/
3636
export const registerDesktopDeviceResponseSchema = z.object({
3737
enabled: z.boolean(),

0 commit comments

Comments
 (0)