fix-ota-again (squashed)

Author: frenchie4111

src/main/index.ts

@@ -1,4 +1,4 @@
-import { app, BrowserWindow, ipcMain, dialog, Menu, shell } from 'electron'
+import { app, autoUpdater as nativeAutoUpdater, BrowserWindow, ipcMain, dialog, Menu, shell } from 'electron'
 import { autoUpdater } from 'electron-updater'
 import { existsSync, readdirSync, statSync } from 'fs'
 import { homedir } from 'os'
@@ -705,14 +705,19 @@ function registerIpcHandlers(): void {
   })
 
   ipcMain.handle('updater:quitAndInstall', () => {
-    log('updater', 'quitAndInstall requested — cleaning up for fast exit')
+    log('updater', 'quitAndInstall requested — tearing down before handing off to Squirrel')
     try {
       stopWatchingStatus?.()
       stopWatchingStatus = null
     } catch (err) {
       log('updater', 'stopWatchingStatus failed', err instanceof Error ? err.message : String(err))
     }
     try {
+      // Kill the whole PTY process group (zsh + claude + any grandchildren),
+      // not just the direct shell child. Leaving descendants alive keeps
+      // libuv handles attached on our side, which makes Electron's quit
+      // sequence hang — and Squirrel.Mac then bails with "original process
+      // did not end" before ShipIt swaps the bundle.
       ptyManager.killAll('SIGKILL')
     } catch (err) {
       log('updater', 'ptyManager.killAll failed', err instanceof Error ? err.message : String(err))
@@ -724,18 +729,10 @@ function registerIpcHandlers(): void {
       log('updater', 'final persistence failed', err instanceof Error ? err.message : String(err))
     }
 
-    // Skip our before-quit handler — it's already been done above, and it can
-    // hang waiting for PTY fds to drain. We just want Squirrel to see us gone.
+    // Skip our before-quit handler — we just did its work above.
     app.removeAllListeners('before-quit')
 
-    // Hard-exit fallback. If Squirrel/Electron's quit takes longer than ~1.5s,
-    // force-kill the process so ShipIt's "target still running" check passes.
-    setTimeout(() => {
-      log('updater', 'fallback app.exit(0) — Squirrel should take over')
-      app.exit(0)
-    }, 1500)
-
-    autoUpdater.quitAndInstall(true, false)
+    autoUpdater.quitAndInstall(true, true)
     return true
   })
 
@@ -888,10 +885,23 @@ function setupAutoUpdater(): void {
     broadcastToAllWindows('updater:status', { state: 'downloaded', version: info.version })
   })
 
-  // Check on startup, then every 10 minutes
-  autoUpdater.checkForUpdatesAndNotify().catch((err) => log('updater', 'check failed', err.message))
+  // Also log native Squirrel.Mac errors. electron-updater wraps Squirrel via
+  // its own MacUpdater but doesn't surface errors from the native side, so
+  // things like "target app still running" or codesign mismatches would
+  // otherwise be invisible. These are the errors that would have diagnosed
+  // previous OTA loops in one glance.
+  if (process.platform === 'darwin') {
+    nativeAutoUpdater.on('error', (err) => {
+      log('updater', `[error] Squirrel.Mac: ${err.message}`)
+    })
+  }
+
+  // Check on startup, then every 10 minutes. We use checkForUpdates (not
+  // checkForUpdatesAndNotify) so there's no native OS notification — the
+  // renderer shows an in-app banner based on the updater:status events.
+  autoUpdater.checkForUpdates().catch((err) => log('updater', 'check failed', err.message))
   setInterval(() => {
-    autoUpdater.checkForUpdatesAndNotify().catch(() => {})
+    autoUpdater.checkForUpdates().catch(() => {})
   }, 10 * 60 * 1000)
 }
 
@@ -951,24 +961,14 @@ app.on('window-all-closed', () => {
   }
 })
 
-let quitWatchdogArmed = false
 app.on('before-quit', () => {
   stopWatchingStatus?.()
   stopWatchingStatus = null
-  ptyManager.killAll()
+  // SIGKILL the whole PTY process group so zsh + claude + grandchildren
+  // all die immediately and release their libuv handles. Without this the
+  // main process can hang draining fds and Squirrel.Mac will abort an
+  // in-flight bundle swap with "original process did not end".
+  ptyManager.killAll('SIGKILL')
   sealAllActive()
   saveConfigSync(config)
-
-  // Force-exit watchdog: if a PTY child (stuck claude / shell) won't die after
-  // SIGHUP, the main process can hang indefinitely draining fds. Give the
-  // graceful path ~1.5s, then hard-exit. electron-updater's ShipIt helper has
-  // already been spawned by this point if an update is pending, so force-exit
-  // is safe — ShipIt runs independently and swaps the bundle once we're gone.
-  if (!quitWatchdogArmed) {
-    quitWatchdogArmed = true
-    setTimeout(() => {
-      log('app', 'quit watchdog fired — force-exiting')
-      app.exit(0)
-    }, 1500).unref()
-  }
 })

