Stop new terminal tabs from closing right after they open (#6801)

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/components/terminal-pane/pty-connection.test.ts

@@ -10445,6 +10445,80 @@ describe('connectPanePty', () => {
       expect(manager.closePane).toHaveBeenCalledWith(2)
     })
 
+    it('does NOT tear down a newborn pane when the snapshot was requested before it bound', async () => {
+      // Why (regression): a snapshot requested before the spawn bound cannot
+      // prove the fresh ptyId dead. Drives the REAL reconcile body to prove the
+      // boundAt wiring, not just forwarding.
+      const { connectPanePty } = await import('./pty-connection')
+      const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null }
+      const transport = createMockTransport('pty-pane-2')
+      transport.connect.mockImplementation(
+        async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
+          capturedDataCallback.current = callbacks.onData ?? null
+          return 'pty-pane-2'
+        }
+      )
+      transportFactoryQueue.push(transport)
+      const manager = createManager(2)
+      const deps = createDeps({
+        restoredLeafId: LEAF_2,
+        paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) }
+      })
+
+      const binding = connectPanePty(createPane(2) as never, manager as never, deps as never)
+      // Why: clear the freshly-split early-return guard so the ONLY remaining
+      // protection is the freshness guard this test exercises.
+      capturedDataCallback.current?.('shell prompt')
+      const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as
+        | ((ptyId: string) => void)
+        | undefined
+      expect(onPtySpawn).toBeTypeOf('function')
+
+      // Record boundAt via the spawn chokepoint; bracket it with a real timestamp.
+      const beforeSpawn = performance.now()
+      onPtySpawn?.('pty-pane-2')
+
+      // requestedAt < boundAt: stale snapshot can't prove the fresh pane dead.
+      binding.reconcileIfSessionDead(new Set(['pty-pane-1']), beforeSpawn - 1)
+
+      expect(manager.closePane).not.toHaveBeenCalled()
+      expect(deps.onPtyExitRef.current).not.toHaveBeenCalled()
+    })
+
+    it('tears down the pane when the snapshot was requested after it bound', async () => {
+      const { connectPanePty } = await import('./pty-connection')
+      const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null }
+      const transport = createMockTransport('pty-pane-2')
+      transport.connect.mockImplementation(
+        async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
+          capturedDataCallback.current = callbacks.onData ?? null
+          return 'pty-pane-2'
+        }
+      )
+      transportFactoryQueue.push(transport)
+      const manager = createManager(2)
+      const deps = createDeps({
+        restoredLeafId: LEAF_2,
+        paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) }
+      })
+
+      const binding = connectPanePty(createPane(2) as never, manager as never, deps as never)
+      // Why: clear the freshly-split early-return guard so onExit reaches close.
+      capturedDataCallback.current?.('shell prompt')
+      const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as
+        | ((ptyId: string) => void)
+        | undefined
+      expect(onPtySpawn).toBeTypeOf('function')
+
+      onPtySpawn?.('pty-pane-2')
+      const afterSpawn = performance.now()
+
+      // requestedAt > boundAt: the snapshot postdates the bind, so absence is real.
+      binding.reconcileIfSessionDead(new Set(['pty-pane-1']), afterSpawn + 1)
+
+      expect(manager.closePane).toHaveBeenCalledWith(2)
+    })
+
     it('routes the last pane through onPtyExitRef when its session is dead', async () => {
       const { connectPanePty } = await import('./pty-connection')
       const transport = createMockTransport('pty-pane-1')

src/renderer/src/components/terminal-pane/pty-connection.ts

@@ -522,7 +522,7 @@ let inactiveForegroundImmediateBudgetWindowStart = 0
 type PanePtyBinding = IDisposable & {
   syncProcessTracking: () => void
   noteVisibilityResume: () => void
-  reconcileIfSessionDead: (liveSessionIds: Set<string>) => void
+  reconcileIfSessionDead: (liveSessionIds: Set<string>, snapshotRequestedAt?: number) => void
 }
 
 function isAgentTaskCompleteNotificationEnabled(): boolean {
@@ -1308,11 +1308,15 @@ export function connectPanePty(
     pane.container.dataset.ptyId = ptyId
   }
   let activePanePtyBinding: string | null = null
+  // Why: bind time so reconcile can ignore a listSessions snapshot requested
+  // before this PTY bound (newborn race). Null disables the guard (fail-safe).
+  let activePanePtyBindingBoundAt: number | null = null
   const clearPanePtyFitBinding = (): void => {
     // Why: fit bindings live in a module-level map, so pane teardown must
     // clear them explicitly instead of relying on DOM removal.
     bindPanePtyId(pane.id, null, deps.tabId)
     activePanePtyBinding = null
+    activePanePtyBindingBoundAt = null
     delete pane.container.dataset.ptyId
   }
 
@@ -1601,6 +1605,9 @@ export function connectPanePty(
   ): void => {
     setPanePtyFitBinding(ptyId)
     activePanePtyBinding = ptyId
+    // Why: record bind time on the spawn/attach chokepoint so the reconcile
+    // guard knows this binding is newer than any pre-bind snapshot.
+    activePanePtyBindingBoundAt = performance.now()
     deps.syncPanePtyLayoutBinding(pane.id, ptyId)
     const tabPtyIds = useAppStore.getState().ptyIdsByTabId?.[deps.tabId] ?? []
     if (options.updateTabPtyId !== 'if-missing' || !tabPtyIds.includes(ptyId)) {
@@ -4537,7 +4544,10 @@ export function connectPanePty(
   // reattach racing the listSessions snapshot is never clobbered, and respect
   // the remote/SSH guards. Suppression semantics come for free via onExit
   // (which consults consumeSuppressedPtyExit) plus the per-ptyId guard above.
-  const reconcileIfSessionDead = (liveSessionIds: Set<string>): void => {
+  const reconcileIfSessionDead = (
+    liveSessionIds: Set<string>,
+    snapshotRequestedAt?: number
+  ): void => {
     if (disposed) {
       return
     }
@@ -4550,7 +4560,9 @@ export function connectPanePty(
       !shouldReconcileDeadSession({
         ptyId: currentPtyId,
         connectionId: transport.getConnectionId?.(),
-        liveSessionIds
+        liveSessionIds,
+        ptyBoundAt: activePanePtyBindingBoundAt,
+        snapshotRequestedAt
       })
     ) {
       return
@@ -4592,10 +4604,13 @@ export function connectPanePty(
     ) {
       return
     }
+    // Why: capture request time before the round-trip so a pane that bound after
+    // this request is not torn down by its (pre-bind) stale snapshot.
+    const requestedAt = performance.now()
     void window.api.pty
       .listSessions()
       .then((sessions) => {
-        reconcileIfSessionDead(new Set(sessions.map((session) => session.id)))
+        reconcileIfSessionDead(new Set(sessions.map((session) => session.id)), requestedAt)
       })
       // Why: a rejected listing is "unknown" — never close a pane on it.
       .catch(() => {})

src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts

@@ -64,11 +64,76 @@ describe('shouldReconcileDeadSession', () => {
       })
     ).toBe(true)
   })
+
+  it('does NOT reconcile a newborn pane bound after the snapshot was requested', () => {
+    // Why (regression): the snapshot predates this binding (boundAt >= requestedAt),
+    // so the fresh ptyId's absence from it is meaningless — it cannot prove death.
+    expect(
+      shouldReconcileDeadSession({
+        ptyId: 'wt@@newborn',
+        connectionId: null,
+        liveSessionIds: new Set(['wt@@alive']),
+        ptyBoundAt: 1000,
+        snapshotRequestedAt: 900
+      })
+    ).toBe(false)
+  })
+
+  it('does NOT reconcile when the binding and snapshot request share a tick (boundAt === requestedAt)', () => {
+    // Why: the guard is inclusive (>=) — a coarse/clamped performance.now() can
+    // land a same-tick bind and request, and that newborn must still be kept.
+    expect(
+      shouldReconcileDeadSession({
+        ptyId: 'wt@@newborn',
+        connectionId: null,
+        liveSessionIds: new Set(['wt@@alive']),
+        ptyBoundAt: 1000,
+        snapshotRequestedAt: 1000
+      })
+    ).toBe(false)
+  })
+
+  it('reconciles when the binding predates the snapshot request (boundAt < requestedAt)', () => {
+    expect(
+      shouldReconcileDeadSession({
+        ptyId: 'wt@@dead',
+        connectionId: null,
+        liveSessionIds: new Set(['wt@@alive']),
+        ptyBoundAt: 900,
+        snapshotRequestedAt: 1000
+      })
+    ).toBe(true)
+  })
+
+  it('ignores the freshness guard when either timestamp is omitted (back-compat)', () => {
+    // Omitted snapshotRequestedAt: behave exactly as today.
+    expect(
+      shouldReconcileDeadSession({
+        ptyId: 'wt@@dead',
+        connectionId: null,
+        liveSessionIds: new Set(['wt@@alive']),
+        ptyBoundAt: 1000
+      })
+    ).toBe(true)
+    // Omitted ptyBoundAt (null): behave exactly as today.
+    expect(
+      shouldReconcileDeadSession({
+        ptyId: 'wt@@dead',
+        connectionId: null,
+        liveSessionIds: new Set(['wt@@alive']),
+        ptyBoundAt: null,
+        snapshotRequestedAt: 1000
+      })
+    ).toBe(true)
+  })
 })
 
 describe('reconcileDeadSessions', () => {
   function createBinding() {
-    return { reconcileIfSessionDead: vi.fn<(liveSessionIds: Set<string>) => void>() }
+    return {
+      reconcileIfSessionDead:
+        vi.fn<(liveSessionIds: Set<string>, snapshotRequestedAt?: number) => void>()
+    }
   }
 
   it('invokes each binding with the resolved live-session id set', async () => {
@@ -82,8 +147,8 @@ describe('reconcileDeadSessions', () => {
       ]
     })
     const expectedSet = new Set(['wt@@alive', 'wt@@other'])
-    expect(bindingA.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet)
-    expect(bindingB.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet)
+    expect(bindingA.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet, expect.any(Number))
+    expect(bindingB.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet, expect.any(Number))
   })
 
   it('treats a rejected listSessions as "unknown" and reconciles nothing', async () => {
@@ -103,6 +168,24 @@ describe('reconcileDeadSessions', () => {
       bindings: [binding],
       listSessions: async () => []
     })
-    expect(binding.reconcileIfSessionDead).toHaveBeenCalledWith(new Set())
+    expect(binding.reconcileIfSessionDead).toHaveBeenCalledWith(new Set(), expect.any(Number))
+  })
+
+  it('forwards a requestedAt timestamp captured before listSessions resolves', async () => {
+    // Why (fail-open guard): if requestedAt is not threaded, a fresh pane bound
+    // after the request is wrongly reconciled. Prove a Number reaches each binding
+    // and that it predates a post-call now (captured before, not after, resolve).
+    const binding = createBinding()
+    const before = performance.now()
+    await reconcileDeadSessions({
+      bindings: [binding],
+      listSessions: async () => [{ id: 'wt@@alive', cwd: '/a', title: 'a' }]
+    })
+    const after = performance.now()
+    expect(binding.reconcileIfSessionDead).toHaveBeenCalledTimes(1)
+    const [, requestedAt] = binding.reconcileIfSessionDead.mock.calls[0]!
+    expect(typeof requestedAt).toBe('number')
+    expect(requestedAt).toBeGreaterThanOrEqual(before)
+    expect(requestedAt).toBeLessThanOrEqual(after)
   })
 })

src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts

@@ -12,7 +12,7 @@ const REMOTE_PTY_ID_PREFIX = 'remote:'
  * the full `PanePtyBinding` shape from pty-connection.
  */
 export type ReconcilableBinding = {
-  reconcileIfSessionDead?: (liveSessionIds: Set<string>) => void
+  reconcileIfSessionDead?: (liveSessionIds: Set<string>, snapshotRequestedAt?: number) => void
 }
 
 /**
@@ -32,8 +32,10 @@ export function shouldReconcileDeadSession(args: {
   ptyId: string | null | undefined
   connectionId: string | null | undefined
   liveSessionIds: Set<string>
+  ptyBoundAt?: number | null
+  snapshotRequestedAt?: number | null
 }): boolean {
-  const { ptyId, connectionId, liveSessionIds } = args
+  const { ptyId, connectionId, liveSessionIds, ptyBoundAt, snapshotRequestedAt } = args
   if (ptyId === null || ptyId === undefined) {
     return false
   }
@@ -45,6 +47,16 @@ export function shouldReconcileDeadSession(args: {
   if (connectionId !== null && connectionId !== undefined) {
     return false
   }
+  // Why: a snapshot requested before this binding existed can't prove it dead
+  // (newborn-PTY reconcile race). Omitting either timestamp keeps prior
+  // pure-membership behavior (back-compat).
+  if (
+    typeof ptyBoundAt === 'number' &&
+    typeof snapshotRequestedAt === 'number' &&
+    ptyBoundAt >= snapshotRequestedAt
+  ) {
+    return false
+  }
   return !liveSessionIds.has(ptyId)
 }
 
@@ -63,6 +75,9 @@ export async function reconcileDeadSessions(args: {
   listSessions: () => Promise<{ id: string; cwd: string; title: string }[]>
 }): Promise<void> {
   let sessions: { id: string }[]
+  // Why: capture the request time BEFORE the round-trip so the decision can tell
+  // a snapshot that predates a fresh binding from one that postdates it.
+  const requestedAt = performance.now()
   try {
     sessions = await args.listSessions()
   } catch {
@@ -71,6 +86,6 @@ export async function reconcileDeadSessions(args: {
   }
   const liveSessionIds = new Set(sessions.map((session) => session.id))
   for (const binding of args.bindings) {
-    binding.reconcileIfSessionDead?.(liveSessionIds)
+    binding.reconcileIfSessionDead?.(liveSessionIds, requestedAt)
   }
 }

src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts

@@ -343,7 +343,7 @@ describe('scheduleVisibilityReconcilePass', () => {
     await Promise.resolve()
     await Promise.resolve()
     expect(listSessions).toHaveBeenCalledTimes(1)
-    expect(reconcileIfSessionDead).toHaveBeenCalledWith(new Set(['live-1']))
+    expect(reconcileIfSessionDead).toHaveBeenCalledWith(new Set(['live-1']), expect.any(Number))
   })
 
   it('does not schedule on an initially visible mount', () => {