Skip to content

Commit d174694

Browse files
committed
feat(close): add Cancel option to running-sessions close dialog
The close-confirmation dialog only offered Kill & Quit / Keep in Background; every dismissal (Escape, window-X) hid the window. Add a third Cancel button that aborts the close and keeps the app open. Also fix a pre-existing race: the backend force-destroyed the window 5s after a close request regardless of an open dialog. The renderer now acks WindowCloseHandling when it takes over the close interactively, clearing the fallback timer; a crashed renderer still hits the 5s fallback. Add preload-allowlist.test.ts asserting every IPC channel is in preload.cjs ALLOWED_CHANNELS — the new channels were initially missing there (same bug class as 08969d3), which silently broke the dialog.
1 parent 775512b commit d174694

10 files changed

Lines changed: 189 additions & 12 deletions

File tree

electron/ipc/channels.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export enum IPC {
5858
WindowToggleMaximize = '__window_toggle_maximize',
5959
WindowClose = '__window_close',
6060
WindowForceClose = '__window_force_close',
61+
WindowCloseHandling = '__window_close_handling',
6162
WindowHide = '__window_hide',
6263
WindowMaximize = '__window_maximize',
6364
WindowUnmaximize = '__window_unmaximize',
@@ -73,6 +74,7 @@ export enum IPC {
7374

7475
// Dialog
7576
DialogConfirm = '__dialog_confirm',
77+
DialogChoice = '__dialog_choice',
7678
DialogOpen = '__dialog_open',
7779

7880
// Shell

electron/ipc/register.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,18 @@ export function registerAllHandlers(win: BrowserWindow): void {
856856
return result.response === 0;
857857
});
858858

859+
ipcMain.handle(IPC.DialogChoice, async (_e, args) => {
860+
const result = await dialog.showMessageBox(win, {
861+
type: args.kind === 'warning' ? 'warning' : 'question',
862+
title: args.title || 'Confirm',
863+
message: args.message,
864+
buttons: args.buttons,
865+
defaultId: args.defaultId ?? 0,
866+
cancelId: args.cancelId ?? args.buttons.length - 1,
867+
});
868+
return result.response;
869+
});
870+
859871
ipcMain.handle(IPC.DialogOpen, async (_e, args) => {
860872
const properties: Array<'openDirectory' | 'openFile' | 'multiSelections'> = [];
861873
if (args?.directory) properties.push('openDirectory');
@@ -973,16 +985,33 @@ export function registerAllHandlers(win: BrowserWindow): void {
973985
});
974986
win.on('resize', createThrottledForwarder(win, IPC.WindowResized, 100));
975987
win.on('move', createThrottledForwarder(win, IPC.WindowMoved, 100));
988+
// Fallback timer that force-destroys the window if the renderer never
989+
// responds to a close request (e.g. it crashed). Cleared once the renderer
990+
// acks that it is handling the close interactively (showing a dialog), so a
991+
// user deliberating over that dialog isn't force-quit out from under it.
992+
let forceCloseTimer: ReturnType<typeof setTimeout> | undefined;
993+
const clearForceCloseTimer = (): void => {
994+
if (forceCloseTimer) {
995+
clearTimeout(forceCloseTimer);
996+
forceCloseTimer = undefined;
997+
}
998+
};
999+
ipcMain.handle(IPC.WindowCloseHandling, clearForceCloseTimer);
1000+
9761001
win.on('close', (e) => {
9771002
e.preventDefault();
9781003
if (!win.isDestroyed()) {
9791004
win.webContents.send(IPC.WindowCloseRequested);
9801005
// Fallback: force-close if renderer doesn't respond within 5 seconds.
981-
// If the renderer calls WindowForceClose first, win.isDestroyed()
982-
// will be true and this is a no-op.
983-
setTimeout(() => {
1006+
// Cleared by WindowCloseHandling if the renderer takes over the close
1007+
// interactively. If the renderer calls WindowForceClose first,
1008+
// win.isDestroyed() will be true and this is a no-op.
1009+
clearForceCloseTimer();
1010+
forceCloseTimer = setTimeout(() => {
9841011
if (!win.isDestroyed()) win.destroy();
9851012
}, 5_000);
9861013
}
9871014
});
1015+
1016+
win.on('closed', clearForceCloseTimer);
9881017
}

electron/preload-allowlist.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { readFileSync } from 'node:fs';
2+
import { join } from 'node:path';
3+
4+
import { describe, expect, it } from 'vitest';
5+
6+
import { IPC } from './ipc/channels.js';
7+
8+
// Regression guard: every IPC channel the renderer can invoke must be in
9+
// preload.cjs ALLOWED_CHANNELS, or `invoke` throws "Blocked IPC channel" at
10+
// runtime. main.ts only console.warns about drift in dev, so without this
11+
// test the mismatch ships silently (see commit 08969d3, same bug class).
12+
describe('preload ALLOWED_CHANNELS', () => {
13+
const preloadSrc = readFileSync(join(__dirname, 'preload.cjs'), 'utf8');
14+
const hasChannel = (channel: string) =>
15+
preloadSrc.includes(`'${channel}'`) || preloadSrc.includes(`"${channel}"`);
16+
17+
it('lists every channel in the IPC enum', () => {
18+
const channels: string[] = Object.values(IPC);
19+
const missing = channels.filter((channel) => !hasChannel(channel));
20+
expect(missing).toEqual([]);
21+
});
22+
});

electron/preload.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ const ALLOWED_CHANNELS = new Set([
7070
'__window_resized',
7171
'__window_moved',
7272
'__window_close_requested',
73+
'__window_close_handling',
7374
// Dialog
7475
'__dialog_confirm',
76+
'__dialog_choice',
7577
'__dialog_open',
7678
// Shell
7779
'__shell_reveal',

src/App.tsx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import { onMount, onCleanup, createEffect, Show, ErrorBoundary, createSignal } f
44
import { invoke } from './lib/ipc';
55
import { IPC } from '../electron/ipc/channels';
66
import { appWindow } from './lib/window';
7-
import { confirm } from './lib/dialog';
7+
import { choice } from './lib/dialog';
8+
import { CLOSE_DIALOG_BUTTONS, resolveCloseChoice } from './lib/close-decision';
89
import { Sidebar } from './components/Sidebar';
910
import { TilingLayout } from './components/TilingLayout';
1011
import { NewTaskDialog } from './components/NewTaskDialog';
@@ -434,24 +435,28 @@ function App() {
434435
runningCount === 1
435436
? '1 running terminal session'
436437
: `${runningCount} running terminal sessions`;
437-
const shouldKill = await confirm(
438-
`You have ${countLabel}. They can be restored on app restart. Kill them and quit, or keep them alive in the background?`,
438+
const selected = await choice(
439+
`You have ${countLabel}. They can be restored on app restart. Kill them and quit, keep them alive in the background, or cancel?`,
439440
{
440441
title: 'Running Terminals',
441442
kind: 'warning',
442-
okLabel: 'Kill & Quit',
443-
cancelLabel: 'Keep in Background',
443+
buttons: [...CLOSE_DIALOG_BUTTONS],
444+
defaultId: 2,
445+
cancelId: 2,
444446
},
445-
).catch(() => false);
447+
).catch(() => 2);
446448

447-
if (shouldKill) {
449+
const action = resolveCloseChoice(selected);
450+
if (action === 'kill') {
448451
await invoke(IPC.KillAllAgents).catch(console.error);
449452
allowClose = true;
450453
await appWindow.close().catch(console.error);
451454
return;
452455
}
453-
454-
await appWindow.hide().catch(console.error);
456+
if (action === 'background') {
457+
await appWindow.hide().catch(console.error);
458+
}
459+
// 'abort': close already prevented above — leave the window open.
455460
} finally {
456461
handlingClose = false;
457462
}

src/lib/close-decision.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { CLOSE_DIALOG_BUTTONS, resolveCloseChoice } from './close-decision';
4+
5+
describe('resolveCloseChoice', () => {
6+
it('maps button 0 (Kill & Quit) to kill', () => {
7+
expect(resolveCloseChoice(0)).toBe('kill');
8+
});
9+
10+
it('maps button 1 (Keep in Background) to background', () => {
11+
expect(resolveCloseChoice(1)).toBe('background');
12+
});
13+
14+
it('maps button 2 (Cancel) to abort', () => {
15+
expect(resolveCloseChoice(2)).toBe('abort');
16+
});
17+
18+
it('falls back to abort for an out-of-range index', () => {
19+
expect(resolveCloseChoice(-1)).toBe('abort');
20+
expect(resolveCloseChoice(99)).toBe('abort');
21+
});
22+
23+
it('exposes button labels with Cancel last', () => {
24+
expect(CLOSE_DIALOG_BUTTONS).toEqual(['Kill & Quit', 'Keep in Background', 'Cancel']);
25+
});
26+
});

src/lib/close-decision.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Maps the close-confirmation dialog's button index to a close action.
2+
//
3+
// The dialog (shown when the window is closed with live PTY sessions) offers
4+
// three choices. Cancel is last and is also the safe fallback for any
5+
// out-of-range index (e.g. Escape / window-X mapped to cancelId).
6+
7+
export type CloseAction = 'kill' | 'background' | 'abort';
8+
9+
export const CLOSE_DIALOG_BUTTONS = ['Kill & Quit', 'Keep in Background', 'Cancel'] as const;
10+
11+
export function resolveCloseChoice(index: number): CloseAction {
12+
switch (index) {
13+
case 0:
14+
return 'kill';
15+
case 1:
16+
return 'background';
17+
default:
18+
return 'abort';
19+
}
20+
}

src/lib/dialog.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
3+
import { IPC } from '../../electron/ipc/channels';
4+
import { choice } from './dialog';
5+
6+
const invoke = vi.fn();
7+
8+
(globalThis as unknown as { window: unknown }).window = {
9+
electron: { ipcRenderer: { invoke } },
10+
};
11+
12+
afterEach(() => {
13+
invoke.mockReset();
14+
});
15+
16+
describe('choice', () => {
17+
it('invokes the DialogChoice channel with message and options', async () => {
18+
invoke.mockResolvedValue(2);
19+
20+
const result = await choice('Pick one', {
21+
title: 'Closing',
22+
kind: 'warning',
23+
buttons: ['Kill & Quit', 'Keep in Background', 'Cancel'],
24+
defaultId: 2,
25+
cancelId: 2,
26+
});
27+
28+
expect(invoke).toHaveBeenCalledWith(IPC.DialogChoice, {
29+
message: 'Pick one',
30+
title: 'Closing',
31+
kind: 'warning',
32+
buttons: ['Kill & Quit', 'Keep in Background', 'Cancel'],
33+
defaultId: 2,
34+
cancelId: 2,
35+
});
36+
expect(result).toBe(2);
37+
});
38+
39+
it('returns the selected button index from the main process', async () => {
40+
invoke.mockResolvedValue(0);
41+
expect(await choice('msg', { buttons: ['A', 'B'] })).toBe(0);
42+
});
43+
});

src/lib/dialog.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,24 @@ export async function confirm(message: string, options?: ConfirmOptions): Promis
1616
}) as Promise<boolean>;
1717
}
1818

19+
interface ChoiceOptions {
20+
title?: string;
21+
kind?: string;
22+
buttons: string[];
23+
/** Button index selected by Enter. */
24+
defaultId?: number;
25+
/** Button index returned for Escape / window-close. */
26+
cancelId?: number;
27+
}
28+
29+
/** Multi-button dialog. Resolves to the index of the chosen button. */
30+
export async function choice(message: string, options: ChoiceOptions): Promise<number> {
31+
return window.electron.ipcRenderer.invoke(IPC.DialogChoice, {
32+
message,
33+
...options,
34+
}) as Promise<number>;
35+
}
36+
1937
interface OpenDialogOptions {
2038
directory?: boolean;
2139
multiple?: boolean;

src/lib/window.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ class AppWindow {
9999
handler: (event: { preventDefault: () => void }) => Promise<void> | void,
100100
): Promise<UnlistenFn> {
101101
return window.electron.ipcRenderer.on(IPC.WindowCloseRequested, () => {
102+
// Tell the backend we're handling this close interactively so it cancels
103+
// its 5s force-destroy fallback. From here the renderer owns the
104+
// outcome: WindowForceClose to quit, hide to background, or nothing to
105+
// abort. (Crash before this point still hits the backend fallback.)
106+
// Best-effort: a failed ack must never abort the close handler below.
107+
try {
108+
void window.electron.ipcRenderer.invoke(IPC.WindowCloseHandling).catch(() => {});
109+
} catch {
110+
/* ack is best-effort; the backend fallback still protects us */
111+
}
102112
let prevented = false;
103113
const result = handler({
104114
preventDefault: () => {

0 commit comments

Comments
 (0)