sim: merge PR #17076

src/renderer/src/components/terminal/running-terminal-close-absence-evidence.test.ts

@@ -132,9 +132,7 @@ describe('terminal-tab close on PTY absence evidence', () => {
     expect(onClose).not.toHaveBeenCalled()
   })
 
-  it('leaves a degraded in-contact probe on its existing close-silently behavior', async () => {
-    // Loss of contact with the child-process probe on a pane the host still routes to:
-    // no `unavailable`, so this guard reaches no new verdict and behaves as it always has.
+  it('asks when an in-contact child-process probe is unverifiable', async () => {
     inspectRuntimeTerminalProcessMock.mockResolvedValue(
       buildPtyProcessInspectionWireResult(
         { verdict: 'unverifiable', reason: 'process table scan degraded' },
@@ -144,7 +142,7 @@ describe('terminal-tab close on PTY absence evidence', () => {
 
     const onClose = await closeTab()
 
-    expect(visibleRequest()).toBeNull()
-    expect(onClose).toHaveBeenCalledTimes(1)
+    expect(visibleRequest()).not.toBeNull()
+    expect(onClose).not.toHaveBeenCalled()
   })
 })

src/renderer/src/components/terminal/running-terminal-close-guard.test.ts

@@ -181,13 +181,40 @@ describe('guardRunningTerminalClose', () => {
     expect(onClose).not.toHaveBeenCalled()
   })
 
-  it('fails open and closes when the probe rejects (wedged relay / legacy provider)', async () => {
+  // Why not fail open: a rejection observed nothing, and this close kills the pty — the
+  // same destruction the window-close path stops and asks about on a rejected probe.
+  it('asks when the probe rejects for a tracked pty (wedged relay / legacy provider)', async () => {
     inspectRuntimeTerminalProcessMock.mockRejectedValue(new Error('rpc_timeout'))
     const onClose = vi.fn()
 
     guard(onClose)
     await settleProbe()
 
+    expect(onClose).not.toHaveBeenCalled()
+    expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1' })
+  })
+
+  // Why per-id and not per-tab: the rejection arm is narrowed by the same liveness map the
+  // `unavailable` arm uses, so a stale layout leaf cannot drag a closable tab into a prompt.
+  it('closes when only a layout-only pty rejects and the tracked pane is idle', async () => {
+    setState({
+      ptyIdsByTabId: { 'tab-1': ['pty-a'] },
+      terminalLayoutsByTabId: {
+        'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-stale' } }
+      }
+    })
+    inspectRuntimeTerminalProcessMock.mockImplementation(async (_settings, ptyId: string) => {
+      if (ptyId === 'pty-stale') {
+        throw new Error('no registered provider owns this PTY id')
+      }
+      return { foregroundProcess: 'zsh', hasChildProcesses: false }
+    })
+    const onClose = vi.fn()
+
+    guard(onClose)
+    await settleProbe()
+
+    expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledTimes(2)
     expect(onClose).toHaveBeenCalledTimes(1)
     expect(visibleRequest()).toBeNull()
   })
@@ -196,9 +223,11 @@ describe('guardRunningTerminalClose', () => {
     // Why not fail open: the host answered "I could not route to this pane", which is the
     // same non-answer this guard's own timeout already prompts on. It applies only to an id
     // the liveness map still vouches for; see running-terminal-close-absence-evidence.test.ts.
+    // `hasChildProcesses` stays false so this exercises the `unavailable` rule, not the
+    // live-child rule above it.
     inspectRuntimeTerminalProcessMock.mockResolvedValue({
       foregroundProcess: null,
-      hasChildProcesses: true,
+      hasChildProcesses: false,
       unavailable: true
     })
     const onClose = vi.fn()
@@ -210,6 +239,24 @@ describe('guardRunningTerminalClose', () => {
     expect(visibleRequest()).not.toBeNull()
   })
 
+  it('asks when a tracked pty child-process inspection is unverifiable', async () => {
+    inspectRuntimeTerminalProcessMock.mockResolvedValue({
+      foregroundProcess: null,
+      hasChildProcesses: false,
+      processEvidence: {
+        foreground: { verdict: 'unverifiable', reason: 'ps timed out' },
+        children: { verdict: 'unverifiable', reason: 'ps timed out' }
+      }
+    })
+    const onClose = vi.fn()
+
+    guard(onClose)
+    await settleProbe()
+
+    expect(onClose).not.toHaveBeenCalled()
+    expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1' })
+  })
+
   it('prompts once for a split tab where only the second pane is busy', async () => {
     setState({
       ptyIdsByTabId: { 'tab-1': ['pty-a', 'pty-b'] },
@@ -357,6 +404,38 @@ describe('guardRunningTerminalClose', () => {
     expect(visibleRequest()?.copyKind).toBe('agent')
   })
 
+  it('closes after a tracked pty exits when only a stale layout probe times out', async () => {
+    setState({
+      ptyIdsByTabId: { 'tab-1': ['pty-a'] },
+      terminalLayoutsByTabId: {
+        'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-stale' } }
+      }
+    })
+    inspectRuntimeTerminalProcessMock.mockImplementation(async (_settings, ptyId: string) => {
+      if (ptyId === 'pty-stale') {
+        return new Promise(() => {})
+      }
+      return {
+        foregroundProcess: 'zsh',
+        hasChildProcesses: false,
+        processEvidence: {
+          foreground: { verdict: 'observed' as const, processName: 'zsh' },
+          children: { verdict: 'exited' as const }
+        }
+      }
+    })
+    vi.useFakeTimers()
+    const onClose = vi.fn()
+
+    guard(onClose)
+    await settleProbe()
+    vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS)
+    vi.useRealTimers()
+
+    expect(onClose).toHaveBeenCalledTimes(1)
+    expect(visibleRequest()).toBeNull()
+  })
+
   it('closes rather than wedging when the timed-out prompt throws', async () => {
     const requestSpy = vi
       .spyOn(useRunningTerminalCloseConfirmStore.getState(), 'requestRunningTerminalCloseConfirm')
@@ -395,9 +474,9 @@ describe('guardRunningTerminalClose', () => {
   })
 
   // Why: an SSH drop zeroes ptyIdsByTabId while the layout still names the pane. The stale
-  // binding is probed, that probe fails on the dead link, and the close falls open — so a
-  // reconnecting tab stays closable instead of being blocked behind a prompt for a pty
-  // nobody can reach. Documented so the behavior is a decision, not an accident.
+  // binding is probed and that probe fails on the dead link, but the id is layout-only, so
+  // the rejection arm's narrowing lets the close through — a reconnecting tab stays closable
+  // instead of being blocked behind a prompt for a pty nobody can reach.
   it('closes a reconnecting ssh tab whose pty ids were already zeroed', async () => {
     setState({ ptyIdsByTabId: { 'tab-1': [] } })
     inspectRuntimeTerminalProcessMock.mockRejectedValue(new Error('ssh_disconnected'))

src/renderer/src/components/terminal/running-terminal-close-guard.ts

@@ -1,8 +1,12 @@
 import { useAppStore } from '@/store'
-import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection'
+import {
+  inspectRuntimeTerminalProcess,
+  type RuntimeTerminalProcessInspection
+} from '@/runtime/runtime-terminal-inspection'
 import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm'
 import type { TerminalTabCloseReason } from '@/store/slices/terminal-tab-retirement'
 import type { AppState } from '@/store/types'
+import { readPtyProcessInspectionEvidence } from '../../../../shared/pty-process-inspection-evidence'
 import { resolveBusyPtyCloseCopyKind } from './terminal-close-copy-kind'
 
 export type RunningTerminalCloseGuardOptions = {
@@ -64,6 +68,36 @@ function collectTabPtyIds(
   return { ptyIds: [...ptyIds], trackedPtyIds }
 }
 
+type SettledCloseProbe =
+  | { status: 'fulfilled'; value: RuntimeTerminalProcessInspection }
+  | { status: 'rejected' }
+
+function shouldConfirmForProbe(
+  ptyId: string,
+  trackedPtyIds: ReadonlySet<string>,
+  probe: SettledCloseProbe | undefined
+): boolean {
+  const tracked = trackedPtyIds.has(ptyId)
+  if (probe === undefined || probe.status === 'rejected') {
+    return tracked
+  }
+  if (probe.value.unavailable === true) {
+    return tracked
+  }
+  const children = readPtyProcessInspectionEvidence(probe.value).children
+  // Why the verdict alone decides, with no vote from `hasChildProcesses`: the boolean is
+  // `children.verdict === 'live'` collapsed, so it says nothing new on the positive pole and
+  // nothing trustworthy on the others. The one producer that publishes `true` beside a
+  // non-`live` verdict is a daemon pane whose handle has no evidence channel, and that host
+  // states outright that such a read proves neither life nor exit — so voting on it would ask
+  // for a non-shell title and close silently for a shell one, off the very same degraded read.
+  // The window-close guard reads this same single signal (#17077).
+  if (children.verdict === 'live') {
+    return true
+  }
+  return children.verdict === 'unverifiable' && tracked
+}
+
 /**
  * Routes an interactive terminal-tab close through the running-process confirmation.
  * Closes immediately when nothing is running, so idle tabs keep today's behavior.
@@ -113,48 +147,57 @@ export function guardRunningTerminalClose(params: {
     decided = true
   }
 
+  const settledProbes = new Map<string, SettledCloseProbe>()
   const probeTimeout = setTimeout(() => {
     try {
-      // Why: a probe that has not answered yet is unknown, not idle. Ask, treating every pty
-      // as a candidate, so a degraded relay costs a click instead of a killed remote command.
-      confirmClose(ptyIds)
+      const busyPtyIds = ptyIds.filter((ptyId) =>
+        shouldConfirmForProbe(ptyId, trackedPtyIds, settledProbes.get(ptyId))
+      )
+      if (busyPtyIds.length === 0) {
+        closeNow()
+        return
+      }
+      confirmClose(busyPtyIds)
     } catch {
       closeNow()
     }
   }, RUNNING_CLOSE_PROBE_TIMEOUT_MS)
 
-  void Promise.allSettled(ptyIds.map((ptyId) => inspectRuntimeTerminalProcess(settings, ptyId)))
+  const probes = ptyIds.map(async (ptyId): Promise<SettledCloseProbe> => {
+    let probe: SettledCloseProbe
+    try {
+      probe = { status: 'fulfilled', value: await inspectRuntimeTerminalProcess(settings, ptyId) }
+    } catch {
+      probe = { status: 'rejected' }
+    }
+    settledProbes.set(ptyId, probe)
+    return probe
+  })
+
+  void Promise.all(probes)
     .then((results) => {
       clearTimeout(probeTimeout)
       if (decided) {
         return
       }
-      // Why: fail open on a *rejection* (wedged relay, legacy provider), matching the Cmd+W
-      // pane path — a close button that silently does nothing is worse than closing a busy
-      // tab. `unavailable` now means exactly "could not ask", which this guard's own timeout
-      // already prompts on, so an answered non-answer asks too — but only for an id the
-      // liveness map still vouches for, the same id set the window-close guard reads. A
-      // layout-only id is usually a leftover leaf whose pane is long gone: it answers
-      // `unavailable` forever, and prompting on it would put a dialog in front of every
-      // cleanly-exited tab. It can still block the close by answering *positively*, which is
-      // the mounting-pane window the union exists for.
-      const busyPtyIds = ptyIds.filter((ptyId, index) => {
-        const result = results[index]
-        if (result?.status !== 'fulfilled') {
-          return false
-        }
-        if (result.value.hasChildProcesses) {
-          return true
-        }
-        return result.value.unavailable === true && trackedPtyIds.has(ptyId)
-      })
+      // Why: a non-answer asks — a rejection (wedged relay, legacy provider) and
+      // `unavailable` ("could not ask") are the same evidence as this guard's own timeout,
+      // and this close kills the pty, so it owes the same prompt the window-close path
+      // already gives. Both narrow to an id the liveness map still vouches for, the id set
+      // the window-close guard reads. A layout-only id is usually a leftover leaf whose pane
+      // is long gone — it answers `unavailable` or throws forever, and prompting on it would
+      // put a dialog in front of every cleanly-exited tab and every reconnecting ssh tab. It
+      // can still block by answering *positively*, the mounting-pane window the union exists for.
+      const busyPtyIds = ptyIds.filter((ptyId, index) =>
+        shouldConfirmForProbe(ptyId, trackedPtyIds, results[index])
+      )
       if (busyPtyIds.length === 0) {
         closeNow()
         return
       }
       confirmClose(busyPtyIds)
     })
-    // Why: allSettled never rejects, so this only fires when the decision above throws (a
+    // Why: each probe catches its own rejection, so this only fires when the decision above throws (a
     // copy-kind lookup, a store subscriber). Without it the tab would silently never close
     // and the user would get no feedback at all; the pane path it replaced had this catch.
     .catch(() => {