From 657a47c25ca366527cd89bc5e394a5fb38c58e9d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:51:39 -0700 Subject: [PATCH] fix(pty): report agent start failures the same way from every launch path (#28724) * feat(telemetry): count agent start failures from the host too The host spawn path (CLI, phone, orchestration workers, headless automations, worktree startup) now records agent_error when starting an agent fails, as the window path already did. Removes C5a's TEMPORARY recordsAgentError input; the host already threads each launch's telemetry, so failures get real kinds. * fix(telemetry): count a claimed start that several requests joined once Concurrent resumes of one agent session join one claimed start. When it fails, every joined request ran the host catch and recorded agent_error, so one failed start counted once per request. Only the request that ran the start records it now; the others still clean up. * fix(telemetry): only skip agent_error for a request that joined another start The host marked a request as joined before calling ensure, so a claim that refused the request on its own (conflict, capacity, unreachable owner) recorded no agent_error. ensure now reports the join itself, right before awaiting the other request's start. * chore: re-run CI after main's localization fix (#28810) --- src/main/ipc/pty-launch-parity.test.ts | 51 ++++++++++++++++++++++- src/main/ipc/pty/ipc/spawn-execute.ts | 3 +- src/main/ipc/pty/runtime/spawn-execute.ts | 7 +++- src/main/ipc/pty/spawn-execute-failure.ts | 6 +-- src/shared/claimed-agent-pty-owner.ts | 3 ++ 5 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/main/ipc/pty-launch-parity.test.ts b/src/main/ipc/pty-launch-parity.test.ts index e9d48c17974..17016cf59cf 100644 --- a/src/main/ipc/pty-launch-parity.test.ts +++ b/src/main/ipc/pty-launch-parity.test.ts @@ -34,6 +34,7 @@ import { } from './pty-launch-parity-host.test-cases' import { isHiddenRendererPty } from './pty-hidden-delivery-gate' import { ptySizes } from './pty/delivery/visibility-state' +import { agentSessionOwners } from './pty/pane/agent-session-owners' import { makePaneSpawnReservationKey, paneSpawnReservationsByOwnerKey @@ -311,7 +312,7 @@ describe('launch facts outside the tables', () => { // 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: [] + host: [['agent_error', { agent_kind: 'claude-code', error_class: 'binary_not_found' }]] } it.each(['window', 'host'] as const)( '%s lane: a start the provider fails undoes everything it claimed', @@ -365,6 +366,54 @@ describe('launch facts outside the tables', () => { } ) + // C5a-2: requests that join one claimed start share its failure; only the request that started it counts it. + it('host lane: concurrent resumes of one Claude session count one failed start once', async () => { + setMainPlatform('darwin') + classifyErrorMock.mockReturnValue({ error_class: 'binary_not_found' }) + const lanes = startParityLanes(suite, { workspace: POSIX_REPO }) + lanes.provider.mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 100)) + throw new Error('spawn boom') + }) + const resume = { + kind: 'explicit' as const, + worktree: `id:${launchWorkspaceId(POSIX_REPO)}`, + agent: 'claude' as const, + providerSession: { key: 'session_id' as const, id: '11111111-1111-4111-8111-111111111111' } + } + + const results = await Promise.allSettled([ + lanes.runtime.ensureAgentSession(resume), + lanes.runtime.ensureAgentSession(resume) + ]) + + expect(results.map((r) => r.status)).toEqual(['rejected', 'rejected']) + expect(lanes.provider).toHaveBeenCalledOnce() + expect(trackMock.mock.calls.filter(([event]) => event === 'agent_error')).toEqual([ + ['agent_error', { agent_kind: 'claude-code', error_class: 'binary_not_found' }] + ]) + }) + + it('host lane: a Claude resume its claim refuses counts one failed start', async () => { + setMainPlatform('darwin') + classifyErrorMock.mockReturnValue({ error_class: 'unknown' }) + const lanes = startParityLanes(suite, { workspace: POSIX_REPO }) + vi.spyOn(agentSessionOwners, 'ensure').mockRejectedValueOnce( + new Error('agent_session_conflict') + ) + + await expect( + lanes.runtime.ensureAgentSession({ + kind: 'explicit', + worktree: `id:${launchWorkspaceId(POSIX_REPO)}`, + agent: 'claude', + providerSession: { key: 'session_id', id: '22222222-2222-4222-8222-222222222222' } + }) + ).rejects.toThrow('agent_session_conflict') + expect(lanes.provider).not.toHaveBeenCalled() + expect(trackMock.mock.calls.filter(([event]) => event === 'agent_error')).toHaveLength(1) + }) + // 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 2e48c431c07..e4b7fe2bb8c 100644 --- a/src/main/ipc/pty/ipc/spawn-execute.ts +++ b/src/main/ipc/pty/ipc/spawn-execute.ts @@ -107,8 +107,7 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { throw failPtySpawnExecute(ctx, err, { sessionId: ctx.effectiveSessionId, ownsNewSession: ctx.isMintedSessionId, - telemetry: args.telemetry, - recordsAgentError: true + telemetry: args.telemetry }) } finally { if (ctx.preAllocatedHandle) { diff --git a/src/main/ipc/pty/runtime/spawn-execute.ts b/src/main/ipc/pty/runtime/spawn-execute.ts index 35c0ee96317..a0302e49b52 100644 --- a/src/main/ipc/pty/runtime/spawn-execute.ts +++ b/src/main/ipc/pty/runtime/spawn-execute.ts @@ -24,6 +24,7 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise refuseSleptWorktree: args.refuseSleptWorktree }) : undefined + let awaitingAnotherStart = false try { if (args.preAllocatedHandle) { ctx.deps.trustedTerminalHandleEnv.add(args.preAllocatedHandle) @@ -116,8 +117,12 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise throw new Error('execution_owner_unavailable') } return await isProviderAgentSessionOwnerLive(ownerProvider, owner) + }, + onJoin: () => { + awaitingAnotherStart = true } }) + awaitingAnotherStart = false ctx.result = providerResult ?? { id: ensured.owner.ptyId, isReattach: true, @@ -200,7 +205,7 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise sessionId: ctx.sessionId, ownsNewSession: ctx.isNewDaemonSession, telemetry: args.telemetry, - recordsAgentError: false + joinedAnotherStart: awaitingAnotherStart }) } finally { if (args.preAllocatedHandle) { diff --git a/src/main/ipc/pty/spawn-execute-failure.ts b/src/main/ipc/pty/spawn-execute-failure.ts index d4a43a0daba..04b678ff139 100644 --- a/src/main/ipc/pty/spawn-execute-failure.ts +++ b/src/main/ipc/pty/spawn-execute-failure.ts @@ -29,8 +29,8 @@ export function failPtySpawnExecute( /** 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 + /** This request awaited another request's claimed start, whose own catch records its failure. */ + joinedAnotherStart?: boolean } ): Error { const runtime = ctx.deps.runtime @@ -79,7 +79,7 @@ export function failPtySpawnExecute( : ctx.isClaudeLaunch ? ('claude-code' as const) : null - if (lane.recordsAgentError && agentKind) { + if (agentKind && !lane.joinedAnotherStart) { track('agent_error', { agent_kind: agentKind, error_class: classifyError(spawnError).error_class, diff --git a/src/shared/claimed-agent-pty-owner.ts b/src/shared/claimed-agent-pty-owner.ts index d5449814696..4e6d78bf8ac 100644 --- a/src/shared/claimed-agent-pty-owner.ts +++ b/src/shared/claimed-agent-pty-owner.ts @@ -60,6 +60,8 @@ export class ClaimedAgentPtyOwnerRegistry { disposition?: AgentSessionClaimedSpawnResult['disposition'] }> isLive?: (owner: LiveAgentSessionOwner) => boolean | Promise + /** Called when this request waits on another request's start instead of starting its own. */ + onJoin?: () => void }): Promise { // Why: callers retain their request objects across retries; snapshot them so // mutation during an awaited liveness/spawn check cannot change registry keys. @@ -92,6 +94,7 @@ export class ClaimedAgentPtyOwnerRegistry { if (reserved.worktreeScopeDigest !== requestedClaim.worktreeScopeDigest) { throw new Error('agent_session_conflict') } + args.onJoin?.() const result = await reserved.promise return { disposition: 'adopted', owner: cloneOwner(result.owner as LiveOwner) } }