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) } }