From b589bb3cc91eacd3e5c8d8623e6da5452eefa432 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:51:06 -0400 Subject: [PATCH] refactor(runtime): main alone starts setup and default tabs for an activated create (#28153) * refactor(runtime): main alone starts setup and default tabs for an activated create An activated CLI or paired create with a window open handed setup and default tabs to the host window, and with navigation 'all' to paired clients too, so both could run setup. Main now provisions them itself. Only when main tried to start setup and failed does it hand setup to the host window, which retries it where its output or spawn error is visible. * fix(runtime): hand the window the whole create when the agent fails to start Main provisioned setup and default tabs after a failed agent start and sent the window only the startup. The window found tabs, seeded nothing, and the agent never launched. A failed agent start now provisions nothing and hands startup, setup and default tabs to the host window, as before this PR. --- .../orca-runtime-activate-managed-worktree.ts | 9 +- .../orca-runtime-create-managed-worktree.ts | 6 +- .../orca-runtime-notify-ssh-state-changed.ts | 14 +- ...worktree-setup-and-startup-part-04.spec.ts | 12 +- .../worktree-setup-and-startup.spec.ts | 12 +- .../worktree-setup-single-creator.spec.ts | 145 ++++++++++++++++++ src/main/runtime/orca-runtime.test.ts | 1 + ...runtime-local-worktree-terminal-startup.ts | 59 ++++--- 8 files changed, 206 insertions(+), 52 deletions(-) create mode 100644 src/main/runtime/orca-runtime-tests/worktree-setup-single-creator.spec.ts diff --git a/src/main/runtime/orca-runtime-activate-managed-worktree.ts b/src/main/runtime/orca-runtime-activate-managed-worktree.ts index 4fe3601c9ce..28df6aa8dd3 100644 --- a/src/main/runtime/orca-runtime-activate-managed-worktree.ts +++ b/src/main/runtime/orca-runtime-activate-managed-worktree.ts @@ -87,14 +87,7 @@ export class OrcaRuntimeWithActivateManagedWorktree extends OrcaRuntimeWithListM this.notifyHostActivateWorktree(repo.id, worktree.id) } if (targetsClients) { - this.notifyClientsActivateWorktree( - repo.id, - worktree.id, - undefined, - undefined, - undefined, - navigation - ) + this.notifyClientsActivateWorktree(repo.id, worktree.id, undefined, navigation) } } if (!targetsHost) { diff --git a/src/main/runtime/orca-runtime-create-managed-worktree.ts b/src/main/runtime/orca-runtime-create-managed-worktree.ts index a651ef3265b..5fde863011b 100644 --- a/src/main/runtime/orca-runtime-create-managed-worktree.ts +++ b/src/main/runtime/orca-runtime-create-managed-worktree.ts @@ -216,13 +216,13 @@ export class OrcaRuntimeWithCreateManagedWorktree extends OrcaRuntimeWithGetWork pasteDraft: (handle, draft) => this.pasteStartupDraftWhenReady(handle, draft), sendFollowup: (handle, followup) => this.sendStartupFollowupWhenReady(handle, followup), provision: (options) => this.provisionManagedWorktreeTerminals(options, worktree), - activate: (repoId, worktreeId, activationSetup, startup, activationDefaultTabs) => + activate: (repoId, worktreeId, startup, failedSetup, failedDefaultTabs) => this.notifyActivateWorktree( repoId, worktreeId, - activationSetup, + failedSetup, startup, - activationDefaultTabs, + failedDefaultTabs, args.navigation ) } diff --git a/src/main/runtime/orca-runtime-notify-ssh-state-changed.ts b/src/main/runtime/orca-runtime-notify-ssh-state-changed.ts index bcd9b1f4665..b864bd15cbf 100644 --- a/src/main/runtime/orca-runtime-notify-ssh-state-changed.ts +++ b/src/main/runtime/orca-runtime-notify-ssh-state-changed.ts @@ -157,6 +157,7 @@ export class OrcaRuntimeWithNotifySshStateChanged extends OrcaRuntimeWithGetStat this.emitClientEvent({ type: 'reposChanged' }) } + /** `setup` and `defaultTabs` go to the host window only: main tried and failed to start them. */ protected notifyActivateWorktree( repoId: string, worktreeId: string, @@ -170,14 +171,7 @@ export class OrcaRuntimeWithNotifySshStateChanged extends OrcaRuntimeWithGetStat this.notifyHostActivateWorktree(repoId, worktreeId, setup, startup, defaultTabs) } if (navigationTargetsClients(navigation)) { - this.notifyClientsActivateWorktree( - repoId, - worktreeId, - setup, - startup, - defaultTabs, - navigation - ) + this.notifyClientsActivateWorktree(repoId, worktreeId, startup, navigation) } } @@ -194,13 +188,11 @@ export class OrcaRuntimeWithNotifySshStateChanged extends OrcaRuntimeWithGetStat protected notifyClientsActivateWorktree( repoId: string, worktreeId: string, - setup?: CreateWorktreeResult['setup'], startup?: WorktreeStartupLaunch, - defaultTabs?: CreateWorktreeResult['defaultTabs'], navigation: RuntimeNavigationTarget = 'clients' ): void { this.emitClientEvent( - toRuntimeActivateWorktreeEvent(repoId, worktreeId, setup, startup, defaultTabs, navigation) + toRuntimeActivateWorktreeEvent(repoId, worktreeId, undefined, startup, undefined, navigation) ) } diff --git a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-04.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-04.spec.ts index c145d17bedf..a10e25a76ff 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-04.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-04.spec.ts @@ -1,3 +1,4 @@ +import type { RuntimeClientEvent } from '../../../shared/runtime-client-events' import { getDefaultWorkspaceSession } from '../../../shared/constants' import { describe, expect, it, vi } from 'vitest' import { @@ -377,7 +378,7 @@ describe('OrcaRuntimeService', () => { ) }) - it('passes the wrapped setup command to activation when startup spawned but setup did not', async () => { + it('hands only the host window a setup main tried and failed to start', async () => { const runtime = new OrcaRuntimeService(store) const spawn = vi .fn() @@ -405,6 +406,8 @@ describe('OrcaRuntimeService', () => { terminalDriverChanged: vi.fn() }) runtime.attachWindow(1) + const events: RuntimeClientEvent[] = [] + const unsubscribe = runtime.onClientEvent((event) => events.push(event)) computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-startup-setup-retry') ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-startup-setup-retry') @@ -438,8 +441,10 @@ describe('OrcaRuntimeService', () => { name: 'runtime-startup-setup-retry', setupDecision: 'run', activate: true, + navigation: 'all', startup: { command: 'claude' } }) + unsubscribe() expect(spawn).toHaveBeenCalledTimes(2) expect(activateWorktree).toHaveBeenCalledWith( @@ -452,6 +457,11 @@ describe('OrcaRuntimeService', () => { undefined, undefined ) + expect(activateWorktree).toHaveBeenCalledOnce() + // The window retries it where the user sees its output or its spawn error; clients never run it. + expect( + events.filter((event) => event.type === 'activateWorktree' && event.setup !== undefined) + ).toEqual([]) const activationSetup = activateWorktree.mock.calls[0]?.[2] as { command?: string } | undefined expect(activationSetup?.command).toContain('printf') }) diff --git a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup.spec.ts index d4e63c978c3..d872bf854e2 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup.spec.ts @@ -95,16 +95,17 @@ describe('OrcaRuntimeService', () => { } } }) + // Main never tried to start setup (no terminal host), so the window is handed nothing. expect(activateWorktree).toHaveBeenCalledWith( 'repo-1', expect.any(String), - result.setup, + undefined, undefined, undefined ) }) - it('passes setup payloads through when explicitly activating CLI-created worktrees', async () => { + it('does not hand the window a setup main never tried to start', async () => { const runtime = new OrcaRuntimeService(store) const activateWorktree = vi.fn() runtime.setNotifier({ @@ -153,10 +154,12 @@ describe('OrcaRuntimeService', () => { activate: true }) + expect(result.setup).toMatchObject({ runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh' }) + // Main never tried to start setup (no terminal host), so the window is handed nothing. expect(activateWorktree).toHaveBeenCalledWith( 'repo-1', expect.any(String), - result.setup, + undefined, undefined, undefined ) @@ -233,10 +236,11 @@ describe('OrcaRuntimeService', () => { runnerScriptPath: 'C:\\repo\\.git\\orca\\setup-runner.sh', shell: { family: 'posix' } }) + // Main never tried to start setup (no terminal host), so the window is handed nothing. expect(activateWorktree).toHaveBeenCalledWith( 'repo-1', expect.any(String), - result.setup, + undefined, undefined, undefined ) diff --git a/src/main/runtime/orca-runtime-tests/worktree-setup-single-creator.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-setup-single-creator.spec.ts new file mode 100644 index 00000000000..0b9f400773c --- /dev/null +++ b/src/main/runtime/orca-runtime-tests/worktree-setup-single-creator.spec.ts @@ -0,0 +1,145 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RuntimeClientEvent } from '../../../shared/runtime-client-events' +import type { WorktreeStartupLaunch } from '../../../shared/worktree/launch-types' +import { + OrcaRuntimeService, + computeWorktreePathMock, + createSetupRunnerScript, + ensurePathWithinWorkspaceMock, + getEffectiveHooks, + listWorktrees, + setRuntimeDesktopSurface, + shouldRunSetupForCreate +} from '../orca-runtime-test-mocks.spec' +import { store } from '../orca-runtime-test-fixtures.spec' + +const WORKTREE_PATH = '/tmp/workspaces/runtime-setup-all' +const RUNNER = '/tmp/repo/.git/orca/setup-runner.sh' + +/** An activated create with a live window, navigating the window and paired clients. */ +async function createActivated(options: { + startup?: WorktreeStartupLaunch + failAgentSpawn?: boolean +}): Promise<{ + activateWorktree: ReturnType + events: RuntimeClientEvent[] + spawnedCommands: (string | undefined)[] +}> { + const runtime = new OrcaRuntimeService(store) + const activateWorktree = vi.fn() + let spawned = 0 + const spawn = vi.fn(async (args: { command?: string }) => { + if (options.failAgentSpawn && args.command === options.startup?.command) { + throw new Error('agent_session_exited_during_start') + } + return { id: `pty-${++spawned}` } + }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree, + createTerminal: vi.fn(), + revealTerminalSession: vi.fn().mockResolvedValue({ tabId: 'tab-revealed' }), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + // A live window, so main hands work to it instead of provisioning in the background. + const liveWindow = { isDestroyed: () => false, webContents: { isDestroyed: () => false } } + setRuntimeDesktopSurface({ + showNotification: () => false, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the create path reads only the window's liveness. + findWindowById: () => liveWindow as never, + onIpc: () => {}, + removeIpcListener: () => {} + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + const events: RuntimeClientEvent[] = [] + const unsubscribe = runtime.onClientEvent((event) => events.push(event)) + + computeWorktreePathMock.mockReturnValue(WORKTREE_PATH) + ensurePathWithinWorkspaceMock.mockReturnValue(WORKTREE_PATH) + vi.mocked(getEffectiveHooks).mockReturnValue({ scripts: { setup: 'pnpm worktree:setup' } }) + vi.mocked(shouldRunSetupForCreate).mockReturnValue(true) + vi.mocked(createSetupRunnerScript).mockReturnValue({ + runnerScriptPath: RUNNER, + envVars: { ORCA_ROOT_PATH: '/tmp/repo', ORCA_WORKTREE_PATH: WORKTREE_PATH } + }) + vi.mocked(listWorktrees).mockResolvedValue([ + { + path: WORKTREE_PATH, + head: 'def', + branch: 'runtime-setup-all', + isBare: false, + isMainWorktree: false + } + ]) + + try { + await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'runtime-setup-all', + setupDecision: 'run', + activate: true, + navigation: 'all', + ...(options.startup ? { startup: options.startup } : {}) + }) + } finally { + unsubscribe() + setRuntimeDesktopSurface(null) + } + return { + activateWorktree, + events, + spawnedCommands: spawn.mock.calls.map(([args]) => args.command) + } +} + +describe('OrcaRuntimeService setup creator', () => { + it('creates the setup terminal once when an activated create navigates the window and clients', async () => { + const { activateWorktree, events, spawnedCommands } = await createActivated({}) + + const hostHandoffs = activateWorktree.mock.calls.filter((call) => call[2] !== undefined) + const clientHandoffs = events.filter( + (event) => event.type === 'activateWorktree' && event.setup !== undefined + ) + expect({ + hostHandoffs: hostHandoffs.length, + clientHandoffs: clientHandoffs.length, + setupSpawns: spawnedCommands.filter((command) => command?.includes(RUNNER)).length + }).toEqual({ hostHandoffs: 0, clientHandoffs: 0, setupSpawns: 1 }) + }) + + it('hands the host window the whole create when the agent fails to start', async () => { + const startup = { command: 'claude' } + const { activateWorktree, events, spawnedCommands } = await createActivated({ + startup, + failAgentSpawn: true + }) + + // Main provisions nothing, so the window seeds the agent's tab and lays setup out around it. + expect(spawnedCommands).toEqual(['claude']) + expect(activateWorktree).toHaveBeenCalledOnce() + expect(activateWorktree).toHaveBeenCalledWith( + 'repo-1', + expect.any(String), + expect.objectContaining({ runnerScriptPath: RUNNER }), + expect.objectContaining({ command: 'claude' }), + undefined + ) + expect( + events.filter((event) => event.type === 'activateWorktree' && event.setup !== undefined) + ).toEqual([]) + }) +}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index bd233359676..c32d20e71d1 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -110,6 +110,7 @@ await import('./orca-runtime-tests/worktree-setup-and-startup.spec') await import('./orca-runtime-tests/worktree-setup-and-startup-part-02.spec') await import('./orca-runtime-tests/worktree-setup-and-startup-part-03.spec') await import('./orca-runtime-tests/worktree-setup-and-startup-part-04.spec') +await import('./orca-runtime-tests/worktree-setup-single-creator.spec') await import('./orca-runtime-tests/gitlab-and-pr-bases.spec') await import('./orca-runtime-tests/gitlab-and-pr-bases-part-02.spec') await import('./orca-runtime-tests/gitlab-and-pr-bases-part-03.spec') diff --git a/src/main/runtime/runtime-local-worktree-terminal-startup.ts b/src/main/runtime/runtime-local-worktree-terminal-startup.ts index 9034228b37d..5ef9eb7611b 100644 --- a/src/main/runtime/runtime-local-worktree-terminal-startup.ts +++ b/src/main/runtime/runtime-local-worktree-terminal-startup.ts @@ -33,9 +33,10 @@ type Ports = { activate: ( repoId: string, worktreeId: string, - setup?: CreateWorktreeResult['setup'], startup?: WorktreeStartupLaunch, - defaultTabs?: CreateWorktreeResult['defaultTabs'] + // Only work main tried to start and failed; the host window retries it where the user sees it. + failedSetup?: CreateWorktreeResult['setup'], + failedDefaultTabs?: CreateWorktreeResult['defaultTabs'] ) => void } @@ -125,32 +126,40 @@ export async function startRuntimeLocalWorktreeTerminals(args: { if (shouldActivate) { const provisionInBackground = ports.provisionInBackground?.() === true - const runtimeWillProvision = - (provisionInBackground && ports.canSpawn) || - (didSpawnStartup && Boolean(setup || defaultTabs)) - if (runtimeWillProvision) { - const provisioned = await ports.provision({ - ...provisionArgs(args, startupTerminalHandle, didSpawnStartup, wrappedSetupCommand), - ...(provisionInBackground ? { surfaceOwner: false as const } : {}) - }) - didSpawnSetup = provisioned.setupSpawned - setupTerminalHandle = provisioned.setupTerminalHandle - } - const activationSetup = didSpawnSetup - ? undefined - : setup - ? { + // Why: a failed agent start hands the whole create to the window, which seeds the agent's tab + // first and lays setup and default tabs around it; main provisioning first would leave it no tab. + const agentStartFailed = Boolean(sequencedStartup && ports.canSpawn && !didSpawnStartup) + if (agentStartFailed && !provisionInBackground) { + ports.activate(repo.id, worktree.id, startup, setup, defaultTabs) + } else { + let failedSetup: CreateWorktreeResult['setup'] + // Why: main alone creates the setup and default tabs; the window draws them from the reveals. + if (ports.canSpawn && (provisionInBackground || setup || defaultTabs)) { + const provisioned = await ports.provision({ + ...provisionArgs( + args, + startupTerminalHandle, + didSpawnStartup, + didSpawnStartup ? wrappedSetupCommand : undefined + ), + ...(provisionInBackground ? { surfaceOwner: false as const } : {}) + }) + didSpawnSetup = provisioned.setupSpawned + setupTerminalHandle = provisioned.setupTerminalHandle + if (setup && !didSpawnSetup) { + failedSetup = { ...setup, ...(didSpawnStartup && wrappedSetupCommand ? { command: wrappedSetupCommand } : {}) } - : undefined - ports.activate( - repo.id, - worktree.id, - activationSetup, - startup && !didSpawnStartup ? startup : undefined, - runtimeWillProvision ? undefined : defaultTabs - ) + } + } + ports.activate( + repo.id, + worktree.id, + startup && !didSpawnStartup ? startup : undefined, + failedSetup + ) + } } else if (ports.canSpawn && (setup || defaultTabs || didSpawnStartup)) { const provisioning = ports.provision({ ...provisionArgs(args, startupTerminalHandle, didSpawnStartup, wrappedSetupCommand),