src/main/pty-manager.ts

@@ -96,13 +96,29 @@ export class PtyManager {
   kill(id: string, signal?: string): void {
     log('pty', `kill id=${id}${signal ? ` signal=${signal}` : ''}`)
     const instance = this.ptys.get(id)
-    if (instance) {
+    if (!instance) return
+    this.ptys.delete(id)
+
+    const pid = instance.pty.pid
+    // node-pty calls setsid() for each spawn, so the shell is its own
+    // process-group leader and PGID === pid. Signalling -pid delivers to
+    // the whole group (zsh + claude + any descendants) in one syscall,
+    // instead of leaving grandchildren attached to our libuv handles.
+    if (pid && pid > 0) {
       try {
-        instance.pty.kill(signal)
+        process.kill(-pid, (signal as NodeJS.Signals) || 'SIGKILL')
       } catch {
-        // ignore — pty may already be dead
+        // Group may already be dead.
       }
-      this.ptys.delete(id)
+    }
+    try {
+      // Belt & suspenders: close the master fd and let node-pty tear down
+      // its own handles. Without this, the read stream on the master can
+      // keep Electron's quit sequence waiting even after the descendants
+      // are gone.
+      instance.pty.kill(signal)
+    } catch {
+      // pty already dead
     }
   }
 

src/renderer/App.tsx

@@ -1,5 +1,5 @@
 import { useState, useEffect, useCallback, useRef, useMemo } from 'react'
-import type { Worktree, TerminalTab, PtyStatus, PRStatus, QuestStep, WorkspacePane, PendingWorktree } from './types'
+import type { Worktree, TerminalTab, PtyStatus, PRStatus, QuestStep, WorkspacePane, PendingWorktree, UpdaterStatus } from './types'
 import type { Action } from './hotkeys'
 import { resolveHotkeys } from './hotkeys'
 import { HotkeysProvider } from './components/Tooltip'
@@ -84,6 +84,8 @@ export default function App(): JSX.Element {
   // Populated whenever the worktree list refreshes.
   const worktreeRepoRef = useRef<Record<string, string>>({})
   const [hooksConsent, setHooksConsent] = useState<'pending' | 'accepted' | 'declined'>('pending')
+  const [updaterStatus, setUpdaterStatus] = useState<UpdaterStatus | null>(null)
+  const [updateBannerDismissed, setUpdateBannerDismissed] = useState(false)
   const [sidebarVisible, setSidebarVisible] = useState(true)
   const [sidebarWidth, setSidebarWidth] = useState<number>(() => {
     const saved = Number(localStorage.getItem('harness:sidebarWidth'))
@@ -615,6 +617,20 @@ const setQuestStep = useCallback((next: QuestStep) => {
     })()
   }, [worktrees])
 
+  // Subscribe to auto-updater status for the in-app update banner.
+  useEffect(() => {
+    const cleanup = window.api.onUpdaterStatus((status) => {
+      setUpdaterStatus(status)
+      // If a new version shows up after a prior dismiss, re-show the banner.
+      if (status.state === 'downloaded') setUpdateBannerDismissed(false)
+    })
+    return cleanup
+  }, [])
+
+  const handleUpdateRestart = useCallback(() => {
+    void window.api.quitAndInstall()
+  }, [])
+
   const handleAddRepo = useCallback(async () => {
     const root = await window.api.addRepo()
     if (root) {
@@ -1357,6 +1373,27 @@ const setQuestStep = useCallback((next: QuestStep) => {
   return (
     <HotkeysProvider bindings={resolvedHotkeys}>
     <div className="flex h-full flex-col">
+      {/* Update-ready banner */}
+      {updaterStatus?.state === 'downloaded' && !updateBannerDismissed && (
+        <div className="bg-success/15 border-b border-success/30 pl-20 pr-4 py-2.5 drag-region flex items-center gap-3 shrink-0">
+          <span className="text-success text-sm flex-1">
+            Harness {updaterStatus.version} is ready to install. Restart to update.
+          </span>
+          <button
+            onClick={handleUpdateRestart}
+            className="px-3 py-1 bg-success/30 hover:bg-success/40 rounded text-sm text-success transition-colors shrink-0 cursor-pointer no-drag"
+          >
+            Restart &amp; install
+          </button>
+          <button
+            onClick={() => setUpdateBannerDismissed(true)}
+            className="px-3 py-1 text-success/80 hover:text-success text-sm transition-colors shrink-0 cursor-pointer no-drag"
+          >
+            Later
+          </button>
+        </div>
+      )}
+
       {/* Hooks consent banner */}
       {hooksConsent === 'pending' && (
         <div className="bg-warning/15 border-b border-warning/30 pl-20 pr-4 py-2.5 drag-region flex items-center gap-3 shrink-0">