From cff9a8e2cf5b96d2889c86a657e08c550998253b Mon Sep 17 00:00:00 2001
From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com>
Date: Sun, 3 May 2026 12:19:51 +0200
Subject: [PATCH] polish(desktop): final cross-cutting fixes

Two real bugs from final review:
- Clear recovery state on window 'closed' so macOS dock-activate doesn't
  drop the recovery surface (window recreated with stale recoveryStore.mode)
- Hoist setOnQuitRequested before createWindow so synchronous boot failures
  reach a wired Quit handler

Three polish items:
- Tray's Change Instance now routes through handleRecoveryAction so both
  paths share one implementation; recovery action's change-instance also
  show()+focus() for hidden-window tray clicks
- install-update action guards against state.updateState !== 'downloaded'
  (defense in depth against malicious or buggy renderers)
- Object.freeze rationale documented in RecoveryStateStore.update
---
 packages/desktop/src/main.ts     | 22 +++++++++++++---------
 packages/desktop/src/recovery.ts | 14 ++++++++++++++
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/packages/desktop/src/main.ts b/packages/desktop/src/main.ts
index 4d3524b8..71752b73 100644
--- a/packages/desktop/src/main.ts
+++ b/packages/desktop/src/main.ts
@@ -372,6 +372,11 @@ function createWindow(): void {
   mainWindow.on('closed', () => {
     setMainWindow(null);
     mainWindow = null;
+    // Clear recovery state so macOS dock-activate (which calls createWindow again)
+    // gets a clean slate. Without this, the new window inherits inRecoveryMode=true
+    // and the recovery surface is silently lost.
+    recoveryStore.markRecoveryExited();
+    recoveryStore.update({ mode: 'normal', reason: null });
   });
 
   // Window focus IPC for notification suppression
@@ -895,23 +900,22 @@ if (!gotTheLock) {
     });
 
     registerIpcHandlers();
+    // Wire the recovery module's quit callback to the local requestQuit BEFORE
+    // createWindow so a synchronous did-fail-load on first load finds a wired
+    // Quit handler. requestQuit is a function declaration and is hoisted.
+    setOnQuitRequested(() => requestQuit());
     createWindow();
     createTray();
 
-    // Wire the recovery module's quit callback to the local requestQuit.
-    setOnQuitRequested(() => requestQuit());
-
     // Tray + macOS app-menu actions. Defined once so the subscriber and the
     // initial-fire share one implementation (no drift on future menu changes).
     const trayActions = {
       onShow: () => { mainWindow?.show(); mainWindow?.focus(); },
       onHide: () => mainWindow?.hide(),
-      onChangeInstance: () => {
-        clearInstanceUrl();
-        mainWindow?.loadFile(getPickerPath());
-        mainWindow?.show();
-        mainWindow?.focus();
-      },
+      // Delegate to handleRecoveryAction so both the tray and the recovery
+      // surface share one implementation path (avoids drift and ensures
+      // recovery state is always cleared on a Change Instance action).
+      onChangeInstance: () => handleRecoveryAction('change-instance'),
       onCheckForUpdates: () => handleRecoveryAction('check-update'),
       onRestartToInstall: () => handleRecoveryAction('install-update'),
       onQuit: () => requestQuit(),
diff --git a/packages/desktop/src/recovery.ts b/packages/desktop/src/recovery.ts
index be3f8191..80a8cfd3 100644
--- a/packages/desktop/src/recovery.ts
+++ b/packages/desktop/src/recovery.ts
@@ -36,6 +36,7 @@ const INITIAL_STATE: RecoveryState = {
 };
 
 export class RecoveryStateStore {
+  // Freeze so the initial get() return cannot be mutated by callers.
   private state: RecoveryState = Object.freeze({ ...INITIAL_STATE }) as RecoveryState;
   private listeners = new Set<(s: RecoveryState) => void>();
   private inRecoveryMode = false;
@@ -45,6 +46,9 @@ export class RecoveryStateStore {
   }
 
   update(partial: Partial<RecoveryState>): void {
+    // Freeze so consumers (incl. subscribers, IPC-cloned renderer reads via
+    // get-recovery-state) cannot mutate the shared state through the live
+    // reference returned by get(). Compile-time Readonly<> is a hint only.
     this.state = Object.freeze({ ...this.state, ...partial }) as RecoveryState;
     // Snapshot before iterating: a listener can subscribe/unsubscribe others
     // (or itself) during notification without affecting the current notify pass.
@@ -370,6 +374,10 @@ export function handleRecoveryAction(action: RecoveryAction): void {
       return;
     }
     case 'install-update': {
+      // Defense in depth: only act when an update is actually downloaded. The
+      // recovery.html UI hides the Restart button unless updateState='downloaded',
+      // but a buggy/malicious renderer could send this action at any time.
+      if (recoveryStore.get().updateState !== 'downloaded') return;
       // Direct quitAndInstall — does NOT rely on autoInstallOnAppQuit.
       // Force-kill-fix: if user reaches recovery and clicks here, install
       // happens cleanly even if the on-quit hook would otherwise be bypassed.
@@ -381,6 +389,12 @@ export function handleRecoveryAction(action: RecoveryAction): void {
       recoveryStore.markRecoveryExited();
       recoveryStore.update({ mode: 'normal', reason: null });
       mainWindowRef?.loadFile(getPickerPath());
+      // Ensure visible — tray clicks may happen with window hidden, and the
+      // recovery surface should also remain visible during the navigation.
+      // When invoked from recovery.html (window already showing), these are
+      // idempotent no-ops.
+      mainWindowRef?.show();
+      mainWindowRef?.focus();
       return;
     }
     case 'open-releases': {