fix(ipc): session:spawn launders provider exceptions before they cross IPC (#1267)

docs/plans/2026-09-26-spawn-ipc-launder.md

@@ -0,0 +1,18 @@
+# session:spawn stops relaying raw provider exceptions over IPC (#1267)
+
+## Evidence
+- `src/main/ipc/session.ts` rethrew whatever `manager.spawn` threw. IPC relays the message, so the recorded posix_spawnp rejection (`testing/fixtures/spawn-failure/posix-spawnp-2026-09-23.json`), plus any environment values, proxy URLs or scoped MCP tokens a launch exception carries, reached the renderer verbatim.
+- `recover()` already flattens its failures to `SESSION_START_FAILED_MESSAGE` (sessionManager recoverSession).
+- Each renderer surface had to remember not to show the text. #1286 made the renderer flatten it once, but the raw text still crossed IPC.
+
+## Change
+- The spawn handler launders its rejection (`launderSpawnError`). Only these cross as themselves:
+  - a missing workspace folder (curated; a path the UI already shows);
+  - a missing provider CLI (curated; names File › Setup…);
+  - this window losing the session.
+- A Claude proxy that would not start becomes its fixed guidance. That sentence and its detection move to shared, so main and renderer use one copy.
+- Everything else becomes the safe sentence, and the raw error is logged in main.
+
+## Tests
+- `ipc/session.test.ts` "session:spawn rejections": the recorded fixture plus a token and credentialed URL is flattened; the curated failures and the window refusal pass; a mitmproxy failure becomes the guidance without its raw text.
+- Red on main, and every branch is mutation-checked.

src/main/ipc/session.test.ts

@@ -30,7 +30,7 @@ vi.mock('@main/window/windowRegistry.js', () => ({
 // The sub-agent watcher polls real directories; nothing here is about fleets.
 vi.mock('@main/subagents/index.js', () => ({ SubAgentWatcherManager: class { observeParentEntry() {} stop() {} stopAll() {} } }))
 
-const { registerSessionIpc } = await import('./session.js')
+const { registerSessionIpc, classifySpawnFailure } = await import('./session.js')
 const { sessionApi } = await import('@preload/api/session.js')
 const { SessionFeedTap } = await import('@main/sessions/sessionFeedTap.js')
 const { EventEmitter } = await import('node:events')
@@ -332,3 +332,115 @@ describe('raw PTY attach ownership (#1311)', () => {
   })
 })
 
+
+// #1267 (steering q22 at the source): session:spawn relayed the raw provider
+// exception over IPC, where every renderer surface had to remember not to
+// show it. Main launders it like recover() does; only curated, secret-free
+// failures cross as themselves.
+describe('session:spawn rejections', () => {
+  const { readFileSync } = require('node:fs') as typeof import('node:fs')
+  const { join } = require('node:path') as typeof import('node:path')
+  const recorded = (JSON.parse(readFileSync(join(import.meta.dirname,
+    '../../../testing/fixtures/spawn-failure/posix-spawnp-2026-09-23.json'), 'utf8')) as { reason: string }).reason
+
+  async function rejectionOf(error: unknown, options: Record<string, unknown> = { kind: 'claude' }, journal?: { record: ReturnType<typeof vi.fn> }): Promise<string> {
+    registerSessionIpc({ spawn: vi.fn(async () => { throw error }) } as never, {} as never, { flushCommitted: () => {} }, journal as never)
+    const handler = harness.handlers.get('session:spawn')!
+    return await Promise.resolve(handler({ sender: {} }, { cwd: '/repo', ...options })).then(() => 'resolved', (e: Error) => e.message)
+  }
+
+  // #1324 review A/B: a non-Error whose toString throws used to escape as
+  // the rejection, carrying whatever it threw.
+  it('never reads a non-Error throw', async () => {
+    const hostile = { toString: () => { throw new Error('token=secret') } }
+    expect(await rejectionOf(hostile)).toBe('Session failed to start. Check provider setup and retry.')
+  })
+
+  // #1324 review A/B: proxy guidance only for a Claude spawn that runs the
+  // proxy; both recognised signatures reach it.
+  it('gives the Claude proxy guidance only to a Claude proxy spawn', async () => {
+    expect(await rejectionOf(new Error('spawn /repo/mitmdump: ENOENT'), { kind: 'codex', useProxy: false })).toBe('Session failed to start. Check provider setup and retry.')
+    expect(await rejectionOf(new Error('Unable to locate mitmAddon.py'), { kind: 'claude', useProxy: true })).toContain('Claude proxy startup failed')
+    expect(await rejectionOf(new Error('Unable to find mitmdump on PATH'), { kind: 'claude', useProxy: true })).toContain('Claude proxy startup failed')
+    expect(await rejectionOf(new Error('Unable to find mitmdump on PATH'), { kind: 'claude', useProxy: false })).toBe('Session failed to start. Check provider setup and retry.')
+  })
+
+  // #1324 review round 2 A/B: an Error's message can be a getter. One that
+  // throws escaped as the rejection; one that answered the window sentence
+  // first and a token next was returned as-is and read again by IPC.
+  it('never lets an Error\'s own message getter reach the rejection', async () => {
+    const throwing = new Error('x')
+    Object.defineProperty(throwing, 'message', { get() { throw new Error('token=secret') } })
+    expect(await rejectionOf(throwing)).toBe('Session failed to start. Check provider setup and retry.')
+    let reads = 0
+    const shifting = new Error('x')
+    Object.defineProperty(shifting, 'message', { get: () => (reads++ === 0 ? 'The requesting window can no longer own this session' : 'token=secret') })
+    registerSessionIpc({ spawn: vi.fn(async () => { throw shifting }) } as never, {} as never, { flushCommitted: () => {} })
+    const rejected = await Promise.resolve(harness.handlers.get('session:spawn')!({ sender: {} }, { cwd: '/repo', kind: 'claude' })).then(() => null, (e: Error) => e)
+    expect(rejected).not.toBe(shifting)
+    expect(rejected!.message).toBe('The requesting window can no longer own this session')
+    expect(rejected!.message).toBe('The requesting window can no longer own this session')
+  })
+
+  // #1324 review round 2 A/B: the guidance needs BOTH a Claude spawn and
+  // useProxy exactly true (sessionManager starts mitmproxy only then).
+  it('gives no proxy guidance to a Codex proxy spawn or a Claude spawn without useProxy', async () => {
+    expect(await rejectionOf(new Error('Unable to find mitmdump on PATH'), { kind: 'codex', useProxy: true })).toBe('Session failed to start. Check provider setup and retry.')
+    expect(await rejectionOf(new Error('Unable to find mitmdump on PATH'), { kind: 'claude' })).toBe('Session failed to start. Check provider setup and retry.')
+  })
+
+  // #1324 review round 2 B/C: every signature a debugger reads from the
+  // journal, one representative each, so a collapsed classifier fails here.
+  it('signs each known failure with its own code', async () => {
+    const { MissingWorkspaceDirectoryError } = await import('@main/workspaceDirectory.js')
+    const { ProviderCliNotFoundError } = await import('@main/sessionManager.js')
+    const signatureOf = (error: unknown, proxyApplies = false) => classifySpawnFailure(error, proxyApplies).signature
+    expect(signatureOf(new Error(recorded))).toBe('posix-spawnp')
+    expect(signatureOf(new MissingWorkspaceDirectoryError('/repo/gone'))).toBe('missing-workspace')
+    expect(signatureOf(new ProviderCliNotFoundError('codex'))).toBe('cli-not-found')
+    expect(signatureOf(new Error('The requesting window can no longer own this session'))).toBe('window-refused')
+    expect(signatureOf({ toString: () => 'x' })).toBe('non-error-throw')
+    expect(signatureOf(new Error('Unable to locate mitmAddon.py'), true)).toBe('claude-proxy')
+    // A Codex error naming mitmdump is what it is, not a Claude proxy failure.
+    expect(signatureOf(new Error('spawn /repo/mitmdump: ENOENT'))).toBe('enoent')
+    expect(signatureOf(new Error('spawn /usr/bin/codex EACCES'))).toBe('eacces')
+    expect(signatureOf(new Error('Session recovery was cancelled'))).toBe('unclassified')
+  })
+
+  // #1324 review C: the laundered rejection is all the incident journal and
+  // a debug bundle see, so the failure's identity is journaled as a fixed
+  // signature, never its text.
+  it('journals which known failure it was, without its text', async () => {
+    const journal = { record: vi.fn() }
+    await rejectionOf(new Error(`${recorded} env=ANTHROPIC_API_KEY=sk-ant-secret`), { kind: 'codex' }, journal)
+    expect(journal.record).toHaveBeenCalledWith(expect.objectContaining({ name: 'session.spawn.failed', data: { kind: 'codex', signature: 'posix-spawnp' } }))
+    expect(JSON.stringify(journal.record.mock.calls)).not.toContain('sk-ant')
+  })
+
+  it('never relays a raw provider exception', async () => {
+    const message = await rejectionOf(new Error(`${recorded} env=ANTHROPIC_API_KEY=sk-ant-secret https://user:pass@proxy.example`))
+    expect(message).toBe('Session failed to start. Check provider setup and retry.')
+  })
+
+  it('keeps the curated failures that name their fix', async () => {
+    const { MissingWorkspaceDirectoryError } = await import('@main/workspaceDirectory.js')
+    const { ProviderCliNotFoundError } = await import('@main/sessionManager.js')
+    expect(await rejectionOf(new MissingWorkspaceDirectoryError('/repo/.worktrees/gone'))).toBe('Workspace folder is missing: /repo/.worktrees/gone')
+    expect(await rejectionOf(new ProviderCliNotFoundError('codex'))).toBe('codex CLI not found. Open Setup (File › Setup…) to install it or enter its path.')
+  })
+
+  it('keeps the window-ownership refusal, which our own code writes', async () => {
+    const { claimSessionForWindow } = await import('@main/window/windowRegistry.js')
+    vi.mocked(claimSessionForWindow).mockReturnValueOnce(null as never)
+    registerSessionIpc({ spawn: vi.fn(async (_options: unknown, claim: (id: string) => void) => { claim('s1'); return 's1' }) } as never, {} as never, { flushCommitted: () => {} })
+    const handler = harness.handlers.get('session:spawn')!
+    const message = await Promise.resolve(handler({ sender: {} }, { cwd: '/repo', kind: 'claude' })).then(() => 'resolved', (e: Error) => e.message)
+    expect(message).toBe('The requesting window can no longer own this session')
+  })
+
+  it('turns a Claude proxy startup failure into the proxy guidance, not its raw text', async () => {
+    const message = await rejectionOf(new Error('Timed out waiting for mitmproxy on 127.0.0.1:51234 with token=abc'), { kind: 'claude', useProxy: true })
+    expect(message).toContain('Claude proxy startup failed')
+    expect(message).not.toContain('token=abc')
+  })
+})

src/main/ipc/session.ts

@@ -1,7 +1,9 @@
 import { ipcMain } from 'electron'
 import { createHash } from 'node:crypto'
 
-import type { SessionManager } from '@main/sessionManager.js'
+import { ProviderCliNotFoundError, type SessionManager } from '@main/sessionManager.js'
+import { MissingWorkspaceDirectoryError } from '@main/workspaceDirectory.js'
+import { CLAUDE_PROXY_STARTUP_FAILED_MESSAGE, isClaudeProxyStartupFailure, SESSION_START_FAILED_MESSAGE } from '@shared/types/session.js'
 import { mainOperations } from '@main/performance/operations.js'
 import type { PasteDebugJournalRegistry } from '@main/pasteDebugJournal.js'
 import type { AppRunJournal } from '@main/incident/AppRunJournal.js'
@@ -36,6 +38,8 @@ import { screenInterest, screenTailHistory } from '@main/sessions/screenInterest
 import type { AgentScreenSnapshot } from '@shared/types/session.js'
 import type { ScreenTailSample } from '@shared/debug/screenTail.js'
 
+const WINDOW_CANNOT_OWN_SESSION = 'The requesting window can no longer own this session'
+
 // One secret per app process is enough for correlation inside that run's
 // incident journal, and unlike a bare SHA-256 it prevents an exported bundle
 // reader from dictionary-guessing common repository paths. Cross-run joins are
@@ -88,14 +92,14 @@ export function registerSessionIpc(
       try {
         return await manager.spawn(options, sessionId => {
           lease = claimSessionForWindow(sessionId, owner)
-          if (!lease) throw new Error('The requesting window can no longer own this session')
+          if (!lease) throw new Error(WINDOW_CANNOT_OWN_SESSION)
         })
       } catch (error) {
         // A failed spawn never returns its minted id to the renderer, so no
         // pane-disposal request can clean this claim later. Release only this
         // admission; a successor recovery may already have claimed the id.
         releaseSession(lease)
-        throw error
+        throw launderSpawnError(error, options, appRunJournal)
       }
     },
   )
@@ -561,3 +565,87 @@ export function registerSessionIpc(
     },
   )
 }
