diff --git a/src/main/ipc/pty-launch-parity.test.ts b/src/main/ipc/pty-launch-parity.test.ts index 773b4b2111f..e9d48c17974 100644 --- a/src/main/ipc/pty-launch-parity.test.ts +++ b/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) diff --git a/src/main/ipc/pty/ipc/spawn-execute.ts b/src/main/ipc/pty/ipc/spawn-execute.ts index 770414cc392..2e48c431c07 100644 --- a/src/main/ipc/pty/ipc/spawn-execute.ts +++ b/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 { @@ -109,69 +104,12 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { 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) diff --git a/src/main/ipc/pty/runtime/spawn-execute.ts b/src/main/ipc/pty/runtime/spawn-execute.ts index ba3dc5d3611..35c0ee96317 100644 --- a/src/main/ipc/pty/runtime/spawn-execute.ts +++ b/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) diff --git a/src/main/ipc/pty/spawn-execute-failure.ts b/src/main/ipc/pty/spawn-execute-failure.ts new file mode 100644 index 00000000000..d4a43a0daba --- /dev/null +++ b/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 +}