diff --git a/mobile/src/tasks/agent-launch-worktree-create.test.ts b/mobile/src/tasks/agent-launch-worktree-create.test.ts index 93b75d7d1c7..428b4f88662 100644 --- a/mobile/src/tasks/agent-launch-worktree-create.test.ts +++ b/mobile/src/tasks/agent-launch-worktree-create.test.ts @@ -61,14 +61,15 @@ describe('readAgentLaunchCreateOutcome', () => { } ) - it('carries a terminal launch warning, so a workspace whose agent never started says why', () => { - // The warning passthrough landed on worktree.create while this route was being written, so it - // has to be carried here too: a launch can seat the workspace and still fail to start the pty, - // and dropping the reason is what leaves the phone on an unexplained empty session. + it('carries the launch warning, so a workspace that is incomplete says why', () => { + // A launch can seat the workspace and still fail to finish it — an unspawned pty, untracked + // files left behind. Dropping the reason is what leaves the phone on a workspace that is + // quietly wrong. The host reports it at the top level, the same place worktree.create does. expect( readAgentLaunchCreateOutcome({ worktreeId: 'wt-1', - outcome: { kind: 'terminal', handle: 'term-1', warning: 'No pty available' } + outcome: { kind: 'structured', sessionId: 's-1', handle: 'agent-session:s-1' }, + warning: 'No pty available' }) ).toEqual({ worktreeId: 'wt-1', warning: 'No pty available' }) }) @@ -76,13 +77,25 @@ describe('readAgentLaunchCreateOutcome', () => { it.each([ { label: 'blank', warning: ' ' }, { label: 'absent', warning: undefined }, - { label: 'non-string', warning: 7 }, - { label: 'structured-surface', warning: 'ignored', kind: 'structured' } - ])('reports no warning when it is $label', ({ warning, kind }) => { + { label: 'non-string', warning: 7 } + ])('reports no warning when it is $label', ({ warning }) => { expect( readAgentLaunchCreateOutcome({ worktreeId: 'wt-1', - outcome: { kind: kind ?? 'terminal', handle: 'term-1', warning } + outcome: { kind: 'terminal', handle: 'term-1' }, + warning + }) + ).toEqual({ worktreeId: 'wt-1' }) + }) + + it('ignores a warning left on the outcome, which is no longer where one lives', () => { + // Pins the contract migration: the warning moved to the top level precisely so a reader never + // has to branch on `outcome.kind` to find it. A host still sending the old shape must not + // sneak one through the surface it happened to build. + expect( + readAgentLaunchCreateOutcome({ + worktreeId: 'wt-1', + outcome: { kind: 'terminal', handle: 'term-1', warning: 'stale shape' } }) ).toEqual({ worktreeId: 'wt-1' }) }) diff --git a/mobile/src/tasks/agent-launch-worktree-create.ts b/mobile/src/tasks/agent-launch-worktree-create.ts index dc4faf53cc8..b8644ed3a28 100644 --- a/mobile/src/tasks/agent-launch-worktree-create.ts +++ b/mobile/src/tasks/agent-launch-worktree-create.ts @@ -14,7 +14,6 @@ import { withoutReservedAgentCreateFields, - type AgentLaunchOutcome, type AgentLaunchResult } from '../../../src/shared/agent-launch-intent' import type { TuiAgent } from '../../../src/shared/tui-agent' @@ -60,46 +59,11 @@ export function readAgentLaunchCreateOutcome(result: unknown): AgentLaunchCreate if (typeof worktreeId !== 'string' || !worktreeId.trim()) { return null } - const warning = parseTerminalLaunchOutcome( - 'outcome' in result ? result.outcome : undefined - )?.warning?.trim() - return { worktreeId, ...(warning ? { warning } : {}) } -} - -/** - * The terminal outcome, narrowed to the fields this reader consumes. Taken from the shared union - * rather than restated, so a change to the contract fails here instead of flowing through. - * - * `handle` is deliberately not required: nothing here reads it, and demanding it would drop the - * warning off a reply that omitted it — a behaviour change smuggled in under a typing change. - */ -type TerminalLaunchOutcome = Pick< - Extract, - 'kind' | 'warning' -> - -/** - * Parses the launch outcome, which arrives as whatever the host sent. - * - * A terminal launch reports its startup failure here: the workspace exists, the agent did not - * start (pty exhaustion). Dropping it is what lands the phone on an unexplained empty session. - * - * Parsed into a named type at this boundary rather than read off a loose `object`, and narrowed - * rather than asserted — a reader that claims the contract's shape without checking it is how a - * malformed reply reaches the UI as a TypeError instead of a message. - */ -function parseTerminalLaunchOutcome(outcome: unknown): TerminalLaunchOutcome | null { - if ( - !outcome || - typeof outcome !== 'object' || - !('kind' in outcome) || - outcome.kind !== 'terminal' - ) { - return null - } + // The launch reports an incomplete create at the top level, the same place `worktree.create` + // puts it, so nothing here branches on which surface the host built to find it. const warning = - 'warning' in outcome && typeof outcome.warning === 'string' ? outcome.warning : undefined - return { kind: 'terminal', ...(warning === undefined ? {} : { warning }) } + 'warning' in result && typeof result.warning === 'string' ? result.warning.trim() : '' + return { worktreeId, ...(warning ? { warning } : {}) } } /** diff --git a/src/main/agent-launch/agent-launch-executor.ts b/src/main/agent-launch/agent-launch-executor.ts index 3f02aaafb36..658e06234ec 100644 --- a/src/main/agent-launch/agent-launch-executor.ts +++ b/src/main/agent-launch/agent-launch-executor.ts @@ -81,7 +81,12 @@ export type AgentLaunchWorkspaceFactory = { * wait-for-setup gate for free. A structured launch has no startup command to sequence and * must await that gate explicitly instead. */ startupAgent: TuiAgent | undefined - }): Promise<{ worktreeId: string; startupTerminalHandle: string | undefined }> + }): Promise<{ + worktreeId: string + startupTerminalHandle: string | undefined + /** Created, but incomplete — surfaced on the launch result rather than dropped. */ + warning?: string + }> } export type AgentLaunchExecution = { @@ -126,6 +131,7 @@ export async function executeAgentLaunch( outcome: { kind: 'terminal', handle: placed.startupTerminalHandle }, worktreeId: placed.worktreeId, receipt: preflight, + ...(placed.warning ? { warning: placed.warning } : {}), ...promptReceipt(intent) } } @@ -140,9 +146,9 @@ export async function executeAgentLaunch( ) execution.onStage?.('surface_create') - let outcome: AgentLaunchResult['outcome'] + let created: { outcome: AgentLaunchResult['outcome']; warning?: string } try { - outcome = await createSurface(execution, placed.worktreeId, settled) + created = 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 @@ -155,22 +161,25 @@ export async function executeAgentLaunch( throw error } settled = downgradeAgentLaunchModeForStructuredRefusal(settled, vocabulary) - outcome = await execution.surfaces + created = await execution.surfaces .createTerminalAgent({ worktreeId: placed.worktreeId, agent: intent.agent, ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) }) .then((terminal) => ({ - kind: 'terminal' as const, - handle: terminal.handle, + outcome: { kind: 'terminal' as const, handle: terminal.handle }, ...(terminal.warning ? { warning: terminal.warning } : {}) })) } + // A create warning outranks a surface one: it is about the workspace itself. The two do not + // co-occur today — an agent-first create that warned already returned above. + const warning = placed.warning ?? created.warning return { - outcome, + outcome: created.outcome, worktreeId: placed.worktreeId, receipt: settled, + ...(warning ? { warning } : {}), ...promptReceipt(intent) } } @@ -190,9 +199,14 @@ function downgradeAgentLaunchModeForStructuredRefusal( async function resolveWorkspace( execution: AgentLaunchExecution, preflight: AgentLaunchModeReceipt -): Promise<{ worktreeId: string; startupTerminalHandle: string | undefined }> { +): Promise<{ + worktreeId: string + startupTerminalHandle: string | undefined + warning?: string +}> { const { intent } = execution if (intent.target.kind === 'existing') { + // Nothing was created, so there is no create warning to carry. return { worktreeId: intent.target.worktree, startupTerminalHandle: undefined } } const workspaces = execution.workspaces @@ -212,7 +226,7 @@ async function createSurface( execution: AgentLaunchExecution, worktreeId: string, settled: AgentLaunchModeReceipt -): Promise { +): Promise<{ outcome: AgentLaunchResult['outcome']; warning?: string }> { const { intent, surfaces } = execution if (settled.mode === 'structured' && isStructuredProvider(intent.agent)) { const session = await surfaces.createStructuredSession({ @@ -220,7 +234,7 @@ async function createSurface( agent: intent.agent, ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) }) - return { kind: 'structured', sessionId: session.sessionId, handle: session.handle } + return { outcome: { kind: 'structured', sessionId: session.sessionId, handle: session.handle } } } const terminal = await surfaces.createTerminalAgent({ worktreeId, @@ -228,8 +242,7 @@ async function createSurface( ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) }) return { - kind: 'terminal', - handle: terminal.handle, + outcome: { kind: 'terminal', handle: terminal.handle }, ...(terminal.warning ? { warning: terminal.warning } : {}) } } 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 edf8dade6c1..7d236377a2e 100644 --- a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts +++ b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts @@ -76,7 +76,10 @@ export function agentLaunchWorkspaceFactory( finishAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) return { worktreeId: result.worktree.id, - startupTerminalHandle: result.startupTerminal?.handle + startupTerminalHandle: result.startupTerminal?.handle, + // Carried, not dropped: `createManagedWorktree` reports a failed startup terminal or an + // uncopied working tree here, and it is the only place the host says so. + ...(result.warning ? { warning: result.warning } : {}) } } catch (error) { releaseAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) diff --git a/src/main/runtime/rpc/methods/agent-launch.test.ts b/src/main/runtime/rpc/methods/agent-launch.test.ts index aaf99d79629..3294f197309 100644 --- a/src/main/runtime/rpc/methods/agent-launch.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch.test.ts @@ -40,6 +40,8 @@ function runtimeStub( state: 'running' | 'skipped' | 'not_configured' | 'spawn_failed' terminalHandle?: string } + /** What `createManagedWorktree` reports when the workspace exists but is incomplete. */ + createWarning?: string } = {} ) { const worktreeCreateResults = new Map>() @@ -73,7 +75,8 @@ function runtimeStub( createManagedWorktree: vi.fn(async (args: Record) => ({ worktree: { id: 'wt-new' }, startupTerminal: args.startupAgent ? { handle: 'term_agent_first' } : undefined, - ...(options.setupReceipt ? { setupReceipt: options.setupReceipt } : {}) + ...(options.setupReceipt ? { setupReceipt: options.setupReceipt } : {}), + ...(options.createWarning ? { warning: options.createWarning } : {}) })), createTerminal: vi.fn(async () => ({ handle: 'term_1' })), showTerminal: vi.fn(async (handle: string) => ({ handle, worktreeId: 'wt-7' })), @@ -428,6 +431,43 @@ describe('the structured session factory', () => { }) }) +describe('a create that succeeded but is incomplete', () => { + // createManagedWorktree reports an unspawned startup terminal or an uncopied working tree as a + // top-level `warning`, and worktree.create hands it straight to mobile. This path narrowed the + // create down to {worktreeId, startupTerminalHandle} and dropped it — on BOTH arms, but the + // structured arm is the one that had no channel for a warning at all. + it('carries a create warning onto a structured launch', async () => { + const runtime = runtimeStub({ + createWarning: 'Could not copy untracked files into the new workspace.' + }) + + const result = await launch(CREATE_LAUNCH, runtime) + + expect(result.outcome.kind).toBe('structured') + expect(result.warning).toBe('Could not copy untracked files into the new workspace.') + }) + + it('carries a create warning onto an agent-first terminal launch', async () => { + // settings: {} leaves the structured preference off, so the launch is agent-first and returns + // on the cached startup handle - the early path that also had to learn to carry a warning. + const runtime = runtimeStub({ + settings: {}, + createWarning: 'Failed to create the startup terminal: no pty' + }) + + const result = await launch(CREATE_LAUNCH, runtime) + + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_agent_first' }) + expect(result.warning).toBe('Failed to create the startup terminal: no pty') + }) + + it('reports no warning when the create had none', async () => { + const runtime = runtimeStub() + const result = await launch(CREATE_LAUNCH, runtime) + expect(result.warning).toBeUndefined() + }) +}) + describe('the terminal factory', () => { it('starts the agent through the runtime launcher when the host refuses a session', async () => { const runtime = runtimeStub({ createSupport: { supported: false, reason: 'wsl' } }) diff --git a/src/shared/agent-launch-intent.ts b/src/shared/agent-launch-intent.ts index 00dabcd5ee4..ff02565112b 100644 --- a/src/shared/agent-launch-intent.ts +++ b/src/shared/agent-launch-intent.ts @@ -73,7 +73,7 @@ export type AgentLaunchIntent = { /** The surface the host actually created. */ export type AgentLaunchOutcome = | { kind: 'structured'; sessionId: string; handle: string } - | { kind: 'terminal'; handle: string; warning?: string } + | { kind: 'terminal'; handle: string } /** Whether the launch text was delivered, for a caller that needs to report or retry it. */ export type AgentLaunchPromptReceipt = { @@ -85,6 +85,18 @@ export type AgentLaunchResult = { outcome: AgentLaunchOutcome /** The workspace the agent runs in, resolved or created. */ worktreeId: string + /** + * The launch completed but something in it did not: a startup terminal that failed to spawn, + * untracked files that could not be copied. `worktree.create` returns this at the top level and + * mobile already surfaces it, so a launch that drops it lands the user on a workspace that is + * quietly incomplete. + * + * Top level rather than on the outcome, and deliberately the ONLY place a launch warning lives: + * it is produced by the create as often as by the surface, it applies to a structured session + * and a terminal alike, and a reader should not have to branch on `outcome.kind` to discover + * that the workspace it just opened is missing something. + */ + warning?: string /** Why the outcome is what it is — always populated, so a downgrade is never silent. */ receipt: AgentLaunchModeReceipt prompt?: AgentLaunchPromptReceipt