+
+/**
+ * What a failed session:spawn tells the renderer (#1267, steering q22 at the
+ * source). IPC relays only an error's message, and a provider launch
+ * exception can carry environment values, proxy URLs or scoped MCP tokens;
+ * every renderer surface used to have to remember not to show it. recover()
+ * already flattens its failures this way (sessionManager recoverSession).
+ * Only failures whose text our own code builds from a fixed template cross
+ * as themselves: a missing workspace folder (a path the user chose and the
+ * pane header already shows; #1324 review A noted it is not secret-free in
+ * general, but it reveals nothing the UI does not), a
+ * missing provider CLI (names File › Setup…), and this window losing the
+ * session. A Claude proxy that would not start becomes its fixed guidance.
+ * Everything else is the one safe sentence. Main logs and journals only a
+ * fixed signature (classifySpawnFailure), never the text.
+ */
+function launderSpawnError(
+  error: unknown,
+  options: Pick<SessionSpawnOptions, 'kind' | 'useProxy'> | undefined,
+  journal: AppRunJournal | undefined,
+): Error {
+  // WHY every read of the thrown value happens exactly once, inside a try,
+  // and every return is a FRESH Error built from that one read (#1324 review
+  // round 2 A/B): the thrown value is provider-controlled. Converting a
+  // non-Error to text runs its toString; an Error's `message` can be a getter
+  // that throws, or that answers the fixed window sentence on the first read
+  // and a token on the next. Returning the original object let IPC read it
+  // again. So nothing of the original crosses except text we compared.
+  let failure: SpawnFailure
+  try {
+    failure = classifySpawnFailure(error, proxyGuidanceApplies(options))
+  } catch {
+    failure = { signature: 'unreadable-throw', message: SESSION_START_FAILED_MESSAGE }
+  }
+  // WHY a signature and not the message (#1324 review C): the laundered
+  // rejection is all the renderer, the incident journal and a debug bundle
+  // ever see, so the one fact that identified the recorded node-pty trap
+  // ("posix_spawnp failed") was lost to every artifact this repo debugs
+  // from. A fixed code carries that fact and nothing else.
+  journal?.record({ area: 'session.spawn', name: 'session.spawn.failed', severity: 'warn', data: { kind: options?.kind ?? null, signature: failure.signature } })
+  console.warn('[session:spawn] provider start failed:', failure.signature)
+  return new Error(failure.message)
+}
+
+/**
+ * Only a Claude spawn that runs the proxy gets the proxy guidance (#1324
+ * review A/B, round 2 A/B). `useProxy` must be exactly true: that is the
+ * test sessionManager uses to start mitmproxy at all, so an omitted value is
+ * a launch without a proxy and must not be told to disable one. A Codex
+ * spawn whose error mentions mitmdump is not a Claude proxy failure either.
+ */
+function proxyGuidanceApplies(options: Pick<SessionSpawnOptions, 'kind' | 'useProxy'> | undefined): boolean {
+  return options?.kind === 'claude' && options.useProxy === true
+}
+
+type SpawnFailure = { signature: string; message: string }
+
+/**
+ * Which known failure a spawn rejection is (a fixed code, safe to journal)
+ * and the one sentence the renderer may see for it. New signatures go here as
+ * they are identified from recorded incidents. Reads `error.message` once;
+ * the caller catches a throwing read.
+ */
+export function classifySpawnFailure(error: unknown, proxyApplies: boolean): SpawnFailure {
+  const generic = (signature: string): SpawnFailure => ({ signature, message: SESSION_START_FAILED_MESSAGE })
+  if (!(error instanceof Error)) return generic('non-error-throw')
+  const raw: unknown = error.message
+  if (typeof raw !== 'string') return generic('unreadable-throw')
+  // Our own fixed templates cross as themselves: a missing workspace folder
+  // (a path the user chose and the pane header already shows; review A noted
+  // it is not secret-free in general, but it reveals nothing the UI does not)
+  // and a missing provider CLI (names File › Setup…).
+  if (error instanceof MissingWorkspaceDirectoryError) return { signature: 'missing-workspace', message: raw }
+  if (error instanceof ProviderCliNotFoundError) return { signature: 'cli-not-found', message: raw }
+  if (raw === WINDOW_CANNOT_OWN_SESSION) return { signature: 'window-refused', message: WINDOW_CANNOT_OWN_SESSION }
+  if (raw.includes('posix_spawnp failed')) return generic('posix-spawnp')
+  // The proxy code only where the guidance applies (round 2 B3): a Codex
+  // `spawn /repo/mitmdump: ENOENT` is an ENOENT, and signing it claude-proxy
+  // would send the next debugger after the wrong subsystem.
+  if (proxyApplies && isClaudeProxyStartupFailure(raw)) return { signature: 'claude-proxy', message: CLAUDE_PROXY_STARTUP_FAILED_MESSAGE }
+  if (/\bENOENT\b/.test(raw)) return generic('enoent')
+  if (/\bEACCES\b/.test(raw)) return generic('eacces')
+  return generic('unclassified')
+}

