Skip to content

Commit 722c2ce

Browse files
committed
Handle command-exit watches on PTY exit
1 parent 28e9f0a commit 722c2ce

6 files changed

Lines changed: 51 additions & 7 deletions

File tree

docs/specs/alert.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,8 @@ The command-exit track is intentionally stricter than WATCHING. It exists for th
307307
| `IDLE` | watched command is still running and attention expires or is explicitly lost | `COMMAND_EXIT_ARMED` | Store `attentionLostAt`. |
308308
| `COMMAND_EXIT_ARMED` | same command finishes, runtime is at least `T_USER_ATTENTION`, and Session lacks attention | `ALERT_RINGING` | Create generated command-exit notification, set `todo = true`, and ring. |
309309
| `COMMAND_EXIT_ARMED` | same command finishes too quickly | `IDLE` | Clear without ringing. |
310+
| `COMMAND_EXIT_ARMED` | PTY exits before a command-finish semantic event, runtime is at least `T_USER_ATTENTION`, and Session lacks attention | `ALERT_RINGING` | Treat process exit as the fallback finish event for commands such as `exec <long command>` or shells that exit before emitting a finish marker. |
311+
| `IDLE` | PTY exits before a command-finish semantic event | `IDLE` | Clear any stored `commandExitWatch`; a dead process must not become armed later. |
310312
| `COMMAND_EXIT_ARMED` | Session regains attention before finish | `IDLE` | Clear the arm; the user is watching again. |
311313
| any | a different command starts | `IDLE` | Replace the watch with the new command if it is eligible. |
312314
| `ALERT_RINGING` | explicit attention boundary / dismiss / TODO clear | `IDLE` | Public status falls back to the other tracks. |

lib/src/lib/alert-manager.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,45 @@ describe('AlertManager in isolation', () => {
352352
});
353353
});
354354

355+
it('finishes an armed command-exit watch when the PTY exits without commandFinish', () => {
356+
const id = 'command-exit-pty-exit';
357+
358+
manager.attend(id);
359+
manager.applyTerminalSemanticEvents(id, [
360+
{ type: 'commandLine', commandLine: 'exec pnpm build' },
361+
{ type: 'commandStart', source: 'osc633_E', startedAt: Date.now() },
362+
]);
363+
364+
vi.advanceTimersByTime(15_000);
365+
expect(manager.getState(id).status).toBe('COMMAND_EXIT_ARMED');
366+
367+
manager.onExit(id, 1);
368+
expect(manager.getState(id)).toMatchObject({
369+
status: 'ALERT_RINGING',
370+
todo: true,
371+
notification: { source: 'COMMAND_EXIT', title: 'Command finished', body: 'exec pnpm build exited 1' },
372+
});
373+
});
374+
375+
it('clears an unarmed command-exit watch when the PTY exits before attention loss', () => {
376+
const id = 'command-exit-pty-exit-unarmed';
377+
378+
manager.attend(id);
379+
manager.applyTerminalSemanticEvents(id, [
380+
{ type: 'commandLine', commandLine: 'exec true' },
381+
{ type: 'commandStart', source: 'osc633_E', startedAt: Date.now() },
382+
]);
383+
384+
manager.onExit(id, 0);
385+
vi.advanceTimersByTime(15_000);
386+
387+
expect(manager.getState(id)).toMatchObject({
388+
status: 'WATCHING_DISABLED',
389+
todo: false,
390+
notification: null,
391+
});
392+
});
393+
355394
it('does not ring command-exit alerts for commands shorter than the attention window', () => {
356395
const id = 'quick-command-exit';
357396

lib/src/lib/alert-manager.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,11 @@ export class AlertManager {
136136
entry?.monitor?.onData();
137137
}
138138

139-
// Intentional no-op: the monitor detects silence and transitions naturally,
140-
// and we keep the entry so alert/todo state survives the PTY exit.
141-
onExit(_id: string): void {}
139+
onExit(id: string, exitCode?: number): void {
140+
const entry = this.entries.get(id);
141+
if (!entry) return;
142+
if (this.finishCommandExitWatch(id, entry, exitCode)) this.notify(id);
143+
}
142144

143145
onResize(id: string): void {
144146
const entry = this.entries.get(id);

lib/src/lib/platform/fake-adapter.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export class FakePtyAdapter implements PlatformAdapter {
157157
this.terminalSizes.delete(id);
158158
this.inputHandlers.delete(id);
159159
this.protocolParsers.delete(id);
160+
this.alertManager.onExit(id, 0);
160161
for (const handler of this.exitHandlers) {
161162
handler({ id, exitCode: 0 });
162163
}
@@ -321,7 +322,7 @@ export class FakePtyAdapter implements PlatformAdapter {
321322
const exitTimer = setTimeout(() => {
322323
if (!this.terminals.has(id)) return;
323324
this.activeTimers.delete(id);
324-
this.alertManager.onExit(id);
325+
this.alertManager.onExit(id, scenario.exitCode ?? 0);
325326
for (const handler of this.exitHandlers) {
326327
handler({ id, exitCode: scenario.exitCode ?? 0 });
327328
}

standalone/src/tauri-adapter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export class TauriAdapter implements PlatformAdapter {
7575

7676
this.unlistenFns.push(
7777
await listen<{ id: string; exitCode: number }>("pty:exit", (event) => {
78-
this.alertManager.onExit(event.payload.id);
78+
this.alertManager.onExit(event.payload.id, event.payload.exitCode);
7979
this.protocolParsers.delete(event.payload.id);
8080
for (const handler of this.exitHandlers) {
8181
handler(event.payload);

vscode-ext/src/message-router.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ ptyManager.addCallbacks({
7575
log.info(`[alert-feed] ${id}: ${before}${after}`);
7676
}
7777
},
78-
onExit(id: string) {
78+
onExit(id: string, exitCode: number) {
7979
log.info(`[alert-feed] ${id}: PTY exited`);
80-
alertManager.onExit(id);
80+
alertManager.onExit(id, exitCode);
8181
alertProtocolParsers.delete(id);
8282
},
8383
});

0 commit comments

Comments
 (0)