From 045eb6c5b449a385f8f3b75aba256a185c3bbebf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:26:37 -0700 Subject: [PATCH] refactor: separate retained workspace creation from composer activation --- .../src/lib/create-requested-worktree.ts | 68 ++++++ .../src/lib/ensure-hooks-confirmed.ts | 163 +++------------ src/renderer/src/lib/hook-trust-inspection.ts | 193 ++++++++++++++++++ .../src/lib/inspect-hooks-trust.test.ts | 79 +++++++ .../lib/retained-worktree-creation.test.ts | 127 ++++++++++++ .../src/lib/retained-worktree-creation.ts | 87 ++++++++ .../src/lib/worktree-creation-flow-execute.ts | 63 +----- .../src/lib/worktree-creation-flow.test.ts | 12 +- .../src/lib/worktree-creation-flow.ts | 8 +- 9 files changed, 596 insertions(+), 204 deletions(-) create mode 100644 src/renderer/src/lib/create-requested-worktree.ts create mode 100644 src/renderer/src/lib/hook-trust-inspection.ts create mode 100644 src/renderer/src/lib/inspect-hooks-trust.test.ts create mode 100644 src/renderer/src/lib/retained-worktree-creation.test.ts create mode 100644 src/renderer/src/lib/retained-worktree-creation.ts diff --git a/src/renderer/src/lib/create-requested-worktree.ts b/src/renderer/src/lib/create-requested-worktree.ts new file mode 100644 index 00000000000..4b14d0917df --- /dev/null +++ b/src/renderer/src/lib/create-requested-worktree.ts @@ -0,0 +1,68 @@ +import { useAppStore } from '@/store' +import { getProvisionedRootCreateOptions } from '@/lib/provisioned-root-create-options' +import { resolveBackendDraftStartup } from '@/lib/worktree-draft-startup-view-mode' +import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation' +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' + +/** Registers a durable workspace without revealing it or running renderer launch actions. */ +export function createRequestedWorktree( + creationId: string, + preparedRequest: WorktreeCreationRequest +): Promise { + const provisionedRoot = getProvisionedRootCreateOptions(preparedRequest) + const structuredLaunch = preparedRequest.agentLaunchRoute === 'structured-native-chat' + const backendStartup = + provisionedRoot || structuredLaunch ? undefined : resolveBackendDraftStartup(preparedRequest) + return useAppStore + .getState() + .createWorktree( + preparedRequest.repoId, + preparedRequest.name, + preparedRequest.baseBranch, + preparedRequest.setupDecision, + preparedRequest.sparseCheckout, + preparedRequest.telemetrySource, + preparedRequest.displayName, + preparedRequest.linkedIssue, + preparedRequest.linkedPR, + preparedRequest.pushTarget, + preparedRequest.agent ?? undefined, + preparedRequest.linkedLinearIssue, + preparedRequest.branchNameOverride, + preparedRequest.workspaceStatus, + preparedRequest.linkedGitLabMR, + preparedRequest.linkedGitLabIssue, + backendStartup, + structuredLaunch ? false : preparedRequest.pendingFirstAgentMessageRename, + creationId, + preparedRequest.linkedLinearIssueWorkspaceId, + preparedRequest.linkedLinearIssueOrganizationUrlKey, + preparedRequest.linkedBitbucketPR, + preparedRequest.linkedAzureDevOpsPR, + preparedRequest.linkedGiteaPR, + preparedRequest.compareBaseRef, + { + ...(preparedRequest.nameWasGenerated ? { nameWasGenerated: true } : {}), + ...(preparedRequest.displayNameKind + ? { displayNameKind: preparedRequest.displayNameKind } + : {}), + ...(preparedRequest.linkedWorkItem !== undefined + ? { linkedWorkItem: preparedRequest.linkedWorkItem } + : {}), + ...(preparedRequest.linkedTaskSourceContext !== undefined + ? { linkedTaskSourceContext: preparedRequest.linkedTaskSourceContext } + : {}), + // Why: the remote host must own task-draft startup so its initial terminal is the agent, not an idle fallback shell. + ...(!structuredLaunch && + !backendStartup && + preparedRequest.agent && + preparedRequest.launchDraftPrompt + ? { startupDraft: preparedRequest.launchDraftPrompt } + : {}), + ...(provisionedRoot ? { provisionedRoot } : {}), + ...(preparedRequest.parentWorktreeId + ? { parentWorktreeId: preparedRequest.parentWorktreeId } + : {}) + } + ) +} diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts index 0a24c776c62..d62cba7670e 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -1,19 +1,18 @@ +import { + canUseRepoWideTrust, + findHookRepo, + settingsForHookRepoOwner, + readHookTrustRequirement +} from './hook-trust-inspection' +export { inspectHooksTrust } from './hook-trust-inspection' import type { AppState } from '@/store/types' -import type { OrcaHooks } from '../../../shared/orca-yaml-hook-types' -import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-source-policy' import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust' import { - checkRuntimeHooks, readRuntimeIssueCommand, type IssueCommandReadResult } from '@/runtime/runtime-hooks-client' -import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner' import { MODAL_DISMISSED_KEY } from '@/store/slices/modal-slot-dismissal' -import { - getRepoExecutionHostId, - parseExecutionHostId, - type ExecutionHostId -} from '../../../shared/execution-host' +import type { ExecutionHostId } from '../../../shared/execution-host' export type HookScriptKind = OrcaHookScriptKind @@ -32,74 +31,6 @@ export function __resetTrustPromptChainForTests(): void { trustPromptChain = Promise.resolve() } -function getSetupTrustContent(yamlHooks: OrcaHooks | null): string { - const defaultTabCommands = (yamlHooks?.defaultTabs ?? []) - .map((tab, index) => { - const command = tab.command?.trim() - if (!command) { - return null - } - const label = tab.title ? ` ${tab.title}` : '' - return `# defaultTabs[${index + 1}]${label}\n${command}` - }) - .filter((entry): entry is string => entry !== null) - return [yamlHooks?.scripts?.setup?.trim(), ...defaultTabCommands].filter(Boolean).join('\n\n') -} - -function getVmRecipeTrustContent(yamlHooks: OrcaHooks | null): string { - return (yamlHooks?.environmentRecipes ?? []) - .map((recipe) => - [ - `# environmentRecipes.${recipe.id}`, - `name: ${recipe.name}`, - recipe.description ? `description: ${recipe.description}` : null, - `create: ${recipe.create}`, - recipe.suspend ? `suspend: ${recipe.suspend}` : null, - recipe.resume ? `resume: ${recipe.resume}` : null, - recipe.destroyDisabled - ? 'destroy: none' - : recipe.destroy - ? `destroy: ${recipe.destroy}` - : null - ] - .filter((entry): entry is string => entry !== null) - .join('\n') - ) - .join('\n\n') -} - -function findHookRepo(state: AppState, repoId: string, hostId?: ExecutionHostId) { - return hostId - ? state.repos.find((repo) => repo.id === repoId && getRepoExecutionHostId(repo) === hostId) - : state.repos.find((repo) => repo.id === repoId) -} - -function settingsForHookRepoOwner( - state: AppState, - repoId: string, - hostId?: ExecutionHostId, - runtimeOwnerEnvironmentId?: string | null -): AppState['settings'] { - const parsedHost = hostId ? parseExecutionHostId(hostId) : null - const runtimeEnvironmentId = - runtimeOwnerEnvironmentId?.trim() || - (hostId - ? parsedHost?.kind === 'runtime' - ? parsedHost.environmentId - : null - : getRuntimeEnvironmentIdForRepo(state, repoId)) - // Why: hook inspection must follow the repo owner. SSH/local repos execute - // through desktop IPC, while runtime repos may differ from the focused host. - return state.settings - ? { ...state.settings, activeRuntimeEnvironmentId: runtimeEnvironmentId } - : ({ activeRuntimeEnvironmentId: runtimeEnvironmentId } as AppState['settings']) -} - -function canUseRepoWideTrust(state: AppState, repoId: string): boolean { - const hasDuplicateRepoId = state.repos.filter((repo) => repo.id === repoId).length > 1 - return Boolean(state.trustedOrcaHooks[repoId]?.all) && !hasDuplicateRepoId -} - async function confirmScriptContent( state: AppState, repoId: string, @@ -233,7 +164,7 @@ export async function readAndConfirmRuntimeIssueCommand( return confirmRuntimeIssueCommandRead(state, repoId, hostId, result, isCancelled) } -export async function ensureHooksConfirmed( +export function ensureHooksConfirmed( state: AppState, repoId: string, scriptKind: HookScriptKind, @@ -242,67 +173,23 @@ export async function ensureHooksConfirmed( isCancelled: () => boolean = NEVER_CANCEL_TRUST_CHECK ): Promise<'run' | 'skip'> { return enqueueTrustPrompt(async () => { - if (isCancelled()) { - return 'skip' - } - if (canUseRepoWideTrust(state, repoId)) { - return 'run' - } - - let scriptContent = '' - try { - if (scriptKind === 'issueCommand') { - // Local overrides are user-owned; only shared orca.yaml commands need repo trust. - // Why: hostId disambiguates duplicate repo ids on the local IPC path, - // matching the checkRuntimeHooks call below. - const result = await readRuntimeIssueCommand( - settingsForHookRepoOwner(state, repoId, hostId, runtimeOwnerEnvironmentId), + const requirement = await readHookTrustRequirement( + state, + repoId, + scriptKind, + hostId, + runtimeOwnerEnvironmentId, + isCancelled + ) + return typeof requirement === 'string' + ? requirement + : confirmScriptContent( + state, repoId, - hostId + scriptKind, + requirement.scriptContent, + hostId, + isCancelled ) - if (result.source === 'local') { - return 'run' - } - if (result.status === 'error') { - return 'skip' - } - if (result.source !== 'shared') { - return 'run' - } - scriptContent = (result.sharedContent ?? '').trim() - } else { - const repo = findHookRepo(state, repoId, hostId) - const localScript = repo?.hookSettings?.scripts?.[scriptKind]?.trim() - const sourcePolicy = resolveHookCommandSourcePolicy( - repo?.hookSettings?.commandSourcePolicy, - { - hasLocalScript: Boolean(localScript) - } - ) - if (sourcePolicy === 'local-only') { - return 'run' - } - const result = await checkRuntimeHooks( - settingsForHookRepoOwner(state, repoId, hostId, runtimeOwnerEnvironmentId), - repoId, - hostId - ) - if (result.status === 'error') { - return 'skip' - } - const yamlHooks = (result.hooks as OrcaHooks | null) ?? null - scriptContent = - scriptKind === 'setup' - ? getSetupTrustContent(yamlHooks) - : scriptKind === 'vmRecipe' - ? getVmRecipeTrustContent(yamlHooks) - : (yamlHooks?.scripts?.[scriptKind] ?? '').trim() - } - } catch { - // Fail closed: if we cannot inspect the script, we cannot trust it. - return 'skip' - } - - return confirmScriptContent(state, repoId, scriptKind, scriptContent, hostId, isCancelled) }) } diff --git a/src/renderer/src/lib/hook-trust-inspection.ts b/src/renderer/src/lib/hook-trust-inspection.ts new file mode 100644 index 00000000000..90282f0f99e --- /dev/null +++ b/src/renderer/src/lib/hook-trust-inspection.ts @@ -0,0 +1,193 @@ +import type { AppState } from '@/store/types' +import type { OrcaHooks } from '../../../shared/orca-yaml-hook-types' +import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-source-policy' +import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust' +import { checkRuntimeHooks, readRuntimeIssueCommand } from '@/runtime/runtime-hooks-client' +import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner' +import { + getRepoExecutionHostId, + parseExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' + +type HookScriptKind = OrcaHookScriptKind +const NEVER_CANCEL_TRUST_CHECK = (): boolean => false + +function getSetupTrustContent(yamlHooks: OrcaHooks | null): string { + const defaultTabCommands = (yamlHooks?.defaultTabs ?? []) + .map((tab, index) => { + const command = tab.command?.trim() + if (!command) { + return null + } + const label = tab.title ? ` ${tab.title}` : '' + return `# defaultTabs[${index + 1}]${label}\n${command}` + }) + .filter((entry): entry is string => entry !== null) + return [yamlHooks?.scripts?.setup?.trim(), ...defaultTabCommands].filter(Boolean).join('\n\n') +} + +function getVmRecipeTrustContent(yamlHooks: OrcaHooks | null): string { + return (yamlHooks?.environmentRecipes ?? []) + .map((recipe) => + [ + `# environmentRecipes.${recipe.id}`, + `name: ${recipe.name}`, + recipe.description ? `description: ${recipe.description}` : null, + `create: ${recipe.create}`, + recipe.suspend ? `suspend: ${recipe.suspend}` : null, + recipe.resume ? `resume: ${recipe.resume}` : null, + recipe.destroyDisabled + ? 'destroy: none' + : recipe.destroy + ? `destroy: ${recipe.destroy}` + : null + ] + .filter((entry): entry is string => entry !== null) + .join('\n') + ) + .join('\n\n') +} + +export function findHookRepo(state: AppState, repoId: string, hostId?: ExecutionHostId) { + return hostId + ? state.repos.find((repo) => repo.id === repoId && getRepoExecutionHostId(repo) === hostId) + : state.repos.find((repo) => repo.id === repoId) +} + +export function settingsForHookRepoOwner( + state: AppState, + repoId: string, + hostId?: ExecutionHostId, + runtimeOwnerEnvironmentId?: string | null +): AppState['settings'] { + const parsedHost = hostId ? parseExecutionHostId(hostId) : null + const runtimeEnvironmentId = + runtimeOwnerEnvironmentId?.trim() || + (hostId + ? parsedHost?.kind === 'runtime' + ? parsedHost.environmentId + : null + : getRuntimeEnvironmentIdForRepo(state, repoId)) + // Why: hook inspection must follow the repo owner. SSH/local repos execute + // through desktop IPC, while runtime repos may differ from the focused host. + return state.settings + ? { ...state.settings, activeRuntimeEnvironmentId: runtimeEnvironmentId } + : ({ activeRuntimeEnvironmentId: runtimeEnvironmentId } as AppState['settings']) +} + +export function canUseRepoWideTrust(state: AppState, repoId: string): boolean { + const hasDuplicateRepoId = state.repos.filter((repo) => repo.id === repoId).length > 1 + return Boolean(state.trustedOrcaHooks[repoId]?.all) && !hasDuplicateRepoId +} + +async function isScriptContentTrusted( + state: AppState, + repoId: string, + scriptKind: HookScriptKind, + scriptContent: string +): Promise { + if (canUseRepoWideTrust(state, repoId) || !scriptContent) { + return true + } + return ( + state.trustedOrcaHooks[repoId]?.[scriptKind]?.contentHash === + (await hashOrcaHookScript(scriptContent)) + ) +} + +export async function readHookTrustRequirement( + state: AppState, + repoId: string, + scriptKind: HookScriptKind, + hostId: ExecutionHostId | undefined, + runtimeOwnerEnvironmentId: string | null | undefined, + isCancelled: () => boolean +): Promise<'run' | 'skip' | { scriptContent: string }> { + if (isCancelled()) { + return 'skip' + } + if (canUseRepoWideTrust(state, repoId)) { + return 'run' + } + + let scriptContent = '' + try { + if (scriptKind === 'issueCommand') { + // Local overrides are user-owned; only shared orca.yaml commands need repo trust. + // Why: hostId disambiguates duplicate repo ids on the local IPC path, + // matching the checkRuntimeHooks call below. + const result = await readRuntimeIssueCommand( + settingsForHookRepoOwner(state, repoId, hostId, runtimeOwnerEnvironmentId), + repoId, + hostId + ) + if (result.source === 'local') { + return 'run' + } + if (result.status === 'error') { + return 'skip' + } + if (result.source !== 'shared') { + return 'run' + } + scriptContent = (result.sharedContent ?? '').trim() + } else { + const repo = findHookRepo(state, repoId, hostId) + const localScript = repo?.hookSettings?.scripts?.[scriptKind]?.trim() + const sourcePolicy = resolveHookCommandSourcePolicy(repo?.hookSettings?.commandSourcePolicy, { + hasLocalScript: Boolean(localScript) + }) + if (sourcePolicy === 'local-only') { + return 'run' + } + const result = await checkRuntimeHooks( + settingsForHookRepoOwner(state, repoId, hostId, runtimeOwnerEnvironmentId), + repoId, + hostId + ) + if (result.status === 'error') { + return 'skip' + } + const yamlHooks = (result.hooks as OrcaHooks | null) ?? null + scriptContent = + scriptKind === 'setup' + ? getSetupTrustContent(yamlHooks) + : scriptKind === 'vmRecipe' + ? getVmRecipeTrustContent(yamlHooks) + : (yamlHooks?.scripts?.[scriptKind] ?? '').trim() + } + } catch { + // Fail closed: if we cannot inspect the script, we cannot trust it. + return 'skip' + } + + return isCancelled() ? 'skip' : { scriptContent } +} + +/** Checks current hook content without opening or replacing the composer's modal. */ +export async function inspectHooksTrust( + state: AppState, + repoId: string, + scriptKind: HookScriptKind, + hostId?: ExecutionHostId, + runtimeOwnerEnvironmentId?: string | null, + isCancelled: () => boolean = NEVER_CANCEL_TRUST_CHECK +): Promise<'run' | 'skip' | 'confirmation-required'> { + const requirement = await readHookTrustRequirement( + state, + repoId, + scriptKind, + hostId, + runtimeOwnerEnvironmentId, + isCancelled + ) + if (typeof requirement === 'string') { + return requirement + } + const trusted = await isScriptContentTrusted(state, repoId, scriptKind, requirement.scriptContent) + if (isCancelled()) { + return 'skip' + } + return trusted ? 'run' : 'confirmation-required' +} diff --git a/src/renderer/src/lib/inspect-hooks-trust.test.ts b/src/renderer/src/lib/inspect-hooks-trust.test.ts new file mode 100644 index 00000000000..a9c4e560d87 --- /dev/null +++ b/src/renderer/src/lib/inspect-hooks-trust.test.ts @@ -0,0 +1,79 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '@/store/types' +import { inspectHooksTrust } from './ensure-hooks-confirmed' +import { hashOrcaHookScript } from './orca-hook-trust' + +const { checkHooks } = vi.hoisted(() => ({ checkHooks: vi.fn() })) +vi.mock('@/runtime/runtime-hooks-client', () => ({ + checkRuntimeHooks: checkHooks, + readRuntimeIssueCommand: vi.fn() +})) + +function makeState(): AppState { + return { + repos: [{ id: 'repo', path: '/repo' }], + trustedOrcaHooks: {}, + settings: null, + openModal: vi.fn(() => { + throw new Error('Inspection must never replace the composer') + }) + } as unknown as AppState +} + +describe('noninteractive composer hook trust', () => { + beforeEach(() => checkHooks.mockReset()) + + it('requires confirmation for unapproved setup and leaves the composer alone', async () => { + const state = makeState() + checkHooks.mockResolvedValue({ hooks: { scripts: { setup: 'pnpm install' } } }) + await expect(inspectHooksTrust(state, 'repo', 'setup')).resolves.toBe('confirmation-required') + expect(state.openModal).not.toHaveBeenCalled() + }) + + it('permits approved content but requires confirmation when default tab commands change', async () => { + const state = makeState() + state.trustedOrcaHooks.repo = { + setup: { contentHash: await hashOrcaHookScript('pnpm install'), approvedAt: 1 } + } + checkHooks.mockResolvedValue({ hooks: { scripts: { setup: 'pnpm install' } } }) + await expect(inspectHooksTrust(state, 'repo', 'setup')).resolves.toBe('run') + checkHooks.mockResolvedValue({ + hooks: { scripts: { setup: 'pnpm install' }, defaultTabs: [{ command: 'pnpm dev' }] } + }) + await expect(inspectHooksTrust(state, 'repo', 'setup')).resolves.toBe('confirmation-required') + }) + + it('does not turn failed inspection into permission to prepare', async () => { + checkHooks.mockRejectedValueOnce(new Error('disconnected')) + await expect(inspectHooksTrust(makeState(), 'repo', 'setup')).resolves.toBe('skip') + checkHooks.mockResolvedValueOnce({ status: 'error', hooks: null }) + await expect(inspectHooksTrust(makeState(), 'repo', 'setup')).resolves.toBe('skip') + }) + + it('inspects the execution owner even when another runtime is focused', async () => { + const state = makeState() + state.settings = { activeRuntimeEnvironmentId: 'elsewhere' } as AppState['settings'] + checkHooks.mockResolvedValue({ hooks: null }) + await expect(inspectHooksTrust(state, 'repo', 'setup', 'runtime:owner')).resolves.toBe('run') + expect(checkHooks).toHaveBeenCalledWith( + expect.objectContaining({ activeRuntimeEnvironmentId: 'owner' }), + 'repo', + 'runtime:owner' + ) + }) + + it('does not reuse repo-wide trust across duplicate repository identities', async () => { + const state = makeState() + state.repos = [state.repos[0], { ...state.repos[0], connectionId: 'remote' }] + state.trustedOrcaHooks.repo = { all: { approvedAt: 1 } } + checkHooks.mockResolvedValue({ hooks: { scripts: { setup: 'untrusted' } } }) + await expect(inspectHooksTrust(state, 'repo', 'setup')).resolves.toBe('confirmation-required') + }) + + it('does not inspect canceled composer work', async () => { + await expect( + inspectHooksTrust(makeState(), 'repo', 'setup', undefined, undefined, () => true) + ).resolves.toBe('skip') + expect(checkHooks).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/retained-worktree-creation.test.ts b/src/renderer/src/lib/retained-worktree-creation.test.ts new file mode 100644 index 00000000000..dc08a9dd09a --- /dev/null +++ b/src/renderer/src/lib/retained-worktree-creation.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it, vi } from 'vitest' +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' +import type { WorktreeCreationRequest } from './pending-worktree-creation' +import { createRetainedWorktreeCreation } from './retained-worktree-creation' + +function request(overrides: Partial = {}): WorktreeCreationRequest { + return { + repoId: 'repo', + name: 'draft', + baseBranch: 'main', + setupDecision: 'skip', + agent: null, + startup: { command: '', env: { PROJECT: 'original' } }, + pendingFirstAgentMessageRename: false, + note: '', + startupPlan: null, + quickPrompt: '', + quickTelemetry: null, + ...overrides + } +} + +const result = { + worktree: { id: 'retained', repoId: 'repo', path: '/workspace/draft' }, + startupTerminal: { tabId: 'same-tab', spawned: true } +} as CreateWorktreeResult + +describe('retained composer worktree creation', () => { + it('joins an unfinished create once, retaining its exact workspace and terminal result', async () => { + let finish!: (value: CreateWorktreeResult) => void + const create = vi.fn(() => new Promise((resolve) => (finish = resolve))) + const controller = createRetainedWorktreeCreation(create) + const original = request() + + expect(controller.start(original, 'host/root/shell')).toBe(true) + const adopted = controller.take(original, 'host/root/shell') + expect(adopted).not.toBeNull() + expect(controller.take(original, 'host/root/shell')).toBeNull() + expect(controller.start(original, 'host/root/shell')).toBe(false) + await Promise.resolve() + finish(result) + expect(await adopted).toBe(result) + expect(create).toHaveBeenCalledTimes(1) + }) + + it('adopts an already completed create despite object property insertion order', async () => { + const controller = createRetainedWorktreeCreation(async () => result) + const original = request() + controller.start(original, 'host') + await Promise.resolve() + const reordered = Object.fromEntries( + Object.entries(original).toReversed() + ) as WorktreeCreationRequest + expect(await controller.take(reordered, 'host')).toBe(result) + }) + + it('isolates and freezes the execution snapshot from subsequent composer edits', async () => { + const create = vi.fn(async (_snapshot: WorktreeCreationRequest) => result) + const controller = createRetainedWorktreeCreation(create) + const original = request() + controller.start(original, 'host') + original.startup!.env!.PROJECT = 'edited' + original.name = 'edited' + await Promise.resolve() + + const snapshot = create.mock.calls[0][0] + expect(snapshot.name).toBe('draft') + expect(snapshot.startup?.env?.PROJECT).toBe('original') + expect(Object.isFrozen(snapshot.startup?.env)).toBe(true) + expect(controller.take(original, 'host')).toBeNull() + expect(controller.start(original, 'host')).toBe(false) + expect(controller.take(request(), 'host')).toBeNull() + }) + + it.each(['different-host', 'different-root', 'different-shell', 'different-environment'])( + 'refuses adoption after changing execution identity to %s', + (identity) => { + const controller = createRetainedWorktreeCreation(async () => result) + controller.start(request(), 'original') + expect(controller.take(request(), identity)).toBeNull() + expect(controller.take(request(), 'original')).toBeNull() + } + ) + + it('finishes ordinary creation when the composer cancels before create begins', async () => { + const create = vi.fn(async () => result) + const controller = createRetainedWorktreeCreation(create) + controller.start(request(), 'host') + controller.retire() + await Promise.resolve() + expect(create).toHaveBeenCalledTimes(1) + expect(controller.take(request(), 'host')).toBeNull() + expect(controller.start(request(), 'host')).toBe(false) + }) + + it('preserves failure for a matching submit without automatically creating again', async () => { + const failure = new Error('host contact lost; outcome unknown') + const create = vi.fn(async () => { + throw failure + }) + const controller = createRetainedWorktreeCreation(create) + controller.start(request(), 'host') + await expect(controller.take(request(), 'host')).rejects.toBe(failure) + expect(controller.start(request(), 'host')).toBe(false) + expect(create).toHaveBeenCalledTimes(1) + }) + + it.each>([ + { agent: 'claude' }, + { startup: { command: 'agent' } }, + { startup: { command: '', launchAgent: 'claude' } }, + { issueCommand: { command: 'automation' } }, + { launchDraftPrompt: 'send this' }, + { agentLaunchRoute: 'structured-native-chat' }, + { ephemeralVmRecipe: { sourceRepoId: 'repo', recipeId: 'vm', projectId: 'project' } }, + { ephemeralVmRuntimeId: 'vm' }, + { ephemeralVmRuntimeEnvironmentId: 'runtime' }, + { ephemeralVmCheckoutMode: 'provisioned-root' }, + { ephemeralVmExpectedRefHead: 'commit' } + ])('refuses unsafe early launch work: %j', (override) => { + const create = vi.fn(async () => result) + const controller = createRetainedWorktreeCreation(create) + expect(controller.start(request(override), 'host')).toBe(false) + expect(create).not.toHaveBeenCalled() + expect(controller.start(request(), 'host')).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/retained-worktree-creation.ts b/src/renderer/src/lib/retained-worktree-creation.ts new file mode 100644 index 00000000000..5bb27cad187 --- /dev/null +++ b/src/renderer/src/lib/retained-worktree-creation.ts @@ -0,0 +1,87 @@ +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' +import type { WorktreeCreationRequest } from './pending-worktree-creation' + +export function canRetainComposerWorktree(request: WorktreeCreationRequest): boolean { + return ( + request.agent === null && + request.startup?.command === '' && + !request.startup.launchAgent && + !request.startup.launchConfig && + !request.startupPlan && + !request.launchDraftPrompt && + !request.issueCommand && + !request.ephemeralVmRecipe && + !request.ephemeralVmRuntimeId && + !request.ephemeralVmRuntimeEnvironmentId && + !request.ephemeralVmCheckoutMode && + !request.ephemeralVmExpectedRefHead && + request.agentLaunchRoute !== 'structured-native-chat' + ) +} + +function serializeRequest(request: WorktreeCreationRequest): string { + return JSON.stringify(request, (_key, value: unknown) => { + if (value && typeof value === 'object' && !Array.isArray(value)) { + return Object.fromEntries( + Object.entries(value).sort(([left], [right]) => left.localeCompare(right)) + ) + } + return value + }) +} + +function freezeRequest(value: unknown): void { + if (!value || typeof value !== 'object') { + return + } + for (const child of Object.values(value)) { + freezeRequest(child) + } + Object.freeze(value) +} + +export function createRetainedWorktreeCreation( + create: (request: WorktreeCreationRequest) => Promise +) { + let started = false + let retired = false + let taken = false + let requestKey: string | null = null + let ownerKey: string | null = null + let creation: Promise | null = null + + return { + start(request: WorktreeCreationRequest, executionIdentity: string): boolean { + if (started || retired || !executionIdentity || !canRetainComposerWorktree(request)) { + return false + } + requestKey = serializeRequest(request) + const snapshot = JSON.parse(requestKey) as WorktreeCreationRequest + freezeRequest(snapshot) + ownerKey = executionIdentity + started = true + creation = Promise.resolve().then(() => create(snapshot)) + // An abandoned composer has no submit awaiting a failed ordinary creation. + void creation.catch(() => undefined) + return true + }, + take( + request: WorktreeCreationRequest, + executionIdentity: string + ): Promise | null { + if (retired || taken || !creation) { + return null + } + if (ownerKey !== executionIdentity || requestKey !== serializeRequest(request)) { + retired = true + return null + } + taken = true + return creation + }, + retire(): void { + // Ordinary workspace persistence owns all work once creation starts. + retired = true + } + } +} diff --git a/src/renderer/src/lib/worktree-creation-flow-execute.ts b/src/renderer/src/lib/worktree-creation-flow-execute.ts index e1a57f615e7..8072b99bb6c 100644 --- a/src/renderer/src/lib/worktree-creation-flow-execute.ts +++ b/src/renderer/src/lib/worktree-creation-flow-execute.ts @@ -8,7 +8,6 @@ import { cleanupEphemeralVmRuntimeForFailedCreate, prepareRequestForCreate } from '@/lib/ephemeral-vm-worktree-creation' -import { getProvisionedRootCreateOptions } from '@/lib/provisioned-root-create-options' import { formatWorkspaceCreateError, getWorkspaceCreateErrorToastMessage @@ -17,7 +16,7 @@ import { isAgentSessionHandleProvider } from '../../../shared/agent-session-prov import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation' import { createBrowserUuid } from '@/lib/browser-uuid' -import { resolveBackendDraftStartup } from '@/lib/worktree-draft-startup-view-mode' +import { createRequestedWorktree } from '@/lib/create-requested-worktree' import { buildWorktreeCreationStartupOpt } from '@/lib/worktree-creation-flow-startup' import { launchStructuredWorktreeSession } from '@/lib/worktree-creation-structured-session' import { completeWorktreeCreation } from '@/lib/worktree-creation-completion' @@ -44,7 +43,8 @@ async function preflightAgentTrust( export async function executeWorktreeCreation( creationId: string, - request: WorktreeCreationRequest + request: WorktreeCreationRequest, + retainedCreation?: Promise ): Promise { const preparedRequest = await prepareRequestForCreate(creationId, request) if (!preparedRequest) { @@ -53,62 +53,7 @@ export async function executeWorktreeCreation( let result: CreateWorktreeResult try { - const provisionedRoot = getProvisionedRootCreateOptions(preparedRequest) - const structuredLaunch = preparedRequest.agentLaunchRoute === 'structured-native-chat' - const backendStartup = - provisionedRoot || structuredLaunch ? undefined : resolveBackendDraftStartup(preparedRequest) - result = await useAppStore - .getState() - .createWorktree( - preparedRequest.repoId, - preparedRequest.name, - preparedRequest.baseBranch, - preparedRequest.setupDecision, - preparedRequest.sparseCheckout, - preparedRequest.telemetrySource, - preparedRequest.displayName, - preparedRequest.linkedIssue, - preparedRequest.linkedPR, - preparedRequest.pushTarget, - preparedRequest.agent ?? undefined, - preparedRequest.linkedLinearIssue, - preparedRequest.branchNameOverride, - preparedRequest.workspaceStatus, - preparedRequest.linkedGitLabMR, - preparedRequest.linkedGitLabIssue, - backendStartup, - structuredLaunch ? false : preparedRequest.pendingFirstAgentMessageRename, - creationId, - preparedRequest.linkedLinearIssueWorkspaceId, - preparedRequest.linkedLinearIssueOrganizationUrlKey, - preparedRequest.linkedBitbucketPR, - preparedRequest.linkedAzureDevOpsPR, - preparedRequest.linkedGiteaPR, - preparedRequest.compareBaseRef, - { - ...(preparedRequest.nameWasGenerated ? { nameWasGenerated: true } : {}), - ...(preparedRequest.displayNameKind - ? { displayNameKind: preparedRequest.displayNameKind } - : {}), - ...(preparedRequest.linkedWorkItem !== undefined - ? { linkedWorkItem: preparedRequest.linkedWorkItem } - : {}), - ...(preparedRequest.linkedTaskSourceContext !== undefined - ? { linkedTaskSourceContext: preparedRequest.linkedTaskSourceContext } - : {}), - // Why: the remote host must own task-draft startup so its initial terminal is the agent, not an idle fallback shell. - ...(!structuredLaunch && - !backendStartup && - preparedRequest.agent && - preparedRequest.launchDraftPrompt - ? { startupDraft: preparedRequest.launchDraftPrompt } - : {}), - ...(provisionedRoot ? { provisionedRoot } : {}), - ...(preparedRequest.parentWorktreeId - ? { parentWorktreeId: preparedRequest.parentWorktreeId } - : {}) - } - ) + result = await (retainedCreation ?? createRequestedWorktree(creationId, preparedRequest)) } catch (error) { // Why: a missing entry means the user cancelled mid-flight — abandon // silently rather than surfacing an error for work they already dismissed. diff --git a/src/renderer/src/lib/worktree-creation-flow.test.ts b/src/renderer/src/lib/worktree-creation-flow.test.ts index 5e9c3293bc6..e358d2552ce 100644 --- a/src/renderer/src/lib/worktree-creation-flow.test.ts +++ b/src/renderer/src/lib/worktree-creation-flow.test.ts @@ -1,3 +1,5 @@ +import { executeWorktreeCreation } from './worktree-creation-flow-execute' +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { PendingWorktreeCreation, @@ -513,20 +515,20 @@ describe('staged background worktree creation', () => { expect(store.setSidebarOpen).not.toHaveBeenCalled() }) - it('keeps a backend startup terminal in the background after the user leaves', async () => { + it('adopts a retained terminal without recreating it or stealing focus', async () => { store.activeView = 'tasks' - store.createWorktree.mockResolvedValueOnce({ + const retained = Promise.resolve({ worktree: { id: 'wt-1', repoId: 'repo-1' }, startupTerminal: { tabId: 'agent-tab', spawned: true } - }) + } as CreateWorktreeResult) - const request = makeRequest({ startup: { command: '' } }) - continueBackgroundWorktreeCreation('creation-1', request, { revealCreationSurface: false }) + void executeWorktreeCreation('creation-1', makeRequest({ startup: { command: '' } }), retained) await flushAsyncWorktreeCreation() + expect(store.createWorktree).not.toHaveBeenCalled() expect(activateAndRevealWorktree).not.toHaveBeenCalled() expect(ensureWorktreeHasInitialTerminal).toHaveBeenCalledWith( store, diff --git a/src/renderer/src/lib/worktree-creation-flow.ts b/src/renderer/src/lib/worktree-creation-flow.ts index dfde4e7fe58..fa767172cba 100644 --- a/src/renderer/src/lib/worktree-creation-flow.ts +++ b/src/renderer/src/lib/worktree-creation-flow.ts @@ -1,3 +1,4 @@ +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' import { useAppStore } from '@/store' import { findPendingLinkedWorkItemCreationId, @@ -46,7 +47,10 @@ function revealPendingCreation( * immediately and the work outlives the now-closed modal. Progress and errors * surface on the pending creation's sidebar row and content panel. */ -export function runBackgroundWorktreeCreation(request: WorktreeCreationRequest): string { +export function runBackgroundWorktreeCreation( + request: WorktreeCreationRequest, + retainedCreation?: Promise +): string { const store = useAppStore.getState() const existingCreationId = findPendingLinkedWorkItemCreationId( store.pendingWorktreeCreations, @@ -62,7 +66,7 @@ export function runBackgroundWorktreeCreation(request: WorktreeCreationRequest): // client over plain HTTP). createBrowserUuid falls back to getRandomValues. const creationId = createBrowserUuid() revealPendingCreation(creationId, request, getInitialWorktreeCreationPhase(request)) - void executeWorktreeCreation(creationId, request) + void executeWorktreeCreation(creationId, request, retainedCreation) return creationId }