diff --git a/mobile/src/components/NewWorktreeModal.tsx b/mobile/src/components/NewWorktreeModal.tsx index 09a0914d4aa..ebbfc168e0d 100644 --- a/mobile/src/components/NewWorktreeModal.tsx +++ b/mobile/src/components/NewWorktreeModal.tsx @@ -51,6 +51,7 @@ import { import { createBlankWorkspace } from '../tasks/blank-workspace-create' import { createWorkspaceFromComposerSource } from '../tasks/source-workspace-create' import { MOBILE_TASKS_CAPABILITY } from '../tasks/mobile-tasks-capability' +import { MOBILE_WORKTREE_CREATE_IDEMPOTENCY_CAPABILITY } from '../tasks/worktree-create-capability' import { normalizeWorkspaceAgent } from '../tasks/workspace-agent-selection' import { filterAvailableTaskProviders, @@ -211,6 +212,7 @@ function NewWorktreeModalContent({ const [note, setNote] = useState('') const [availableProviders, setAvailableProviders] = useState([]) const [tasksSupported, setTasksSupported] = useState(false) + const [idempotentWorktreeCreateSupported, setIdempotentWorktreeCreateSupported] = useState(false) const [showAdvanced, setShowAdvanced] = useState(false) const [setupHookDetails, setSetupHookDetails] = useState(null) const [trustedOrcaHooks, setTrustedOrcaHooks] = useState({}) @@ -417,6 +419,9 @@ function NewWorktreeModalContent({ const capabilities = (statusResult?.result as { capabilities?: string[] } | undefined)?.capabilities ?? [] setTasksSupported(capabilities.includes(MOBILE_TASKS_CAPABILITY)) + setIdempotentWorktreeCreateSupported( + capabilities.includes(MOBILE_WORKTREE_CREATE_IDEMPOTENCY_CAPABILITY) + ) const glabInstalled = (okResult(preflightRes)?.result as { glab?: { installed?: boolean } } | undefined)?.glab ?.installed === true @@ -698,7 +703,8 @@ function NewWorktreeModalContent({ }, workspaceName: trimmedName || undefined, note: trimmedNote, - nameIsAutoManaged: composer.isNameAutoManaged + nameIsAutoManaged: composer.isNameAutoManaged, + supportsIdempotentCutoverRetry: idempotentWorktreeCreateSupported }) : await createBlankWorkspace({ client, @@ -707,7 +713,8 @@ function NewWorktreeModalContent({ startupCommand: command, createdWithAgentId, comment: trimmedNote, - setupDecision + setupDecision, + supportsIdempotentCutoverRetry: idempotentWorktreeCreateSupported }) if ('error' in result) { setError(result.error) diff --git a/mobile/src/tasks/blank-workspace-create.test.ts b/mobile/src/tasks/blank-workspace-create.test.ts index 1fe37dba473..ba0fc4f1066 100644 --- a/mobile/src/tasks/blank-workspace-create.test.ts +++ b/mobile/src/tasks/blank-workspace-create.test.ts @@ -34,7 +34,8 @@ describe('createBlankWorkspace', () => { startupCommand: undefined, createdWithAgentId: undefined, comment: undefined, - setupDecision: 'inherit' + setupDecision: 'inherit', + supportsIdempotentCutoverRetry: true }) expect(result).toEqual({ worktreeId: 'wt-1', name: 'octopus' }) @@ -67,7 +68,8 @@ describe('createBlankWorkspace', () => { startupCommand: 'claude', createdWithAgentId: 'claude', comment: 'spike', - setupDecision: 'run' + setupDecision: 'run', + supportsIdempotentCutoverRetry: true }) expect(calls[0]?.params).toMatchObject({ @@ -96,7 +98,8 @@ describe('createBlankWorkspace', () => { startupCommand: undefined, createdWithAgentId: undefined, comment: undefined, - setupDecision: 'inherit' + setupDecision: 'inherit', + supportsIdempotentCutoverRetry: true }) expect(result).toEqual({ worktreeId: 'wt-3', name: 'octopus-2' }) @@ -121,7 +124,8 @@ describe('createBlankWorkspace', () => { startupCommand: undefined, createdWithAgentId: undefined, comment: undefined, - setupDecision: 'inherit' + setupDecision: 'inherit', + supportsIdempotentCutoverRetry: true }) expect(result).toEqual({ worktreeId: 'wt-4', name: 'octopus-2' }) @@ -139,7 +143,8 @@ describe('createBlankWorkspace', () => { startupCommand: undefined, createdWithAgentId: undefined, comment: undefined, - setupDecision: 'skip' + setupDecision: 'skip', + supportsIdempotentCutoverRetry: true }) expect(result).toEqual({ error: 'SSH connection is not available' }) diff --git a/mobile/src/tasks/blank-workspace-create.ts b/mobile/src/tasks/blank-workspace-create.ts index 014af558198..be21be54378 100644 --- a/mobile/src/tasks/blank-workspace-create.ts +++ b/mobile/src/tasks/blank-workspace-create.ts @@ -14,10 +14,12 @@ export async function createBlankWorkspace(args: { createdWithAgentId: TuiAgent | undefined comment: string | undefined setupDecision: WorkspaceCreateSetupDecision + supportsIdempotentCutoverRetry: boolean }): Promise { return createWorktreeWithNameRetry({ client: args.client, baseName: args.baseName, + supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, buildParams: (name) => { const params: Record = { repo: `id:${args.repoId}`, diff --git a/mobile/src/tasks/source-workspace-create.test.ts b/mobile/src/tasks/source-workspace-create.test.ts index d23e0c66317..592375a7735 100644 --- a/mobile/src/tasks/source-workspace-create.test.ts +++ b/mobile/src/tasks/source-workspace-create.test.ts @@ -30,7 +30,8 @@ const baseArgs = { setupDecision: 'inherit' as const, agent, workspaceName: undefined, - note: undefined + note: undefined, + supportsIdempotentCutoverRetry: true } describe('createWorkspaceFromComposerSource', () => { diff --git a/mobile/src/tasks/source-workspace-create.ts b/mobile/src/tasks/source-workspace-create.ts index d16cc174142..2c6055dd3b2 100644 --- a/mobile/src/tasks/source-workspace-create.ts +++ b/mobile/src/tasks/source-workspace-create.ts @@ -30,6 +30,7 @@ export type CreateWorkspaceFromComposerArgs = { workspaceName: string | undefined note: string | undefined nameIsAutoManaged?: boolean + supportsIdempotentCutoverRetry: boolean } export async function createWorkspaceFromComposerSource( @@ -92,6 +93,7 @@ async function createWorkItemWorkspace(args: { workspaceName: string | undefined note: string | undefined nameIsAutoManaged?: boolean + supportsIdempotentCutoverRetry: boolean }): Promise { const { client, selection, targetRepoId, setupDecision, agent, workspaceName, note } = args const item = selection.item @@ -136,6 +138,7 @@ async function createWorkItemWorkspace(args: { return createWorktreeWithNameRetry({ client, baseName, + supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, buildParams: (name) => ({ ...params, name }) }) } @@ -148,6 +151,7 @@ async function createBranchWorkspace(args: { agent: WorkspaceCreateAgentBundle workspaceName: string | undefined note: string | undefined + supportsIdempotentCutoverRetry: boolean }): Promise { const { client, selection, targetRepoId, setupDecision, agent, workspaceName, note } = args const createdWithAgentId = agent.choice === 'blank' ? undefined : agent.choice @@ -173,6 +177,7 @@ async function createBranchWorkspace(args: { return createWorktreeWithNameRetry({ client, baseName, + supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, maxAttempts: 1, buildParams: (name) => applyCommon({ @@ -195,6 +200,7 @@ async function createBranchWorkspace(args: { return createWorktreeWithNameRetry({ client, baseName, + supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, buildParams: (candidate) => { const params: Record = { repo: `id:${targetRepoId}`, @@ -219,6 +225,7 @@ async function createNewBranchWorkspace(args: { agent: WorkspaceCreateAgentBundle workspaceName: string | undefined note: string | undefined + supportsIdempotentCutoverRetry: boolean }): Promise { const { client, selection, targetRepoId, setupDecision, agent, note } = args const createdWithAgentId = agent.choice === 'blank' ? undefined : agent.choice @@ -230,6 +237,7 @@ async function createNewBranchWorkspace(args: { return createWorktreeWithNameRetry({ client, baseName: selection.branchName, + supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, buildParams: (candidate) => { const params: Record = { repo: `id:${targetRepoId}`, diff --git a/mobile/src/tasks/worktree-create-capability.ts b/mobile/src/tasks/worktree-create-capability.ts new file mode 100644 index 00000000000..36321885c28 --- /dev/null +++ b/mobile/src/tasks/worktree-create-capability.ts @@ -0,0 +1,4 @@ +// Why: older hosts strip worktree.create's clientMutationId, so mobile must not +// replay an ambiguous create unless the host advertises idempotency support. +// Mirrors WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY in the shared protocol. +export const MOBILE_WORKTREE_CREATE_IDEMPOTENCY_CAPABILITY = 'worktree.create-idempotency.v1' diff --git a/mobile/src/tasks/worktree-create-retry.test.ts b/mobile/src/tasks/worktree-create-retry.test.ts index 6c5328cefda..5a7d39ffb1e 100644 --- a/mobile/src/tasks/worktree-create-retry.test.ts +++ b/mobile/src/tasks/worktree-create-retry.test.ts @@ -39,8 +39,6 @@ function scriptedClient( } as unknown as RpcClient } -const noSleep = async (): Promise => {} - describe('createWorktreeWithNameRetry', () => { it('stamps a clientMutationId on the create request', async () => { const attempts: Attempt[] = [] @@ -49,8 +47,8 @@ describe('createWorktreeWithNameRetry', () => { client, baseName: 'otter', buildParams: (name) => ({ repo: 'id:r', name }), - mintMutationId: () => 'key-1', - sleep: noSleep + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-1' }) expect(result).toEqual({ worktreeId: 'wt-1', name: 'otter' }) expect(attempts).toHaveLength(1) @@ -67,8 +65,8 @@ describe('createWorktreeWithNameRetry', () => { client, baseName: 'seal', buildParams: (name) => ({ repo: 'id:r', name }), - mintMutationId: () => 'key-mig', - sleep: noSleep + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-mig' }) expect(result).toEqual({ worktreeId: 'wt-2', name: 'seal' }) expect(attempts).toHaveLength(2) @@ -87,8 +85,8 @@ describe('createWorktreeWithNameRetry', () => { client, baseName: 'crab', buildParams: (name) => ({ repo: 'id:r', name }), - mintMutationId: () => 'key-x', - sleep: noSleep + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-x' }) ).rejects.toBeInstanceOf(LogicalClientCutoverError) // Initial attempt + 5 retries. @@ -103,8 +101,8 @@ describe('createWorktreeWithNameRetry', () => { client, baseName: 'eel', buildParams: (name) => ({ repo: 'id:r', name }), - mintMutationId: () => 'key-t', - sleep: noSleep + supportsIdempotentCutoverRetry: true, + mintMutationId: () => 'key-t' }) ).rejects.toThrow('Request timed out') expect(attempts).toHaveLength(1) @@ -121,8 +119,8 @@ describe('createWorktreeWithNameRetry', () => { client, baseName: 'topic', buildParams: (name) => ({ repo: 'id:r', name }), - mintMutationId: () => `key-${(n += 1)}`, - sleep: noSleep + supportsIdempotentCutoverRetry: true, + mintMutationId: () => `key-${(n += 1)}` }) expect(result).toEqual({ worktreeId: 'wt-3', name: 'topic-2' }) expect(attempts).toHaveLength(2) @@ -131,4 +129,20 @@ describe('createWorktreeWithNameRetry', () => { expect(attempts[1]!.params.clientMutationId).toBe('key-2') expect(attempts[1]!.params.name).toBe('topic-2') }) + + it('does not replay an ambiguous cutover when the host lacks idempotency support', async () => { + const attempts: Attempt[] = [] + const client = scriptedClient([{ throws: new LogicalClientCutoverError() }], attempts) + await expect( + createWorktreeWithNameRetry({ + client, + baseName: 'ray', + buildParams: (name) => ({ repo: 'id:r', name }), + supportsIdempotentCutoverRetry: false, + mintMutationId: () => 'must-not-be-used' + }) + ).rejects.toBeInstanceOf(LogicalClientCutoverError) + expect(attempts).toHaveLength(1) + expect(attempts[0]!.params.clientMutationId).toBeUndefined() + }) }) diff --git a/mobile/src/tasks/worktree-create-retry.ts b/mobile/src/tasks/worktree-create-retry.ts index 0d605a9a8f3..d487fc09490 100644 --- a/mobile/src/tasks/worktree-create-retry.ts +++ b/mobile/src/tasks/worktree-create-retry.ts @@ -22,16 +22,15 @@ export type WorktreeCreateResult = { worktreeId: string; name: string } | { erro // instead of surfacing "RPC interrupted by connection migration" with the // worktree silently created. const WORKTREE_CREATE_CUTOVER_MAX_RETRIES = 5 -const WORKTREE_CREATE_CUTOVER_RETRY_DELAY_MS = 300 export type CreateWorktreeWithNameRetryArgs = { client: RpcClient baseName: string buildParams: (name: string) => Record + supportsIdempotentCutoverRetry: boolean maxAttempts?: number // Injected in tests; production mints a fresh idempotency key per candidate. mintMutationId?: () => string - sleep?: (ms: number) => Promise } // Creates a worktree, retrying with a numeric suffix on a name-collision error. @@ -42,18 +41,24 @@ export type CreateWorktreeWithNameRetryArgs = { export async function createWorktreeWithNameRetry( args: CreateWorktreeWithNameRetryArgs ): Promise { - const { client, baseName, buildParams } = args + const { client, baseName, buildParams, supportsIdempotentCutoverRetry } = args const maxAttempts = args.maxAttempts ?? CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS const mintMutationId = args.mintMutationId ?? defaultWorktreeCreateMutationId - const sleep = args.sleep ?? defaultSleep let lastError: string | null = null for (let attempt = 0; attempt < maxAttempts; attempt += 1) { const candidateName = getClientWorktreeCreateCandidate(baseName, attempt) - // Why: one idempotency key per candidate name. A cutover retry reuses the - // SAME key so the host dedupes (no duplicate worktree), while a name-collision - // bump is a genuinely new create that must mint a fresh key. - const params = { ...buildParams(candidateName), clientMutationId: mintMutationId() } - const response = await sendWorktreeCreateResilient(client, params, sleep) + const candidateParams = buildParams(candidateName) + // Why: older hosts strip unknown fields, so only stamp and replay when the + // host advertises idempotency. One key per candidate makes cutover retries + // safe while a name-collision bump remains a genuinely new create. + const params = supportsIdempotentCutoverRetry + ? { ...candidateParams, clientMutationId: mintMutationId() } + : candidateParams + const response = await sendWorktreeCreateResilient( + client, + params, + supportsIdempotentCutoverRetry + ) if (response.ok) { const result = (response as RpcSuccess).result as { worktree: { id: string } } return { worktreeId: result.worktree.id, name: candidateName } @@ -71,7 +76,7 @@ export async function createWorktreeWithNameRetry( async function sendWorktreeCreateResilient( client: RpcClient, params: Record, - sleep: (ms: number) => Promise + supportsIdempotentCutoverRetry: boolean ): Promise { for (let migrationRetry = 0; ; migrationRetry += 1) { try { @@ -80,13 +85,14 @@ async function sendWorktreeCreateResilient( }) } catch (error) { if ( + !supportsIdempotentCutoverRetry || !isLogicalClientCutoverError(error) || migrationRetry >= WORKTREE_CREATE_CUTOVER_MAX_RETRIES ) { throw error } - // Give migrateTo a beat to settle on the authenticated replacement session. - await sleep(WORKTREE_CREATE_CUTOVER_RETRY_DELAY_MS) + // Why: LogicalClientCutoverError is raised only after migrateTo installs an + // authenticated replacement, so retry immediately instead of adding UI lag. } } } @@ -102,7 +108,3 @@ function defaultWorktreeCreateMutationId(): string { const randomPart = Math.random().toString(36).slice(2, 10) return `worktree-create:${Date.now().toString(36)}:${randomPart}` } - -function defaultSleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)) -} diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 4ecc79b1bc2..66ce78ca40b 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1416,6 +1416,25 @@ describe('OrcaRuntimeService.dedupeWorktreeCreate', () => { expect(retried).toEqual(first) }) + it('expires settled successes after the reconnect window', async () => { + vi.useFakeTimers() + try { + const runtime = new OrcaRuntimeService(store) + let calls = 0 + const factory = (): Promise<{ n: number }> => Promise.resolve({ n: (calls += 1) }) + await runtime.dedupeWorktreeCreate('id:r', 'key-1', factory) + await vi.advanceTimersByTimeAsync(59_999) + await runtime.dedupeWorktreeCreate('id:r', 'key-1', factory) + expect(calls).toBe(1) + + await vi.advanceTimersByTimeAsync(1) + await runtime.dedupeWorktreeCreate('id:r', 'key-1', factory) + expect(calls).toBe(2) + } finally { + vi.useRealTimers() + } + }) + it('drops a failed create so a genuine retry starts fresh', async () => { const runtime = new OrcaRuntimeService(store) let calls = 0 @@ -1555,6 +1574,7 @@ describe('OrcaRuntimeService', () => { expect(status.capabilities).toContain('terminal.binary-stream.v1') expect(status.capabilities).toContain('workspace-ports.v1') expect(status.capabilities).toContain('mobile.tasks.v1') + expect(status.capabilities).toContain('worktree.create-idempotency.v1') expect(status.capabilities).toContain('project-host-setup.v1') expect(status.capabilities).toContain('linear.issue-attribute-filter.v1') expect(status.capabilities).not.toContain('browser.screencast.v1') diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index f6242c93e99..a1fc0ae817f 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -44,6 +44,10 @@ export const BROWSER_CERTIFICATE_TRUST_RUNTIME_CAPABILITY = 'browser.certificate // floor-taking input. Mobile must not forward replies unless advertised. export const TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY = 'terminal.query-reply-input.v1' as const +// Why: older hosts strip worktree.create's clientMutationId, so mobile must only +// replay ambiguous cutovers when the host advertises idempotent create support. +export const WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY = + 'worktree.create-idempotency.v1' as const export const RUNTIME_CAPABILITIES = [ 'runtime.status.compat.v1', @@ -60,7 +64,8 @@ export const RUNTIME_CAPABILITIES = [ FOLDER_WORKSPACE_PATH_STATUS_RUNTIME_CAPABILITY, LINEAR_ISSUE_ATTRIBUTE_FILTER_RUNTIME_CAPABILITY, AI_VAULT_RUNTIME_CAPABILITY, - TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY + TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY, + WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY ] as const export type RuntimeCapability = (typeof RUNTIME_CAPABILITIES)[number] | (string & {})