fix(claude): close remaining structured session P1s

This commit is contained in:
Merge Sim
2026-09-02 11:38:54 -07:00
parent bf5c960464
commit 490a58f1d2
14 changed files with 256 additions and 32 deletions
+50
View File
@@ -0,0 +1,50 @@
# Claude structured Fable P1 fix
## Outcome
Fixed the three release-blocking findings against parent `bf5c9604640e8462814829dfec0b35ff2b612d96`:
1. Structured Claude leaf sampling now rejects stdout `user`/`assistant` UUIDs with a non-null `parent_tool_use_id`, and transcript branch proof marks those rows as disallowed. If a durable close or first-hand crash proof rejects the sampled cursor, the adapter re-proves from `previousLeafUuid: null` and persists the valid main-transcript leaf when available. Existing bounded ancestry, malformed-tail, sibling-branch, and resume protections remain fail-closed.
2. Structured SDK launches now carry `systemPrompt: { type: 'preset', preset: 'claude_code' }` in the base options. The connection test injects the SDK query boundary and pins the option passed to initialization, while account/config roots, setting sources, and the legacy CLI path remain unchanged.
3. `StructuredAgentSessionAdapterRouter.closeSession` retains the adapter owner when close is false/unproven and removes it only after an exact `closed === true`, allowing a later retry to reach the same provider adapter.
## Exact files changed
- `src/main/claude/claude-stream-json-connection.test.ts`
- `src/main/claude/claude-stream-json-connection.ts`
- `src/main/claude/claude-structured-launch-resolution.test.ts`
- `src/main/claude/claude-structured-launch-resolution.ts`
- `src/main/claude/claude-structured-session-adapter.ts`
- `src/main/claude/claude-structured-session-close.ts`
- `src/main/claude/claude-structured-session-recovery.test.ts`
- `src/main/claude/claude-transcript-branch-proof.ts`
- `src/main/claude/claude-tui-exit.test.ts`
- `src/main/claude/claude-tui-exit.ts`
- `src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts`
- `src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts`
- `src/main/native-chat/session-file-resolver.test.ts`
## Red-first tests and verification
The new tests were run before implementation and failed for each targeted behavior: parent-tool-use UUIDs were sampled/accepted, stale-cursor recovery retained the old leaf, the base system prompt was absent, and router ownership was dropped after a false close. After implementation:
- Focused P1 suites: 6 files, 69 tests passed.
- All Claude suites plus router/session resolver: 60 files, 512 tests passed, 2 skipped.
- Node typecheck: `node_modules/.bin/tsc --noEmit -p config/tsconfig.node.json` passed.
- Web typecheck: `node_modules/.bin/tsc --noEmit -p config/tsconfig.tc.web.json` passed.
- Native changed-area Oxlint with warnings denied passed.
- Changed-code quality gate passed with zero new findings across 13 files.
- Changed-file `oxfmt --check` and `git diff --check` passed.
- `pnpm-lock.yaml` was restored byte-for-byte to the parent and has no diff.
## Five-finding disposition
- Sidechain/subagent UUID and stale-cursor recovery: proven P1, fixed here; graceful close and first-hand crash regressions are covered.
- Missing Claude Code system prompt: proven P1, fixed here; the SDK query boundary is pinned.
- Router close ownership loss: proven P1, fixed here; false-then-true close retry is covered.
- Grouped-answer handling: proven separately and already fixed on the parent by the existing grouped-question change/report; no regression or scope overlap was found here, so it is dismissed from this dispatch.
- Local-image and cleanup-retry leads: reviewed as existing, unrelated paths; neither is changed or regressed by this diff, so both are dismissed from this dispatch (no new release-blocking evidence).
## Harness/environment and artifact notes
The repository's pnpm bootstrap added an `@pnpm/exe` lockfile entry while invoking pnpm commands; it was removed and the lockfile was rechecked clean. No Electron/mobile UI flow applies to these main-process/adapter changes. No pre-existing untracked artifacts were edited or deleted; the child remains at the requested parent lineage with only the source, tests, and this report changed.
@@ -4,7 +4,7 @@ import { join } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { spawnProcess, type SpawnedProcess } from '../../shared/child-process/run-process'
import type { ProcessSpec } from '../../shared/child-process/process-spec'
import type { CanUseTool } from '@anthropic-ai/claude-agent-sdk'
import { query, type CanUseTool, type Options } from '@anthropic-ai/claude-agent-sdk'
import {
openClaudeStreamJsonConnection,
type ClaudeStreamJsonConnection,
@@ -85,14 +85,20 @@ const spawnedChildren: SpawnedProcess[] = []
async function open(
launch: ClaudeStreamJsonLaunch,
handlers: Parameters<typeof openClaudeStreamJsonConnection>[1] = {}
handlers: Parameters<typeof openClaudeStreamJsonConnection>[1] = {},
queryImpl?: typeof query
): Promise<ClaudeStreamJsonConnection> {
const connection = await openClaudeStreamJsonConnection(launch, handlers, (spec) => {
spawned.push(spec)
const child = spawnProcess(spec)
spawnedChildren.push(child)
return child
})
const connection = await openClaudeStreamJsonConnection(
launch,
handlers,
(spec) => {
spawned.push(spec)
const child = spawnProcess(spec)
spawnedChildren.push(child)
return child
},
queryImpl
)
openConnections.push(connection)
return connection
}
@@ -121,6 +127,17 @@ function readReportSafely(scenario: { readReport: () => ScriptedCliReport }) {
}
describe('Claude stream-json connection', () => {
it('passes the Claude Code system-prompt preset through to SDK query', async () => {
const scenario = scriptScenario([HOLD_OPEN])
let captured: Options | undefined
await open(launchFor(scenario), {}, (params) => {
captured = params.options
return query(params)
})
expect(captured?.systemPrompt).toEqual({ type: 'preset', preset: 'claude_code' })
})
it('hands the child a derived environment, the resolved CLI path, and keeps the pid', async () => {
vi.stubEnv('ANTHROPIC_API_KEY', 'sk-ant-SHELL-LEAK')
vi.stubEnv('CLAUDE_CODE_CHILD_SESSION', '1')
@@ -94,12 +94,13 @@ function exitError(stderrTail: string, status: ExitStatus | null, cause?: Error)
export async function openClaudeStreamJsonConnection(
launch: ClaudeStreamJsonLaunch,
handlers: ClaudeStreamJsonConnectionHandlers = {},
spawnImpl: typeof spawnProcess = spawnProcess
spawnImpl: typeof spawnProcess = spawnProcess,
queryImpl?: typeof ClaudeAgentSdk.query
): Promise<ClaudeStreamJsonConnection> {
const { query } = await loadClaudeAgentSdk()
const spawner = createClaudeCodeProcessSpawn(spawnImpl)
const inbox = createClaudeUserMessageQueue()
const session = query({
const session = (queryImpl ?? query)({
prompt: inbox.messages,
options: {
...launch.options,
@@ -77,6 +77,7 @@ describe('claude structured launch resolution', () => {
settingSources: [...CLAUDE_DEFAULT_SETTING_SOURCES],
supportedDialogKinds: [],
extraArgs: { 'replay-user-messages': null },
systemPrompt: { type: 'preset', preset: 'claude_code' },
sessionId: first.providerSessionId
})
expect(first.options.resume).toBeUndefined()
@@ -13,6 +13,7 @@ export const CLAUDE_DEFAULT_SETTING_SOURCES = ['user', 'project', 'local'] as co
export type ClaudeStructuredSdkOptions = Pick<
ClaudeAgentSdkOptions,
| 'includePartialMessages'
| 'systemPrompt'
| 'settingSources'
| 'supportedDialogKinds'
| 'extraArgs'
@@ -33,6 +34,8 @@ export type ClaudeStructuredSdkOptions = Pick<
*/
export const CLAUDE_STRUCTURED_BASE_OPTIONS: ClaudeStructuredSdkOptions = {
includePartialMessages: true,
// Keep the SDK on Claude Code's own system-prompt contract.
systemPrompt: { type: 'preset', preset: 'claude_code' },
settingSources: [...CLAUDE_DEFAULT_SETTING_SOURCES],
supportedDialogKinds: [],
extraArgs: { 'replay-user-messages': null }
@@ -27,6 +27,7 @@ import {
closeClaudeSession,
settleClaudeExitedSession
} from './claude-structured-session-close'
import { readClaudeTranscriptLeafWithReproof } from './claude-transcript-branch-proof'
export type { ClaudeStructuredLaunch } from './claude-structured-launch-resolution'
export type {
@@ -97,10 +98,13 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
private async persistSessionHandle(sessionId: string, session: ClaudeSession): Promise<void> {
try {
const transcriptLeaf = await this.deps.readTranscriptLeaf?.({
providerSessionId: session.providerSessionId,
previousLeafUuid: session.leafUuid
})
const transcriptLeaf = this.deps.readTranscriptLeaf
? await readClaudeTranscriptLeafWithReproof({
readTranscriptLeaf: this.deps.readTranscriptLeaf,
providerSessionId: session.providerSessionId,
previousLeafUuid: session.leafUuid
})
: null
if (transcriptLeaf) {
session.leafUuid = transcriptLeaf
}
@@ -5,6 +5,7 @@ import type {
} from './claude-structured-session-state'
import { cancelClaudeAcquisitionAttempt } from './claude-structured-session-state'
import { closeProcessRegistry } from '../../shared/child-process/close-process-registry'
import { readClaudeTranscriptLeafWithReproof } from './claude-transcript-branch-proof'
export function settleClaudeDispatchWaiters(session: ClaudeSession): void {
for (const waiter of session.dispatchWaiters.splice(0)) {
@@ -50,10 +51,13 @@ export async function closeClaudePublishedSession(input: {
return false
}
try {
const transcriptLeaf = await input.readTranscriptLeaf?.({
providerSessionId: session.providerSessionId,
previousLeafUuid: session.leafUuid
})
const transcriptLeaf = input.readTranscriptLeaf
? await readClaudeTranscriptLeafWithReproof({
readTranscriptLeaf: input.readTranscriptLeaf,
providerSessionId: session.providerSessionId,
previousLeafUuid: session.leafUuid
})
: null
if (transcriptLeaf) {
session.leafUuid = transcriptLeaf
}
@@ -72,17 +72,14 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => {
expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'durable-tail' })
})
it('keeps the observed leaf when transcript validation rejects a stale branch', async () => {
it('re-proves from the transcript root when the observed cursor rejects a stale branch', async () => {
const claude = fakeClaude()
const persistedHandles: unknown[] = []
const adapter = adapterFor(
claude,
{},
[],
persistedHandles,
undefined,
vi.fn().mockRejectedValue(new Error('sibling branch'))
)
const readTranscriptLeaf = vi
.fn()
.mockRejectedValueOnce(new Error('sibling branch'))
.mockResolvedValueOnce('reproved-main-leaf')
const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf)
await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' })
claude.connections[0].handlers.onMessage?.({
type: 'assistant',
@@ -92,7 +89,15 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => {
await adapter.closeSession('session-1')
expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'observed-tail' })
expect(readTranscriptLeaf).toHaveBeenNthCalledWith(1, {
providerSessionId: PROVIDER_SESSION_ID,
previousLeafUuid: 'observed-tail'
})
expect(readTranscriptLeaf).toHaveBeenNthCalledWith(2, {
providerSessionId: PROVIDER_SESSION_ID,
previousLeafUuid: null
})
expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'reproved-main-leaf' })
})
it('persists the last transcript leaf before an unexpected first-hand exit', async () => {
@@ -151,6 +156,34 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => {
expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'durable-crash-leaf' })
})
it('re-proves a first-hand crash cursor from the transcript root after stale validation', async () => {
const claude = fakeClaude()
const persistedHandles: unknown[] = []
const readTranscriptLeaf = vi
.fn()
.mockRejectedValueOnce(new Error('stale cursor'))
.mockResolvedValueOnce('reproved-crash-leaf')
const adapter = adapterFor(claude, {}, [], persistedHandles, undefined, readTranscriptLeaf)
await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' })
claude.connections[0].handlers.onMessage?.({
type: 'assistant',
session_id: PROVIDER_SESSION_ID,
uuid: 'stale-observed-tail'
})
claude.connections[0].handlers.onExit?.(new Error('crashed'))
await tick()
expect(readTranscriptLeaf).toHaveBeenNthCalledWith(1, {
providerSessionId: PROVIDER_SESSION_ID,
previousLeafUuid: 'stale-observed-tail'
})
expect(readTranscriptLeaf).toHaveBeenNthCalledWith(2, {
providerSessionId: PROVIDER_SESSION_ID,
previousLeafUuid: null
})
expect(persistedHandles.at(-1)).toMatchObject({ leafUuid: 'reproved-crash-leaf' })
})
it('publishes lifecycle recovery even when crash-cursor persistence fails', async () => {
const claude = fakeClaude()
const events: ClaudeStructuredSessionEvent[] = []
@@ -74,6 +74,7 @@ export function proveClaudeTranscriptBranchFromJsonl(input: {
const existing = nodes.get(uuid)
const disallowedLeaf =
row.isSidechain === true ||
row.parent_tool_use_id != null ||
row.type === 'result' ||
row.type === 'stream_event' ||
(row.type === 'system' && row.subtype === 'init')
@@ -160,3 +161,28 @@ export async function proveClaudeTranscriptBranch(input: {
previousLeafUuid: input.previousLeafUuid
})
}
/** Re-run a durable branch proof from the transcript root when a sampled cursor is stale. */
export async function readClaudeTranscriptLeafWithReproof(input: {
readTranscriptLeaf: (input: {
providerSessionId: string
previousLeafUuid: string | null
}) => Promise<string | null>
providerSessionId: string
previousLeafUuid: string | null
}): Promise<string | null> {
try {
return await input.readTranscriptLeaf({
providerSessionId: input.providerSessionId,
previousLeafUuid: input.previousLeafUuid
})
} catch (error) {
if (input.previousLeafUuid === null) {
throw error
}
return input.readTranscriptLeaf({
providerSessionId: input.providerSessionId,
previousLeafUuid: null
})
}
}
+22 -1
View File
@@ -2,7 +2,11 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { completeClaudeTuiExit, readClaudeTranscriptLeafUuid } from './claude-tui-exit'
import {
completeClaudeTuiExit,
readClaudeTranscriptEntryUuid,
readClaudeTranscriptLeafUuid
} from './claude-tui-exit'
const roots: string[] = []
@@ -11,6 +15,23 @@ afterEach(async () => {
})
describe('Claude TUI exit', () => {
it('does not sample UUIDs from subagent stdout frames with a parent tool use', () => {
expect(
readClaudeTranscriptEntryUuid({
type: 'assistant',
uuid: 'subagent-assistant',
parent_tool_use_id: 'parent-tool'
})
).toBeNull()
expect(
readClaudeTranscriptEntryUuid({
type: 'assistant',
uuid: 'main-assistant',
parent_tool_use_id: null
})
).toBe('main-assistant')
})
it('reads the authoritative last-prompt leaf from a transcript tail', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-claude-tui-exit-'))
roots.push(root)
+3 -1
View File
@@ -19,7 +19,9 @@ function validLeafUuid(value: unknown): string | null {
}
export function readClaudeTranscriptEntryUuid(value: Record<string, unknown>): string | null {
return value.isSidechain === true || (value.type !== 'user' && value.type !== 'assistant')
return value.isSidechain === true ||
value.parent_tool_use_id != null ||
(value.type !== 'user' && value.type !== 'assistant')
? null
: validLeafUuid(value.uuid)
}
@@ -31,3 +31,30 @@ describe('StructuredAgentSessionAdapterRouter.releaseAcquisition', () => {
expect(codex.releaseAcquisition).toHaveBeenCalledTimes(1)
})
})
describe('StructuredAgentSessionAdapterRouter.closeSession', () => {
it('retains the owner after an unproven close so a later retry reaches the same adapter', async () => {
const claude = adapterOf(vi.fn(async () => true))
const closeSession = vi.fn().mockResolvedValueOnce(false).mockResolvedValueOnce(true)
const dispatch = vi.fn().mockResolvedValue({ state: 'unknown', reason: 'test' })
claude.closeSession = closeSession
claude.dispatch = dispatch
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.closeSession('session-1')).resolves.toBe(false)
await expect(
router.dispatch({
sessionId: 'session-1',
clientMessageId: 'client-1',
body: {} as never,
fence: 1
})
).resolves.toMatchObject({ state: 'unknown' })
await expect(router.closeSession('session-1')).resolves.toBe(true)
expect(closeSession).toHaveBeenCalledTimes(2)
expect(dispatch).toHaveBeenCalledTimes(1)
})
})
@@ -72,8 +72,11 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi
return false
}
const closed = await adapter.closeSession?.(sessionId)
this.owners.delete(sessionId)
return closed === true
if (closed === true) {
this.owners.delete(sessionId)
return true
}
return false
}
async closeAll(): Promise<void> {
@@ -206,6 +206,38 @@ describe('resolveSessionFilePath', () => {
)
})
it('rejects a main leaf whose ancestry crosses a parent-tool-use sidechain', async () => {
const root = await makeRoot('orca-native-chat-resolve-claude-parent-tool-ancestry-')
const transcript = join(root, 'transcript.jsonl')
await writeFile(
transcript,
[
{ type: 'user', uuid: 'main-user', parentUuid: null, sessionId: 'session-1' },
{
type: 'assistant',
uuid: 'subagent-assistant',
parentUuid: 'main-user',
sessionId: 'session-1',
parent_tool_use_id: 'tool-use-1'
},
{
type: 'assistant',
uuid: 'main-after-sidechain',
parentUuid: 'subagent-assistant',
sessionId: 'session-1'
},
{ type: 'last-prompt', leafUuid: 'main-after-sidechain', sessionId: 'session-1' }
]
.map((record) => JSON.stringify(record))
.join('\n'),
'utf8'
)
await expect(readClaudeTranscriptLeafUuid(transcript, 'session-1')).rejects.toThrow(
'not on the main transcript'
)
})
it('globs Claude project subdirs for <sessionId>.jsonl', async () => {
const root = await makeRoot('orca-native-chat-resolve-claude-')
const claudeProjectsDir = join(root, 'claude-projects')