fix(conversations): a failed in-place resume is reported on its pane (#1241)

docs/plans/2026-09-25-conversations-resume-in-place-failure.md

@@ -0,0 +1,15 @@
+# Resuming in place from the Conversations picker must say when it fails (#1241)
+
+## Verified failure
+`ConversationsPicker.resume` closes the picker, then awaits `workspace.replaceSession(...)`. Its callers fire it with `void`. Two outcomes then disappear:
+- **A rejected swap** (the recorded `posix_spawnp failed` spawn error was used) became an unhandled rejection. The existing renderer test harness reports it as one.
+- **An `undefined` return** (no command target, missing meta, a refused commit) was silent.
+
+In both cases the user saw nothing change.
+
+## Design
+- The target pane is read before the picker closes. Every failed outcome is reported on that pane with a toast, the way `builtInMcpReload` reports the same call. A thrown message is shown as-is; `undefined` gets "Couldn't resume <label> in this pane."
+- With no pane to resume into (reachable only in "everywhere" scope), the picker stays open and says so inline instead of closing into a no-op.
+
+## Tests
+Renderer tests with the recorded spawn failure, the `undefined` outcome, and the no-target case. Red on main (unhandled rejection, no toast); each branch is mutation-checked.

src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx

@@ -5,6 +5,8 @@ import type { Conversation, ConversationListResponse } from '@shared/conversatio
 import type { Workspace } from '@renderer/workspace/workspaceStore'
 import { ConversationsPicker } from './ConversationsPicker'
 import { oneLaneStage } from '@renderer/workspace/testing/stageFixtures'
+import { readFileSync } from 'node:fs'
+import { join } from 'node:path'
 
 const originalApi = Object.getOwnPropertyDescriptor(window, 'api')
 afterEach(() => {
@@ -37,13 +39,14 @@ function install(list = vi.fn(async () => response())) {
 }
 // The picker reads the commanded pane through commandTargetSessionId, which
 // walks state.tabs / activeTabId / dispatchMode, so the mock carries them.
-type WorkspaceMock = Workspace & { replaceSession: Mock; newTab: Mock }
+type WorkspaceMock = Workspace & { replaceSession: Mock; newTab: Mock; showPaneToast: Mock }
 function workspace(over: Record<string, unknown> = {}): WorkspaceMock {
   return {
     activeTab: { id: 't', focusedSessionId: 's' },
     state: { tabs: [{ id: 't', title: 'fixture' }], activeTabId: 't', stage: oneLaneStage('s'),   pinnedSessionIds: [], sessions: { s: { cwd: '/fixture/repo', kind: 'claude', projectId: 't', joinedAt: 0 } } },
     replaceSession: vi.fn(async () => 's2'),
     newTab: vi.fn(async () => undefined),
+    showPaneToast: vi.fn(),
     ...over,
   } as unknown as WorkspaceMock
 }
@@ -76,6 +79,63 @@ describe('ConversationsPicker', () => {
     // other caller of replaceSession continues the same agent and omits it.
     await waitFor(() => expect(ws.replaceSession).toHaveBeenCalledWith('/fixture/repo/.worktrees/extension-platform', { resumeSessionId: '01a08ddd-6327-7482-bd79-d1ade559677c', kind: 'codex', newConversation: true }))
     expect(onClose).toHaveBeenCalled()
+    // A successful swap says nothing (#1262 review A).
+    expect(ws.showPaneToast).not.toHaveBeenCalled()
+  })
+
+  // #1241: the picker closes, then swaps the pane. A failed swap used to be an
+  // unhandled rejection (only journaled) or a silent `undefined`; the user saw
+  // nothing change. The reason now lands on the pane it was meant for.
+  const recordedSpawnFailure = (JSON.parse(readFileSync(join(import.meta.dirname,
+    '../../../../../../testing/fixtures/spawn-failure/posix-spawnp-2026-09-23.json'), 'utf8')) as { reason: string }).reason
+
+  it('tells the pane why an in-place resume failed to start (#1241)', async () => {
+    install()
+    const ws = workspace({ replaceSession: vi.fn(async () => { throw new Error(recordedSpawnFailure) }) })
+    render(<ConversationsPicker open focusSearch={false} workspace={ws} onClose={vi.fn()} />)
+    await screen.findByText('break down this project')
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Enter' })
+    await waitFor(() => expect(ws.showPaneToast).toHaveBeenCalledWith('s', "Couldn't resume Project context bootstrapping: Session failed to start. Check provider setup and retry."))
+    // Never the raw IPC rejection: it can carry environment values or tokens.
+    expect(JSON.stringify(ws.showPaneToast.mock.calls)).not.toContain('posix_spawnp')
+    expect(JSON.stringify(ws.showPaneToast.mock.calls)).not.toContain('Error invoking remote method')
+  })
+
+  it('tells the pane when an in-place resume could not happen at all (#1241)', async () => {
+    install()
+    const ws = workspace({ replaceSession: vi.fn(async () => undefined) })
+    render(<ConversationsPicker open focusSearch={false} workspace={ws} onClose={vi.fn()} />)
+    await screen.findByText('break down this project')
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Enter' })
+    await waitFor(() => expect(ws.showPaneToast).toHaveBeenCalledWith('s', "Couldn't resume Project context bootstrapping in this pane."))
+  })
+
+  it('does not leave an unhandled rejection when a new-tab resume fails (#1262 review A)', async () => {
+    install()
+    const ws = workspace({ activeTab: null, newTab: vi.fn(async () => { throw new Error('newTab already toasted this') }) })
+    render(<ConversationsPicker open focusSearch={false} workspace={ws} onClose={vi.fn()} />)
+    fireEvent.click(screen.getByRole('button', { name: 'everywhere' }))
+    await screen.findByText('break down this project')
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Enter' })
+    await waitFor(() => expect(ws.newTab).toHaveBeenCalled())
+  })
+
+  it('stays open and says why when no pane is selected to resume into (#1241)', async () => {
+    install()
+    const onClose = vi.fn()
+    const base = workspace()
+    const ws = workspace({ state: { ...base.state, stage: { lanes: [{}], rows: [{ length: 1 }], focusedLane: 0 } } })
+    render(<ConversationsPicker open focusSearch={false} workspace={ws} onClose={onClose} />)
+    // Without a pane only the everywhere scope lists rows.
+    fireEvent.click(screen.getByRole('button', { name: 'everywhere' }))
+    await screen.findByText('break down this project')
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Enter' })
+    expect(await screen.findByText(/no agent pane is selected/)).toBeInTheDocument()
+    expect(onClose).not.toHaveBeenCalled()
+    // Moving the highlight clears the error that named the previous row.
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'ArrowDown' })
+    await waitFor(() => expect(screen.queryByText(/no agent pane is selected/)).not.toBeInTheDocument())
+    expect(ws.replaceSession).not.toHaveBeenCalled()
   })
 
   it('does not resume the highlighted row when Enter presses a filter chip (#867)', async () => {

src/renderer/src/features/conversations/ui/ConversationsPicker.tsx

@@ -6,6 +6,7 @@ import { AGENT_PROVIDER_KINDS, type AgentProviderKind } from '@shared/types/prov
 import { useEnabledAgentProviderKinds } from '@renderer/features/providers/store'
 import { Dialog, DialogContent, DialogDescription, DialogTitle } from '@renderer/components/ui/dialog'
 import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId'
+import { SESSION_START_FAILED_MESSAGE } from '@shared/types/session'
 import type { Workspace } from '@renderer/workspace/workspaceStore'
 import { useResizableSplitter } from '@renderer/features/shared/useResizableSplitter'
 import { SessionPreviewPane } from '@renderer/features/session-preview/ui/SessionPreviewPane'
@@ -82,6 +83,9 @@ export function ConversationsPicker({ open, focusSearch, workspace, onClose }: P
   // scope), but not when loadMore appends rows below it.
   const headId = response?.rows[0]?.nativeId ?? null
   useEffect(() => { setSelected(0) }, [headId, query, scope, providers, includeChildren])
+  // An inline resume error names the row it was about; moving the highlight
+  // makes it stale (#1262 review B).
+  useEffect(() => { setResumeError(null) }, [selected])
   useEffect(() => {
     listRef.current?.querySelector<HTMLElement>(`[data-conversation-index="${selected}"]`)?.scrollIntoView({ block: 'nearest' })
   }, [selected])
@@ -100,6 +104,14 @@ export function ConversationsPicker({ open, focusSearch, workspace, onClose }: P
       setResumeError(`Can't resume ${row.nativeId.slice(0, 8)}: the transcript records no working directory.`)
       return
     }
+    // The pane the swap is aimed at, read BEFORE the picker closes: it is the
+    // one place a failure can still be reported once the picker is gone.
+    const targetSessionId = commandTargetSessionId(workspace)
+    if (workspace.activeTab && !targetSessionId) {
+      // Nothing to swap into; closing would only hide a no-op.
+      setResumeError(`Can't resume ${row.label} here: no agent pane is selected.`)
+      return
+    }
     onClose()
     if (workspace.activeTab) {
       // In-place swap: the pane stays where it is, what runs in it changes.
@@ -110,10 +122,30 @@ export function ConversationsPicker({ open, focusSearch, workspace, onClose }: P
       // a stranger's conversation in, and inheriting parentage here would file
       // it as somebody's orchestration child — reported to that parent as its
       // worker's answer, and killed by `close_run`.
-      await workspace.replaceSession(row.cwd, { resumeSessionId: row.nativeId, kind: row.provider, newConversation: true })
+      //
+      // WHY every outcome is reported on the pane (#1241): the picker has
+      // already closed, and the callers fire this with `void`, so a spawn
+      // failure (e.g. the provider's CLI removed since the transcript was
+      // written) was an unhandled rejection and an `undefined` (no command
+      // target, missing meta, a refused commit) was silence. The pane just
+      // stayed as it was. builtInMcpReload reports the same call this way.
+      try {
+        const replaced = await workspace.replaceSession(row.cwd, { resumeSessionId: row.nativeId, kind: row.provider, newConversation: true })
+        if (!replaced) workspace.showPaneToast(targetSessionId!, `Couldn't resume ${row.label} in this pane.`)
+      } catch {
+        // WHY a fixed sentence and never the rejection's text (steering q22,
+        // #1262 review B): the spawn rejection is Electron's wrapper around the
+        // raw provider exception, which can carry environment values, proxy
+        // URLs or scoped MCP tokens. Main's recovery path and the reload path
+        // (#1252) show this same safe message for the same failure.
+        workspace.showPaneToast(targetSessionId!, `Couldn't resume ${row.label}: ${SESSION_START_FAILED_MESSAGE}`)
+      }
     } else {
       // Fresh launch with nothing to replace: a new tab in the row's cwd.
-      await workspace.newTab(row.cwd, row.nativeId, row.provider)
+      // newTab already toasts its own failure and rethrows; the catch only
+      // keeps this `void`-fired resume from ending in an unhandled rejection
+      // (#1262 review A).
+      await workspace.newTab(row.cwd, row.nativeId, row.provider).catch(() => undefined)
     }
   }, [onClose, workspace])
 

src/renderer/src/workspace/hook/helpers.ts

@@ -229,24 +229,34 @@ export function useWorkspaceHelpers(
 export function usePaneToast(
   paneToastTimers: WorkspaceRefs['paneToastTimers'],
   updateRuntime: (id: SessionId, patch: Partial<SessionRuntime>) => void,
+  stateRef?: WorkspaceRefs['stateRef'],
 ): (sessionId: SessionId, message: string, durationMs?: number) => void {
   // Single-slot, auto-dismiss. Calling showPaneToast while a previous
   // toast is still visible replaces it and resets the timer. The
   // timeout ref lives outside React state so we can clear it without
   // causing a re-render.
   return useCallback(
     (sessionId: SessionId, message: string, durationMs = 2000) => {
+      // A pane that is gone (closed while an async action it started was
+      // still running) gets no toast: updateRuntime would recreate a runtime
+      // row for the dead id, and its dismiss timer would write it again, so
+      // the row was never removed (#1262 review A; builtInMcpReload shared
+      // the shape).
+      if (stateRef && !stateRef.current.sessions[sessionId]) return
       // Clear any in-flight timer for this pane.
       const prev = paneToastTimers.current[sessionId]
       if (prev) clearTimeout(prev)
 
       updateRuntime(sessionId, { paneToast: message })
 
       paneToastTimers.current[sessionId] = setTimeout(() => {
-        updateRuntime(sessionId, { paneToast: null })
         delete paneToastTimers.current[sessionId]
+        // Closed while its toast was showing: clearing it would recreate the
+        // dead pane's runtime row, the other half of the same leak.
+        if (stateRef && !stateRef.current.sessions[sessionId]) return
+        updateRuntime(sessionId, { paneToast: null })
       }, durationMs)
     },
-    [paneToastTimers, updateRuntime],
+    [paneToastTimers, updateRuntime, stateRef],
   )
 }

src/renderer/src/workspace/hook/index.ts

@@ -256,7 +256,7 @@ export function useWorkspace(
     useWorkspaceHelpers(setRuntimes, refs)
 
   // ---- Pane toast (needs updateRuntime, so after helpers) ----
-  const showPaneToast = usePaneToast(refs.paneToastTimers, updateRuntime)
+  const showPaneToast = usePaneToast(refs.paneToastTimers, updateRuntime, refs.stateRef)
 
   // ---- Actions ----
   const isCodexSession = useCallback(

src/renderer/src/workspace/hook/paneToast.renderer.test.ts

@@ -0,0 +1,29 @@
+import { renderHook } from '@testing-library/react'
+import { expect, it, vi } from 'vitest'
+
+import { usePaneToast } from './helpers'
+
+// #1262 review A: an async action can finish after its pane was closed. A
+// toast for that pane used to recreate a runtime row for the dead id, and the
+// dismiss timer wrote it again, so the row was never removed.
+it('shows no toast for a pane that is no longer in the workspace', () => {
+  vi.useFakeTimers()
+  try {
+    const updateRuntime = vi.fn()
+    const timers = { current: {} as Record<string, ReturnType<typeof setTimeout>> }
+    const stateRef = { current: { sessions: { live: {} } } }
+    const { result } = renderHook(() => usePaneToast(timers as never, updateRuntime, stateRef as never))
+    result.current('gone', 'late message')
+    vi.runAllTimers()
+    expect(updateRuntime).not.toHaveBeenCalled()
+    result.current('live', 'shown')
+    expect(updateRuntime).toHaveBeenCalledWith('live', { paneToast: 'shown' })
+    // Closed while the toast shows: the dismiss must not recreate its row.
+    delete (stateRef.current.sessions as Record<string, unknown>).live
+    updateRuntime.mockClear()
+    vi.runAllTimers()
+    expect(updateRuntime).not.toHaveBeenCalled()
+  } finally {
+    vi.useRealTimers()
+  }
+})