perf(ipc): cap the flush handshake at one ipcMain listener and free window-rpc on teardown

apps/desktop/src/main/index.phase2.test.ts

@@ -1744,7 +1744,9 @@ describe('main index phase2 exports', () => {
     )?.[1] as (event: { preventDefault: () => void }) => void
     beforeQuitHandler({ preventDefault: vi.fn() })
 
-    expect(getFlushDoneListeners()).toHaveLength(browserWindows.length)
+    // One shared listener covers every in-flight flush; the request id is what
+    // separates them.
+    expect(getFlushDoneListeners()).toHaveLength(1)
 
     completeFlush(browserWindows[0])
     await flushUntil(() => closeVaultMock.mock.calls.length > 0, 40)
@@ -1755,6 +1757,40 @@ describe('main index phase2 exports', () => {
     expect(closeVaultMock).toHaveBeenCalled()
   })
 
+  it('keeps one flush-done listener when more than ten windows flush at once', async () => {
+    // `app:flush-done` lives on the shared ipcMain, and Node warns
+    // (MaxListenersExceededWarning) past ten listeners on one channel. A
+    // listener per window tripped that on any quit with enough windows open.
+    vi.useFakeTimers()
+    whenReadyMock.mockResolvedValue(undefined)
+
+    await importMainModule()
+    await flushReadyWork()
+    const { ipcMain } = await import('electron')
+
+    for (let i = 0; i < 12; i++) createBrowserWindowMock()
+    expect(browserWindows.length).toBeGreaterThan(10)
+
+    const beforeQuitHandler = appOnMock.mock.calls.find(
+      ([event]) => event === 'before-quit'
+    )?.[1] as (event: { preventDefault: () => void }) => void
+    beforeQuitHandler({ preventDefault: vi.fn() })
+
+    const flushListeners = getFlushDoneListeners()
+    expect(flushListeners).toHaveLength(1)
+    // Every window still got its own request, so no flush is silently skipped.
+    const requestIds = browserWindows.map((window) => getFlushRequestId(window))
+    expect(requestIds.every((requestId) => typeof requestId === 'string')).toBe(true)
+    expect(new Set(requestIds).size).toBe(browserWindows.length)
+
+    for (const window of browserWindows) completeFlush(window)
+    await flushUntil(() => closeVaultMock.mock.calls.length > 0)
+
+    expect(closeVaultMock).toHaveBeenCalled()
+    // Nothing is pending any more, so the shared listener is detached too.
+    expect(ipcMain.removeListener).toHaveBeenCalledWith('app:flush-done', flushListeners[0])
+  })
+
   it('ignores a flush-done from another window even when it carries the pending request id', async () => {
     // Defence in depth on top of the request id: no renderer may answer on
     // another window's behalf, because that window's saves are still in flight.

apps/desktop/src/main/index.ts

@@ -1916,6 +1916,52 @@ function registerQuickCaptureTestHooks(): void {
 // Track if shutdown is already in progress to prevent duplicate handling
 let isShuttingDown = false
 
+const FLUSH_DONE_CHANNEL = 'app:flush-done'
+
+interface PendingFlush {
+  windowId: number
+  webContents: Electron.WebContents
+  settle: () => void
+}
+
+// One entry per in-flight flush, keyed by request id. `app:flush-done` used to
+// get one ipcMain listener per window, so a multi-window quit registered N
+// listeners on the same channel and tripped Node's MaxListenersExceededWarning
+// past 10 windows. The request id already scopes each reply, so a single shared
+// listener plus this map does the same job with a constant listener count.
+const pendingFlushes = new Map<string, PendingFlush>()
+let flushDoneListenerAttached = false
+
+// A reply only counts when it comes from the window we asked AND answers the
+// request we are waiting on. Without both checks the first window to reply
+// resolved all of them, and a late reply to an earlier request satisfied the
+// next one — either way a renderer gets torn down with unsaved edits pending.
+const handleFlushDone = (event: IpcMainEvent, doneRequestId?: string): void => {
+  if (typeof doneRequestId !== 'string') return
+  const pending = pendingFlushes.get(doneRequestId)
+  if (!pending) return
+  if (event.sender !== pending.webContents) return
+  shutdownLog.info('flushWindow: flush-done received from window', pending.windowId)
+  pending.settle()
+}
+
+function addPendingFlush(requestId: string, pending: PendingFlush): void {
+  pendingFlushes.set(requestId, pending)
+  if (flushDoneListenerAttached) return
+  ipcMain.on(FLUSH_DONE_CHANNEL, handleFlushDone)
+  flushDoneListenerAttached = true
+}
+
+function removePendingFlush(requestId: string): void {
+  pendingFlushes.delete(requestId)
+  if (pendingFlushes.size > 0 || !flushDoneListenerAttached) return
+  // Detach once nothing is waiting. The timeout path used to skip cleanup: a
+  // renderer that never answers left a listener on the shared ipcMain forever,
+  // and window-close runs this on every close.
+  ipcMain.removeListener(FLUSH_DONE_CHANNEL, handleFlushDone)
+  flushDoneListenerAttached = false
+}
+
 function flushWindow(win: BrowserWindow, timeoutMs = 2000): Promise<void> {
   return new Promise<void>((resolve) => {
     if (win.isDestroyed() || !win.webContents) {
@@ -1926,39 +1972,23 @@ function flushWindow(win: BrowserWindow, timeoutMs = 2000): Promise<void> {
 
     shutdownLog.info('flushWindow: requesting flush from window', win.id)
 
-    const channel = 'app:flush-done'
     const requestId = randomUUID()
     let settled = false
 
     const settle = (): void => {
       if (settled) return
       settled = true
       clearTimeout(timer)
-      // The timeout path used to skip this: a renderer that never answers left a
-      // listener on the shared ipcMain forever, and window-close runs this on
-      // every close.
-      ipcMain.removeListener(channel, handler)
+      removePendingFlush(requestId)
       resolve()
     }
 
-    // `app:flush-done` is one shared channel for every in-flight flush, so a
-    // reply only counts when it comes from this window AND answers this request.
-    // Without both checks the first window to reply resolved all of them, and a
-    // late reply to an earlier request satisfied the next one — either way a
-    // renderer gets torn down with unsaved edits still pending.
-    const handler = (event: IpcMainEvent, doneRequestId?: string): void => {
-      if (event.sender !== win.webContents) return
-      if (doneRequestId !== requestId) return
-      shutdownLog.info('flushWindow: flush-done received from window', win.id)
-      settle()
-    }
-
     const timer = setTimeout(() => {
       shutdownLog.warn('flushWindow: timeout for window', win.id)
       settle()
     }, timeoutMs)
 
-    ipcMain.on(channel, handler)
+    addPendingFlush(requestId, { windowId: win.id, webContents: win.webContents, settle })
     win.webContents.send('app:request-flush', requestId)
   })
 }

apps/desktop/src/main/lib/window-rpc.test.ts

@@ -23,6 +23,27 @@ vi.mock('electron', () => ({
 import { ipcMain } from 'electron'
 import { mainToRendererInvoke } from './window-rpc'
 
+// Minimal stand-in for the target `webContents`: `send` plus the EventEmitter
+// surface the RPC uses to notice the window going away mid-call.
+function createWebContentsMock() {
+  const destroyedListeners = new Set<() => void>()
+  return {
+    send: vi.fn(),
+    once: vi.fn((event: string, listener: () => void) => {
+      if (event === 'destroyed') destroyedListeners.add(listener)
+    }),
+    removeListener: vi.fn((event: string, listener: () => void) => {
+      if (event === 'destroyed') destroyedListeners.delete(listener)
+    }),
+    emitDestroyed: () => {
+      for (const listener of [...destroyedListeners]) {
+        destroyedListeners.delete(listener)
+        listener()
+      }
+    }
+  }
+}
+
 describe('mainToRendererInvoke', () => {
   beforeEach(() => {
     vi.useFakeTimers()
@@ -37,7 +58,7 @@ describe('mainToRendererInvoke', () => {
   })
 
   it('sends a main invoke request and resolves with the matching renderer response', async () => {
-    const webContents = { send: vi.fn() }
+    const webContents = createWebContentsMock()
     const win = {
       isDestroyed: () => false,
       webContents
@@ -61,7 +82,7 @@ describe('mainToRendererInvoke', () => {
   })
 
   it('ignores responses from other renderers and resolves null on timeout', async () => {
-    const webContents = { send: vi.fn() }
+    const webContents = createWebContentsMock()
     const win = {
       isDestroyed: () => false,
       webContents
@@ -76,4 +97,32 @@ describe('mainToRendererInvoke', () => {
 
     await expect(promise).resolves.toBeNull()
   })
+
+  it('drops the per-call listener as soon as the target window is destroyed', async () => {
+    // A window that goes away mid-call never answers. Waiting out the full
+    // timeout parks a listener on the shared ipcMain (and the caller) for
+    // nothing, so teardown has to release it immediately.
+    const webContents = createWebContentsMock()
+    const win = {
+      isDestroyed: () => false,
+      webContents
+    } as unknown as Electron.BrowserWindow
+
+    const promise = mainToRendererInvoke<string>(win, 'agent_mcp:get_current_note', undefined, {
+      timeoutMs: 2_000
+    })
+
+    expect(hoisted.listeners.has('main:invoke:response:request-1')).toBe(true)
+
+    webContents.emitDestroyed()
+
+    await expect(promise).resolves.toBeNull()
+    expect(hoisted.listeners.has('main:invoke:response:request-1')).toBe(false)
+    expect(ipcMain.removeListener).toHaveBeenCalledWith(
+      'main:invoke:response:request-1',
+      expect.any(Function)
+    )
+    // Timers are still frozen: the cleanup happened on teardown, not on timeout.
+    expect(vi.getTimerCount()).toBe(0)
+  })
 })

apps/desktop/src/main/lib/window-rpc.ts

@@ -25,8 +25,11 @@ export async function mainToRendererInvoke<T>(
   options: MainToRendererInvokeOptions = {}
 ): Promise<T | null> {
   if (win.isDestroyed()) return null
-  if (typeof win.webContents.isDestroyed === 'function' && win.webContents.isDestroyed())
-    return null
+  // Resolve the target once: reading `win.webContents` again from inside the
+  // response handler throws if the window was destroyed while the call was in
+  // flight, and the captured reference keeps the sender check stable.
+  const target = win.webContents
+  if (typeof target.isDestroyed === 'function' && target.isDestroyed()) return null
 
   const requestId = randomUUID()
   const responseChannel = getResponseChannel(requestId)
@@ -38,6 +41,7 @@ export async function mainToRendererInvoke<T>(
     const cleanup = (): void => {
       clearTimeout(timeout)
       ipcMain.removeListener(responseChannel, handleResponse)
+      target.removeListener('destroyed', handleTargetDestroyed)
     }
 
     const settle = (result: T | null): void => {
@@ -47,17 +51,26 @@ export async function mainToRendererInvoke<T>(
       resolve(result)
     }
 
+    // A reply on this per-request channel only counts when it comes from the
+    // window we asked. A foreign sender is dropped rather than settled — letting
+    // it settle would hand any renderer a way to null out another window's call.
     const handleResponse = (event: IpcMainEvent, result: T | null): void => {
-      if (event.sender !== win.webContents) return
+      if (event.sender !== target) return
       settle(result)
     }
 
+    // The window went away before answering (closed, or reloaded out from under
+    // the request). No reply is ever coming, so drop the per-call listener now
+    // instead of parking it on the shared ipcMain until the timeout fires.
+    const handleTargetDestroyed = (): void => settle(null)
+
     ipcMain.on(responseChannel, handleResponse)
+    target.once('destroyed', handleTargetDestroyed)
     const timeout = setTimeout(() => settle(null), timeoutMs)
 
     try {
       const message: MainInvokePayload = { requestId, channel, payload }
-      win.webContents.send(MAIN_INVOKE_CHANNEL, message)
+      target.send(MAIN_INVOKE_CHANNEL, message)
     } catch {
       settle(null)
     }

apps/docs/src/architecture/ipc.md

@@ -122,3 +122,11 @@ The redundant work is therefore removed at the value, not at the sender. Subscri
 This matters most on the sync path: `sync/item-handlers/settings-handler.ts` re-broadcasts the whole merged `general` / `editor` / `inbox` group on every applied settings item, not only the fields that differ, so without the identity check each apply re-rendered every settings consumer in every window. React cannot use its eager-state shortcut on the first dispatch after a real state change, so that one still renders the hook's own component once before bailing out; every subsequent no-op echo costs zero renders.
 
 Write the merge as `setSettings((prev) => mergeSettingsPatch(prev, value))`, never `setSettings((prev) => ({ ...prev, ...value }))` — the spread mints a new object every time and defeats the bail-out. The comparison is shallow, matching the merge it guards; groups whose values are nested objects (keyboard bindings arrive as fresh references over IPC) simply never hit the bail-out, which is correct rather than a missed update.
+
+## Main → Renderer Request/Response
+
+Two main-process handshakes need an _answer_ from one specific window rather than a fan-out: `mainToRendererInvoke` in `src/main/lib/window-rpc.ts` (Vault MCP tools asking a window for the current note, a desktop API call, a canvas write) and the shutdown flush in `src/main/index.ts`, which asks each window to persist pending edits before the app quits. Both reply over `ipcMain`, which is process-wide, so both follow the same two rules.
+
+**Scope every reply to its request.** `mainToRendererInvoke` gives each call its own response channel (`main:invoke:response:<uuid>`); the flush handshake shares one `app:flush-done` channel and carries the request id in the payload. Both then also compare `event.sender` against the window that was asked. Without the sender check a renderer could answer on another window's behalf — for the flush that means a window gets torn down with unsaved edits still in flight. A reply from an unexpected sender is dropped, never treated as a settled answer.
+
+**Never leave a listener behind, and never scale listener count with window count.** Node warns (`MaxListenersExceededWarning`) past ten listeners on a single channel, so the flush keeps one shared `app:flush-done` listener plus a map of pending request ids: it attaches when the first flush starts and detaches when the last one settles, whether that was a reply or the 2 s timeout. A per-call listener is fine on a per-request channel, but it has to be released on every exit path — `mainToRendererInvoke` drops its listener on reply, on timeout, and as soon as the target `webContents` is destroyed, so a window closed mid-call resolves `null` right away instead of parking a listener for the rest of the timeout.