mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
fix(claude): classify cleanup after a first-hand exit as a root exit instead of a proven tree
When the CLI died between a successful acquire and the host's commit or proof of the lease, handleExit had already removed the session, so releaseAcquisition found nothing and reported true. The attach flow then settled exit-proven with deathEvidence claiming cleanup proved no provider child remains, though the tree was never verified. The adapter now keeps the exit that removed a published session until the session is acquired again; acquisition cleanup runs that connection's close ladder and classifies its verdict exactly as a start-time failure would be, so the record reads root-exit-observed. The wire helper keeps that typed classification and its provider diagnostic instead of wrapping it as unproven, and the router gives up its owner even when the release throws. Claude-Session: https://claude.ai/code/session_01HfdhsvSJucLw4cTZxzg2CP
This commit is contained in:
@@ -38,10 +38,14 @@ import {
|
||||
type ClaudeAcquisitionRegistry,
|
||||
type ClaudeAcquisitionAttempt,
|
||||
type ClaudeSession,
|
||||
type ClaudeSessionExit,
|
||||
type ClaudeStructuredSessionAdapterDeps,
|
||||
type ClaudeStructuredSessionEvent
|
||||
} from './claude-structured-session-state'
|
||||
import { closeClaudePublishedSessionForDeps } from './claude-structured-session-close'
|
||||
import {
|
||||
closeClaudePublishedSessionForDeps,
|
||||
closeClaudeSession
|
||||
} from './claude-structured-session-close'
|
||||
|
||||
export const CLAUDE_STRUCTURED_INIT_TIMEOUT_MS = 10_000
|
||||
|
||||
@@ -78,12 +82,14 @@ export async function acquireClaudeSession({
|
||||
deps,
|
||||
sessions,
|
||||
acquisitions,
|
||||
exits,
|
||||
callbacks
|
||||
}: {
|
||||
input: StructuredAgentSessionAcquireInput
|
||||
deps: ClaudeStructuredSessionAdapterDeps
|
||||
sessions: Map<string, ClaudeSession>
|
||||
acquisitions: ClaudeAcquisitionRegistry
|
||||
exits: Map<string, ClaudeSessionExit>
|
||||
callbacks: AcquireCallbacks
|
||||
}): Promise<AgentSessionAcquisition> {
|
||||
const sessionId = input.identity.sessionId
|
||||
@@ -133,6 +139,8 @@ export async function acquireClaudeSession({
|
||||
new Error(`claude session ${sessionId} could not be stopped`)
|
||||
)
|
||||
}
|
||||
// Any earlier session is closed or already gone: its exit says nothing about this start.
|
||||
exits.delete(sessionId)
|
||||
acquisitions.assertCurrent(sessionId, attempt)
|
||||
const launch = await deps.resolveLaunch({ identity: input.identity })
|
||||
observedLeafUuid = launch.resumeLeafUuid
|
||||
@@ -246,3 +254,28 @@ export async function acquireClaudeSession({
|
||||
attempt.finish()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cleanup for an acquisition the host could not commit or prove. A session that
|
||||
* a first-hand exit already removed is not an absence to report as proven: the
|
||||
* ladder on its connection still answers, and that answer is classified exactly
|
||||
* as a start-time failure would be.
|
||||
*/
|
||||
export async function releaseClaudeAcquisition(input: {
|
||||
sessionId: string
|
||||
sessions: Map<string, ClaudeSession>
|
||||
acquisitions: ClaudeAcquisitionRegistry
|
||||
exits: Map<string, ClaudeSessionExit>
|
||||
persistHandle?: ClaudeStructuredSessionAdapterDeps['persistHandle']
|
||||
onEvent?: ClaudeStructuredSessionAdapterDeps['onEvent']
|
||||
}): Promise<boolean> {
|
||||
const exit = input.exits.get(input.sessionId)
|
||||
if (!exit || input.sessions.has(input.sessionId) || input.acquisitions.get(input.sessionId)) {
|
||||
return closeClaudeSession(input)
|
||||
}
|
||||
input.exits.delete(input.sessionId)
|
||||
if (await exit.connection.close()) {
|
||||
return true
|
||||
}
|
||||
throw claudeAcquisitionCleanupError(exit.connection, exit.error)
|
||||
}
|
||||
|
||||
@@ -665,6 +665,57 @@ describe('ClaudeStructuredSessionAdapter acquisition cleanup', () => {
|
||||
expect(error).not.toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError)
|
||||
})
|
||||
|
||||
/** A published session whose CLI then exits first-hand, with the verdict its ladder holds. */
|
||||
async function exitedAfterPublish(
|
||||
exitVerdict: ClaudeStreamJsonConnection['exitVerdict']
|
||||
): Promise<{ adapter: ClaudeStructuredSessionAdapter; connection: FakeConnection }> {
|
||||
const claude = fakeClaude({ unprovenCloseVerdict: exitVerdict })
|
||||
const adapter = await acquired(claude)
|
||||
const connection = claude.connections[0]
|
||||
connection.handlers.onExit?.(new Error('claude stream-json exited (code 1): crashed'))
|
||||
return { adapter, connection }
|
||||
}
|
||||
|
||||
it('classifies cleanup after a first-hand exit removed the session as a root exit, never as proven', async () => {
|
||||
// The host may still be committing or proving the lease when the child dies;
|
||||
// its cleanup must find the exit the ladder observed, not an absence.
|
||||
const { adapter, connection } = await exitedAfterPublish({
|
||||
root: 'exited',
|
||||
tree: 'unverifiable'
|
||||
})
|
||||
const error = await adapter.releaseAcquisition({ sessionId: 'session-1' }).catch((e) => e)
|
||||
|
||||
expect(error).toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError)
|
||||
expect((error as Error).message).toBe('claude stream-json exited (code 1): crashed')
|
||||
expect(connection.closeCount).toBe(1)
|
||||
})
|
||||
|
||||
it('never releases after an exit that left a descendant observed alive', async () => {
|
||||
const { adapter } = await exitedAfterPublish({ root: 'exited', tree: 'live' })
|
||||
const error = await adapter.releaseAcquisition({ sessionId: 'session-1' }).catch((e) => e)
|
||||
|
||||
expect(error).toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError)
|
||||
expect(error).not.toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError)
|
||||
})
|
||||
|
||||
it('forgets a retained exit once the session is acquired again', async () => {
|
||||
const options: Parameters<typeof fakeClaude>[0] = {}
|
||||
const claude = fakeClaude(options)
|
||||
const adapter = await acquired(claude)
|
||||
const first = claude.connections[0]
|
||||
first.handlers.onExit?.(new Error('claude stream-json exited (code 1): crashed'))
|
||||
first.exitVerdict = { root: 'exited', tree: 'unverifiable' }
|
||||
first.close = async () => false
|
||||
options.exitBeforeInit = 'claude stream-json exited (code 1): not logged in'
|
||||
|
||||
await expect(
|
||||
adapter.acquire({ identity: identityFor(), fence: 8, spawnToken: 'spawn-10' })
|
||||
).rejects.toThrow('not logged in')
|
||||
// The second start's own proven close is the answer; the first exit is stale.
|
||||
await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true)
|
||||
expect(first.closeCount).toBe(0)
|
||||
})
|
||||
|
||||
it('reports unproven published-session cleanup so callers can retry safely', async () => {
|
||||
const claude = fakeClaude()
|
||||
const adapter = await acquired(claude)
|
||||
|
||||
@@ -6,7 +6,10 @@ import type {
|
||||
import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink'
|
||||
import { answerClaudePrompt, cancelClaudeTurn } from './claude-structured-control-actions'
|
||||
import { dispatchClaudeTurn } from './claude-structured-dispatch'
|
||||
import { acquireClaudeSession } from './claude-structured-session-acquisition'
|
||||
import {
|
||||
acquireClaudeSession,
|
||||
releaseClaudeAcquisition
|
||||
} from './claude-structured-session-acquisition'
|
||||
export { CLAUDE_STRUCTURED_INIT_TIMEOUT_MS } from './claude-structured-session-acquisition'
|
||||
import { supportsClaudeStructuredLocation } from './claude-structured-location-support'
|
||||
import { setClaudeStructuredOption } from './claude-structured-options'
|
||||
@@ -15,6 +18,7 @@ import {
|
||||
ClaudeAcquisitionRegistry,
|
||||
type ClaudeAcquisitionAttempt,
|
||||
type ClaudeSession,
|
||||
type ClaudeSessionExit,
|
||||
type ClaudeStructuredSessionAdapterDeps,
|
||||
type ClaudeStructuredSessionEvent
|
||||
} from './claude-structured-session-state'
|
||||
@@ -36,6 +40,7 @@ const DISPATCH_ACK_TIMEOUT_MS = 10_000
|
||||
export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAdapter {
|
||||
private readonly sessions = new Map<string, ClaudeSession>()
|
||||
private readonly acquisitions = new ClaudeAcquisitionRegistry()
|
||||
private readonly exits = new Map<string, ClaudeSessionExit>()
|
||||
|
||||
constructor(private readonly deps: ClaudeStructuredSessionAdapterDeps) {}
|
||||
|
||||
@@ -47,6 +52,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
|
||||
deps: this.deps,
|
||||
sessions: this.sessions,
|
||||
acquisitions: this.acquisitions,
|
||||
exits: this.exits,
|
||||
callbacks: {
|
||||
deliver: (attempt, sessionId, event) => this.deliver(attempt, sessionId, event),
|
||||
emit: (session, events, event) => this.emit(session, events, event),
|
||||
@@ -70,6 +76,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
|
||||
return
|
||||
}
|
||||
this.sessions.delete(sessionId)
|
||||
this.exits.set(sessionId, { connection: session.connection, error })
|
||||
this.emit(session, session.events, { type: 'ended', sessionId, reason: error.message })
|
||||
settleClaudeExitedSession(session)
|
||||
}
|
||||
@@ -109,7 +116,14 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
|
||||
readClaudeStructuredSessionOptions(this.session(input.sessionId), this.deps.requestTimeoutMs)
|
||||
|
||||
releaseAcquisition = (input: { sessionId: string }): Promise<boolean> =>
|
||||
this.closeSession(input.sessionId)
|
||||
releaseClaudeAcquisition({
|
||||
sessionId: input.sessionId,
|
||||
sessions: this.sessions,
|
||||
acquisitions: this.acquisitions,
|
||||
exits: this.exits,
|
||||
...(this.deps.persistHandle ? { persistHandle: this.deps.persistHandle } : {}),
|
||||
...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {})
|
||||
})
|
||||
|
||||
closeSession = (sessionId: string): Promise<boolean> =>
|
||||
closeClaudeSession({
|
||||
|
||||
@@ -74,6 +74,16 @@ export type ClaudeSession = {
|
||||
events: StructuredAgentSessionEventSink | undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* The first-hand exit that removed a published session. Kept until the session
|
||||
* is acquired again so acquisition cleanup that arrives after the exit finds
|
||||
* what the ladder observed, not an absence it would otherwise report as proven.
|
||||
*/
|
||||
export type ClaudeSessionExit = {
|
||||
connection: ClaudeStreamJsonConnection
|
||||
error: Error
|
||||
}
|
||||
|
||||
export type ClaudeAcquisitionAttempt = {
|
||||
connection: ClaudeStreamJsonConnection | null
|
||||
prompts: ClaudePromptRegistry
|
||||
|
||||
+33
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter'
|
||||
import { StructuredAgentSessionAdapterRouter } from './structured-agent-session-adapter-router'
|
||||
|
||||
function adapterOf(
|
||||
releaseAcquisition: StructuredAgentSessionAdapter['releaseAcquisition']
|
||||
): StructuredAgentSessionAdapter {
|
||||
return {
|
||||
acquire: vi.fn(async () => ({ process: { pid: 1 } }) as never),
|
||||
releaseAcquisition,
|
||||
dispatch: vi.fn(),
|
||||
cancelTurn: vi.fn(),
|
||||
answerPrompt: vi.fn(),
|
||||
setOption: vi.fn()
|
||||
} as unknown as StructuredAgentSessionAdapter
|
||||
}
|
||||
|
||||
describe('StructuredAgentSessionAdapterRouter.releaseAcquisition', () => {
|
||||
it('drops the owner even when its release reports a typed failure', async () => {
|
||||
const failure = new Error('root exited')
|
||||
const claude = adapterOf(vi.fn().mockRejectedValueOnce(failure).mockResolvedValue(false))
|
||||
const codex = adapterOf(vi.fn(async () => false))
|
||||
const router = new StructuredAgentSessionAdapterRouter({ claude, codex }, async () => {})
|
||||
const identity = { sessionId: 'session-1', agent: 'claude' } as never
|
||||
await router.acquire({ identity, fence: 1, spawnToken: 'spawn-1' })
|
||||
|
||||
await expect(router.releaseAcquisition({ sessionId: 'session-1' })).rejects.toBe(failure)
|
||||
// With no owner left, a later release asks every adapter instead of the stale one.
|
||||
await expect(router.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(false)
|
||||
expect(claude.releaseAcquisition).toHaveBeenCalledTimes(2)
|
||||
expect(codex.releaseAcquisition).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -30,9 +30,11 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi
|
||||
async releaseAcquisition(input: { sessionId: string }): Promise<boolean> {
|
||||
const adapter = this.owners.get(input.sessionId)
|
||||
if (adapter) {
|
||||
const released = await adapter.releaseAcquisition?.(input)
|
||||
this.owners.delete(input.sessionId)
|
||||
return released === true
|
||||
try {
|
||||
return (await adapter.releaseAcquisition?.(input)) === true
|
||||
} finally {
|
||||
this.owners.delete(input.sessionId)
|
||||
}
|
||||
}
|
||||
let released = false
|
||||
for (const candidate of Object.values(this.adapters)) {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
AgentSessionAcquisitionExitUnprovenError,
|
||||
AgentSessionAcquisitionRootExitObservedError,
|
||||
rethrowAfterAgentSessionAcquisitionCleanup
|
||||
} from './structured-agent-session-adapter'
|
||||
|
||||
@@ -28,6 +29,27 @@ describe('failed agent-session acquisition cleanup', () => {
|
||||
).rejects.toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError)
|
||||
})
|
||||
|
||||
it('keeps a first-hand root exit that cleanup observed, with the provider diagnostic', async () => {
|
||||
const cause = new Error('proof failed')
|
||||
const exit = new AgentSessionAcquisitionRootExitObservedError(
|
||||
new Error('claude stream-json exited (code 1): crashed')
|
||||
)
|
||||
const error = await rethrowAfterAgentSessionAcquisitionCleanup(
|
||||
{
|
||||
releaseAcquisition: vi.fn(async () => {
|
||||
throw exit
|
||||
})
|
||||
},
|
||||
'session-1',
|
||||
cause
|
||||
).catch((thrown: unknown) => thrown)
|
||||
|
||||
expect(error).toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError)
|
||||
expect(error).not.toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError)
|
||||
expect((error as Error).message).toBe('claude stream-json exited (code 1): crashed')
|
||||
expect((error as Error).cause).toMatchObject({ errors: [cause, exit] })
|
||||
})
|
||||
|
||||
it('reports unproven exit when cleanup throws', async () => {
|
||||
const error = await rethrowAfterAgentSessionAcquisitionCleanup(
|
||||
{
|
||||
|
||||
@@ -119,7 +119,9 @@ export type StructuredAgentSessionAdapter = {
|
||||
* at — the store rejects a link minted at any other fence. */
|
||||
acquire(input: StructuredAgentSessionAcquireInput): Promise<AgentSessionAcquisition>
|
||||
/** Reaps an acquired provider when the host cannot commit or prove its lease.
|
||||
* Returns true only after provider child exit is proven. */
|
||||
* Returns true only after provider child exit is proven. Throws
|
||||
* `AgentSessionAcquisitionRootExitObservedError` when the provider root's own
|
||||
* exit was observed first-hand but its descendants could not be verified. */
|
||||
releaseAcquisition?(input: { sessionId: string }): Promise<boolean>
|
||||
dispatch(input: {
|
||||
sessionId: string
|
||||
@@ -168,9 +170,15 @@ export async function rethrowAfterAgentSessionAcquisitionCleanup(
|
||||
try {
|
||||
released = (await adapter.releaseAcquisition?.({ sessionId })) === true
|
||||
} catch (cleanupError) {
|
||||
throw new AgentSessionAcquisitionExitUnprovenError(
|
||||
new AggregateError([cause, cleanupError], 'agent session acquisition cleanup failed')
|
||||
)
|
||||
// A root exit the cleanup observed first-hand keeps its classification and its
|
||||
// provider diagnostic; the failure that triggered cleanup rides along as cause.
|
||||
throw cleanupError instanceof AgentSessionAcquisitionRootExitObservedError
|
||||
? new AgentSessionAcquisitionRootExitObservedError(
|
||||
new AggregateError([cause, cleanupError], cleanupError.message)
|
||||
)
|
||||
: new AgentSessionAcquisitionExitUnprovenError(
|
||||
new AggregateError([cause, cleanupError], 'agent session acquisition cleanup failed')
|
||||
)
|
||||
}
|
||||
if (released) {
|
||||
throw cause
|
||||
|
||||
Reference in New Issue
Block a user