Skip to content

Commit a6bdda5

Browse files
committed
fix(desktop): end a terminating kill's command instead of the shell, name Windows commands in their own shell's syntax before a restart, and tidy session docs
1 parent 7f795c6 commit a6bdda5

9 files changed

Lines changed: 214 additions & 80 deletions

File tree

‎apps/desktop/e2e/fixtures/updater.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ void app.whenReady().then(async () => {
101101
installStatePath: join(directory, 'update-install.json'),
102102
describeInFlightWork: async () =>
103103
describeInFlightWork({
104-
terminalCommands,
104+
terminalCommands: terminalCommands.map((command) => ({ command, shell: 'zsh' })),
105105
uncheckedTerminalCount: 0,
106106
computerUseApp: null,
107107
browserToolRunning,

‎apps/desktop/src/main/in-flight-work.test.ts‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ describe('countWindowsWithUnsavedChanges', () => {
3030
})
3131

3232
describe('describeInFlightWork', () => {
33+
const zsh = (command: string) => ({ command, shell: 'zsh' })
3334
const IDLE: InFlightWork = {
3435
terminalCommands: [],
3536
uncheckedTerminalCount: 0,
@@ -51,23 +52,35 @@ describe('describeInFlightWork', () => {
5152
})
5253

5354
it('starts the sentence for an unannounced command with a capital', () => {
54-
expect(describeInFlightWork({ ...IDLE, terminalCommands: [''] })).toEqual([
55+
expect(describeInFlightWork({ ...IDLE, terminalCommands: [zsh('')] })).toEqual([
5556
'A command is still running in a terminal.',
5657
])
57-
expect(describeInFlightWork({ ...IDLE, terminalCommands: ['npm run dev'] })).toEqual([
58+
expect(describeInFlightWork({ ...IDLE, terminalCommands: [zsh('npm run dev')] })).toEqual([
5859
'npm is still running in a terminal.',
5960
])
6061
})
6162

6263
it('counts unannounced commands rather than naming "a command" twice', () => {
63-
expect(describeInFlightWork({ ...IDLE, terminalCommands: ['', ''] })).toEqual([
64+
expect(describeInFlightWork({ ...IDLE, terminalCommands: [zsh(''), zsh('')] })).toEqual([
6465
'2 terminal commands are still running.',
6566
])
66-
expect(describeInFlightWork({ ...IDLE, terminalCommands: ['', 'npm run dev'] })).toEqual([
67-
'npm and a command are still running in terminals.',
68-
])
6967
expect(
70-
describeInFlightWork({ ...IDLE, terminalCommands: ['', 'npm run dev', 'claude'] })
68+
describeInFlightWork({ ...IDLE, terminalCommands: [zsh(''), zsh('npm run dev')] })
69+
).toEqual(['npm and a command are still running in terminals.'])
70+
expect(
71+
describeInFlightWork({
72+
...IDLE,
73+
terminalCommands: [zsh(''), zsh('npm run dev'), zsh('claude')],
74+
})
7175
).toEqual(['3 terminal commands are still running.'])
7276
})
77+
78+
it("names a Windows command by its program, read in its own shell's syntax", () => {
79+
expect(
80+
describeInFlightWork({
81+
...IDLE,
82+
terminalCommands: [{ command: 'C:\\tools\\node.exe server.js', shell: 'pwsh' }],
83+
})
84+
).toEqual(['node is still running in a terminal.'])
85+
})
7386
})

‎apps/desktop/src/main/in-flight-work.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { describeRunningCommand } from '@sim/terminal-protocol'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { sleep } from '@sim/utils/helpers'
55
import type { WebContents } from 'electron'
6+
import type { RunningTerminalCommand } from '@/main/terminal'
67

78
const logger = createLogger('DesktopInFlightWork')
89

@@ -60,8 +61,8 @@ export async function countWindowsWithUnsavedChanges(
6061
}
6162

6263
export interface InFlightWork {
63-
/** The command line holding each busy terminal's foreground (`''` when unannounced). */
64-
terminalCommands: readonly string[]
64+
/** The command holding each busy terminal's foreground. */
65+
terminalCommands: readonly RunningTerminalCommand[]
6566
/** Terminals whose foreground could not be read, which may be running a command. */
6667
uncheckedTerminalCount: number
6768
/** The app a computer-use action is driving, `''` when it has no name, or null when idle. */
@@ -80,11 +81,11 @@ export interface InFlightWork {
8081
* One sentence for the busy terminals. Commands are named while at most two are running and
8182
* at most one is unannounced; past that, two "a command"s would say nothing, so they are counted.
8283
*/
83-
function describeTerminalCommands(commands: readonly string[]): string | null {
84+
function describeTerminalCommands(commands: readonly RunningTerminalCommand[]): string | null {
8485
if (commands.length === 0) return null
8586
const named = commands
86-
.filter((command) => command.trim())
87-
.map((command) => describeRunningCommand(command))
87+
.filter(({ command }) => command.trim())
88+
.map(({ command, shell }) => describeRunningCommand(command, shell))
8889
const unnamed = commands.length - named.length
8990
if (unnamed > 1 || commands.length > MAX_NAMED_TERMINAL_COMMANDS) {
9091
return `${commands.length} terminal commands are still running.`

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

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,18 @@ export interface TerminalServiceOptions {
166166
runLedger?: RunLedger
167167
}
168168

169+
/** A command holding a terminal's foreground. */
170+
export interface RunningTerminalCommand {
171+
/** Its command line, `''` when the shell did not announce it. */
172+
command: string
173+
/** The program name of the shell it was typed into (`zsh`, `pwsh`), whose syntax names its program. */
174+
shell: string
175+
}
176+
169177
/** The terminal work a restart would cut off. */
170178
export interface TerminalWork {
171-
/** The command line holding each busy terminal's foreground, `''` when unannounced. */
172-
commands: string[]
179+
/** The command holding each busy terminal's foreground. */
180+
commands: RunningTerminalCommand[]
173181
/** Terminals whose foreground could not be read, which may be running anything. */
174182
uncheckedCount: number
175183
}
@@ -319,16 +327,20 @@ export class TerminalService {
319327
const groups = this.options.processGroups ?? osProcessGroups
320328
const perSession = await Promise.all(
321329
[...this.sessions.values()].map(
322-
async (session): Promise<{ command: string | null; unchecked: boolean }> => {
323-
if (session.isBusy) return { command: session.foreground ?? '', unchecked: false }
330+
async (
331+
session
332+
): Promise<{ command: RunningTerminalCommand | null; unchecked: boolean }> => {
333+
const running = (command: string) => ({ command, shell: session.shellProgram })
334+
if (session.isBusy)
335+
return { command: running(session.foreground ?? ''), unchecked: false }
324336
if (session.hasShellIntegration) return { command: null, unchecked: false }
325337
const foreground = await groups.foreground(
326338
session.pid,
327339
session.spawnedAt,
328340
session.shellReadyAt
329341
)
330342
return {
331-
command: foreground?.length ? '' : null,
343+
command: foreground?.length ? running('') : null,
332344
unchecked: foreground === null,
333345
}
334346
}
@@ -1018,7 +1030,7 @@ export class TerminalService {
10181030
(await groups.foreground(session.pid, startedAt, session.shellReadyAt)) ?? []
10191031
let command = groups.readBeforeInterrupt ? await readCommand() : undefined
10201032
if (session.agentCommandToolCallId !== toolCallId) return
1021-
session.kill('SIGINT')
1033+
session.interrupt()
10221034
for (const escalation of groups.escalations) {
10231035
if (await this.commandEnds(session, toolCallId, STOP_ESCALATION_MS)) return
10241036
const remaining =
@@ -1036,6 +1048,30 @@ export class TerminalService {
10361048
}
10371049
}
10381050

1051+
/**
1052+
* Sends a terminating signal to the command holding a plain shell's foreground, never the shell:
1053+
* the tool stops what is running, and the shell is the user's tab. On macOS and Linux that is
1054+
* the tty's foreground process group; on Windows, which has no signals, each process the shell
1055+
* started since the command began is ended with its tree. `signalled` is false when nothing but
1056+
* the shell held the foreground, or the OS could not say what did.
1057+
*/
1058+
private async terminateForeground(
1059+
session: TerminalSession,
1060+
signal: 'SIGTERM' | 'SIGKILL',
1061+
stopped: AbortSignal
1062+
): Promise<{ signal: string; terminalId: string; signalled: boolean }> {
1063+
const groups = this.options.processGroups ?? osProcessGroups
1064+
const command =
1065+
(await groups.foreground(
1066+
session.pid,
1067+
session.foregroundStartedAt ?? session.spawnedAt,
1068+
session.shellReadyAt
1069+
)) ?? []
1070+
if (stopped.aborted) throw stoppedBeforeStart()
1071+
if (command.length > 0) groups.signal(command, signal)
1072+
return { signal, terminalId: session.terminalId, signalled: command.length > 0 }
1073+
}
1074+
10391075
private async dispatch(
10401076
toolCallId: string,
10411077
operation: TerminalOperation,
@@ -1159,8 +1195,11 @@ export class TerminalService {
11591195
await sendKey(target, signal === 'SIGKILL' ? 'C-\\' : 'C-c', session.env)
11601196
return { signal, terminalId: session.terminalId, pane: target }
11611197
}
1162-
session.kill(signal)
1163-
return { signal, terminalId: session.terminalId }
1198+
if (signal === 'SIGINT') {
1199+
session.interrupt()
1200+
return { signal, terminalId: session.terminalId }
1201+
}
1202+
return await this.terminateForeground(session, signal, latch.signal)
11641203
}
11651204
default:
11661205
throw new TerminalError('INVALID_REQUEST', `Unknown terminal operation: ${operation}`)

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
* marker. That build asks for the terminal's device attributes as it starts and holds the shell's
88
* first output until it hears back, three seconds when nothing answers, so the pty answers that
99
* one query itself, as xterm.js would, and keeps it from the output so the reply is never sent
10-
* twice; an agent's shell has no panel to answer it at all. node-pty there cannot send a signal
11-
* either, only end the shell.
10+
* twice; an agent's shell has no panel to answer it at all.
1211
*/
1312
import { spawn } from '@lydell/node-pty'
1413

@@ -48,8 +47,8 @@ export interface TerminalPty {
4847
resize(cols: number, rows: number): void
4948
pause(): void
5049
resume(): void
51-
/** Sends `signal` to the shell; on Windows, which has no signals, ends it. */
52-
kill(signal?: NodeJS.Signals): void
50+
/** Ends the shell. */
51+
kill(): void
5352
}
5453

5554
/** Spawns `options.shellPath` in a pseudo-terminal, set up for the platform it runs on. */
@@ -95,6 +94,6 @@ export function spawnTerminalPty(options: TerminalPtyOptions): TerminalPty {
9594
resize: (cols, rows) => pty.resize(cols, rows),
9695
pause: () => pty.pause(),
9796
resume: () => pty.resume(),
98-
kill: (signal) => (windows ? pty.kill() : pty.kill(signal)),
97+
kill: () => pty.kill(),
9998
}
10099
}

0 commit comments

Comments
 (0)