From bd88b2acea26327dd01a1e65fa2c40c1802b439d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:28:58 -0700 Subject: [PATCH] fix(orchestration): resolve worker-start new-child/new-top-level targets without requiring a managed caller worktree (#34388) `orca worker start --worktree new-top-level --repo ` no longer requires the dispatching terminal to sit inside a managed git worktree; the explicit repo is all a top-level worktree needs. new-child (and new-top-level without --repo) still need a caller worktree, and now fail with an actionable invalid_argument error before any effects are recorded instead of a bare selector_not_found. Fixes #22345 --- .../worker/local-worker-start.ts | 27 +++++++----- ...ker-start-agent-placement-teardown.test.ts | 2 +- .../worker/worker-start-agent-placement.ts | 20 ++++----- ...orker-start-launch-executor-parity.test.ts | 4 +- .../worker/worker-worktree-creation.ts | 44 +++++++++++++++---- .../worker/workers-new-worktree.test.ts | 43 ++++++++++++++++++ 6 files changed, 108 insertions(+), 32 deletions(-) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts index 0d39e6d4354..f52fc2b0128 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts @@ -18,6 +18,7 @@ import { import { EXISTING_WORKTREE_SETUP, placeWorkerAgent } from './worker-start-agent-placement' import { awaitStructuredWorkerSetupGate } from './worker-start-structured-setup-gate' import { assertOrchestrationWorktreeCreationSupported } from './folder-worktree-placement' +import { resolveWorkerWorktreeCreationTarget } from './worker-worktree-creation' import type { WorkerStartInput } from './worker-start-schema' import { persistGatedSetupSpawnFailure, @@ -57,17 +58,21 @@ export async function startLocalWorker(args: { const chat = terminal ? chatAssigneeOf(terminal) : null const requestedWorktree = params.worktree ?? 'current' const createsWorktree = requestedWorktree === 'new-child' || requestedWorktree === 'new-top-level' + const creationTargetFor = (callerWorktreeId: string) => + resolveWorkerWorktreeCreationTarget({ + runtime, + requestedWorktree, + repo: params.repo, + callerWorktreeId + }) const launchParams = await resolveWorkerConfiguredAgentParams(runtime, params, async () => { const callerWorkspaceId = await resolveDispatchCallerWorktreeId( runtime, params.from, callerSession ) - const parent = createsWorktree - ? await runtime.showManagedWorktree(`id:${callerWorkspaceId}`) - : undefined return createsWorktree - ? { repo: params.repo ?? parent?.repoId } + ? { repo: (await creationTargetFor(callerWorkspaceId)).repoSelector } : { worktree: requestedWorktree === 'current' ? `id:${callerWorkspaceId}` : requestedWorktree } @@ -98,18 +103,18 @@ export async function startLocalWorker(args: { params.from, callerSession ) - const creationWorktree = createsWorktree - ? await runtime.showManagedWorktree(`id:${coordinatorWorktreeId}`) + const creationTarget = createsWorktree + ? await creationTargetFor(coordinatorWorktreeId) : undefined - if (creationWorktree) { + if (creationTarget) { await assertOrchestrationWorktreeCreationSupported({ runtime, - repoSelector: params.repo ?? creationWorktree.repoId, + repoSelector: creationTarget.repoSelector, existingPlacement: 'current or an exact existing folder workspace' }) } let resolvedWorktree: Awaited>['worktree'] | undefined = - creationWorktree + creationTarget ? undefined : requestedWorktree === 'current' ? await runtime.showManagedTerminalWorkspace(`id:${coordinatorWorktreeId}`) @@ -132,7 +137,7 @@ export async function startLocalWorker(args: { mode, resolvedWorktreeId: resolvedWorktree?.id ?? null, name: params.name ?? null, - repo: params.repo ?? creationWorktree?.repoId ?? null, + repo: creationTarget?.repoSelector ?? params.repo ?? null, baseBranch: params.baseBranch ?? null, terminal: params.terminal ?? null, agent: agent ?? null, @@ -188,7 +193,7 @@ export async function startLocalWorker(args: { taskId: task.id, params, requestedWorktree, - creationWorktree, + creationTarget, resolvedWorktree, mode, agent, diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement-teardown.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement-teardown.test.ts index 0d971d6a4b8..a7f9d874736 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement-teardown.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement-teardown.test.ts @@ -88,7 +88,7 @@ describe('a worker launch that throws after its session exists', () => { taskId: 'task_1', params: WorkerStartParams.parse({ from: 'term_coord', task: 'task_1' }), requestedWorktree: 'id:wt_existing', - creationWorktree: undefined, + creationTarget: undefined, resolvedWorktree: { id: 'wt_existing', repoId: 'repo_1' }, mode: STRUCTURED, agent: 'claude', diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement.ts index 9a3c609a02f..611d092bbb1 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement.ts @@ -20,7 +20,7 @@ import { type WorkerEffect, type WorkerSetupReceipt } from './worker-topology' -import { createWorkerWorktree } from './worker-worktree-creation' +import { createWorkerWorktree, type WorkerWorktreeCreationTarget } from './worker-worktree-creation' /** Name only the two required fields so untyped worktree accessors cannot introduce any into unions. */ type PlacedWorktree = { id: string; repoId: string } @@ -43,8 +43,8 @@ type WorkerAgentPlacementArgs = { taskId: string params: WorkerStartInput requestedWorktree: string - /** The coordinator's worktree, present only when this start creates one. */ - creationWorktree: PlacedWorktree | undefined + /** Present only when this start creates a worktree. */ + creationTarget: WorkerWorktreeCreationTarget | undefined /** The already-resolved placement, present only when this start does not create one. */ resolvedWorktree: PlacedWorktree | undefined /** Decided before the dispatch record; for an existing worktree, already settled on its host. */ @@ -69,7 +69,7 @@ export const EXISTING_WORKTREE_SETUP: WorkerSetupReceipt = { export async function placeWorkerAgent( args: WorkerAgentPlacementArgs ): Promise { - const existing = args.creationWorktree ? undefined : requireWorktree(args.resolvedWorktree) + const existing = args.creationTarget ? undefined : requirePlacement(args.resolvedWorktree) if (existing && args.params.terminal) { args.effects.push({ kind: 'terminal', @@ -130,7 +130,7 @@ async function launchWorkerAgent( db: args.db, dispatchId: args.dispatchId, requestedWorktree: args.requestedWorktree, - coordinatorWorktree: requireWorktree(args.creationWorktree), + target: requirePlacement(args.creationTarget), params: args.params, agent, // The executor withholds the startup agent exactly when the worker is to be a session. @@ -161,7 +161,7 @@ async function launchWorkerAgent( } return { mode: launched.receipt, - worktree: existing ?? requireWorktree(created?.worktree), + worktree: existing ?? requirePlacement(created?.worktree), terminalHandle: launched.outcome.handle, structuredSession, setupReceipt: created?.setupReceipt ?? EXISTING_WORKTREE_SETUP, @@ -216,11 +216,11 @@ function workerSurfaceFactory( } } -function requireWorktree(worktree: PlacedWorktree | undefined): PlacedWorktree { - if (!worktree) { - throw new Error('Worker topology did not resolve a worktree.') +function requirePlacement(placement: T | undefined): T { + if (!placement) { + throw new Error('Worker topology did not resolve a placement.') } - return worktree + return placement } function requireAgent(agent: TuiAgent | undefined): TuiAgent { diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-launch-executor-parity.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-launch-executor-parity.test.ts index c6180d50e84..8a148a5d13f 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-launch-executor-parity.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-launch-executor-parity.test.ts @@ -41,7 +41,7 @@ const TERMINAL: AgentLaunchModeReceipt = { reason: 'user_default', detail: 'Started a terminal agent worker, the default for new agent tabs in your settings.' } -const COORDINATOR = { id: 'wt_coord', repoId: 'repo_1' } +const CREATION_TARGET = { repoSelector: 'repo_1', parentWorktreeId: 'wt_coord' } const EXISTING = { id: 'wt_existing', repoId: 'repo_1' } function fakes() { @@ -91,7 +91,7 @@ function place( ...(args.terminal ? { terminal: args.terminal } : {}) }), requestedWorktree: args.existing ? 'id:wt_existing' : 'new-child', - creationWorktree: args.existing ? undefined : COORDINATOR, + creationTarget: args.existing ? undefined : CREATION_TARGET, resolvedWorktree: args.existing ? EXISTING : undefined, mode: args.mode, agent: args.terminal ? undefined : 'claude', diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-worktree-creation.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-worktree-creation.ts index 5fa4e288a10..04941ccacc6 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-worktree-creation.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-worktree-creation.ts @@ -4,19 +4,47 @@ import type { AgentLaunchPreferences } from '../../../../../../shared/agent-sess import type { TuiAgent } from '../../../../../../shared/tui-agent' import type { OrcaRuntimeService } from '../../../../orca-runtime' import type { OrchestrationDb } from '../../../../orchestration/db' +import { OrchestrationError } from '../../../../orchestration/orchestration-error' import type { WorkerEffect, WorkerSetupReceipt } from './worker-topology' +/** The repo a new worker worktree is created in, and its parent for `new-child`. */ +export type WorkerWorktreeCreationTarget = { repoSelector: string; parentWorktreeId?: string } + +export async function resolveWorkerWorktreeCreationTarget(args: { + runtime: Pick + requestedWorktree: string + repo: string | undefined + callerWorktreeId: string +}): Promise { + const { requestedWorktree, repo } = args + // An explicit repo is all a top-level worktree needs, so callers outside a worktree can use it. + if (requestedWorktree === 'new-top-level' && repo) { + return { repoSelector: repo } + } + const callerWorktree = await args.runtime + .showManagedWorktree(`id:${args.callerWorktreeId}`) + .catch((error: unknown) => { + if (error instanceof Error && error.message === 'selector_not_found') { + throw new OrchestrationError( + 'invalid_argument', + `The dispatching terminal's workspace could not be resolved to a git worktree Orca manages, which --worktree ${requestedWorktree} needs; run from one, or pass --worktree new-top-level --repo .` + ) + } + throw error + }) + return { + repoSelector: repo ?? callerWorktree.repoId, + ...(requestedWorktree === 'new-child' ? { parentWorktreeId: callerWorktree.id } : {}) + } +} + export async function createWorkerWorktree(args: { runtime: OrcaRuntimeService db: OrchestrationDb dispatchId: string requestedWorktree: string - coordinatorWorktree: Pick< - Awaited>, - 'id' | 'repoId' - > + target: WorkerWorktreeCreationTarget params: { - repo?: string name?: string baseBranch?: string displayName?: string @@ -36,11 +64,11 @@ export async function createWorkerWorktree(args: { terminalHandle: string | undefined setupReceipt: WorkerSetupReceipt }> { - const { runtime, db, dispatchId, requestedWorktree, coordinatorWorktree, params, effects } = args + const { runtime, db, dispatchId, requestedWorktree, target, params, effects } = args const setupDecision = params.setup ?? 'run' db.recordWorkerStage({ dispatchId, stage: 'worktree_creating', effects }) const created = await runtime.createManagedWorktree({ - repoSelector: params.repo ?? coordinatorWorktree.repoId, + repoSelector: target.repoSelector, name: params.name as string, baseBranch: params.baseBranch, displayName: params.displayName, @@ -64,7 +92,7 @@ export async function createWorkerWorktree(args: { : {}), activate: false, lineage: { - parentWorktree: requestedWorktree === 'new-child' ? coordinatorWorktree.id : undefined, + parentWorktree: target.parentWorktreeId, noParent: requestedWorktree === 'new-top-level', callerTerminalHandle: params.from } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/workers-new-worktree.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/workers-new-worktree.test.ts index 31babedbc1f..9767b01a06b 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/workers-new-worktree.test.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/workers-new-worktree.test.ts @@ -11,6 +11,7 @@ import type { RpcRequest } from '../../../core' import { ORCHESTRATION_METHODS } from '../../orchestration' import { prepareFederationWorkerLaunchOnHost } from './worker-opencode-model-preflight' import { FederationAttachStartParams } from '../federation/federation-start-schema' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../../../shared/constants' describe('orchestration new-worktree workers', () => { type CreateWorktreeResult = Awaited> @@ -288,6 +289,48 @@ describe('orchestration new-worktree workers', () => { expect(db.getDispatchContext(task.id)).toBeUndefined() }) + describe('from a terminal outside a managed git worktree', () => { + beforeEach(async () => { + const coordinator = await runtime.showTerminal('term_coord') + vi.mocked(runtime.showTerminal).mockResolvedValue({ + ...coordinator, + worktreeId: FLOATING_TERMINAL_WORKTREE_ID + }) + vi.mocked(runtime.showManagedWorktree).mockRejectedValue(new Error('selector_not_found')) + }) + + it('creates a new-top-level worktree in the explicit repo', async () => { + mockCreatedWorktree() + + const { result } = await startWorker({ worktree: 'new-top-level', repo: 'repo' }) + + expect(runtime.createManagedWorktree).toHaveBeenCalledWith( + expect.objectContaining({ + repoSelector: 'repo', + lineage: expect.objectContaining({ noParent: true, parentWorktree: undefined }) + }) + ) + expect(result).toMatchObject({ state: 'ready' }) + }) + + it.each([ + ['new-child', { repo: 'repo' }], + ['new-top-level', {}] + ] as const)( + 'refuses %s without a caller worktree before creating effects', + async (worktree, extra) => { + const createWorktree = vi.spyOn(runtime, 'createManagedWorktree') + + await expect(startWorker({ worktree, ...extra })).rejects.toMatchObject({ + code: 'invalid_argument', + message: expect.stringContaining('could not be resolved to a git worktree Orca manages') + }) + expect(createWorktree).not.toHaveBeenCalled() + expect(db.getDispatchContext(db.listTasks()[0]!.id)).toBeUndefined() + } + ) + }) + it('injects the execution host CLI command without a Dispatch capability', async () => { mockCreatedWorktree() vi.mocked(runtime.getTerminalOrchestrationCliCommand).mockReturnValue('orca-ide')