fix(panes): break the tray/host import cycle; debounce saves; restore before show

Five findings from CodeRabbit on #103.

1. CIRCULAR IMPORT. pane-tray imported pane-hosts while pane-hosts imported
   pane-tray. In the main process that is not a style question: whichever module
   loses the load race sees the other's exports half-initialised, and it fails at
   whatever moment the graph happens to resolve in — which is to say, not on your
   machine.

   One direction only now: pane-hosts → pane-tray. What the tray needs from the
   host (toggle/showAll/hideAll/hasWindow/getMainWindow) is INJECTED through
   initTray(), wired in main.ts.

2. SYNCHRONOUS DISK WRITES ON EVERY DRAG FRAME. `save()` was wired straight to
   'moved'/'resized', and setDesktopConfig is writeFileSync + renameSync. On
   macOS that is dozens of blocking writes per second, in the main process,
   while the user drags.

   Debounced to 400ms — and then flushed on 'close', because a debounce that
   drops the last move is worse than no debounce: nudge a pane, close it a moment
   later, and you would lose the position you just chose, which is the exact thing
   remembered geometry exists to prevent. 'close' (not 'closed') because the
   window has to still exist to be measured.

3. A MINIMIZED PANE COULD NOT BE BROUGHT BACK. Panes go to the tray by being
   minimized and then hidden — and hiding a minimized window does not un-minimize
   it. So show() from the tray restored a window that was still minimized:
   present, but not on screen. Which reads as the tray being broken. Everything
   now goes through reveal(), which restores first.

4. PROTOTYPE POLLUTION VIA PANE ID. A pane id arrives from the RENDERER (it is
   the tail of the frame name window.open() supplied) and is used as a KEY in the
   persisted paneWindows map. `__proto__` is not an id, it is a way to mutate
   Object.prototype from a plugin. Rejected on write, the map is rebuilt on a
   null-prototype object, and reads are own-property checks — otherwise a polluted
   or hand-edited config hands back geometry for a pane that was never saved.

5. Removed getMainWindowRef(), exported and referenced nowhere.

Not applicable: the finding about req.width/req.height/req.title reaching
BrowserWindow unvalidated. That was the `pane:open` IPC, which no longer exists —
main does not create pane windows at all now (the renderer must, so it can adopt
its element into them). The sizes now come from the window Electron already made.

Signed-off-by: topkoa <topkoa@gmail.com>

src/main/main.ts

@@ -126,7 +126,10 @@ import * as updateManager from './update-manager';
 import type { UpdateChannel } from './update-manager';
 import { installAppMenu } from './app-menu';
 import { sanitizeWindowBounds, MIN_WIDTH, MIN_HEIGHT } from './window-bounds';
-import { initPaneHosts, closeAllPanes, adoptPaneWindow, paneIdFromFrameName } from './pane-hosts';
+import {
+    initPaneHosts, closeAllPanes, adoptPaneWindow, paneIdFromFrameName,
+    togglePaneWindow, showAllPaneWindows, hideAllPaneWindows, hasPaneWindow,
+} from './pane-hosts';
 import { initTray, destroyTray } from './pane-tray';
 
 // Linux: enable Chromium's PipeWire capturer feature so getUserMedia can see
