mirror of
https://github.com/stablyai/orca.git
synced 2026-11-03 02:27:43 +00:00
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)
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -107,8 +107,7 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise<void> {
|
||||
throw failPtySpawnExecute(ctx, err, {
|
||||
sessionId: ctx.effectiveSessionId,
|
||||
ownsNewSession: ctx.isMintedSessionId,
|
||||
telemetry: args.telemetry,
|
||||
recordsAgentError: true
|
||||
telemetry: args.telemetry
|
||||
})
|
||||
} finally {
|
||||
if (ctx.preAllocatedHandle) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -60,6 +60,8 @@ export class ClaimedAgentPtyOwnerRegistry {
|
||||
disposition?: AgentSessionClaimedSpawnResult['disposition']
|
||||
}>
|
||||
isLive?: (owner: LiveAgentSessionOwner) => boolean | Promise<boolean>
|
||||
/** Called when this request waits on another request's start instead of starting its own. */
|
||||
onJoin?: () => void
|
||||
}): Promise<AgentSessionClaimedSpawnResult> {
|
||||
// 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) }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user