fix(desktop): dispose orphaned PTYs on renderer crash/reload (follow-up to #48421)

apps/desktop/electron/main.ts

@@ -65,7 +65,6 @@ import {
 } from './desktop-uninstall'
 import { installEmbedReferer } from './embed-referer'
 import { readDirForIpc } from './fs-read-dir'
-import { resolvePickerDefaultPath } from './wsl-path-bridge'
 import { probeGatewayWebSocket } from './gateway-ws-probe'
 import { scanGitRepos } from './git-repo-scan'
 import {
@@ -97,6 +96,7 @@ import {
 import { createLinkTitleWindow, guardLinkTitleSession, readLinkTitleWindowTitle } from './link-title-window'
 import { serializeJsonBody, setJsonRequestHeaders } from './oauth-net-request'
 import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing'
+import { recoverRendererAfterCrash } from './renderer-crash-recovery'
 import {
   buildSessionWindowUrl,
   chatWindowWebPreferences,
@@ -131,6 +131,7 @@ import { buildPathExtCandidates, chooseUpdaterArgs, getVenvSitePackagesEntries,
 import { readWindowsUserEnvVar } from './windows-user-env'
 import { isPackagedInstallPath as isPackagedInstallPathUnderRoots } from './workspace-cwd'
 import { readWslWindowsClipboardImage } from './wsl-clipboard-image'
+import { resolvePickerDefaultPath } from './wsl-path-bridge'
 
 const USER_DATA_OVERRIDE = process.env.HERMES_DESKTOP_USER_DATA_DIR
 
@@ -7241,32 +7242,41 @@ function createWindow() {
 
   wireCommonWindowHandlers(mainWindow, zoomWiringForWindowKind('chat'))
 
-  mainWindow.webContents.on('render-process-gone', (_event, details) => {
+  mainWindow.webContents.on('render-process-gone', (event, details) => {
     rememberLog(`[renderer] render-process-gone reason=${details?.reason} exitCode=${details?.exitCode}`)
 
     if (details?.reason === 'crashed' || details?.reason === 'oom') {
-      const now = Date.now()
-      rendererReloadTimes = rendererReloadTimes.filter(t => now - t < RENDERER_RELOAD_WINDOW_MS)
-
-      if (rendererReloadTimes.length >= RENDERER_RELOAD_MAX) {
-        rememberLog(
-          `[renderer] suppressing reload: ${rendererReloadTimes.length} crashes within ${RENDERER_RELOAD_WINDOW_MS}ms (likely a crash loop)`
-        )
-
-        return
-      }
-
-      rendererReloadTimes.push(now)
-      setImmediate(() => {
-        if (!mainWindow || mainWindow.isDestroyed()) {
-          return
-        }
+      rendererReloadTimes = recoverRendererAfterCrash({
+        disposeTerminalSession,
+        maxReloads: RENDERER_RELOAD_MAX,
+        now: Date.now(),
+        onDisposeError: (terminalId, error) => {
+          rememberLog(
+            `[renderer] failed to dispose terminal ${terminalId} after crash: ${error instanceof Error ? error.message : String(error)}`
+          )
+        },
+        onReload: () => {
+          setImmediate(() => {
+            if (!mainWindow || mainWindow.isDestroyed()) {
+              return
+            }
 
-        try {
-          mainWindow.webContents.reload()
-        } catch (err) {
-          rememberLog(`[renderer] reload after crash failed: ${err?.message || err}`)
-        }
+            try {
+              mainWindow.webContents.reload()
+            } catch (err) {
+              rememberLog(`[renderer] reload after crash failed: ${err?.message || err}`)
+            }
+          })
+        },
+        onSuppress: count => {
+          rememberLog(
+            `[renderer] suppressing reload: ${count} crashes within ${RENDERER_RELOAD_WINDOW_MS}ms (likely a crash loop)`
+          )
+        },
+        reloadTimes: rendererReloadTimes,
+        reloadWindowMs: RENDERER_RELOAD_WINDOW_MS,
+        rendererWebContentsId: event.sender.id,
+        terminalSessions
       })
     }
   })

apps/desktop/electron/renderer-crash-recovery.test.ts

@@ -0,0 +1,144 @@
+import assert from 'node:assert/strict'
+
+import { test } from 'vitest'
+
+import { recoverRendererAfterCrash } from './renderer-crash-recovery'
+
+test('disposes only the crashed renderer terminals before scheduling reload', () => {
+  const calls: string[] = []
+
+  const terminalSessions = new Map([
+    ['matching-1', { webContentsId: 7 }],
+    ['other-renderer', { webContentsId: 8 }],
+    ['matching-2', { webContentsId: 7 }]
+  ])
+
+  const reloadTimes = recoverRendererAfterCrash({
+    disposeTerminalSession: id => calls.push(`dispose:${id}`),
+    maxReloads: 2,
+    now: 1_000,
+    onDisposeError: (id, error) => calls.push(`error:${id}:${error}`),
+    onReload: () => calls.push('reload'),
+    onSuppress: count => calls.push(`suppress:${count}`),
+    reloadTimes: [],
+    reloadWindowMs: 500,
+    rendererWebContentsId: 7,
+    terminalSessions
+  })
+
+  assert.deepEqual(calls, ['dispose:matching-1', 'dispose:matching-2', 'reload'])
+  assert.deepEqual(reloadTimes, [1_000])
+})
+
+test('disposes the crashed renderer terminals before reload-loop suppression', () => {
+  const calls: string[] = []
+
+  const terminalSessions = new Map([
+    ['matching', { webContentsId: 7 }],
+    ['other-renderer', { webContentsId: 8 }]
+  ])
+
+  const reloadTimes = recoverRendererAfterCrash({
+    disposeTerminalSession: id => calls.push(`dispose:${id}`),
+    maxReloads: 2,
+    now: 1_000,
+    onDisposeError: (id, error) => calls.push(`error:${id}:${error}`),
+    onReload: () => calls.push('reload'),
+    onSuppress: count => calls.push(`suppress:${count}`),
+    reloadTimes: [700, 900],
+    reloadWindowMs: 500,
+    rendererWebContentsId: 7,
+    terminalSessions
+  })
+
+  assert.deepEqual(calls, ['dispose:matching', 'suppress:2'])
+  assert.deepEqual(reloadTimes, [700, 900])
+})
+
+test('expired reload attempts do not suppress recovery', () => {
+  const calls: string[] = []
+
+  const reloadTimes = recoverRendererAfterCrash({
+    disposeTerminalSession: id => calls.push(`dispose:${id}`),
+    maxReloads: 2,
+    now: 1_000,
+    onDisposeError: (id, error) => calls.push(`error:${id}:${error}`),
+    onReload: () => calls.push('reload'),
+    onSuppress: count => calls.push(`suppress:${count}`),
+    reloadTimes: [100, 700],
+    reloadWindowMs: 500,
+    rendererWebContentsId: 7,
+    terminalSessions: new Map([['matching', { webContentsId: 7 }]])
+  })
+
+  assert.deepEqual(calls, ['dispose:matching', 'reload'])
+  assert.deepEqual(reloadTimes, [700, 1_000])
+})
+
+test('continues cleanup and reload when one terminal disposer throws', () => {
+  const calls: string[] = []
+
+  const terminalSessions = new Map([
+    ['broken', { webContentsId: 7 }],
+    ['healthy', { webContentsId: 7 }]
+  ])
+
+  recoverRendererAfterCrash({
+    disposeTerminalSession: id => {
+      calls.push(`dispose:${id}`)
+
+      if (id === 'broken') {
+        throw new Error('kill failed')
+      }
+    },
+    maxReloads: 2,
+    now: 1_000,
+    onDisposeError: (id, error) => calls.push(`error:${id}:${error instanceof Error ? error.message : error}`),
+    onReload: () => calls.push('reload'),
+    onSuppress: count => calls.push(`suppress:${count}`),
+    reloadTimes: [],
+    reloadWindowMs: 500,
+    rendererWebContentsId: 7,
+    terminalSessions
+  })
+
+  assert.deepEqual(calls, [
+    'dispose:broken',
+    'error:broken:kill failed',
+    'dispose:healthy',
+    'reload'
+  ])
+})
+
+test('continues cleanup and reload when dispose error reporting throws', () => {
+  const calls: string[] = []
+
+  const terminalSessions = new Map([
+    ['broken', { webContentsId: 7 }],
+    ['healthy', { webContentsId: 7 }]
+  ])
+
+  recoverRendererAfterCrash({
+    disposeTerminalSession: id => {
+      calls.push(`dispose:${id}`)
+
+      if (id === 'broken') {
+        throw new Error('kill failed')
+      }
+    },
+    maxReloads: 2,
+    now: 1_000,
+    onDisposeError: id => {
+      calls.push(`error:${id}`)
+      throw new Error('logger failed')
+    },
+    onReload: () => calls.push('reload'),
+    onSuppress: count => calls.push(`suppress:${count}`),
+    reloadTimes: [],
+    reloadWindowMs: 500,
+    rendererWebContentsId: 7,
+    terminalSessions
+  })
+
+  assert.deepEqual(calls, ['dispose:broken', 'error:broken', 'dispose:healthy', 'reload'])
+})

apps/desktop/electron/renderer-crash-recovery.ts

@@ -0,0 +1,58 @@
+interface RendererTerminalSession {
+  webContentsId?: number
+}
+
+interface RendererCrashRecoveryOptions {
+  disposeTerminalSession: (id: string) => unknown
+  maxReloads: number
+  now: number
+  onDisposeError: (id: string, error: unknown) => void
+  onReload: () => void
+  onSuppress: (count: number) => void
+  reloadTimes: number[]
+  reloadWindowMs: number
+  rendererWebContentsId: number | null | undefined
+  terminalSessions: ReadonlyMap<string, RendererTerminalSession>
+}
+
+export function recoverRendererAfterCrash({
+  disposeTerminalSession,
+  maxReloads,
+  now,
+  onDisposeError,
+  onReload,
+  onSuppress,
+  reloadTimes,
+  reloadWindowMs,
+  rendererWebContentsId,
+  terminalSessions
+}: RendererCrashRecoveryOptions): number[] {
+  if (rendererWebContentsId != null) {
+    for (const [terminalId, sessionInfo] of [...terminalSessions.entries()]) {
+      if (sessionInfo.webContentsId === rendererWebContentsId) {
+        try {
+          disposeTerminalSession(terminalId)
+        } catch (error) {
+          try {
+            onDisposeError(terminalId, error)
+          } catch {
+            // Renderer recovery must not depend on error-reporting success.
+          }
+        }
+      }
+    }
+  }
+
+  const recentReloadTimes = reloadTimes.filter(time => now - time < reloadWindowMs)
+
+  if (recentReloadTimes.length >= maxReloads) {
+    onSuppress(recentReloadTimes.length)
+
+    return recentReloadTimes
+  }
+
+  recentReloadTimes.push(now)
+  onReload()
+
+  return recentReloadTimes
+}