From d5e1402ea8cbb2e312eef4927720b71832e39068 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:58:59 -0700 Subject: [PATCH] fix(agent-launch): preserve setup and refusal fallbacks --- .../agent-launch-executor.test.ts | 48 ++++++++++- .../agent-launch/agent-launch-executor.ts | 54 +++++++++++- .../rpc/methods/agent-launch-surfaces.ts | 10 ++- .../methods/agent-launch-worktree-creation.ts | 40 ++++++++- .../runtime/rpc/methods/agent-launch.test.ts | 82 ++++++++++++++++++- src/main/runtime/rpc/methods/agent-launch.ts | 20 +++++ 6 files changed, 246 insertions(+), 8 deletions(-) diff --git a/src/main/agent-launch/agent-launch-executor.test.ts b/src/main/agent-launch/agent-launch-executor.test.ts index d9364863655..092262470a8 100644 --- a/src/main/agent-launch/agent-launch-executor.test.ts +++ b/src/main/agent-launch/agent-launch-executor.test.ts @@ -9,7 +9,11 @@ */ import { describe, expect, it, vi } from 'vitest' -import { executeAgentLaunch, type AgentLaunchExecution } from './agent-launch-executor' +import { + AgentLaunchStructuredSessionRefusedError, + executeAgentLaunch, + type AgentLaunchExecution +} from './agent-launch-executor' import type { AgentLaunchIntent } from '../../shared/agent-launch-intent' const STRUCTURED_PREFERENCE = { @@ -22,6 +26,7 @@ function harness(options: { settings?: Record | null createSupport?: { supported: boolean; reason?: 'agent' | 'remote' | 'wsl' } createSupportThrows?: boolean + structuredCreateError?: Error }) { const calls: string[] = [] const createWorktree = vi.fn( @@ -42,6 +47,9 @@ function harness(options: { }) const createStructuredSession = vi.fn(async () => { calls.push('createStructuredSession') + if (options.structuredCreateError) { + throw options.structuredCreateError + } return { sessionId: 'sess-1', handle: 'handle_structured' } }) const createTerminalAgent = vi.fn(async () => { @@ -124,6 +132,44 @@ describe('a structured launch that creates its own worktree', () => { expect(result.receipt).toMatchObject({ reason: 'structured_support_unknown' }) }) + it('falls back only for a definitive structured refusal after the worktree exists', async () => { + const h = harness({ + structuredCreateError: new AgentLaunchStructuredSessionRefusedError( + 'structured_agent_session_unsupported', + 'unsupported' + ) + }) + const result = await h.run(CREATE_INTENT) + + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createStructuredSession', + 'createTerminalAgent' + ]) + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1' }) + expect(result.receipt).toMatchObject({ + mode: 'terminal', + reason: 'structured_unsupported_on_host' + }) + }) + + it('does not create a duplicate terminal when structured creation is unknown', async () => { + const h = harness({ + structuredCreateError: new AgentLaunchStructuredSessionRefusedError( + 'agent_session_operation_unknown', + 'unknown' + ) + }) + + await expect(h.run(CREATE_INTENT)).rejects.toThrow('unknown') + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createStructuredSession' + ]) + }) + it('strips a stale startupAgent out of a migrated create payload', async () => { const h = harness({}) await h.run({ diff --git a/src/main/agent-launch/agent-launch-executor.ts b/src/main/agent-launch/agent-launch-executor.ts index df07803f12a..3f02aaafb36 100644 --- a/src/main/agent-launch/agent-launch-executor.ts +++ b/src/main/agent-launch/agent-launch-executor.ts @@ -34,6 +34,7 @@ import type { import { withoutReservedAgentCreateFields } from '../../shared/agent-launch-intent' import type { TuiAgent } from '../../shared/tui-agent' import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import { isDefinitiveAgentSessionCreateRefusal } from '../../shared/agent-session-definitive-refusal' import { decideAgentLaunchMode, readAgentLaunchModeSettings, @@ -59,6 +60,17 @@ export type AgentLaunchSurfaceFactory = { }): Promise<{ handle: string; warning?: string }> } +/** A structured create refusal that proves no session was committed, so the launch may downgrade. */ +export class AgentLaunchStructuredSessionRefusedError extends Error { + readonly code: string + + constructor(code: string, message: string) { + super(message) + this.name = 'AgentLaunchStructuredSessionRefusedError' + this.code = code + } +} + /** Creating the workspace, when the intent asks for one. Injected so orchestration keeps recording * its own worktree stages and residual-resource effects around the same call. */ export type AgentLaunchWorkspaceFactory = { @@ -119,7 +131,7 @@ export async function executeAgentLaunch( } execution.onStage?.('mode_settle') - const settled = await resolveAgentLaunchModeOnHost( + let settled = await resolveAgentLaunchModeOnHost( runtime, preflight, placed.worktreeId, @@ -128,7 +140,33 @@ export async function executeAgentLaunch( ) execution.onStage?.('surface_create') - const outcome = await createSurface(execution, placed.worktreeId, settled) + let outcome: AgentLaunchResult['outcome'] + try { + outcome = await createSurface(execution, placed.worktreeId, settled) + } catch (error) { + // The structured create path distinguishes a definitive pre-commit refusal from an unknown + // outcome. Only the former is safe to replace with a terminal in the same workspace; retrying + // after an unknown attach outcome could create two agents. + if ( + settled.mode !== 'structured' || + !(error instanceof AgentLaunchStructuredSessionRefusedError) || + !isDefinitiveAgentSessionCreateRefusal(error.code) + ) { + throw error + } + settled = downgradeAgentLaunchModeForStructuredRefusal(settled, vocabulary) + outcome = await execution.surfaces + .createTerminalAgent({ + worktreeId: placed.worktreeId, + agent: intent.agent, + ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) + }) + .then((terminal) => ({ + kind: 'terminal' as const, + handle: terminal.handle, + ...(terminal.warning ? { warning: terminal.warning } : {}) + })) + } return { outcome, worktreeId: placed.worktreeId, @@ -137,6 +175,18 @@ export async function executeAgentLaunch( } } +function downgradeAgentLaunchModeForStructuredRefusal( + receipt: AgentLaunchModeReceipt, + vocabulary: AgentLaunchModeVocabulary +): AgentLaunchModeReceipt { + return { + mode: 'terminal', + preferred: receipt.preferred, + reason: 'structured_unsupported_on_host', + detail: `Your default is a structured chat session, but the host refused to create one here; started ${vocabulary.terminal} instead.` + } +} + async function resolveWorkspace( execution: AgentLaunchExecution, preflight: AgentLaunchModeReceipt diff --git a/src/main/runtime/rpc/methods/agent-launch-surfaces.ts b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts index 0767dba6885..8d60944b261 100644 --- a/src/main/runtime/rpc/methods/agent-launch-surfaces.ts +++ b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts @@ -12,7 +12,10 @@ import { randomUUID } from 'node:crypto' import { narrowStructuredLaunchSeedOptions } from '../../../../shared/native-chat-session-option-defaults' import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation' import { structuredAgentSessionTabId } from '../../../../shared/structured-agent-session-projection' -import type { AgentLaunchSurfaceFactory } from '../../../agent-launch/agent-launch-executor' +import { + AgentLaunchStructuredSessionRefusedError, + type AgentLaunchSurfaceFactory +} from '../../../agent-launch/agent-launch-executor' import { getStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' import type { RpcContext } from '../core' @@ -47,7 +50,10 @@ export function agentLaunchSurfaceFactory(context: RpcContext): AgentLaunchSurfa activate: true }) if (!created.ok) { - throw new Error(created.refusal.message) + throw new AgentLaunchStructuredSessionRefusedError( + created.refusal.code, + created.refusal.message + ) } return { sessionId: created.value.sessionId, diff --git a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts index ecd4a5608e8..18842b06089 100644 --- a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts +++ b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts @@ -11,6 +11,7 @@ import { buildCliWorkspaceProvenance } from '../../../../shared/cli-workspace-provenance' import type { TuiAgent } from '../../../../shared/tui-agent' +import type { OrcaRuntimeService } from '../../orca-runtime' import { finishAutomationWorkspaceProvenanceRequest, releaseAutomationWorkspaceProvenanceRequest, @@ -27,6 +28,8 @@ type WorktreeCreateParams = Extract< { kind: 'create-worktree' } >['create'] +const STRUCTURED_SETUP_WAIT_TIMEOUT_MS = 60_000 + export function agentLaunchWorkspaceFactory( context: RpcContext, agent: TuiAgent @@ -61,8 +64,16 @@ export function agentLaunchWorkspaceFactory( ), // The launch owns the agent whichever surface it settles on, so the workspace records // it even when no startup terminal was created for it. - createdWithAgent: agent + createdWithAgent: agent, + // Structured sessions have no startup command to sequence behind setup. Provision the + // setup terminal synchronously and attach a completion token so the launch can wait + // before creating the chat surface. + awaitTerminalProvisioning: true, + observeSetupCompletion: true }) + if (!startupAgent) { + await waitForStructuredSetup(runtime, result.setupReceipt) + } finishAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) return { worktreeId: result.worktree.id, @@ -76,3 +87,30 @@ export function agentLaunchWorkspaceFactory( } } } + +async function waitForStructuredSetup( + runtime: Pick, + receipt: Awaited>['setupReceipt'] +): Promise { + if ( + !receipt || + receipt.startupPolicy !== 'wait-for-setup' || + receipt.state !== 'running' || + !receipt.terminalHandle + ) { + return + } + let timer: ReturnType | undefined + try { + await Promise.race([ + runtime.waitForSetupTerminalCompletion(receipt.terminalHandle), + new Promise((resolve) => { + timer = setTimeout(resolve, STRUCTURED_SETUP_WAIT_TIMEOUT_MS) + }) + ]) + } catch { + // Setup completion is evidence, not a reason to strand a launch when the PTY disappears. + } finally { + clearTimeout(timer) + } +} diff --git a/src/main/runtime/rpc/methods/agent-launch.test.ts b/src/main/runtime/rpc/methods/agent-launch.test.ts index c745cff846b..7c6c8bf6784 100644 --- a/src/main/runtime/rpc/methods/agent-launch.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch.test.ts @@ -35,8 +35,14 @@ function runtimeStub( options: { settings?: Record createSupport?: { supported: boolean; reason?: 'agent' | 'remote' | 'wsl' } + setupReceipt?: { + startupPolicy: 'start-immediately' | 'wait-for-setup' + state: 'running' | 'skipped' | 'not_configured' | 'spawn_failed' + terminalHandle?: string + } } = {} ) { + const waitForSetupTerminalCompletion = vi.fn(async () => ({ exitCode: 0 })) return { getClientSettings: vi.fn(() => options.settings ?? STRUCTURED_PREFERENCE), getStructuredAgentSessionCreateSupport: vi.fn( @@ -48,13 +54,17 @@ function runtimeStub( showRepo: vi.fn(async () => ({ id: 'repo-1' })), createManagedWorktree: vi.fn(async (args: Record) => ({ worktree: { id: 'wt-new' }, - startupTerminal: args.startupAgent ? { handle: 'term_agent_first' } : undefined + startupTerminal: args.startupAgent ? { handle: 'term_agent_first' } : undefined, + ...(options.setupReceipt ? { setupReceipt: options.setupReceipt } : {}) })), createTerminal: vi.fn(async () => ({ handle: 'term_1' })), + showTerminal: vi.fn(async (handle: string) => ({ handle, worktreeId: 'wt-7' })), + isTerminalRunningAgent: vi.fn(async () => true), showManagedTerminalWorkspace: vi.fn(async (selector: string) => ({ id: selector.replace(/^id:/, '') })), - ensureStructuredAgentSessionHost: vi.fn(async () => {}) + ensureStructuredAgentSessionHost: vi.fn(async () => {}), + waitForSetupTerminalCompletion } } @@ -179,6 +189,48 @@ describe('what agent.launch accepts', () => { }).success ).toBe(true) }) + + it('validates a reused terminal against the addressed workspace', async () => { + const runtime = runtimeStub() + const result = await launch( + { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' }, + reuseTerminal: { handle: 'term_live' } + }, + runtime + ) + + expect(runtime.showTerminal).toHaveBeenCalledWith('term_live') + expect(runtime.isTerminalRunningAgent).toHaveBeenCalledWith('term_live') + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_live' }) + }) + + it('rejects a reused terminal from a different workspace before launching', async () => { + const runtime = runtimeStub() + runtime.showTerminal.mockResolvedValue({ handle: 'term_live', worktreeId: 'wt-other' }) + + await expect( + launch( + { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' }, + reuseTerminal: { handle: 'term_live' } + }, + runtime + ) + ).rejects.toThrow('agent_launch_terminal_worktree_mismatch') + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + }) + + it('rejects reusing a terminal while creating a new workspace', async () => { + const runtime = runtimeStub() + await expect( + launch({ ...CREATE_LAUNCH, reuseTerminal: { handle: 'term_live' } }, runtime) + ).rejects.toThrow('agent_launch_reuse_requires_existing_workspace') + expect(runtime.showTerminal).not.toHaveBeenCalled() + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + }) }) describe('the worktree factory', () => { @@ -188,11 +240,37 @@ describe('the worktree factory', () => { const args = createArgs(runtime) expect(args.startupAgent).toBeUndefined() + expect(args.awaitTerminalProvisioning).toBe(true) + expect(args.observeSetupCompletion).toBe(true) // Still recorded on the workspace: the launch owns the agent whichever surface it settles on. expect(args.createdWithAgent).toBe('claude') expect(result.outcome.kind).toBe('structured') }) + it('waits for a setup-gated structured workspace before creating its session', async () => { + const runtime = runtimeStub({ + setupReceipt: { + startupPolicy: 'wait-for-setup', + state: 'running', + terminalHandle: 'setup-1' + } + }) + const order: string[] = [] + runtime.waitForSetupTerminalCompletion.mockImplementation(async () => { + order.push('setup-complete') + return { exitCode: 0 } + }) + createStructuredSession.mockImplementationOnce(async () => { + order.push('structured-create') + return { ok: true as const, value: { sessionId: 'sess-1' } } + }) + + await launch(CREATE_LAUNCH, runtime) + + expect(order).toEqual(['setup-complete', 'structured-create']) + expect(runtime.waitForSetupTerminalCompletion).toHaveBeenCalledWith('setup-1') + }) + it('keeps agent-first creation for a launch the user wants as a terminal', async () => { const runtime = runtimeStub({ settings: {} }) const result = await launch(CREATE_LAUNCH, runtime) diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index fec0f5b162e..e1ece033ff6 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -67,6 +67,25 @@ async function agentLaunchIntent( } } +async function validateReusedTerminal( + intent: AgentLaunchIntent, + runtime: Pick +): Promise { + if (!intent.reuseTerminal) { + return + } + if (intent.target.kind !== 'existing') { + throw new Error('agent_launch_reuse_requires_existing_workspace') + } + const terminal = await runtime.showTerminal(intent.reuseTerminal.handle) + if (terminal.worktreeId !== intent.target.worktree) { + throw new Error('agent_launch_terminal_worktree_mismatch') + } + if (!(await runtime.isTerminalRunningAgent(intent.reuseTerminal.handle))) { + throw new Error('agent_launch_terminal_not_running_agent') + } +} + export const AGENT_LAUNCH_METHODS = [ defineMethod({ name: 'agent.launch', @@ -76,6 +95,7 @@ export const AGENT_LAUNCH_METHODS = [ throw new Error('agent_launch_unsupported') } const intent = await agentLaunchIntent(params, context.runtime) + await validateReusedTerminal(intent, context.runtime) return executeAgentLaunch({ runtime: context.runtime, intent,