@@ -1188,7 +1191,16 @@ async function startup(): Promise<void> {
     // Must come after createWindow — both reach the renderer through mainWindow,
     // and Tray requires a ready app.
     initPaneHosts({ getMainWindow: () => mainWindow });
-    initTray({ getMainWindow: () => mainWindow });
+    // The tray's pane actions are INJECTED, not imported: pane-hosts already imports
+    // pane-tray (to push the menu), and importing back would make the two modules
+    // mutually dependent — a require cycle whose loser sees half-initialised exports.
+    initTray({
+        getMainWindow: () => mainWindow,
+        toggleWindow: togglePaneWindow,
+        showAll: showAllPaneWindows,
+        hideAll: hideAllPaneWindows,
+        hasWindow: hasPaneWindow,
+    });
 
     // Install our application menu (replaces Electron's default so View →
     // Zoom In also accepts the unshifted Ctrl+= key — see app-menu.ts).

src/main/pane-hosts.ts

@@ -47,21 +47,69 @@ let getMainWindow: () => BrowserWindow | null = () => null;
 // ── Geometry ────────────────────────────────────────────────────────────────
 
 function savedFor(paneId: string): SavedPaneWindow {
-    return getDesktopConfig().paneWindows?.[paneId] ?? {};
+    const saved = getDesktopConfig().paneWindows ?? {};
+    // Own-property check: a polluted or hand-edited config would otherwise hand back
+    // a value off the prototype chain for a pane that was never saved at all.
+    return Object.prototype.hasOwnProperty.call(saved, paneId) ? saved[paneId] : {};
+}
+
+// A pane id reaches us from the RENDERER (it is the tail of the frame name a
+// window.open() supplied), and it is used as a key in the persisted map. So it is
+// untrusted input in key position: `__proto__` and friends are not ids, they are a
+// way to mutate Object.prototype from a plugin.
+const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
+function isUnsafePaneId(paneId: string): boolean {
+    return UNSAFE_KEYS.has(paneId);
 }
 
 function persist(paneId: string, patch: SavedPaneWindow): void {
+    if (isUnsafePaneId(paneId)) return;
     try {
         // setDesktopConfig merges shallowly, so paneWindows must be
         // read-modify-written or one pane's save would drop every other pane's.
-        const all = { ...(getDesktopConfig().paneWindows ?? {}) };
+        //
+        // Built on a null-prototype object: whatever is in the config file (hand
+        // edited, corrupt, or written by an older build) cannot smuggle a prototype
+        // into a map we then write keys onto.
+        const all: Record<string, SavedPaneWindow> = Object.create(null);
+        const saved = getDesktopConfig().paneWindows ?? {};
+        for (const key of Object.keys(saved)) {
+            if (!isUnsafePaneId(key)) all[key] = saved[key];
+        }
         all[paneId] = { ...(all[paneId] ?? {}), ...patch };
-        setDesktopConfig({ paneWindows: all });
+        setDesktopConfig({ paneWindows: { ...all } });
     } catch (err) {
         console.warn(`[panes] failed to persist geometry for ${paneId}:`, err);
     }
 }
 
+// setDesktopConfig is writeFileSync + renameSync. Wiring that straight to 'moved'
+// and 'resized' means a synchronous disk write for every frame of a drag — on
+// macOS, dozens per second, in the main process, where they block everything else.
+// Write once the gesture settles instead.
+const GEOMETRY_SAVE_DEBOUNCE_MS = 400;
+const saveTimers = new Map<string, NodeJS.Timeout>();
+
+function persistSoon(paneId: string, patch: () => SavedPaneWindow | null): void {
+    clearTimeout(saveTimers.get(paneId));
+    saveTimers.set(paneId, setTimeout(() => {
+        saveTimers.delete(paneId);
+        const value = patch();
+        if (value) persist(paneId, value);
+    }, GEOMETRY_SAVE_DEBOUNCE_MS));
+}
+
+// Debouncing means the last move can still be in flight when the window goes. Write
+// it out NOW, while the window is alive enough to be measured — otherwise a user who
+// nudges a pane and closes it a moment later loses the position they just chose,
+// which is exactly the thing remembered geometry exists to prevent.
+function flushGeometry(win: BrowserWindow, paneId: string): void {
+    const timer = saveTimers.get(paneId);
+    if (timer) { clearTimeout(timer); saveTimers.delete(paneId); }
+    if (win.isDestroyed()) return;
+    persist(paneId, { bounds: { ...win.getNormalBounds(), maximized: false } });
+}
+
 // ── Adoption ────────────────────────────────────────────────────────────────
 
 export function paneIdFromFrameName(frameName: string): string | null {
@@ -98,8 +146,8 @@ export function adoptPaneWindow(win: BrowserWindow, paneId: string): void {
     // in a crash, and the whole point of remembering geometry is that you never
     // place it twice.
     const save = (): void => {
-        if (win.isDestroyed()) return;
+        persistSoon(paneId, () =>
-        persist(paneId, { bounds: { ...win.getNormalBounds(), maximized: false } });
+            win.isDestroyed() ? null : { bounds: { ...win.getNormalBounds(), maximized: false } });
     };
     win.on('moved', save);
     win.on('resized', save);
@@ -114,6 +162,10 @@ export function adoptPaneWindow(win: BrowserWindow, paneId: string): void {
         refreshTray();
     });
 
+    // 'close' fires while the window still exists; 'closed' after it is gone. The
+    // final geometry can only be read from the former.
+    win.on('close', () => flushGeometry(win, paneId));
+
     win.on('closed', () => {
         windows.delete(paneId);
         refreshTray();
@@ -153,16 +205,26 @@ function refreshTray(): void {
     }));
 }
 
+// A pane is sent to the tray by MINIMIZING it and then hiding it — and hiding a
+// minimized window does not un-minimize it. So show() alone would restore a window
+// that is still minimized: present, but not on screen, which reads as the tray
+// being broken. Always restore first.
+function reveal(win: BrowserWindow): void {
+    if (win.isDestroyed()) return;
+    if (win.isMinimized()) win.restore();
+    win.show();
+}
+
 export function togglePaneWindow(paneId: string): boolean {
     const win = windows.get(paneId);
     if (!win || win.isDestroyed()) return false;   // not open → only the renderer can open it
-    if (win.isVisible()) win.hide(); else win.show();
+    if (win.isVisible() && !win.isMinimized()) win.hide(); else reveal(win);
     refreshTray();
     return true;
 }
 
 export function showAllPaneWindows(): void {
-    windows.forEach((win) => { if (!win.isDestroyed() && !win.isVisible()) win.show(); });
+    windows.forEach((win) => { if (!win.isDestroyed()) reveal(win); });
     refreshTray();
 }
 
@@ -191,7 +253,3 @@ export function initPaneHosts(deps: { getMainWindow: () => BrowserWindow | null
         refreshTray();
     });
 }
