From 84bbf3b54009c609ff75443298d868eb0a538c03 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:33:31 -0700 Subject: [PATCH] test(preload): pin the one hop that keeps a malformed survival answer from skipping the quit warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule that only an explicit yes is a yes had two expressions. The one in resolveLocalPtysSurviveQuit answers a typed in-process getter with a single production call site whose every path returns a boolean literal: instrumented across 35,022 tests it was evaluated 6 times, saw 'boolean' 6 times, and discriminated 0 times, and both deleting the check and swapping it for Boolean() reddened 0 of 115 while inverting it reddened 5. Deleted, since even a type violation there is caught downstream. The expression that survives is readWindowCloseRequestPayload, on the IPC hop where the answer really is unknown. Its own tests were green, but its only production call site was inline in the api object and no test in the tree imported it — forwarding the raw payload instead reddened 0 of 11,136. Extracted so the hop is reachable, and pinned: bypassing the reader now reddens 7, weakening it to Boolean() 5, acking after the callback 1, echoing a raw requestId 1, and dropping the unsubscribe 1. --- src/main/window/window-close-decision.ts | 8 +- src/preload/index.ts | 18 +-- .../window-close-request-subscription.test.ts | 125 ++++++++++++++++++ .../window-close-request-subscription.ts | 36 +++++ 4 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 src/preload/window-close-request-subscription.test.ts create mode 100644 src/preload/window-close-request-subscription.ts diff --git a/src/main/window/window-close-decision.ts b/src/main/window/window-close-decision.ts index 953dad13468..2333d2d00d6 100644 --- a/src/main/window/window-close-decision.ts +++ b/src/main/window/window-close-decision.ts @@ -45,13 +45,19 @@ export function resolveWindowCloseAction(state: WindowCloseState): WindowCloseAc * Only a definite yes may skip the warning. A missing getter (window built before the * daemon wiring exists) or a throwing one is an undetermined answer, and an * undetermined answer is not a yes. + * + * Why no `=== true` on the getter's result: the answer is read here from a typed + * in-process function, and it is read again from an `unknown` on the other side of + * the IPC hop by `readWindowCloseRequestPayload`, which is where a shape that is + * neither a clean yes nor a clean no can actually arrive. That rule lives once, + * there; a second copy here answered nothing any producer could ask it. */ export function resolveLocalPtysSurviveQuit(getLocalPtysSurviveQuit?: () => boolean): boolean { if (!getLocalPtysSurviveQuit) { return false } try { - return getLocalPtysSurviveQuit() === true + return getLocalPtysSurviveQuit() } catch { return false } diff --git a/src/preload/index.ts b/src/preload/index.ts index ae41a528c49..2c7f26207c1 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -19,10 +19,8 @@ import { import type { DocPreviewGrantRequest } from './api/doc-preview-api' import type { AppIdentity } from '../shared/app-identity' import type { PtyProcessInspectionEvidence } from '../shared/pty-process-inspection-evidence' -import { - readWindowCloseRequestPayload, - type WindowCloseRequestPayload -} from '../shared/window-close-request' +import type { WindowCloseRequestPayload } from '../shared/window-close-request' +import { subscribeToWindowCloseRequest } from './window-close-request-subscription' import type { MacCapturedDigitRowChord } from '../shared/macos-symbolic-hotkeys' import type { ComputerAwakeStatus } from '../shared/computer-awake-mode' import type { @@ -4534,16 +4532,8 @@ const api = { /** Fired by main when the user tries to close the window; renderer confirms running * terminals then calls confirmWindowClose(). A quit (Cmd+Q / app.quit) skips that * dialog only when main also reports the local PTYs survive it. */ - onWindowCloseRequested: (callback: (data: WindowCloseRequestPayload) => void): (() => void) => { - const listener = (_event: Electron.IpcRendererEvent, data: unknown): void => { - const payload = readWindowCloseRequestPayload(data) - // Why: main cannot reach will-quit while a frozen renderer owns the window close handshake. - ipcRenderer.send('window:close-request-received', payload.requestId) - callback(payload) - } - ipcRenderer.on('window:close-requested', listener) - return () => ipcRenderer.removeListener('window:close-requested', listener) - }, + onWindowCloseRequested: (callback: (data: WindowCloseRequestPayload) => void): (() => void) => + subscribeToWindowCloseRequest(ipcRenderer, callback), /** Tell the main process to proceed with the window close. */ confirmWindowClose: (): void => { ipcRenderer.send('window:confirm-close') diff --git a/src/preload/window-close-request-subscription.test.ts b/src/preload/window-close-request-subscription.test.ts new file mode 100644 index 00000000000..21bd762d0c5 --- /dev/null +++ b/src/preload/window-close-request-subscription.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it, vi } from 'vitest' +import type { WindowCloseRequestPayload } from '../shared/window-close-request' +import { subscribeToWindowCloseRequest } from './window-close-request-subscription' + +type Listener = (event: unknown, data: unknown) => void + +function installIpc(): { + ipcRenderer: Parameters[0] + emit: (data: unknown) => void + sent: unknown[][] + removed: Listener[] +} { + const listeners: Listener[] = [] + const sent: unknown[][] = [] + const removed: Listener[] = [] + const ipcRenderer = { + on: vi.fn((_channel: string, listener: Listener) => { + listeners.push(listener) + return ipcRenderer + }), + removeListener: vi.fn((_channel: string, listener: Listener) => { + removed.push(listener) + return ipcRenderer + }), + send: vi.fn((_channel: string, ...args: unknown[]) => { + sent.push(args) + }) + } as unknown as Parameters[0] + return { + ipcRenderer, + emit: (data: unknown) => { + for (const listener of listeners) { + listener({}, data) + } + }, + sent, + removed + } +} + +function deliver(data: unknown): { + payload: WindowCloseRequestPayload | null + sent: unknown[][] +} { + const { ipcRenderer, emit, sent } = installIpc() + let payload: WindowCloseRequestPayload | null = null + subscribeToWindowCloseRequest(ipcRenderer, (received) => { + payload = received + }) + emit(data) + return { payload, sent } +} + +/** + * The whole quit-survival property rests on this hop: main's answer crosses IPC + * untyped, and the renderer spends `localPtysSurviveQuit: true` by closing over + * running terminals with no warning. Nothing else in the tree reads the payload, + * so if the normalization is skipped here it is skipped everywhere — which is + * exactly what happened while this listener was inline in index.ts and no test + * could reach it. + */ +describe('window close request subscription', () => { + it('delivers an explicit survival yes unchanged', () => { + const { payload } = deliver({ isQuitting: true, localPtysSurviveQuit: true, requestId: 4 }) + + expect(payload).toEqual({ isQuitting: true, localPtysSurviveQuit: true, requestId: 4 }) + }) + + it.each([ + ['a truthy string', 'yes'], + ['a truthy number', 1], + ['an object', {}], + ['null', null] + ])('refuses to spend %s as a survival yes', (_label, value) => { + const { payload } = deliver({ isQuitting: true, localPtysSurviveQuit: value }) + + expect(payload?.localPtysSurviveQuit).toBe(false) + }) + + it('reads a payload with no survival field as "does not survive"', () => { + const { payload } = deliver({ isQuitting: true }) + + expect(payload?.localPtysSurviveQuit).toBe(false) + }) + + it('still answers when the payload is not an object at all', () => { + const { payload, sent } = deliver(undefined) + + expect(payload).toEqual({ + isQuitting: false, + localPtysSurviveQuit: false, + requestId: undefined + }) + expect(sent).toEqual([[undefined]]) + }) + + // Why the ack and not just the payload: main clears its force-destroy timer on an + // exact requestId match, so echoing a non-numeric one back would never match and a + // frozen-renderer quit would sit on the timer instead of being acknowledged. + it('acknowledges before the renderer runs, echoing only a numeric requestId', () => { + const order: string[] = [] + const { ipcRenderer, emit, sent } = installIpc() + vi.mocked(ipcRenderer.send).mockImplementation((_channel: string, ...args: unknown[]) => { + order.push('ack') + sent.push(args) + }) + subscribeToWindowCloseRequest(ipcRenderer, () => order.push('callback')) + + emit({ isQuitting: true, requestId: 9 }) + emit({ isQuitting: true, requestId: '9' }) + + expect(order).toEqual(['ack', 'callback', 'ack', 'callback']) + expect(sent).toEqual([[9], [undefined]]) + }) + + it('unsubscribes the listener it registered', () => { + const { ipcRenderer, removed } = installIpc() + + const dispose = subscribeToWindowCloseRequest(ipcRenderer, () => {}) + dispose() + + expect(removed).toHaveLength(1) + expect(removed[0]).toBe(vi.mocked(ipcRenderer.on).mock.calls[0][1]) + }) +}) diff --git a/src/preload/window-close-request-subscription.ts b/src/preload/window-close-request-subscription.ts new file mode 100644 index 00000000000..8ae8f99ae3e --- /dev/null +++ b/src/preload/window-close-request-subscription.ts @@ -0,0 +1,36 @@ +import type { IpcRenderer } from 'electron' +import { + readWindowCloseRequestPayload, + type WindowCloseRequestPayload +} from '../shared/window-close-request' + +const CLOSE_REQUESTED_CHANNEL = 'window:close-requested' +const CLOSE_REQUEST_RECEIVED_CHANNEL = 'window:close-request-received' + +/** + * Subscribes the renderer to main's `window:close-requested`. + * + * Why the payload is read here and not forwarded: this is the only boundary the + * survival answer crosses untyped, and the renderer spends + * `localPtysSurviveQuit: true` as permission to close over running work with no + * warning. So the rule that only an explicit yes is a yes lives once, in + * `readWindowCloseRequestPayload`, and this is its single call site — forwarding + * the raw payload would restore the unconditional quit bypass for anything that + * is not a clean boolean (docs/reference/ssh-execution-boundary.md). + * + * Extracted from the api object so that hop is reachable by a test at all; while + * it was inline in index.ts nothing in the tree could observe it. + */ +export function subscribeToWindowCloseRequest( + ipcRenderer: Pick, + callback: (data: WindowCloseRequestPayload) => void +): () => void { + const listener = (_event: Electron.IpcRendererEvent, data: unknown): void => { + const payload = readWindowCloseRequestPayload(data) + // Why: main cannot reach will-quit while a frozen renderer owns the window close handshake. + ipcRenderer.send(CLOSE_REQUEST_RECEIVED_CHANNEL, payload.requestId) + callback(payload) + } + ipcRenderer.on(CLOSE_REQUESTED_CHANNEL, listener) + return () => ipcRenderer.removeListener(CLOSE_REQUESTED_CHANNEL, listener) +}