src/main/window/sessionRoutingComposition.test.ts

@@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import type { AppWindowHooks } from './appWindow.js'
 import type { SessionManager } from '@main/sessionManager.js'
 import type { LspManager } from '@main/lspManager.js'
+import { SESSION_START_FAILED_MESSAGE } from '@shared/types/session'
 
 const harness = vi.hoisted(() => ({
   history: vi.fn(),
@@ -147,15 +148,19 @@ describe('session routing through its real lifecycle callers', () => {
     expect(registry.captureSessionWindowLease('pane')).toBe(lease)
   })
 
+  // The raw provider text never crosses IPC (#1267, session:spawn launders
+  // it); what this test pins is that laundering did not skip the lease
+  // release, which runs in the same failure path.
   it('releases a failed spawn id which no renderer ever received', async () => {
     registry.createAppWindow()
     manager.spawn = vi.fn<SessionManager['spawn']>(async (_options, onId) => {
       onId?.('failed-pane')
-      throw new Error('fixture startup failure')
+      throw new Error('fixture startup failure token=secret')
     })
-    await expect(harness.handlers.get('session:spawn')!({
+    const rejection = await Promise.resolve(harness.handlers.get('session:spawn')!({
       sender: harness.built[0]!.webContents,
-    }, {})).rejects.toThrow('fixture startup failure')
+    }, {})).then(() => null, (error: Error) => error.message)
+    expect(rejection).toBe(SESSION_START_FAILED_MESSAGE)
     expect(registry.captureSessionWindowLease('failed-pane')).toBeNull()
   })