From 6181599894fd95c4bd0065b6c672a9e70c8cf044 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 17 Jul 2026 17:24:53 -0400 Subject: [PATCH] fix(mobile): reconcile workspace creates across Relay migration Co-authored-by: Orca --- mobile/app/h/[hostId]/tasks.tsx | 5 +- .../src/tasks/blank-workspace-create.test.ts | 9 +- .../src/tasks/source-workspace-create.test.ts | 6 +- .../tasks/worktree-create-migration.test.ts | 125 ++++++++++++++ mobile/src/tasks/worktree-create-migration.ts | 51 ++++++ mobile/src/tasks/worktree-create-retry.ts | 5 +- .../terminal/terminal-webview-engine.test.ts | 52 ++++++ mobile/src/terminal/terminal-webview-html.ts | 7 +- ...erminal-webview-webgl-recovery-injected.ts | 4 +- .../worktree-create-idempotency.test.ts | 85 ++++++++++ .../methods/worktree-create-idempotency.ts | 56 +++++++ .../rpc/methods/worktree-schemas.test.ts | 12 ++ .../runtime/rpc/methods/worktree-schemas.ts | 1 + src/main/runtime/rpc/methods/worktree.test.ts | 21 +++ src/main/runtime/rpc/methods/worktree.ts | 155 ++++++++++-------- 15 files changed, 515 insertions(+), 79 deletions(-) create mode 100644 mobile/src/tasks/worktree-create-migration.test.ts create mode 100644 mobile/src/tasks/worktree-create-migration.ts create mode 100644 src/main/runtime/rpc/methods/worktree-create-idempotency.test.ts create mode 100644 src/main/runtime/rpc/methods/worktree-create-idempotency.ts diff --git a/mobile/app/h/[hostId]/tasks.tsx b/mobile/app/h/[hostId]/tasks.tsx index 191a47038f2..baf53865845 100644 --- a/mobile/app/h/[hostId]/tasks.tsx +++ b/mobile/app/h/[hostId]/tasks.tsx @@ -89,6 +89,7 @@ import { workspaceSshStatusLabel } from '../../../src/tasks/workspace-ssh-gate' import { WORKTREE_CREATE_TIMEOUT_MS } from '../../../src/tasks/workspace-create-timeout' +import { sendMigrationSafeWorktreeCreate } from '../../../src/tasks/worktree-create-migration' import { isSetupHookTrusted, normalizeSetupHookTrust, @@ -5610,7 +5611,9 @@ export default function MobileTasksScreen() { sparseCheckout: sparseCheckoutOverride }) } - const response = await client.sendRequest('worktree.create', params, { + const response = await sendMigrationSafeWorktreeCreate({ + client, + params, timeoutMs: WORKTREE_CREATE_TIMEOUT_MS }) if (!isSuccess(response)) { diff --git a/mobile/src/tasks/blank-workspace-create.test.ts b/mobile/src/tasks/blank-workspace-create.test.ts index 1d35d29f758..9d0cd048c7e 100644 --- a/mobile/src/tasks/blank-workspace-create.test.ts +++ b/mobile/src/tasks/blank-workspace-create.test.ts @@ -1,7 +1,11 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import { createBlankWorkspace } from './blank-workspace-create' +vi.mock('expo-crypto', () => ({ + randomUUID: () => '00000000-0000-4000-8000-000000000001' +})) + type Call = { method: string; params: unknown } function fakeClient(script: (method: string, call: number) => unknown, calls: Call[]): RpcClient { @@ -45,7 +49,8 @@ describe('createBlankWorkspace', () => { repo: 'id:repo-1', startupCommand: undefined, setupDecision: 'inherit', - name: 'octopus' + name: 'octopus', + clientMutationId: '00000000-0000-4000-8000-000000000001' } }) const params = calls[0]?.params as Record diff --git a/mobile/src/tasks/source-workspace-create.test.ts b/mobile/src/tasks/source-workspace-create.test.ts index d23e0c66317..caa4f160db3 100644 --- a/mobile/src/tasks/source-workspace-create.test.ts +++ b/mobile/src/tasks/source-workspace-create.test.ts @@ -1,8 +1,12 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import { createWorkspaceFromComposerSource } from './source-workspace-create' import type { MobileComposerCreateSelection } from './mobile-composer-source-types' +vi.mock('expo-crypto', () => ({ + randomUUID: () => '00000000-0000-4000-8000-000000000001' +})) + type Call = { method: string; params: Record } function fakeClient(handle: (method: string, call: number) => unknown, calls: Call[]): RpcClient { diff --git a/mobile/src/tasks/worktree-create-migration.test.ts b/mobile/src/tasks/worktree-create-migration.test.ts new file mode 100644 index 00000000000..d4dee6c7660 --- /dev/null +++ b/mobile/src/tasks/worktree-create-migration.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it, vi } from 'vitest' +import * as ExpoCrypto from 'expo-crypto' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import { createWorktreeWithNameRetry } from './worktree-create-retry' +import { + sendMigrationSafeWorktreeCreate, + WORKTREE_CREATE_MIGRATION_UNCERTAIN_ERROR +} from './worktree-create-migration' + +vi.mock('expo-crypto', () => ({ + randomUUID: vi.fn(() => '00000000-0000-4000-8000-000000000001') +})) + +const success = (worktreeId: string): RpcResponse => ({ + id: 'response', + ok: true, + result: { worktree: { id: worktreeId } }, + _meta: { runtimeId: 'runtime' } +}) + +const failure = (message: string): RpcResponse => ({ + id: 'response', + ok: false, + error: { code: 'invalid_argument', message }, + _meta: { runtimeId: 'runtime' } +}) + +function clientWithSend(sendRequest: RpcClient['sendRequest']): RpcClient { + return { sendRequest } as RpcClient +} + +describe('migration-safe worktree creation', () => { + it('replays once with the identical candidate and mutation ID after cutover', async () => { + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new LogicalClientCutoverError()) + .mockResolvedValueOnce(success('wt-1')) + + const response = await sendMigrationSafeWorktreeCreate({ + client: clientWithSend(sendRequest), + params: { repo: 'repo-1', name: 'migration-safe' }, + timeoutMs: 100, + clientMutationId: '1cd77fbf-d9d0-4723-9a65-a9064d93f582' + }) + + expect(response).toEqual(success('wt-1')) + expect(sendRequest).toHaveBeenCalledTimes(2) + expect(sendRequest.mock.calls[0]?.[1]).toEqual(sendRequest.mock.calls[1]?.[1]) + expect(sendRequest.mock.calls[1]?.[1]).toEqual({ + repo: 'repo-1', + name: 'migration-safe', + clientMutationId: '1cd77fbf-d9d0-4723-9a65-a9064d93f582' + }) + }) + + it('turns an old-desktop collision after cutover into actionable uncertainty', async () => { + const sendRequest = vi + .fn() + .mockRejectedValueOnce(new LogicalClientCutoverError()) + .mockResolvedValueOnce(failure('Branch "migration-safe" already exists.')) + + await expect( + sendMigrationSafeWorktreeCreate({ + client: clientWithSend(sendRequest), + params: { repo: 'repo-1', name: 'migration-safe' }, + timeoutMs: 100, + clientMutationId: '9ea97e27-4f66-4ca3-ae8a-e11c1f10990b' + }) + ).rejects.toThrow(WORKTREE_CREATE_MIGRATION_UNCERTAIN_ERROR) + expect(sendRequest).toHaveBeenCalledTimes(2) + }) + + it('does not suffix after a second interrupted migration', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue(new LogicalClientCutoverError()) + + await expect( + createWorktreeWithNameRetry({ + client: clientWithSend(sendRequest), + baseName: 'migration-safe', + buildParams: (name) => ({ repo: 'repo-1', name }) + }) + ).rejects.toThrow(WORKTREE_CREATE_MIGRATION_UNCERTAIN_ERROR) + expect(sendRequest).toHaveBeenCalledTimes(2) + expect(sendRequest.mock.calls.map((call) => (call[1] as { name: string }).name)).toEqual([ + 'migration-safe', + 'migration-safe' + ]) + }) + + it('still suffixes an ordinary non-migration collision with a fresh mutation ID', async () => { + const sendRequest = vi + .fn() + .mockResolvedValueOnce(failure('Branch "octopus" already exists.')) + .mockResolvedValueOnce(success('wt-2')) + const mutationIds = [ + '4203a4ad-f6df-4260-b40d-5e4d59c5e254', + '8d292184-d3c3-42a3-9fd5-294fe4e2d15f' + ] + vi.mocked(ExpoCrypto.randomUUID).mockImplementation(() => mutationIds.shift()!) + + await expect( + createWorktreeWithNameRetry({ + client: clientWithSend(sendRequest), + baseName: 'octopus', + buildParams: (name) => ({ repo: 'repo-1', name }) + }) + ).resolves.toEqual({ worktreeId: 'wt-2', name: 'octopus-2' }) + expect(sendRequest.mock.calls.map((call) => call[1])).toEqual([ + { + repo: 'repo-1', + name: 'octopus', + clientMutationId: '4203a4ad-f6df-4260-b40d-5e4d59c5e254' + }, + { + repo: 'repo-1', + name: 'octopus-2', + clientMutationId: '8d292184-d3c3-42a3-9fd5-294fe4e2d15f' + } + ]) + }) +}) diff --git a/mobile/src/tasks/worktree-create-migration.ts b/mobile/src/tasks/worktree-create-migration.ts new file mode 100644 index 00000000000..cd7b75dd1dc --- /dev/null +++ b/mobile/src/tasks/worktree-create-migration.ts @@ -0,0 +1,51 @@ +import * as ExpoCrypto from 'expo-crypto' +import { isRetryableWorktreeCreateConflict } from '../../../src/shared/new-workspace/worktree-create-retry-policy' +import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' + +export const WORKTREE_CREATE_MIGRATION_UNCERTAIN_ERROR = + 'Connection changed while this workspace was being created. It may already exist—refresh the workspace list before trying again.' + +class WorktreeCreateMigrationUncertainError extends Error { + constructor() { + super(WORKTREE_CREATE_MIGRATION_UNCERTAIN_ERROR) + } +} + +export async function sendMigrationSafeWorktreeCreate(args: { + client: RpcClient + params: Record + timeoutMs: number + clientMutationId?: string +}): Promise { + const requestParams = { + ...args.params, + clientMutationId: args.clientMutationId ?? ExpoCrypto.randomUUID() + } + const send = (): Promise => + args.client.sendRequest('worktree.create', requestParams, { timeoutMs: args.timeoutMs }) + + try { + return await send() + } catch (error) { + if (!(error instanceof LogicalClientCutoverError)) { + throw error + } + } + + try { + const response = await send() + // Why: old desktops ignore the operation ID. A same-name collision after + // cutover can mean the first request committed, so suffixing could duplicate it. + if (!response.ok && isRetryableWorktreeCreateConflict(response.error.message)) { + throw new WorktreeCreateMigrationUncertainError() + } + return response + } catch (error) { + if (error instanceof WorktreeCreateMigrationUncertainError) { + throw error + } + throw new WorktreeCreateMigrationUncertainError() + } +} diff --git a/mobile/src/tasks/worktree-create-retry.ts b/mobile/src/tasks/worktree-create-retry.ts index 4eb749f5c23..70eb3a22e37 100644 --- a/mobile/src/tasks/worktree-create-retry.ts +++ b/mobile/src/tasks/worktree-create-retry.ts @@ -6,6 +6,7 @@ import { isRetryableWorktreeCreateConflict } from '../../../src/shared/new-workspace/worktree-create-retry-policy' import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout' +import { sendMigrationSafeWorktreeCreate } from './worktree-create-migration' // Why: server-side collision checks (branch already exists locally / on a remote // / already has PR #N) can fire even after a pre-flight basename dedupe — @@ -30,7 +31,9 @@ export async function createWorktreeWithNameRetry(args: { let lastError: string | null = null for (let attempt = 0; attempt < maxAttempts; attempt += 1) { const candidateName = getClientWorktreeCreateCandidate(baseName, attempt) - const response = await client.sendRequest('worktree.create', buildParams(candidateName), { + const response = await sendMigrationSafeWorktreeCreate({ + client, + params: buildParams(candidateName), timeoutMs: WORKTREE_CREATE_TIMEOUT_MS }) if (response.ok) { diff --git a/mobile/src/terminal/terminal-webview-engine.test.ts b/mobile/src/terminal/terminal-webview-engine.test.ts index 503a25a1961..34938fa6a91 100644 --- a/mobile/src/terminal/terminal-webview-engine.test.ts +++ b/mobile/src/terminal/terminal-webview-engine.test.ts @@ -90,6 +90,25 @@ attachWebglAddon(true);`).runInNewContext(context) } } +function shouldUseWebglRenderer(navigator: { + userAgent: string + platform: string + maxTouchPoints: number +}): boolean { + const functionStart = terminalHtmlSource.indexOf(' function isIOSWebView()') + const functionEnd = terminalHtmlSource.indexOf( + ' // Why: iOS WebKit does not reliably resolve', + functionStart + ) + expect(functionStart).toBeGreaterThanOrEqual(0) + expect(functionEnd).toBeGreaterThan(functionStart) + const context = { navigator, useWebgl: false } + new Script( + `${terminalHtmlSource.slice(functionStart, functionEnd)}\nuseWebgl = shouldUseWebglRenderer();` + ).runInNewContext(context) + return context.useWebgl +} + describe('terminal WebView bundled engine', () => { it('keeps the assembled terminal HTML free of external engine URLs', () => { expect(XTERM_HTML).not.toMatch(/\bhttps?:\/\//) @@ -210,6 +229,39 @@ describe('terminal WebView bundled engine', () => { expect(timers).toHaveLength(0) }) + it.each([ + { + name: 'iPhone', + navigator: { + userAgent: 'Mozilla/5.0 (iPhone; CPU iPhone OS 19_0 like Mac OS X)', + platform: 'iPhone', + maxTouchPoints: 5 + }, + expected: false + }, + { + name: 'touch MacIntel iPadOS', + navigator: { + userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 15_0)', + platform: 'MacIntel', + maxTouchPoints: 5 + }, + expected: false + }, + { + name: 'Android', + navigator: { + userAgent: 'Mozilla/5.0 (Linux; Android 16)', + platform: 'Linux armv8l', + maxTouchPoints: 5 + }, + expected: true + } + ])('selects the expected renderer for $name', ({ navigator, expected }) => { + expect(shouldUseWebglRenderer(navigator)).toBe(expected) + expect(terminalHtmlSource).toContain('if (shouldUseWebglRenderer()) attachWebglAddon(true);') + }) + it('falls back to a refreshed DOM renderer when the delayed WebGL retry fails', () => { const { addons, term, timers } = createWebglRecoveryHarness(true) diff --git a/mobile/src/terminal/terminal-webview-html.ts b/mobile/src/terminal/terminal-webview-html.ts index e9d188b74ef..9d7a4ce45fc 100644 --- a/mobile/src/terminal/terminal-webview-html.ts +++ b/mobile/src/terminal/terminal-webview-html.ts @@ -262,6 +262,11 @@ window.onerror = function(msg) { if (/iP(ad|hone|od)/.test(navigator.userAgent)) return true; return navigator.platform === 'MacIntel' && navigator.maxTouchPoints > 1; } + function shouldUseWebglRenderer() { + // Why: iOS WebKit can leave transient blank glyph-atlas cells that repaint + // later even though xterm's buffer is correct; the DOM renderer avoids that atlas. + return !isIOSWebView(); + } // Why: iOS WebKit does not reliably resolve "SF Mono" by CSS family name and can // fall to a non-monospace face; lead with the ui-monospace generic to avoid that. var TERMINAL_FONT_FALLBACKS = '"Menlo", "Monaco", "Cascadia Mono", "Consolas", "DejaVu Sans Mono", "Liberation Mono", "Symbols Nerd Font Mono", monospace'; @@ -740,7 +745,7 @@ ${TERMINAL_WEBGL_RECOVERY_JS} var nextTerm = term; pendingTerm = nextTerm; term.open(surface); - attachWebglAddon(true); + if (shouldUseWebglRenderer()) attachWebglAddon(true); if (window.Unicode11Addon && window.Unicode11Addon.Unicode11Addon) try { term.loadAddon(new window.Unicode11Addon.Unicode11Addon()); term.unicode.activeVersion = '11'; } catch (e) {} if (typeof replayData === 'string' && replayData.length > 0) { enqueueWrite(replayData); diff --git a/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts b/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts index 8e4d27c2348..5768e2a96b3 100644 --- a/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts +++ b/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts @@ -25,7 +25,7 @@ export const TERMINAL_WEBGL_RECOVERY_JS = ` try { addon.dispose(); } catch (e) {} refreshTerminalSurface(); if (!allowRecovery) return; - // Why: one delayed retry handles transient iOS context loss without + // Why: one delayed retry handles transient context loss without // entering a GPU crash loop; a second loss stays on the DOM renderer. cancelWebglContextRecovery(); var recoveryTerm = term; @@ -53,7 +53,7 @@ export const TERMINAL_WEBGL_RECOVERY_JS = ` document.addEventListener('visibilitychange', function() { if (document.visibilityState !== 'visible') return; - // Why: iOS may restore the xterm model while discarding GPU pixels/theme + // Why: WebKit may restore the xterm model while discarding surface/theme // paint state, so visibility must rebuild the atlas and repaint every row. applyTerminalTheme(terminalThemeInput); try { if (webglAddon && webglAddon.clearTextureAtlas) webglAddon.clearTextureAtlas(); } catch (e) {} diff --git a/src/main/runtime/rpc/methods/worktree-create-idempotency.test.ts b/src/main/runtime/rpc/methods/worktree-create-idempotency.test.ts new file mode 100644 index 00000000000..904f87ca793 --- /dev/null +++ b/src/main/runtime/rpc/methods/worktree-create-idempotency.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from 'vitest' +import { runIdempotentWorktreeCreate } from './worktree-create-idempotency' + +describe('worktree.create idempotency', () => { + it('shares concurrent and settled-success replays for one runtime', async () => { + const runtime = {} + let resolveCreate!: (value: { worktree: { id: string } }) => void + const create = vi.fn( + () => + new Promise<{ worktree: { id: string } }>((resolve) => { + resolveCreate = resolve + }) + ) + const args = { + runtime, + clientMutationId: '12bf2c0f-49d9-46c9-9005-c231715c53ad', + params: { repo: 'repo-1', name: 'migration-safe' }, + create + } + + const first = runIdempotentWorktreeCreate(args) + const concurrent = runIdempotentWorktreeCreate(args) + await Promise.resolve() + expect(create).toHaveBeenCalledTimes(1) + + const created = { worktree: { id: 'wt-1' } } + resolveCreate(created) + await expect(Promise.all([first, concurrent])).resolves.toEqual([created, created]) + await expect(runIdempotentWorktreeCreate(args)).resolves.toBe(created) + expect(create).toHaveBeenCalledTimes(1) + }) + + it('rejects reuse of a mutation ID with different parameters', async () => { + const runtime = {} + const clientMutationId = 'fdb51f1b-39dc-4fb4-8e36-8017b3ec1639' + await runIdempotentWorktreeCreate({ + runtime, + clientMutationId, + params: { repo: 'repo-1', name: 'first' }, + create: async () => ({ worktree: { id: 'wt-1' } }) + }) + + await expect( + runIdempotentWorktreeCreate({ + runtime, + clientMutationId, + params: { repo: 'repo-1', name: 'second' }, + create: async () => ({ worktree: { id: 'wt-2' } }) + }) + ).rejects.toThrow('cannot be reused with different') + }) + + it('drops failures so the same operation can retry', async () => { + const runtime = {} + const create = vi + .fn<() => Promise<{ worktree: { id: string } }>>() + .mockRejectedValueOnce(new Error('temporary failure')) + .mockResolvedValueOnce({ worktree: { id: 'wt-retried' } }) + const args = { + runtime, + clientMutationId: '6cb64ce3-eaac-496c-a5e6-2c395c12252a', + params: { repo: 'repo-1', name: 'retryable' }, + create + } + + await expect(runIdempotentWorktreeCreate(args)).rejects.toThrow('temporary failure') + await expect(runIdempotentWorktreeCreate(args)).resolves.toEqual({ + worktree: { id: 'wt-retried' } + }) + expect(create).toHaveBeenCalledTimes(2) + }) + + it('does not share mutation IDs across runtimes', async () => { + const create = vi.fn(async () => ({ worktree: { id: 'wt-1' } })) + const request = { + clientMutationId: '0fab3dbf-16b8-4588-a1e5-81e931415e4c', + params: { repo: 'repo-1', name: 'isolated' }, + create + } + + await runIdempotentWorktreeCreate({ ...request, runtime: {} }) + await runIdempotentWorktreeCreate({ ...request, runtime: {} }) + expect(create).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/main/runtime/rpc/methods/worktree-create-idempotency.ts b/src/main/runtime/rpc/methods/worktree-create-idempotency.ts new file mode 100644 index 00000000000..ecf020f3eba --- /dev/null +++ b/src/main/runtime/rpc/methods/worktree-create-idempotency.ts @@ -0,0 +1,56 @@ +import { isDeepStrictEqual } from 'node:util' +import { InvalidArgumentError } from '../core' + +const WORKTREE_CREATE_RESULT_TTL_MS = 5 * 60_000 + +type WorktreeCreateEntry = { + params: unknown + result: Promise +} + +const worktreeCreatesByRuntime = new WeakMap>() + +export async function runIdempotentWorktreeCreate(args: { + runtime: object + clientMutationId: string | undefined + params: unknown + create: () => Promise +}): Promise { + if (!args.clientMutationId) { + return args.create() + } + const clientMutationId = args.clientMutationId + + let entries = worktreeCreatesByRuntime.get(args.runtime) + if (!entries) { + entries = new Map() + worktreeCreatesByRuntime.set(args.runtime, entries) + } + + const existing = entries.get(clientMutationId) + if (existing) { + if (!isDeepStrictEqual(existing.params, args.params)) { + throw new InvalidArgumentError( + 'clientMutationId cannot be reused with different worktree.create parameters' + ) + } + return existing.result as Promise + } + + // Why: publish the promise before creation begins so concurrent migration replays + // share one mutation; retain success briefly for a response lost during cutover. + const entry: WorktreeCreateEntry = { + params: args.params, + result: Promise.resolve().then(args.create) + } + entries.set(clientMutationId, entry) + const drop = (): void => { + if (entries?.get(clientMutationId) === entry) { + entries.delete(clientMutationId) + } + } + void entry.result.then(() => { + setTimeout(drop, WORKTREE_CREATE_RESULT_TTL_MS).unref?.() + }, drop) + return entry.result as Promise +} diff --git a/src/main/runtime/rpc/methods/worktree-schemas.test.ts b/src/main/runtime/rpc/methods/worktree-schemas.test.ts index 4f42e5fc4ce..038afc79a16 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.test.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.test.ts @@ -2,6 +2,18 @@ import { describe, expect, it } from 'vitest' import { WorktreeCreate } from './worktree-schemas' describe('worktree RPC schemas', () => { + it('accepts a UUID mutation ID and rejects malformed IDs', () => { + expect( + WorktreeCreate.safeParse({ + repo: 'repo-1', + clientMutationId: '7350e466-9f8e-427f-8d42-2c70c8d9f801' + }).success + ).toBe(true) + expect(WorktreeCreate.safeParse({ repo: 'repo-1', clientMutationId: 'reused' }).success).toBe( + false + ) + }) + it('rejects invalid startup agent values', () => { const parsed = WorktreeCreate.safeParse({ repo: 'repo-1', diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index 96f2813576e..f76b876cf32 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -65,6 +65,7 @@ export const WorktreeCreate = z .unknown() .transform((v) => (typeof v === 'string' ? v : '')) .pipe(z.string().min(1, 'Missing repo selector')), + clientMutationId: z.string().uuid().optional(), name: OptionalString, baseBranch: OptionalString, compareBaseRef: OptionalString, diff --git a/src/main/runtime/rpc/methods/worktree.test.ts b/src/main/runtime/rpc/methods/worktree.test.ts index 8bc68319d2d..69af9d4c799 100644 --- a/src/main/runtime/rpc/methods/worktree.test.ts +++ b/src/main/runtime/rpc/methods/worktree.test.ts @@ -154,6 +154,27 @@ describe('worktree RPC methods', () => { }) }) + it('returns the original workspace when a create mutation is replayed', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + showRepo: vi.fn().mockResolvedValue(repo), + createManagedWorktree: vi.fn().mockResolvedValue({ worktree: { id: 'wt-created' } }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS }) + const params = { + repo: 'repo-1', + name: 'migration-safe', + clientMutationId: 'c39750ef-6c87-4bb4-a152-e22ac7eb7a7d' + } + + const first = await dispatcher.dispatch(makeRequest('worktree.create', params)) + const replay = await dispatcher.dispatch(makeRequest('worktree.create', params)) + + expect(first).toMatchObject({ ok: true, result: { worktree: { id: 'wt-created' } } }) + expect(replay).toMatchObject({ ok: true, result: { worktree: { id: 'wt-created' } } }) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + }) + it('mints automation provenance from a valid dispatch request on worktree creation', async () => { const dispatchToken = createAutomationDispatchToken('automation-1', 'run-1') const runtime = { diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index 8132a96e3f8..d0b6a54e7d5 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -19,6 +19,7 @@ import { WorktreeSet, WorktreeSortOrder } from './worktree-schemas' +import { runIdempotentWorktreeCreate } from './worktree-create-idempotency' export const WORKTREE_METHODS: RpcMethod[] = [ defineMethod({ @@ -70,79 +71,91 @@ export const WORKTREE_METHODS: RpcMethod[] = [ defineMethod({ name: 'worktree.create', params: WorktreeCreate, - handler: async (params, { runtime }) => { - const repo = await runtime.showRepo(params.repo) - const automationProvenance = resolveAutomationWorkspaceProvenance({ - authority: runtime, - repoSelector: params.repo, - repo, - request: params.automationProvenanceRequest - }) - // Why: provenance tokens are reserved before creation so retries can recover, - // but failed create attempts must release the reservation for a safe retry. - try { - const result = await runtime.createManagedWorktree({ - repoSelector: params.repo, - name: params.name ?? '', - baseBranch: params.baseBranch, - compareBaseRef: params.compareBaseRef, - branchNameOverride: params.branchNameOverride, - linkedIssue: params.linkedIssue, - linkedPR: params.linkedPR, - linkedLinearIssue: params.linkedLinearIssue, - linkedLinearIssueWorkspaceId: params.linkedLinearIssueWorkspaceId, - linkedLinearIssueOrganizationUrlKey: params.linkedLinearIssueOrganizationUrlKey, - linkedGitLabMR: params.linkedGitLabMR, - linkedGitLabIssue: params.linkedGitLabIssue, - linkedBitbucketPR: params.linkedBitbucketPR, - linkedAzureDevOpsPR: params.linkedAzureDevOpsPR, - linkedGiteaPR: params.linkedGiteaPR, - comment: params.comment, - displayName: params.displayName, - telemetrySource: params.telemetrySource, - workspaceStatus: params.workspaceStatus, - manualOrder: params.manualOrder, - sparseCheckout: params.sparseCheckout, - pushTarget: params.pushTarget, - runHooks: params.runHooks === true, - activate: params.activate === true, - setupDecision: params.setupDecision, - createdWithAgent: params.createdWithAgent ?? params.startupAgent, - automationProvenance, - startup: params.startupCommand - ? { - command: params.startupCommand, - ...(params.startupEnv ? { env: params.startupEnv } : {}), - ...(params.startupLaunchConfig ? { launchConfig: params.startupLaunchConfig } : {}), - ...(params.startupCommandDelivery - ? { startupCommandDelivery: params.startupCommandDelivery } - : {}) + handler: async (params, { runtime }) => + runIdempotentWorktreeCreate({ + runtime, + clientMutationId: params.clientMutationId, + params, + create: async () => { + const repo = await runtime.showRepo(params.repo) + const automationProvenance = resolveAutomationWorkspaceProvenance({ + authority: runtime, + repoSelector: params.repo, + repo, + request: params.automationProvenanceRequest + }) + // Why: provenance tokens are reserved before creation so retries can recover, + // but failed create attempts must release the reservation for a safe retry. + try { + const result = await runtime.createManagedWorktree({ + repoSelector: params.repo, + name: params.name ?? '', + baseBranch: params.baseBranch, + compareBaseRef: params.compareBaseRef, + branchNameOverride: params.branchNameOverride, + linkedIssue: params.linkedIssue, + linkedPR: params.linkedPR, + linkedLinearIssue: params.linkedLinearIssue, + linkedLinearIssueWorkspaceId: params.linkedLinearIssueWorkspaceId, + linkedLinearIssueOrganizationUrlKey: params.linkedLinearIssueOrganizationUrlKey, + linkedGitLabMR: params.linkedGitLabMR, + linkedGitLabIssue: params.linkedGitLabIssue, + linkedBitbucketPR: params.linkedBitbucketPR, + linkedAzureDevOpsPR: params.linkedAzureDevOpsPR, + linkedGiteaPR: params.linkedGiteaPR, + comment: params.comment, + displayName: params.displayName, + telemetrySource: params.telemetrySource, + workspaceStatus: params.workspaceStatus, + manualOrder: params.manualOrder, + sparseCheckout: params.sparseCheckout, + pushTarget: params.pushTarget, + runHooks: params.runHooks === true, + activate: params.activate === true, + setupDecision: params.setupDecision, + createdWithAgent: params.createdWithAgent ?? params.startupAgent, + automationProvenance, + startup: params.startupCommand + ? { + command: params.startupCommand, + ...(params.startupEnv ? { env: params.startupEnv } : {}), + ...(params.startupLaunchConfig + ? { launchConfig: params.startupLaunchConfig } + : {}), + ...(params.startupCommandDelivery + ? { startupCommandDelivery: params.startupCommandDelivery } + : {}) + } + : undefined, + ...(params.startupAgent ? { startupAgent: params.startupAgent } : {}), + ...(params.startupPrompt !== undefined + ? { startupPrompt: params.startupPrompt } + : {}), + startupDraft: params.startupDraft, + lineage: { + parentWorkspace: params.parentWorkspace, + envParentWorkspace: params.envParentWorkspace, + parentWorktree: params.parentWorktree, + ...(params.cwdParentWorktree + ? { cwdParentWorktree: params.cwdParentWorktree } + : {}), + noParent: params.noParent === true, + callerTerminalHandle: params.callerTerminalHandle, + orchestrationContext: params.orchestrationContext } - : undefined, - ...(params.startupAgent ? { startupAgent: params.startupAgent } : {}), - ...(params.startupPrompt !== undefined ? { startupPrompt: params.startupPrompt } : {}), - startupDraft: params.startupDraft, - lineage: { - parentWorkspace: params.parentWorkspace, - envParentWorkspace: params.envParentWorkspace, - parentWorktree: params.parentWorktree, - ...(params.cwdParentWorktree ? { cwdParentWorktree: params.cwdParentWorktree } : {}), - noParent: params.noParent === true, - callerTerminalHandle: params.callerTerminalHandle, - orchestrationContext: params.orchestrationContext + }) + finishAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) + // Why: agent callers need a stable dispatch target without traversing + // terminal-list layout duplicates after creating the worktree. + return params.startupAgent && result.startupTerminal?.handle + ? { ...result, agentTerminalHandle: result.startupTerminal.handle } + : result + } catch (error) { + releaseAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) + throw error } - }) - finishAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) - // Why: agent callers need a stable dispatch target without traversing - // terminal-list layout duplicates after creating the worktree. - return params.startupAgent && result.startupTerminal?.handle - ? { ...result, agentTerminalHandle: result.startupTerminal.handle } - : result - } catch (error) { - releaseAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) - throw error - } - } + } + }) }), defineMethod({ name: 'worktree.prefetchCreateBase',