refactor(pty): one failure cleanup and agent_error for both terminal spawn paths (#28613)

* test(pty): pin a failed provider start on both spawn lanes

Characterizes main before the two execute steps merge: the error is
rethrown, the pre-spawn hidden mark, pending registration, worktree spawn
gate, cached size and pane reservation are all undone, and only the window
lane records agent_error.

* refactor(pty): one failure cleanup and agent_error for both spawn lanes

The window (pty:spawn) and the host (runtime controller) each undid a failed
execute step with their own copy of the same cleanup: provisional execution
context, early-exit fence, pending registration, cached size and provider
state for a minted session id. Both now call failPtySpawnExecute, which also
records agent_error in one place.

recordsAgentError is TEMPORARY: host starts still record none, as on main,
until C5a-2 removes the input. The window's extra hidden-mark rollback in this
catch duplicated its outer catch, which still runs, and is gone.

* chore: re-run CI against current main

src/main/ipc/pty-launch-parity.test.ts

@@ -28,8 +28,17 @@ import {
 import {
   expectedHostProvider,
   HOST_LAUNCH_CASES,
+  HOST_LEAF_ID,
+  HOST_TAB_ID,
   type HostLaunchCase
 } from './pty-launch-parity-host.test-cases'
+import { isHiddenRendererPty } from './pty-hidden-delivery-gate'
+import { ptySizes } from './pty/delivery/visibility-state'
+import {
+  makePaneSpawnReservationKey,
+  paneSpawnReservationsByOwnerKey
+} from './pty/pane/spawn-reservation'
+import { makePaneKey } from '../../shared/stable-pane-id'
 import { CALLER_LAUNCH_CASES, OPENCODE_MODEL } from './pty-launch-parity-caller.test-cases'
 import { RpcDispatcher } from '../runtime/rpc/dispatcher'
 import { DESKTOP_RPC_CALLER } from '../runtime/rpc/rpc-caller-identity'
@@ -299,6 +308,63 @@ describe('launch facts outside the tables', () => {
     expect(trackMock.mock.calls.filter(([event]) => event === 'agent_error')).toEqual(c.calls)
   })
 
+  // C5a (one execute) keeps this on both lanes; C5a-2 flips only the host's agent_error.
+  const FAILED_START_AGENT_ERROR: Record<'window' | 'host', unknown[]> = {
+    window: [['agent_error', { agent_kind: 'claude-code', error_class: 'binary_not_found' }]],
+    host: []
+  }
+  it.each(['window', 'host'] as const)(
+    '%s lane: a start the provider fails undoes everything it claimed',
+    async (lane) => {
+      setMainPlatform('darwin')
+      classifyErrorMock.mockReturnValue({ error_class: 'binary_not_found' })
+      const lanes = startParityLanes(suite, { workspace: POSIX_REPO })
+      let hiddenWhileSpawning = false
+      lanes.provider.mockImplementationOnce(async (options: { sessionId?: string }) => {
+        hiddenWhileSpawning = isHiddenRendererPty(String(options.sessionId))
+        throw new Error('spawn boom')
+      })
+      const gateReleased = vi.fn()
+      const acquireGate = lanes.runtime.acquireWorktreeTerminalSpawn.bind(lanes.runtime)
+      vi.spyOn(lanes.runtime, 'acquireWorktreeTerminalSpawn').mockImplementation(async (...a) => {
+        const release = await acquireGate(...a)
+        return () => {
+          gateReleased()
+          release()
+        }
+      })
+      const cancelRegistration = vi.spyOn(lanes.runtime, 'cancelPendingPtyRegistration')
+      const worktreeId = launchWorkspaceId(POSIX_REPO)
+      const launch =
+        lane === 'host'
+          ? runHostCall(lanes, HOST_LAUNCH_CASES[0]!)
+          : lanes.spawnWindow({
+              cols: 80,
+              rows: 24,
+              cwd: POSIX_PATH,
+              worktreeId,
+              tabId: HOST_TAB_ID,
+              leafId: HOST_LEAF_ID,
+              command: 'claude',
+              initiallyHidden: true,
+              telemetry: { agent_kind: 'claude-code', launch_source: 'task_page' }
+            })
+
+      await expect(launch).rejects.toThrow('spawn boom')
+      const sessionId = String(recordOf(lanes.provider.mock.calls.at(-1)?.[0]).sessionId)
+      expect(trackMock.mock.calls).toEqual(FAILED_START_AGENT_ERROR[lane])
+      expect(hiddenWhileSpawning).toBe(true)
+      expect(isHiddenRendererPty(sessionId)).toBe(false)
+      expect(cancelRegistration.mock.calls).toEqual([[sessionId, undefined]])
+      expect(gateReleased).toHaveBeenCalledOnce()
+      expect(ptySizes.has(sessionId)).toBe(false)
+      const paneKey = makePaneKey(HOST_TAB_ID, HOST_LEAF_ID)
+      expect(
+        paneSpawnReservationsByOwnerKey.has(makePaneSpawnReservationKey(worktreeId, null, paneKey)!)
+      ).toBe(false)
+    }
+  )
+
   // C5e (presentation merge): a worker asks the window to reveal its titled tab without activating it.
   it('row 10: an orchestration worker requests a titled, inactive reveal', async () => {
     setMainPlatform(WORKER.os)

src/main/ipc/pty/ipc/spawn-execute.ts

@@ -1,15 +1,10 @@
 import { ensureWslHookRelayForReattach } from '../../../agent-hooks/wsl-hook-relay-reattach'
 import { noteWslDistrosRunning } from '../../../wsl-background-distro-gate'
-import { classifyError } from '../../../telemetry/classify-error'
-import { track } from '../../../telemetry/client'
-import { getCohortAtEmit } from '../../../telemetry/cohort-classifier'
-import { agentKindSchema } from '../../../../shared/telemetry-events'
-import { normalizeNodePtySpawnError } from '../provider/liveness'
 import { resolveStablePaneOwner, spawnForStablePane } from '../pane/stable-owner'
 import { excludeReplacedPaneOwner } from '../pane/pane-owner-replacement'
 import { assertSpawnReplyWasLive } from '../pane/agent-session-owners'
-import { ptySizes } from '../delivery/visibility-state'
 import { clearProviderPtyState } from '../provider/state-cleanup'
+import { failPtySpawnExecute } from '../spawn-execute-failure'
 import type { PtyIpcSpawnState } from './spawn-state'
 
 export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise<void> {
@@ -109,69 +104,12 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise<void> {
     ctx.deps.runtime?.preparePtyExecutionContext?.(ctx.result.id, spawnedWslDistro)
     ctx.spawnTiming.mark('provider_spawn')
   } catch (err) {
-    if (
-      (ctx.isMintedSessionId || ctx.preparedProvisionalExecutionContext) &&
-      ctx.effectiveSessionAppId
-    ) {
-      ctx.deps.runtime?.preparePtyExecutionContext?.(ctx.effectiveSessionAppId, null, {
-        resetIncarnation: true
-      })
-    }
-    // Why: a stale hidden mark on this session id would gate a later visible attach that reuses it.
-    if (ctx.preSpawnHiddenMarkId !== null) {
-      ctx.deps.transitionSpawnHiddenRendererPtyDeliveryState(ctx.preSpawnHiddenMarkId, false)
-    }
-    const rawMessage = err instanceof Error ? err.message : String(err)
-    if (rawMessage === 'agent_session_exited_during_start' && ctx.rejectedRegistrationCandidate) {
-      ctx.deps.runtime?.releaseRejectedPtyRegistrationFence?.(
-        ctx.rejectedRegistrationCandidate.id,
-        ctx.rejectedRegistrationCandidate.incarnationId
-      )
-    }
-    if (ctx.pendingRegistrationPtyId) {
-      ctx.deps.runtime?.cancelPendingPtyRegistration?.(
-        ctx.pendingRegistrationPtyId,
-        ctx.rejectedRegistrationCandidate?.incarnationId
-      )
-      ctx.pendingRegistrationPtyId = null
-    }
-    const spawnError = normalizeNodePtySpawnError(err)
-    const exitedBeforeSpawnReply =
-      ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true
-    if (ctx.effectiveSessionAppId !== undefined) {
-      if (
-        ctx.hadSessionSizeBeforeAttach &&
-        ctx.sessionSizeBeforeAttach &&
-        !exitedBeforeSpawnReply
-      ) {
-        ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach)
-      } else {
-        ptySizes.delete(ctx.effectiveSessionAppId)
-      }
-    }
-    // Why: provider state buildPtyHostEnv materialized for this minted id leaks if spawn failed.
-    if (ctx.isMintedSessionId && ctx.effectiveSessionId !== undefined) {
-      clearProviderPtyState(ctx.effectiveSessionId)
-    }
-    // Why: telemetry-plan.md§agent_error — attribute the error to the renderer-threaded agent_kind, else sniff the command for `claude`; raw messages are dropped at the validator boundary.
-    const rendererAgentKindParse =
-      args.telemetry?.agent_kind !== undefined
-        ? agentKindSchema.safeParse(args.telemetry.agent_kind)
-        : null
-    const errorAgentKind = rendererAgentKindParse?.success
-      ? rendererAgentKindParse.data
-      : ctx.isClaudeLaunch
-        ? ('claude-code' as const)
-        : null
-    if (errorAgentKind) {
-      const classified = classifyError(spawnError)
-      track('agent_error', {
-        agent_kind: errorAgentKind,
-        error_class: classified.error_class,
-        ...getCohortAtEmit()
-      })
-    }
-    throw spawnError
+    throw failPtySpawnExecute(ctx, err, {
+      sessionId: ctx.effectiveSessionId,
+      ownsNewSession: ctx.isMintedSessionId,
+      telemetry: args.telemetry,
+      recordsAgentError: true
+    })
   } finally {
     if (ctx.preAllocatedHandle) {
       ctx.deps.trustedTerminalHandleEnv.delete(ctx.preAllocatedHandle)

src/main/ipc/pty/runtime/spawn-execute.ts

@@ -1,6 +1,5 @@
 import type { PtySpawnResult } from '../../../providers/pty-provider-contract'
 import { ptyIncarnationById } from '../provider/ownership-state'
-import { ptySizes } from '../delivery/visibility-state'
 import { tryGetProviderForPty } from '../provider/registry'
 import { ensureWslHookRelayForReattach } from '../../../agent-hooks/wsl-hook-relay-reattach'
 import { noteWslDistrosRunning } from '../../../wsl-background-distro-gate'
@@ -11,7 +10,8 @@ import {
 } from '../pane/agent-session-owners'
 import { spawnForStablePane, resolveStablePaneOwner } from '../pane/stable-owner'
 import { clearProviderPtyState } from '../provider/state-cleanup'
-import { isProviderAgentSessionOwnerLive, normalizeNodePtySpawnError } from '../provider/liveness'
+import { isProviderAgentSessionOwnerLive } from '../provider/liveness'
+import { failPtySpawnExecute } from '../spawn-execute-failure'
 import type { RuntimePtySpawnState } from './spawn-state'
 import { markRuntimeSpawnHiddenBeforeSpawn } from './spawn-hidden-delivery'
 
@@ -196,46 +196,12 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise
     }
     ctx.deps.runtime?.preparePtyExecutionContext?.(ctx.result.id, spawnedWslDistro)
   } catch (err) {
-    if (
-      (ctx.isNewDaemonSession || ctx.preparedProvisionalExecutionContext) &&
-      ctx.effectiveSessionAppId
-    ) {
-      ctx.deps.runtime?.preparePtyExecutionContext?.(ctx.effectiveSessionAppId, null, {
-        resetIncarnation: true
-      })
-    }
-    const rawMessage = err instanceof Error ? err.message : String(err)
-    if (rawMessage === 'agent_session_exited_during_start' && ctx.rejectedRegistrationCandidate) {
-      ctx.deps.runtime?.releaseRejectedPtyRegistrationFence?.(
-        ctx.rejectedRegistrationCandidate.id,
-        ctx.rejectedRegistrationCandidate.incarnationId
-      )
-    }
-    if (ctx.pendingRegistrationPtyId) {
-      ctx.deps.runtime?.cancelPendingPtyRegistration?.(
-        ctx.pendingRegistrationPtyId,
-        ctx.rejectedRegistrationCandidate?.incarnationId
-      )
-      ctx.pendingRegistrationPtyId = null
-    }
-    const spawnError = normalizeNodePtySpawnError(err)
-    const exitedBeforeSpawnReply =
-      ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true
-    if (ctx.effectiveSessionAppId !== undefined) {
-      if (
-        ctx.hadSessionSizeBeforeAttach &&
-        ctx.sessionSizeBeforeAttach &&
-        !exitedBeforeSpawnReply
-      ) {
-        ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach)
-      } else {
-        ptySizes.delete(ctx.effectiveSessionAppId)
-      }
-    }
-    if (ctx.isNewDaemonSession && ctx.sessionId !== undefined) {
-      clearProviderPtyState(ctx.sessionId)
-    }
-    throw spawnError
+    throw failPtySpawnExecute(ctx, err, {
+      sessionId: ctx.sessionId,
+      ownsNewSession: ctx.isNewDaemonSession,
+      telemetry: args.telemetry,
+      recordsAgentError: false
+    })
   } finally {
     if (args.preAllocatedHandle) {
       ctx.deps.trustedTerminalHandleEnv.delete(args.preAllocatedHandle)

src/main/ipc/pty/spawn-execute-failure.ts

@@ -0,0 +1,90 @@
+import type { PtySpawnResult } from '../../providers/pty-provider-contract'
+import type { OrcaRuntimeService } from '../../runtime/orca-runtime'
+import { classifyError } from '../../telemetry/classify-error'
+import { track } from '../../telemetry/client'
+import { getCohortAtEmit } from '../../telemetry/cohort-classifier'
+import { agentKindSchema } from '../../../shared/telemetry-events'
+import { normalizeNodePtySpawnError } from './provider/liveness'
+import { ptySizes } from './delivery/visibility-state'
+import { clearProviderPtyState } from './provider/state-cleanup'
+
+/** What both spawn lanes' execute step claims before the provider call, under the same names. */
+type PtySpawnExecuteState = {
+  deps: { runtime?: OrcaRuntimeService }
+  isClaudeLaunch: boolean
+  effectiveSessionAppId: string | undefined
+  hadSessionSizeBeforeAttach: boolean
+  sessionSizeBeforeAttach: { cols: number; rows: number } | undefined
+  preparedProvisionalExecutionContext: boolean
+  pendingRegistrationPtyId: string | null
+  rejectedRegistrationCandidate: PtySpawnResult | null
+}
+
+/** Undoes what the execute step claimed for a start that threw, and returns the error to rethrow. */
+export function failPtySpawnExecute(
+  ctx: PtySpawnExecuteState,
+  err: unknown,
+  lane: {
+    sessionId: string | undefined
+    /** This spawn minted the session id, so the state built for it must go. */
+    ownsNewSession: boolean
+    telemetry: { agent_kind?: unknown } | undefined
+    // TEMPORARY(C5a-2): keeps host starts out of agent_error as on main; C5a-2 deletes this input.
+    recordsAgentError: boolean
+  }
+): Error {
+  const runtime = ctx.deps.runtime
+  if (
+    (lane.ownsNewSession || ctx.preparedProvisionalExecutionContext) &&
+    ctx.effectiveSessionAppId
+  ) {
+    runtime?.preparePtyExecutionContext?.(ctx.effectiveSessionAppId, null, {
+      resetIncarnation: true
+    })
+  }
+  const rawMessage = err instanceof Error ? err.message : String(err)
+  if (rawMessage === 'agent_session_exited_during_start' && ctx.rejectedRegistrationCandidate) {
+    runtime?.releaseRejectedPtyRegistrationFence?.(
+      ctx.rejectedRegistrationCandidate.id,
+      ctx.rejectedRegistrationCandidate.incarnationId
+    )
+  }
+  if (ctx.pendingRegistrationPtyId) {
+    runtime?.cancelPendingPtyRegistration?.(
+      ctx.pendingRegistrationPtyId,
+      ctx.rejectedRegistrationCandidate?.incarnationId
+    )
+    ctx.pendingRegistrationPtyId = null
+  }
+  const spawnError = normalizeNodePtySpawnError(err)
+  const exitedBeforeSpawnReply = ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true
+  if (ctx.effectiveSessionAppId !== undefined) {
+    if (ctx.hadSessionSizeBeforeAttach && ctx.sessionSizeBeforeAttach && !exitedBeforeSpawnReply) {
+      ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach)
+    } else {
+      ptySizes.delete(ctx.effectiveSessionAppId)
+    }
+  }
+  // Why: provider state the env build materialized for this minted id leaks if spawn failed.
+  if (lane.ownsNewSession && lane.sessionId !== undefined) {
+    clearProviderPtyState(lane.sessionId)
+  }
+  // Why: telemetry-plan.md§agent_error — attribute the error to the threaded agent_kind, else sniff the command for `claude`; raw messages are dropped at the validator boundary.
+  const threadedKind =
+    lane.telemetry?.agent_kind !== undefined
+      ? agentKindSchema.safeParse(lane.telemetry.agent_kind)
+      : null
+  const agentKind = threadedKind?.success
+    ? threadedKind.data
+    : ctx.isClaudeLaunch
+      ? ('claude-code' as const)
+      : null
+  if (lane.recordsAgentError && agentKind) {
+    track('agent_error', {
+      agent_kind: agentKind,
+      error_class: classifyError(spawnError).error_class,
+      ...getCohortAtEmit()
+    })
+  }
+  return spawnError
+}