diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9097f89fd41..9098a6f566d 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -29,7 +29,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/claude-structured-exit-mid-response.test.ts', 'src/main/runtime/claude-structured-failed-start-resume.test.ts', 'src/main/runtime/claude-structured-resumed-start-failure.test.ts', - 'src/main/runtime/claude-structured-send-held-for-startup.test.ts', + 'src/main/runtime/claude-structured-send-during-startup.test.ts', 'src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts', 'src/main/runtime/claude-structured-session-integration.test.ts', 'src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts', diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index cffe4284978..c8705ab6cc7 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -46,6 +46,7 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/native-chat/agent-session-wire/structured-agent-session-attach-reconciliation.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts', + 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-option-queue.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts', diff --git a/src/main/claude/claude-adapter-capture-replay.test.ts b/src/main/claude/claude-adapter-capture-replay.test.ts index 8e877a135a8..14cc590e2b2 100644 --- a/src/main/claude/claude-adapter-capture-replay.test.ts +++ b/src/main/claude/claude-adapter-capture-replay.test.ts @@ -24,7 +24,8 @@ import type { import { fakeClaude, identityFor, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from './claude-structured-session-test-support' type CapturedEvent = @@ -153,7 +154,7 @@ async function replayCapture(name: string): Promise { if (event.kind === 'dispatch') { if (!startedAwaited) { // The proof frames have been delivered by now; startup can settle. - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') startedAwaited = true } const body: AgentJournalMessageItem = { diff --git a/src/main/claude/claude-command-lifecycle.test.ts b/src/main/claude/claude-command-lifecycle.test.ts index f4182a60174..88aa6d4a12f 100644 --- a/src/main/claude/claude-command-lifecycle.test.ts +++ b/src/main/claude/claude-command-lifecycle.test.ts @@ -23,7 +23,8 @@ import { fakeClaude, identityFor, PROVIDER_SESSION_ID, - type FakeConnection + type FakeConnection, + claudeStartupSettled } from './claude-structured-session-test-support' type CapturedEvent = @@ -195,7 +196,7 @@ async function replayCapture( proofDelivered = true } else if (event.kind === 'dispatch') { if (proofDelivered) { - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') } await expect( adapter.dispatch({ diff --git a/src/main/claude/claude-fold-steer-receipt.test.ts b/src/main/claude/claude-fold-steer-receipt.test.ts index 4726dc55d82..bc15a52cecd 100644 --- a/src/main/claude/claude-fold-steer-receipt.test.ts +++ b/src/main/claude/claude-fold-steer-receipt.test.ts @@ -23,7 +23,8 @@ import type { ClaudeStructuredLaunch } from './claude-structured-launch-resoluti import { fakeClaude, identityFor, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from './claude-structured-session-test-support' import { assistantText, @@ -100,7 +101,7 @@ async function riggedAdapter( } const adapter = new ClaudeStructuredSessionAdapter(deps) await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9', events: sink }) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') return { adapter, claude, diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index 5359b057798..f9c61b9b8e4 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -575,11 +575,11 @@ describe('Claude stream-json connection', () => { const init = { providerSessionId: SESSION_ID, uuid: null, model: null, message: {} } // With no ambient auth, every true below can only have come from the CLI's settings. - expect(claudeAuthDiagnostic(init, null)).toMatchObject({ + expect(claudeAuthDiagnostic(null, init, null)).toMatchObject({ baseUrlConfigured: false, authTokenConfigured: false }) - const diagnostic = claudeAuthDiagnostic(init, await connection.getSettings()) + const diagnostic = claudeAuthDiagnostic(null, init, await connection.getSettings()) expect(diagnostic).toMatchObject({ baseUrlConfigured: true, authTokenConfigured: true, diff --git a/src/main/claude/claude-structured-acquisition-launch.ts b/src/main/claude/claude-structured-acquisition-launch.ts index 4de526c7c4d..66f0bf0d665 100644 --- a/src/main/claude/claude-structured-acquisition-launch.ts +++ b/src/main/claude/claude-structured-acquisition-launch.ts @@ -7,6 +7,10 @@ import { import { stopAgentSessionProviderRoot } from '../native-chat/agent-session-wire/structured-agent-session-provider-exit-proof' import { withAgentSessionCreatePhase } from '../observability/agent-session-instrumentation' import type { ClaudeStructuredLaunch } from './claude-structured-launch-resolution' +import { + claudeStructuredSpawnOptions, + type ClaudeStructuredSpawnOptions +} from './claude-structured-spawn-options' import { cancelClaudeAcquisitionAttempt, type ClaudeAcquisitionAttempt, @@ -21,6 +25,11 @@ import { closeClaudePublishedSessionForDeps } from './claude-structured-session-close' +/** The launch with the chat's saved options in its spawn options. */ +export type ClaudeAcquisitionLaunch = ClaudeStructuredLaunch & { + savedOptions: Omit +} + export async function resolveClaudeAcquisitionLaunch(args: { input: StructuredAgentSessionAcquireInput deps: ClaudeStructuredSessionAdapterDeps @@ -30,7 +39,7 @@ export async function resolveClaudeAcquisitionLaunch(args: { callbacks: ClaudeAcquireCallbacks previous: ClaudeAcquisitionAttempt | undefined attempt: ClaudeAcquisitionAttempt -}): Promise { +}): Promise { const { input, deps, sessions, acquisitions, exits, callbacks, previous, attempt } = args const sessionId = input.identity.sessionId return withAgentSessionCreatePhase('auth_settle', input.recordPhase, async () => { @@ -87,6 +96,15 @@ export async function resolveClaudeAcquisitionLaunch(args: { : new AgentSessionPreSpawnError(error) }) acquisitions.assertCurrent(sessionId, attempt) - return launch + const spawn = claudeStructuredSpawnOptions({ launch, saved: input.options }) + return { + ...launch, + options: spawn.sdkOptions, + savedOptions: { + options: spawn.options, + skipped: spawn.skipped, + fastModeAtStart: spawn.fastModeAtStart + } + } }) } diff --git a/src/main/claude/claude-structured-child-env.ts b/src/main/claude/claude-structured-child-env.ts new file mode 100644 index 00000000000..5f787ca6060 --- /dev/null +++ b/src/main/claude/claude-structured-child-env.ts @@ -0,0 +1,79 @@ +// What a structured Claude child's environment is built from, shared by its launch and the +// `--version` probe that runs the same binary. + +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' +import { applyClaudeEnvPatch } from '../claude-accounts/environment' +import { resolveClaudeCommand } from '../codex-cli/command' +import { withoutInheritedClaudeConfigDir } from './claude-config-dir-pin' +import type { ClaudeStructuredLaunchResolverDeps } from './claude-structured-launch-resolution' + +function cloneDefinedEnv(env: NodeJS.ProcessEnv | Record): Record { + const next: Record = {} + for (const [key, value] of Object.entries(env)) { + if (value !== undefined) { + next[key] = value + } + } + return next +} + +export type ClaudeEnvDeps = Pick< + ClaudeStructuredLaunchResolverDeps, + 'resolveCommand' | 'resolveEnv' | 'resolveInheritedEnv' +> + +/** What a child's env is built from, before any auth policy applies to it. */ +export type ClaudeChildEnvSources = { + command: string + overlay: Record | undefined + inheritedEnv: Record +} + +export async function resolveClaudeChildEnvSources( + deps: ClaudeEnvDeps +): Promise { + const command = (deps.resolveCommand ?? resolveClaudeCommand)() + const overlay = await deps.resolveEnv?.() + const inheritedEnv = deps.resolveInheritedEnv + ? await deps.resolveInheritedEnv() + : cloneDefinedEnv(process.env) + return { command, overlay: overlay ? cloneDefinedEnv(overlay) : undefined, inheritedEnv } +} + +// Why the overlay merges onto the inherited env rather than replacing it: the child +// still needs PATH and the rest of the shell environment, and withCliRuntimeOnPath +// derives PATH from what it is handed. Ambient Anthropic auth is stripped from the +// inherited half only when a managed account owns the credential; a system-auth +// user's own key is their sign-in and must reach the child. +export function claudeChildEnv( + sources: ClaudeChildEnvSources, + stripAuthEnv: boolean, + decorateEnv: (env: Record) => Record = (env) => env +): Record { + return withCliRuntimeOnPath( + sources.command, + decorateEnv({ + ...applyClaudeEnvPatch( + withoutInheritedClaudeConfigDir(sources.inheritedEnv, process.platform), + {}, + { stripAuthEnv, platform: process.platform } + ), + ...sources.overlay + }), + { platform: process.platform } + ) +} + +/** The env a `--version` probe runs with: the launch's own env, PATH and shims included, minus the + * Claude auth variables, auth-like custom headers and an inherited CLAUDE_CONFIG_DIR, which asking + * a version needs none of. Everything else is what this same binary receives at launch anyway. */ +export function claudeProbeEnv(sources: ClaudeChildEnvSources): Record { + return applyClaudeEnvPatch( + claudeChildEnv(sources, true), + {}, + { + stripAuthEnv: true, + platform: process.platform + } + ) +} diff --git a/src/main/claude/claude-structured-dispatch-test-support.ts b/src/main/claude/claude-structured-dispatch-test-support.ts index 5af0f17243b..80a774e648b 100644 --- a/src/main/claude/claude-structured-dispatch-test-support.ts +++ b/src/main/claude/claude-structured-dispatch-test-support.ts @@ -29,6 +29,8 @@ export function sessionFor(send: Mock = vi.fn().mockResolvedValue(undefined)): C reportedModelMutation: 0, confirmedOptions: new Set(), restoreSkippedOptions: new Set(), + launchedModel: null, + fastModeAtStart: false, capabilities: [], events: undefined, translator: null, diff --git a/src/main/claude/claude-structured-effort-default-at-rest.test.ts b/src/main/claude/claude-structured-effort-default-at-rest.test.ts index 92cf562b9dc..dcdad5b868b 100644 --- a/src/main/claude/claude-structured-effort-default-at-rest.test.ts +++ b/src/main/claude/claude-structured-effort-default-at-rest.test.ts @@ -25,7 +25,8 @@ import { PROVIDER_SESSION_ID, fakeClaude, identityFor, - recordingJournalSink + recordingJournalSink, + claudeStartupSettled } from './claude-structured-session-test-support' const SESSION = 'session-1' @@ -120,7 +121,7 @@ async function startChild( events: recordingJournalSink(), ...(options ? { options } : {}) }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) return adapter } diff --git a/src/main/claude/claude-structured-effort-reporting.test.ts b/src/main/claude/claude-structured-effort-reporting.test.ts index 2c319c86ee7..3305a40d77a 100644 --- a/src/main/claude/claude-structured-effort-reporting.test.ts +++ b/src/main/claude/claude-structured-effort-reporting.test.ts @@ -1,9 +1,6 @@ import { describe, expect, it } from 'vitest' import { AgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error' -import { - restoreClaudeStructuredSessionOptions, - setClaudeStructuredOption -} from './claude-structured-options' +import { setClaudeStructuredOption } from './claude-structured-options' import { readClaudeSettingsEffort } from './claude-structured-session-options' import type { ClaudeSession } from './claude-structured-session-state' import type { ClaudeStructuredSessionEvent } from './claude-structured-session-adapter' @@ -238,29 +235,4 @@ describe('Claude effort against the model that must run it', () => { setClaudeStructuredOption(session, { key: 'effort', value: 'high' }, undefined) ).rejects.toBeInstanceOf(AgentSessionOptionRejectedError) }) - - it('keeps a disagreeing effort through restore instead of skipping it', async () => { - const calls: string[] = [] - const { session } = sessionWith('high', calls, { model: 'sonnet', catalog: [SONNET] }) - session.options.set('effort', 'low') - - await restoreClaudeStructuredSessionOptions(session, undefined) - - expect(session.options.get('effort')).toBe('low') - expect(session.restoreSkippedOptions.has('effort')).toBe(false) - expect(session.confirmedOptions.has('effort')).toBe(false) - }) - - it('drops a stale effort on restore instead of replaying it onto the new model', async () => { - const calls: string[] = [] - const { session } = sessionWith('high', calls, { model: 'sonnet', catalog: [HAIKU, SONNET] }) - session.options.set('model', 'haiku') - session.options.set('effort', 'high') - - await restoreClaudeStructuredSessionOptions(session, undefined) - - expect(session.options.has('effort')).toBe(false) - expect(session.restoreSkippedOptions.has('effort')).toBe(true) - expect(calls.filter((call) => call.startsWith('apply:'))).toEqual([]) - }) }) diff --git a/src/main/claude/claude-structured-event-delivery.ts b/src/main/claude/claude-structured-event-delivery.ts index 6db9f38b48a..9e543a37a65 100644 --- a/src/main/claude/claude-structured-event-delivery.ts +++ b/src/main/claude/claude-structured-event-delivery.ts @@ -1,4 +1,8 @@ import { claudePromptCardWritten } from './claude-child-work-evidence' +import { + resetClaudeRetiredModel, + retireClaudeLaunchedModel +} from './claude-structured-retired-model' import type { ClaudeSession, ClaudeStructuredSessionAdapterDeps, @@ -8,7 +12,7 @@ import type { type ClaudeEventDelivery = { session: ClaudeSession | null event: ClaudeStructuredSessionEvent - deps: Pick + deps: Pick publishChildWork: ( sessionId: string, session?: ClaudeSession | null, @@ -38,6 +42,18 @@ export function emitClaudeStructuredSessionEvent({ } session?.translator?.handle(event) deps.onEvent?.(event) + const retired = + event.type === 'message' && session ? retireClaudeLaunchedModel(session, event.message) : null + if (session && retired !== null) { + deps.onEvent?.({ + type: 'options-skipped', + sessionId: event.sessionId, + fence: session.fence, + acquisitionGeneration: session.acquisitionGeneration, + options: { model: retired } + }) + resetClaudeRetiredModel(session, deps, event.sessionId, retired) + } publishChildWork(event.sessionId, session, event.type === 'message' ? event.message : null) // A prompt blocks its child only after the journal has written its card. void claudePromptCardWritten(session, event)?.then(() => publishChildWork(event.sessionId)) diff --git a/src/main/claude/claude-structured-init-proof.ts b/src/main/claude/claude-structured-init-proof.ts index e138abd3641..869bc453577 100644 --- a/src/main/claude/claude-structured-init-proof.ts +++ b/src/main/claude/claude-structured-init-proof.ts @@ -84,11 +84,16 @@ export function claudeInitializationAuthError( } export function claudeAuthDiagnostic( - init: ClaudeInitObservation, + initialization: unknown, + init: ClaudeInitObservation | null, settings: unknown ): ClaudeAuthDiagnostic { const env = isRecord(settings) && isRecord(settings.env) ? settings.env : {} - const apiKeySource = readClaudeFrameString(init.message, 'apiKeySource') + const account = + isRecord(initialization) && isRecord(initialization.account) ? initialization.account : {} + const apiKeySource = + readClaudeFrameString(account, 'apiKeySource') ?? + (init ? readClaudeFrameString(init.message, 'apiKeySource') : null) const configured = (key: string): boolean => (typeof env[key] === 'string' && (env[key] as string).trim().length > 0) || Boolean(process.env[key]?.trim()) diff --git a/src/main/claude/claude-structured-launch-resolution.ts b/src/main/claude/claude-structured-launch-resolution.ts index 4b03c941018..3f86b31c666 100644 --- a/src/main/claude/claude-structured-launch-resolution.ts +++ b/src/main/claude/claude-structured-launch-resolution.ts @@ -11,12 +11,10 @@ import { } from '../../shared/agent-session-provider-handle' import { claudeProviderHandleLeafUuid } from '../../shared/agent-session-provider-handle-encoding' import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' -import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import { structuredSessionChildIdentityEnv } from '../runtime/structured-session-child-identity-env' import { CLAUDE_AUTH_ENV_CONFLICT_MESSAGE, CLAUDE_AUTH_SWITCH_IN_PROGRESS_MESSAGE, - applyClaudeEnvPatch, hasClaudeAuthEnvConflict } from '../claude-accounts/environment' import type { ClaudeStructuredAuthPolicy } from '../claude-accounts/claude-structured-auth-policy' @@ -30,9 +28,14 @@ import { structuredClaudeMatchesActiveManagedAccount, type ClaudeManagedAccountGateSettings } from '../native-chat/claude-structured-managed-account-support' -import { resolveClaudeCommand } from '../codex-cli/command' import { resolveSessionFilePath } from '../native-chat/session-file-resolver' -import { withoutInheritedClaudeConfigDir } from './claude-config-dir-pin' +import { + claudeChildEnv, + claudeProbeEnv, + resolveClaudeChildEnvSources, + type ClaudeChildEnvSources, + type ClaudeEnvDeps +} from './claude-structured-child-env' import { claudeStructuredLaunchArgs } from './claude-structured-launch-args' import type { ClaudeThinkingDisplaySupport } from './claude-thinking-display-support' import type { AgentSessionRecordStore } from '../runtime/agent-session-record-store' @@ -54,6 +57,7 @@ export type ClaudeStructuredSdkOptions = Pick< | 'effort' | 'permissionMode' | 'allowDangerouslySkipPermissions' + | 'settings' | 'sessionId' | 'resume' > @@ -75,16 +79,6 @@ export const CLAUDE_STRUCTURED_BASE_OPTIONS: ClaudeStructuredSdkOptions = { extraArgs: { 'replay-user-messages': null } } -function cloneDefinedEnv(env: NodeJS.ProcessEnv | Record): Record { - const next: Record = {} - for (const [key, value] of Object.entries(env)) { - if (value !== undefined) { - next[key] = value - } - } - return next -} - /** * Agent Permissions as query-start options. * @@ -159,67 +153,6 @@ async function claudeTranscriptExists(input: { export type ClaudeStructuredInvocation = { command: string; env: Record } -type ClaudeEnvDeps = Pick< - ClaudeStructuredLaunchResolverDeps, - 'resolveCommand' | 'resolveEnv' | 'resolveInheritedEnv' -> - -/** What a child's env is built from, before any auth policy applies to it. */ -export type ClaudeChildEnvSources = { - command: string - overlay: Record | undefined - inheritedEnv: Record -} - -export async function resolveClaudeChildEnvSources( - deps: ClaudeEnvDeps -): Promise { - const command = (deps.resolveCommand ?? resolveClaudeCommand)() - const overlay = await deps.resolveEnv?.() - const inheritedEnv = deps.resolveInheritedEnv - ? await deps.resolveInheritedEnv() - : cloneDefinedEnv(process.env) - return { command, overlay: overlay ? cloneDefinedEnv(overlay) : undefined, inheritedEnv } -} - -// Why the overlay merges onto the inherited env rather than replacing it: the child -// still needs PATH and the rest of the shell environment, and withCliRuntimeOnPath -// derives PATH from what it is handed. Ambient Anthropic auth is stripped from the -// inherited half only when a managed account owns the credential; a system-auth -// user's own key is their sign-in and must reach the child. -function claudeChildEnv( - sources: ClaudeChildEnvSources, - stripAuthEnv: boolean, - decorateEnv: (env: Record) => Record = (env) => env -): Record { - return withCliRuntimeOnPath( - sources.command, - decorateEnv({ - ...applyClaudeEnvPatch( - withoutInheritedClaudeConfigDir(sources.inheritedEnv, process.platform), - {}, - { stripAuthEnv, platform: process.platform } - ), - ...sources.overlay - }), - { platform: process.platform } - ) -} - -/** The env a `--version` probe runs with: the launch's own env, PATH and shims included, minus the - * Claude auth variables, auth-like custom headers and an inherited CLAUDE_CONFIG_DIR, which asking - * a version needs none of. Everything else is what this same binary receives at launch anyway. */ -export function claudeProbeEnv(sources: ClaudeChildEnvSources): Record { - return applyClaudeEnvPatch( - claudeChildEnv(sources, true), - {}, - { - stripAuthEnv: true, - platform: process.platform - } - ) -} - /** * The one place a structured Claude child's binary and environment are * resolved. The session launch and the session-less catalog probe both build diff --git a/src/main/claude/claude-structured-model-preflight.test.ts b/src/main/claude/claude-structured-model-preflight.test.ts index 930947e112d..a44ca0232b8 100644 --- a/src/main/claude/claude-structured-model-preflight.test.ts +++ b/src/main/claude/claude-structured-model-preflight.test.ts @@ -1,9 +1,6 @@ import { describe, expect, it } from 'vitest' import { AgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error' -import { - restoreClaudeStructuredSessionOptions, - setClaudeStructuredOption -} from './claude-structured-options' +import { setClaudeStructuredOption } from './claude-structured-options' import type { ClaudeSession } from './claude-structured-session-state' /** Verbatim row shapes from Claude Code 2.1.260's list_models response. */ @@ -58,18 +55,6 @@ describe('Claude model pre-flight against the catalog the CLI listed', () => { expect(session.options.has('model')).toBe(false) }) - it('refuses an unlisted model replayed by restore, and skips it', async () => { - // Needs no user error: a model valid when it was persisted can be retired. - const { session, calls } = sessionWith([DEFAULT_ROW, SONNET]) - session.options.set('model', 'claude-opus-4-retired') - - await restoreClaudeStructuredSessionOptions(session, undefined) - - expect(calls).toEqual(['list_models']) - expect(session.options.has('model')).toBe(false) - expect([...session.restoreSkippedOptions]).toEqual(['model']) - }) - it('applies a model the provider lists', async () => { const { session, calls } = sessionWith([DEFAULT_ROW, SONNET, HAIKU]) @@ -89,8 +74,7 @@ describe('Claude model pre-flight against the catalog the CLI listed', () => { }) it('refuses nothing when list_models is unavailable', async () => { - // A CLI predating list_models would otherwise have every model refused, and - // restore swallows the rejection, so the user's pick would vanish silently. + // A CLI predating list_models would otherwise have every model refused. const { session, calls } = sessionWith('unavailable') await expect( @@ -118,15 +102,4 @@ describe('Claude model pre-flight against the catalog the CLI listed', () => { ).resolves.toEqual({ model: 'sonnet' }) expect(calls).toEqual(['list_models', 'set_model:sonnet']) }) - - it('leaves a restored model the provider lists in place', async () => { - const { session, calls } = sessionWith([DEFAULT_ROW, SONNET]) - session.options.set('model', 'sonnet') - - await restoreClaudeStructuredSessionOptions(session, undefined) - - expect(calls).toEqual(['list_models', 'set_model:sonnet']) - expect(session.options.get('model')).toBe('sonnet') - expect([...session.restoreSkippedOptions]).toEqual([]) - }) }) diff --git a/src/main/claude/claude-structured-options-context-window.test.ts b/src/main/claude/claude-structured-options-context-window.test.ts index 200bb083069..fb901fb9085 100644 --- a/src/main/claude/claude-structured-options-context-window.test.ts +++ b/src/main/claude/claude-structured-options-context-window.test.ts @@ -4,10 +4,8 @@ import { selectStructuredAgentContextUsage } from '../../shared/structured-agent import { assistantFrame, journal, userFrame } from './claude-context-usage-test-support' import { sessionFor } from './claude-structured-dispatch-test-support' import { createClaudeJournalTranslator } from './claude-structured-journal-translation' -import { - restoreClaudeStructuredSessionOptions, - setClaudeStructuredOption -} from './claude-structured-options' +import { setClaudeStructuredOption } from './claude-structured-options' +import { adoptClaudeStructuredSpawnOptions } from './claude-structured-spawn-options' import type { ClaudeSession } from './claude-structured-session-state' function ringSession(catalog: unknown[] = []) { @@ -38,10 +36,13 @@ function ringSession(catalog: unknown[] = []) { } describe('the context ring after a session option write', () => { - it('sizes a new session from the model its restore applied', async () => { - const s = ringSession([{ value: 'opus[1m]', displayName: 'Opus (1M)' }]) - s.session.options.set('model', 'opus[1m]') - await restoreClaudeStructuredSessionOptions(s.session, undefined) + it('sizes a new session from the model it was launched with', () => { + const s = ringSession() + adoptClaudeStructuredSpawnOptions(s.session, { + options: new Map([['model', 'opus[1m]']]), + skipped: [], + fastModeAtStart: false + }) expect(s.respond('turn-a', 1_000)).toMatchObject({ windowTokens: 1_000_000, percentage: 10 }) }) @@ -53,15 +54,18 @@ describe('the context ring after a session option write', () => { expect(s.respond('turn-b', 2_000)).toMatchObject({ windowTokens: 1_000_000, percentage: 10 }) }) - it('implies no window for a model the child refused or a restore could not put back', async () => { + it('implies no window for a model the child refused or a mode the launch left out', async () => { const refused = ringSession() refused.setModel.mockRejectedValueOnce(new ClaudeControlRequestError('set_model', 'refused')) await expect(refused.write('model', 'opus[1m]')).rejects.toThrow() expect(refused.respond('turn-a', 1_000)).toBeNull() - const skipped = ringSession([{ value: 'sonnet', displayName: 'Sonnet' }]) - skipped.session.options.set('model', 'retired-model[1m]') - await restoreClaudeStructuredSessionOptions(skipped.session, undefined) + const skipped = ringSession() + adoptClaudeStructuredSpawnOptions(skipped.session, { + options: new Map([['model', 'sonnet[1m]']]), + skipped: ['permissionMode'], + fastModeAtStart: false + }) expect(skipped.respond('turn-a', 1_000)).toBeNull() }) diff --git a/src/main/claude/claude-structured-options.test.ts b/src/main/claude/claude-structured-options.test.ts index e5b7a7ea8a7..2843d4f7ddd 100644 --- a/src/main/claude/claude-structured-options.test.ts +++ b/src/main/claude/claude-structured-options.test.ts @@ -1,19 +1,12 @@ import { describe, expect, it, vi } from 'vitest' -import { - restoreClaudeStructuredSessionOptions, - setClaudeStructuredOption -} from './claude-structured-options' +import { setClaudeStructuredOption } from './claude-structured-options' import type { ClaudeSession } from './claude-structured-session-state' -import { - ClaudeControlRequestError, - ClaudeControlRequestTimeoutError -} from './claude-agent-sdk-control-requests' +import { ClaudeControlRequestTimeoutError } from './claude-agent-sdk-control-requests' import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' import { ClaudeChildWorkDecoder } from './claude-child-work-decoder' import { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog' import { createClaudeSessionStartup } from './claude-structured-session-startup-state' import { - claudeStructuredSessionOptionsFrom, observeClaudeFastModeFacts, readClaudeStructuredSessionOptions } from './claude-structured-session-options' @@ -45,6 +38,8 @@ function sessionFor(setModel: ClaudeSession['connection']['setModel']): ClaudeSe reportedModelMutation: 0, confirmedOptions: new Set(), restoreSkippedOptions: new Set(), + launchedModel: null, + fastModeAtStart: false, capabilities: [], events: undefined, translator: null, @@ -145,8 +140,7 @@ describe('Claude structured Fast mode', () => { } ) - // Turning Fast off needs no support evidence, so it must not pay a catalog round - // trip — restore replays a stored `false` on every acquire. + // Turning Fast off needs no support evidence, so it must not pay a catalog round trip. it('reads no catalog to turn Fast off, but does to turn it on', async () => { const { session } = fastModeSession(true) const listed = session.connection.supportedModels @@ -167,17 +161,6 @@ describe('Claude structured Fast mode', () => { expect(reads).toBe(1) }) - it('restores explicit Fast off when model support is unknown', async () => { - const { session, applyFlagSettings } = fastModeSession(undefined) - session.options.set('fastMode', 'false') - - await restoreClaudeStructuredSessionOptions(session, undefined) - - expect(session.options.get('fastMode')).toBe('false') - expect(session.restoreSkippedOptions.has('fastMode')).toBe(false) - expect(applyFlagSettings).toHaveBeenCalledWith({ fastMode: false }, { timeoutMs: undefined }) - }) - it('resolves the running CLI default model before applying Fast', async () => { const { session, applyFlagSettings } = fastModeSession(true) session.options.delete('model') @@ -439,37 +422,7 @@ describe('Claude Fast mode reported by the session frame alone', () => { }) }) -describe('Claude structured option restore under the request deadline', () => { - it('keeps a saved choice the CLI never answered as wanted but unconfirmed, and drops a refused one', async () => { - const session = sessionFor(async () => { - throw new ClaudeControlRequestTimeoutError('set_model') - }) - session.connection.applyFlagSettings = async () => { - throw new ClaudeControlRequestTimeoutError('apply_flag_settings') - } - session.connection.setPermissionMode = async () => { - throw new ClaudeControlRequestError('set_permission_mode', 'unknown mode') - } - // Startup already read the CLI's own model and effort, and vouched for them. - session.reportedOptions = { model: 'claude-sonnet-5', effort: 'medium' } - session.confirmedOptions.add('effort') - session.options = new Map([ - ['model', 'sonnet'], - ['effort', 'high'], - ['permissionMode', 'plan'] - ]) - vi.spyOn(console, 'warn').mockImplementation(() => {}) - - await expect(restoreClaudeStructuredSessionOptions(session, 10)).resolves.toBeUndefined() - - expect(Object.fromEntries(session.options)).toEqual({ model: 'sonnet', effort: 'high' }) - expect([...session.restoreSkippedOptions]).toEqual(['permissionMode']) - expect(claudeStructuredSessionOptionsFrom(session, null).current).toEqual({ - model: 'sonnet', - effort: 'high' - }) - }) - +describe('Claude structured option write under the request deadline', () => { it("keeps a timed-out client write as the deadline's own error, not a rejection", async () => { const session = sessionFor(async () => { throw new ClaudeControlRequestTimeoutError('set_model') diff --git a/src/main/claude/claude-structured-options.ts b/src/main/claude/claude-structured-options.ts index 2f0b7ac4b70..15f33ea1b01 100644 --- a/src/main/claude/claude-structured-options.ts +++ b/src/main/claude/claude-structured-options.ts @@ -1,10 +1,6 @@ import type { EffortLevel, PermissionMode } from '@anthropic-ai/claude-agent-sdk' import { ClaudeControlRequestError } from './claude-stream-json-connection' -import { ClaudeControlRequestTimeoutError } from './claude-agent-sdk-control-requests' -import { - AgentSessionOptionRejectedError, - isAgentSessionOptionRejectedError -} from '../native-chat/agent-session-wire/structured-agent-session-option-error' +import { AgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error' import { claudeCatalogAdmitsModel, claudeModelEffortLevels, @@ -48,7 +44,7 @@ export function isClaudeStructuredOptionKey(key: string): boolean { return CLAUDE_STRUCTURED_OPTION_KEYS.has(key) } -/** A client's write; the startup restore writes through `setClaudeStructuredOption` directly. */ +/** A client's write to a live session. */ export function setClaudeStructuredSessionOption( session: ClaudeSession, input: { key: string; value: string }, @@ -69,9 +65,7 @@ export function setClaudeStructuredSessionOption( export async function setClaudeStructuredOption( session: ClaudeSession, input: { key: string; value: string }, - timeoutMs: number | undefined, - /** What the key held before a restore cleared the map; a live write reads the map. */ - heldBeforeRestore?: string + timeoutMs: number | undefined ): Promise>> { const fastMode = input.key === 'fastMode' @@ -99,17 +93,15 @@ export async function setClaudeStructuredOption( // One read answers every catalog question this write asks, so the guards below // cannot each pay a round trip for the same list nor disagree about the model. // Two writes ask nothing of it and so read nothing: an effort write with no current - // model has nothing to look up, and turning Fast off needs no support evidence — - // which is every restore replaying a stored `false`. + // model has nothing to look up, and turning Fast off needs no support evidence. const needsCatalog = input.key === 'model' || (input.key === 'fastMode' && fastMode === true) || (input.key === 'effort' && readClaudeCurrentModel(session).id !== undefined) const listed = needsCatalog ? await readClaudeListedModels(session, timeoutMs) : [] // The child stores an effort its model has no control for and keeps it across - // every later model switch and restore, so refuse before the write rather than - // read the acceptance back as adoption. Refused here, restore drops the stale - // value instead of replaying it onto a model that cannot use it. + // every later model switch, so refuse before the write rather than read the + // acceptance back as adoption. if (input.key === 'effort') { const { modelId, levels } = claudeModelEffortLevels(session, listed) if (levels && !levels.has(input.value)) { @@ -144,8 +136,7 @@ export async function setClaudeStructuredOption( } // set_model resolves for a model the provider never lists and the session then // fails every turn with zero tokens, so the acceptance proves nothing and only - // the catalog does. Restore replays a pick the provider may since have retired, - // which reaches here with no user error at all. + // the catalog does. if (input.key === 'model' && !claudeCatalogAdmitsModel(listed, input.value)) { throw new AgentSessionOptionRejectedError(`claude does not list a model named ${input.value}`) } @@ -156,7 +147,7 @@ export async function setClaudeStructuredOption( const modelWasConfirmed = readClaudeCurrentModel(session).confirmed const mutationSequence = ++session.optionMutationSequence // Read with the fence bump, so a write that lands after an earlier one's bookkeeping compares against it. - const held = heldBeforeRestore ?? session.options.get(input.key) + const held = session.options.get(input.key) // Only a model write can stale the model report — an effort or permission-mode // write does not change what the child is running. Leaving the stamp behind // would drop the session back to the written model and refuse, on the next @@ -247,44 +238,3 @@ export async function setClaudeStructuredOption( } return Object.fromEntries(session.options) } - -export async function restoreClaudeStructuredSessionOptions( - session: ClaudeSession, - timeoutMs: number | undefined -): Promise { - // Any write that was already in flight belongs to the previous acquisition - // state and must not repopulate this map after restore starts. - session.optionMutationSequence += 1 - // The fence bump is not a write, so the report the session already holds is still - // current as of this instant; leaving the stamp behind would make every restored - // session read as unconfirmed until its next turn. - session.reportedModelMutation = session.optionMutationSequence - const options = [...session.options.entries()] - session.options.clear() - for (const [key, value] of options) { - try { - await setClaudeStructuredOption(session, { key, value }, timeoutMs, value) - } catch (error) { - // A write the CLI never answered must not fault a start that is otherwise fine. Silence is - // not a refusal, so the choice stays wanted, unconfirmed, and the next start retries it. - if (error instanceof ClaudeControlRequestTimeoutError) { - console.warn( - `[claude-structured] restore of ${key} for ${session.providerSessionId} was not answered in time; keeping it unconfirmed` - ) - session.options.set(key, value) - session.confirmedOptions.delete(key) - continue - } - if (!isAgentSessionOptionRejectedError(error)) { - throw error - } - // A stale or unavailable preference must not poison every future acquire; - // the provider's current value remains authoritative and is re-persisted. - session.restoreSkippedOptions.add(key) - // The journal's window was measured under the value this child did not take. - if (CONTEXT_WINDOW_KEYS.has(key)) { - session.translator?.modelMayHaveChanged() - } - } - } -} diff --git a/src/main/claude/claude-structured-prompt-ownership.ts b/src/main/claude/claude-structured-prompt-ownership.ts index 07e3960c6ed..a7d1c41c8bf 100644 --- a/src/main/claude/claude-structured-prompt-ownership.ts +++ b/src/main/claude/claude-structured-prompt-ownership.ts @@ -63,7 +63,8 @@ export async function cancelClaudeStructuredTurn(input: { const timeoutMs = Math.min(input.timeoutMs ?? CLAUDE_STOP_GRACE_MS, CLAUDE_STOP_GRACE_MS) const session = requireSession(sessions, request.sessionId) const acquisitionGeneration = session.acquisitionGeneration - // Before startup lands nothing was written, so there is nothing to interrupt. + // A CLI still starting answers no interrupt: Stop ends its child next (`stopEndsSession`), and + // that end settles every message it was handed and never echoed as stopped. if (request.prompt || session.startup.state === 'pending') { return { cancelled: false } } diff --git a/src/main/claude/claude-structured-real-cli-fold.test.ts b/src/main/claude/claude-structured-real-cli-fold.test.ts index 13e4ee5e672..8abcc0de2c9 100644 --- a/src/main/claude/claude-structured-real-cli-fold.test.ts +++ b/src/main/claude/claude-structured-real-cli-fold.test.ts @@ -27,6 +27,7 @@ import { realClaudeCommand, realClaudeLaunchHome } from './claude-real-cli-availability-test-support' +import { claudeStartupSettled } from './claude-structured-session-test-support' import { claudeProviderHandle } from '../../shared/agent-session-provider-handle-encoding' const SESSION_ID = 'real-cli-fold' @@ -108,8 +109,8 @@ describe.skipIf(!realClaudeAvailable)('Claude structured real CLI fold', () => { spawnToken: 'real-cli-fold', events: sink }) - await adapter.awaitStarted(SESSION_ID) - // Startup proved from the SessionStart hook frame, with no init yet. + await claudeStartupSettled(adapter, SESSION_ID) + // Startup landed on the initialize answer, with no turn and so no init yet. expect( events.some( (event) => diff --git a/src/main/claude/claude-structured-real-cli.test.ts b/src/main/claude/claude-structured-real-cli.test.ts index b12ca6ff1e0..0e9345537bb 100644 --- a/src/main/claude/claude-structured-real-cli.test.ts +++ b/src/main/claude/claude-structured-real-cli.test.ts @@ -8,6 +8,7 @@ import * as claudeConnection from './claude-stream-json-connection' import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' import { resolveSessionFilePath } from '../native-chat/session-file-resolver' import { CLAUDE_STRUCTURED_BASE_OPTIONS } from './claude-structured-launch-resolution' +import { readClaudeInit } from './claude-structured-init-proof' import { realClaudeAuthenticated, realClaudeAuthStatus, @@ -21,6 +22,7 @@ import { type ClaudeStructuredSessionEvent } from './claude-structured-session-adapter' import type { ClaudeStructuredSessionAdapterDeps } from './claude-structured-session-state' +import { claudeStartupSettled } from './claude-structured-session-test-support' import { claudeProviderHandle } from '../../shared/agent-session-provider-handle-encoding' const command = realClaudeCommand @@ -32,7 +34,8 @@ function realAdapter( events: ClaudeStructuredSessionEvent[] = [], cwd = process.cwd(), onDispatchSettledLate?: ClaudeStructuredSessionAdapterDeps['onDispatchSettledLate'], - env = realClaudeLaunchHome().env + env = realClaudeLaunchHome().env, + waitsForStartup = true ): ClaudeStructuredSessionAdapter { const adapter = new ClaudeStructuredSessionAdapter({ resolveLaunch: async () => ({ @@ -51,11 +54,14 @@ function realAdapter( readProcessStartTime: async () => 1, now: () => 2 }) + if (!waitsForStartup) { + return adapter + } // These proofs read startup facts, which land after the session is published. const acquire = adapter.acquire adapter.acquire = async (input) => { const acquisition = await acquire(input) - await adapter.awaitStarted(input.identity.sessionId) + await claudeStartupSettled(adapter, input.identity.sessionId) return acquisition } return adapter @@ -332,6 +338,69 @@ describe.skipIf(!realClaudeAvailable)(suiteTitle, () => { 90_000 ) + // The contract a chat's first message depends on: saved options ride the launch, and the + // message is written as soon as the child is published, before the CLI answers initialize. + it.skipIf(!realClaudeAuthenticated)( + 'runs a message written before initialize answers, under the saved options it was launched with', + async () => { + const providerSessionId = randomUUID() + const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = realAdapter( + providerSessionId, + claudeConfigDir, + events, + process.cwd(), + undefined, + realClaudeLaunchHome().env, + false + ) + const messages = (): Record[] => + events.flatMap((event) => (event.type === 'message' ? [event.message] : [])) + + try { + await adapter.acquire({ + identity: identity(providerSessionId), + fence: 1, + spawnToken: 'real-cli-saved-options', + options: { model: 'sonnet', permissionMode: 'plan', effort: 'low' } + }) + const dispatched = await adapter.dispatch({ + sessionId: 'real-cli-handshake', + clientMessageId: 'real-cli-saved-options-1', + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'Reply with exactly: OK. Do not use any tools.' }] + }, + fence: 1 + }) + // Read as the write resolves: the CLI had not answered initialize, so nothing held it. + const answeredAtDispatch = adapter['sessions'].get('real-cli-handshake')?.startup.answered + const startedAtDispatch = events.some((event) => event.type === 'started') + expect(dispatched).toEqual({ state: 'admitted' }) + expect(answeredAtDispatch).toBe(false) + expect(startedAtDispatch).toBe(false) + const deadline = Date.now() + 90_000 + while (!messages().some((m) => m.type === 'result') && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 250)) + } + + // The echo of the message Orca wrote, then a turn that ended in success. + expect(messages()).toContainEqual(expect.objectContaining({ type: 'user', isReplay: true })) + expect(messages().find((m) => m.type === 'result')).toMatchObject({ is_error: false }) + // The turn's own init names what the child was launched with. + expect(messages().find((m) => m.type === 'system' && m.subtype === 'init')).toMatchObject({ + model: 'claude-sonnet-5', + permissionMode: 'plan' + }) + } finally { + await adapter.closeAll() + } + }, + 120_000 + ) + // The window a Stop naming no turn exists for: Orca has written the message, and Claude has not // started its reply, so no turn id exists. Only the live binary can say the interrupt lands there. it.skipIf(!realClaudeAuthenticated)( @@ -487,6 +556,54 @@ describe.skipIf(!realClaudeAvailable)(suiteTitle, () => { 150_000 ) + // With no SessionStart hook (Orca's status hooks off), the CLI names no session before the + // first turn; the chat must start on the initialize answer and take the message. + it.skipIf(!realClaudeAuthenticated)( + 'starts and answers with every hook disabled, so no start frame precedes the turn', + async () => { + const providerSessionId = randomUUID() + const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') + const cwd = await mkdtemp(join(tmpdir(), 'orca-no-hooks-')) + await mkdir(join(cwd, '.claude'), { recursive: true }) + await writeFile( + join(cwd, '.claude', 'settings.json'), + JSON.stringify({ disableAllHooks: true }) + ) + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = realAdapter(providerSessionId, claudeConfigDir, events, cwd) + const frames = (): Record[] => + events.flatMap((event) => (event.type === 'message' ? [event.message] : [])) + try { + await adapter.acquire({ + identity: identity(providerSessionId), + fence: 1, + spawnToken: 'real-cli-no-hooks' + }) + expect(frames().filter((frame) => readClaudeInit(frame) !== null)).toEqual([]) + await expect( + adapter.dispatch({ + sessionId: 'real-cli-handshake', + clientMessageId: 'real-cli-no-hooks-1', + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'Reply with the single word ok.' }] + }, + fence: 1 + }) + ).resolves.toEqual({ state: 'admitted' }) + await vi.waitFor( + () => expect(frames()).toContainEqual(expect.objectContaining({ type: 'result' })), + { timeout: 120_000, interval: 200 } + ) + } finally { + await adapter.closeAll() + await rm(cwd, { recursive: true, force: true }) + } + }, + 150_000 + ) + it('turns a real silent unauthenticated startup into sign-in guidance', async () => { const claudeConfigDir = await mkdtemp(join(tmpdir(), 'orca-claude-no-auth-')) const providerSessionId = randomUUID() diff --git a/src/main/claude/claude-structured-retired-model.test.ts b/src/main/claude/claude-structured-retired-model.test.ts new file mode 100644 index 00000000000..86397cc59f6 --- /dev/null +++ b/src/main/claude/claude-structured-retired-model.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, it } from 'vitest' +import { claudeAcquireCatalogAccess } from './claude-structured-acquire-catalog' +import { AgentModelCatalogStore } from '../native-chat/agent-model-catalog/agent-model-catalog-store' +import type { ClaudeStructuredSessionEvent } from './claude-structured-session-state' +import { ClaudeStructuredSessionAdapter } from './claude-structured-session-adapter' +import { + claudeCatalogRowsOfAccount, + retireClaudeLaunchedModel +} from './claude-structured-retired-model' +import { + adapterFor, + claudeFrame, + claudeStartupSettled, + fakeClaude, + tick, + identityFor, + PROVIDER_SESSION_ID, + recordingJournalSink +} from './claude-structured-session-test-support' + +const ACCOUNT_HOME = '/accounts/claude' + +function catalogAccess(store: AgentModelCatalogStore) { + const access = claudeAcquireCatalogAccess(store, ACCOUNT_HOME) + if (!access) { + throw new Error('a store and an account home always give access') + } + return access +} +const RETIRED = 'claude-retired-1' +/** The rows Claude Code 2.1.280 lists for a child launched with `--model claude-bogus-9`: its own + * native rows, then one for the launched id, named by that id. */ +const NATIVE_OPUS = { value: 'opus', resolvedModel: 'claude-opus-5', displayName: 'Opus' } +const LAUNCHED_ROW = { + value: RETIRED, + resolvedModel: RETIRED, + displayName: RETIRED, + description: 'Custom model' +} + +function modelNotFound(parentToolUseId: string | null = null): Record { + return { + type: 'assistant', + uuid: 'reply-1', + session_id: PROVIDER_SESSION_ID, + parent_tool_use_id: parentToolUseId, + error: 'model_not_found', + is_api_error_message: true, + message: { model: '', role: 'assistant', content: [] } + } +} + +function sessionLaunchedWith(model: string | null, picked: string | null = model) { + return { + launchedModel: model, + options: new Map(picked === null ? [] : [['model', picked]]), + restoreSkippedOptions: new Set() + } +} + +describe("the CLI's word that a launched model does not exist", () => { + it('drops the launched model once, as a skipped saved option', () => { + const session = sessionLaunchedWith(RETIRED) + + expect(retireClaudeLaunchedModel(session, modelNotFound())).toBe(RETIRED) + expect(session.options.has('model')).toBe(false) + expect([...session.restoreSkippedOptions]).toEqual(['model']) + expect(retireClaudeLaunchedModel(session, modelNotFound())).toBeNull() + }) + + it.each([ + ['a subagent reply', sessionLaunchedWith(RETIRED), modelNotFound('toolu_1')], + ['another error', sessionLaunchedWith(RETIRED), { ...modelNotFound(), error: 'rate_limit' }], + ['a model picked since the launch', sessionLaunchedWith(RETIRED, 'opus'), modelNotFound()], + ['a launch that named no model', sessionLaunchedWith(null), modelNotFound()] + ])('keeps the saved model for %s', (_case, session, message) => { + expect(retireClaudeLaunchedModel(session, message)).toBeNull() + expect([...session.restoreSkippedOptions]).toEqual([]) + }) +}) + +describe("the account catalog a child's listing writes through", () => { + const rows = [ + { id: 'opus', label: 'Opus' }, + { id: RETIRED, label: RETIRED } + ] + + function access(cached: string[] | null) { + const store = new AgentModelCatalogStore() + const handle = catalogAccess(store) + if (cached) { + store.recordSuccess(handle.fingerprint, 'claude', { + models: cached.map((id) => ({ id, label: id, isDefault: false, efforts: [] })), + fastModeTierByModel: new Map(), + origin: 'probe' + }) + } + return handle + } + + it('leaves out the row the launch added for its own model', () => { + expect(claudeCatalogRowsOfAccount(access(null), rows, RETIRED)).toEqual([rows[0]]) + }) + + it('keeps that row when the account already listed the model', () => { + expect(claudeCatalogRowsOfAccount(access([RETIRED]), rows, RETIRED)).toEqual(rows) + }) + + it('keeps a launched native model, which carries its own display name', () => { + expect(claudeCatalogRowsOfAccount(access(null), rows, 'opus')).toEqual(rows) + }) + + it('keeps every row for a launch that named no model', () => { + expect(claudeCatalogRowsOfAccount(access(null), rows, null)).toEqual(rows) + }) + + it('never writes a launched-only model into the account catalog at start', async () => { + const store = new AgentModelCatalogStore() + const handle = catalogAccess(store) + const claude = fakeClaude({ initModels: [NATIVE_OPUS, LAUNCHED_ROW] }) + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = new ClaudeStructuredSessionAdapter({ + resolveLaunch: async () => ({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: '/work/repo', + claudeConfigDir: ACCOUNT_HOME, + providerSessionId: PROVIDER_SESSION_ID, + resumeLeafUuid: null, + resumesTranscript: true, + continuesChain: true + }), + onEvent: (event) => events.push(event), + openConnection: claude.openConnection, + readProcessStartTime: async () => 1_700_000_000_000, + now: () => 1_700_000_000_500, + persistHandle: async () => {}, + modelCatalog: store + }) + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + events: recordingJournalSink(), + options: { model: RETIRED } + }) + await claudeStartupSettled(adapter, 'session-1') + + expect(events.some((event) => event.type === 'started')).toBe(true) + expect(store.get(handle.fingerprint)?.models.map((model) => model.id)).toEqual(['opus']) + await adapter.closeAll() + }) +}) + +describe('a live child still launched with a retired model', () => { + async function healed(holdReset: boolean) { + let answerReset: () => void = () => {} + const claude = fakeClaude({ + initModels: [NATIVE_OPUS, LAUNCHED_ROW], + routes: { + list_models: () => [NATIVE_OPUS], + set_model: (params) => + holdReset && params?.model === undefined + ? new Promise((resolve) => (answerReset = resolve)) + : undefined + } + }) + const adapter = adapterFor(claude) + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + options: { model: RETIRED } + }) + claudeFrame(claude.connections[0]!, modelNotFound()) + return { claude, adapter, answerReset: () => answerReset() } + } + + function setModelCalls(claude: ReturnType) { + return claude.connections[0]!.calls.filter((call) => call.subtype === 'set_model') + } + + it("goes back to the CLI's own default, so the next turn runs", async () => { + const { claude, adapter } = await healed(false) + await tick() + + expect(setModelCalls(claude)).toEqual([{ subtype: 'set_model', params: { model: undefined } }]) + const options = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current.model).not.toBe(RETIRED) + await adapter.closeAll() + }) + + it('keeps a model the user picks before the reset lands', async () => { + const { claude, adapter, answerReset } = await healed(true) + await adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'opus', fence: 7 }) + answerReset() + await tick() + + // The pick goes out after the reset, so the CLI runs it; the reset's bookkeeping stands aside. + expect(setModelCalls(claude).map((call) => call.params?.model)).toEqual([undefined, 'opus']) + const options = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current.model).toBe('opus') + await adapter.closeAll() + }) +}) diff --git a/src/main/claude/claude-structured-retired-model.ts b/src/main/claude/claude-structured-retired-model.ts new file mode 100644 index 00000000000..5c25ad46181 --- /dev/null +++ b/src/main/claude/claude-structured-retired-model.ts @@ -0,0 +1,90 @@ +// A chat's saved model is passed to the CLI as `--model` unchecked, so one the provider has since +// retired fails the turn. The CLI says so itself: a synthetic reply whose `error` is +// `model_not_found` (Claude Code 2.1.280). That reply is the evidence the saved model cannot run. + +import type { AgentModelCatalogSessionAccess } from '../native-chat/agent-model-catalog/agent-model-catalog-store' +import type { + ClaudeSession, + ClaudeStructuredSessionAdapterDeps +} from './claude-structured-session-state' + +/** The launched model, once, when a root reply says it does not exist and the chat still has it + * picked: the session stops holding it, so the record drops it. Null otherwise. */ +export function retireClaudeLaunchedModel( + session: Pick, + message: Record +): string | null { + const launched = session.launchedModel + if ( + launched === null || + message.type !== 'assistant' || + (message.parent_tool_use_id ?? null) !== null || + message.error !== 'model_not_found' || + session.options.get('model') !== launched + ) { + return null + } + session.options.delete('model') + session.restoreSkippedOptions.add('model') + return launched +} + +/** Moves the live child to the CLI's account default (`set_model` with no model), which unlike a + * fresh launch ignores ANTHROPIC_MODEL and a settings `model`. A user's pick made meanwhile owns + * the model; a refused or unanswered reset changes nothing but the log. */ +export function resetClaudeRetiredModel( + session: ClaudeSession, + deps: Pick, + sessionId: string, + retired: string +): void { + const sequence = session.optionMutationSequence + void session.connection.setModel(undefined, { timeoutMs: deps.requestTimeoutMs }).then( + () => { + if (sequence !== session.optionMutationSequence) { + return + } + // The retired id is no longer what runs; the next turn's own report names the default. + if (session.reportedOptions.model === retired) { + delete session.reportedOptions.model + } + if (session.appliedOptions?.model === retired) { + const { model: _retired, ...applied } = session.appliedOptions + session.appliedOptions = applied + } + }, + (error: unknown) => + deps.logger?.warn('putting a retired Claude model back on the default failed', { + scope: 'claude-retired-model-reset', + sessionId, + error + }) + ) +} + +/** The saved values this child showed it cannot run. Only a retired launch model is one: a launch + * never skips a model, so a skipped model is that. */ +export function claudeRetiredOptions( + session: Pick +): { retiredOptions?: Record } { + return session.restoreSkippedOptions.has('model') && session.launchedModel !== null + ? { retiredOptions: { model: session.launchedModel } } + : {} +} + +/** The rows of a listing the account's catalog keeps. A child launched with `--model X` lists X + * itself (Claude Code 2.1.280 adds a row named by the raw id, "Custom model"), whether or not X + * exists: that row is the launch talking, not the account, unless the account already listed X. + * Native rows carry a display name of their own, so a row named by its id is the tell. */ +export function claudeCatalogRowsOfAccount( + access: AgentModelCatalogSessionAccess, + rows: readonly T[], + launchedModel: string | null +): T[] { + const listedBefore = access.store + .get(access.fingerprint) + ?.models.some((model) => model.id === launchedModel) + return rows.filter( + (row) => listedBefore === true || row.id !== launchedModel || row.label !== row.id + ) +} diff --git a/src/main/claude/claude-structured-session-acquisition-options.ts b/src/main/claude/claude-structured-session-acquisition-options.ts index 7557419dd08..fff9cb4dfc3 100644 --- a/src/main/claude/claude-structured-session-acquisition-options.ts +++ b/src/main/claude/claude-structured-session-acquisition-options.ts @@ -3,7 +3,6 @@ import { readClaudeSettingsFastMode, readClaudeSettingsFastModePerSessionOptIn } from './claude-structured-session-options' -import { restoredClaudeStructuredSessionOptions } from './claude-structured-options' import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' export async function readClaudeStructuredSessionSettings( @@ -16,22 +15,12 @@ export async function readClaudeStructuredSessionSettings( export function prepareClaudeStructuredSessionAcquisitionOptions(args: { settings: unknown initialization: unknown - inputOptions: Readonly> | undefined - /** A fresh CLI session has not carried a per-session Fast opt-in over from anywhere. */ - resumesTranscript: boolean }) { - const fastMode = readClaudeSettingsFastMode(args.settings) - const fastModePerSessionOptIn = readClaudeSettingsFastModePerSessionOptIn(args.settings) - const fastModeFacts = readClaudeFastModeFacts(args.initialization) - const options = restoredClaudeStructuredSessionOptions(args.inputOptions) - if ( - !args.resumesTranscript && - fastModePerSessionOptIn === true && - options.get('fastMode') === 'true' - ) { - options.delete('fastMode') + return { + fastMode: readClaudeSettingsFastMode(args.settings), + fastModePerSessionOptIn: readClaudeSettingsFastModePerSessionOptIn(args.settings), + fastModeFacts: readClaudeFastModeFacts(args.initialization) } - return { fastMode, fastModePerSessionOptIn, fastModeFacts, options } } export function claudeStructuredSessionPublicationOptions(input: { diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 285606fb8bf..0e10d929fa0 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -18,7 +18,7 @@ import { import { claudeConfigDirEnvPatch } from './claude-config-dir-pin' import { CLAUDE_SPAWN_TOKEN_ENV, claudeProcessIdentity } from './claude-structured-owner-identity' import { ClaudePromptRegistry } from './claude-structured-prompt-replies' -import { restoredClaudeStructuredSessionOptions } from './claude-structured-options' +import { adoptClaudeStructuredSpawnOptions } from './claude-structured-spawn-options' import { createClaudeSessionJournalTranslator } from './claude-structured-journal-translation' import { observeClaudeFastModeFacts } from './claude-structured-session-options' import { @@ -91,10 +91,10 @@ export async function acquireClaudeSession({ const onMessage = (message: Record): void => { const init = readClaudeInit(message) if (readClaudeFrameString(message, 'session_id') !== expectedProviderSessionId) { - // An init proof for another (or unnamed) provider must fail acquisition + // An init proof for another (or unnamed) provider must fail the start or end the session // promptly, while ordinary foreign frames stay quarantined silently. if (init || (message.type === 'system' && message.subtype === 'init')) { - initProof.reject(new Error('claude provider session expected')) + initProof.refuse() } return } @@ -251,12 +251,13 @@ export async function acquireClaudeSession({ ...(unbindReadingControl ? { unbindReadingControl } : {}), process, acquisitionGeneration: mintClaudeAcquisitionGeneration(deps), - options: restoredClaudeStructuredSessionOptions(input.options), + options: launch.savedOptions.options, ...(deps.mintLinkId ? { linkId: deps.mintLinkId() } : {}), observedAt: deps.now?.() ?? Date.now() }) const session = publication.session liveSession = session + adoptClaudeStructuredSpawnOptions(session, launch.savedOptions) const catalogAccess = claudeAcquireCatalogAccess(deps.modelCatalog, launch.claudeConfigDir) if (catalogAccess) { session.catalogAccess = catalogAccess @@ -279,21 +280,19 @@ export async function acquireClaudeSession({ initProof, sessionId, providerSessionId: launch.providerSessionId, + startup: session.startup, resumesTranscript: launch.resumesTranscript, - inputOptions: input.options, requestTimeoutMs: deps.requestTimeoutMs, emit }), isCurrent: () => sessions.get(sessionId) === session, - requestTimeoutMs: deps.requestTimeoutMs, fault: (error) => callbacks.handleExit(sessionId, attempt, error), - onStarted: (options) => + report: (event) => emit({ - type: 'started', + ...event, sessionId, fence: input.fence, - acquisitionGeneration: session.acquisitionGeneration, - ...options + acquisitionGeneration: session.acquisitionGeneration }) }) ]) @@ -304,8 +303,8 @@ export async function acquireClaudeSession({ exits.get(sessionId)?.error ?? new Error('claude session ended before acquisition returned') ) } - // The start applies its facts and restores saved options only after publish, so the child - // is `starting` until `started` says otherwise. + // The start reads its facts only after publish, so the child is `starting` until `started` + // says otherwise; it already takes input. return { ...publication.acquisition, providerChildPhase: 'starting' } } catch (error) { unbindReadingControl?.() diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index cd5b4841de6..cc59dfde301 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -7,8 +7,11 @@ import { AgentSessionPromptAnswerRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' -import { ClaudeControlRequestError } from './claude-stream-json-connection' import { CLAUDE_SPAWN_TOKEN_ENV } from './claude-structured-owner-identity' +import { + ClaudeControlRequestError, + ClaudeControlRequestTimeoutError +} from './claude-agent-sdk-control-requests' import type { ClaudeStructuredSessionAdapter, ClaudeStructuredSessionEvent @@ -67,7 +70,7 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { expect(events[0]).toMatchObject({ type: 'message', message: { subtype: 'hook_started' } }) }) - it('restores persisted model and effort before publishing a reacquired session', async () => { + it('launches a reacquired session with its persisted model and effort, and writes neither', async () => { const claude = fakeClaude() const adapter = adapterFor(claude, { resumesTranscript: true, continuesChain: true }) @@ -78,69 +81,19 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { options: { model: 'opus', effort: 'high' } }) - expect(claude.connections[0].calls.slice(-4)).toEqual([ - { subtype: 'set_model', params: { model: 'opus' } }, - // The restored model's advertised levels gate the replay, so a stale effort - // is dropped rather than re-applied to a model with no effort control. - { subtype: 'list_models' }, - { subtype: 'apply_flag_settings', params: { settings: { effortLevel: 'high' } } }, - // The effort is only recorded once the child reports having adopted it. - { subtype: 'get_settings' } + expect(claude.connections[0].launch.options).toMatchObject({ model: 'opus', effort: 'high' }) + expect(claude.connections[0].calls.map((call) => call.subtype)).toEqual([ + 'initialize', + 'get_settings' ]) await expect(adapter.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toMatchObject({ current: { model: 'opus', effort: 'high' } }) }) - it('restores an encoded Fast preference through the absolute flag setting', async () => { + it('launches a resumed conversation with its saved Fast as a flag setting', async () => { const claude = fakeClaude({ - settings: { effective: { fastMode: false, fastModePerSessionOptIn: false } }, - routes: { - list_models: () => [{ value: 'opus', displayName: 'Opus', supportsFastMode: true }] - } - }) - const adapter = adapterFor(claude) - - await adapter.acquire({ - identity: identityFor(), - fence: 7, - spawnToken: 'spawn-9', - options: { model: 'opus', fastMode: 'true' } - }) - - expect(claude.connections[0].calls).toContainEqual({ - subtype: 'apply_flag_settings', - params: { settings: { fastMode: true } } - }) - }) - - it('does not carry a saved opt-in into a new per-session-opt-in child', async () => { - const claude = fakeClaude({ - settings: { effective: { fastMode: false, fastModePerSessionOptIn: true } }, - routes: { - list_models: () => [{ value: 'opus', displayName: 'Opus', supportsFastMode: true }] - } - }) - const adapter = adapterFor(claude) - - await adapter.acquire({ - identity: identityFor(), - fence: 7, - spawnToken: 'spawn-9', - options: { model: 'opus', fastMode: 'true' } - }) - - expect( - claude.connections[0].calls.filter((call) => call.subtype === 'apply_flag_settings') - ).toEqual([]) - }) - - it('restores Fast when reacquiring the same per-session-opt-in conversation', async () => { - const claude = fakeClaude({ - settings: { effective: { fastMode: false, fastModePerSessionOptIn: true } }, - routes: { - list_models: () => [{ value: 'opus', displayName: 'Opus', supportsFastMode: true }] - } + settings: { effective: { fastMode: true, fastModePerSessionOptIn: false } } }) const adapter = adapterFor(claude, { resumesTranscript: true, continuesChain: true }) @@ -151,85 +104,149 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { options: { model: 'opus', fastMode: 'true' } }) - expect(claude.connections[0].calls).toContainEqual({ - subtype: 'apply_flag_settings', - params: { settings: { fastMode: true } } - }) - }) - - it('self-heals a Fast preference the running model no longer supports', async () => { - const claude = fakeClaude({ - settings: { effective: { fastMode: false } }, - routes: { - list_models: () => [{ value: 'opus', displayName: 'Opus', supportsFastMode: false }] - } - }) - const adapter = adapterFor(claude) - - await expect( - adapter.acquire({ - identity: identityFor(), - fence: 7, - spawnToken: 'spawn-9', - options: { model: 'opus', fastMode: 'true' } - }) - ).resolves.toBeDefined() - - expect(adapter.readOptionRestoreFailures('session-1')).toContain('fastMode') + expect(claude.connections[0].launch.options.settings).toEqual({ fastMode: true }) expect( claude.connections[0].calls.filter((call) => call.subtype === 'apply_flag_settings') ).toEqual([]) }) - it.each([ - ['model', 'set_model', { model: 'retired-model' }], - ['effort', 'apply_flag_settings', { effort: 'retired-effort' }], - ['permissionMode', 'set_permission_mode', { permissionMode: 'retired-mode' }] - ] as const)( - 'self-heals a persisted %s rejected during restore', - async (key, subtype, options) => { - const claude = fakeClaude({ - routes: { - [subtype]: () => { - throw new ClaudeControlRequestError(subtype, 'value is no longer available') - } - } - }) - const adapter = adapterFor(claude) - - await expect( - adapter.acquire({ - identity: identityFor(), - fence: 7, - spawnToken: 'spawn-9', - options - }) - ).resolves.toBeDefined() - expect(adapter.readOptionRestoreFailures('session-1')).toEqual([key]) - } - ) - - it('does not treat a transport timeout while restoring an option as recoverable', async () => { + it('does not carry a saved Fast into a new conversation whose settings opt in per session', async () => { const claude = fakeClaude({ - routes: { - set_model: () => { - throw new Error('claude set_model request timed out') - } - } + settings: { effective: { fastMode: false, fastModePerSessionOptIn: true } } }) - const input = { + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9', - options: { model: 'temporarily-unavailable' } - } - - // Restore runs after publish, so its failure ends the session rather than the create. - await expect(endedAtStartup(claude, input)).resolves.toMatchObject({ - reason: 'claude set_model request timed out', - startupUnproven: true + options: { model: 'opus', fastMode: 'true' } }) - expect(claude.connections[0]?.closeCount).toBe(1) + + expect(claude.connections[0].launch.options.settings).toBeUndefined() + expect( + claude.connections[0].calls.filter((call) => call.subtype === 'apply_flag_settings') + ).toEqual([]) + // Left out, not refused: the record takes the provider's own Fast, as before. + expect(adapter.readOptionRestoreFailures('session-1')).toEqual([]) + expect(events.find((event) => event.type === 'started')).toMatchObject({ + reportedOptions: { fastMode: false } + }) + }) + + // No message waits on it: the launch carries no Fast, and the start applies it once the CLI's + // settings show no per-session opt-in, as the live CLI reports with no settings file at all. + it('applies a saved Fast on to a new conversation once its settings allow it, and keeps the pick', async () => { + const claude = fakeClaude({ + settings: { applied: {}, effective: {}, sources: {} }, + initModels: [{ value: 'opus', displayName: 'Opus', supportsFastMode: true }] + }) + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + options: { model: 'opus', fastMode: 'true' } + }) + + expect(claude.connections[0].launch.options.settings).toBeUndefined() + expect(claude.connections[0].calls.map((call) => call.subtype)).toEqual([ + 'initialize', + 'get_settings', + 'apply_flag_settings' + ]) + expect(claude.connections[0].calls.at(-1)).toEqual({ + subtype: 'apply_flag_settings', + params: { settings: { fastMode: true } } + }) + expect(events.find((event) => event.type === 'started')).toMatchObject({ + reportedOptions: { fastMode: true }, + restoreSkippedOptions: [] + }) + }) + + it.each([ + ['refuses', new ClaudeControlRequestError('apply_flag_settings', 'not for this org'), false], + ['never answers', new ClaudeControlRequestTimeoutError('apply_flag_settings'), true] + ] as const)( + 'settles a saved Fast on a new conversation when the CLI %s its apply', + async (_case, failure, kept) => { + let answerApply: (error: Error) => void = () => {} + const claude = fakeClaude({ + settings: { applied: {}, effective: {}, sources: {} }, + initModels: [{ value: 'opus', displayName: 'Opus', supportsFastMode: true }], + routes: { + apply_flag_settings: () => new Promise((_, reject) => (answerApply = reject)) + } + }) + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + options: { model: 'opus', fastMode: 'true' } + }) + + // Started, with the pick, while the apply is still unanswered: nothing waits on it. + expect(claude.connections[0].calls.at(-1)?.subtype).toBe('apply_flag_settings') + expect(events.find((event) => event.type === 'started')).toMatchObject({ + reportedOptions: { fastMode: true }, + restoreSkippedOptions: [] + }) + answerApply(failure) + await tick() + + // A refusal drops the pick, as main's refused restore did, from the record too, after + // `started`; silence keeps it wanted and unconfirmed. + const skipped = events.filter((event) => event.type === 'options-skipped') + expect(skipped).toEqual( + kept ? [] : [expect.objectContaining({ fence: 7, options: { fastMode: 'true' } })] + ) + expect(adapter.readOptionRestoreFailures('session-1')).toEqual(kept ? [] : ['fastMode']) + const options = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current.fastMode === true).toBe(kept) + expect(options.current.confirmed ?? []).not.toContain('fastMode') + } + ) + + it('drops a saved Fast on from a new conversation whose model the CLI lists without it', async () => { + const claude = fakeClaude({ + settings: { applied: {}, effective: {}, sources: {} }, + initModels: [{ value: 'opus', displayName: 'Opus', supportsFastMode: false }] + }) + const adapter = adapterFor(claude) + + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + options: { model: 'opus', fastMode: 'true' } + }) + + expect( + claude.connections[0].calls.filter((call) => call.subtype === 'apply_flag_settings') + ).toEqual([]) + expect(adapter.readOptionRestoreFailures('session-1')).toEqual(['fastMode']) + }) + + it.each([ + ['effort', { effort: 'retired-effort' }], + ['permissionMode', { permissionMode: 'retired-mode' }] + ] as const)('self-heals a persisted %s the CLI would refuse at launch', async (key, options) => { + const claude = fakeClaude() + const adapter = adapterFor(claude) + + await expect( + adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9', options }) + ).resolves.toBeDefined() + expect(adapter.readOptionRestoreFailures('session-1')).toEqual([key]) + expect(claude.connections[0].launch.options).not.toHaveProperty('effort') + expect(claude.connections[0].launch.options).not.toHaveProperty('permissionMode') }) it('recovers a cancellable lifecycle when the replay arrives after dispatch returned', async () => { diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 6d4ec1c3153..e79f3cab5d2 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -1,4 +1,3 @@ -import type { SubmissionRejectionFact } from '../../shared/agent-session-failure' import { dispatchClaudeCommand } from './claude-structured-command-dispatch' import type { AgentSessionAcquisition, @@ -13,10 +12,7 @@ import { acquireClaudeSession } from './claude-structured-session-acquisition' import { supportsClaudeStructuredLocation } from './claude-structured-location-support' import { setClaudeStructuredSessionOption } from './claude-structured-options' import { readClaudeStructuredSessionOptions } from './claude-structured-session-options' -import { - awaitClaudeSessionStarted, - claudeStartupSettledWithin -} from './claude-structured-session-startup-state' +import { claudeStartupSettledWithin } from './claude-structured-session-startup-state' import { CLAUDE_DEFAULT_REQUEST_TIMEOUT_MS } from './claude-agent-sdk-control-requests' import { ClaudeAcquisitionRegistry, @@ -129,11 +125,6 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda * apart without guessing at wall-clock. */ drainObservedExits = (): Promise => drainClaudeObservedExits(this.exits) - /** Resolves once a published session's startup has landed, faulted, or been ended by a close; - * with the reason when it did not land. */ - awaitStarted = (sessionId: string): Promise => - awaitClaudeSessionStarted(this.sessions.get(sessionId)) - /** Restart reconciliation reads the transcript a resume replays; these maps track liveness. */ providerHistoryWindow: NonNullable = ( input @@ -242,6 +233,8 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda input, this.deps.requestTimeoutMs ) + startAnswered = (sessionId: string): boolean | undefined => + this.sessions.get(sessionId)?.startup.answered awaitOptionWritable = (sessionId: string): Promise => claudeStartupSettledWithin( this.sessions.get(sessionId), diff --git a/src/main/claude/claude-structured-session-close.ts b/src/main/claude/claude-structured-session-close.ts index f066699c4f1..baeda27ae2f 100644 --- a/src/main/claude/claude-structured-session-close.ts +++ b/src/main/claude/claude-structured-session-close.ts @@ -143,7 +143,7 @@ async function finalizeClaudePublishedSession( } if (!session.closeEnded) { session.closeEnded = true - const ended = { + const ended: ClaudeStructuredSessionEvent = { type: 'ended', sessionId: input.sessionId, reason: 'claude session closed', @@ -151,8 +151,9 @@ async function finalizeClaudePublishedSession( cause: 'requested-close', fence: session.fence, acquisitionGeneration: session.acquisitionGeneration, - observedAt: Date.now() - } as const + observedAt: Date.now(), + ...(session.startup.answered ? {} : { startupUnanswered: true as const }) + } try { try { session.translator?.handle(ended) diff --git a/src/main/claude/claude-structured-session-exit-lifecycle.ts b/src/main/claude/claude-structured-session-exit-lifecycle.ts index 854646490a8..c512d58c867 100644 --- a/src/main/claude/claude-structured-session-exit-lifecycle.ts +++ b/src/main/claude/claude-structured-session-exit-lifecycle.ts @@ -106,7 +106,8 @@ export function settleClaudeUnexpectedExit( fence: exit.session.fence, acquisitionGeneration: exit.session.acquisitionGeneration, observedAt: deps.now?.() ?? Date.now(), - ...(exit.session.startup.state === 'proven' ? {} : { startupUnproven: true }) + ...(exit.session.startup.state === 'proven' ? {} : { startupUnproven: true }), + ...(exit.session.startup.answered ? {} : { startupUnanswered: true }) } try { lifecycle.emit(exit.session, ended) diff --git a/src/main/claude/claude-structured-session-options.ts b/src/main/claude/claude-structured-session-options.ts index 082f45726fe..1718a10e447 100644 --- a/src/main/claude/claude-structured-session-options.ts +++ b/src/main/claude/claude-structured-session-options.ts @@ -14,6 +14,7 @@ import { } from './claude-structured-model-catalog' import type { ClaudeSession } from './claude-structured-session-state' import { structuredAgentSessionOptionModels } from '../native-chat/agent-session-wire/structured-agent-session-option-models' +import { claudeCatalogRowsOfAccount } from './claude-structured-retired-model' import { decodeStructuredAgentSessionOptionValue } from '../../shared/structured-agent-session-option-codec' /** @@ -122,14 +123,6 @@ export function readClaudeCurrentModel(session: ClaudeSession): { } } -/** - * The effort levels the session's current model advertises, with the catalog id - * that matched so a refusal names the model the pill shows. Levels are null when - * nothing identified the model: `apply_flag_settings` accepts and stores any - * level for a model with no effort control, so the catalog is the only evidence - * of a refusal — and an absent or unlisted one is not evidence, or a live CLI - * that predates `list_models` would have every effort refused under it. - */ /** One catalog read serves a whole option write. The admit check, the effort guard * and the Fast guard all ask about the same list; each taking its own read made a * single model write pay for two `list_models` round trips and let two guards answer @@ -143,6 +136,14 @@ export async function readClaudeListedModels( return catalog ? listedModels({ models: catalog }) : [] } +/** + * The effort levels the session's current model advertises, with the catalog id + * that matched so a refusal names the model the pill shows. Levels are null when + * nothing identified the model: `apply_flag_settings` accepts and stores any + * level for a model with no effort control, so the catalog is the only evidence + * of a refusal — and an absent or unlisted one is not evidence, or a live CLI + * that predates `list_models` would have every effort refused under it. + */ export function claudeModelEffortLevels( session: ClaudeSession, models: readonly ListedModel[] @@ -270,9 +271,10 @@ function writeClaudeCatalogThrough(session: ClaudeSession, discovered: ListedMod if (discovered.length === 0 || !session.catalogAccess) { return } - const support = claudeFastModeSupport(discovered, undefined) + const rows = claudeCatalogRowsOfAccount(session.catalogAccess, discovered, session.launchedModel) + const support = claudeFastModeSupport(rows, undefined) session.catalogAccess.store.recordSuccess(session.catalogAccess.fingerprint, 'claude', { - models: catalogClaudeModels(session, discovered), + models: catalogClaudeModels(session, rows), ...(support ? { fastModeSupport: support } : {}), fastModeTierByModel: new Map(), origin: 'live-session' @@ -323,9 +325,9 @@ function observeClaudeSettingsReadback( } /** The options as main already holds them, over `catalog`; asks the CLI nothing. Startup's - * settings readback and restore's confirmations are applied by the time a start proves, and - * the SDK answers `list_models` from its initialize result, so a started session's snapshot - * passes that result here rather than paying two round trips for what it already read. */ + * settings readback is applied by the time a start proves, and the SDK answers `list_models` + * from its initialize result, so a started session's snapshot passes that result here rather + * than paying two round trips for what it already read. */ export function claudeStructuredSessionOptionsFrom( session: ClaudeSession, catalog: unknown[] | null, diff --git a/src/main/claude/claude-structured-session-publication.ts b/src/main/claude/claude-structured-session-publication.ts index c685c2cd2ae..8c3b524086b 100644 --- a/src/main/claude/claude-structured-session-publication.ts +++ b/src/main/claude/claude-structured-session-publication.ts @@ -64,6 +64,8 @@ export function createClaudeSessionPublication(input: { reportedModelMutation: 0, confirmedOptions: new Set(), restoreSkippedOptions: new Set(), + launchedModel: null, + fastModeAtStart: false, translator: input.translator, events: input.events, ...(input.unbindReadingControl ? { unbindReadingControl: input.unbindReadingControl } : {}), diff --git a/src/main/claude/claude-structured-session-recovery.test.ts b/src/main/claude/claude-structured-session-recovery.test.ts index 86598780b45..e84ff7dbe5f 100644 --- a/src/main/claude/claude-structured-session-recovery.test.ts +++ b/src/main/claude/claude-structured-session-recovery.test.ts @@ -10,7 +10,8 @@ import { fakeClaude, identityFor, PROVIDER_SESSION_ID, - tick + tick, + claudeStartupSettled } from './claude-structured-session-test-support' import { invokeCanUseTool } from './claude-can-use-tool-test-support' import { @@ -349,7 +350,7 @@ describe('ClaudeStructuredSessionAdapter close and exit recovery', () => { spawnToken: 'spawn-9', events: journalSink }) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') const first = claude.connections[0] const oldPrompt = invokeCanUseTool(first, 'Bash', 'permission-retained', 'tool-retained') const oldSession = ( diff --git a/src/main/claude/claude-structured-session-startup-state.ts b/src/main/claude/claude-structured-session-startup-state.ts index f0d3a252e20..ba4107fa7f4 100644 --- a/src/main/claude/claude-structured-session-startup-state.ts +++ b/src/main/claude/claude-structured-session-startup-state.ts @@ -1,7 +1,7 @@ -// Where a Claude start stands. A session is published once its child is spawned, before the CLI -// has answered initialize. Nothing is written to it until startup lands (init facts read and saved -// options restored): the host's delivery loop waits on `settled` before it hands a message over, -// so a first turn never runs under defaults the restore was about to replace. +// Where a Claude start stands. A session is published once its child is spawned, launched with the +// chat's saved options, before the CLI has answered initialize. Messages are written to it at once +// (the CLI queues them behind its start); only an option write, a control request initialize must +// answer first, waits for startup to land. import type { SubmissionRejectionFact } from '../../shared/agent-session-failure' import { providerStartupFailureFact } from '../native-chat/agent-session-wire/structured-agent-session-failure-text' @@ -9,10 +9,11 @@ import type { ClaudeSession } from './claude-structured-session-state' export type ClaudeSessionStartup = { state: 'pending' | 'proven' | 'failed' + /** The CLI answered initialize: it may have run what it was handed. Before that it ran nothing. */ + answered: boolean failure: Error | null /** Resolves once startup has landed or faulted, or the child exited or was closed; never - * rejects. A close must end it: the delivery loop waits here, and a start Stop cut short - * would otherwise hold that loop forever. */ + * rejects. A close must end it: an option write waits here. */ settled: Promise end: () => void } @@ -22,7 +23,7 @@ export function createClaudeSessionStartup(): ClaudeSessionStartup { const ended = new Promise((resolve) => { end = resolve }) - return { state: 'pending', failure: null, settled: ended, end } + return { state: 'pending', answered: false, failure: null, settled: ended, end } } export function claudeStartupFailureFact(session: ClaudeSession): SubmissionRejectionFact | null { @@ -31,16 +32,6 @@ export function claudeStartupFailureFact(session: ClaudeSession): SubmissionReje : null } -export async function awaitClaudeSessionStarted( - session: ClaudeSession | undefined -): Promise { - if (!session) { - return - } - await session.startup.settled - return claudeStartupFailureFact(session) ?? undefined -} - /** Resolves when startup lands or `timeoutMs` passes; a stuck start then refuses the write as before. */ export function claudeStartupSettledWithin( session: ClaudeSession | undefined, diff --git a/src/main/claude/claude-structured-session-startup.test.ts b/src/main/claude/claude-structured-session-startup.test.ts index a49120f5359..58a81a2ca6f 100644 --- a/src/main/claude/claude-structured-session-startup.test.ts +++ b/src/main/claude/claude-structured-session-startup.test.ts @@ -5,10 +5,14 @@ import { adapterAtPublishFor, fakeClaude, identityFor, - USER_MESSAGE + USER_MESSAGE, + claudeStartupSettled } from './claude-structured-session-test-support' import { CLAUDE_DEFAULT_REQUEST_TIMEOUT_MS } from './claude-agent-sdk-control-requests' +/** Far past any deadline a start used to have. */ +const NEVER_MS = 10 * 60_000 + type LateSettlement = Parameters< NonNullable >[0] @@ -55,7 +59,7 @@ describe('Claude structured session publishes before the CLI answers initialize' expect(adapter.readCommands('session-1')).toBeUndefined() await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') expect(events.find((event) => event.type === 'options')).toMatchObject({ models: [{ value: 'claude-sonnet' }] @@ -65,19 +69,15 @@ describe('Claude structured session publishes before the CLI answers initialize' await adapter.closeAll() }) - it('reports `started` once saved options are restored, having written no prompt of its own', async () => { + it('reports `started` with the options its child was launched with, having written none', async () => { const claude = fakeClaude({ initDelayMs: SLOW_INIT_MS, initModel: 'claude-opus-9' }) const { adapter, events } = startingAdapter(claude) - const order: string[] = [] - claude.routes.set_model = () => { - order.push('set_model') - return undefined - } await adapter.acquire({ ...ACQUIRE, options: { model: 'opus' } }) + expect(claude.connections[0].launch.options.model).toBe('opus') expect(events.some((event) => event.type === 'started')).toBe(false) await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') const startedAt = events.findIndex((event) => event.type === 'started') expect(events[startedAt]).toEqual({ @@ -85,12 +85,14 @@ describe('Claude structured session publishes before the CLI answers initialize' sessionId: 'session-1', fence: 7, acquisitionGeneration: expect.any(String), - // What the restore just proved, carried so the host never asks the CLI again. + // What the child was launched with, carried so the host never asks the CLI again. reportedOptions: expect.objectContaining({ model: 'opus' }), restoreSkippedOptions: [] }) - // The restore wrote the saved model before `started`; no message waits inside the adapter. - expect(order).toEqual(['set_model']) + expect(claude.connections[0].calls.map(({ subtype }) => subtype)).toEqual([ + 'initialize', + 'get_settings' + ]) expect(claude.connections[0].sent).toEqual([]) expect(events.slice(0, startedAt).some((event) => event.type === 'options')).toBe(true) await adapter.closeAll() @@ -132,24 +134,63 @@ describe('Claude structured session publishes before the CLI answers initialize' await adapter.closeAll() }) - // The host's delivery loop waits here before it hands a message over, so the adapter no longer - // holds prompts of its own: nothing is written until startup lands because nothing is sent. - it('resolves awaitStarted only once startup lands', async () => { + // The child was launched with the chat's saved options, so nothing waits on initialize: the SDK + // streams the message to the CLI, which takes it behind its own start. + it('writes a message sent before initialize answers at once', async () => { const claude = fakeClaude({ initDelayMs: SLOW_INIT_MS }) - const { adapter } = startingAdapter(claude) - await adapter.acquire(ACQUIRE) - let started = false - const waited = adapter.awaitStarted('session-1').then(() => { - started = true - }) - - await vi.advanceTimersByTimeAsync(SLOW_INIT_MS - 1) - expect(started).toBe(false) - await vi.advanceTimersByTimeAsync(1) - await waited + const { adapter, events } = startingAdapter(claude) + await adapter.acquire({ ...ACQUIRE, options: { model: 'opus' } }) await expect(adapter.dispatch(PROMPT)).resolves.toEqual({ state: 'admitted' }) expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) + expect(events.some((event) => event.type === 'started')).toBe(false) + + await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) + await claudeStartupSettled(adapter, 'session-1') + expect(events.some((event) => event.type === 'started')).toBe(true) + await adapter.closeAll() + }) + + it('confirms a launched effort only when the settings readback reports that effort', async () => { + const reportsHigh = { + applied: { model: 'claude-sonnet-5', effort: 'high' }, + effective: { model: 'claude-sonnet-5', effortLevel: 'high', env: {} }, + sources: {} + } + const agreeing = fakeClaude({ settings: reportsHigh }) + const disagreeing = fakeClaude({ settings: reportsHigh }) + const started = async (claude: ReturnType, effort: string) => { + const { adapter } = startingAdapter(claude) + await adapter.acquire({ ...ACQUIRE, options: { effort } }) + await claudeStartupSettled(adapter, 'session-1') + return adapter + } + + const kept = await started(agreeing, 'high') + await expect(kept.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toMatchObject({ + current: { effort: 'high', confirmed: expect.arrayContaining(['effort']) } + }) + // The saved pick stays wanted; only the readback could vouch for it, and it did not. + const unconfirmed = await started(disagreeing, 'low') + const options = await unconfirmed.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current.effort).toBe('low') + expect(options.current.confirmed ?? []).not.toContain('effort') + await kept.closeAll() + await unconfirmed.closeAll() + }) + + // Measured on Claude Code 2.1.280: `--effort xhigh` reads back as `applied.effort: 'xhigh'` with + // `effective` empty, since `effective` holds only the settings files. + it('confirms a launched effort the CLI reports only as applied', async () => { + const claude = fakeClaude({ + settings: { applied: { model: 'claude-opus-5', effort: 'xhigh' }, effective: {}, sources: {} } + }) + const { adapter } = startingAdapter(claude) + await adapter.acquire({ ...ACQUIRE, options: { effort: 'xhigh' } }) + await claudeStartupSettled(adapter, 'session-1') + + const options = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current).toMatchObject({ effort: 'xhigh', confirmed: ['effort'] }) await adapter.closeAll() }) @@ -162,7 +203,7 @@ describe('Claude structured session publishes before the CLI answers initialize' await adapter.acquire(ACQUIRE) await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') await adapter.drainObservedExits() expect(events.find((event) => event.type === 'ended')).toMatchObject({ @@ -184,7 +225,7 @@ describe('Claude structured session publishes before the CLI answers initialize' await adapter.acquire(ACQUIRE) await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') await adapter.drainObservedExits() // A failed start is released on the same evidence a failed create is. @@ -194,11 +235,97 @@ describe('Claude structured session publishes before the CLI answers initialize' }) }) + it('lands a start whose CLI answers initialize only after minutes', async () => { + const claude = fakeClaude({ initDelayMs: NEVER_MS - 1 }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + + await vi.advanceTimersByTimeAsync(NEVER_MS) + await claudeStartupSettled(adapter, 'session-1') + + expect(events.some((event) => event.type === 'started')).toBe(true) + expect(events.some((event) => event.type === 'ended')).toBe(false) + expect(claude.connections[0].closeCount).toBe(0) + await adapter.closeAll() + }) + + // No deadline ends it: a Stop or a close does, and every message is already written meanwhile. + it('keeps a start whose CLI never answers initialize, taking messages, until it is closed', async () => { + const claude = fakeClaude({ initDelayMs: 10 * NEVER_MS }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + await expect(adapter.dispatch(PROMPT)).resolves.toEqual({ state: 'admitted' }) + + await vi.advanceTimersByTimeAsync(NEVER_MS) + + expect(events.some((event) => event.type === 'ended' || event.type === 'started')).toBe(false) + expect(claude.connections[0].closeCount).toBe(0) + expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + expect(claude.connections[0].closeCount).toBe(1) + }) + + it('fails a start at once when a frame names another session before initialize answers', async () => { + const claude = fakeClaude({ initDelayMs: NEVER_MS, initProof: 'none' }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + + claude.connections[0].handlers.onMessage?.({ + type: 'system', + subtype: 'hook_started', + hook_name: 'SessionStart:startup', + session_id: 'foreign-session' + }) + await vi.advanceTimersByTimeAsync(0) + await claudeStartupSettled(adapter, 'session-1') + await adapter.drainObservedExits() + + expect(events.find((event) => event.type === 'ended')).toMatchObject({ + reason: 'claude provider session expected', + failure: { kind: 'startFailed' }, + startupUnproven: true + }) + }) + + // Only a SessionStart hook sends a start frame before the first turn; without one, the + // initialize answer is the whole start. + it('lands a start whose CLI sends no start frame before its first turn', async () => { + const claude = fakeClaude({ initProof: 'none' }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + await claudeStartupSettled(adapter, 'session-1') + + expect(events.some((event) => event.type === 'started')).toBe(true) + await expect(adapter.dispatch(PROMPT)).resolves.toEqual({ state: 'admitted' }) + expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) + await adapter.closeAll() + }) + + it('ends a started session whose first frame names another provider session', async () => { + const claude = fakeClaude({ initProof: 'none' }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + await claudeStartupSettled(adapter, 'session-1') + + claude.connections[0].handlers.onMessage?.({ + type: 'system', + subtype: 'init', + session_id: 'foreign-session' + }) + await adapter.drainObservedExits() + + expect(events.find((event) => event.type === 'ended')).toMatchObject({ + reason: 'claude provider session expected', + cause: 'unexpected-exit' + }) + expect(claude.connections[0].closeCount).toBe(1) + }) + it('ends an unauthenticated start with sign-in guidance', async () => { const claude = fakeClaude({ initAccount: { apiProvider: 'firstParty', tokenSource: 'none' } }) const { adapter, events } = startingAdapter(claude) await adapter.acquire(ACQUIRE) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') await adapter.drainObservedExits() expect(events.find((event) => event.type === 'ended')).toMatchObject({ @@ -218,7 +345,7 @@ describe('Claude structured session publishes before the CLI answers initialize' const claude = fakeClaude({ initAccount: { apiProvider: 'firstParty', ...account } }) const { adapter, events } = startingAdapter(claude) await adapter.acquire(ACQUIRE) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') expect(events.some((event) => event.type === 'started')).toBe(true) expect(events.some((event) => event.type === 'ended')).toBe(false) @@ -231,7 +358,7 @@ describe('Claude structured session publishes before the CLI answers initialize' }) const { adapter, events } = startingAdapter(claude) await adapter.acquire(ACQUIRE) - await adapter.awaitStarted('session-1') + await claudeStartupSettled(adapter, 'session-1') await adapter.drainObservedExits() expect(events.find((event) => event.type === 'ended')).toMatchObject({ @@ -239,14 +366,14 @@ describe('Claude structured session publishes before the CLI answers initialize' }) }) - // A Stop that closes a child still starting must end the wait the host's delivery loop is in, - // though initialize never answers; otherwise every later send joins a loop that never moves. + // A Stop that closes a child still starting must end the wait an option write is in, though + // initialize never answers. it('ends the wait on a start closed before init, without faulting it', async () => { const claude = fakeClaude({ initDelayMs: 10 * SLOW_INIT_MS }) const { adapter, events } = startingAdapter(claude) await adapter.acquire(ACQUIRE) let ended = false - const waited = adapter.awaitStarted('session-1').then(() => { + const waited = claudeStartupSettled(adapter, 'session-1').then(() => { ended = true }) @@ -263,17 +390,19 @@ describe('Claude structured session publishes before the CLI answers initialize' expect(events.some((event) => event.type === 'started')).toBe(false) }) - it('interrupts nothing when Stop lands before init: nothing was written', async () => { + // The CLI answers no control request before initialize; Claude's Stop ends the child instead. + it('sends no interrupt to a CLI still starting, though it was handed a message', async () => { const claude = fakeClaude({ initDelayMs: SLOW_INIT_MS }) const { adapter } = startingAdapter(claude) await adapter.acquire(ACQUIRE) + await adapter.dispatch(PROMPT) - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-1', fence: 7 }) - ).resolves.toEqual({ cancelled: false }) + await expect(adapter.cancelTurn({ sessionId: 'session-1', fence: 7 })).resolves.toEqual({ + cancelled: false + }) expect(claude.connections[0].calls.map(({ subtype }) => subtype)).not.toContain('interrupt') - expect(claude.connections[0].sent).toEqual([]) + expect(adapter.stopEndsSession()).toBe(true) await adapter.closeAll() }) }) diff --git a/src/main/claude/claude-structured-session-startup.ts b/src/main/claude/claude-structured-session-startup.ts index ce14a64dd2c..e7e0c12f338 100644 --- a/src/main/claude/claude-structured-session-startup.ts +++ b/src/main/claude/claude-structured-session-startup.ts @@ -1,9 +1,10 @@ -// What Claude reports at initialize, read after the session is already published. None of it -// gates the create: a slow start is still a start, and every way it can fail (exit, auth, -// a foreign session id) faults the published session through its exit path. +// What Claude reports at initialize, read in the background once the session is published. None of +// it gates the create or a message: the child was launched with the chat's saved options and takes +// input at once. Every way the start can fail (exit, auth, a foreign session id) faults the +// published session through its exit path; a CLI that never answers is ended by a Stop or a close. import type { - StructuredAgentSessionAcquireInput, + StructuredAgentSessionOptionsSkippedEvent, StructuredAgentSessionStartedEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' @@ -15,69 +16,124 @@ import { readClaudeModels, type ClaudeInitObservation } from './claude-structured-init-proof' -import { restoreClaudeStructuredSessionOptions } from './claude-structured-options' import { claudeStructuredSessionPublicationOptions, prepareClaudeStructuredSessionAcquisitionOptions, readClaudeStructuredSessionSettings } from './claude-structured-session-acquisition-options' +import { claudeRetiredOptions } from './claude-structured-retired-model' import { claudeStructuredSessionOptionsFrom, observeClaudeSettingsApplied, readClaudeSettingsEffort } from './claude-structured-session-options' -import { failClaudeStartup } from './claude-structured-session-startup-state' +import { + failClaudeStartup, + type ClaudeSessionStartup +} from './claude-structured-session-startup-state' import type { ClaudeSession, ClaudeStructuredSessionEvent } from './claude-structured-session-state' +import { + admitClaudeStartFastMode, + applyClaudeStartFastMode +} from './claude-structured-start-fast-mode' +/** The CLI's own frame naming the session it runs (system/init or a SessionStart hook). Only a + * SessionStart hook sends one before the first turn, so startup takes it when it came and never + * waits for it. */ export type ClaudeInitProof = { promise: Promise resolve: (init: ClaudeInitObservation) => void reject: (error: Error) => void + /** A frame named another provider session. */ + refuse: () => void + /** The proof seen so far, or null; throws when it was refused or the child failed first. */ + seen: () => ClaudeInitObservation | null + /** Set once startup has read the proof: a later refusal ends the session. */ + onRefusal: ((error: Error) => void) | null } export function createClaudeInitProof(): ClaudeInitProof { - let resolve = (_init: ClaudeInitObservation): void => {} - let reject = (_error: Error): void => {} - const promise = new Promise((resolvePromise, rejectPromise) => { - resolve = resolvePromise - reject = rejectPromise + let resolvePromise = (_init: ClaudeInitObservation): void => {} + let rejectPromise = (_error: Error): void => {} + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve + rejectPromise = reject }) void promise.catch(() => {}) - return { promise, resolve, reject } + let outcome: { init: ClaudeInitObservation } | { error: Error } | null = null + const reject = (error: Error): void => { + outcome ??= { error } + rejectPromise(error) + } + const proof: ClaudeInitProof = { + promise, + resolve: (init) => { + outcome ??= { init } + resolvePromise(init) + }, + reject, + refuse: () => { + // A session already proven by its own frame keeps that proof. + if (outcome && 'init' in outcome) { + return + } + const error = new Error('claude provider session expected') + reject(error) + proof.onRefusal?.(error) + }, + seen: () => { + if (outcome && 'error' in outcome) { + throw outcome.error + } + return outcome?.init ?? null + }, + onRefusal: null + } + return proof } export type StructuredAgentSessionStartedOptions = Pick< StructuredAgentSessionStartedEvent, - 'reportedOptions' | 'restoreSkippedOptions' + 'reportedOptions' | 'restoreSkippedOptions' | 'retiredOptions' > +/** A lifecycle event the start reports, before the session's identity is stamped on it. */ +export type ClaudeStartupReport = + | ({ type: 'started' } & StructuredAgentSessionStartedOptions) + | Pick + export type ClaudeStartupFacts = { - init: ClaudeInitObservation + init: ClaudeInitObservation | null + initProof: ClaudeInitProof initialization: unknown settings: unknown prepared: ReturnType + resumesTranscript: boolean + requestTimeoutMs: number | undefined } -/** Settles on the CLI's answers or on its exit; there is no startup timer. */ +/** Settles on the CLI's initialize answer, or on its exit or a refused proof. */ export async function readClaudeStartupFacts(input: { connection: ClaudeStreamJsonConnection initProof: ClaudeInitProof sessionId: string providerSessionId: string + startup: Pick resumesTranscript: boolean - inputOptions: StructuredAgentSessionAcquireInput['options'] requestTimeoutMs: number | undefined emit: (event: ClaudeStructuredSessionEvent) => void }): Promise { - const [initialization, init] = await Promise.all([ + // The CLI's first answer has no request deadline of its own; the reads after it do. + const initialization = await Promise.race([ input.connection.initializationResult().then((result) => { + input.startup.answered = true const authError = claudeInitializationAuthError(result) if (authError) { throw authError } return result }), - input.initProof.promise + input.initProof.promise.then(() => new Promise(() => {})) ]) if (input.connection.closed) { throw new Error('claude session closed before startup completed') @@ -87,30 +143,26 @@ export async function readClaudeStartupFacts(input: { sessionId: input.sessionId, models: readClaudeModels(initialization) }) - if (init.providerSessionId !== input.providerSessionId) { - throw new Error( - `claude proved session ${init.providerSessionId}, expected ${input.providerSessionId}` - ) - } + input.initProof.seen() const settings = await readClaudeStructuredSessionSettings( input.connection, input.requestTimeoutMs ) + // Read again: a frame naming another session may have come during the settings read. + const init = input.initProof.seen() input.emit({ type: 'auth-diagnostic', sessionId: input.sessionId, - diagnostic: claudeAuthDiagnostic(init, settings) + diagnostic: claudeAuthDiagnostic(initialization, init, settings) }) return { init, + initProof: input.initProof, initialization, settings, - prepared: prepareClaudeStructuredSessionAcquisitionOptions({ - settings, - initialization, - inputOptions: input.inputOptions, - resumesTranscript: input.resumesTranscript - }) + prepared: prepareClaudeStructuredSessionAcquisitionOptions({ settings, initialization }), + resumesTranscript: input.resumesTranscript, + requestTimeoutMs: input.requestTimeoutMs } } @@ -119,29 +171,40 @@ function applyClaudeStartupFacts(session: ClaudeSession, facts: ClaudeStartupFac const effort = readClaudeSettingsEffort(settings) const published = claudeStructuredSessionPublicationOptions(prepared) // A turn's own init frame may already have reported the running model. - if (init.model && session.reportedOptions.model === undefined) { + if (init?.model && session.reportedOptions.model === undefined) { session.reportedOptions.model = init.model session.reportedModelMutation = session.optionMutationSequence } observeClaudeSettingsApplied(session, settings) + // The readback vouches for a value the child was launched with only when it reports that value. + const agrees = (key: string, reported: string): boolean => + !session.options.has(key) || session.options.get(key) === reported if (effort) { session.reportedOptions.effort = effort + if (agrees('effort', effort)) { + session.confirmedOptions.add('effort') + } + } + // A launch `--effort` shows only in `applied`, never in `effective` (measured on 2.1.280). + const launchedEffort = session.options.get('effort') + if (launchedEffort !== undefined && session.appliedOptions?.effort === launchedEffort) { session.confirmedOptions.add('effort') } if (published.fastMode !== null) { session.reportedOptions.fastMode = published.fastMode - session.confirmedOptions.add('fastMode') + if (agrees('fastMode', String(published.fastMode))) { + session.confirmedOptions.add('fastMode') + } } if (published.fastModePerSessionOptIn !== null) { session.fastModePerSessionOptIn = published.fastModePerSessionOptIn } session.fastModeState ??= published.fastModeState session.fastModeDisabledReason ??= published.fastModeDisabledReason - session.options = prepared.options - session.capabilities = readClaudeCapabilities(session.capabilities, initialization, init.message) + session.capabilities = readClaudeCapabilities(session.capabilities, initialization, init?.message) // A catalog frame that streamed in after publish is newer than the initialize answer. if (session.commands.commands === undefined) { - session.commands = new ClaudeSlashCommandCatalog(init.message, initialization) + session.commands = new ClaudeSlashCommandCatalog(init?.message, initialization) } session.events?.publish() } @@ -160,16 +223,16 @@ function claudeStartedReportedOptions( return persisted } -/** Applies startup facts to the published session and restores saved options; only then does the - * session take input. Any failure faults the session so the user sees why it never started. */ +/** Applies startup facts to the published session, which already takes input. Any failure faults + * the session so the user sees why it never started. */ export async function settleClaudeSessionStartup(input: { session: ClaudeSession facts: Promise isCurrent: () => boolean - requestTimeoutMs: number | undefined fault: (error: Error) => void - /** Startup has proven; `options` is what the child now reports, snapshotted from memory. */ - onStarted: (options: StructuredAgentSessionStartedOptions) => void + /** `started` once startup has proven, with what the child now reports, snapshotted from memory; + * then any saved option the child showed it cannot run. */ + report: (event: ClaudeStartupReport) => void }): Promise { const { session } = input const superseded = (): boolean => { @@ -184,20 +247,36 @@ export async function settleClaudeSessionStartup(input: { if (superseded()) { return } + // From here no read of the proof is pending, so a frame naming another session ends it. + facts.initProof.onRefusal = (error) => { + failClaudeStartup(session, error) + if (input.isCurrent()) { + input.fault(error) + } + } applyClaudeStartupFacts(session, facts) - await restoreClaudeStructuredSessionOptions(session, input.requestTimeoutMs) + const startFastMode = admitClaudeStartFastMode(session, facts) if (!superseded()) { - input.onStarted({ + input.report({ + type: 'started', // `list_models` is answered from this same initialize result, so nothing is re-read. reportedOptions: claudeStartedReportedOptions( session, readClaudeModels(facts.initialization) ), - restoreSkippedOptions: [...session.restoreSkippedOptions] + restoreSkippedOptions: [...session.restoreSkippedOptions], + ...claudeRetiredOptions(session) }) if (session.startup.state === 'pending') { session.startup.state = 'proven' } + if (startFastMode !== null) { + void applyClaudeStartFastMode(session, facts, startFastMode, (event) => { + if (input.isCurrent()) { + input.report(event) + } + }) + } } } catch (caught) { const error = caught instanceof Error ? caught : new Error(String(caught)) diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index 876090c07e4..e327e231621 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -6,7 +6,10 @@ import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' -import type { StructuredAgentSessionStartedEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { + StructuredAgentSessionOptionsSkippedEvent, + StructuredAgentSessionStartedEvent +} from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeStreamJsonConnection, openClaudeStreamJsonConnection @@ -65,8 +68,9 @@ export type ClaudeStructuredSessionEvent = fence: number } | { type: 'auth-diagnostic'; sessionId: string; diagnostic: ClaudeAuthDiagnostic } - /** Startup facts applied and saved options restored; held prompts are about to be written. */ + /** Startup facts applied; the child already took any message written to it. */ | StructuredAgentSessionStartedEvent + | StructuredAgentSessionOptionsSkippedEvent | { type: 'ended' sessionId: string @@ -80,6 +84,8 @@ export type ClaudeStructuredSessionEvent = observedAt?: number /** The child ended before proving startup, so reacquiring would repeat the same start. */ startupUnproven?: true + /** The child ended before it answered initialize, so it ran nothing it was handed. */ + startupUnanswered?: true } export type ClaudeLateDispatchOutcome = @@ -179,6 +185,10 @@ export type ClaudeSession = { /** Options whose recorded value the provider reported, not merely accepted. */ confirmedOptions: Set restoreSkippedOptions: Set + /** The model the child was launched with, as `--model`; null when it runs the CLI's own. */ + launchedModel: string | null + /** The launch left the saved Fast out, so the start applies it once the settings are read. */ + fastModeAtStart: boolean /** Absent when the adapter runs without a host catalog store (tests). */ catalogAccess?: AgentModelCatalogSessionAccess /** CLI-advertised protocol capabilities from init; gates interrupt-receipt handling. */ @@ -206,7 +216,7 @@ export type ClaudeSession = { translator: ClaudeJournalTranslator | null events: StructuredAgentSessionEventSink | undefined unbindReadingControl?: () => void - /** Published at spawn; init facts, option restore and queued prompts land when startup does. */ + /** Published at spawn with its saved options; init facts land when startup does. */ startup: ClaudeSessionStartup } diff --git a/src/main/claude/claude-structured-session-test-support.ts b/src/main/claude/claude-structured-session-test-support.ts index 2b5d28c99bc..c29c6693591 100644 --- a/src/main/claude/claude-structured-session-test-support.ts +++ b/src/main/claude/claude-structured-session-test-support.ts @@ -16,6 +16,7 @@ import { } from './claude-structured-session-adapter' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { claudeProviderHandle } from '../../shared/agent-session-provider-handle-encoding' +export { claudeStartupSettled } from './claude-structured-startup-settled-test-support' export const PROVIDER_SESSION_ID = '819cf9f8-e43c-4ad7-b50f-54aa158a726a' @@ -52,8 +53,9 @@ export function fakeClaude( initSessionId?: string initUuid?: string initModel?: string - /** 'session-start' (default) mirrors live: a SessionStart hook frame proves the - * session and system/init arrives only when the first command starts a cycle. + /** 'session-start' (default) mirrors live with a SessionStart hook installed: its + * frame proves the session and system/init arrives only when the first command starts a + * cycle; 'none' mirrors live without one. * 'init' emits init at startup — an UNMEASURED shape, opt-in only. */ initProof?: 'init' | 'session-start' | 'none' initAccount?: unknown @@ -127,8 +129,9 @@ export function fakeClaude( // sessions prove startup with a SessionStart hook frame instead. emitCycleInit() } else if (options.initProof !== 'none') { - // The live proof order (measured through Orca's adapter): SessionStart - // hook frames arrive first; system/init only when a cycle starts. + // The live order with a SessionStart hook installed (Orca's status hooks): + // its frames arrive first; system/init only when a cycle starts. Without + // one, nothing arrives before the first turn ('none'). handlers.onMessage?.({ type: 'system', subtype: 'hook_started', @@ -159,12 +162,13 @@ export function fakeClaude( }, getSettings: async () => { connection.calls.push({ subtype: 'get_settings' }) - // Shape measured from Claude Code 2.1.258: {applied, effective, sources}, - // and the only place the session's current effort is reported. + // Shape measured from Claude Code 2.1.258: {applied, effective, sources}. A launch + // `--effort` shows only in `applied`, with no `effective.effortLevel` (2.1.280). + const launched = launch.options.effort return ( options.settings ?? { - applied: { model: 'claude-sonnet-5', effort: 'high', advisor: null, ultracode: false }, - effective: { model: 'claude-sonnet-5', effortLevel: 'high', env: {} }, + applied: { model: 'claude-sonnet-5', effort: launched ?? 'high', ultracode: false }, + effective: { model: 'claude-sonnet-5', ...(launched ? {} : { effortLevel: 'high' }) }, sources: {} } ) @@ -175,7 +179,7 @@ export function fakeClaude( }, setModel: async (model) => { connection.calls.push({ subtype: 'set_model', params: { model } }) - routed('set_model', { model }) + await routed('set_model', { model }) }, setPermissionMode: async (mode) => { connection.calls.push({ subtype: 'set_permission_mode', params: { mode } }) @@ -183,7 +187,7 @@ export function fakeClaude( }, applyFlagSettings: async (settings) => { connection.calls.push({ subtype: 'apply_flag_settings', params: { settings } }) - routed('apply_flag_settings', { settings }) + await routed('apply_flag_settings', { settings }) }, interrupt: async (interruptOptions) => { connection.calls.push({ @@ -248,7 +252,7 @@ export function adapterFor( const acquire = adapter.acquire adapter.acquire = async (input) => { const acquisition = await acquire(input) - await adapter.awaitStarted(input.identity.sessionId) + await adapter['sessions'].get(input.identity.sessionId)?.startup.settled return acquisition } return adapter diff --git a/src/main/claude/claude-structured-spawn-options.test.ts b/src/main/claude/claude-structured-spawn-options.test.ts new file mode 100644 index 00000000000..42780ae4998 --- /dev/null +++ b/src/main/claude/claude-structured-spawn-options.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest' +import { + CLAUDE_STRUCTURED_BASE_OPTIONS, + claudeStructuredPermissionOptions, + type ClaudeStructuredSdkOptions +} from './claude-structured-launch-resolution' +import { claudeStructuredSpawnOptions } from './claude-structured-spawn-options' + +function launched( + saved: Record, + options: { resumesTranscript?: boolean; base?: ClaudeStructuredSdkOptions } = {} +) { + return claudeStructuredSpawnOptions({ + launch: { + options: options.base ?? CLAUDE_STRUCTURED_BASE_OPTIONS, + resumesTranscript: options.resumesTranscript ?? true + }, + saved + }) +} + +const BYPASS_LAUNCH: ClaudeStructuredSdkOptions = { + ...CLAUDE_STRUCTURED_BASE_OPTIONS, + extraArgs: { + ...CLAUDE_STRUCTURED_BASE_OPTIONS.extraArgs, + ...claudeStructuredPermissionOptions('bypassPermissions').extraArgs + } +} + +describe('a Claude chat launched with its saved options', () => { + it('passes the saved model, effort, Fast and permission mode as launch options', () => { + const spawn = launched({ + model: 'opus', + effort: 'high', + fastMode: 'true', + permissionMode: 'plan' + }) + + expect(spawn.sdkOptions).toMatchObject({ + model: 'opus', + effort: 'high', + settings: { fastMode: true }, + permissionMode: 'plan', + // The base launch is kept whole. + includePartialMessages: true, + extraArgs: { 'replay-user-messages': null } + }) + expect(Object.fromEntries(spawn.options)).toEqual({ + model: 'opus', + effort: 'high', + fastMode: 'true', + permissionMode: 'plan' + }) + expect(spawn.skipped).toEqual([]) + }) + + // A resolved id, a model newer than any listing Orca holds, a retired one: the CLI's own answer + // decides, and a turn's model-not-found error drops it then. + it.each(['claude-sonnet-5', 'claude-next-9', 'claude-retired-1'])( + 'passes the saved model %s as it was picked', + (model) => { + const spawn = launched({ model, effort: 'xhigh' }) + + expect(spawn.sdkOptions).toMatchObject({ model, effort: 'xhigh' }) + expect(spawn.skipped).toEqual([]) + } + ) + + it('leaves out an effort or permission mode no Claude can parse, and Fast it cannot decode', () => { + const spawn = launched({ effort: 'ludicrous', permissionMode: 'retired-mode', fastMode: 'yes' }) + + expect(spawn.sdkOptions).not.toHaveProperty('effort') + expect(spawn.sdkOptions).not.toHaveProperty('permissionMode') + expect(spawn.sdkOptions).not.toHaveProperty('settings') + expect(spawn.skipped).toEqual(['effort', 'fastMode', 'permissionMode']) + }) + + it('keeps a saved Fast on for a new conversation, for its start to apply', () => { + const spawn = launched({ fastMode: 'true' }, { resumesTranscript: false }) + + expect(spawn.sdkOptions).not.toHaveProperty('settings') + expect(spawn.options.get('fastMode')).toBe('true') + expect(spawn.fastModeAtStart).toBe(true) + expect(spawn.skipped).toEqual([]) + }) + + it('passes a saved Fast off to a new conversation', () => { + const spawn = launched({ fastMode: 'false' }, { resumesTranscript: false }) + + expect(spawn.sdkOptions.settings).toEqual({ fastMode: false }) + expect(spawn.fastModeAtStart).toBe(false) + }) + + // The SDK writes the agent Arguments after its own options, so left in, the Arguments' flag would + // reach the CLI as a second `--model` after the chat's pick. + it("replaces the agent Arguments' model and effort with the chat's saved ones", () => { + const base = { + ...CLAUDE_STRUCTURED_BASE_OPTIONS, + extraArgs: { ...CLAUDE_STRUCTURED_BASE_OPTIONS.extraArgs, model: 'opus', effort: 'max' } + } + + expect(launched({ model: 'sonnet', effort: 'low' }, { base }).sdkOptions).toMatchObject({ + model: 'sonnet', + effort: 'low', + extraArgs: { 'replay-user-messages': null } + }) + expect(launched({ model: 'sonnet', effort: 'low' }, { base }).sdkOptions.extraArgs).toEqual({ + 'replay-user-messages': null + }) + // With nothing saved, the Arguments decide. + expect(launched({}, { base }).sdkOptions.extraArgs).toEqual(base.extraArgs) + }) + + // One `--settings` reaches the CLI: the Arguments' file is kept, and the start applies Fast. + it.each(['true', 'false'])( + "keeps the agent Arguments' settings and leaves a saved Fast %s to the start", + (fastMode) => { + const base = { + ...CLAUDE_STRUCTURED_BASE_OPTIONS, + extraArgs: { ...CLAUDE_STRUCTURED_BASE_OPTIONS.extraArgs, settings: '/repo/claude.json' } + } + const spawn = launched({ fastMode }, { base }) + + expect(spawn.sdkOptions).not.toHaveProperty('settings') + expect(spawn.sdkOptions.extraArgs).toEqual(base.extraArgs) + expect(spawn.options.get('fastMode')).toBe(fastMode) + expect(spawn.fastModeAtStart).toBe(true) + } + ) + + it('launches a saved bypass under an Agent Permissions bypass with the owned bypass flag', () => { + const spawn = launched({ permissionMode: 'bypassPermissions' }, { base: BYPASS_LAUNCH }) + + expect(spawn.sdkOptions.extraArgs).toEqual({ + 'replay-user-messages': null, + 'dangerously-skip-permissions': null + }) + expect(spawn.sdkOptions).not.toHaveProperty('permissionMode') + expect(spawn.sdkOptions).not.toHaveProperty('allowDangerouslySkipPermissions') + }) + + it('never widens the Agent Permissions setting to a saved bypass', () => { + const spawn = launched({ permissionMode: 'bypassPermissions' }) + + expect(spawn.sdkOptions.extraArgs).toEqual({ 'replay-user-messages': null }) + expect(spawn.sdkOptions).not.toHaveProperty('allowDangerouslySkipPermissions') + expect(spawn.skipped).toEqual(['permissionMode']) + }) + + // Known limit: the allow flag that would keep bypass reachable is one older CLIs reject at start. + it('starts a saved narrower mode under an Agent Permissions bypass without any bypass flag', () => { + const spawn = launched({ permissionMode: 'acceptEdits' }, { base: BYPASS_LAUNCH }) + + expect(spawn.sdkOptions.permissionMode).toBe('acceptEdits') + expect(spawn.sdkOptions).not.toHaveProperty('allowDangerouslySkipPermissions') + expect(spawn.sdkOptions.extraArgs).toEqual({ 'replay-user-messages': null }) + }) + + it('starts a saved narrower mode without the allow flag when Agent Permissions prompts', () => { + const spawn = launched({ permissionMode: 'plan' }) + + expect(spawn.sdkOptions.permissionMode).toBe('plan') + expect(spawn.sdkOptions).not.toHaveProperty('allowDangerouslySkipPermissions') + }) +}) diff --git a/src/main/claude/claude-structured-spawn-options.ts b/src/main/claude/claude-structured-spawn-options.ts new file mode 100644 index 00000000000..be341435864 --- /dev/null +++ b/src/main/claude/claude-structured-spawn-options.ts @@ -0,0 +1,166 @@ +// A chat's saved options as the Claude child's launch options. The child starts already running +// them, so its first message can be written at once instead of waiting for initialize to answer and +// a control request to apply each one. + +import type { EffortLevel, PermissionMode } from '@anthropic-ai/claude-agent-sdk' +import { decodeStructuredAgentSessionOptionValue } from '../../shared/structured-agent-session-option-codec' +import type { + ClaudeStructuredLaunch, + ClaudeStructuredSdkOptions +} from './claude-structured-launch-resolution' +import type { ClaudeSession } from './claude-structured-session-state' +import { restoredClaudeStructuredSessionOptions } from './claude-structured-options' + +const EFFORT_LEVELS: ReadonlySet = new Set([ + 'low', + 'medium', + 'high', + 'xhigh', + 'max' +]) +const PERMISSION_MODES: ReadonlySet = new Set([ + 'default', + 'acceptEdits', + 'bypassPermissions', + 'plan', + 'dontAsk', + 'auto' +]) + +/** Whether `options` launch with the Agent Permissions bypass. */ +function claudeStructuredOptionsBypassPermissions(options: ClaudeStructuredSdkOptions): boolean { + return options.extraArgs?.['dangerously-skip-permissions'] !== undefined +} + +/** `options` launched in `mode` instead of the mode they carry; never more than they allow. */ +function claudeStructuredOptionsWithPermissionMode( + options: ClaudeStructuredSdkOptions, + mode: PermissionMode +): ClaudeStructuredSdkOptions { + if (mode === 'bypassPermissions') { + return options + } + // Known limit: no switch back to bypass later; that needs the allow flag older CLIs reject. + const { 'dangerously-skip-permissions': _bypass, ...extraArgs } = options.extraArgs ?? {} + return { ...options, permissionMode: mode, extraArgs } +} + +/** The saved value stands in for the agent Arguments' own flag for it: the chat's pick wins, and + * the CLI is never handed the flag twice. */ +function withoutConfiguredFlag( + options: ClaudeStructuredSdkOptions, + flag: string +): ClaudeStructuredSdkOptions { + if (!options.extraArgs || !Object.hasOwn(options.extraArgs, flag)) { + return options + } + const { [flag]: _configured, ...extraArgs } = options.extraArgs + return { ...options, extraArgs } +} + +function isEffortLevel(value: string): value is EffortLevel { + return EFFORT_LEVELS.has(value) +} + +function isPermissionMode(value: string): value is PermissionMode { + return PERMISSION_MODES.has(value) +} + +export type ClaudeStructuredSpawnOptions = { + sdkOptions: ClaudeStructuredSdkOptions + /** The chat's options as the session holds them: what was launched, and a Fast the start applies. */ + options: Map + /** Saved options left out: the provider's own value wins and is re-persisted. */ + skipped: readonly string[] + /** A saved Fast the launch does not carry, which the start applies. */ + fastModeAtStart: boolean +} + +/** + * Every saved value is passed as the user chose it; the CLI's own answer (a turn's error, the + * settings readback) says what it ran. Only a value no Claude version can parse is left out, by + * the SDK's types, not the installed binary's: one that binary rejects fails its start. + */ +export function claudeStructuredSpawnOptions(input: { + launch: Pick + saved: Readonly> | undefined +}): ClaudeStructuredSpawnOptions { + const saved = restoredClaudeStructuredSessionOptions(input.saved) + const options = new Map() + const skipped: string[] = [] + let sdkOptions: ClaudeStructuredSdkOptions = { ...input.launch.options } + let fastModeAtStart = false + const model = saved.get('model') + if (model !== undefined) { + options.set('model', model) + sdkOptions = { ...withoutConfiguredFlag(sdkOptions, 'model'), model } + } + const effort = saved.get('effort') + if (effort !== undefined) { + if (isEffortLevel(effort)) { + options.set('effort', effort) + sdkOptions = { ...withoutConfiguredFlag(sdkOptions, 'effort'), effort } + } else { + skipped.push('effort') + } + } + const fastMode = saved.get('fastMode') + if (fastMode !== undefined) { + const decoded = decodeStructuredAgentSessionOptionValue('fastMode', fastMode) + if (typeof decoded !== 'boolean') { + skipped.push('fastMode') + } else { + options.set('fastMode', fastMode) + // A new CLI session may opt in to Fast per session, which only its settings say, and the + // agent Arguments' own `--settings` would be replaced by one carrying Fast: either way the + // start applies the saved Fast once it has read them (`applyClaudeStartFastMode`). + if ( + (!decoded || input.launch.resumesTranscript) && + !Object.hasOwn(sdkOptions.extraArgs ?? {}, 'settings') + ) { + sdkOptions.settings = { fastMode: decoded } + } else { + fastModeAtStart = true + } + } + } + const permissionMode = saved.get('permissionMode') + if (permissionMode !== undefined) { + // Bypass is the Agent Permissions setting's to grant; a saved pick never widens it. + if ( + isPermissionMode(permissionMode) && + (permissionMode !== 'bypassPermissions' || + claudeStructuredOptionsBypassPermissions(input.launch.options)) + ) { + options.set('permissionMode', permissionMode) + sdkOptions = claudeStructuredOptionsWithPermissionMode(sdkOptions, permissionMode) + } else { + skipped.push('permissionMode') + } + } + return { sdkOptions, options, skipped, fastModeAtStart } +} + +/** The published session takes on what its child was launched with. */ +export function adoptClaudeStructuredSpawnOptions( + session: Pick< + ClaudeSession, + 'restoreSkippedOptions' | 'translator' | 'launchedModel' | 'fastModeAtStart' + >, + spawn: Pick +): void { + session.launchedModel = spawn.options.get('model') ?? null + session.fastModeAtStart = spawn.fastModeAtStart + for (const key of spawn.skipped) { + session.restoreSkippedOptions.add(key) + } + const model = spawn.options.get('model') + if (model !== undefined) { + session.translator?.modelWritten(model) + } + // The journal's window was measured under a mode this child did not take (`opusplan`'s plan + // mode runs Opus); a launch never leaves a model out. + if (spawn.skipped.includes('permissionMode')) { + session.translator?.modelMayHaveChanged() + } +} diff --git a/src/main/claude/claude-structured-start-fast-mode.test.ts b/src/main/claude/claude-structured-start-fast-mode.test.ts new file mode 100644 index 00000000000..24057e865fc --- /dev/null +++ b/src/main/claude/claude-structured-start-fast-mode.test.ts @@ -0,0 +1,34 @@ +// A saved Fast the launch leaves out beside the agent Arguments' own `--settings`, applied by the +// start once the child answers. + +import { expect, it, vi } from 'vitest' +import { adapterFor, fakeClaude, identityFor } from './claude-structured-session-test-support' + +it("applies a resumed conversation's saved Fast off after start beside the Arguments' settings", async () => { + const claude = fakeClaude({ + settings: { effective: { fastMode: true, fastModePerSessionOptIn: false } } + }) + const adapter = adapterFor(claude, { + resumesTranscript: true, + continuesChain: true, + options: { extraArgs: { settings: '/repo/claude.json' } } + }) + + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + options: { fastMode: 'false' } + }) + + expect(claude.connections[0].launch.options).not.toHaveProperty('settings') + expect(claude.connections[0].launch.options.extraArgs).toEqual({ + settings: '/repo/claude.json' + }) + await vi.waitFor(() => + expect(claude.connections[0].calls.at(-1)).toEqual({ + subtype: 'apply_flag_settings', + params: { settings: { fastMode: false } } + }) + ) +}) diff --git a/src/main/claude/claude-structured-start-fast-mode.ts b/src/main/claude/claude-structured-start-fast-mode.ts new file mode 100644 index 00000000000..df997380f1a --- /dev/null +++ b/src/main/claude/claude-structured-start-fast-mode.ts @@ -0,0 +1,72 @@ +// A saved Fast the Claude child was launched without, applied once its start has read the settings +// that decide whether it may run. + +import { ClaudeControlRequestError } from './claude-agent-sdk-control-requests' +import { readClaudeModels } from './claude-structured-init-proof' +import { listedModels } from './claude-structured-model-catalog' +import { claudeModelFastModeSupport } from './claude-structured-session-options' +import type { ClaudeStartupFacts, ClaudeStartupReport } from './claude-structured-session-startup' +import type { ClaudeSession } from './claude-structured-session-state' + +/** A saved Fast the launch left out (`fastModeAtStart`): on for a new conversation, whose settings + * may opt in to it per session, or either value beside the agent Arguments' own `--settings`. + * Those settings now read: a new conversation's opt-in drops it, as before, and so do the guards a + * live Fast write takes. The value still to be applied, or null. */ +export function admitClaudeStartFastMode( + session: ClaudeSession, + facts: ClaudeStartupFacts +): string | null { + const fastMode = session.options.get('fastMode') + if (!session.fastModeAtStart || fastMode === undefined) { + return null + } + if (fastMode !== 'true') { + return fastMode + } + if (!facts.resumesTranscript && facts.prepared.fastModePerSessionOptIn === true) { + session.options.delete('fastMode') + return null + } + // Over the listing this start already holds. + const listed = listedModels({ models: readClaudeModels(facts.initialization) }) + const blocked = + session.fastModeDisabledReason !== undefined && + !['preference', 'sdk_opt_in_required'].includes(session.fastModeDisabledReason) + if ( + blocked || + (listed.length > 0 && claudeModelFastModeSupport(session, listed).supported !== true) + ) { + session.options.delete('fastMode') + session.restoreSkippedOptions.add('fastMode') + return null + } + return fastMode +} + +/** Applies a saved Fast the launch left out after `started`, so nothing waits on it. A refusal + * drops it as main's refused restore did, from the record too; silence keeps it wanted and + * unconfirmed. A write the user made meanwhile owns the option. */ +export async function applyClaudeStartFastMode( + session: ClaudeSession, + facts: ClaudeStartupFacts, + fastMode: string, + report: (event: ClaudeStartupReport) => void +): Promise { + const sequence = session.optionMutationSequence + try { + await session.connection.applyFlagSettings( + { fastMode: fastMode === 'true' }, + { timeoutMs: facts.requestTimeoutMs } + ) + } catch (error) { + if (sequence !== session.optionMutationSequence) { + return + } + session.confirmedOptions.delete('fastMode') + if (error instanceof ClaudeControlRequestError) { + session.options.delete('fastMode') + session.restoreSkippedOptions.add('fastMode') + report({ type: 'options-skipped', options: { fastMode } }) + } + } +} diff --git a/src/main/claude/claude-structured-startup-settled-test-support.ts b/src/main/claude/claude-structured-startup-settled-test-support.ts new file mode 100644 index 00000000000..721e1acda5a --- /dev/null +++ b/src/main/claude/claude-structured-startup-settled-test-support.ts @@ -0,0 +1,10 @@ +import type { ClaudeStructuredSessionAdapter } from './claude-structured-session-adapter' + +/** Resolves once the session's background startup read has landed, faulted, or been ended. */ +export async function claudeStartupSettled( + adapter: ClaudeStructuredSessionAdapter, + sessionId: string +): Promise { + // Element access reaches the adapter's private map, which no production caller needs. + await adapter['sessions'].get(sessionId)?.startup.settled +} diff --git a/src/main/claude/claude-subagent-permission-request.test.ts b/src/main/claude/claude-subagent-permission-request.test.ts index 956d89becd2..c3db6a4011b 100644 --- a/src/main/claude/claude-subagent-permission-request.test.ts +++ b/src/main/claude/claude-subagent-permission-request.test.ts @@ -36,7 +36,8 @@ import type { ClaudeStructuredSessionEvent } from './claude-structured-session-s import { fakeClaude, identityFor, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from './claude-structured-session-test-support' const SESSION = 'session-1' @@ -222,7 +223,7 @@ async function pipeline() { spawnToken: 'spawn-9', events: deferred.sink }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) const settle = async (): Promise => { for (let round = 0; round < 3; round += 1) { expect(await deferred.drained()).toEqual({ ok: true }) diff --git a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts index b52e3648b72..9e9b1f6c57c 100644 --- a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts +++ b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts @@ -38,9 +38,9 @@ export async function markJournalPendingSubmissionsUnknown( return unresolved.map((entry) => entry.clientMessageId) } -/** Settles every submission a child that never proved its start left unanswered as `rejected`: - * such a child accepted nothing, so each is provably unwritten and safe to send again. A queued - * submission was never handed to that child; the delivery loop settles it. */ +/** Settles as `rejected` every submission a child handed over and never echoed, when that child + * ended in its start: one that never answered initialize ran nothing, so each is safe to send + * again. A queued submission was never handed to that child; the delivery loop settles it. */ export async function rejectJournalPendingSubmissions( journal: AgentSessionJournal, fence: number, diff --git a/src/main/native-chat/agent-session-journal/journal-store.ts b/src/main/native-chat/agent-session-journal/journal-store.ts index 9b676cb26b3..146c141bda0 100644 --- a/src/main/native-chat/agent-session-journal/journal-store.ts +++ b/src/main/native-chat/agent-session-journal/journal-store.ts @@ -338,7 +338,7 @@ export class AgentSessionJournal { return markJournalPendingSubmissionsUnknown(this, fence, reason) } - /** Reject unanswered sends after an owner that never proved its start ended: none was written. */ + /** Reject sends a child that ended in its start was handed and never echoed: none ran. */ async rejectPendingSubmissions( fence: number, rejection: AgentJournalDispatchRejection diff --git a/src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts b/src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts index f2bf6c953f0..9341fbce6e1 100644 --- a/src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts +++ b/src/main/native-chat/agent-session-journal/journal-unsent-send-hold.ts @@ -54,6 +54,21 @@ export function unsentSendKeptAsCard( return persons ? body : null } +/** A send handed to the agent that it has neither echoed nor refused. Handed to a child that never + * answered its start, it ran nowhere. */ +export function isUnansweredHandedOverSubmission( + entry: Pick< + AgentJournalSubmission, + 'handoverRecorded' | 'dispatchState' | 'handedOverAt' | 'recovered' + > +): boolean { + return ( + !isQueuedAgentJournalSubmission(entry) && + (entry.dispatchState === 'pending' || + (entry.dispatchState === 'unknown' && entry.recovered !== true)) + ) +} + /** * Settles every send `hold` names. Each is rejected with the hold's cause in its own row, and a * kept one becomes a card in that row's transaction, so a crash between them can never leave @@ -64,17 +79,25 @@ export function unsentSendKeptAsCard( */ export async function holdUnsentSends( journal: AgentSessionJournal, - input: { fence: number; hostInstance: string; hold: UnsentSendHold } + input: { + fence: number + hostInstance: string + hold: UnsentSendHold + /** In place of the queued sends: those a child that ended before it answered its start was + * handed and never echoed. It ran none, so each is unsent as surely as a queued one. */ + unrun?: true + } ): Promise { const { hold } = input const unsent = journal .submissions() - .filter( - (entry) => - isQueuedAgentJournalSubmission(entry) && - (hold.cause === 'hostRestarted' - ? journal.wroteBeforeOpen(entry.acceptedSequence) - : (hold.which?.(entry) ?? true)) + .filter((entry) => + input.unrun + ? isUnansweredHandedOverSubmission(entry) + : isQueuedAgentJournalSubmission(entry) && + (hold.cause === 'hostRestarted' + ? journal.wroteBeforeOpen(entry.acceptedSequence) + : (hold.which?.(entry) ?? true)) ) .sort((a, b) => (a.acceptedSequence ?? 0) - (b.acceptedSequence ?? 0)) if (unsent.length === 0) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts index fb0150c7aba..28a675ee83d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts @@ -595,20 +595,22 @@ describe('what an earlier host process left behind', () => { describe('a child that exits before its message is handed over', () => { it('rejects the message with the exit reason instead of starting another child (W24)', async () => { - // Each child the loop starts dies between its start step and its handover step. - const awaitStarted = vi.fn(async (sessionId: string) => { - await host.handleAdapterEvent({ + await host.close(SESSION, 'evict') + await startHost() + // Each child the loop starts dies as its start step returns: its exit, asked for on the lane + // then, lands before the handover step. + acquire.mockImplementation(async (input) => { + const child = await spawnChild(input) + void host.handleAdapterEvent({ type: 'ended', - sessionId, - fence: store.getRecord(sessionId)!.lease.runtimeFence, + sessionId: SESSION, + fence: input.fence, acquisitionGeneration: `generation-${acquire.mock.calls.length}`, reason: 'codex app-server crashed', cause: 'unexpected-exit' }) + return child }) - adapterExtras = { awaitStarted } - await host.close(SESSION, 'evict') - await startHost() const id = await accept('hello') @@ -622,11 +624,10 @@ describe('a child that exits before its message is handed over', () => { }) }) -describe('a start whose failure the delivery loop settles before the exit is published', () => { +describe('a start that fails after it was handed messages', () => { it('keeps one row in the words its rejected messages carry', async () => { - // The adapter's startup answer and its later exit event word the same start differently. - const awaitStarted = vi.fn(async () => agentSessionFailureFact('startFailed')) - adapterExtras = { awaitStarted } + // A starting child takes both messages at once; written, neither is answered. + dispatch.mockImplementation(async () => ({ state: 'admitted' as const })) acquire.mockImplementation(async (input) => ({ ...(await spawnChild(input)), providerChildPhase: 'starting' as const @@ -636,7 +637,7 @@ describe('a start whose failure the delivery loop settles before the exit is pub const first = await accept('first') const second = await accept('second') - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(2)) await host.handleAdapterEvent({ type: 'ended', sessionId: SESSION, @@ -655,17 +656,16 @@ describe('a start whose failure the delivery loop settles before the exit is pub item.itemId.includes('start-failure') ) expect(rows).toHaveLength(1) - const words = agentSessionFailureWords(agentSessionFailureFact('startFailed'), { - surface: 'rejection', - agentName: 'Codex', - provider: 'codex' + // Worded once, from the exit's own diagnostic, for the row and every message it was handed. + const row = rows[0].body + expect(row).toMatchObject({ + failure: { kind: 'providerStartFailed', detail: { text: 'codex: config.toml is invalid' } } }) - expect(rows[0].body).toMatchObject({ text: words.reason, failure: words.rejection }) for (const id of [first, second]) { expect(await submission(id)).toMatchObject({ dispatchState: 'rejected', - reason: words.reason, - rejection: words.rejection + reason: row.kind === 'status' ? row.text : null, + rejection: row.kind === 'status' ? row.failure : null }) } }) @@ -721,22 +721,19 @@ describe('Stop withdraws what is queued', () => { expect(dispatch).not.toHaveBeenCalled() }) - it('stops a child still proving its start, and the delivery loop ends with it (W17c)', async () => { - const ended = deferred() - const awaitStarted = vi.fn(() => ended.promise) - const closeSession = vi.fn(async () => { - ended.resolve() - return true - }) - adapterExtras = { awaitStarted, closeSession } + it('stops a child still proving its start, rejecting what it was handed, and the next send starts another (W17c)', async () => { + const closeSession = vi.fn(async () => true) + adapterExtras = { closeSession } await host.close(SESSION, 'evict') await startHost() acquire.mockImplementationOnce(async (input) => ({ ...(await spawnChild(input)), providerChildPhase: 'starting' as const })) + // Written to the starting child at once; it never answers. + dispatch.mockResolvedValueOnce({ state: 'admitted' }) const id = await accept('hello') - await eventually(async () => expect(awaitStarted).toHaveBeenCalled()) + await eventually(async () => expect((await submission(id))?.handedOverAt).toBeDefined()) expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) @@ -745,25 +742,27 @@ describe('Stop withdraws what is queued', () => { reason: DISPATCH_REJECTED_CANCELLED }) expect(closeSession).toHaveBeenCalled() - // A loop still waiting on that start would swallow this send; it starts a new child instead. - awaitStarted.mockImplementation(async () => undefined) const next = await accept('after stop') await eventually(async () => expect((await submission(next))?.dispatchState).toBe('accepted')) - expect(dispatch).toHaveBeenCalledTimes(1) + expect(dispatch).toHaveBeenCalledTimes(2) }) }) describe('an eviction between acceptance and handover', () => { it('rejects the message as not sent, never leaves it in doubt (W24)', async () => { const started = deferred() - adapterExtras = { awaitStarted: () => started.promise } await host.close(SESSION, 'evict') await startHost() + acquire.mockImplementationOnce(async (input) => { + await started.promise + return spawnChild(input) + }) const id = await accept('hello') await eventually(async () => expect(acquire).toHaveBeenCalledTimes(2)) - // Between the delivery loop's start step and its handover step. - await host.close(SESSION, 'evict') + // Asked for during the delivery loop's start step, the close runs ahead of its handover step. + const closing = host.close(SESSION, 'evict') started.resolve() + await closing expect(await reopened(id)).toMatchObject({ dispatchState: 'rejected', @@ -773,20 +772,19 @@ describe('an eviction between acceptance and handover', () => { }) it('rejects a queued message behind a handed-over one, which alone stays in doubt (W24)', async () => { - const second = deferred() - const awaitStarted = vi.fn(async (): Promise => undefined) - adapterExtras = { awaitStarted } await host.close(SESSION, 'evict') await startHost() dispatch.mockResolvedValueOnce({ state: 'admitted' }) const handed = await accept('handed over') await eventually(async () => expect((await submission(handed))?.handedOverAt).toBeDefined()) - awaitStarted.mockImplementation(() => second.promise) - const queued = await accept('still queued') - await eventually(async () => expect(awaitStarted).toHaveBeenCalledTimes(2)) - - await host.close(SESSION, 'evict') + // Held, the second send is accepted and the close runs next, ahead of its handover. + const second = deferred() + void host['tasks'].serialize(SESSION, () => second.promise) + const accepting = accept('still queued') + const closing = host.close(SESSION, 'evict') second.resolve() + const queued = await accepting + await closing expect(await reopened(queued)).toMatchObject({ dispatchState: 'rejected', @@ -799,9 +797,12 @@ describe('an eviction between acceptance and handover', () => { it('does not stop an idle child while a message is still queued for it (W24)', async () => { idleMs = 0 const started = deferred() - adapterExtras = { awaitStarted: () => started.promise } await host.close(SESSION, 'evict') await startHost() + acquire.mockImplementationOnce(async (input) => { + await started.promise + return spawnChild(input) + }) const id = await accept('hello') await eventually(async () => expect(acquire).toHaveBeenCalledTimes(2)) // The idle sweep ticks every few milliseconds meanwhile. @@ -831,7 +832,6 @@ describe('a close that stops the child and then a wind-down step fails', () => { async ({ end, starting, kind, verdict }) => { const started = deferred() adapterExtras = { - awaitStarted: () => started.promise, // Once: the host's own teardown acknowledges again. acknowledgeSessionRelease: vi.fn().mockImplementationOnce(() => { throw new Error('release acknowledgement failed') @@ -839,15 +839,20 @@ describe('a close that stops the child and then a wind-down step fails', () => { } await host.close(SESSION, 'evict') await startHost() - acquire.mockImplementationOnce(async (input) => ({ - ...(await spawnChild(input)), - ...(starting ? { providerChildPhase: 'starting' as const } : {}) - })) + acquire.mockImplementationOnce(async (input) => { + await started.promise + return { + ...(await spawnChild(input)), + ...(starting ? { providerChildPhase: 'starting' as const } : {}) + } + }) const id = await accept('hello') await eventually(() => expect(acquire).toHaveBeenCalledTimes(2)) - await expect(END_CHILD[end]()).resolves.toBeUndefined() + // Asked for during the start step, the close runs ahead of the handover step. + const ending = END_CHILD[end]() started.resolve() + await expect(ending).resolves.toBeUndefined() await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) const rejected = (await submission(id))! diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts index 6f879a0fab9..ee8e1384750 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts @@ -1,4 +1,3 @@ -import type { SubmissionRejectionFact } from '../../../shared/agent-session-failure' import type { StructuredAgentSessionAtRestCommands } from './structured-agent-session-at-rest-commands' import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' import type { @@ -163,8 +162,8 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi awaitOptionWritable = (sessionId: string): Promise => this.liveOwnerOrNull(sessionId)?.awaitOptionWritable?.(sessionId) ?? Promise.resolve() - awaitStarted = (sessionId: string): Promise => - this.liveOwnerOrNull(sessionId)?.awaitStarted?.(sessionId) ?? Promise.resolve() + startAnswered = (sessionId: string): boolean | undefined => + this.liveOwnerOrNull(sessionId)?.startAnswered?.(sessionId) prepareReadOptions = (input: { sessionId: string; fence: number }) => this.liveOwnerOrNull(input.sessionId)?.prepareReadOptions?.(input) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 8acac49cb68..b05e89136f0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -205,10 +205,12 @@ export type StructuredAgentSessionEndedEvent = { observedAt?: number /** The provider ended before it finished starting, so resuming it would repeat the failure. */ startupUnproven?: true + /** The provider ended before it answered its start, so it ran nothing it was handed. */ + startupUnanswered?: true } -/** The child a publish-first acquire handed over has now proven its start: startup facts applied - * and saved options restored. What it reports from here on is fact, not a catalog guess. */ +/** The child a publish-first acquire handed over has now proven its start: startup facts applied. + * What it reports from here on is fact, not a catalog guess. */ export type StructuredAgentSessionStartedEvent = { type: 'started' sessionId: string @@ -217,16 +219,31 @@ export type StructuredAgentSessionStartedEvent = { /** What the child proved, snapshotted by the adapter from what startup already read. The host * handles this inside the session's serialized step, so it must not ask the CLI. */ reportedOptions: AgentSessionOptionsResult['current'] - /** Saved options the restore could not apply; the host drops them rather than persist them. */ + /** Saved options the child could not take; the host drops them rather than persist them. */ restoreSkippedOptions: readonly string[] + /** Values the child showed it cannot run: a report naming the same value is not persisted. */ + retiredOptions?: Readonly> +} + +/** A running child showed saved options it cannot run, as a model the provider reports missing. + * The record drops them, so the next start uses the provider's own. */ +export type StructuredAgentSessionOptionsSkippedEvent = { + type: 'options-skipped' + sessionId: string + fence: number + acquisitionGeneration: string + /** Each saved value the child showed it cannot run; a record holding another value keeps it. */ + options: Readonly> } export type StructuredAgentSessionLifecycleEvent = | StructuredAgentSessionEndedEvent | StructuredAgentSessionStartedEvent + | StructuredAgentSessionOptionsSkippedEvent /** Whether the provider child behind an acquisition has proven its start. A publish-first - * acquire hands over a `starting` child and the `started` lifecycle event flips it. */ + * acquire hands over a `starting` child, which already takes input, and the `started` lifecycle + * event flips it. */ export type StructuredAgentSessionProviderChildPhase = 'starting' | 'ready' export type StructuredAgentSessionAcquireInput = { @@ -365,10 +382,9 @@ export type StructuredAgentSessionAdapter = StructuredAgentSessionAdapterStop & ): Promise>> /** Resolves once a live session can take an option write, or after a bound; never rejects. */ awaitOptionWritable?(sessionId: string): Promise - /** Resolves once a session published before it proved its start has proven it, failed, or been - * closed; at once for any other. A start that did not land resolves with the chat's words for - * why. Never rejects. */ - awaitStarted?(sessionId: string): Promise + /** False while the live child has not answered its start, so it has run nothing it was handed. + * Absent or undefined reads as answered. */ + startAnswered?(sessionId: string): boolean | undefined /** Fetch off the lane, then apply the result under the same child's fence. */ prepareReadOptions?(input: { sessionId: string diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts index edfe42f6d85..12ced4eacf8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts @@ -102,8 +102,9 @@ export function mutateWithChatStop( return { ok: true, value: { ...named, cancelled: await withdrew } } } if (child?.phase === 'starting') { - // A start that may never land is the one thing here Stop has to end; the chat stays. - // The event is issued first and lands behind the withdrawal, in the journal's queue order. + // A start that may never land takes no interrupt, so Stop ends it; the chat stays. Its end + // settles what the child was handed as stopped (`unrunRejection`). The event is + // issued first and lands behind the withdrawal, in the journal's queue order. const effect = tookEffect() try { await stopChild() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts index 0022df7c156..524b4fd4d3f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts @@ -25,10 +25,16 @@ export async function joinStructuredAgentSessionChildClose( sessionId: string, child: StructuredAgentSessionProviderChild ): Promise { + // Read before the close: once the child is gone the provider can no longer say. + const unanswered = context.deps.adapter.startAnswered?.(sessionId) === false if (!(await closeProviderRoot(context, sessionId))) { return 'unverifiable' } - await context.endExitedChild(sessionId, child, { expected: true, reason: 'closed by Orca' }) + await context.endExitedChild(sessionId, child, { + expected: true, + reason: 'closed by Orca', + ...(unanswered ? { startupUnanswered: true } : {}) + }) context.restartWitness?.stopped(sessionId) return 'exited' } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts index c606a73e310..45fad626ed1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts @@ -3,7 +3,10 @@ // every step after it is bookkeeping: each is attempted and reported, none keeps the child on // record, and the record ends in `finally`. `expected` changes only what the chat is told. -import type { SubmissionRejectionFact } from '../../../shared/agent-session-failure' +import { + agentSessionFailureFact, + type SubmissionRejectionFact +} from '../../../shared/agent-session-failure' import { PROVIDER_EXIT_ROW_PREFIX } from '../../../shared/agent-session-stop-row-identity' import { structuredAgentSessionFailureWordsContext } from './structured-agent-session-send-preparation' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -37,6 +40,7 @@ export type StructuredAgentSessionChildExit = { /** Host receipt of the exit: the end time of a turn it interrupted. */ observedAt?: number startupUnproven?: true + startupUnanswered?: true } export type StructuredAgentSessionChildExitSession = Pick< @@ -57,6 +61,13 @@ export type StructuredAgentSessionChildExitContext< serialize: (sessionId: string, task: () => Promise) => Promise now: () => number logger: StructuredAgentSessionLogger + /** Settles what a child that ended before it answered its start was handed and never echoed, as + * the chat settles a queued send for the same end (`holdUnsentSends`). */ + holdUnrunSends?: ( + sessionId: string, + fence: number, + cause: 'chatClosed' | 'hostRestarted' + ) => Promise /** Lets the child's sink and the adapter's route for it go; absent leaves both to the next attach. */ route?: { runtimeState: Pick @@ -81,7 +92,8 @@ export function settleStructuredAgentSessionChildExit< reason: event.reason, ...(event.failure ? { failure: event.failure } : {}), ...(event.observedAt === undefined ? {} : { observedAt: event.observedAt }), - ...(event.startupUnproven ? { startupUnproven: event.startupUnproven } : {}) + ...(event.startupUnproven ? { startupUnproven: event.startupUnproven } : {}), + ...(event.startupUnanswered ? { startupUnanswered: event.startupUnanswered } : {}) }) }) } @@ -106,6 +118,29 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< // The host's own phase decides, so a provider that omits the flag still gets a start that // failed told as one: the row says so. const exitedDuringStartup = exit.startupUnproven === true || child.phase === 'starting' + // A close during startup settles what the child was handed, never echoed, by who asked: a + // person's Stop stops it; the host's stop fails the start, as an exit of its own would. + const startClose = expected && exitedDuringStartup ? close?.cause : undefined + const startFailure = expected + ? startClose === 'host-stop' + ? agentSessionFailureFact('hostStopped') + : undefined + : exit.failure + // A send the child was handed and never echoed cannot have run when a person's Stop ended its + // start, or any close ended it before it answered its start: settled as the chat settles a queued + // send for the same end. A Stop withdraws it as cancelled; a quit or close keeps a person's + // message as a held card. A host stop fails the start (`startFailure`). + const unrunRejection = + startClose === 'user-stop' ? agentSessionFailureFact('cancelled') : undefined + const unrunHold = + expected && + exit.startupUnanswered && + close?.cause !== 'host-stop' && + close?.cause !== 'user-stop' + ? close?.quit + ? ('hostRestarted' as const) + : ('chatClosed' as const) + : undefined const endChild = (): void => { endProviderChild(session, { generation: child.generation, @@ -138,7 +173,16 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< } catch (error) { logExitFailure(context, sessionId, 'exit-lifecycle-barrier', error) } + if (unrunHold && context.holdUnrunSends) { + await context + .holdUnrunSends(sessionId, child.fence, unrunHold) + .catch((error: unknown) => logExitFailure(context, sessionId, 'exit-unrun-hold', error)) + } const unfinishedWork = captureUnfinishedStructuredAgentSessionWork(session.journal) + // The host's stop fails only a start something was handed to; an idle one goes quietly. + const startFailed = + exitedDuringStartup && + (!expected || (startClose === 'host-stop' && unfinishedWork.hadUnsettledSubmissions)) // Folded before the fallback's end is built, so the end reads it (`turnEndAfterStop`). await close?.recorded const generation = child.generation ?? 'unknown' @@ -156,17 +200,18 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< failureTextContext: structuredAgentSessionFailureWordsContext(record, session.journal), // A failed start always says why: no response was running to carry the reason. showUnexpectedExitOutcome: - !expected && - (exitedDuringStartup || + startFailed || + (!expected && unfinishedStructuredAgentSessionWorkWasInterrupted( unfinishedWork, session.journal, observedAt )), - ...(!expected && exit.failure ? { exitFailure: exit.failure } : {}), - ...(!expected && exitedDuringStartup && child.generation + ...(startFailure ? { exitFailure: startFailure } : {}), + ...(startFailed && child.generation ? { exitedDuringStartup: { generation: child.generation } } - : {}) + : {}), + ...(unrunRejection ? { unrunRejection } : {}) }) if (!settled.ok) { logExitFailure(context, sessionId, 'exit-settlement', settled.error) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.ts index cfb7dfd5483..861e7fc7b9f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-compact-stop.test.ts @@ -12,7 +12,8 @@ import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-s import { fakeClaude, PROVIDER_SESSION_ID, - type FakeConnection + type FakeConnection, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -92,7 +93,7 @@ beforeEach(async () => { }) ) expect(attached).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) }) afterEach(async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts index de44f4f8a2b..33fe1fe5751 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts @@ -12,7 +12,8 @@ import { isStructuredAgentSessionMainAgentWorking } from '../../../shared/struct import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { fakeClaude, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -84,7 +85,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts new file mode 100644 index 00000000000..595e1817754 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts @@ -0,0 +1,223 @@ +// A Claude CLI that never answers initialize still takes the chat's message at once, and the +// chat's Stop still ends it. On the shipping adapter and host, against a CLI that echoes nothing. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { DISPATCH_REJECTED_CANCELLED } from '../../../shared/structured-agent-session-dispatch-rejection' +import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-live-turn' +import { isStructuredAgentSessionMainAgentWorking } from '../../../shared/structured-agent-session-main-agent-working' +import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' +import { + fakeClaude, + PROVIDER_SESSION_ID +} from '../../claude/claude-structured-session-test-support' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { structuredClaudeLifecycleEvent } from '../../runtime/structured-claude-runtime-adapter' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { claudeAndCodexDeclared } from './structured-agent-session-adapter-router-test-support' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + hostTestAttachParams, + hostTestMessage, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' + +const CALLER = { callerKey: 'client-1' } + +let root: string +let host: StructuredAgentSessionHost +let adapter: ClaudeStructuredSessionAdapter +let store: AgentSessionRecordStore +let claude: ReturnType +/** Answers initialize, which the CLI otherwise never does; it echoes nothing either way. */ +let answerInitialize: () => void = () => {} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-claude-hung-start-stop-')) + resetHostTestOperationIds() + // Alive, and silent: no initialize answer, no start frame, no echo of what it is sent. + claude = fakeClaude({ initProof: 'none', replayUuid: null }) + const openConnection = claude.openConnection + adapter = new ClaudeStructuredSessionAdapter({ + resolveLaunch: async () => ({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: root, + claudeConfigDir: join(root, 'claude-home'), + providerSessionId: PROVIDER_SESSION_ID, + resumeLeafUuid: null, + resumesTranscript: false, + continuesChain: false + }), + onEvent: (event) => { + const mapped = structuredClaudeLifecycleEvent(event) + if (mapped) { + void host.handleAdapterEvent(mapped) + } + }, + onDispatchSettledLate: (settlement) => void host.settleLateDispatch(settlement), + openConnection: async (...args) => { + const connection = await openConnection(...args) + const fake = claude.connections.at(-1) + connection.initializationResult = () => { + fake?.calls.push({ subtype: 'initialize' }) + return new Promise((resolve) => { + answerInitialize = () => resolve({ models: [] }) + }) + } + return connection + }, + readProcessStartTime: async () => 1_700_000_000_000, + now: () => NOW + }) + store = await openTestAgentSessionRecordStore(root) + host = new StructuredAgentSessionHost({ + agents: claudeAndCodexDeclared(), + store, + adapter: Object.assign(adapter, { supportsCreate: () => true }), + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-a', + logger: createStructuredAgentSessionLogger(), + now: () => NOW + }) + const params = hostTestAttachParams(null, { + provider: 'claude', + agent: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: join(root, 'claude-home') }, + providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } + }) + expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) +}) + +afterEach(async () => { + await adapter.closeAll() + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +function envelope( + method: 'agentSession.send' | 'agentSession.cancel', + fields: Parameters[0]['fields'] +) { + return { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: store.getRecord(SESSION)!.lease.runtimeFence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }) + } +} + +/** The one rule every session list's Working and the chat's own Stop read. */ +async function readsWorking(): Promise { + const snapshot = await host.journalSnapshot(SESSION) + return isStructuredAgentSessionMainAgentWorking( + activeStructuredAgentSessionTurnId(snapshot.items), + snapshot.submissions, + store.getRecord(SESSION)!.lease.runtimeFence + ) +} + +it('writes the message to a start that never answers, and a Stop ends it: stopped, nothing working', async () => { + expect(host['sessions'].get(SESSION)?.child?.phase).toBe('starting') + const body = hostTestMessage('hello') + const sent = await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) + if (!sent.ok) { + throw new Error('send refused') + } + const id = sent.value.clientMessageId + const [connection] = claude.connections + + // Handed over and written while initialize is still unanswered. + await vi.waitFor(() => + expect(connection!.sent.some((message) => JSON.stringify(message).includes('hello'))).toBe(true) + ) + expect(connection!.calls).toEqual([{ subtype: 'initialize' }]) + expect(host['sessions'].get(SESSION)?.child?.phase).toBe('starting') + expect(await readsWorking()).toBe(true) + + const stopped = await host.cancel(CALLER, { envelope: envelope('agentSession.cancel', {}) }) + + expect(stopped).toMatchObject({ ok: true, value: { cancelled: true } }) + await vi.waitFor(() => expect(host['sessions'].get(SESSION)?.child).toBeNull()) + expect(connection!.closeCount).toBeGreaterThan(0) + // Stopped, not in doubt: the child it was written to is gone and never echoed it. + await vi.waitFor(async () => + expect( + (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === id + ) + ).toMatchObject({ dispatchState: 'rejected', reason: DISPATCH_REJECTED_CANCELLED }) + ) + expect(await readsWorking()).toBe(false) +}) + +async function sendHello(): Promise { + const body = hostTestMessage('hello') + // A person's send, which a close keeps as a held card. + const sent = await host.send(CALLER, { + envelope: envelope('agentSession.send', { body }), + body, + userSend: true + }) + if (!sent.ok) { + throw new Error('send refused') + } + await vi.waitFor(() => + expect( + claude.connections[0]!.sent.some((message) => JSON.stringify(message).includes('hello')) + ).toBe(true) + ) + return sent.value.clientMessageId +} + +async function submission(id: string) { + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === id + ) +} + +// A CLI that never answered initialize ran nothing it was handed: a close settles it as it does a +// queued send, keeping the person's words as a held card. +it.each(['user-close', 'evict'] as const)( + 'keeps a message written to a start that never answered as a held card when a %s ends it', + async (cause) => { + const id = await sendHello() + + await host.close(SESSION, cause) + + await vi.waitFor(async () => + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + reason: 'The chat closed before this message was sent.', + keptAsQueuedMessageId: id + }) + ) + } +) + +// Once it answered, the CLI may have taken the message without echoing it yet: a close leaves it in +// doubt, as for any running child. +it('leaves a message in doubt when the start answered and the chat closes before the echo', async () => { + answerInitialize() + await adapter['sessions'].get(SESSION)?.startup.settled + const id = await sendHello() + + await host.close(SESSION, 'user-close') + + await vi.waitFor(async () => + expect(await submission(id)).toMatchObject({ dispatchState: 'unknown' }) + ) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts index 6d4952670a4..e6e2524841e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts @@ -14,12 +14,14 @@ import { projectStructuredAgentSessionStatus } from '../../../shared/structured- import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { fakeClaude, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' import { structuredClaudeLifecycleEvent } from '../../runtime/structured-claude-runtime-adapter' import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import { holdDelivery } from './structured-agent-session-delivery-hold.test-fixture' import { StructuredAgentSessionHost } from './structured-agent-session-host' import { HOST_TEST_NOW as NOW, @@ -100,7 +102,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) }) @@ -253,22 +255,19 @@ it('withdraws a follow-up still queued on the host when the turn the Stop names const connection = claude.connections[0]! const turnId = await openFirstTurn(connection) await endFirstTurn(connection) - // Holds the delivery loop between its start check and the handover, with the follow-up queued. - let release!: () => void - const held = new Promise((resolve) => (release = resolve)) - const awaitStarted = vi.spyOn(adapter, 'awaitStarted').mockImplementationOnce(async () => { - await held - }) + // Holds the delivery loop ahead of the handover, with the follow-up queued. + const { held, release } = holdDelivery() const followUp = await send('And then this.') - await eventually(() => expect(awaitStarted).toHaveBeenCalled()) + await held const submission = (await host.journalSnapshot(SESSION)).submissions.find( (entry) => entry.clientMessageId === followUp ) expect(submission && isQueuedAgentJournalSubmission(submission)).toBe(true) // Nothing reached Claude, so the host's withdrawal is the whole Stop, as with no turn named. - expect(await stop(turnId)).toMatchObject({ ok: true, value: { cancelled: true } }) + const stopped = stop(turnId) release() + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: true } }) await eventually(async () => expect(await dispatch(followUp)).toEqual({ state: 'rejected', @@ -297,16 +296,13 @@ it('withdraws a host-queued follow-up but leaves a newer turn running when the S const newerTurnId = await liveTurnId() expect(newerTurnId).not.toBeNull() expect(newerTurnId).not.toBe(olderTurnId) - let release!: () => void - const held = new Promise((resolve) => (release = resolve)) - const awaitStarted = vi.spyOn(adapter, 'awaitStarted').mockImplementationOnce(async () => { - await held - }) + const { held, release } = holdDelivery() const followUp = await send('And then this.') - await eventually(() => expect(awaitStarted).toHaveBeenCalled()) + await held - expect(await stop(olderTurnId)).toMatchObject({ ok: true, value: { cancelled: false } }) + const stopped = stop(olderTurnId) release() + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: false } }) await eventually(async () => expect(await dispatch(followUp)).toEqual({ state: 'rejected', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-ends-session.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-ends-session.test.ts index ed06f809c6d..9e55397a38f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-ends-session.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-ends-session.test.ts @@ -133,7 +133,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await adapter['sessions'].get(SESSION)?.startup.settled await Promise.all(lifecycle) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts index afb4ac1c1c1..00a0644136c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts @@ -13,7 +13,8 @@ import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-s import { fakeClaude, PROVIDER_SESSION_ID, - type FakeConnection + type FakeConnection, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -102,7 +103,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.ts index cd7ccb1cab4..7b005ffb817 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-note-end.test.ts @@ -12,6 +12,7 @@ import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-a import { claudeUnwrittenUserMessageError } from '../../claude/claude-agent-sdk-user-message-queue' import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { + claudeStartupSettled, fakeClaude, PROVIDER_SESSION_ID, type FakeConnection @@ -108,7 +109,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) await host.subscribe({ id: 'stop-note-live', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-turn-end.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-turn-end.test.ts index 6107c402b88..6221c2ae885 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-turn-end.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-turn-end.test.ts @@ -14,7 +14,8 @@ import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-s import { fakeClaude, PROVIDER_SESSION_ID, - type FakeConnection + type FakeConnection, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -87,7 +88,7 @@ beforeEach(async () => { providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } }) expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index e51b8e672f9..e0130423d5e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -210,8 +210,8 @@ export async function handOverStructuredAgentSessionCommand( command: { clientMessageId, ...turn, running } }) } catch (error) { - // A child that had not proven its start took nothing, so the command provably did not run. Any - // other throw is a lost reply: the command may have run. + // A child still starting throws only for a start that failed before the write, so the command + // provably did not run. Any other throw is a lost reply: the command may have run. const unsent = ctx.providerChildPhase?.() === 'starting' ? { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts index 0133fbc9fed..e19127fde7c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts @@ -2,7 +2,6 @@ // list and a send each see only the one that happened. Ticks are driven by hand. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { AgentSessionStatusSummary } from '../../../shared/agent-session-wire' import { readStructuredSessionGateFacts } from '../../runtime/orchestration/structured-mailbox-pointer-host' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' @@ -222,38 +221,22 @@ describe('the sweep and the lease (P2-20)', () => { }) describe('a start that never finishes (P2-15)', () => { - it('is stopped by the sweep, and the delivery loop alone writes its one row and rejection', async () => { + it('is stopped by the sweep, and the message it was handed carries its one row and rejection', async () => { const stopReason = 'Codex never finished starting, so Orca stopped it.' - const order: string[] = [] - const started = Promise.withResolvers() - rig.adapter.closeSession.mockImplementation(async () => { - order.push('stopped') - started.resolve() - return true - }) const spawn = rig.adapter.acquire.getMockImplementation()! rig.adapter.acquire.mockImplementationOnce(async (input) => ({ ...(await spawn(input)), providerChildPhase: 'starting' as const })) - Object.assign(rig.host.deps.adapter, { awaitStarted: () => started.promise }) - // The loop rejects the queued messages in the same append as its row. - const append = AgentSessionJournal.prototype.appendLifecycleBatch - vi.spyOn(AgentSessionJournal.prototype, 'appendLifecycleBatch').mockImplementation(function ( - this: AgentSessionJournal, - ...args - ) { - if (args[0].rejectsQueued) { - order.push(`rejected: ${args[0].rejectsQueued.reason}`) - } - return append.apply(this, args) - }) + // Written to the starting child at once; it never answers. + rig.adapter.dispatch.mockResolvedValueOnce({ state: 'admitted' }) const reader = collectSubscriber() const attached = await rig.host.attach(CALLER, hostTestAttachParams(null)) expect(attached.ok).toBe(true) await rig.host.subscribe({ id: 'reader', sessionId: SESSION, emit: reader.emit }) const sent = await rig.host.send(CALLER, restTestSend('stuck behind the start', fence())) expect(sent.ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledOnce()) rig.clock.now += IDLE_MS + 1 await sweepOnce(rig.host) @@ -263,8 +246,6 @@ describe('a start that never finishes (P2-15)', () => { expect.objectContaining({ dispatchState: 'rejected', reason: stopReason }) ) ) - // One rejection, written after the stop by the loop, with the stop's own words. - expect(order).toEqual(['stopped', `rejected: ${stopReason}`]) const errorRows = reader.events.flatMap((event) => event.type === 'batch' ? event.batch.items.filter( @@ -275,11 +256,10 @@ describe('a start that never finishes (P2-15)', () => { expect(errorRows.map((item) => (item.body.kind === 'status' ? item.body.text : null))).toEqual([ stopReason ]) - expect(rig.adapter.dispatch).not.toHaveBeenCalled() const again = await rig.host.send(CALLER, restTestSend('try again', fence())) expect(again.ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledOnce(), COLD_START) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2), COLD_START) await expect(sweepOnce(rig.host)).resolves.toBeUndefined() }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts index 4033c70de1c..2e5c30dd4af 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts @@ -89,8 +89,9 @@ export function createStructuredAgentSessionConversationLifetime(host: { providerHoldsDispatch: (sessionId) => deps().adapter.holdsDispatch?.(sessionId) === true, // The host puts an idle agent to rest: a turn it cuts short is news, not the user's Stop. stopAgent: (sessionId) => stopAgent(sessionId, { cause: 'evict', resting: true }), - // A host stop: the delivery loop waiting on this child writes the one error row and rejects - // what is queued with it, both worded from the hostStopped fact. + // A host stop: the child's end rejects what it was handed and writes the one error row, and + // the delivery loop rejects what is still queued under the same row, all worded from the + // hostStopped fact. stopStartingAgent: (sessionId) => stopStructuredAgentSessionAgentUnderSerialize(host.context(), sessionId, { cause: 'host-stop' diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts index cb3a1bbe583..f75534ecbab 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts @@ -16,6 +16,7 @@ import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-rec import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { holdLane } from './structured-agent-session-delivery-hold.test-fixture' import { HOST_TEST_NOW as NOW, HOST_TEST_SESSION as SESSION, @@ -46,7 +47,6 @@ let store: AgentSessionRecordStore let host: StructuredAgentSessionHost let dispatch: Mock let cancelTurn: Mock -let awaitStarted: Mock> let closeSession: Mock> let setOption: Mock let acknowledgeSessionRelease: Mock< @@ -67,7 +67,6 @@ beforeEach(async () => { // Admitted, not accepted: the message is written and its turn has not opened. dispatch = vi.fn(async () => ({ state: 'admitted' as const })) cancelTurn = vi.fn(async () => ({ cancelled: true })) - awaitStarted = vi.fn(async () => undefined) closeSession = vi.fn(async () => true) setOption = vi.fn(async () => undefined) acknowledgeSessionRelease = vi.fn() @@ -99,7 +98,6 @@ beforeEach(async () => { } }, dispatch, - awaitStarted, closeSession, acknowledgeSessionRelease, releaseAcquisition: vi.fn(async () => true), @@ -343,14 +341,14 @@ describe('a Stop that names no turn', () => { }) it('withdraws what is queued on a ready child and asks the provider for nothing more', async () => { - const started = Promise.withResolvers() - awaitStarted.mockImplementationOnce(() => started.promise) + // Held, the send is accepted and the Stop runs next, ahead of the handover the send asks for. + const release = holdLane(host, SESSION) const { id, result } = send('hello') + const stopped = stop() + release() await result - await eventually(() => expect(awaitStarted).toHaveBeenCalled()) - expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) - started.resolve(undefined) + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: true } }) expect(await submission(id)).toMatchObject({ dispatchState: 'rejected', @@ -570,21 +568,20 @@ describe('a Stop that names its turn, as an older client sends it', () => { expect(await statusRows()).toEqual([]) }) - async function queueOnHost(): Promise<{ id: string; release: () => void }> { - const started = Promise.withResolvers() - awaitStarted.mockImplementationOnce(() => started.promise) - const { id, result } = send('hello') - await result - await eventually(() => expect(awaitStarted).toHaveBeenCalled()) - return { id, release: () => started.resolve(undefined) } + /** A send queued on the host and not yet handed over: the lane is held until `release`, so a + * step asked for before it runs ahead of the handover. */ + function queueOnHost(): { id: string; release: () => void } { + const release = holdLane(host, SESSION) + return { id: send('hello').id, release } } it('reports success with no row when it withdrew a queued message and the turn had ended', async () => { - const queued = await queueOnHost() + const queued = queueOnHost() cancelTurn.mockResolvedValueOnce({ cancelled: false }) - expect(await stop('turn-1')).toMatchObject({ ok: true, value: { cancelled: true } }) + const stopped = stop('turn-1') queued.release() + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: true } }) expect(await submission(queued.id)).toMatchObject({ dispatchState: 'rejected' }) expect(await statusRows()).toEqual([]) @@ -592,14 +589,15 @@ describe('a Stop that names its turn, as an older client sends it', () => { // Codex refuses an interrupt for a turn that has ended. it('reports success with no row when the provider refused a turn that had ended', async () => { - const queued = await queueOnHost() + const queued = queueOnHost() cancelTurn.mockResolvedValueOnce({ cancelled: false, refusal: { detail: { text: 'no such turn', audience: 'person' } } }) - expect(await stop('turn-1')).toMatchObject({ ok: true, value: { cancelled: true } }) + const stopped = stop('turn-1') queued.release() + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: true } }) expect(await submission(queued.id)).toMatchObject({ dispatchState: 'rejected' }) expect(await statusRows()).toEqual([]) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts index 071de4b4157..b9217f948af 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts @@ -127,6 +127,8 @@ export async function settleStructuredAgentSessionDeadGeneration(input: { /** The provider never finished starting: the start that failed, keyed by the child's * generation. Its row is the one the delivery loop writes for the same start. */ exitedDuringStartup?: { generation: string | null } + /** A person's Stop ended a starting child before what it was handed could run: each is rejected so. */ + unrunRejection?: SubmissionRejectionFact }): Promise { try { const hasUnfinishedWork = hasUnfinishedStructuredAgentSessionWork(input.journal) @@ -134,18 +136,21 @@ export async function settleStructuredAgentSessionDeadGeneration(input: { if (!showUnexpectedExitOutcome && !hasUnfinishedWork) { return { ok: true } } - // A queued message is the delivery loop's to settle: it was never handed to this child. A - // child that never proved its start accepted nothing either — input is written only after it - // initializes — so every send it was handed is rejected with the child's own diagnostic. A - // proven child's handed-over sends stay in doubt. + // A queued message is the delivery loop's to settle: it was never handed to this child. A send + // a child still starting was handed and never echoed did not run, and its root is gone: it is + // rejected, with the child's own diagnostic or as the Stop that ended it. A proven child's + // handed-over sends stay in doubt. const startupFailure = input.exitedDuringStartup ? structuredAgentSessionStartFailure({ exit: input.exitFailure }, input.failureTextContext) : null - if (!startupFailure) { + const closed = input.unrunRejection + const unrun = + startupFailure ?? (closed && agentSessionFailureWords(closed, { surface: 'rejection' })) + if (!unrun) { await withdrawCodexSendsNoTurnOpenedFor(input.journal, input.fence) } - await (startupFailure - ? input.journal.rejectPendingSubmissions(input.fence, startupFailure) + await (unrun + ? input.journal.rejectPendingSubmissions(input.fence, unrun) : input.journal.markPendingSubmissionsUnknown(input.fence, input.pendingSubmissionReason)) const items = input.journal.snapshot().items const mutations: JournalLifecycleMutationInput[] = [] diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-hold.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-hold.test-fixture.ts new file mode 100644 index 00000000000..33f0f6a2059 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-hold.test-fixture.ts @@ -0,0 +1,37 @@ +// Ways a test holds a session's work at a chosen step, so a Stop or close asked for meanwhile runs +// in a known order against the delivery loop's handover. + +import { vi } from 'vitest' +import { StructuredAgentSessionDeliveryLoop } from './structured-agent-session-delivery-loop' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' + +/** Holds the delivery loop's next step as it begins, inside the session's lane, until `release`: + * a Stop or close asked for meanwhile runs ahead of the handover. `held` resolves once the step + * has reached the hold. */ +export function holdDelivery(): { held: Promise; release: () => void } { + const gate = Promise.withResolvers() + const reached = Promise.withResolvers() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: `prepare` is a prototype method whose privacy is compile-time only; the spy calls it unchanged. + const loop = StructuredAgentSessionDeliveryLoop.prototype as unknown as { + prepare: (...args: unknown[]) => Promise + } + const prepare = loop.prepare + const step = vi.spyOn(loop, 'prepare').mockImplementation(async function ( + this: unknown, + ...args: unknown[] + ) { + step.mockRestore() + reached.resolve() + await gate.promise + return prepare.apply(this, args) + }) + return { held: reached.promise, release: () => gate.resolve() } +} + +/** Holds the session's lane until the returned release: whatever is asked for meanwhile runs in + * the order it was asked, ahead of a handover the delivery loop asks for after it. */ +export function holdLane(host: StructuredAgentSessionHost, sessionId: string): () => void { + const held = Promise.withResolvers() + void host['tasks'].serialize(sessionId, () => held.promise) + return () => held.resolve() +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts index 5ba5e8f9539..8c28932baf8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts @@ -15,10 +15,7 @@ import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' -import { - agentSessionFailureFact, - type SubmissionRejectionFact -} from '../../../shared/agent-session-failure' +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import type { AgentSessionFailureWordsContext } from '../../../shared/agent-session-failure-words' import { holdUnsentSends } from '../agent-session-journal/journal-unsent-send-hold' import { structuredAgentSessionHostInstance } from './structured-agent-session-queued-pause' @@ -124,11 +121,8 @@ export class StructuredAgentSessionDeliveryLoop { ) return } - // A child published before it proved its start takes no input yet; waited for outside - // the queue so a Stop can reach it meanwhile. - const failure = await this.deps.adapter.awaitStarted?.(sessionId) const handed = await this.deps.serialize(sessionId, () => - this.handOver(sessionId, prepared.awaited, failure || null) + this.handOver(sessionId, prepared.awaited) ) if (handed === 'stop') { return @@ -203,8 +197,7 @@ export class StructuredAgentSessionDeliveryLoop { private async handOver( sessionId: string, - awaited: StructuredAgentSessionProviderChildIdentity | null, - startFailure: SubmissionRejectionFact | null + awaited: StructuredAgentSessionProviderChildIdentity | null ): Promise { const session = this.deps.sessions.get(sessionId) if (!session || this.disposed) { @@ -217,10 +210,9 @@ export class StructuredAgentSessionDeliveryLoop { child && awaited && child.generation === awaited.generation && child.fence === awaited.fence ? child : null - // The host's `starting` trails the adapter's `started` by one serialized step, so for the child - // waited on, the adapter's own answer decides whether its start landed. - if (!awaitedChild || (awaitedChild.phase === 'starting' && startFailure !== null)) { - // The child waited on is gone, replaced by another, or settled its start without proving it. + // A child still starting takes input: the provider queues it behind its own start. + if (!awaitedChild) { + // The child started for this message is gone or replaced by another. const ended = awaitedChild ? undefined : session.lastEndedChild const endedFailure = ended ? structuredAgentSessionEndedChildFailure(ended) : undefined // A user's Stop or close is not a failure: the next step starts, or waits on, a child for @@ -232,12 +224,12 @@ export class StructuredAgentSessionDeliveryLoop { startKey: awaited?.generation ?? null, cause: endedFailure ?? // Gone with no end observed: nothing says the provider stopped. - { failure: startFailure ?? agentSessionFailureFact('startFailed') } + { failure: agentSessionFailureFact('startFailed') } }) } // Never steer into a turn a person's Stop is ending: the message runs after it, as its own // turn, and the turn's end is a commit that wakes the loop again. Read here, at the handover, - // because a Stop can land while this step waits on the child's start. + // because a Stop can take the lane between the step that judged the send and this one. if (this.deps.stopping(sessionId)) { return this.stop(sessionId) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts index 398d04873d4..bf4c74bd308 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts @@ -1,5 +1,7 @@ import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import type { StructuredAgentSessionLifecycleEvent } from './structured-agent-session-adapter' +import { holdUnsentSends } from '../agent-session-journal/journal-unsent-send-hold' +import { structuredAgentSessionHostInstance } from './structured-agent-session-queued-pause' import { stopAgentSessionProviderRoot } from './structured-agent-session-provider-exit-proof' import type { StructuredAgentSessionHostDeps, @@ -7,7 +9,10 @@ import type { StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' import type { StructuredAgentSessionSinkBarrier } from './structured-agent-session-event-sink' -import { settleStructuredAgentSessionProviderStarted } from './structured-agent-session-provider-started' +import { + settleStructuredAgentSessionOptionsSkipped, + settleStructuredAgentSessionProviderStarted +} from './structured-agent-session-provider-started' import { endExitedStructuredAgentSessionChildUnderSerialize, settleStructuredAgentSessionChildExit, @@ -39,6 +44,17 @@ export class StructuredAgentSessionEventRecovery { return { ...this.context, logger: deps.logger, + holdUnrunSends: async (sessionId, fence, cause) => { + const journal = this.context.sessions.get(sessionId)?.journal + if (journal) { + await holdUnsentSends(journal, { + fence, + hostInstance: structuredAgentSessionHostInstance(), + hold: { cause }, + unrun: true + }) + } + }, route: { runtimeState, acknowledgeRelease: (sessionId) => deps.adapter.acknowledgeSessionRelease?.(sessionId) @@ -95,6 +111,9 @@ export class StructuredAgentSessionEventRecovery { if (event.type === 'started') { return settleStructuredAgentSessionProviderStarted(this.context, event) } + if (event.type === 'options-skipped') { + return settleStructuredAgentSessionOptionsSkipped(this.context, event) + } await settleStructuredAgentSessionChildExit(this.exitContext, event) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts index fda7bc9c27e..84dec78d071 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts @@ -114,6 +114,7 @@ export async function stopStructuredAgentSessionAgentUnderSerialize( : Promise.resolve(null) child.close = { cause, + ...('quit' in ending && ending.quit ? { quit: true as const } : {}), reason: ('reason' in ending ? ending.reason : undefined) ?? null, recorded, requestedAt: session.journal.cursor() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts index c1eee4eabc9..2230efa15c9 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts @@ -52,6 +52,8 @@ export type StructuredAgentSessionProviderChildIdentity = { export type StructuredAgentSessionChildClose = { /** The first stop's, which the child's end keeps however many asks join it. */ readonly cause: StructuredAgentSessionStopCause + /** Asked for by quit, whose leftovers the next open settles as a crash's. */ + readonly quit?: true readonly reason: string | null /** The Stop event that stop wrote, folded before the work it ends is settled, with the settle a * person's close that named no turn opens; a repeated ask reopens it. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts index 9cb6dbab4ef..7de8ad28581 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts @@ -15,6 +15,7 @@ import { REST_TEST_SESSION as SESSION, REST_TEST_THREAD as THREAD, restTestSend, + sweepOnce, sweepTicks, type RestTestRig } from './structured-agent-session-rest-test-rig' @@ -96,20 +97,25 @@ describe('the idle sweep', () => { it('never stops an agent a message is queued for, and hands the message over (P2-08)', async () => { await foundRestTestChat(rig) rig.adapter.closeSession.mockClear() - // The loop takes the message and waits on the child's start, outside the lock. - const started = Promise.withResolvers() - const awaitStarted = vi.fn(() => started.promise) - Object.assign(rig.host.deps.adapter, { awaitStarted }) + // Only the sweep this test asks for runs, at the step it is asked for. + rig.host.collaboratorsForTests().lifetime.idleSweep.dispose() + await sweepTicks(2) const reader = collectSubscriber() await rig.host.subscribe({ id: 'reader', sessionId: SESSION, emit: reader.emit }) - const sent = await rig.host.send(CALLER, restTestSend('queued one', fence())) - expect(sent.ok).toBe(true) - await vi.waitFor(() => expect(awaitStarted).toHaveBeenCalled()) - rig.clock.now += IDLE_MS + 1 + // Held, the send is accepted, the agent goes idle past the window, and the sweep runs, all + // ahead of the handover the send asks for. + const held = Promise.withResolvers() + void rig.host['tasks'].serialize(SESSION, () => held.promise) + const sending = rig.host.send(CALLER, restTestSend('queued one', fence())) + void rig.host['tasks'].serialize(SESSION, async () => { + rig.clock.now += IDLE_MS + 1 + }) + const swept = sweepOnce(rig.host) + held.resolve() + expect((await sending).ok).toBe(true) - await sweepTicks() + await swept expect(rig.adapter.closeSession).not.toHaveBeenCalled() - started.resolve() await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) const settled = (await rig.host.journalSnapshot(SESSION)).submissions.at(-1) expect(settled?.dispatchState).toBe('accepted') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.ts index 5719dd172cf..763acc16c78 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-main-agent-working-agreement.test.ts @@ -89,7 +89,6 @@ beforeEach(async () => { } }, dispatch, - awaitStarted: vi.fn(async () => undefined), closeSession: vi.fn(async () => true), releaseAcquisition: vi.fn(async () => true), cancelTurn: vi.fn(async () => ({ cancelled: true })), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts index d3461a7e9ee..a9cdc2d1418 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts @@ -21,11 +21,13 @@ export async function readNativeSessionOptions(input: { } /** The record's options once the provider has reported: its model, effort and Fast replace the - * saved ones, other saved options stay, and any the restore could not apply are dropped. */ + * saved ones, other saved options stay, and any the child could not take are dropped. */ export function nativeSessionOptionsFromReport(input: { reported: AgentSessionOptionsResult['current'] restoreSkipped: readonly string[] priorOptions?: Readonly> + /** Values the child showed it cannot run: its report of the same value is not kept. */ + retired?: Readonly> }): Readonly> { const { reported, priorOptions } = input const restored = priorOptions ? { ...priorOptions } : {} @@ -39,10 +41,16 @@ export function nativeSessionOptionsFromReport(input: { reported.fastMode === undefined ? undefined : encodeStructuredAgentSessionOptionValue('fastMode', reported.fastMode) - return { + const options: Record = { ...restored, model: reported.model, ...(reported.effort ? { effort: reported.effort } : {}), ...(fastMode !== undefined && fastMode !== null ? { fastMode } : {}) } + for (const [key, value] of Object.entries(input.retired ?? {})) { + if (options[key] === value) { + delete options[key] + } + } + return options } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts index 73887ceef69..c906785c541 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts @@ -17,7 +17,8 @@ import { AgentHookServer, _internals } from '../../agent-hooks/server' import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { fakeClaude, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -161,8 +162,8 @@ async function dispatchState(clientMessageId: string): Promise { for (let turn = 0; turn < 2000 && connection.sent.length === 0; turn += 1) { await new Promise((resolve) => setImmediate(resolve)) @@ -175,45 +176,43 @@ function waitOutSeveralSweeps(): Promise { } describe('a Claude chat whose CLI is still starting', () => { - it('keeps a message queued inside the idle window, and delivers it once startup lands', async () => { + it('writes a message at once, and keeps the agent inside the idle window while startup is still out', async () => { await attachStarting() - const held = await send('sent while starting') + const sent = await send('sent while starting') + // Written before initialize answers: the CLI queues it behind its own start. + await untilSent(claude.connections[0]) + expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) + await vi.waitFor(async () => expect(await dispatchState(sent)).toBe('accepted')) clock += IDLE_MS - 1 await waitOutSeveralSweeps() expect(host.hasSession(SESSION)).toBe(true) expect(claude.connections[0].closeCount).toBe(0) - expect(await dispatchState(held)).toBe('pending') landInit() - await adapter.awaitStarted(SESSION) - - await vi.waitFor( - () => expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]), - { timeout: 3000 } - ) - await vi.waitFor(async () => expect(await dispatchState(held)).toBe('accepted')) + await claudeStartupSettled(adapter, SESSION) + expect(host.hasSession(SESSION)).toBe(true) }) - it('gives the message it hands over at startup a full idle window to open its turn', async () => { + it('gives a message written while starting a full idle window after startup lands to open its turn', async () => { await attachStarting() - const held = await send('sent while starting') const connection = claude.connections[0] // Claude echoes a prompt only when it starts that turn, which a loaded machine delays. connection.send = async (message) => { connection.sent.push(message) } + const held = await send('sent while starting') + await untilSent(connection) + expect(connection.sent).toEqual([expect.objectContaining({ type: 'user' })]) clock += IDLE_MS - 1 await waitOutSeveralSweeps() expect(host.hasSession(SESSION)).toBe(true) - // Startup lands just before the window closes; the handover's publish starts it again. + // Startup lands just before the window closes; the agent stays while its message waits. landInit() - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) - await untilSent(connection) - expect(connection.sent).toEqual([expect.objectContaining({ type: 'user' })]) clock += IDLE_MS - 1 await waitOutSeveralSweeps() @@ -229,7 +228,7 @@ describe('a Claude chat whose CLI is still starting', () => { it('stops the agent and closes the conversation once its turn has finished and it idled', async () => { await attachStarting() landInit() - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) const answered = await send('answered') await vi.waitFor(async () => expect(await dispatchState(answered)).toBe('accepted')) claude.connections[0].handlers.onMessage?.({ @@ -267,7 +266,7 @@ describe('a chat whose settled lead still has background work running', () => { async function settleTurnLeavingTask(taskType: string): Promise { await attachStarting() landInit() - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) const fanOut = await send('fan out') await vi.waitFor(async () => expect(await dispatchState(fanOut)).toBe('accepted')) frame({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts index 2551cd33237..4913ee123ca 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts @@ -274,21 +274,17 @@ function deferred() { describe('Stop on a child still proving its start', () => { it('ends the child and keeps the conversation and its readers (R1)', async () => { - const ended = deferred() - adapterExtras = { - awaitStarted: vi.fn(() => ended.promise), - closeSession: vi.fn(async () => { - ended.resolve() - return true - }) - } + adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementationOnce(spawnStartingChild) + // Written to the starting child at once; it never answers. + dispatch.mockResolvedValueOnce({ state: 'admitted' }) const first = await accept('hello') const journal = conversation()?.journal const events = await subscribe() const frames = watchStatus() await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) + await eventually(async () => expect((await submission(first))?.handedOverAt).toBeDefined()) expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) @@ -309,7 +305,7 @@ describe('Stop on a child still proving its start', () => { const next = await accept('after stop') await eventually(async () => expect((await submission(next))?.dispatchState).toBe('accepted')) expect(conversation()?.journal).toBe(journal) - expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([next]) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([first, next]) // The reader opened before the Stop saw the next message delivered on the same stream. expect( events.some( @@ -386,53 +382,38 @@ describe('a published child that dies while it proves its start', () => { const EXIT = START_EXIT const TEXT = START_TEXT - it.each([['the loop sees the start fail first'], ['the exit is processed first']])( - 'leaves one error row keyed by the start, and every queued message rejected with it: %s (R2)', - async (order) => { - const settled = deferred() - adapterExtras = { awaitStarted: vi.fn(() => settled.promise) } - await restartHost() - acquire.mockImplementation(spawnStartingChild) - const first = await accept('first') - const events = await subscribe() - const second = await accept('second') - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalled()) - const child = currentChild() + it('leaves one error row keyed by the start, and every message it was handed rejected with it (R2)', async () => { + await restartHost() + acquire.mockImplementation(spawnStartingChild) + // Written to the starting child at once; it never answers. + dispatch.mockImplementation(async () => ({ state: 'admitted' as const })) + const first = await accept('first') + const events = await subscribe() + const second = await accept('second') + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(2)) + const child = currentChild() - if (order === 'the exit is processed first') { - await exit(child, EXIT, true) - settled.resolve(START_FAILURE) - } else { - settled.resolve(START_FAILURE) - await eventually(async () => - expect((await submission(second))?.dispatchState).toBe('rejected') - ) - await exit(child, EXIT, true) - } + await exit(child, EXIT, true) - await eventually(async () => - expect((await submission(second))?.dispatchState).toBe('rejected') - ) - expect(await statusRows()).toEqual([ - { - itemId: `orca:${encodeURIComponent(`start-failure:${child.acquisitionGeneration}`)}`, - text: TEXT, - tone: 'error', - failure: START_FAILURE - } - ]) - for (const id of [first, second]) { - expect(await submission(id)).toMatchObject({ - dispatchState: 'rejected', - reason: TEXT, - rejection: START_FAILURE - }) + await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + expect(await statusRows()).toEqual([ + { + itemId: `orca:${encodeURIComponent(`start-failure:${child.acquisitionGeneration}`)}`, + text: TEXT, + tone: 'error', + failure: START_FAILURE } - expect(rejectedIn(events, second)).toBe(true) - expect(dispatch).not.toHaveBeenCalled() - expect(acquire).toHaveBeenCalledTimes(2) + ]) + for (const id of [first, second]) { + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + reason: TEXT, + rejection: START_FAILURE + }) } - ) + expect(rejectedIn(events, second)).toBe(true) + expect(acquire).toHaveBeenCalledTimes(2) + }) }) /** A start by an operation that needs the agent, such as a goal change, outside the loop. */ @@ -447,7 +428,6 @@ describe('a start another operation made that dies while a sent message waits on const TEXT = START_TEXT it("is the message's own failed start: one error row, the message rejected, no second start (R2)", async () => { - adapterExtras = { awaitStarted: vi.fn(async () => START_FAILURE) } await restartHost() acquire.mockImplementation(spawnStartingChild) // An operation that needs the agent starts a child that has not proven its start. @@ -512,8 +492,7 @@ describe('a start another operation made that dies while a sent message waits on expect(acquire).toHaveBeenCalledTimes(3) }) - it('waits on a child started since the failed one, not on the failure (R2)', async () => { - adapterExtras = { awaitStarted: vi.fn(async () => undefined) } + it('hands over to a child started since the failed one, not to the failure (R2)', async () => { await restartHost() acquire.mockImplementation(spawnStartingChild) await startForOperation() @@ -536,13 +515,29 @@ describe('a start another operation made that dies while a sent message waits on describe('a child that ends before its message is handed over', () => { it('starts one child for the message, then rejects it and stops (R2)', async () => { - // The child the loop starts exits between its start step and its handover step. - adapterExtras = { - awaitStarted: vi.fn(async () => { - await exit(currentChild(), 'codex app-server crashed') - }) - } await restartHost() + // The child the loop starts exits as its start step returns: that exit, asked for on the lane + // then, lands before the handover step. + acquire.mockImplementation(async (input) => { + const child = await spawnChild(input) + const { acquisitionGeneration } = child + if (acquisitionGeneration === undefined) { + throw new Error('spawnChild always names a generation') + } + void host.handleAdapterEvent({ + type: 'ended', + sessionId: SESSION, + fence: input.fence, + acquisitionGeneration, + reason: 'codex app-server crashed', + failure: { + kind: 'providerExited', + detail: { text: 'codex app-server crashed', audience: 'log' } + }, + cause: 'unexpected-exit' + }) + return child + }) const id = await accept('hello') const events = await subscribe() @@ -566,47 +561,34 @@ describe('a child that ends before its message is handed over', () => { }) }) -describe('another child indexed while the loop waits on the one it started', () => { - it('hands nothing over until the child now there has proven its start (R2)', async () => { - const starts = new Map>>() - const startOf = (generation: string) => { - const start = starts.get(generation) ?? deferred() - starts.set(generation, start) - return start - } - adapterExtras = { - awaitStarted: vi.fn(() => startOf(currentChild().acquisitionGeneration).promise), - // The stop ends the child without settling the start the loop is waiting on. - closeSession: vi.fn(async () => true) - } +describe('another child indexed after a Stop ended the starting one', () => { + it('hands the next message to the child now there, still starting (R2)', async () => { + adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementation(spawnStartingChild) + // The starting child takes the first message at once and never answers it. + dispatch.mockResolvedValueOnce({ state: 'admitted' }) const first = await accept('first') - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalledTimes(1)) + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) const stopped = currentChild() expect(await stop()).toMatchObject({ ok: true }) - const second = await accept('second') - // An operation that needs the agent starts its own child before the loop's handover step runs. + // An operation that needs the agent starts its own child before the second message is sent. await host['serialize'](SESSION, () => ensureStructuredAgentSessionAgent(host['attachContext'](), SESSION) ) const replacement = currentChild() expect(replacement.acquisitionGeneration).not.toBe(stopped.acquisitionGeneration) + const secondId = await accept('second') - startOf(stopped.acquisitionGeneration).resolve() - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalledTimes(2)) - expect(dispatch).not.toHaveBeenCalled() - - await host.handleAdapterEvent({ - type: 'started', - ...replacement, - reportedOptions: { model: 'sonnet' }, - restoreSkippedOptions: [] + await eventually(async () => + expect((await submission(secondId))?.dispatchState).toBe('accepted') + ) + expect(conversation()?.child).toMatchObject({ + generation: replacement.acquisitionGeneration, + phase: 'starting' }) - startOf(replacement.acquisitionGeneration).resolve() - - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('accepted')) - expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([second]) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([first, secondId]) + expect(acquire).toHaveBeenCalledTimes(3) expect(await submission(first)).toMatchObject({ reason: DISPATCH_REJECTED_CANCELLED }) }) }) @@ -712,23 +694,17 @@ async function settleLoop(): Promise { } describe('how a stopped child ends the start its loop was waiting on', () => { - /** A child the loop waits on, whose start the stop below does not settle, so a message sent - * after the stop is queued when the loop next looks. */ + /** A starting child that took the first message and never answered it, then the stop below; + * the message sent after the stop is the next delivery's. */ async function stoppedWhileStarting(stop: () => Promise) { - const start = deferred() - adapterExtras = { - awaitStarted: vi.fn(() => start.promise), - closeSession: vi.fn(async () => true) - } + adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementationOnce(spawnStartingChild) + dispatch.mockResolvedValueOnce({ state: 'admitted' }) await accept('first') - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalledTimes(1)) + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) await stop() - const second = await accept('second') - adapterExtras.awaitStarted = undefined - start.resolve() - return second + return accept('second') } it("goes on after a user's Stop and delivers what was sent since (R2)", async () => { @@ -750,22 +726,29 @@ describe('how a stopped child ends the start its loop was waiting on', () => { ) } - /** A message queued behind a starting child when the close's stop cut it. `beforeStart` runs - * after the stop and before the loop looks again. */ + /** A message queued for a starting child when the close's stop cut it: asked for during the + * start step, the stop runs before the handover. `beforeStart` runs after the stop and before + * the loop looks again. */ async function closedWhileStarting(beforeStart: () => void = () => undefined) { const start = deferred() - adapterExtras = { - awaitStarted: vi.fn(() => start.promise), - closeSession: vi.fn(async () => true) - } + adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() - acquire.mockImplementationOnce(spawnStartingChild) + acquire.mockImplementationOnce(async (input) => { + await start.promise + return spawnStartingChild(input) + }) const first = await accept('first') - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalledTimes(1)) - await closeStopOnly() - const starts = acquire.mock.calls.length - beforeStart() + await eventually(() => expect(acquire).toHaveBeenCalledTimes(2)) + let starts = 0 + const closed = host['serialize'](SESSION, async () => { + await stopStructuredAgentSessionAgentUnderSerialize(host['lifetimeContext'](), SESSION, { + cause: 'user-close' + }) + starts = acquire.mock.calls.length + beforeStart() + }) start.resolve() + await closed await settleLoop() return { first, starts } } @@ -784,17 +767,21 @@ describe('how a stopped child ends the start its loop was waiting on', () => { it('keeps a person’s message the user closed as a held card, and starts no child for it', async () => { const start = deferred() - adapterExtras = { - awaitStarted: vi.fn(() => start.promise), - closeSession: vi.fn(async () => true) - } + adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() - acquire.mockImplementationOnce(spawnStartingChild) + // The start step holds the queue: the message is still queued when the close's stop runs. + acquire.mockImplementationOnce(async (input) => { + await start.promise + return spawnStartingChild(input) + }) const first = await accept('first', { person: true }) - await eventually(() => expect(adapterExtras.awaitStarted).toHaveBeenCalledTimes(1)) - await closeStopOnly() - const starts = acquire.mock.calls.length + await eventually(() => expect(acquire).toHaveBeenCalledTimes(2)) + let starts = 0 + const closed = closeStopOnly().then(() => { + starts = acquire.mock.calls.length + }) start.resolve() + await closed await settleLoop() expect(await submission(first)).toMatchObject({ dispatchState: 'rejected', ...CHAT_CLOSED }) @@ -857,25 +844,31 @@ describe('how a stopped child ends the start its loop was waiting on', () => { it("fails the start after a host stop, as a start Orca stopped rather than the provider's (R2)", async () => { const reason = 'the start watchdog fired' - const second = await stoppedWhileStarting(() => - host['serialize'](SESSION, () => - stopStructuredAgentSessionAgentUnderSerialize(host['lifetimeContext'](), SESSION, { - cause: 'host-stop', - reason - }) - ) + adapterExtras = { closeSession: vi.fn(async () => true) } + await restartHost() + acquire.mockImplementationOnce(spawnStartingChild) + dispatch.mockResolvedValueOnce({ state: 'admitted' }) + const first = await accept('first') + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await host['serialize'](SESSION, () => + stopStructuredAgentSessionAgentUnderSerialize(host['lifetimeContext'](), SESSION, { + cause: 'host-stop', + reason + }) ) - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + // What the stopped start was handed fails with it, said once; a later send starts afresh. + await eventually(async () => expect((await submission(first))?.dispatchState).toBe('rejected')) // The sentence is the constructor's, not the reason the stop was given. const text = 'Codex never finished starting, so Orca stopped it.' - expect(await submission(second)).toMatchObject({ + expect(await submission(first)).toMatchObject({ reason: text, rejection: { kind: 'hostStopped' } }) expect(await statusRows()).toEqual([ { itemId: expect.any(String), text, tone: 'error', failure: { kind: 'hostStopped' } } ]) - expect(dispatch).not.toHaveBeenCalled() + const second = await accept('second') + await eventually(async () => expect((await submission(second))?.dispatchState).toBe('accepted')) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.test.ts index 18cfaba38fd..734f262f7b5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.test.ts @@ -1,6 +1,6 @@ // A publish-first create proves nothing about the model until Claude answers startup. The record // must never hold the catalog's default in the meantime: an owner handoff or a reopen would -// replay it as a `set_model` and silently move a user whose CLI default is not Sonnet. +// launch it as `--model` and silently move a user whose CLI default is not Sonnet. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -10,7 +10,8 @@ import type { AgentSessionStatusEvent } from '../../../shared/agent-session-wire import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { fakeClaude, - PROVIDER_SESSION_ID + PROVIDER_SESSION_ID, + claudeStartupSettled } from '../../claude/claude-structured-session-test-support' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -118,7 +119,7 @@ describe('a publish-first Claude create whose init is slow', () => { expect(store.getRecord(SESSION)?.options?.model).toBeUndefined() expect(lastPhase()).toBe('starting') - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) expect(store.getRecord(SESSION)?.options?.model).toBe('claude-opus-9') @@ -132,7 +133,7 @@ describe('a publish-first Claude create whose init is slow', () => { ).resolves.toMatchObject({ ok: true }) expect(store.getRecord(SESSION)?.options?.model).toBe('opus') - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) expect(store.getRecord(SESSION)?.options?.model).toBe('opus') @@ -142,7 +143,7 @@ describe('a publish-first Claude create whose init is slow', () => { it('keeps the picked model across a resume whose new child starts on its own default', async () => { const params = claudeParams() await host.attach(CALLER, { ...params, options: { model: 'opus' } }) - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) await host.close(SESSION, 'evict') const releasedFence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 @@ -152,11 +153,11 @@ describe('a publish-first Claude create whose init is slow', () => { ok: true }) expect(store.getRecord(SESSION)?.lease.runtimeFence).toBeGreaterThan(releasedFence) - // The new child's init reports its CLI default; the saved pick is restored over it. + // The new child is launched with the saved pick, whatever its CLI default. expect(store.getRecord(SESSION)?.options?.model).toBe('opus') expect(lastPhase()).toBe('starting') - await adapter.awaitStarted(SESSION) + await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) expect(store.getRecord(SESSION)?.options?.model).toBe('opus') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts index acc2e6488c3..589bf37c261 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts @@ -10,7 +10,10 @@ // asks the provider nothing: the event carries what the child proved. import { agentSessionLeaseAdmitsWriter } from '../../../shared/agent-session-lease-adjudication' -import type { StructuredAgentSessionStartedEvent } from './structured-agent-session-adapter' +import type { + StructuredAgentSessionOptionsSkippedEvent, + StructuredAgentSessionStartedEvent +} from './structured-agent-session-adapter' import type { StructuredAgentSessionHostDeps, StructuredAgentSessionHostSession @@ -75,8 +78,51 @@ async function persistStartedOptions( options: nativeSessionOptionsFromReport({ reported: event.reportedOptions, restoreSkipped: event.restoreSkippedOptions, + ...(event.retiredOptions ? { retired: event.retiredOptions } : {}), ...(record.options ? { priorOptions: record.options } : {}) }), now: context.now() }) } + +/** A running child showed saved options it cannot run: the record drops them, as a start that + * skipped them would, so the next start runs the provider's own. Reported, never thrown. */ +export function settleStructuredAgentSessionOptionsSkipped( + context: StructuredAgentSessionProviderStartedContext, + event: StructuredAgentSessionOptionsSkippedEvent +): Promise { + return context.serialize(event.sessionId, async () => { + const { store } = context.deps + const record = store.getRecord(event.sessionId) + if ( + !record?.options || + record.lease.runtimeFence !== event.fence || + !agentSessionLeaseAdmitsWriter(record.lease) + ) { + return + } + const options = { ...record.options } + for (const [key, value] of Object.entries(event.options)) { + // A pick made since the child launched is the user's, whatever the child showed. + if (options[key] === value) { + delete options[key] + } + } + try { + await store.replaceSessionOptions({ + sessionId: event.sessionId, + fence: event.fence, + options, + now: context.now() + }) + } catch (error) { + context.deps.logger.warn('dropping a saved option the provider cannot run failed', { + scope: 'provider-options-skipped', + sessionId: event.sessionId, + error + }) + } finally { + context.publishStatus?.(event.sessionId) + } + }) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-card-holds.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-card-holds.test.ts index b169bbe4e5e..10b48321bc9 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-card-holds.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-card-holds.test.ts @@ -385,16 +385,13 @@ describe('after a restart, nothing sends by itself', () => { await rig.settleAccepted(working, 'stopped') // The agent at rest goes; Resume's hand-off of A must start a new one, which never starts. await rig.host.collaboratorsForTests().lifetime.idleSweep.tick() - let release: () => void = () => undefined - rig.awaitStarted.mockImplementation( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + // The hand-off is recorded, never handed over: the new child stays in its spawn. + const release = rig.holdNextStart() expect(await rig.resume()).toMatchObject({ ok: true, value: { resumed: true } }) await eventually(async () => expect(await rig.handoff(first)).toBeDefined()) const cutShort = await rig.handoffId(first) expect((await rig.handoff(first))?.handedOverAt).toBeUndefined() rig.crashRestartHostProcess() - rig.awaitStarted.mockImplementation(async () => undefined) release() await published() // The new host refuses the leftover hand-off, and A waits again in its own place. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig-provider.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig-provider.test-fixture.ts index 864ad55f13c..ccabd39503e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig-provider.test-fixture.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig-provider.test-fixture.ts @@ -20,6 +20,8 @@ export type QueuedRigProviderOptions = { starting?: true /** The provider's Stop ends its child, as Claude's does. */ stopEndsSession?: true + /** Its child never answers its start, so it runs nothing it is handed. */ + startUnanswered?: true } export function createQueuedRigProvider( @@ -31,9 +33,9 @@ export function createQueuedRigProvider( const dispatch: Mock = vi.fn(async () => ({ state: 'admitted' as const })) - const awaitStarted: Mock> = vi.fn( - async () => undefined - ) + // Every start the host asks for; one `holdNextStart` holds stays in its spawn until released. + const starts: Mock<() => void> = vi.fn() + let startHold: Promise | null = null // The provider's receipt of a /compact; its end arrives later, as `finishCompact` writes it. const compact: Mock> = vi.fn(async () => ({ state: 'accepted' as const, @@ -49,6 +51,10 @@ export function createQueuedRigProvider( const adapter: StructuredAgentSessionAdapter = { acquire: async ({ identity, fence, spawnToken, events: sink }) => { + starts() + const hold = startHold + startHold = null + await hold events = sink const resumes = options.restartable === true && @@ -72,12 +78,12 @@ export function createQueuedRigProvider( } }, dispatch, - awaitStarted, closeSession, releaseAcquisition: vi.fn(async () => true), compact, cancelTurn, ...(options.stopEndsSession ? { stopEndsSession: () => true } : {}), + ...(options.startUnanswered ? { startAnswered: () => false } : {}), answerPrompt: vi.fn(async () => undefined), setOption: vi.fn(async () => undefined) } @@ -99,6 +105,13 @@ export function createQueuedRigProvider( ) } + /** Holds the next start in its spawn, with what it was asked for still queued, until released. */ + function holdNextStart(): () => void { + let release = (): void => {} + startHold = new Promise((resolve) => (release = resolve)) + return () => release() + } + /** The event sink the provider writes through. */ function providerEvents(): StructuredAgentSessionEventSink { if (!events) { @@ -110,7 +123,8 @@ export function createQueuedRigProvider( return { adapter, dispatch, - awaitStarted, + starts, + holdNextStart, compact, cancelTurn, closeSession, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts index 58ef7a607ba..a0e3af88c49 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts @@ -13,6 +13,7 @@ import type { AgentSessionQueuePause } from '../../../shared/agent-session-wire' import type { AgentMessageSource } from '../../../shared/agent-session-message-source' import { agentSessionMessagePayload } from '../../../shared/structured-agent-session-send-mutation' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { AgentSessionRecoveryCapsule } from '../../runtime/agent-session-recovery-capsule' import { StructuredAgentSessionHost } from './structured-agent-session-host' import { rotateStructuredAgentSessionHostInstanceForTests } from './structured-agent-session-queued-pause' import { @@ -45,13 +46,15 @@ export async function createQueuedMessageTestRig( options: QueuedRigProviderOptions & { /** Lets a test sweep idle chats on its own `tick`. */ idleSweep?: { idleMs: number; intervalMs: number } + /** Teardown records its restart offers, which `restartOffers` lists. */ + recoveryCapsule?: true } = {} ) { const root = await mkdtemp(join(tmpdir(), 'orca-queued-messages-')) resetHostTestOperationIds() const store = await openTestAgentSessionRecordStore(root) const provider = createQueuedRigProvider(store, options) - const { dispatch, awaitStarted, compact, cancelTurn, closeSession } = provider + const { dispatch, compact, cancelTurn, closeSession } = provider const makeHost = () => new StructuredAgentSessionHost({ agents: claudeAndCodexDeclared(), @@ -62,7 +65,8 @@ export async function createQueuedMessageTestRig( claimKeyId: 'key-1', mintSpawnToken: () => 'spawn-1', now: () => NOW, - ...(options.idleSweep ? { idleSweep: options.idleSweep } : {}) + ...(options.idleSweep ? { idleSweep: options.idleSweep } : {}), + ...(options.recoveryCapsule ? { recoveryCapsule: new AgentSessionRecoveryCapsule(root) } : {}) }) let host = makeHost() expect(await host.attach(QUEUED_RIG_CALLER, hostTestAttachParams(null))).toMatchObject({ @@ -229,6 +233,11 @@ export async function createQueuedMessageTestRig( return page.page.queuePause ?? null } + /** The restart offers this host lists for the chats an earlier one stopped. */ + async function restartOffers() { + return (await host.restartResume.list()).map(({ sessionId, work }) => ({ sessionId, work })) + } + function resume(clientOperationId = hostTestOperationId()) { return host.queuedMessagesResume(QUEUED_RIG_CALLER, { envelope: envelope({}, 'agentSession.queuedMessagesResume', clientOperationId) @@ -249,8 +258,9 @@ export async function createQueuedMessageTestRig( dispatch, cancelTurn, closeSession, - awaitStarted, compact, + starts: provider.starts, + holdNextStart: provider.holdNextStart, finishCompact: provider.finishCompact, providerEvents: provider.providerEvents, envelope, @@ -269,6 +279,7 @@ export async function createQueuedMessageTestRig( crashRestartHostProcess, quitRestartHostProcess, queuePause, + restartOffers, resume, dispose } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts index 7418e7eb8df..686940c4fa7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts @@ -31,16 +31,16 @@ import { hostTestMessage, hostTestOperationId } from './structured-agent-session-host-test-data' +import { holdDelivery } from './structured-agent-session-delivery-hold.test-fixture' let rig: QueuedMessageTestRig let host: QueuedMessageTestRig['host'] let store: QueuedMessageTestRig['store'] let dispatch: QueuedMessageTestRig['dispatch'] -let awaitStarted: QueuedMessageTestRig['awaitStarted'] beforeEach(async () => { rig = await createQueuedMessageTestRig() - ;({ host, store, dispatch, awaitStarted } = rig) + ;({ host, store, dispatch } = rig) }) afterEach(() => rig.dispose()) @@ -378,29 +378,27 @@ describe('Stop and Delete', () => { expect(await drafts()).toHaveLength(2) }) - /** A draft consumed into a submission the delivery loop has not handed over: - * the loop is held at the child's start proof until the returned release. */ + /** A draft consumed into a submission the delivery loop has not handed over: its delivery step + * is held until the returned release, so a step asked for meanwhile runs ahead of the handover. */ async function consumedButNotHandedOver(): Promise<{ draftId: string; release: () => void }> { const working = await workingSend() const queued = await send('stopped in flight', 'queue-if-active').result if (!queued.ok || !('queued' in queued.value)) { throw new Error('expected a queued receipt') } - let release: () => void = () => undefined - awaitStarted.mockImplementationOnce( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + const { release } = holdDelivery() await settleAccepted(working, 'a') const draftId = queued.value.queued.messageId await eventually(async () => expect(await rig.handoff(draftId)).toBeDefined()) expect((await rig.handoff(draftId))?.handedOverAt).toBeUndefined() - return { draftId, release: () => release() } + return { draftId, release } } it("a Stop between consume and the agent's receipt sends the draft back to waiting, paused like the rest", async () => { const { draftId, release } = await consumedButNotHandedOver() - const stopped = await stop() + const stopping = stop() release() + const stopped = await stopping expect(stopped).toMatchObject({ ok: true }) expect(await drafts()).toEqual([{ messageId: draftId, state: 'waiting' }]) expect(await rig.queuePause()).toEqual({ reason: 'stopped' }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts index cf1746eaaf0..9f37f62873e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts @@ -11,6 +11,7 @@ import { hostTestMessage, hostTestOperationId } from './structured-agent-session-host-test-data' +import { holdDelivery } from './structured-agent-session-delivery-hold.test-fixture' import { QUEUED_RIG_CALLER, createQueuedMessageTestRig, @@ -393,13 +394,16 @@ describe('a card handed off after a restart', () => { ) await rig.restartHostProcess() // Sent again in this process, then withdrawn by a Stop before the agent had it. - let release: () => void = () => undefined - rig.awaitStarted.mockImplementationOnce( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) - expect(await rig.sendNow(draftId)).toMatchObject({ ok: true }) - await rig.stop() + // Its delivery is held, so the Stop runs ahead of the handover. + const { held, release } = holdDelivery() + const handedOver = rig.dispatch.mock.calls.length + const sending = rig.sendNow(draftId) + await held + const stopping = rig.stop() release() + expect(await sending).toMatchObject({ ok: true }) + await stopping + expect(rig.dispatch).toHaveBeenCalledTimes(handedOver) await eventually(async () => expect(await rig.drafts()).toEqual([{ messageId: draftId, state: 'waiting' }]) ) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts index 5e23fbe1a84..6cf71ea894c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, expect, it, vi } from 'vitest' import { JournalQueuedMessages } from '../agent-session-journal/journal-queued-messages' import { AgentSessionJournal } from '../agent-session-journal/journal-store' import { HOST_TEST_SESSION as SESSION } from './structured-agent-session-host-test-data' +import { holdDelivery } from './structured-agent-session-delivery-hold.test-fixture' import { createQueuedMessageTestRig, eventually, @@ -43,11 +44,8 @@ it('Stop, then a user send: the withdrawn draft and the paused cards behind it d const a = await queuedDraft('A') const b = await queuedDraft('B') const c = await queuedDraft('C') - // The turn ends and the drain consumes A; the agent's start is held, so A is not handed over. - let release: () => void = () => undefined - rig.awaitStarted.mockImplementationOnce( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + // The turn ends and the drain consumes A; its delivery is held, so A is not handed over. + const { release } = holdDelivery() await rig.settleAccepted(working, 'working') await eventually(async () => expect(await rig.handoff(a)).toBeDefined()) // Never under the draft's own id: the submission names A by its link. @@ -55,8 +53,10 @@ it('Stop, then a user send: the withdrawn draft and the paused cards behind it d expect(firstA).not.toBe(a) expect((await rig.submission(firstA))?.handedOverAt).toBeUndefined() - expect(await rig.stop()).toMatchObject({ ok: true }) + // The Stop runs once the held step returns, ahead of A's handover. + const stopped = rig.stop() release() + expect(await stopped).toMatchObject({ ok: true }) // A is back in its place, behind the same queue pause as B and C: no returned card blocks them. const paused = [a, b, c].map((messageId) => ({ messageId, state: 'waiting' })) expect(await rig.drafts()).toEqual(paused) @@ -104,10 +104,7 @@ it('Stop, then a user send: the withdrawn draft and the paused cards behind it d it('a Stop whose withdrawal throws after landing still answers; the draft it released sends again under a fresh id', async () => { const working = await rig.workingSend() const a = await queuedDraft('A') - let release: () => void = () => undefined - rig.awaitStarted.mockImplementationOnce( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + const { release } = holdDelivery() await rig.settleAccepted(working, 'working') await eventually(async () => expect(await rig.handoff(a)).toBeDefined()) const firstA = await rig.handoffId(a) @@ -124,7 +121,9 @@ it('a Stop whose withdrawal throws after landing still answers; the draft it rel }) const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) try { - expect(await rig.stop()).toMatchObject({ ok: true }) + const stopped = rig.stop() + release() + expect(await stopped).toMatchObject({ ok: true }) expect(warned).toHaveBeenCalledWith( "[agent-session] stop-queued-bookkeeping: Stop's withdrawal failed", expect.objectContaining({ step: 'withdrawal', error: new Error('disk full') }) @@ -142,15 +141,12 @@ it('a Stop whose withdrawal throws after landing still answers; the draft it rel }) }) -/** A consumed card whose delivery is held at the agent's start, so a Stop withdraws it; - * the settlement hook throws on that withdrawal's row, leaving the card owed a return. */ +/** A consumed card whose delivery is held, so a Stop withdraws it ahead of its handover; the + * settlement hook throws on that withdrawal's row, leaving the card owed a return. */ async function stopWithSkippedSettlement(): Promise<{ a: string; working: string }> { const working = await rig.workingSend() const a = await queuedDraft('A') - let release: () => void = () => undefined - rig.awaitStarted.mockImplementationOnce( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + const { release } = holdDelivery() await rig.settleAccepted(working, 'working') await eventually(async () => expect(await rig.handoff(a)).toBeDefined()) const settle = JournalQueuedMessages.prototype.onRowInTransaction @@ -166,7 +162,9 @@ async function stopWithSkippedSettlement(): Promise<{ a: string; working: string }) const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) try { - expect(await rig.stop()).toMatchObject({ ok: true }) + const stopped = rig.stop() + release() + expect(await stopped).toMatchObject({ ok: true }) } finally { hook.mockRestore() warned.mockRestore() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts index 5f4e5bb6e79..6a868201baa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts @@ -104,6 +104,7 @@ export function createStructuredAgentSessionRestartResume( sessions, getRecord: deps.store.getRecord, childWork: surfaces.readChildWork, + startAnswered: (sessionId) => deps.adapter.startAnswered?.(sessionId), ...(deps.recoveryCapsule ? { capsule: deps.recoveryCapsule } : {}), teardownId: randomUUID(), now: surfaces.now, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.test.ts index 739ad2c5a33..dc71bc09f07 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.test.ts @@ -20,6 +20,7 @@ function witnessed() { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: clear reads only the record's provider. AGENTS[sessionId] ? ({ provider: AGENTS[sessionId] } as AgentSessionRecord) : null, childWork: () => undefined, + startAnswered: () => undefined, capsule: { record: async (markers) => void recorded.push([...markers]) }, teardownId: 'teardown', now: () => 1, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.ts index 683fe79d9e8..648f92b6d59 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-witnesses.ts @@ -33,6 +33,7 @@ export function createStructuredAgentSessionRestartWitnesses(deps: { sessions: ReadonlyMap> getRecord: (sessionId: string) => AgentSessionRecord | null childWork: WorkingAtStopInput['childWork'] + startAnswered: NonNullable capsule?: Pick teardownId: string now: () => number @@ -72,6 +73,7 @@ export function createStructuredAgentSessionRestartWitnesses(deps: { session: deps.sessions.get(sessionId), getRecord: deps.getRecord, childWork: deps.childWork, + startAnswered: deps.startAnswered, trigger, teardownId: deps.teardownId, now: deps.now() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts index 77b48a375a4..6cef8ef9a1f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts @@ -1,21 +1,17 @@ -// A start a queued message waited on can be seen failing twice: by the delivery loop, when the -// adapter settles the start without proving it, and by the exit settlement, when the child's exit -// lands. The chat gets one row for that start, the loop's, in the words the message was rejected -// with — whichever of the two reports first. +// A start that fails can be written up twice: by the exit settlement, for a message the starting +// child was handed, and by the delivery loop, for a message the exit found still queued. The chat +// gets one row for that start, in the words the message was rejected with. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { - agentSessionFailureFact, - type SubmissionRejectionFact -} from '../../../shared/agent-session-failure' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' import type { AgentSessionSubscribeEvent } from '../../../shared/agent-session-wire' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import { StructuredAgentSessionHost } from './structured-agent-session-host' import { HOST_TEST_NOW as NOW, @@ -33,9 +29,6 @@ import { NO_STRUCTURED_AGENTS } from './structured-agent-session-adapter-router- const CALLER = { callerKey: 'client-1' } const EXIT_REASON = 'Claude Code is not signed in. Sign in with the Claude CLI' -const ADAPTER_FAILURE = agentSessionFailureFact('notSignedIn') -const ADAPTER_FAILURE_TEXT = - 'Codex is not signed in for the selected account. Sign in, then send your message again.' // The exit's reason is Orca's log text; the row says only that the start stopped. const EXIT_TEXT = 'Codex stopped before it finished starting. Send your message to try again.' // The first child (generation-1) is lost at setup; the send starts generation-2. @@ -52,8 +45,9 @@ let root: string let store: AgentSessionRecordStore let host: StructuredAgentSessionHost let generation = 0 -let settleStart: (failure: SubmissionRejectionFact | undefined) => void = () => {} -let awaitStarted = vi.fn<() => Promise>() +/** The next start's child exits as its start step returns, before the handover step. */ +let exitOnStart = false +let dispatch: Mock let frames: AgentSessionSubscribeEvent[] = [] function exitBeforeProof(): Promise { @@ -68,7 +62,7 @@ function exitBeforeProof(): Promise { }) } -async function sendQueued(text: string): Promise { +async function send(text: string): Promise { const body = hostTestMessage(text) const sent = await host.send(CALLER, { envelope: { @@ -84,9 +78,7 @@ async function sendQueued(text: string): Promise { body }) expect(sent).toMatchObject({ ok: true }) - // The loop started a child and waits on its start with the message still queued. - await eventually(() => expect(awaitStarted).toHaveBeenCalledOnce()) - expect(generation).toBe(2) + await eventually(() => expect(generation).toBe(2)) return sent.ok ? sent.value.clientMessageId : '' } @@ -118,31 +110,49 @@ beforeEach(async () => { resetHostTestOperationIds() generation = 0 frames = [] - awaitStarted = vi.fn( - () => new Promise((resolve) => (settleStart = resolve)) - ) + exitOnStart = false + dispatch = vi.fn(async () => ({ state: 'admitted' as const })) store = await openTestAgentSessionRecordStore(root) host = new StructuredAgentSessionHost({ agents: NO_STRUCTURED_AGENTS, logger: createStructuredAgentSessionLogger(), store, adapter: { - acquire: vi.fn(async ({ fence, spawnToken }) => ({ - process: { hostId: 'local', pid: 4242, processStartTimeMs: 1_700_000_000_000, spawnToken }, - link: { - linkId: `link-${fence}`, - handle: codexProviderHandle(THREAD), - origin: generation === 0 ? ('created' as const) : ('resumed' as const), - mintedAtFence: fence, - observedAt: NOW - }, - acquisitionGeneration: `generation-${++generation}`, - providerChildPhase: 'starting' as const - })), - awaitStarted, + acquire: vi.fn(async ({ fence, spawnToken }) => { + const child = { + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken + }, + link: { + linkId: `link-${fence}`, + handle: codexProviderHandle(THREAD), + origin: generation === 0 ? ('created' as const) : ('resumed' as const), + mintedAtFence: fence, + observedAt: NOW + }, + acquisitionGeneration: `generation-${++generation}`, + providerChildPhase: 'starting' as const + } + if (exitOnStart) { + // Asked for on the lane while the start step runs, so it lands before the handover. + void host.handleAdapterEvent({ + type: 'ended', + sessionId: SESSION, + fence, + acquisitionGeneration: child.acquisitionGeneration, + reason: EXIT_REASON, + cause: 'unexpected-exit', + startupUnproven: true + }) + } + return child + }), releaseAcquisition: vi.fn(async () => true), closeSession: vi.fn(async () => true), - dispatch: vi.fn(async () => ({ state: 'admitted' as const })), + dispatch: (...args) => dispatch(...args), cancelTurn: vi.fn(async () => ({ cancelled: true })), answerPrompt: vi.fn(async () => undefined), setOption: vi.fn(async () => undefined) @@ -164,36 +174,32 @@ afterEach(async () => { await rm(root, { recursive: true, force: true }) }) -describe('a queued message whose start fails and whose child then exits', () => { - it("keeps the loop's row when the loop saw the failure first", async () => { - const queued = await sendQueued('hello') +describe('a start that fails before it proves itself', () => { + it("writes one row in the exit's words for a message the child was handed", async () => { + const sent = await send('hello') + // A starting child takes the message at once. + await eventually(() => expect(dispatch).toHaveBeenCalledOnce()) - settleStart(ADAPTER_FAILURE) - await eventually(async () => - expect(await submission(queued)).toMatchObject({ - dispatchState: 'rejected', - reason: ADAPTER_FAILURE_TEXT, - rejection: ADAPTER_FAILURE - }) - ) await exitBeforeProof() + await eventually(async () => + expect(await submission(sent)).toMatchObject({ dispatchState: 'rejected', reason: EXIT_TEXT }) + ) await host.flushStreamedEvents(SESSION) - expect(await startRows()).toEqual([ADAPTER_FAILURE_TEXT]) - expect(publishedStartRows()).toEqual([ADAPTER_FAILURE_TEXT]) + expect(await startRows()).toEqual([EXIT_TEXT]) + expect(publishedStartRows()).toEqual([EXIT_TEXT]) }) it('leaves the row to the loop when the exit lands while the message still waits', async () => { - const queued = await sendQueued('hello') + exitOnStart = true + const queued = await send('hello') - await exitBeforeProof() - expect(await submission(queued)).toMatchObject({ dispatchState: 'pending' }) - settleStart(undefined) await eventually(async () => expect(await submission(queued)).toMatchObject({ dispatchState: 'rejected' }) ) await host.flushStreamedEvents(SESSION) + expect(dispatch).not.toHaveBeenCalled() expect(await startRows()).toEqual([EXIT_TEXT]) // Written once, after the message was settled, not first by the exit and again by the loop. expect(publishedStartRows()).toEqual([EXIT_TEXT]) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-steer-while-stopping.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-steer-while-stopping.test.ts index 1bc687e31e3..17610df9240 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-steer-while-stopping.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-steer-while-stopping.test.ts @@ -15,6 +15,7 @@ import { eventually, type QueuedMessageTestRig } from './structured-agent-session-queued-message-rig.test-fixture' +import { holdDelivery } from './structured-agent-session-delivery-hold.test-fixture' let rig: QueuedMessageTestRig @@ -167,27 +168,27 @@ describe("a message sent while a person's Stop ends the turn", () => { await eventually(() => expect(rig.dispatch.mock.calls.length).toBe(dispatched + 1)) }) - // The delivery step that judged the send waits on the child's start outside the session's lane. - it('holds a send that a Stop overtook while its delivery step waited on the agent', async () => { + // The delivery step that judged the send and its handover are two turns of the session's lane. + it('holds a send that a Stop overtook between its delivery step and the handover', async () => { rig = await createQueuedMessageTestRig() const sent = await rig.workingSend() await rig.settleAccepted(sent, 'sent') await turn('turn-1', sent, 'running') + await laneDrained() const status = watchStatus() - const started = Promise.withResolvers() - const awaited = rig.awaitStarted.mock.calls.length - rig.awaitStarted.mockImplementationOnce(() => started.promise) + const step = holdDelivery() const first = rig.send('steer this in') expect(await first.result).toMatchObject({ ok: true }) - await eventually(() => expect(rig.awaitStarted.mock.calls.length).toBe(awaited + 1)) + await step.held const dispatched = rig.dispatch.mock.calls.length - // The Stop withdraws the first send; a second one arrives before the step resumes. - expect(await rig.stop()).toMatchObject({ ok: true }) - await eventually(() => expect(status()).toMatchObject({ stopping: true })) + // The Stop withdraws the first send; a second one arrives before the handover. + const stopped = rig.stop() const second = rig.send('and this one') + step.release() + expect(await stopped).toMatchObject({ ok: true }) expect(await second.result).toMatchObject({ ok: true }) - started.resolve(undefined) + await eventually(() => expect(status()).toMatchObject({ stopping: true })) await laneDrained() expect(rig.dispatch.mock.calls.length).toBe(dispatched) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts index 8d46b360b9f..556606bc4a8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts @@ -103,27 +103,23 @@ async function queuedDraft(text: string): Promise { * behind the start, which the Stop holds. */ async function stopOfStart(options: { held?: true } = {}): Promise { rig = await createQueuedMessageTestRig({ starting: true, restartable: true }) - let release: () => void = () => undefined - rig.awaitStarted.mockImplementation( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) + // Handed over at once to a child that never proves its start; nothing echoes it. rig.send('work on this') await eventually(() => expect(childPhase()).toBe('starting')) const held = options.held ? await queuedDraft('queued behind the start') : undefined expect(await rig.stop()).toMatchObject({ ok: true }) - release() expect(stopEvents()).toEqual([expect.objectContaining({ reason: 'user-stop' })]) expect(stopEvents()[0]).not.toHaveProperty('turnId') await eventually(() => expect(childPhase()).toBeUndefined()) - rig.awaitStarted.mockImplementation(async () => undefined) return held } /** Orchestration mail after the Stop starts a new child and its turn runs. */ async function mailTurn(): Promise { + const handedOver = rig.dispatch.mock.calls.length const mail = rig.send('mail for the worker') await mail.result - await eventually(() => expect(rig.dispatch).toHaveBeenCalled()) + await eventually(() => expect(rig.dispatch).toHaveBeenCalledTimes(handedOver + 1)) await turnOpenedBy(mail.id) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts index aa9ff4d041d..3f1afc89d6b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts @@ -77,15 +77,6 @@ function idleSweep() { return rig.host.collaboratorsForTests().lifetime.idleSweep } -/** Holds every start until the returned release. */ -function holdStart(): () => void { - let release: () => void = () => undefined - rig.awaitStarted.mockImplementation( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) - return () => release() -} - describe('every Stop entry writes its event, with its reason, before it ends the child', () => { it.each([ // A person closing this chat: its tab, its launch, or a /clear that replaces it. @@ -124,7 +115,6 @@ describe('every Stop entry writes its event, with its reason, before it ends the restartable: true, idleSweep: MANUAL_IDLE_SWEEP }) - holdStart() rig.send('work on this') await eventually(async () => expect(rig.host.collaboratorsForTests().sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe( @@ -419,7 +409,6 @@ describe("a person's Stop pause and the Stop events after it", () => { // The agent at rest goes, writing nothing; mail then starts a new child, which never lands. await idleSweep().tick() expect(await rig.queuePause()).toEqual({ reason: 'stopped' }) - holdStart() rig.send('mail for the lead') await eventually(async () => expect(rig.host.collaboratorsForTests().sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe( @@ -431,7 +420,8 @@ describe("a person's Stop pause and the Stop events after it", () => { await idleSweep().tick() expect(atClose.events?.map((event) => event.reason)).toEqual(['user-stop', 'host-stop']) - expect(await rig.handoff(held)).toBeUndefined() expect(await rig.queuePause()).not.toEqual({ reason: 'stopped' }) + // The pause ended with the host's stop, so the card goes out to the next start. + await eventually(async () => expect(await rig.handoff(held)).toBeDefined()) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts index 2a8c496e8b2..f635fa7b402 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts @@ -8,6 +8,7 @@ import { agentSessionFailureWords } from '../../../shared/agent-session-failure- import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' import type { JournalStopEvent } from '../agent-session-journal/journal-row-schema' import { HOST_TEST_SESSION, hostTestOperationId } from './structured-agent-session-host-test-data' +import { holdLane } from './structured-agent-session-delivery-hold.test-fixture' import { createQueuedMessageTestRig, eventually, @@ -64,15 +65,6 @@ function withdraw(clientMessageId: string) { }) } -/** Holds every start until the returned release. */ -function holdStart(): () => void { - let release: () => void = () => undefined - rig.awaitStarted.mockImplementation( - () => new Promise((resolve) => (release = () => resolve(undefined))) - ) - return () => release() -} - describe("a Stop's event", () => { it('reaches the journal before the interrupt, naming the turn it stopped and who asked', async () => { rig = await createQueuedMessageTestRig() @@ -134,8 +126,9 @@ describe("a Stop's event", () => { it('at an agent still starting, reaches the journal before the start is ended, and holds a card queued before it', async () => { rig = await createQueuedMessageTestRig({ starting: true, restartable: true }) - const release = holdStart() + // A starting agent takes the send at once; it never proves its start. rig.send('work on this') + await eventually(() => expect(rig.dispatch).toHaveBeenCalledTimes(1)) const held = await queuedDraft('queued while it starts') let atEnd: JournalStopEvent[] | undefined rig.closeSession.mockImplementationOnce(async () => { @@ -143,7 +136,6 @@ describe("a Stop's event", () => { return true }) expect(await rig.stop()).toMatchObject({ ok: true, value: { cancelled: true } }) - release() await new Promise((resolve) => setTimeout(resolve, 250)) expect(await rig.handoff(held)).toBeUndefined() expect(await rig.queuePause()).toEqual({ reason: 'stopped' }) @@ -158,11 +150,14 @@ describe("a Stop's event", () => { rig = await createQueuedMessageTestRig() expect(await rig.stop()).toMatchObject({ ok: true, value: { cancelled: false } }) expect(stopEvents()).toEqual([]) - const release = holdStart() - const waiting = rig.send('waits for the start') - await eventually(async () => expect(await rig.submission(waiting.id)).toBeDefined()) - expect(await rig.stop()).toMatchObject({ ok: true, value: { cancelled: true } }) + // Held, the send is accepted and the Stop runs next, ahead of the handover the send asks for. + const release = holdLane(rig.host, HOST_TEST_SESSION) + const waiting = rig.send('waits for its handover') + const stopped = rig.stop() release() + expect(await stopped).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(await rig.submission(waiting.id)).toMatchObject({ dispatchState: 'rejected' }) + expect(rig.dispatch).not.toHaveBeenCalled() expect(rig.cancelTurn).not.toHaveBeenCalled() expect(stopEvents()).toEqual([ { reason: 'user-stop', caller: QUEUED_RIG_CALLER.callerKey, at: expect.any(Number) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts index 8956e2d84fc..192837888d1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts @@ -87,9 +87,9 @@ function invalid( } /** A thrown adapter error is indistinguishable from a lost reply, so it settles as `unknown` - * rather than as a rejection — unless the child had not proven its start. Such a child has - * accepted nothing (input is written only after it initializes), so a dispatch it could not - * take is provably unwritten and is rejected with the cause the adapter gave. */ + * rather than as a rejection — unless the child had not proven its start. A dispatch to such a + * child throws only when the start failed before the write (a write's own failure is an outcome, + * not a throw), so it is provably unwritten and is rejected with the cause the adapter gave. */ async function dispatchSafely( ctx: AgentSessionHandoverContext, clientMessageId: string, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts new file mode 100644 index 00000000000..5112b6ff74d --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts @@ -0,0 +1,112 @@ +// A message handed to an agent whose start never answers provably never ran: its CLI takes no +// message before it answers initialize. So when the chat ends then, the message is settled as a +// queued one is for the same end (`journal-unsent-send-hold.ts`): a quit or a close keeps a +// person's words as a held card, a person's Stop withdraws it. Against the real host, store and +// journal, with an agent that stays starting. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' +import { QUEUED_MESSAGE_PAUSED_KEPT } from '../../../shared/agent-session-queued-message-wire' +import { + createQueuedMessageTestRig, + eventually, + type QueuedMessageTestRig +} from './structured-agent-session-queued-message-rig.test-fixture' +import { HOST_TEST_SESSION as SESSION } from './structured-agent-session-host-test-data' + +const words = (kind: 'hostRestarted' | 'chatClosed' | 'cancelled') => + agentSessionFailureWords(agentSessionFailureFact(kind), { surface: 'rejection' }) +const KEPT = { state: 'waiting', paused: true } + +let rig: QueuedMessageTestRig + +beforeEach(async () => { + rig = await createQueuedMessageTestRig({ + restartable: true, + starting: true, + startUnanswered: true, + stopEndsSession: true, + recoveryCapsule: true + }) +}) + +afterEach(async () => { + await rig.dispose() +}) + +/** Sent to the starting agent and handed over to it, never echoed. */ +async function handedToHungStart(text: string): Promise { + // No child, so this send starts one that never answers. + await rig.host.close(SESSION, 'evict') + const sent = rig.send(text) + await sent.result + await eventually(async () => expect((await rig.submission(sent.id))?.handedOverAt).toBeDefined()) + expect(rig.host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + return sent.id +} + +describe('a message handed to a start that never answered, then the chat ends', () => { + it('is a held card after a quit, rejected as a restart, as a queued one is', async () => { + const id = await handedToHungStart('kept through quit') + await rig.quitRestartHostProcess() + + expect(await rig.submission(id)).toMatchObject({ + dispatchState: 'rejected', + ...words('hostRestarted'), + keptAsQueuedMessageId: id + }) + expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) + const [card] = + rig.host.collaboratorsForTests().sessions.get(SESSION)?.journal.queuedMessages.list() ?? [] + expect(card).toMatchObject({ holdReason: QUEUED_MESSAGE_PAUSED_KEPT }) + // Nothing ran, so there is nothing to resume: the card alone holds the words. + expect(await rig.restartOffers()).toEqual([]) + }) + + it('is offered for resume after a quit once the start answered, as the control', async () => { + await rig.dispose() + rig = await createQueuedMessageTestRig({ + restartable: true, + starting: true, + stopEndsSession: true, + recoveryCapsule: true + }) + const id = await handedToHungStart('may have run') + await rig.quitRestartHostProcess() + + expect(await rig.restartOffers()).toEqual([ + { sessionId: SESSION, work: { kind: 'submission', id } } + ]) + }) + + it.each(['user-close', 'evict'] as const)( + 'is a held card after a %s, rejected as closed, as a queued one is', + async (cause) => { + const id = await handedToHungStart('kept at close') + await rig.host.close(SESSION, cause) + rig.crashRestartHostProcess() + + expect(await rig.submission(id)).toMatchObject({ + dispatchState: 'rejected', + ...words('chatClosed'), + keptAsQueuedMessageId: id + }) + expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) + } + ) + + it("is withdrawn by a person's Stop, as a queued one is, and kept as no card", async () => { + const id = await handedToHungStart('stopped') + + expect(await rig.stop()).toMatchObject({ ok: true, value: { cancelled: true } }) + + await eventually(async () => + expect(await rig.submission(id)).toMatchObject({ + dispatchState: 'rejected', + ...words('cancelled') + }) + ) + expect(await rig.drafts()).toEqual([]) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.ts index defcc402af8..d3a658d4265 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unsent-send-hold.test.ts @@ -73,22 +73,48 @@ function sendRequest(text: string, delivery?: 'queue-if-active') { } } -/** Accepted while the agent is starting, and never handed over: its start never finishes. */ +/** Lets the spawn `acceptWhileStarting` holds finish; until then the message stays queued. */ +let releaseStart = (): void => {} + +/** Accepted while the agent is starting, and never handed over: Orca stops during its spawn. */ async function acceptWhileStarting( - request: Parameters[1] + request: Parameters[1], + // Sent with it, so each is accepted before the spawn takes the chat's lane. + ...alsoQueued: Parameters[1][] ): Promise { // No child, so this send must start one. await rig.host.close(SESSION, 'evict') - const startsBefore = rig.awaitStarted.mock.calls.length - rig.awaitStarted.mockImplementationOnce(() => new Promise(() => undefined)) - expect(await rig.host.send(QUEUED_RIG_CALLER, request)).toMatchObject({ - ok: true, - value: { submission: { dispatchState: 'pending', handoverRecorded: true } } - }) - await eventually(() => expect(rig.awaitStarted.mock.calls.length).toBeGreaterThan(startsBefore)) + const startsBefore = rig.starts.mock.calls.length + releaseStart = rig.holdNextStart() + const sent = await Promise.all( + [request, ...alsoQueued].map((each) => rig.host.send(QUEUED_RIG_CALLER, each)) + ) + for (const result of sent) { + expect(result).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'pending', handoverRecorded: true } } + }) + } + await eventually(() => expect(rig.starts.mock.calls.length).toBeGreaterThan(startsBefore)) return request.envelope.clientOperationId } +/** Closes the chat while its spawn is held: the close takes the lane once the spawn returns. */ +async function closeWhileStarting(cause: 'user-close' | 'evict'): Promise { + const closed = rig.host.close(SESSION, cause) + releaseStart() + releaseStart = () => {} + await closed +} + +/** The app quits: delivery stops first, so the held spawn finishing hands nothing over. */ +async function quitRestart(): Promise { + const quit = rig.quitRestartHostProcess() + releaseStart() + releaseStart = () => {} + await quit +} + function journal(): AgentSessionJournal { const open = rig.host.collaboratorsForTests().sessions.get(SESSION)?.journal if (!open) { @@ -105,7 +131,7 @@ function dispatchedTexts(): string[] { describe('a message accepted while the agent starts, then Orca stops', () => { it.each([ - { how: 'quit', restart: () => rig.quitRestartHostProcess() }, + { how: 'quit', restart: () => quitRestart() }, { how: 'crash', restart: async () => rig.crashRestartHostProcess() } ])('is a held card after a $how, and its submission is rejected unseen', async ({ restart }) => { const id = await acceptWhileStarting(sendRequest('hello after restart')) @@ -137,7 +163,7 @@ describe('a message accepted while the agent starts, then Orca stops', () => { it('quit writes nothing for it: no closed-chat rejection, still queued until the next open', async () => { const id = await acceptWhileStarting(sendRequest('kept through quit')) - await rig.quitRestartHostProcess() + await quitRestart() // Read beside the host, without opening the chat through it: the row the quit left. const leftover = await peekSubmission(id) expect(leftover).toMatchObject({ dispatchState: 'pending', handoverRecorded: true }) @@ -145,9 +171,8 @@ describe('a message accepted while the agent starts, then Orca stops', () => { }) it('keeps several in the order they were accepted, ahead of the cards already queued', async () => { - const first = await acceptWhileStarting(sendRequest('first')) const second = sendRequest('second') - expect(await rig.host.send(QUEUED_RIG_CALLER, second)).toMatchObject({ ok: true }) + const first = await acceptWhileStarting(sendRequest('first'), second) await rig.crashRestartHostProcess() expect(await rig.drafts()).toEqual([ @@ -160,7 +185,7 @@ describe('a message accepted while the agent starts, then Orca stops', () => { describe('only an action on the card releases a kept card', () => { it('a later message is delivered alone; Resume sends nothing; its own Send sends it once', async () => { const id = await acceptWhileStarting(sendRequest('the kept words')) - await rig.quitRestartHostProcess() + await quitRestart() // The person types the same words again, as one who cannot see cards would. const retyped = rig.send('the kept words') @@ -187,10 +212,9 @@ describe('only an action on the card releases a kept card', () => { }) it('Send now sends it at once, and Delete removes it', async () => { - const sent = await acceptWhileStarting(sendRequest('send me now')) const deleted = sendRequest('delete me') - await rig.host.send(QUEUED_RIG_CALLER, deleted) - await rig.quitRestartHostProcess() + const sent = await acceptWhileStarting(sendRequest('send me now'), deleted) + await quitRestart() expect(await rig.deleteQueued(deleted.envelope.clientOperationId)).toMatchObject({ ok: true, @@ -209,7 +233,7 @@ describe('only an action on the card releases a kept card', () => { it.each(['Edit', 'Delete'] as const)('%s leaves no trace of the kept send', async (action) => { const request = sendRequest('the kept words') const id = await acceptWhileStarting(request) - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) expect(await rig.deleteQueued(id)).toMatchObject({ ok: true, value: { deleted: true } }) const edited = action === 'Edit' ? rig.send('the edited words') : null @@ -263,7 +287,7 @@ describe('only an action on the card releases a kept card', () => { // After a second restart the kept card is another process's, yet it pauses nothing else. it('a later restart leaves the cards queued after it unpaused', async () => { const id = await acceptWhileStarting(sendRequest('kept twice over')) - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) rig.crashRestartHostProcess() const working = await rig.workingSend() @@ -280,7 +304,7 @@ describe('only an action on the card releases a kept card', () => { it('stays kept when its own Send is cut short by another quit', async () => { const id = await acceptWhileStarting(sendRequest('the kept words')) - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) // A /compact runs, so the Send now waits behind it, and Orca quits before it is handed over. const fields = { command: 'compact' as const } @@ -294,7 +318,7 @@ describe('only an action on the card releases a kept card', () => { expect(await rig.sendNow(id)).toMatchObject({ ok: true }) await new Promise((resolve) => setTimeout(resolve, 100)) expect((await rig.handoff(id))?.handedOverAt).toBeUndefined() - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) expect(journal().queuedMessages.get(id)?.holdReason).toBe(QUEUED_MESSAGE_PAUSED_KEPT) @@ -328,7 +352,7 @@ describe('cards queued behind a working turn, then Orca stops', () => { value: { queued: { state: 'waiting' } } }) if (how === 'quit') { - await rig.quitRestartHostProcess() + await quitRestart() } else { rig.crashRestartHostProcess() } @@ -423,7 +447,7 @@ describe('the queue at a quit', () => { expect(await rig.sendNow(pushed.id)).toMatchObject({ ok: true }) await new Promise((resolve) => setTimeout(resolve, 100)) expect((await rig.handoff(pushed.id))?.handedOverAt).toBeUndefined() - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([ { messageId: pushed.id, ...KEPT }, @@ -444,7 +468,7 @@ describe('a message accepted while the agent starts, then the chat closes', () = 'after a %s it is a held card the reopened chat shows, rejected as closed', async (cause) => { const id = await acceptWhileStarting(sendRequest('kept at close')) - await rig.host.close(SESSION, cause) + await closeWhileStarting(cause) rig.crashRestartHostProcess() expect(await rig.submission(id)).toMatchObject({ dispatchState: 'rejected', ...CHAT_CLOSED }) @@ -478,7 +502,7 @@ describe('the same send arriving again after the restart', () => { it('a client that never asked to queue is answered from its own record: one card, still held, nothing sent', async () => { const request = sendRequest('sent again by the outbox') const id = await acceptWhileStarting(request) - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toEqual([{ messageId: id, ...KEPT }]) const submissionsBefore = (await rig.host.journalSnapshot(SESSION)).submissions.length @@ -590,7 +614,7 @@ describe('what is not kept', () => { } return resolve.apply(this, args) }) - await rig.quitRestartHostProcess() + await quitRestart() // The open's write failed; a new send wakes the delivery loop, whose first step settles it. const next = rig.send('wakes the loop') await next.result @@ -603,7 +627,7 @@ describe('what is not kept', () => { describe('/clear carries a kept card still held', () => { it('a turn in the new conversation never releases it', async () => { const id = await acceptWhileStarting(sendRequest('carried through clear')) - await rig.quitRestartHostProcess() + await quitRestart() expect(await rig.drafts()).toHaveLength(1) const fields = { command: 'clear' as const } const cleared = await rig.host.conversationCommand(QUEUED_RIG_CALLER, { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts index 9f42431e773..8279cf83d18 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-working-at-teardown.ts @@ -49,6 +49,7 @@ import { newestStructuredAgentSessionTurn } from '../../../shared/structured-agent-session-live-turn' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { isUnansweredHandedOverSubmission } from '../agent-session-journal/journal-unsent-send-hold' import { structuredAgentSessionShownStatus } from './structured-agent-session-shown-work' /** A send Orca journaled that the provider has neither opened a turn for nor refused. Mirrors the @@ -172,6 +173,8 @@ export function structuredAgentSessionWorkingAtStop(input: { getRecord: (sessionId: string) => AgentSessionRecord | null /** The session's child records, the same read the status feed publishes. */ childWork: (sessionId: string) => readonly AgentChildWorkView[] | undefined + /** False when the session's live child never answered its start; the exit reads the same fact. */ + startAnswered?: (sessionId: string) => boolean | undefined trigger: AgentSessionResumeTrigger /** Stable teardown identity for continuation deduplication, not launch ancestry. */ teardownId: string @@ -182,9 +185,13 @@ export function structuredAgentSessionWorkingAtStop(input: { return null } const snapshot = session.journal.snapshot() - // A queued message reached no agent, so it is no work to resume: the next open settles it. + // A queued message reached no agent, and one handed to a start that never answered ran nowhere: + // neither is work to resume. The next open or the exit keeps it as unsent. + const unanswered = input.startAnswered?.(sessionId) === false const handedOver = snapshot.submissions.filter( - (submission) => !isQueuedAgentJournalSubmission(submission) + (submission) => + !isQueuedAgentJournalSubmission(submission) && + !(unanswered && isUnansweredHandedOverSubmission(submission)) ) const childWork = input.childWork(sessionId) const status = structuredAgentSessionShownStatus( diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts index 56515bbe6c0..9664aaf6557 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts @@ -371,23 +371,26 @@ it('refuses the command for an agent that does not declare compaction, whatever it('refuses the command at handover when the provider opened a turn meanwhile (B3)', async () => { await attach() const events = state.acquire.mock.calls.at(-1)?.[0].events - // The provider starts a turn of its own after acceptance, before the command is handed over. - Object.assign(state.host.deps.adapter, { - awaitStarted: vi.fn(async () => { - events?.appendItem( - { provider: 'codex', threadId: THREAD, turnId: 'provider-turn', ordinal: 0 }, - { kind: 'turn', turnId: 'provider-turn', state: 'running' }, - { turnScope: AGENT_JOURNAL_THREAD_SCOPE, lifecycle: true } - ) - }) - }) const params = compactParams() + // Held, the command is accepted, then the provider starts a turn of its own, both ahead of the + // handover the acceptance asks for. + const held = Promise.withResolvers() + void state.host['tasks'].serialize(SESSION, () => held.promise) + const commanded = state.host.conversationCommand(CALLER, params) + void state.host['tasks'].serialize(SESSION, async () => { + events?.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'provider-turn', ordinal: 0 }, + { kind: 'turn', turnId: 'provider-turn', state: 'running' }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE, lifecycle: true } + ) + }) + held.resolve() const refused = { kind: 'commandRefused', refusal: { code: 'agent_session_operation_invalid', details: { reason: 'turnActive' } } } - await expect(state.host.conversationCommand(CALLER, params)).resolves.toMatchObject({ + await expect(commanded).resolves.toMatchObject({ ok: true, value: { state: 'completed', error: "This command didn't run. Try it again.", failure: refused } }) diff --git a/src/main/runtime/claude-structured-retired-saved-model.test.ts b/src/main/runtime/claude-structured-retired-saved-model.test.ts new file mode 100644 index 00000000000..138a240984d --- /dev/null +++ b/src/main/runtime/claude-structured-retired-saved-model.test.ts @@ -0,0 +1,141 @@ +// A chat's saved model rides the launch unchecked. When the CLI answers a turn by saying that model +// does not exist, the record drops it, so the next start runs the CLI's own default; the user's +// message and the CLI's own answer stay as they are. Against the production runtime, adapter, +// record store and host, with only the CLI process scripted. + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import { claudeSessionIdForOrcaSession } from '../claude/claude-structured-launch-resolution' +import type { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { createScriptedClaudeRuntime } from './structured-claude-scripted-runtime-test-support' + +const SESSION = 'claude-retired-saved-model' +const CALLER = { callerKey: 'client-1' } +const RETIRED = 'claude-retired-1' + +let claude = createScriptedClaudeRuntime([SESSION]) + +afterEach(async () => { + await claude.dispose() + claude = createScriptedClaudeRuntime([SESSION]) +}) + +/** The reply Claude Code 2.1.280 sends when its `--model` names a model that does not exist. */ +function modelNotFoundReply(uuid: string, parentToolUseId: string | null = null) { + return { + type: 'assistant', + uuid, + session_id: claudeSessionIdForOrcaSession(SESSION), + parent_tool_use_id: parentToolUseId, + error: 'model_not_found', + is_api_error_message: true, + message: { + model: '', + role: 'assistant', + content: [ + { + type: 'text', + text: `There's an issue with the selected model (${RETIRED}). It may not exist or you may not have access to it.` + } + ] + } + } +} + +async function startedWith(options: Record): Promise { + claude.behave(SESSION, { sendsNoStartFrame: true }) + const host = await claude.install() + await expect( + host.attach(CALLER, claude.attachParams(SESSION, null, { options })) + ).resolves.toMatchObject({ ok: true }) + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') + ) + return host +} + +describe("a Claude chat whose saved model the CLI says doesn't exist", () => { + it('drops the model from the record, keeping the rest, so the next start runs the default', async () => { + const host = await startedWith({ model: RETIRED, effort: 'high' }) + expect(claude.child(SESSION).launch.options.model).toBe(RETIRED) + expect(host.deps.store.getRecord(SESSION)?.options?.model).toBe(RETIRED) + + claude.child(SESSION).handlers.onMessage?.(modelNotFoundReply('reply-1')) + + await vi.waitFor(() => + expect(host.deps.store.getRecord(SESSION)?.options).not.toHaveProperty('model') + ) + expect(host.deps.store.getRecord(SESSION)?.options).toMatchObject({ effort: 'high' }) + }) + + it("keeps a model the reply did not come from: a subagent's own error", async () => { + const host = await startedWith({ model: RETIRED }) + + claude.child(SESSION).handlers.onMessage?.(modelNotFoundReply('reply-1', 'toolu_subagent')) + await host.flushStreamedEvents(SESSION) + + expect(host.deps.store.getRecord(SESSION)?.options?.model).toBe(RETIRED) + }) + + // The CLI's word is about the model it was launched with; a pick made since is the user's. + it('keeps a model the user picked while the heal was on its way', async () => { + const host = await startedWith({ model: RETIRED }) + const record = () => host.deps.store.getRecord(SESSION) + const changed = await host.setOption(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: `${Date.now()}-${'1'.padStart(32, '0')}`, + expectedRuntimeFence: record()?.lease.runtimeFence ?? 0, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.setOption', + sessionId: SESSION, + fields: { key: 'model', value: 'opus' } + }) + }, + key: 'model', + value: 'opus' + }) + expect(changed, JSON.stringify(changed)).toMatchObject({ ok: true }) + expect(record()?.options?.model).toBe('opus') + + await host.handleAdapterEvent({ + type: 'options-skipped', + sessionId: SESSION, + fence: record()?.lease.runtimeFence ?? 0, + acquisitionGeneration: 'generation-heal', + options: { model: RETIRED } + }) + + expect(record()?.options?.model).toBe('opus') + }) + + // A slow settings readback lets the turn's error land first; `started` then reports the CLI's + // own model, which is still the retired one, and must not write it back. + it('does not let a start that lands after the heal write the retired model back', async () => { + claude.behave(SESSION, { + sendsNoStartFrame: true, + startupSettingsReadHangs: true, + controlTimeoutMs: 300 + }) + const host = await claude.install() + await expect( + host.attach(CALLER, claude.attachParams(SESSION, null, { options: { model: RETIRED } })) + ).resolves.toMatchObject({ ok: true }) + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) + claude.child(SESSION).handlers.onMessage?.({ + type: 'system', + subtype: 'init', + session_id: claudeSessionIdForOrcaSession(SESSION), + model: RETIRED + }) + claude.child(SESSION).handlers.onMessage?.(modelNotFoundReply('reply-1')) + + await vi.waitFor( + () => expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready'), + { timeout: 5_000 } + ) + expect(host.deps.store.getRecord(SESSION)?.options).not.toHaveProperty('model') + }) +}) diff --git a/src/main/runtime/claude-structured-send-held-for-startup.test.ts b/src/main/runtime/claude-structured-send-during-startup.test.ts similarity index 69% rename from src/main/runtime/claude-structured-send-held-for-startup.test.ts rename to src/main/runtime/claude-structured-send-during-startup.test.ts index efb8d3cde16..a50615fbed4 100644 --- a/src/main/runtime/claude-structured-send-held-for-startup.test.ts +++ b/src/main/runtime/claude-structured-send-during-startup.test.ts @@ -1,9 +1,10 @@ -// A Claude chat is published the moment its child spawns, before the CLI has answered initialize. -// A send in that window — into a fresh start, or into the restart the delivery loop makes for a -// send after a start that failed — is accepted and stays queued until the child proves its start. -// When the CLI dies first, the queued message is rejected with the CLI's own diagnostic, the chat -// shows the cause once, and nothing is left as a delivery nobody can confirm. Against the production runtime, -// adapter, record store and host, with only the CLI process scripted. +// A Claude chat is published the moment its child spawns, before the CLI has answered initialize, +// and launched with the chat's saved options. A send in that window — into a fresh start, or into +// the restart the delivery loop makes for a send after a start that failed — is written to the CLI +// at once, which takes it behind its own start. When the CLI dies before answering, the message is +// rejected with the CLI's own diagnostic, the chat shows the cause once, and nothing is left as a +// delivery nobody can confirm. Against the production runtime, adapter, record store and host, with +// only the CLI process scripted. import { afterEach, describe, expect, it, vi } from 'vitest' import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' @@ -43,7 +44,7 @@ async function send(host: StructuredAgentSessionHost, text: string): Promise { - it('restarts once, rejects the held message with the diagnostic when that start dies too, then delivers once the CLI is healthy', async () => { + it('restarts once, rejects the message it wrote with the diagnostic when that start dies too, then delivers once the CLI is healthy', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() await expect(host.attach(CALLER, claude.attachParams(SESSION, null))).resolves.toMatchObject({ @@ -89,10 +90,11 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { expect(await statusRows(host)).toEqual([STARTUP_TEXT]) const releasedFence = fence(host) - // The delivery loop asks for the child back and the message waits for its start; the CLI - // dies again first. + // The delivery loop asks for the child back and writes the message to it at once; the CLI + // dies again before it answers initialize. const held = await send(host, 'hello?') await vi.waitFor(() => expect(claude.children(SESSION)).toHaveLength(2)) + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) await vi.waitFor(() => expect(host.deps.store.getRecord(SESSION)?.lease.claimStatus).toBe('live') ) @@ -111,7 +113,6 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { // The restart moved the fence twice: its acquisition, and the exit that released it. expect(fence(host)).toBe(releasedFence + 2) expect(claude.children(SESSION)).toHaveLength(2) - expect(claude.child(SESSION).calls).not.toContain('send') // The user signs in and retries: one restart, proven, written to the CLI. claude.behave(SESSION, {}) @@ -127,29 +128,53 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { }) describe('a send while the first Claude start is still answering initialize', () => { - it('is queued, and written once the CLI proves its start', async () => { + it('is written to the CLI at once, before it answers', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() await host.attach(CALLER, claude.attachParams(SESSION, null)) - await send(host, 'hello') - expect(claude.child(SESSION).calls).not.toContain('send') + const sent = await send(host, 'hello') + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') - // The CLI answers: startup lands and the queued message is written to the proven child. + // The CLI answers: the start lands under the message already written, and nothing fails. claude.child(SESSION).answerInit() - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') + ) + expect(claude.child(SESSION).calls.filter((call) => call === 'send')).toHaveLength(1) + expect((await submission(host, sent))?.dispatchState).not.toBe('rejected') expect(claude.children(SESSION)).toHaveLength(1) expect(await statusRows(host)).toEqual([]) }) - it('is rejected with the diagnostic when the CLI dies first, and restarts nothing', async () => { + // Only a SessionStart hook makes the CLI name its session before the first turn, and that hook + // comes from status hooks a user can turn off; the initialize answer alone starts the chat. + it('is written at once, and the start lands, with no start frame before the first turn', async () => { + claude.behave(SESSION, { initHangs: true, sendsNoStartFrame: true }) + const host = await claude.install() + await host.attach(CALLER, claude.attachParams(SESSION, null)) + + await send(host, 'hello') + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + claude.child(SESSION).answerInit() + + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') + ) + expect(claude.children(SESSION)).toHaveLength(1) + expect(await statusRows(host)).toEqual([]) + }) + + it('is rejected with the diagnostic when the CLI dies before answering, and restarts nothing', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() await host.attach(CALLER, claude.attachParams(SESSION, null)) const startedFence = fence(host) const held = await send(host, 'hello') + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) await failLatestStart(host, 1) await vi.waitFor(async () => @@ -163,6 +188,5 @@ describe('a send while the first Claude start is still answering initialize', () expect(await statusRows(host)).toEqual([STARTUP_TEXT]) expect(fence(host)).toBe(startedFence + 1) expect(claude.children(SESSION)).toHaveLength(1) - expect(claude.child(SESSION).calls).not.toContain('send') }) }) diff --git a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts index c5b21f9027f..3eb815bca50 100644 --- a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts +++ b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts @@ -1,6 +1,6 @@ -// A send is accepted into a chat whose Claude child is gone, and its delivery restarts the child. -// When that child dies before it proves its start, the message was never handed to it — delivery -// waits for the start — so the send settles `rejected` with the child's own diagnostic, never as a +// A send is accepted into a chat whose Claude child is gone, and its delivery restarts the child +// and writes the message to it at once. When that child dies before it proves its start, it never +// ran the message, so the send settles `rejected` with the child's own diagnostic, never as a // delivery nobody can confirm, and a client that was subscribed the whole time receives the // failure row and the rejected submission over the wire. Against the production runtime, adapter, // record store and host, with only the CLI scripted. @@ -140,6 +140,8 @@ describe('a send whose restarted Claude child dies before it proves its start', const sent = await send(host, 'hello?') // Accepted: the answer comes before the restart it needs. expect(answered.get(sent)).toBe('pending') + await eventually(() => expect(claude.children(SESSION)).toHaveLength(2)) + await eventually(() => expect(claude.child(SESSION).calls).toContain('send')) await failLatestStart(host, 2) // Provably not delivered, with the cause; not "unconfirmed". @@ -154,7 +156,6 @@ describe('a send whose restarted Claude child dies before it proves its start', expect(await statusRows(host)).toEqual([STARTUP_TEXT, STARTUP_TEXT]) expect(fence(host)).toBe(releasedFence + 2) expect(claude.children(SESSION)).toHaveLength(2) - expect(claude.child(SESSION).calls).not.toContain('send') // Retry under a new id: one restart, and once the CLI is healthy the message is written. claude.behave(SESSION, {}) diff --git a/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts b/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts index 863cbaeaebf..65215d60996 100644 --- a/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts +++ b/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts @@ -1,7 +1,7 @@ -// A Claude start can fail on Orca's side while the CLI is still running: a saved option it can't -// restore, or an init frame naming another session. Orca ends that child itself, so the chat must -// not say Claude stopped on its own; only an exit Orca saw says that. Against the production -// runtime, adapter, record store and host, with only the CLI process scripted. +// A Claude start can fail on Orca's side while the CLI is still running: an init frame naming +// another session. Orca ends that child itself, so the chat must not say Claude stopped on its own; +// only an exit Orca saw says that. Against the production runtime, adapter, record store and host, +// with only the CLI process scripted. import { afterEach, describe, expect, it, vi } from 'vitest' import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' @@ -75,7 +75,8 @@ async function released(host: StructuredAgentSessionHost): Promise { } describe('a Claude start that Orca fails while the CLI is still running', () => { - it('reads as a start that could not happen when a saved option cannot be restored', async () => { + // Saved options ride the launch, so no option write runs at startup that could fail it. + it('starts with a saved option the CLI would not take as a write', async () => { claude.behave(SESSION, { optionWritesFail: true }) const host = await claude.install() await expect( @@ -84,21 +85,23 @@ describe('a Claude start that Orca fails while the CLI is still running', () => claude.attachParams(SESSION, null, { options: { permissionMode: 'plan' } }) ) ).resolves.toMatchObject({ ok: true }) - await released(host) + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') + ) - expect(claude.child(SESSION).calls).toContain('set_permission_mode') - expect(await failureRows(host)).toEqual([ - { text: expect.stringMatching(/^Claude couldn't start\./), kind: 'startFailed' } - ]) + expect(claude.child(SESSION).launch.options.permissionMode).toBe('plan') + expect(claude.child(SESSION).calls).not.toContain('set_permission_mode') + expect(await failureRows(host)).toEqual([]) }) - it('rejects a held message as a start that could not happen when init names another session', async () => { + it('rejects a message it wrote as a start that could not happen when init names another session', async () => { claude.behave(SESSION, { initHangs: true, initNamesForeignSession: true }) const host = await claude.install() await expect(host.attach(CALLER, claude.attachParams(SESSION, null))).resolves.toMatchObject({ ok: true }) const held = await send(host, 'hello') + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) claude.child(SESSION).answerInit() await released(host) @@ -113,7 +116,6 @@ describe('a Claude start that Orca fails while the CLI is still running', () => expect(await failureRows(host)).toEqual([ { text: expect.stringMatching(/^Claude couldn't start\./), kind: 'startFailed' } ]) - expect(claude.child(SESSION).calls).not.toContain('send') }) it('still says Claude stopped when the CLI exits on its own before its start lands', async () => { diff --git a/src/main/runtime/claude-structured-startup-unanswered-control-request.test.ts b/src/main/runtime/claude-structured-startup-unanswered-control-request.test.ts index 56ba0061883..e431658a078 100644 --- a/src/main/runtime/claude-structured-startup-unanswered-control-request.test.ts +++ b/src/main/runtime/claude-structured-startup-unanswered-control-request.test.ts @@ -1,10 +1,8 @@ -// A Claude start has no deadline of its own, but each option write the restore replays, and -// startup's own settings read, is a control request under the ordinary request deadline. A CLI -// that answers initialize and then never answers one of those used to fault the whole session -// when that deadline fired: a start that was merely slow died with the deadline's error as its -// cause. Now the unanswered request is skipped and startup lands on the CLI's own values, while -// the saved choice stays saved for the next start to retry. Against -// the production runtime, adapter, record store and host, with only the CLI process scripted. +// A Claude start asks the CLI one control request after initialize, its settings read, under the +// ordinary request deadline; the chat's saved options ride the launch, so no option write runs at +// startup. A CLI that answers initialize and then answers no control request still starts, on its +// launch options, and the saved choices stay saved. Against the production runtime, adapter, record +// store and host, with only the CLI process scripted. import { afterEach, describe, expect, it, vi } from 'vitest' import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' @@ -16,6 +14,8 @@ import { createScriptedClaudeRuntime } from './structured-claude-scripted-runtim const SESSION = 'claude-startup-unanswered-control' const CALLER = { callerKey: 'client-1' } const DEADLINE_MS = 50 +/** As live: no frame before the first turn names a model, so only a turn reports one. */ +const LIVE_START = { sendsNoStartFrame: true } let claude = createScriptedClaudeRuntime([SESSION]) let operations = 0 @@ -105,8 +105,8 @@ async function sendTo( } describe('a Claude start whose CLI answers initialize but not a control request', () => { - it('lands with the unanswered option write skipped instead of faulting at the deadline, and keeps the saved choice', async () => { - claude.behave(SESSION, { optionWritesHang: true, controlTimeoutMs: DEADLINE_MS }) + it('launches with the saved choices, writes none of them, and keeps them saved', async () => { + claude.behave(SESSION, { ...LIVE_START, optionWritesHang: true, controlTimeoutMs: DEADLINE_MS }) vi.spyOn(console, 'warn').mockImplementation(() => {}) const host = await claude.install() const saved = { model: 'sonnet', permissionMode: 'plan' } @@ -114,28 +114,29 @@ describe('a Claude start whose CLI answers initialize but not a control request' host.attach(CALLER, claude.attachParams(SESSION, null, { options: saved })) ).resolves.toMatchObject({ ok: true }) - // The restore asked; the CLI never answered; startup went on without it. - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('set_model')) - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('set_permission_mode')) await vi.waitFor(() => expect(record(host)?.lease.claimStatus).toBe('live'), { timeout: DEADLINE_MS * 40 }) - // The live child runs on the CLI's own model; silence is not a refusal, so the saved - // choice is neither replaced by that value nor dropped, and the next start retries it. + expect(claude.child(SESSION).launch.options).toMatchObject({ + model: 'sonnet', + permissionMode: 'plan' + }) + // The record keeps the saved choices through the start. await vi.waitFor(() => expect(record(host)?.options).toEqual({ ...saved, effort: 'high' }), { timeout: DEADLINE_MS * 40 }) expect(host.deps.adapter.readOptionRestoreFailures?.(SESSION)).toEqual([]) expect(await statusRows(host)).toEqual([]) expect(claude.children(SESSION)).toHaveLength(1) + expect(claude.child(SESSION).calls).not.toContain('set_model') + expect(claude.child(SESSION).calls).not.toContain('set_permission_mode') - // The proven child takes the next message. await send(host, 'hello') await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) }) - it('keeps the unanswered saved choice when the user later changes a different option', async () => { - const behavior = { optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } + it('keeps the launched saved choice when the user later changes a different option', async () => { + const behavior = { ...LIVE_START, optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } claude.behave(SESSION, behavior) vi.spyOn(console, 'warn').mockImplementation(() => {}) const host = await claude.install() @@ -155,10 +156,10 @@ describe('a Claude start whose CLI answers initialize but not a control request' expect(record(host)?.options).toMatchObject({ model: 'sonnet', permissionMode: 'plan' }) }) - it('replays the unanswered saved model after a turn reports another model, another option changes, and the chat is cleared', async () => { + it('launches the saved model again after a turn reports another model, another option changes, and the chat is cleared', async () => { const sessions = [SESSION] claude = createScriptedClaudeRuntime(sessions) - const behavior = { optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } + const behavior = { ...LIVE_START, optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } claude.behave(SESSION, behavior) vi.spyOn(console, 'warn').mockImplementation(() => {}) const host = await claude.install() @@ -205,14 +206,17 @@ describe('a Claude start whose CLI answers initialize but not a control request' }) const replacement = cleared.ok ? cleared.value.replacementSessionId! : '' sessions.push(replacement) + claude.behave(replacement, LIVE_START) // The cleared chat starts nothing until its first message. expect(claude.children(replacement)).toEqual([]) await sendTo(host, replacement, 'first message') - // Started fresh under its own derived id, and it replays the saved model rather than the one + // Started fresh under its own derived id, launched with the saved model rather than the one // the turn reported. - await vi.waitFor(() => expect(claude.child(replacement).calls).toContain('set_model')) + await vi.waitFor(() => expect(claude.children(replacement)).toHaveLength(1)) expect(claude.child(replacement).launch.options).toMatchObject({ - sessionId: claudeSessionIdForOrcaSession(replacement) + sessionId: claudeSessionIdForOrcaSession(replacement), + model: 'sonnet', + permissionMode: 'plan' }) expect(claude.child(replacement).launch.options.resume).toBeUndefined() await vi.waitFor(() => @@ -225,8 +229,8 @@ describe('a Claude start whose CLI answers initialize but not a control request' expect(record(host)?.options).toEqual({ model: 'sonnet', permissionMode: 'plan' }) }) - it('replaces the unanswered saved model with the one the user then sets', async () => { - const behavior = { optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } + it('replaces the launched saved model with the one the user then sets', async () => { + const behavior = { ...LIVE_START, optionWritesHang: true, controlTimeoutMs: DEADLINE_MS } claude.behave(SESSION, behavior) vi.spyOn(console, 'warn').mockImplementation(() => {}) const host = await claude.install() @@ -234,7 +238,7 @@ describe('a Claude start whose CLI answers initialize but not a control request' host.attach(CALLER, claude.attachParams(SESSION, null, { options: { model: 'sonnet' } })) ).resolves.toMatchObject({ ok: true }) // The record holds the saved model from creation; only the start's own report (effort) says - // startup finished, and an option write before then is refused as still starting. + // startup finished, and an option write before then waits for it. await vi.waitFor( () => expect(record(host)?.options).toEqual({ model: 'sonnet', effort: 'high' }), { timeout: DEADLINE_MS * 40 } diff --git a/src/main/runtime/structured-agent-session-lifecycle-delivery.ts b/src/main/runtime/structured-agent-session-lifecycle-delivery.ts index ba307c2cfb0..f228b96ce35 100644 --- a/src/main/runtime/structured-agent-session-lifecycle-delivery.ts +++ b/src/main/runtime/structured-agent-session-lifecycle-delivery.ts @@ -3,9 +3,9 @@ // Exit recovery runs on one chain so teardown can drain it: exit callbacks arrive from child // process tasks, and a fire-and-forget one could otherwise append after the host flushed and // removed its journal directory. That chain orders nothing across sessions, and a recovery on it -// can run a whole reacquisition, so `started` and the end of a close the host asked for stay off -// it: each takes only its own session's serialized step, and is tracked here so the same drain -// still waits for it. +// can run a whole reacquisition, so `started`, a dropped saved option and the end of a close the +// host asked for stay off it: each takes only its own session's serialized step, and is tracked +// here so the same drain still waits for it. import type { StructuredAgentSessionLifecycleEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' @@ -27,7 +27,7 @@ export function createStructuredAgentSessionLifecycleDelivery(input: { await input.handle(event) } catch (error) { input.logger.warn(`delivering a provider ${event.type} event to the host failed`, { - scope: event.type === 'started' ? 'lifecycle-started' : 'lifecycle-exit', + scope: event.type === 'ended' ? 'lifecycle-exit' : `lifecycle-${event.type}`, sessionId: event.sessionId, error }) @@ -35,7 +35,7 @@ export function createStructuredAgentSessionLifecycleDelivery(input: { } return { deliver: (event) => { - if (event.type === 'started' || event.cause === 'requested-close') { + if (event.type !== 'ended' || event.cause === 'requested-close') { // Called now, so the step is queued on its own session's lane, behind nothing of another's. const settling = settle(event) settlingStarts.add(settling) diff --git a/src/main/runtime/structured-claude-runtime-adapter.ts b/src/main/runtime/structured-claude-runtime-adapter.ts index 4212b9788b8..288c7a7ddd5 100644 --- a/src/main/runtime/structured-claude-runtime-adapter.ts +++ b/src/main/runtime/structured-claude-runtime-adapter.ts @@ -51,7 +51,7 @@ export type StructuredClaudeRuntimeAdapterDeps = { export function structuredClaudeLifecycleEvent( event: ClaudeStructuredSessionEvent ): StructuredAgentSessionLifecycleEvent | null { - if (event.type === 'started') { + if (event.type === 'started' || event.type === 'options-skipped') { return event } // Every exit of a child with an identity, expected or not: the host ends that child's record. @@ -71,7 +71,8 @@ export function structuredClaudeLifecycleEvent( acquisitionGeneration: event.acquisitionGeneration, // The instant the translator ended the open turn at; the host reads the exit's turn by it. ...(event.observedAt === undefined ? {} : { observedAt: event.observedAt }), - ...(event.startupUnproven ? { startupUnproven: event.startupUnproven } : {}) + ...(event.startupUnproven ? { startupUnproven: event.startupUnproven } : {}), + ...(event.startupUnanswered ? { startupUnanswered: event.startupUnanswered } : {}) } } return null diff --git a/src/main/runtime/structured-claude-scripted-runtime-test-support.ts b/src/main/runtime/structured-claude-scripted-runtime-test-support.ts index b379afbe1e2..77e66d31d63 100644 --- a/src/main/runtime/structured-claude-scripted-runtime-test-support.ts +++ b/src/main/runtime/structured-claude-scripted-runtime-test-support.ts @@ -43,11 +43,12 @@ export type ScriptedClaudeBehavior = { optionWritesHang?: boolean /** Startup's own settings read goes unanswered. */ startupSettingsReadHangs?: boolean - /** Every option write loses its answer while the CLI keeps running (not a refusal), so a - * start that restores one faults. */ + /** Every option write loses its answer while the CLI keeps running (not a refusal). */ optionWritesFail?: boolean /** The init frame names another provider session than the one launched. */ initNamesForeignSession?: boolean + /** No start frame before the first turn, as with no SessionStart hook configured. */ + sendsNoStartFrame?: boolean } export type ScriptedClaudeChild = { @@ -123,7 +124,10 @@ export function createScriptedClaudeRuntime(sessionIds: readonly string[]) { exitVerdict: { root: 'live', tree: 'unverifiable' }, initializationResult: () => { const initialized = { models: [{ value: 'sonnet', displayName: 'Sonnet' }] } - const announce = (): void => + const announce = (): void => { + if (behavior.sendsNoStartFrame) { + return + } handlers.onMessage?.({ type: 'system', subtype: 'init', @@ -131,6 +135,7 @@ export function createScriptedClaudeRuntime(sessionIds: readonly string[]) { model: 'claude-sonnet-5', apiKeySource: 'none' }) + } if (behavior.initHangs) { return new Promise((resolve, reject) => { failInit = reject