-
-export function getMainWindowRef(): BrowserWindow | null {
-    return getMainWindow();
-}

src/main/pane-tray.ts

@@ -18,7 +18,6 @@
 import { Menu, Tray, nativeImage, app } from 'electron';
 import * as path from 'path';
 import { IPC_PANE_EVENT_TOGGLE } from './ipc-channels';
-import { togglePaneWindow, showAllPaneWindows, hideAllPaneWindows, hasPaneWindow } from './pane-hosts';
 
 export interface TrayPane {
     id: string;
@@ -27,9 +26,25 @@ export interface TrayPane {
     open?: boolean;
 }
 
+// What the tray needs from the pane host, INJECTED rather than imported.
+//
+// pane-hosts imports setTrayPanes from this file, so importing back from there
+// would make the two modules mutually dependent — and a require cycle in the main
+// process is not a style question: whichever module loads second sees the other's
+// exports half-initialised, which fails at whatever moment the graph happens to
+// resolve in. One direction only: pane-hosts → pane-tray, and the actions come in
+// through initTray().
+export interface TrayPaneActions {
+    getMainWindow: () => Electron.BrowserWindow | null;
+    toggleWindow: (paneId: string) => boolean;
+    showAll: () => void;
+    hideAll: () => void;
+    hasWindow: (paneId: string) => boolean;
+}
+
 let tray: Tray | null = null;
 let panes: TrayPane[] = [];
-let getMainWindow: () => Electron.BrowserWindow | null = () => null;
+let actions: TrayPaneActions | null = null;
 
 function iconPath(): string {
     // Windows wants an .ico; macOS and Linux take a PNG. macOS additionally wants
@@ -40,7 +55,7 @@ function iconPath(): string {
 }
 
 function showMainWindow(): void {
-    const win = getMainWindow();
+    const win = actions?.getMainWindow() ?? null;
     if (!win || win.isDestroyed()) return;
     if (win.isMinimized()) win.restore();
     win.show();
@@ -56,8 +71,8 @@ function buildMenu(): Menu {
             // If we already own a window for this pane, showing/hiding it is a
             // main-process job and instant. If we don't, only the renderer can
             // decide what opening it means (it might belong in the dock), so ask.
-            if (hasPaneWindow(p.id)) { togglePaneWindow(p.id); return; }
+            if (actions?.hasWindow(p.id)) { actions.toggleWindow(p.id); return; }
-            const win = getMainWindow();
+            const win = actions?.getMainWindow() ?? null;
             if (win && !win.isDestroyed()) win.webContents.send(IPC_PANE_EVENT_TOGGLE, { paneId: p.id });
         },
     }));
@@ -71,8 +86,8 @@ function buildMenu(): Menu {
         template.push({ label: 'Panes', enabled: false });
         template.push(...paneItems);
         template.push({ type: 'separator' });
-        template.push({ label: 'Show all panes', click: showAllPaneWindows });
+        template.push({ label: 'Show all panes', click: () => actions?.showAll() });
-        template.push({ label: 'Hide all panes', click: hideAllPaneWindows });
+        template.push({ label: 'Hide all panes', click: () => actions?.hideAll() });
     } else {
         template.push({ label: 'No panes', enabled: false });
     }
@@ -92,9 +107,9 @@ export function setTrayPanes(next: TrayPane[]): void {
     tray.setContextMenu(buildMenu());
 }
 
-export function initTray(deps: { getMainWindow: () => Electron.BrowserWindow | null }): void {
+export function initTray(deps: TrayPaneActions): void {
     if (tray) return;
-    getMainWindow = deps.getMainWindow;
+    actions = deps;
 
     const image = nativeImage.createFromPath(iconPath());
     if (image.isEmpty()) {