From abc8386e149e555ece2be601e631eeed32fe62bb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 17 Sep 2026 10:06:11 -0700 Subject: [PATCH] fix(mobile): name a create's launch so a lost reply cannot build two workspaces (#21137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): name a create's launch so a lost reply cannot build two workspaces `agent.launch` admits a caller-supplied `operationId` through a durable ledger, so exactly one execution happens and every replay returns the recorded answer. No client sent one, so the machinery was inert and the original defect was still live: mobile retries a lost create by design, and a retried launch built a second agent in a second workspace. Mobile now mints an operation id per create candidate and sends it whenever the host advertises `agent.launch.replay.v1`. The invariant is one operation per candidate. `computeAgentLaunchFingerprint` folds `target` whole, so the workspace name is inside the fingerprint; carrying one id across a name-collision bump would meet its own row under a differing fingerprint and refuse `agent_session_operation_conflict`, failing the create outright on the second candidate. The id is therefore minted beside `clientMutationId` at the top of each loop iteration and reused verbatim by every retry arm inside that candidate — never re-minted, since a new id is a new operation. Admission runs ahead of every effect, so `_invalid` / `_expired` / `_capacity` prove nothing launched: those re-send the same candidate unnamed rather than let bookkeeping fail a create the host would have performed. `_unknown` is the one refusal that is not safe to re-send, and it surfaces. Also corrects a false comment: the legacy path caches the whole launch under `clientMutationId`, so inside its 60s window a replay adds neither a workspace nor a surface, and outside it adds both — not "a second surface, never a second workspace". * fix(mobile): preserve launch identity on refusals * fix(mobile): use launch receipts to authorize replay * test: move mobile launch replay coverage outside node project * fix(mobile): enforce replay-safe launch delivery at the host * test: run mobile launch contracts in mobile checks * test: cover mobile launch contract workflow dependencies --- .github/workflows/mobile.yml | 17 +- .../mobile-launch-contract-workflow.test.mjs | 34 ++ .../use-new-workspace-create-submit.ts | 3 +- .../mobile-structured-agent-session-launch.ts | 2 +- .../mobile-structured-agent-session-rpc.ts | 18 +- .../mobile-structured-agent-session-send.ts | 2 +- .../structured-session-operation-id.ts | 20 + .../tasks/agent-launch-mobile-replay.test.ts | 164 ++++++++ .../src/tasks/agent-launch-worktree-create.ts | 13 +- .../src/tasks/blank-workspace-create.test.ts | 10 +- mobile/src/tasks/blank-workspace-create.ts | 2 +- .../mobile-agent-launch-architecture.test.ts | 364 ++++++++++++++++++ .../mobile-workspace-create-operations.ts | 19 +- .../src/tasks/source-workspace-create.test.ts | 10 +- mobile/src/tasks/source-workspace-create.ts | 8 +- .../tasks/worktree-create-capability.test.ts | 52 ++- .../src/tasks/worktree-create-capability.ts | 19 +- .../worktree-create-launch-operation.test.ts | 333 ++++++++++++++++ mobile/src/tasks/worktree-create-retry.ts | 118 ++++-- src/main/ipc/worktree-remote.ts | 7 +- src/main/runtime/rpc/errors.ts | 4 + .../rpc/methods/agent-launch-schemas.ts | 1 + src/main/runtime/rpc/methods/agent-launch.ts | 57 ++- ...runtime-local-worktree-create-candidate.ts | 3 +- .../runtime-rpc-mobile-method-allowlist.ts | 1 + .../worktree-create-collision.ts | 4 + src/shared/protocol-version.ts | 19 +- .../rpc-contract/agent-launch-params.ts | 3 + .../rpc-params-catalog.generated.ts | 3 +- 29 files changed, 1194 insertions(+), 116 deletions(-) create mode 100644 config/scripts/mobile-launch-contract-workflow.test.mjs create mode 100644 mobile/src/session/structured-session-operation-id.ts create mode 100644 mobile/src/tasks/agent-launch-mobile-replay.test.ts create mode 100644 mobile/src/tasks/mobile-agent-launch-architecture.test.ts create mode 100644 mobile/src/tasks/worktree-create-launch-operation.test.ts create mode 100644 src/shared/new-workspace/worktree-create-collision.ts diff --git a/.github/workflows/mobile.yml b/.github/workflows/mobile.yml index c16a1e9892b..1e94add89ad 100644 --- a/.github/workflows/mobile.yml +++ b/.github/workflows/mobile.yml @@ -9,6 +9,18 @@ on: - ready_for_review paths: - 'mobile/**' + # Mobile launch contracts exercise the real host dispatcher and durable receipt store. + - 'src/main/agent-launch/**' + - 'src/main/runtime/rpc/**' + - 'src/main/runtime/runtime-rpc/**' + - 'src/main/runtime/runtime-rpc.ts' + - 'src/main/runtime/device-registry.ts' + - 'src/main/runtime/orca-runtime.ts' + - 'src/main/runtime/agent-session-*.ts' + - 'src/main/native-chat/agent-session-wire/**' + - 'src/shared/agent-launch-*.ts' + - 'src/shared/agent-session-*.ts' + - 'src/shared/new-workspace/worktree-create-collision.ts' # Why: the mobile terminal link parsers are conformance-tested against # these shared fixtures; desktop-side fixture edits must re-run this suite. - 'src/shared/terminal-file-link-conformance.ts' @@ -21,11 +33,6 @@ on: # schema edit anywhere under here changes mobile's types, so a desktop-only # change can break mobile's typecheck with no other mobile signal. - 'src/shared/rpc-contract/**' - # Why: the catalog above holds params only. This file is the sole holder of - # the agent.launch RESULT shape, and mobile imports it as a value, not just - # a type. CROSS_VERSION_WIRE_PREFIXES already treats it as wire-critical, so - # without this one gate classes it that way while this one cannot see it. - - 'src/shared/agent-launch-intent.ts' # Why: this job holds the only checks that load the Fastfile, so edits to # it or to the release workflow it guards must re-run them. - '.github/workflows/mobile.yml' diff --git a/config/scripts/mobile-launch-contract-workflow.test.mjs b/config/scripts/mobile-launch-contract-workflow.test.mjs new file mode 100644 index 00000000000..fa24180e1fc --- /dev/null +++ b/config/scripts/mobile-launch-contract-workflow.test.mjs @@ -0,0 +1,34 @@ +import { existsSync, readFileSync } from 'node:fs' +import { dirname, matchesGlob, relative, resolve, sep } from 'node:path' +import { expect, it } from 'vitest' +import ts from 'typescript-api' +import { parse } from 'yaml' + +const projectDir = resolve(import.meta.dirname, '../..') +const workflow = parse(readFileSync(resolve(projectDir, '.github/workflows/mobile.yml'), 'utf8')) + +it.each(['agent-launch-mobile-replay', 'mobile-agent-launch-architecture'])( + 'runs Mobile Checks when a direct root dependency of %s changes', + (name) => { + const suite = resolve(projectDir, `mobile/src/tasks/${name}.test.ts`) + // Parse source only: the root test project must never load the mobile dependency graph. + const imports = ts + .preProcessFile(readFileSync(suite, 'utf8'), true) + .importedFiles.filter((entry) => entry.fileName.startsWith('.')) + .map((entry) => + relative(projectDir, resolve(dirname(suite), `${entry.fileName}.ts`)) + .split(sep) + .join('/') + ) + .filter((file) => file.startsWith('src/')) + + expect(imports).not.toEqual([]) + for (const file of imports) { + expect(existsSync(resolve(projectDir, file)), file).toBe(true) + expect( + workflow.on.pull_request.paths.some((pattern) => matchesGlob(file, pattern)), + file + ).toBe(true) + } + } +) diff --git a/mobile/src/components/use-new-workspace-create-submit.ts b/mobile/src/components/use-new-workspace-create-submit.ts index fe697ef7b9c..df47a69b65a 100644 --- a/mobile/src/components/use-new-workspace-create-submit.ts +++ b/mobile/src/components/use-new-workspace-create-submit.ts @@ -14,6 +14,7 @@ import { import { createWorkspaceFromComposerSource } from '../tasks/source-workspace-create' import { normalizeWorkspaceAgent } from '../tasks/workspace-agent-selection' import type { WorkspaceCreateSetupDecision } from '../tasks/workspace-create-params' +import type { AgentLaunchSupport } from '../tasks/agent-launch-worktree-create' import type { WorkspaceSshGate } from '../tasks/workspace-ssh-gate' import type { useMobileComposerSource } from '../tasks/use-mobile-composer-source' import type { WorktreeCreateIdempotencySupport } from '../tasks/worktree-create-idempotency-policy' @@ -56,7 +57,7 @@ export function useNewWorkspaceCreateSubmit(args: { trustedOrcaHooks: PersistedTrustedOrcaHooks setTrustedOrcaHooks: (trust: PersistedTrustedOrcaHooks) => void getWorktreeCreateCutoverSupport: () => Promise - getAgentLaunchSupport: () => Promise + getAgentLaunchSupport: () => Promise transitionDrawer: (view: Exclude) => void setError: Dispatch> onCreated: (worktreeId: string, name: string, warning?: string) => void diff --git a/mobile/src/session/mobile-structured-agent-session-launch.ts b/mobile/src/session/mobile-structured-agent-session-launch.ts index 8a35f7499ec..fb9af825617 100644 --- a/mobile/src/session/mobile-structured-agent-session-launch.ts +++ b/mobile/src/session/mobile-structured-agent-session-launch.ts @@ -16,7 +16,7 @@ import { structuredAgentSessionCreate, structuredAgentSupportProbe } from './mobile-session-launch-operations' -import { structuredSessionRandomUuid } from './mobile-structured-agent-session-rpc' +import { structuredSessionRandomUuid } from './structured-session-operation-id' type StructuredCreateSupport = { supported?: boolean diff --git a/mobile/src/session/mobile-structured-agent-session-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index 4e3861a058e..27c50d85104 100644 --- a/mobile/src/session/mobile-structured-agent-session-rpc.ts +++ b/mobile/src/session/mobile-structured-agent-session-rpc.ts @@ -7,10 +7,8 @@ import type { AgentSessionMutationResult, AgentSessionWireRefusalCode } from '../../../src/shared/agent-session-wire' -import { - createStructuredAgentSessionOperationId, - structuredAgentSessionPayloadFingerprint -} from '../../../src/shared/structured-agent-session-mutation' +import { structuredAgentSessionPayloadFingerprint } from '../../../src/shared/structured-agent-session-mutation' +import { structuredSessionOperationId } from './structured-session-operation-id' import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' import type { RpcClient } from '../transport/rpc-client' import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client' @@ -73,18 +71,6 @@ export async function callAgentSession( return response.result as TResult } -/** React Native has no guaranteed `crypto.randomUUID`; the fallback keeps the same - * 32-hex entropy shape the durable id and fingerprint helpers validate. */ -export function structuredSessionRandomUuid(): string { - return typeof globalThis.crypto?.randomUUID === 'function' - ? globalThis.crypto.randomUUID() - : Array.from({ length: 32 }, () => Math.floor(Math.random() * 16).toString(16)).join('') -} - -export function structuredSessionOperationId(now: number = Date.now()): string { - return createStructuredAgentSessionOperationId(structuredSessionRandomUuid, now) -} - function isReplayableStructuredSessionOperationId(operationId: string, now: number): boolean { const timestamp = parseAgentSessionOperationTimestamp(operationId) return ( diff --git a/mobile/src/session/mobile-structured-agent-session-send.ts b/mobile/src/session/mobile-structured-agent-session-send.ts index f9f5af44cdb..82d237d4d4a 100644 --- a/mobile/src/session/mobile-structured-agent-session-send.ts +++ b/mobile/src/session/mobile-structured-agent-session-send.ts @@ -11,9 +11,9 @@ import type { RpcClient } from '../transport/rpc-client' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { requestStructuredAgentSessionMutation, - structuredSessionOperationId, timeoutForDeadline } from './mobile-structured-agent-session-rpc' +import { structuredSessionOperationId } from './structured-session-operation-id' import { mobileStructuredSendDelivery } from './mobile-structured-send-delivery' import { clearMobileStructuredSendOperation, diff --git a/mobile/src/session/structured-session-operation-id.ts b/mobile/src/session/structured-session-operation-id.ts new file mode 100644 index 00000000000..ab639254942 --- /dev/null +++ b/mobile/src/session/structured-session-operation-id.ts @@ -0,0 +1,20 @@ +/** + * Minting the durable operation ids mobile names its mutations with. + * + * Its own module because the create path mints one too, and reaching the session RPC module for it + * would pull the native-chat write graph into `tasks/` for two functions that depend on nothing. + */ + +import { createStructuredAgentSessionOperationId } from '../../../src/shared/structured-agent-session-mutation' + +/** React Native has no guaranteed `crypto.randomUUID`; the fallback keeps the same + * 32-hex entropy shape the durable id and fingerprint helpers validate. */ +export function structuredSessionRandomUuid(): string { + return typeof globalThis.crypto?.randomUUID === 'function' + ? globalThis.crypto.randomUUID() + : Array.from({ length: 32 }, () => Math.floor(Math.random() * 16).toString(16)).join('') +} + +export function structuredSessionOperationId(now: number = Date.now()): string { + return createStructuredAgentSessionOperationId(structuredSessionRandomUuid, now) +} diff --git a/mobile/src/tasks/agent-launch-mobile-replay.test.ts b/mobile/src/tasks/agent-launch-mobile-replay.test.ts new file mode 100644 index 00000000000..12a3bf495a0 --- /dev/null +++ b/mobile/src/tasks/agent-launch-mobile-replay.test.ts @@ -0,0 +1,164 @@ +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 { createWorktreeWithNameRetry } from './worktree-create-retry' +import type { RpcClient } from '../transport/rpc-client' +import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import { + AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS, + AGENT_SESSION_OPERATION_FUTURE_SKEW_MS +} from '../../../src/shared/agent-session-host-authority' +import { setStructuredAgentSessionHost } from '../../../src/main/native-chat/agent-session-wire/structured-agent-session-registry' +import type { StructuredAgentSessionHost } from '../../../src/main/native-chat/agent-session-wire/structured-agent-session-host' +import { AgentSessionRecordStore } from '../../../src/main/runtime/agent-session-record-store' +import type { OrcaRuntimeService } from '../../../src/main/runtime/orca-runtime' +import { RpcDispatcher } from '../../../src/main/runtime/rpc/dispatcher' +import { runtimeStub } from '../../../src/main/runtime/rpc/methods/agent-launch.test-fixture' + +const createStructuredSession = vi.fn() +vi.mock('../../../src/main/runtime/rpc/methods/structured-agent-session-create', () => ({ + createStructuredAgentSessionForWorktree: (...args: unknown[]) => createStructuredSession(...args) +})) +const { AGENT_LAUNCH_METHODS } = await import('../../../src/main/runtime/rpc/methods/agent-launch') + +let directory: string +let store: AgentSessionRecordStore + +beforeEach(async () => { + createStructuredSession.mockReset() + createStructuredSession.mockResolvedValue({ ok: true, value: { sessionId: 'session-1' } }) + directory = await mkdtemp(join(tmpdir(), 'orca-mobile-launch-replay-')) + store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch reads only deps.store; structured session creation is the injected boundary above. + setStructuredAgentSessionHost({ deps: { store } } as unknown as StructuredAgentSessionHost) +}) + +afterEach(async () => { + vi.restoreAllMocks() + setStructuredAgentSessionHost(null) + await rm(directory, { recursive: true, force: true }) +}) + +function mobileLaunch( + args: { + loseFirstReplyAfterMs?: number + replay?: boolean + restartAfterReply?: boolean + replyLoss?: 'cutover' | 'timeout' + } = {} +) { + const runtime = { ...runtimeStub(), getRuntimeId: () => 'runtime-1' } + const runtimeAfterRestart = { ...runtimeStub(), getRuntimeId: () => 'runtime-2' } + const dispatchers = [runtime, runtimeAfterRestart].map( + (host) => + new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this fixture implements the launch handler and dispatcher metadata dependencies. + runtime: host as unknown as OrcaRuntimeService, + methods: AGENT_LAUNCH_METHODS + }) + ) + const operationId = `${Date.now()}-000000000000000000000000000000aa` + const attempts: unknown[] = [] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mobile retry loop reaches only these transport members; requests use the real host dispatcher. + const client = { + getState: () => 'connected', + sendRequest: async (method: string, params: unknown) => { + attempts.push(params) + const dispatcher = dispatchers[args.restartAfterReply && attempts.length > 1 ? 1 : 0]! + const response = await dispatcher.dispatch({ + id: `request-${attempts.length}`, + authToken: 'token', + method, + params + }) + if (attempts.length === 1 && args.loseFirstReplyAfterMs !== undefined) { + const later = Date.now() + args.loseFirstReplyAfterMs + vi.spyOn(Date, 'now').mockReturnValue(later) + if (args.replyLoss === 'timeout') { + throw markRpcDeliveryUnknown(new Error('Request timed out')) + } + throw new LogicalClientCutoverError() + } + return response + } + } as unknown as RpcClient + const result = createWorktreeWithNameRetry({ + client, + baseName: 'otter', + buildParams: (name) => ({ repo: 'id:repo-1', name }), + worktreeCreateIdempotency: { dedupeTtlMs: 60_000 }, + agentLaunch: { agent: 'claude', supported: { replay: args.replay !== false } }, + mintLaunchOperationId: () => operationId + }) + return { runtime, runtimeAfterRestart, attempts, operationId, result } +} + +describe('mobile launch retries through the host ledger', () => { + it('does not replay an unnamed launch after an older host loses its in-memory receipt', async () => { + const launch = mobileLaunch({ + replay: false, + restartAfterReply: true, + loseFirstReplyAfterMs: 1 + }) + + const outcome = await launch.result.catch((error: unknown) => error) + expect( + launch.runtime.createManagedWorktree.mock.calls.length + + launch.runtimeAfterRestart.createManagedWorktree.mock.calls.length + ).toBe(1) + expect(outcome).toBeInstanceOf(LogicalClientCutoverError) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(launch.runtimeAfterRestart.createManagedWorktree).not.toHaveBeenCalled() + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(launch.attempts).toHaveLength(1) + }) + + it.each([ + 'agent_session_operation_capacity', + 'agent_session_operation_invalid', + 'agent_session_operation_expired' + ])('does not create another workspace after a nested %s refusal', async (code) => { + createStructuredSession.mockResolvedValue({ ok: false, refusal: { code, message: code } }) + const launch = mobileLaunch() + + await expect(launch.result).resolves.toEqual({ error: 'agent_session_operation_unknown' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(launch.attempts).toHaveLength(1) + expect(store.listOperationRows()[0]?.outcome.status).toBe('unknown') + }) + + it('keeps the operation identity after its receipt expires during a lost reply', async () => { + const launch = mobileLaunch({ + loseFirstReplyAfterMs: + AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS + AGENT_SESSION_OPERATION_FUTURE_SKEW_MS + 1 + }) + + await expect(launch.result).resolves.toEqual({ error: 'agent_session_operation_expired' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(launch.attempts).toHaveLength(2) + }) + + it('replays a lost reply beyond the legacy cache window without creating again', async () => { + const launch = mobileLaunch({ loseFirstReplyAfterMs: 61_000 }) + + await expect(launch.result).resolves.toEqual({ worktreeId: 'wt-new', name: 'otter' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(launch.attempts).toHaveLength(2) + expect(launch.attempts[1]).toEqual(launch.attempts[0]) + expect(launch.runtime.dedupeWorktreeCreate).not.toHaveBeenCalled() + }) + + it('recovers a named launch whose reply timed out on a connected transport', async () => { + const launch = mobileLaunch({ loseFirstReplyAfterMs: 10 * 60_000, replyLoss: 'timeout' }) + + await expect(launch.result).resolves.toEqual({ worktreeId: 'wt-new', name: 'otter' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(launch.attempts).toHaveLength(2) + expect(launch.attempts[1]).toEqual(launch.attempts[0]) + }) +}) diff --git a/mobile/src/tasks/agent-launch-worktree-create.ts b/mobile/src/tasks/agent-launch-worktree-create.ts index 26bd55b671a..3348d471ce1 100644 --- a/mobile/src/tasks/agent-launch-worktree-create.ts +++ b/mobile/src/tasks/agent-launch-worktree-create.ts @@ -20,10 +20,17 @@ import type { TuiAgent } from '../../../src/shared/tui-agent' import type { RpcSendParams } from '../transport/rpc-params-contract' import type { WorkspaceCreateParams } from './workspace-create-params' +/** What this host's `agent.launch` can do, in the `| false` shape `worktree.create`'s own + * idempotency probe already uses: `false` is an older host with no `agent.launch` at all. */ +export type AgentLaunchSupport = { + /** The host deduplicates operationId durably and refuses unknown or expired outcomes. */ + replay: boolean +} + export type WorktreeCreateAgentLaunch = { agent: TuiAgent /** Resolved before the first create: an older host has no `agent.launch` at all. */ - supported: boolean | Promise + supported: AgentLaunchSupport | false | Promise } /** `worktreeId` is tied to the shared contract so a change to it fails this reader's typecheck @@ -35,10 +42,12 @@ export type AgentLaunchCreateOutcome = { export function agentLaunchCreateParams( agent: TuiAgent, - create: WorkspaceCreateParams + create: WorkspaceCreateParams, + operationId?: string | null ): RpcSendParams<'agent.launch'> { return { agent, + ...(operationId ? { operationId } : {}), target: { kind: 'create-worktree', create: withoutReservedAgentCreateFields(create) } } } diff --git a/mobile/src/tasks/blank-workspace-create.test.ts b/mobile/src/tasks/blank-workspace-create.test.ts index d2bd5e749fa..61930859b7f 100644 --- a/mobile/src/tasks/blank-workspace-create.test.ts +++ b/mobile/src/tasks/blank-workspace-create.test.ts @@ -146,7 +146,7 @@ describe('createBlankWorkspace', () => { setupDecision: 'run', nameWasGenerated: false, worktreeCreateIdempotency: IDEMPOTENT_CREATE_SUPPORT, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(result).toEqual({ worktreeId: 'wt-9', name: 'manatee' }) @@ -205,7 +205,7 @@ describe('createBlankWorkspace', () => { setupDecision: 'inherit', nameWasGenerated: false, worktreeCreateIdempotency: IDEMPOTENT_CREATE_SUPPORT, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(calls[0]?.method).toBe('worktree.create') @@ -230,7 +230,7 @@ describe('createBlankWorkspace', () => { setupDecision: 'inherit', nameWasGenerated: false, worktreeCreateIdempotency: IDEMPOTENT_CREATE_SUPPORT, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(result).toEqual({ worktreeId: 'wt-12', name: 'octopus-2' }) @@ -260,7 +260,7 @@ describe('createBlankWorkspace', () => { setupDecision: 'inherit', nameWasGenerated: false, worktreeCreateIdempotency: IDEMPOTENT_CREATE_SUPPORT, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(result).toEqual({ worktreeId: 'wt-13', name: 'octopus' }) @@ -347,7 +347,7 @@ describe('createBlankWorkspace', () => { { label: 'worktree.create', supported: false, agent: undefined, reply: { worktree: {} } }, { label: 'agent.launch', - supported: true, + supported: { replay: false }, agent: 'codex' as const, reply: { outcome: { kind: 'structured', sessionId: 's-1' } } } diff --git a/mobile/src/tasks/blank-workspace-create.ts b/mobile/src/tasks/blank-workspace-create.ts index 7e96d96b71c..af59df31822 100644 --- a/mobile/src/tasks/blank-workspace-create.ts +++ b/mobile/src/tasks/blank-workspace-create.ts @@ -24,7 +24,7 @@ export async function createBlankWorkspace(args: { nameWasGenerated: boolean worktreeCreateIdempotency: WorktreeCreateIdempotencyProbe /** Whether the host can settle the surface itself; false keeps the agent-first create. */ - agentLaunchSupported: boolean | Promise + agentLaunchSupported: WorktreeCreateAgentLaunch['supported'] }): Promise { const agentLaunch: WorktreeCreateAgentLaunch | undefined = args.createdWithAgentId ? { agent: args.createdWithAgentId, supported: args.agentLaunchSupported } diff --git a/mobile/src/tasks/mobile-agent-launch-architecture.test.ts b/mobile/src/tasks/mobile-agent-launch-architecture.test.ts new file mode 100644 index 00000000000..6dde7a7a793 --- /dev/null +++ b/mobile/src/tasks/mobile-agent-launch-architecture.test.ts @@ -0,0 +1,364 @@ +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 { AgentSessionRecordStore } from '../../../src/main/runtime/agent-session-record-store' +import { OrcaRuntimeRpcServer } from '../../../src/main/runtime/runtime-rpc' +import { DeviceRegistry } from '../../../src/main/runtime/device-registry' +import type { AuthenticatedMobileSocket } from '../../../src/main/runtime/rpc/mobile-socket-wiring' +import { RpcDispatcher } from '../../../src/main/runtime/rpc/dispatcher' +import { AgentLaunch } from '../../../src/main/runtime/rpc/methods/agent-launch-schemas' +import { runtimeStub } from '../../../src/main/runtime/rpc/methods/agent-launch.test-fixture' +import { setStructuredAgentSessionHost } from '../../../src/main/native-chat/agent-session-wire/structured-agent-session-registry' +import type { StructuredAgentSessionHost } from '../../../src/main/native-chat/agent-session-wire/structured-agent-session-host' +import type { OrcaRuntimeService } from '../../../src/main/runtime/orca-runtime' +import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import { createStableLogicalRpcClient } from '../transport/stable-logical-rpc-client' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' +import { WorktreeCreateCollisionError } from '../../../src/shared/new-workspace/worktree-create-collision' +import { createWorktreeWithNameRetry } from './worktree-create-retry' +import { readNewWorktreeRuntimeCapabilities } from './worktree-create-capability' +import { + AGENT_LAUNCH_RUNTIME_CAPABILITY, + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' + +const createStructuredSession = vi.fn() +vi.mock('../../../src/main/runtime/rpc/methods/structured-agent-session-create', () => ({ + createStructuredAgentSessionForWorktree: (...args: unknown[]) => createStructuredSession(...args) +})) +const { AGENT_LAUNCH_METHODS } = await import('../../../src/main/runtime/rpc/methods/agent-launch') + +let directory: string +let store: AgentSessionRecordStore + +beforeEach(async () => { + createStructuredSession.mockReset() + createStructuredSession.mockResolvedValue({ ok: true, value: { sessionId: 'session-1' } }) + directory = await mkdtemp(join(tmpdir(), 'orca-launch-architecture-')) + store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch reads deps.store; session creation is injected above. + setStructuredAgentSessionHost({ deps: { store } } as unknown as StructuredAgentSessionHost) +}) + +afterEach(async () => { + setStructuredAgentSessionHost(null) + await rm(directory, { recursive: true, force: true }) +}) + +function scenario( + options: { + replacement?: 'legacy' | 'current' + loss?: 'cutover' | 'timeout' + collision?: boolean + } = {} +) { + const runtime = { ...runtimeStub(), getRuntimeId: () => 'runtime-1' } + const replacement = { ...runtimeStub(), getRuntimeId: () => 'runtime-2' } + if (options.collision) { + runtime.createManagedWorktree.mockRejectedValueOnce( + new WorktreeCreateCollisionError('Branch "otter" already exists locally.') + ) + } + const dispatchers = [runtime, replacement].map( + (host, index) => + new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this fixture supplies the methods reached by the real launch handler. + runtime: host as unknown as OrcaRuntimeService, + methods: + index === 0 || options.replacement === 'current' + ? AGENT_LAUNCH_METHODS + : AGENT_LAUNCH_METHODS.filter((method) => method.name === 'agent.launch').map( + (method) => ({ + ...method, + // Older hosts accept the method but strip this optional field before running it. + params: AgentLaunch.omit({ operationId: true }) + }) + ) + }) + ) + let sent = 0 + let minted = 0 + let rejectFirst: ((error: Error) => void) | undefined + let directReplacement = false + const physical = (index: number): RpcClient => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the logical client reaches these transport methods; each mutation uses the production dispatcher. + return { + getState: () => 'connected', + onStateChange: () => () => {}, + close: () => rejectFirst?.(markRpcDeliveryUnknown(new Error('Connection lost'))), + sendRequest: async (method: string, params: unknown) => { + if (method === 'status.get') { + return { + id: 'status', + ok: true, + result: { + capabilities: + index === 0 + ? [ + AGENT_LAUNCH_RUNTIME_CAPABILITY, + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY + ] + : [AGENT_LAUNCH_RUNTIME_CAPABILITY] + }, + _meta: { runtimeId: `runtime-${index + 1}` } + } + } + sent += 1 + const response = await new Promise((resolve, reject) => { + void dispatchers[directReplacement ? 1 : index]!.dispatchStreaming( + { + id: `request-${sent}`, + authToken: 'token', + method, + params + }, + (reply) => resolve(JSON.parse(reply)), + { + clientKind: 'mobile', + pairedDeviceId: 'paired-device-1', + clientId: `credential-${index}`, + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY] + } + ).catch(reject) + }) + if (options.replacement && sent === 1) { + if (options.loss === 'timeout') { + directReplacement = true + throw markRpcDeliveryUnknown(new Error('Request timed out')) + } + return new Promise((_, reject) => { + rejectFirst = reject + void client.migrateTo(physical(1), 'relay') + }) + } + return response + } + } as unknown as RpcClient + } + const client = createStableLogicalRpcClient(physical(0), 'lan') + const result = readNewWorktreeRuntimeCapabilities(client) + .then((support) => + createWorktreeWithNameRetry({ + client, + baseName: 'otter', + buildParams: (name) => ({ repo: 'id:repo-1', name }), + worktreeCreateIdempotency: { dedupeTtlMs: 60_000 }, + agentLaunch: { agent: 'claude', supported: support.agentLaunch }, + mintLaunchOperationId: () => `${Date.now()}-${(++minted).toString(16).padStart(32, '0')}` + }) + ) + .finally(() => client.close()) + return { runtime, replacement, result } +} + +describe('mobile launch retry authority', () => { + it('admits a paired mobile create through the WebSocket method allowlist', async () => { + const runtime = { + ...runtimeStub(), + getRuntimeId: () => 'runtime-1', + configureNotificationDismissalStore: () => {} + } + const server = new OrcaRuntimeRpcServer({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture supplies the constructor and launch method dependencies. + runtime: runtime as unknown as OrcaRuntimeService, + userDataPath: directory, + enableWebSocket: false + }) + server['deviceRegistry'] = new DeviceRegistry(directory) + const mobile = server['deviceRegistry'].addDevice('test-phone', 'mobile') + const replies: RpcResponse[] = [] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: with no pairing provider or physical socket, this admission path reads only clientCapabilities. + const authenticatedSocket = { + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY] + } as unknown as AuthenticatedMobileSocket + await server['handleWebSocketMessage']( + JSON.stringify({ + id: 'mobile-create', + deviceToken: mobile.token, + method: 'agent.launchReplay', + params: { + operationId: `${Date.now()}-${'a'.repeat(32)}`, + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'otter' } } + } + }), + (reply) => replies.push(JSON.parse(reply)), + () => {}, + undefined, + undefined, + mobile.token, + authenticatedSocket + ) + expect(replies).toEqual([ + expect.objectContaining({ + ok: true, + result: expect.objectContaining({ worktreeId: 'wt-new' }) + }) + ]) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(store.listOperationRows()[0]?.callerKey).toBe(mobile.deviceId) + }) + + it('rejects an unnamed replay request before workspace creation', async () => { + const runtime = { ...runtimeStub(), getRuntimeId: () => 'runtime-1' } + const dispatcher = new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture implements the launch handler dependencies. + runtime: runtime as unknown as OrcaRuntimeService, + methods: AGENT_LAUNCH_METHODS + }) + const response = await dispatcher.dispatch({ + id: 'missing-operation', + authToken: 'token', + method: 'agent.launchReplay', + params: { + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'otter' } } + } + }) + expect(response.ok).toBe(false) + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + expect(createStructuredSession).not.toHaveBeenCalled() + }) + + it('shares one durable receipt across entry points and a reopened store', async () => { + const runtime = { ...runtimeStub(), getRuntimeId: () => 'runtime-1' } + const dispatcher = new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture implements the launch handler dependencies. + runtime: runtime as unknown as OrcaRuntimeService, + methods: AGENT_LAUNCH_METHODS + }) + const request = { + id: 'same-operation', + authToken: 'token', + method: 'agent.launch', + params: { + operationId: `${Date.now()}-${'e'.repeat(32)}`, + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'otter' } } + } + } + const first = await dispatcher.dispatch(request) + expect(first.ok).toBe(true) + store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: launch admission reads only deps.store from the installed host. + setStructuredAgentSessionHost({ deps: { store } } as unknown as StructuredAgentSessionHost) + await expect( + dispatcher.dispatch({ ...request, method: 'agent.launchReplay' }) + ).resolves.toEqual(first) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(store.listOperationRows()).toHaveLength(1) + }) + + it('does not duplicate when a replacement host strips operationId', async () => { + const launch = scenario({ replacement: 'legacy' }) + await expect(launch.result).resolves.toMatchObject({ + error: expect.stringContaining('Unknown method') + }) + expect( + launch.runtime.createManagedWorktree.mock.calls.length + + launch.replacement.createManagedWorktree.mock.calls.length + ).toBe(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) + + it('replays an exhausted collision after restart without searching names again', async () => { + const runtime = { ...runtimeStub(), getRuntimeId: () => 'runtime-1' } + runtime.createManagedWorktree.mockRejectedValueOnce( + new WorktreeCreateCollisionError('Branch "otter" already exists locally.') + ) + const dispatcher = new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture implements the launch handler dependencies. + runtime: runtime as unknown as OrcaRuntimeService, + methods: AGENT_LAUNCH_METHODS + }) + const request = { + id: 'exhausted-operation', + authToken: 'token', + method: 'agent.launchReplay', + params: { + operationId: `${Date.now()}-${'f'.repeat(32)}`, + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'otter' } } + } + } + await expect(dispatcher.dispatch(request)).resolves.toMatchObject({ + ok: false, + error: { + code: 'worktree_create_collision', + message: 'Branch "otter" already exists locally.' + } + }) + store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: launch admission reads only deps.store from the installed host. + setStructuredAgentSessionHost({ deps: { store } } as unknown as StructuredAgentSessionHost) + await expect(dispatcher.dispatch(request)).resolves.toMatchObject({ + ok: false, + error: { code: 'worktree_create_collision' } + }) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).not.toHaveBeenCalled() + }) + + it('does not interpret a nested refusal message as a fresh workspace candidate', async () => { + createStructuredSession.mockResolvedValueOnce({ + ok: false, + refusal: { + code: 'agent_session_operation_unknown', + message: 'Branch "setup" already exists.' + } + }) + const launch = scenario() + await expect(launch.result).resolves.toEqual({ error: 'agent_session_operation_unknown' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) + + it('refuses an older receiver reached after a connected timeout', async () => { + const launch = scenario({ replacement: 'legacy', loss: 'timeout' }) + await expect(launch.result).resolves.toMatchObject({ + error: expect.stringContaining('Unknown method') + }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(launch.replacement.createManagedWorktree).not.toHaveBeenCalled() + }) + + it('replays through a current replacement under the same paired-device identity', async () => { + const launch = scenario({ replacement: 'current' }) + await expect(launch.result).resolves.toEqual({ worktreeId: 'wt-new', name: 'otter' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(launch.replacement.createManagedWorktree).not.toHaveBeenCalled() + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) + + it('leaves exhausted name selection with the host and records a definitive failure', async () => { + const launch = scenario({ collision: true }) + await expect(launch.result).resolves.toEqual({ + error: 'Branch "otter" already exists locally.' + }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).not.toHaveBeenCalled() + expect(store.listOperationRows()[0]?.outcome).toMatchObject({ + status: 'failed', + code: 'worktree_create_collision' + }) + }) + + it.each([ + 'agent_launch_unsupported', + 'agent_launch_replay_unsupported', + 'method_not_found', + 'worktree_create_collision' + ])('does not downgrade or rename after nested %s text', async (message) => { + createStructuredSession.mockResolvedValueOnce({ + ok: false, + refusal: { code: 'agent_session_operation_unknown', message } + }) + const launch = scenario() + await expect(launch.result).resolves.toEqual({ error: 'agent_session_operation_unknown' }) + expect(launch.runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) +}) diff --git a/mobile/src/tasks/mobile-workspace-create-operations.ts b/mobile/src/tasks/mobile-workspace-create-operations.ts index 3da90f200ed..ed223a129b0 100644 --- a/mobile/src/tasks/mobile-workspace-create-operations.ts +++ b/mobile/src/tasks/mobile-workspace-create-operations.ts @@ -1,8 +1,13 @@ +import { z } from 'zod' +import { + isAgentLaunchResult, + type AgentLaunchResult +} from '../../../src/shared/agent-launch-intent' import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' +import { rpcResultVariant } from '../transport/rpc-operation-result-reader' import { rpcUncheckedPayloadReader } from '../transport/rpc-reader-payload' -// Creating a workspace from a task. Every reply here is one the call site only re-typed, so the -// readers are unchecked: moving a shape check in would be a validation change, not a migration. +// Legacy readers preserve their existing validation; the replay-required route validates receipts. /** * worktree.create. A lost reply is *unknown*, never failed — `worktree-create-retry.ts` replays on @@ -35,6 +40,16 @@ export const agentLaunchRun = bindDeferredRpcOperation( }) ) +export const agentLaunchReplayRun = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'agent.launch-replay', + method: 'agent.launchReplay', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: rpcResultVariant('agent-launch-receipt', z.custom(isAgentLaunchResult)) + }) +) + /** * The start point for a workspace created from a linked pull request. Refusal throws the host's * message; an accepted reply can still carry a soft `{ error }` the caller raises itself. diff --git a/mobile/src/tasks/source-workspace-create.test.ts b/mobile/src/tasks/source-workspace-create.test.ts index 24506b909e6..73064c4412c 100644 --- a/mobile/src/tasks/source-workspace-create.test.ts +++ b/mobile/src/tasks/source-workspace-create.test.ts @@ -322,7 +322,7 @@ describe('createWorkspaceFromComposerSource', () => { selection, ...baseArgs, agent: { choice: 'claude' }, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(result).toEqual({ worktreeId: 'wt-branch-launch', name: 'topic' }) @@ -350,7 +350,7 @@ describe('createWorkspaceFromComposerSource', () => { selection, ...baseArgs, agent: { choice: 'codex' }, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(calls.map((call) => call.method)).toEqual(['agent.launch']) @@ -369,7 +369,7 @@ describe('createWorkspaceFromComposerSource', () => { selection, ...baseArgs, agent: { choice: 'claude' }, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(calls[0]!.method).toBe('agent.launch') @@ -402,7 +402,7 @@ describe('createWorkspaceFromComposerSource', () => { selection, ...baseArgs, agent: { choice: 'claude' }, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(calls.map((call) => call.method)).toEqual(['worktree.create']) @@ -440,7 +440,7 @@ describe('createWorkspaceFromComposerSource', () => { client, selection, ...baseArgs, - agentLaunchSupported: true + agentLaunchSupported: { replay: false } }) expect(calls[0]!.method).toBe('worktree.create') diff --git a/mobile/src/tasks/source-workspace-create.ts b/mobile/src/tasks/source-workspace-create.ts index 81cc7ab3a6e..fb86701707a 100644 --- a/mobile/src/tasks/source-workspace-create.ts +++ b/mobile/src/tasks/source-workspace-create.ts @@ -35,7 +35,7 @@ export type CreateWorkspaceFromComposerArgs = { note: string | undefined worktreeCreateIdempotency: WorktreeCreateIdempotencyProbe /** Whether the host can settle the surface itself; false keeps the agent-first create. */ - agentLaunchSupported: boolean | Promise + agentLaunchSupported: WorktreeCreateAgentLaunch['supported'] } export async function createWorkspaceFromComposerSource( @@ -52,7 +52,7 @@ export async function createWorkspaceFromComposerSource( function resolveComposerAgentLaunch( agentId: TuiAgent | undefined, - supported: boolean | Promise + supported: WorktreeCreateAgentLaunch['supported'] ): WorktreeCreateAgentLaunch | undefined { return agentId ? { agent: agentId, supported } : undefined } @@ -168,7 +168,7 @@ async function createBranchWorkspace(args: { nameIsAutoManaged?: boolean note: string | undefined worktreeCreateIdempotency: WorktreeCreateIdempotencyProbe - agentLaunchSupported: boolean | Promise + agentLaunchSupported: WorktreeCreateAgentLaunch['supported'] }): Promise { const { client, @@ -259,7 +259,7 @@ async function createNewBranchWorkspace(args: { nameIsAutoManaged?: boolean note: string | undefined worktreeCreateIdempotency: WorktreeCreateIdempotencyProbe - agentLaunchSupported: boolean | Promise + agentLaunchSupported: WorktreeCreateAgentLaunch['supported'] }): Promise { const { client, diff --git a/mobile/src/tasks/worktree-create-capability.test.ts b/mobile/src/tasks/worktree-create-capability.test.ts index fd2bbaa14c2..830e1a61805 100644 --- a/mobile/src/tasks/worktree-create-capability.test.ts +++ b/mobile/src/tasks/worktree-create-capability.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { AGENT_LAUNCH_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' +import { + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY, + AGENT_LAUNCH_REPLAY_RUNTIME_CAPABILITY, + AGENT_LAUNCH_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' import type { RpcClient } from '../transport/rpc-client' import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' import { readNewWorktreeRuntimeCapabilities } from './worktree-create-capability' @@ -71,11 +75,55 @@ describe('readNewWorktreeRuntimeCapabilities', () => { ).resolves.toEqual({ tasksSupported: false, worktreeCreateIdempotency: false, - agentLaunch: true, + // Advertising the method is not advertising the ledger: `operationId` degrades silently on a + // host that has one and not the other, so the two are answered separately. + agentLaunch: { replay: false }, hostPlatform: 'darwin' }) }) + it('reads launch replay support only when the host advertises the ledger', async () => { + await expect( + readNewWorktreeRuntimeCapabilities( + statusClient([ + { + capabilities: [ + AGENT_LAUNCH_RUNTIME_CAPABILITY, + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY + ] + } + ]) + ) + ).resolves.toEqual({ + tasksSupported: false, + worktreeCreateIdempotency: false, + agentLaunch: { replay: true }, + hostPlatform: 'darwin' + }) + }) + + // A host cannot admit an operation for a method it does not have, so the ledger capability alone + // must not turn the launch route on. + it('ignores the replay capability on a host without agent.launch', async () => { + await expect( + readNewWorktreeRuntimeCapabilities( + statusClient([{ capabilities: [AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY] }]) + ) + ).resolves.toMatchObject({ agentLaunch: false }) + }) + + it('does not authorize replay from the optional-identity capability alone', async () => { + await expect( + readNewWorktreeRuntimeCapabilities( + statusClient([ + { + capabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY, AGENT_LAUNCH_REPLAY_RUNTIME_CAPABILITY] + } + ]) + ) + ).resolves.toMatchObject({ agentLaunch: { replay: false } }) + }) + it('uses the bounded fallback for an old idempotent host without an advertisement', async () => { await expect( readNewWorktreeRuntimeCapabilities(statusClient([['worktree.create-idempotency.v1']])) diff --git a/mobile/src/tasks/worktree-create-capability.ts b/mobile/src/tasks/worktree-create-capability.ts index 3b91052bcf5..f3787728975 100644 --- a/mobile/src/tasks/worktree-create-capability.ts +++ b/mobile/src/tasks/worktree-create-capability.ts @@ -1,5 +1,9 @@ import { useCallback, useEffect, useRef, useState } from 'react' -import { AGENT_LAUNCH_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' +import { + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY, + AGENT_LAUNCH_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' +import type { AgentLaunchSupport } from './agent-launch-worktree-create' import type { RpcClient } from '../transport/rpc-client' import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client' import { readMobileRuntimeHostPlatform } from '../transport/mobile-runtime-host-platform' @@ -21,9 +25,9 @@ const STATUS_CUTOVER_MAX_RETRIES = 5 export type NewWorktreeRuntimeCapabilities = { tasksSupported: boolean worktreeCreateIdempotency: WorktreeCreateIdempotencySupport | false - /** Whether the host can route a create through `agent.launch`; an older one only knows - * `worktree.create` + `startupAgent`, which is always a terminal agent. */ - agentLaunch: boolean + /** Whether the host can route a create through `agent.launch`, and what that launch supports; + * an older one only knows `worktree.create` + `startupAgent`, always a terminal agent. */ + agentLaunch: AgentLaunchSupport | false hostPlatform: NodeJS.Platform | null } @@ -59,7 +63,10 @@ export async function readNewWorktreeRuntimeCapabilities( const advertisedIdempotency = result.worktreeCreateIdempotency return { tasksSupported: capabilities.includes(MOBILE_TASKS_CAPABILITY), - agentLaunch: capabilities.includes(AGENT_LAUNCH_RUNTIME_CAPABILITY), + // Unsupported stays plain `false`, the same shape `worktreeCreateIdempotency` uses. + agentLaunch: capabilities.includes(AGENT_LAUNCH_RUNTIME_CAPABILITY) + ? { replay: capabilities.includes(AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY) } + : false, worktreeCreateIdempotency: supportsIdempotency ? advertisedIdempotency === undefined ? { dedupeTtlMs: WORKTREE_CREATE_DEDUPE_TTL_LEGACY_HOST_MS } @@ -88,7 +95,7 @@ export function useNewWorktreeRuntimeCapabilities( tasksSupported: boolean hostPlatform: NodeJS.Platform | null getWorktreeCreateCutoverSupport: () => Promise - getAgentLaunchSupport: () => Promise + getAgentLaunchSupport: () => Promise } { const [tasksSupported, setTasksSupported] = useState(false) const [hostPlatform, setHostPlatform] = useState(null) diff --git a/mobile/src/tasks/worktree-create-launch-operation.test.ts b/mobile/src/tasks/worktree-create-launch-operation.test.ts new file mode 100644 index 00000000000..76e4f3be391 --- /dev/null +++ b/mobile/src/tasks/worktree-create-launch-operation.test.ts @@ -0,0 +1,333 @@ +import { describe, expect, it } from 'vitest' +import { parseAgentSessionOperationTimestamp } from '../../../src/shared/agent-session-host-authority' +import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import type { RpcClient } from '../transport/rpc-client' +import type { ConnectionState } from '../transport/types' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import { WORKTREE_CREATE_DEDUPE_TTL_LEGACY_HOST_MS } from './worktree-create-idempotency-policy' +import { createWorktreeWithNameRetry, type WorktreeCreateResult } from './worktree-create-retry' + +type Attempt = { method: string; params: Record } + +const IDEMPOTENT_CREATE_SUPPORT = { dedupeTtlMs: WORKTREE_CREATE_DEDUPE_TTL_LEGACY_HOST_MS } + +// Lets a parked replay reach its state wait before the test resumes the transport. +async function flush(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)) +} + +// A transport scripted per call, recording what each attempt put on the wire. Narrower than the +// one `worktree-create-retry.test.ts` drives: the launch route needs a receipt, a coded refusal +// and one ambiguous drop, and nothing here reads the replay deadline. +function scriptedLaunchClient( + outcomes: Array< + | { launched: string } + | { created: string } + | { errorCode: string; errorMessage?: string } + | { throws: unknown; dropsConnection?: boolean } + >, + attempts: Attempt[] +): RpcClient & { reconnect: () => void } { + let call = 0 + let state: ConnectionState = 'connected' + const listeners = new Set<(next: ConnectionState) => void>() + const setState = (next: ConnectionState): void => { + state = next + for (const listener of listeners) { + listener(next) + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The retry loop reads only these members of RpcClient; spelling out the rest would be a fake transport pretending to be a real one. + return { + reconnect: () => setState('connected'), + getState: () => state, + getLastInboundAt: () => null, + onStateChange: (listener: (next: ConnectionState) => void) => { + listeners.add(listener) + return () => listeners.delete(listener) + }, + sendRequest: async (method: string, params?: unknown) => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The port is untyped by construction; the suite asserts on what was sent, not on its declared shape. + attempts.push({ method, params: (params ?? {}) as Record }) + const outcome = outcomes[Math.min(call, outcomes.length - 1)]! + call += 1 + if ('throws' in outcome) { + if (outcome.dropsConnection) { + setState('reconnecting') + } + throw outcome.throws + } + if ('errorCode' in outcome) { + return { + id: '1', + ok: false, + error: { code: outcome.errorCode, message: outcome.errorMessage ?? outcome.errorCode }, + _meta: { runtimeId: 'r' } + } + } + return { + id: '1', + ok: true, + // A launch answers with a bare `worktreeId`; `worktree.create` wraps one in `worktree`. + result: + 'launched' in outcome + ? { + worktreeId: outcome.launched, + outcome: { kind: 'terminal', handle: 'term_x' }, + receipt: { + mode: 'terminal', + preferred: 'terminal', + reason: 'user_default', + detail: 'Terminal selected' + } + } + : { worktree: { id: outcome.created } }, + _meta: { runtimeId: 'r' } + } + } + } as unknown as RpcClient & { reconnect: () => void } +} + +// operationId names the full launch; the host owns suffix selection and bypasses the legacy cache. +describe('agent.launch operation id', () => { + const launchOperationIds = (attempts: Attempt[]): unknown[] => + attempts.map((attempt) => attempt.params.operationId) + const launchCandidateNames = (attempts: Attempt[]): unknown[] => + attempts.map((attempt) => { + const target = attempt.params.target + if (!target || typeof target !== 'object' || !('create' in target)) { + return undefined + } + const create = target.create + return create && typeof create === 'object' && 'name' in create ? create.name : undefined + }) + + function launchRetry(args: { + client: RpcClient + attempts: Attempt[] + replay?: boolean + supported?: boolean + worktreeCreateIdempotency?: false + mintLaunchOperationId?: () => string + }): Promise { + let minted = 0 + return createWorktreeWithNameRetry({ + client: args.client, + baseName: 'otter', + buildParams: (name) => ({ repo: 'id:r', name }), + worktreeCreateIdempotency: args.worktreeCreateIdempotency ?? IDEMPOTENT_CREATE_SUPPORT, + mintMutationId: () => 'key-launch', + agentLaunch: { + agent: 'claude', + supported: args.supported === false ? false : { replay: args.replay !== false } + }, + mintLaunchOperationId: args.mintLaunchOperationId ?? (() => `op-${(minted += 1)}`) + }) + } + + it('names the launch and reuses that name on an ambiguous replay of the same candidate', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [ + { + throws: markRpcDeliveryUnknown(new Error('Connection interrupted')), + dropsConnection: true + }, + { launched: 'wt-launch' } + ], + attempts + ) + const pending = launchRetry({ client, attempts }) + await flush() + client.reconnect() + + await expect(pending).resolves.toEqual({ worktreeId: 'wt-launch', name: 'otter' }) + expect(attempts.map((attempt) => attempt.method)).toEqual([ + 'agent.launchReplay', + 'agent.launchReplay' + ]) + // The whole point: the replay is the SAME operation, so the host returns the recorded + // answer instead of launching a second agent in a second workspace. + expect(launchOperationIds(attempts)).toEqual(['op-1', 'op-1']) + }) + + it('reuses the launch name across a connection-migration cutover too', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [{ throws: new LogicalClientCutoverError() }, { launched: 'wt-mig' }], + attempts + ) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + worktreeId: 'wt-mig', + name: 'otter' + }) + expect(launchOperationIds(attempts)).toEqual(['op-1', 'op-1']) + }) + + it('uses launch replay support independently of worktree.create idempotency', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [{ throws: new LogicalClientCutoverError() }, { launched: 'wt-replay' }], + attempts + ) + + await expect( + launchRetry({ client, attempts, worktreeCreateIdempotency: false }) + ).resolves.toEqual({ worktreeId: 'wt-replay', name: 'otter' }) + expect(launchOperationIds(attempts)).toEqual(['op-1', 'op-1']) + }) + + it('bounds named timeout retries without minting another operation', async () => { + const attempts: Attempt[] = [] + const error = markRpcDeliveryUnknown(new Error('Request timed out')) + const client = scriptedLaunchClient([{ throws: error }], attempts) + + await expect(launchRetry({ client, attempts })).rejects.toBe(error) + expect(attempts).toHaveLength(3) + expect(launchOperationIds(attempts)).toEqual(['op-1', 'op-1', 'op-1']) + }) + + it('does not restart the host name search with another operation after a collision', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [ + { errorCode: 'worktree_create_collision', errorMessage: 'already exists locally' }, + { launched: 'wt-bumped' } + ], + attempts + ) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + error: 'already exists locally' + }) + expect(launchCandidateNames(attempts)).toEqual(['otter']) + expect(launchOperationIds(attempts)).toEqual(['op-1']) + }) + + it('sends no launch name to a host that advertises agent.launch without the ledger', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient([{ launched: 'wt-plain' }], attempts) + await expect(launchRetry({ client, attempts, replay: false })).resolves.toEqual({ + worktreeId: 'wt-plain', + name: 'otter' + }) + expect(attempts[0]!.method).toBe('agent.launch') + expect(attempts[0]!.params.operationId).toBeUndefined() + // Byte-identical to today: the rest of the payload is untouched. + expect(attempts[0]!.params.target).toEqual({ + kind: 'create-worktree', + create: { repo: 'id:r', name: 'otter', clientMutationId: 'key-launch' } + }) + }) + + it('sends no launch name at all when the host has no agent.launch', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient([{ created: 'wt-legacy-route' }], attempts) + await expect(launchRetry({ client, attempts, supported: false })).resolves.toEqual({ + worktreeId: 'wt-legacy-route', + name: 'otter' + }) + expect(attempts[0]!.method).toBe('worktree.create') + expect(attempts[0]!.params.operationId).toBeUndefined() + }) + + // These codes also describe a refused nested attach or an expired receipt after creation. + it.each([ + 'agent_session_operation_capacity', + 'agent_session_operation_invalid', + 'agent_session_operation_expired' + ])('preserves the operation identity when the host refuses with %s', async (code) => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient([{ errorCode: code }, { launched: 'wt-unnamed' }], attempts) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ error: code }) + expect(attempts).toHaveLength(1) + expect(launchOperationIds(attempts)).toEqual(['op-1']) + }) + + // An unsettled claim is evidence that the launch may already have run. + it('surfaces an unknown operation without re-sending and without re-minting', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [{ errorCode: 'agent_session_operation_unknown' }], + attempts + ) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + error: 'agent_session_operation_unknown' + }) + expect(attempts).toHaveLength(1) + expect(launchOperationIds(attempts)).toEqual(['op-1']) + }) + + // A conflict can only come from a client that reused one id across two payloads. Re-sending it + // unnamed would hide that bug behind a create that quietly works. + it('surfaces an operation conflict rather than re-sending unnamed', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [{ errorCode: 'agent_session_operation_conflict' }], + attempts + ) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + error: 'agent_session_operation_conflict' + }) + expect(attempts).toHaveLength(1) + }) + + // The pre-existing downgrade arm: a host that refuses the method drops to `worktree.create`, + // which has no operation ledger, so the id must not ride along. + it('drops the launch name when the host refuses agent.launch itself', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [ + { errorCode: 'method_not_found', errorMessage: 'Unknown method' }, + { created: 'wt-downgraded' } + ], + attempts + ) + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + worktreeId: 'wt-downgraded', + name: 'otter' + }) + expect(attempts.map((attempt) => attempt.method)).toEqual([ + 'agent.launchReplay', + 'worktree.create' + ]) + expect(launchOperationIds(attempts)).toEqual(['op-1', undefined]) + }) + + it('does not downgrade a launch when a replacement connection refuses the method', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient( + [ + { throws: new LogicalClientCutoverError() }, + { errorCode: 'agent_launch_unsupported' }, + { created: 'wt-duplicate' } + ], + attempts + ) + + await expect(launchRetry({ client, attempts })).resolves.toEqual({ + error: 'agent_launch_unsupported' + }) + expect(attempts.map((attempt) => attempt.method)).toEqual([ + 'agent.launchReplay', + 'agent.launchReplay' + ]) + expect(launchOperationIds(attempts)).toEqual(['op-1', 'op-1']) + }) + + it('mints a real durable id in production', async () => { + const attempts: Attempt[] = [] + const client = scriptedLaunchClient([{ launched: 'wt-real' }], attempts) + await createWorktreeWithNameRetry({ + client, + baseName: 'otter', + buildParams: (name) => ({ repo: 'id:r', name }), + worktreeCreateIdempotency: IDEMPOTENT_CREATE_SUPPORT, + agentLaunch: { agent: 'claude', supported: { replay: true } } + }) + // The host refuses anything else on the wire; parsing it back is what proves the default + // minter, not a test double, produces the shipped shape. + expect( + parseAgentSessionOperationTimestamp(String(attempts[0]!.params.operationId)) + ).toBeCloseTo(Date.now(), -4) + }) +}) diff --git a/mobile/src/tasks/worktree-create-retry.ts b/mobile/src/tasks/worktree-create-retry.ts index 955f6d63a1a..b1c52b5f796 100644 --- a/mobile/src/tasks/worktree-create-retry.ts +++ b/mobile/src/tasks/worktree-create-retry.ts @@ -2,7 +2,11 @@ import type { TuiAgent } from '../../../src/shared/tui-agent' import type { RpcClient } from '../transport/rpc-client' import type { RpcResponse } from '../transport/types' import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' -import { agentLaunchRun, worktreeCreateRun } from './mobile-workspace-create-operations' +import { + agentLaunchRun, + agentLaunchReplayRun, + worktreeCreateRun +} from './mobile-workspace-create-operations' import { waitForRpcClientReconnected } from '../transport/rpc-client-reconnect-wait' import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client' import { @@ -17,6 +21,7 @@ import { readAgentLaunchCreateOutcome, type WorktreeCreateAgentLaunch } from './agent-launch-worktree-create' +import { structuredSessionOperationId } from '../session/structured-session-operation-id' import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout' import type { WorkspaceCreateParams } from './workspace-create-params' import { @@ -66,6 +71,8 @@ export type CreateWorktreeWithNameRetryArgs = { maxAttempts?: number // Injected in tests; production mints a fresh idempotency key per candidate. mintMutationId?: () => string + // Injected in tests; the replay-required launch keeps one identity across all deliveries. + mintLaunchOperationId?: () => string } // Creates a worktree, retrying with a numeric suffix on a name-collision error. @@ -82,9 +89,10 @@ export async function createWorktreeWithNameRetry( const worktreeCreateIdempotency = await args.worktreeCreateIdempotency // Why: the route must settle before the first create, so a name-collision retry cannot land on // a different method than the attempt it replaces. - let launchAgent = await resolveAgentLaunchRoute(args.agentLaunch) + let launch = await resolveAgentLaunchRoute(args.agentLaunch) const maxAttempts = args.maxAttempts ?? CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS const mintMutationId = args.mintMutationId ?? defaultWorktreeCreateMutationId + const mintLaunchOperationId = args.mintLaunchOperationId ?? structuredSessionOperationId let lastError: string | null = null for (let attempt = 0; attempt < maxAttempts; attempt += 1) { const candidateName = args.nameWasGenerated @@ -97,22 +105,38 @@ export async function createWorktreeWithNameRetry( const params = worktreeCreateIdempotency ? { ...candidateParams, clientMutationId: mintMutationId() } : candidateParams - let response = await sendWorktreeCreateResilient( + // Replay-required launches own suffix selection on the host; this id names the entire create. + const launchOperationId = launch?.replay ? mintLaunchOperationId() : null + const sent = await sendWorktreeCreateResilient( client, - launchAgent, + launch?.agent ?? null, + launchOperationId, params, worktreeCreateIdempotency ) - if (!response.ok && launchAgent && isAgentLaunchUnsupportedRefusal(response.error)) { + let response = sent.response + if ( + !response.ok && + !sent.replayed && + launch && + (launchOperationId + ? response.error.code === 'method_not_found' || + response.error.code === 'forbidden' || + response.error.code === 'agent_launch_replay_unsupported' + : isAgentLaunchUnsupportedRefusal(response.error)) + ) { // The probe said the host knows `agent.launch` but it refused the call — most likely this // client's capability list had not landed yet. Downgrade for good rather than fail a create. - launchAgent = null - response = await sendWorktreeCreateResilient(client, null, params, worktreeCreateIdempotency) + launch = null + response = ( + await sendWorktreeCreateResilient(client, null, null, params, worktreeCreateIdempotency) + ).response } + // Ledger refusals can follow workspace creation or an expired receipt; never retry unnamed. // Why the raw refusal: the retry decision below is `isRetryableWorktreeCreateConflict` over the // host's message, and no acceptance policy carries a refusal message through without throwing. if (response.ok) { - const created = readCreateResult(response, launchAgent !== null) + const created = readCreateResult(response, launch) if (created) { return { worktreeId: created.worktreeId, @@ -124,7 +148,8 @@ export async function createWorktreeWithNameRetry( break } lastError = response.error.message - if (!isRetryableWorktreeCreateConflict(lastError ?? '')) { + // The replay-required host already exhausted its candidates; only legacy hosts need this loop. + if (launch?.replay || !isRetryableWorktreeCreateConflict(lastError ?? '')) { break } } @@ -133,11 +158,12 @@ export async function createWorktreeWithNameRetry( async function resolveAgentLaunchRoute( launch: WorktreeCreateAgentLaunch | undefined -): Promise { +): Promise<{ agent: TuiAgent; replay: boolean } | null> { if (!launch) { return null } - return (await launch.supported) ? launch.agent : null + const support = await launch.supported + return support ? { agent: launch.agent, replay: support.replay } : null } // A launch receipt carries no display name, so the candidate stands in; the session route @@ -145,10 +171,11 @@ async function resolveAgentLaunchRoute( // a create that seated the workspace but could not start the agent surface. function readCreateResult( response: RpcResponse, - launched: boolean + launch: { replay: boolean } | null ): { worktreeId: string; displayName?: string; warning?: string } | null { - if (launched) { - return readAgentLaunchCreateOutcome(agentLaunchRun.interpret(response)) + if (launch) { + const operation = launch.replay ? agentLaunchReplayRun : agentLaunchRun + return readAgentLaunchCreateOutcome(operation.interpret(response)) } // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. const created = worktreeCreateRun.interpret(response) as { @@ -172,16 +199,24 @@ function readCreateResult( // Sends the create, re-issuing whenever the request went delivery-ambiguous — // the frame reached the wire but no response came back, so the host may already have -// built the worktree. The shared clientMutationId keeps the retry idempotent host-side — -// `agent.launch` carries it in the same create payload, so a replayed launch reconciles onto the -// first worktree and can at worst add a second surface inside it, never a second workspace. +// built the worktree. Every arm below re-sends the SAME two names: a new one would be a new +// operation and would defeat both mechanisms. +// +// On the `worktree.create` route the shared clientMutationId keeps the retry idempotent host-side. +// On the launch route it does NOT reach the ledger: `agent.launch` caches the whole launch — the +// worktree AND the surface — under that id for 60s, so inside that window a replay adds neither, +// and outside it adds both. `launchOperationId` is what makes the replay durably safe, and it is +// only sent when the host advertised the ledger. // A definite failure (never sent, or a server error response) is returned to the caller untouched. async function sendWorktreeCreateResilient( client: RpcClient, launchAgent: TuiAgent | null, + launchOperationId: string | null, params: WorkspaceCreateParams, worktreeCreateIdempotency: WorktreeCreateIdempotencySupport | false -): Promise { +): Promise<{ response: RpcResponse; replayed: boolean }> { + // Only the selected method's receipt can authorize replay after an ambiguous delivery. + const replaySupport = launchAgent ? launchOperationId : worktreeCreateIdempotency let migrationRetry = 0 let ambiguousRetry = 0 const firstSentAt = Date.now() @@ -190,15 +225,28 @@ async function sendWorktreeCreateResilient( try { // `request` is the transport promise itself, so a delivery-unknown rejection reaches the // catch below as the object the transport marked — the WeakSet cannot see through a wrapper. - return await (launchAgent - ? agentLaunchRun.request(client, agentLaunchCreateParams(launchAgent, params), { - timeoutMs: WORKTREE_CREATE_TIMEOUT_MS - }) + const response = await (launchAgent + ? launchOperationId + ? agentLaunchReplayRun.request( + client, + { + ...agentLaunchCreateParams(launchAgent, params), + operationId: launchOperationId + }, + { timeoutMs: WORKTREE_CREATE_TIMEOUT_MS } + ) + : agentLaunchRun.request( + client, + agentLaunchCreateParams(launchAgent, params, launchOperationId), + { timeoutMs: WORKTREE_CREATE_TIMEOUT_MS } + ) : worktreeCreateRun.request(client, params, { timeoutMs: WORKTREE_CREATE_TIMEOUT_MS })) + // A refusal on the replacement connection says nothing about what the first call created. + return { response, replayed: migrationRetry > 0 || ambiguousRetry > 0 } } catch (error) { - if (!worktreeCreateIdempotency) { + if (!replaySupport) { throw error } if (isLogicalClientCutoverError(error)) { @@ -213,29 +261,21 @@ async function sendWorktreeCreateResilient( if (!isRpcDeliveryUnknown(error) || ambiguousRetry >= WORKTREE_CREATE_AMBIGUOUS_MAX_RETRIES) { throw error } - // Why: every transport path that reports a *drop* leaves 'connected' before the - // rejection reaches us (rpc-client.ts:675/695/1213 set state first or reject via - // queueMicrotask; the relay's fail() publishes synchronously). So still being - // 'connected' here means the socket was healthy the whole time and only the - // response went missing — the request-timeout path, which surfaces after - // WORKTREE_CREATE_TIMEOUT_MS. That says nothing about when the host actually - // resolved, so the dedupe record may be long gone and a replay would build a - // second worktree instead of reconciling. Fail the create instead. - if (client.getState() === 'connected') { + // A legacy cache may expire before a request timeout; durable receipts refuse unsafe replay. + if (typeof replaySupport !== 'string' && client.getState() === 'connected') { throw error } - // Computed once: a later ambiguity reads a fresher lastInboundAt from the - // replacement session, which would push the deadline past the record it respects. - replayDeadlineAt ??= resolveReplayDeadline(client, firstSentAt, worktreeCreateIdempotency) + // Keep the legacy deadline fixed; the host itself refuses expired durable operation IDs. + replayDeadlineAt ??= + typeof replaySupport === 'string' + ? Infinity + : resolveReplayDeadline(client, firstSentAt, replaySupport) const remainingWindowMs = replayDeadlineAt - Date.now() if (remainingWindowMs <= 0) { throw error } ambiguousRetry += 1 - // Why: unlike a cutover, no replacement session exists yet — resending now - // would just hit the dead one, so wait for the transport to come back and - // surface the original ambiguity if it does not. Clamped to the window so the - // wait itself cannot carry the replay past the host's record. + // Disconnected transports must reconnect before resend; bound the wait even for durable IDs. if ( !(await waitForRpcClientReconnected( client, diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index beff64afc65..1bfe141f4fc 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -42,6 +42,7 @@ import { resolveDefaultBaseRefWithLocalGit } from '../git/repo' import { getBranchConflictKindViaExec } from '../git/repo-branch-conflict' +import { WorktreeCreateCollisionError } from '../../shared/new-workspace/worktree-create-collision' import { resolveLocalGitUsername, getSshGitUsername } from '../git/git-username' import { hasCommitObjectViaGitExec } from '../git/commit-object-ref' import { @@ -1990,7 +1991,7 @@ export async function createRemoteWorktree( } if (!remotePathResolved) { if (lastBranchConflictKind) { - throw new Error( + throw new WorktreeCreateCollisionError( `Branch "${branchName}" already exists ${lastBranchConflictKind === 'local' ? 'locally' : 'on a remote'}. Pick a different ${branchConflictSubject}.` ) } @@ -2658,12 +2659,12 @@ async function performLocalWorktreeCreate( // narrowing does not reach the message. const existingReviewNumber = lastExistingReviewNumber if (existingReviewNumber !== null) { - throw new Error( + throw new WorktreeCreateCollisionError( `Branch "${branchName}" already has PR #${String(existingReviewNumber)}. Pick a different ${branchConflictSubject}.` ) } if (lastBranchConflictKind) { - throw new Error( + throw new WorktreeCreateCollisionError( `Branch "${branchName}" already exists ${lastBranchConflictKind === 'local' ? 'locally' : 'on a remote'}. Pick a different ${branchConflictSubject}.` ) } diff --git a/src/main/runtime/rpc/errors.ts b/src/main/runtime/rpc/errors.ts index df246112b32..ec1d40081c8 100644 --- a/src/main/runtime/rpc/errors.ts +++ b/src/main/runtime/rpc/errors.ts @@ -24,6 +24,7 @@ import { GIT_DIFF_TOO_LARGE_CODE } from '../../../shared/git-diff-transport-budg import { AUTOMATION_OWNER_CONFLICT_CODES } from '../../../shared/automation-owner-conflict' import { ARCHIVE_HOOK_FAILED_REMOVAL_CODE } from '../../../shared/worktree/archive-hook-removal-gate' import { NESTED_WORKER_DEPTH_EXCEEDED_CODE } from '../../../shared/nested-worker-depth' +import { WORKTREE_CREATE_COLLISION_CODE } from '../../../shared/new-workspace/worktree-create-collision' export function successResponse(id: string, meta: RpcEnvelopeMeta, result: unknown): RpcSuccess { return { @@ -54,6 +55,8 @@ export function errorResponse( // on — expanding or renaming entries without updating the CLI would silently // change user-visible error codes. const RUNTIME_PASSTHROUGH_CODES: ReadonlySet = new Set([ + WORKTREE_CREATE_COLLISION_CODE, + 'agent_launch_replay_unsupported', 'runtime_unavailable', 'selector_not_found', 'selector_ambiguous', @@ -80,6 +83,7 @@ const RUNTIME_PASSTHROUGH_CODES: ReadonlySet = new Set([ const COMPUTER_PASSTHROUGH_CODES: ReadonlySet = new Set(Object.values(COMPUTER_ERROR_CODES)) const LINEAR_PASSTHROUGH_CODES: ReadonlySet = new Set(LINEAR_ERROR_CODES) const STRUCTURED_RUNTIME_PASSTHROUGH_CODES: ReadonlySet = new Set([ + WORKTREE_CREATE_COLLISION_CODE, 'worktree_id_requires_full_path', 'run_not_found', 'run_required', diff --git a/src/main/runtime/rpc/methods/agent-launch-schemas.ts b/src/main/runtime/rpc/methods/agent-launch-schemas.ts index 995c3d836f1..7486d79bbfa 100644 --- a/src/main/runtime/rpc/methods/agent-launch-schemas.ts +++ b/src/main/runtime/rpc/methods/agent-launch-schemas.ts @@ -1,4 +1,5 @@ export { AgentLaunch, + AgentLaunchReplay, type AgentLaunchParams } from '../../../../shared/rpc-contract/agent-launch-params' diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index d9da45950f8..af6e018a36d 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -27,11 +27,15 @@ import type { AgentLaunchTarget } from '../../../../shared/agent-launch-intent' import { agentSessionOperationKey } from '../../../../shared/agent-session-operation-ledger' +import { + WorktreeCreateCollisionError, + WORKTREE_CREATE_COLLISION_CODE +} from '../../../../shared/new-workspace/worktree-create-collision' import { executeAgentLaunch } from '../../../agent-launch/agent-launch-executor' import type { OrcaRuntimeService } from '../../orca-runtime' import { defineMethod, type RpcContext } from '../core' import { admitAgentLaunchOperation, agentLaunchOperationCallerKey } from './agent-launch-replay' -import { AgentLaunch, type AgentLaunchParams } from './agent-launch-schemas' +import { AgentLaunch, AgentLaunchReplay, type AgentLaunchParams } from './agent-launch-schemas' import { agentLaunchSurfaceFactory } from './agent-launch-surfaces' import { agentLaunchWorkspaceFactory } from './agent-launch-worktree-creation' @@ -184,6 +188,12 @@ type ActiveAgentLaunch = { promise: Promise } +class AgentLaunchExecutionError extends Error { + constructor(cause: unknown) { + super('agent_session_operation_unknown', { cause }) + } +} + const activeAgentLaunchesByRuntime = new WeakMap< OrcaRuntimeService, Map @@ -218,13 +228,16 @@ async function executeReplaySafeAgentLaunch( await settleQuietly(admission.fail(agentLaunchFailureCode(error))) throw error } - // Any later failure may follow a created surface, so the claimed row must stay `unknown`. - const result = await runAgentLaunch( - intent, - context, - admission.attachOperationId, - admission.callerKey - ) + // Only a typed pre-creation collision proves that the claimed launch had no effects. + let result: AgentLaunchResult + try { + result = await runAgentLaunch(intent, context, admission.attachOperationId, admission.callerKey) + } catch (error) { + if (error instanceof WorktreeCreateCollisionError) { + await settleQuietly(admission.fail(WORKTREE_CREATE_COLLISION_CODE)) + } + throw new AgentLaunchExecutionError(error) + } // Settlement is bookkeeping; failure leaves the truthful `unknown` refusal for later retries. await settleQuietly(admission.settle(result)) return result @@ -257,6 +270,29 @@ function runReplaySafeAgentLaunch( } export const AGENT_LAUNCH_METHODS = [ + defineMethod({ + name: 'agent.launchReplay', + params: AgentLaunchReplay, + handler: async (params, context): Promise => { + if (!supportsAgentLaunch(context)) { + throw new Error('agent_launch_replay_unsupported') + } + try { + return await runReplaySafeAgentLaunch(params, context) + } catch (error) { + // Nested failures cannot authorize another workspace, regardless of their message or code. + if (error instanceof AgentLaunchExecutionError) { + if (error.cause instanceof WorktreeCreateCollisionError) { + throw Object.assign(new Error(error.cause.message, { cause: error.cause }), { + code: WORKTREE_CREATE_COLLISION_CODE + }) + } + throw new Error('agent_session_operation_unknown', { cause: error.cause }) + } + throw error + } + } + }), defineMethod({ name: 'agent.launch', params: AgentLaunch, @@ -273,7 +309,10 @@ export const AGENT_LAUNCH_METHODS = [ operationId: params.operationId }, context - ) + ).catch((error: unknown) => { + // Preserve the original error contract for callers of the optional-identity method. + throw error instanceof AgentLaunchExecutionError ? error.cause : error + }) } }) ] diff --git a/src/main/runtime/runtime-local-worktree-create-candidate.ts b/src/main/runtime/runtime-local-worktree-create-candidate.ts index 0ffef7162de..aacf6ae0b49 100644 --- a/src/main/runtime/runtime-local-worktree-create-candidate.ts +++ b/src/main/runtime/runtime-local-worktree-create-candidate.ts @@ -1,4 +1,5 @@ import type { Repo } from '../../shared/repo-types' +import { WorktreeCreateCollisionError } from '../../shared/new-workspace/worktree-create-collision' import type { CreateWorktreeArgs } from '../../shared/worktree/create-types' import type { getPRForBranch } from '../github/client' import { @@ -195,7 +196,7 @@ export async function resolveRuntimeLocalWorktreeCreateCandidate(args: { } if (!worktreePathResolved) { if (branchConflictKind) { - throw new Error( + throw new WorktreeCreateCollisionError( `Branch "${branchName}" already exists ${branchConflictKind === 'local' ? 'locally' : 'on a remote'}.` ) } diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts index 69cd49ccd30..ada9d7f0155 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts @@ -7,6 +7,7 @@ export const MOBILE_RPC_METHOD_ALLOWLIST = new Set([ 'accounts.subscribe', 'accounts.unsubscribe', 'agent.launch', + 'agent.launchReplay', 'aiVault.listSessions', 'aiVault.searchSessions', 'aiVault.searchStatus', diff --git a/src/shared/new-workspace/worktree-create-collision.ts b/src/shared/new-workspace/worktree-create-collision.ts new file mode 100644 index 00000000000..bc584a97aa2 --- /dev/null +++ b/src/shared/new-workspace/worktree-create-collision.ts @@ -0,0 +1,4 @@ +export const WORKTREE_CREATE_COLLISION_CODE = 'worktree_create_collision' as const + +// Marks an exhausted name search before any workspace was created. +export class WorktreeCreateCollisionError extends Error {} diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 1b9734627ab..a7c15bc1296 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -257,19 +257,13 @@ export const NOTIFICATIONS_REMOTE_PUSH_RUNTIME_CAPABILITY = 'notifications.remot // v2 makes prompt delivery an outcome union and top-level warnings the only supported shape. export const AGENT_LAUNCH_RUNTIME_CAPABILITY = 'agent.launch.v2' as const -/** - * The host admits `agent.launch` through the durable operation ledger, so a caller that names its - * launch with `operationId` gets exactly one execution and a recorded answer on every retry. - * - * This one is negotiated host-to-client, unlike `agent.launch.v1`, because of how RPC params - * degrade: an older host strips `operationId` as an unknown key and runs the launch anyway, with no - * error. A client that retried on the strength of having sent an id would get a second agent and - * never learn why. So `operationId` is optional on the wire — shipped mobile sends none and keeps - * today's behaviour verbatim — and a client may only treat a retry as safe once the host has - * advertised this. - */ +// Optional identity support on agent.launch; mobile replay across replacement hosts requires the new method. export const AGENT_LAUNCH_REPLAY_RUNTIME_CAPABILITY = 'agent.launch.replay.v1' as const +// agent.launchReplay requires the ledger; older replacement hosts must reject the method. +export const AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY = + 'agent.launch.replay-required.v1' as const + // Generic native clients include the CLI and must not claim Electron-only page // placement support. export const NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES = [ @@ -378,7 +372,8 @@ export const RUNTIME_CAPABILITIES = [ AUTOMATION_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, NOTIFICATIONS_REMOTE_PUSH_RUNTIME_CAPABILITY, AGENT_LAUNCH_RUNTIME_CAPABILITY, - AGENT_LAUNCH_REPLAY_RUNTIME_CAPABILITY + AGENT_LAUNCH_REPLAY_RUNTIME_CAPABILITY, + AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY ] as const export type RuntimeCapability = (typeof RUNTIME_CAPABILITIES)[number] | (string & {}) diff --git a/src/shared/rpc-contract/agent-launch-params.ts b/src/shared/rpc-contract/agent-launch-params.ts index 987b4f6e3f6..d719d0b0676 100644 --- a/src/shared/rpc-contract/agent-launch-params.ts +++ b/src/shared/rpc-contract/agent-launch-params.ts @@ -69,3 +69,6 @@ export const AgentLaunch = z.object({ }) export type AgentLaunchParams = z.infer + +// A distinct method prevents an older receiver from silently dropping the replay requirement. +export const AgentLaunchReplay = AgentLaunch.required({ operationId: true }) diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index e8ca11e2ca2..46ae6f7d27f 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -34,7 +34,7 @@ import { SelectCodexAccountForTargetParams } from './accounts-params' import { PrepareCodexForWslPaneParams } from './agent-hooks-params' -import { AgentLaunch } from './agent-launch-params' +import { AgentLaunch, AgentLaunchReplay } from './agent-launch-params' import { CreateAgentSessionParams, EnsureAgentSessionParams } from './agent-session-params' import { AiVaultListSessionsParams, @@ -557,6 +557,7 @@ export const RPC_PARAMS_BY_METHOD = { 'accounts.subscribe': null, 'accounts.unsubscribe': AccountsUnsubscribeParams, 'agent.launch': AgentLaunch, + 'agent.launchReplay': AgentLaunchReplay, 'agentHooks.prepareCodexForWslPane': PrepareCodexForWslPaneParams, 'agentSession.cancel': CancelParams, 'agentSession.close': OptionsParams,