From e6989dbe739ffbf4ab487f3eeecd762f2772cafc Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:35:33 -0700 Subject: [PATCH] feat(agent-launch): report the pane a terminal launch created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `term_*` handle is a main-side mapping the renderer cannot resolve (terminal-handle-links.ts:309), so a client that draws its own tabs had no way to name the tab it had just asked `agent.launch` to build. The runtime already mints that pane, bakes it into the PTY's environment and hands it to its own reveal; the surface factory then dropped it on the floor. Carry it through as `paneKey` on the terminal outcome. Identity, not placement: where the pane goes — which group, what order, whether it takes focus — stays with whichever client is drawing, and nothing here rides the wire for it. One field rather than a tabId/leafId pair, because the key already holds both and two copies of one fact can disagree. Absent when this launch minted no pane: a reused terminal was already running, and a worktree-create startup terminal is built by the create, which reports only a handle. Naming the wrong surface is worse than naming none. Optional on the wire and optional on the read side. Mobile parses the receipt with a loose object and is deliberately mode-blind, so it ignores the field; the persisted-row guard checks it when present and accepts a row written before it existed, because a read rule stricter than the write side turns one odd row into a refused replay. --- .../agent-launch/agent-launch-executor.ts | 10 +- .../rpc/methods/agent-launch-surfaces.ts | 3 + .../agent-launch-terminal-identity.test.ts | 154 ++++++++++++++++++ .../rpc/methods/agent-launch.test-fixture.ts | 4 + src/shared/agent-launch-intent.ts | 33 +++- 5 files changed, 199 insertions(+), 5 deletions(-) create mode 100644 src/main/runtime/rpc/methods/agent-launch-terminal-identity.test.ts diff --git a/src/main/agent-launch/agent-launch-executor.ts b/src/main/agent-launch/agent-launch-executor.ts index 1dc58ee8d33..7c8d7687c82 100644 --- a/src/main/agent-launch/agent-launch-executor.ts +++ b/src/main/agent-launch/agent-launch-executor.ts @@ -77,7 +77,9 @@ export type AgentLaunchSurfaceFactory = { cwd?: string /** The one member of the `agent_started` triple the host cannot derive for itself. */ launchSource?: string - }): Promise<{ handle: string; warning?: string }> + /** `paneKey` names the pane this create minted, for a caller that presents its own tabs; a + * factory whose runtime does not report one omits it rather than inventing a key. */ + }): Promise<{ handle: string; paneKey?: string; warning?: string }> /** * Commits the launch text as the session's first turn, answering with the transcript row's id. * @@ -380,7 +382,11 @@ async function createTerminalSurface( ...(intent.launchSource ? { launchSource: intent.launchSource } : {}) }) return { - outcome: { kind: 'terminal', handle: terminal.handle }, + outcome: { + kind: 'terminal', + handle: terminal.handle, + ...(terminal.paneKey ? { paneKey: terminal.paneKey } : {}) + }, ...(terminal.warning ? { warning: terminal.warning } : {}), ...(startupPrompt ? { promptRodeLaunchCommand: true } : {}) } diff --git a/src/main/runtime/rpc/methods/agent-launch-surfaces.ts b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts index 3df28fd0717..eb1ba13f360 100644 --- a/src/main/runtime/rpc/methods/agent-launch-surfaces.ts +++ b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts @@ -111,6 +111,9 @@ export function agentLaunchSurfaceFactory( }) return { handle: terminal.handle, + // The runtime already minted this pane and baked it into the PTY's env and its own reveal; + // dropping it here was what left a client with no way to name the tab it just asked for. + ...(terminal.paneKey ? { paneKey: terminal.paneKey } : {}), ...(terminal.warning ? { warning: terminal.warning } : {}) } }, diff --git a/src/main/runtime/rpc/methods/agent-launch-terminal-identity.test.ts b/src/main/runtime/rpc/methods/agent-launch-terminal-identity.test.ts new file mode 100644 index 00000000000..bd1a5182442 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-terminal-identity.test.ts @@ -0,0 +1,154 @@ +/** + * Whether a terminal launch names the pane it created. + * + * A `term_*` handle is a main-side mapping — terminal-handle-links.ts:309 says the renderer cannot + * resolve one — so a client that draws its own tabs could not tell which tab `agent.launch` had + * just asked the host to build. The runtime already mints that pane and reports it as `paneKey`; + * the surface factory used to discard it, which is the one identity channel this method declined. + * + * Identity travels; placement does not. Nothing here asks for a group, an order or a focus — those + * stay with whichever client is drawing, and mobile, which has no tabs, reads none of this. + */ + +import { describe, expect, it, vi } from 'vitest' +import { parsePaneKey } from '../../../../shared/stable-pane-id' +import { isAgentLaunchResult } from '../../../../shared/agent-launch-intent' +import type { RpcContext } from '../core' +import { + CAPABLE_CLIENT, + methodNamed, + rpcContext, + runtimeStub, + type AgentLaunchRuntimeStub as RuntimeStub +} from './agent-launch.test-fixture' + +vi.mock('./structured-agent-session-create', () => ({ + createStructuredAgentSessionForWorktree: async () => ({ + ok: true, + value: { sessionId: 'sess-1' } + }) +})) + +const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') +const AGENT_LAUNCH = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launch') + +/** Shaped like a pane the runtime really mints: `randomUUID()` for the tab and for the leaf. */ +const TAB_ID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d' +const LEAF_ID = '3f2504e0-4f89-41d3-9a0c-0305e82c3301' +const PANE_KEY = `${TAB_ID}:${LEAF_ID}` + +/** Settings with no structured preference, so every launch here settles as a terminal. */ +const TERMINAL_ONLY = {} + +async function launch(params: unknown, runtime: RuntimeStub, context: Partial = {}) { + const parsed = AGENT_LAUNCH.params.safeParse(params) + if (!parsed.success) { + throw new Error(parsed.error.issues[0]?.message ?? 'invalid') + } + return AGENT_LAUNCH.handler(parsed.data, rpcContext(runtime, { ...CAPABLE_CLIENT, ...context })) +} + +const EXISTING_LAUNCH = { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' } +} + +describe('the pane a terminal launch created', () => { + it('reports the pane key the runtime minted', async () => { + const runtime = runtimeStub({ settings: TERMINAL_ONLY, terminalPaneKey: PANE_KEY }) + + const result = await launch(EXISTING_LAUNCH, runtime) + + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1', paneKey: PANE_KEY }) + }) + + it('reports a pane key a client can resolve to a tab and a leaf', async () => { + const runtime = runtimeStub({ settings: TERMINAL_ONLY, terminalPaneKey: PANE_KEY }) + + const result = await launch(EXISTING_LAUNCH, runtime) + + // The point of carrying identity at all: the client gets the two ids `store.createTab` needs. + const pane = + result.outcome.kind === 'terminal' ? parsePaneKey(result.outcome.paneKey ?? '') : null + expect(pane).toMatchObject({ tabId: TAB_ID, leafId: LEAF_ID }) + }) + + it('carries the pane key through a downgrade to a terminal', async () => { + // The downgrade builds its terminal through the same factory, so it must not lose identity + // the outright-terminal path keeps. + const runtime = runtimeStub({ + createSupport: { supported: false, reason: 'wsl' }, + terminalPaneKey: PANE_KEY + }) + + const result = await launch(EXISTING_LAUNCH, runtime) + + expect(result.receipt).toMatchObject({ mode: 'terminal', reason: 'wsl_execution_runtime' }) + expect(result.outcome).toMatchObject({ kind: 'terminal', paneKey: PANE_KEY }) + }) + + it('omits the pane key when the runtime reported none', async () => { + const runtime = runtimeStub({ settings: TERMINAL_ONLY }) + + const result = await launch(EXISTING_LAUNCH, runtime) + + // Absent, not empty: a client must be able to tell "no pane to adopt" from "a pane called ''". + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1' }) + }) + + it('omits the pane key for a reused terminal, which this launch did not create', async () => { + const runtime = runtimeStub({ settings: TERMINAL_ONLY, terminalPaneKey: PANE_KEY }) + + const result = await launch( + { ...EXISTING_LAUNCH, reuseTerminal: { handle: 'term_live' } }, + runtime + ) + + expect(runtime.createTerminal).not.toHaveBeenCalled() + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_live' }) + }) + + it('omits the pane key for a worktree-create startup terminal', async () => { + // Agent-first creation builds the agent inside the create, which reports only a handle. + // Reporting the pane of a terminal this launch never created would name the wrong surface. + const runtime = runtimeStub({ settings: TERMINAL_ONLY, terminalPaneKey: PANE_KEY }) + + const result = await launch( + { + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'task' } } + }, + runtime + ) + + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_agent_first' }) + }) +}) + +describe('reading a recorded launch back', () => { + // The replay resolver narrows a stored row with this guard, so a field it does not check is a + // field a replay hands back unvalidated — and one it over-checks is a refused replay. + const BASE = { + worktreeId: 'wt-7', + receipt: { mode: 'terminal', preferred: 'terminal', reason: 'user_default', detail: 'ok' } + } + + it('accepts a row carrying a pane key', () => { + expect( + isAgentLaunchResult({ + ...BASE, + outcome: { kind: 'terminal', handle: 't', paneKey: PANE_KEY } + }) + ).toBe(true) + }) + + it('accepts a row written before the field existed', () => { + expect(isAgentLaunchResult({ ...BASE, outcome: { kind: 'terminal', handle: 't' } })).toBe(true) + }) + + it('refuses a row whose pane key is not a string', () => { + expect( + isAgentLaunchResult({ ...BASE, outcome: { kind: 'terminal', handle: 't', paneKey: 7 } }) + ).toBe(false) + }) +}) diff --git a/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts b/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts index bcb14ec2784..98b497ef6dd 100644 --- a/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts +++ b/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts @@ -27,6 +27,9 @@ export type AgentLaunchRuntimeStubOptions = { createWarning?: string /** What `createTerminal` reports when the surface itself came up degraded. */ terminalWarning?: string + /** The pane `createTerminal` minted. Off by default so the existing outcome assertions keep + * modelling a runtime that reports none — the arm `RuntimeTerminalCreate.paneKey?` allows. */ + terminalPaneKey?: string } export function runtimeStub(options: AgentLaunchRuntimeStubOptions = {}) { @@ -67,6 +70,7 @@ export function runtimeStub(options: AgentLaunchRuntimeStubOptions = {}) { // Args are declared so a test can assert what the launch asked for, not merely that it asked. createTerminal: vi.fn(async (_selector: string, _options?: Record) => ({ handle: 'term_1', + ...(options.terminalPaneKey ? { paneKey: options.terminalPaneKey } : {}), ...(options.terminalWarning ? { warning: options.terminalWarning } : {}) })), showTerminal: vi.fn(async (handle: string) => ({ handle, worktreeId: 'wt-7' })), diff --git a/src/shared/agent-launch-intent.ts b/src/shared/agent-launch-intent.ts index 056f32e436f..c27b5cb1ad6 100644 --- a/src/shared/agent-launch-intent.ts +++ b/src/shared/agent-launch-intent.ts @@ -88,7 +88,26 @@ export type AgentLaunchIntent = { /** The surface the host actually created. */ export type AgentLaunchOutcome = | { kind: 'structured'; sessionId: string; handle: string } - | { kind: 'terminal'; handle: string } + | { + kind: 'terminal' + handle: string + /** + * The pane the host minted for this agent, as `tabId:leafId` — read it with `parsePaneKey`. + * + * Identity, not placement. The host already mints this pair, bakes it into the PTY's + * environment and hands it to its own reveal; a client that draws its own tabs previously had + * no way to learn it, because a `term_*` handle is a main-side mapping the renderer cannot + * resolve. Where that pane goes — which group, what order, whether it takes focus — stays + * with the client and never rides this wire. + * + * One field rather than a `tabId`/`leafId` pair, because the key already carries both and two + * copies of one fact can disagree. + * + * Absent when this launch minted no pane: a reused terminal was already running, and a + * worktree-create startup terminal is built by the create, which reports only a handle. + */ + paneKey?: string + } /** * What became of the launch text. * @@ -210,12 +229,20 @@ function isAgentLaunchOutcome(value: unknown): value is AgentLaunchOutcome { return false } // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the assertion claims only that the keys may be present and unknown, which is true of any object. - const outcome = value as { kind?: unknown; handle?: unknown; sessionId?: unknown } + const outcome = value as { + kind?: unknown + handle?: unknown + sessionId?: unknown + paneKey?: unknown + } if (typeof outcome.handle !== 'string' || outcome.handle.length === 0) { return false } return outcome.kind === 'terminal' - ? true + ? // Checked when present, ignored when absent: a row written before this field existed, or by a + // runtime that minted no pane, still reads. Deliberately not parsed — a read-side shape rule + // stricter than the write side turns one odd row into a refused replay. + outcome.paneKey === undefined || typeof outcome.paneKey === 'string' : outcome.kind === 'structured' && typeof outcome.sessionId === 'string' && outcome.sessionId.length > 0