diff --git a/src/cli/handlers/worktree-setup-decision-recovery.ts b/src/cli/handlers/worktree-setup-decision-recovery.ts new file mode 100644 index 00000000000..27acc186b39 --- /dev/null +++ b/src/cli/handlers/worktree-setup-decision-recovery.ts @@ -0,0 +1,34 @@ +import { RuntimeClientError, RuntimeRpcFailureError } from '../runtime-client' + +// Why match the text: every host version refuses an undecided `ask` create with exactly this +// message and the generic runtime_error code, so it is the only stable signal. +const SETUP_DECISION_REQUIRED = 'Setup decision required for this repository' +const SETUP_DECISION_NEXT_STEP = + 'Pass --setup run to run the setup script, or --setup skip to create without it.' + +/** Why here, not on the host: desktop and phone choose setup in their own UI, so the host's + * shared refusal names no flag; only the CLI user answers it with --setup. */ +export async function withSetupDecisionRecovery(create: Promise): Promise { + try { + return await create + } catch (error) { + throw attachSetupDecisionRecovery(error) + } +} + +function attachSetupDecisionRecovery(error: unknown): unknown { + if (!(error instanceof RuntimeClientError) || error.message !== SETUP_DECISION_REQUIRED) { + return error + } + const data = { + ...(error.data && typeof error.data === 'object' ? error.data : {}), + nextSteps: [SETUP_DECISION_NEXT_STEP] + } + if (error instanceof RuntimeRpcFailureError) { + return new RuntimeRpcFailureError({ + ...error.response, + error: { ...error.response.error, data } + }) + } + return new RuntimeClientError(error.code, error.message, data) +} diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index e1b2fd904c7..f4980263ed9 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -38,6 +38,7 @@ import { import { getOptionalLinearIssueLinkFlag } from './worktree-linear-issue-link' import { getOptionalWorktreeUnreadFlag } from './worktree-unread-flag' import { getReviewTargetLinkFlags } from './worktree-review-link-flags' +import { withSetupDecisionRecovery } from './worktree-setup-decision-recovery' import { assertGitLabLinkFlagProjectsMatch } from './worktree-gitlab-link-context' function getEnvParentWorkspace(): string | undefined { @@ -206,37 +207,39 @@ export const WORKTREE_HANDLERS: Record = { const name = getRequiredStringFlag(flags, 'name') const repo = await getCreateRepoSelector(flags, cwdParentWorktree, client) await assertGitLabLinkFlagProjectsMatch(flags, client, { repo }) - const result = await client.call('worktree.create', { - repo, - name, - displayName: name, - displayNameKind: 'user', - baseBranch: getOptionalStringFlag(flags, 'base-branch'), - ...reviewLinks, - ...linearIssueLink, - comment: getOptionalStringFlag(flags, 'comment'), - runHooks: flags.get('run-hooks') === true, - activate, - // CLI activation targets its runtime's desktop, never unrelated paired viewers. - ...(activate ? { navigation: 'host' as const } : {}), - ...(setupDecision ? { setupDecision } : {}), - parentWorktree: explicitParentWorktree, - ...(explicitParentWorkspace ? { parentWorkspace: explicitParentWorkspace } : {}), - ...(envParentWorkspace ? { envParentWorkspace } : {}), - ...(cwdParentWorktree ? { cwdParentWorktree } : {}), - noParent, - callerTerminalHandle, - // Why: marks the workspace as CLI-created so the sidebar can badge and - // filter it. Sent on every `worktree create` — hand-typed or agent-run. - cliProvenanceRequest: callerTerminalHandle ? { callerTerminalHandle } : {}, - ...(startupAgent - ? { - startupAgent, - startupPrompt: getPresentStringFlag(flags, 'prompt', { allowEmpty: true }) ?? '', - launchSource: 'cli' - } - : {}) - }) + const result = await withSetupDecisionRecovery( + client.call('worktree.create', { + repo, + name, + displayName: name, + displayNameKind: 'user', + baseBranch: getOptionalStringFlag(flags, 'base-branch'), + ...reviewLinks, + ...linearIssueLink, + comment: getOptionalStringFlag(flags, 'comment'), + runHooks: flags.get('run-hooks') === true, + activate, + // CLI activation targets its runtime's desktop, never unrelated paired viewers. + ...(activate ? { navigation: 'host' as const } : {}), + ...(setupDecision ? { setupDecision } : {}), + parentWorktree: explicitParentWorktree, + ...(explicitParentWorkspace ? { parentWorkspace: explicitParentWorkspace } : {}), + ...(envParentWorkspace ? { envParentWorkspace } : {}), + ...(cwdParentWorktree ? { cwdParentWorktree } : {}), + noParent, + callerTerminalHandle, + // Why: marks the workspace as CLI-created so the sidebar can badge and + // filter it. Sent on every `worktree create` — hand-typed or agent-run. + cliProvenanceRequest: callerTerminalHandle ? { callerTerminalHandle } : {}, + ...(startupAgent + ? { + startupAgent, + startupPrompt: getPresentStringFlag(flags, 'prompt', { allowEmpty: true }) ?? '', + launchSource: 'cli' + } + : {}) + }) + ) printHookWarning(result.result, json) printLineageSummary(result.result, json) printResult(result, json, formatWorktreeShow) diff --git a/src/cli/index-worktree-create-target.test.ts b/src/cli/index-worktree-create-target.test.ts index dc7249ec5b4..5b643a02efc 100644 --- a/src/cli/index-worktree-create-target.test.ts +++ b/src/cli/index-worktree-create-target.test.ts @@ -43,6 +43,7 @@ vi.mock('child_process', async () => { import { main } from './index' import { buildWorktree, okFixture, queueFixtures, worktreeListFixture } from './test-fixtures' import { pairRuntimeEnvironment, useWorktreeAwarenessEnvironment } from './index-test-harness' +import { RuntimeRpcFailureError } from './runtime-client' describe('orca cli worktree awareness', () => { useWorktreeAwarenessEnvironment({ @@ -303,4 +304,29 @@ describe('orca cli worktree awareness', () => { expect.objectContaining({ cliProvenanceRequest: {} }) ) }) + + it('tells the user which --setup flag answers an undecided ask repo', async () => { + callMock.mockRejectedValueOnce( + new RuntimeRpcFailureError({ + id: 'req_create', + ok: false, + error: { code: 'runtime_error', message: 'Setup decision required for this repository' }, + _meta: { runtimeId: 'runtime-1' } + }) + ) + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const priorExitCode = process.exitCode + + await main( + ['worktree', 'create', '--repo', 'id:repo-1', '--name', 'child', '--no-parent'], + '/tmp/repo' + ) + + expect(errSpy.mock.calls.flat().join('\n')).toBe( + 'Setup decision required for this repository\n' + + 'Next step: Pass --setup run to run the setup script, or --setup skip to create without it.' + ) + expect(process.exitCode).toBe(1) + process.exitCode = priorExitCode + }) }) diff --git a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts index ee2f98dced5..8c372436588 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-03.spec.ts @@ -248,6 +248,81 @@ describe('OrcaRuntimeService', () => { expect(spawn).toHaveBeenCalledTimes(1) }) + it('skips with a warning a setup hook only the new branch adds when an ask repo got no decision', async () => { + const metaById: Record = {} + const runtimeStore = { + ...store, + getAllWorktreeMeta: () => metaById, + getWorktreeMeta: (worktreeId: string) => metaById[worktreeId], + setWorktreeMeta: (worktreeId: string, meta: Partial) => { + metaById[worktreeId] = { ...(metaById[worktreeId] ?? makeWorktreeMeta()), ...meta } + return metaById[worktreeId] + } + } + const runtime = new OrcaRuntimeService(runtimeStore as never) + const worktreesChanged = vi.fn() + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-branch-added-setup' }), + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.setNotifier({ + worktreesChanged, + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + revealTerminalSession: vi.fn().mockResolvedValue({ tabId: 'tab-branch-added-setup' }), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + runtime.attachWindow(1) + + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-branch-added-setup') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-branch-added-setup') + // The main checkout has no setup hook; only the new worktree's orca.yaml does. + vi.mocked(getEffectiveHooks).mockImplementation((_repo, worktreePath) => + worktreePath ? { scripts: { setup: 'pnpm worktree:setup' } } : null + ) + // An `ask` repo: an undecided create throws, as the real policy check does. + vi.mocked(shouldRunSetupForCreate).mockImplementation((_repo, decision) => { + if (decision === 'run' || decision === 'skip') { + return decision === 'run' + } + throw new Error('Setup decision required for this repository') + }) + vi.mocked(listWorktrees).mockResolvedValue([ + { + path: '/tmp/workspaces/runtime-branch-added-setup', + head: 'def', + branch: 'runtime-branch-added-setup', + isBare: false, + isMainWorktree: false + } + ]) + + const result = await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'runtime-branch-added-setup', + setupDecision: 'inherit', + awaitTerminalProvisioning: true + }) + + expect(worktreesChanged).toHaveBeenCalledWith(TEST_REPO_ID) + expect(result.setupReceipt).toMatchObject({ + requested: 'inherit', + hookFound: true, + state: 'skipped' + }) + expect(result.warning).toContain('orca.yaml setup hook skipped') + expect(createSetupRunnerScript).not.toHaveBeenCalled() + }) + it('materializes default tabs for inactive local managed worktree creates', async () => { const metaById: Record = {} const runtimeStore = { diff --git a/src/main/runtime/runtime-local-worktree-create.test.ts b/src/main/runtime/runtime-local-worktree-create.test.ts index c89ff71e996..87f19ab07cd 100644 --- a/src/main/runtime/runtime-local-worktree-create.test.ts +++ b/src/main/runtime/runtime-local-worktree-create.test.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { resolve } from 'node:path' import type { Store } from '../persistence' import type { WorktreeMeta } from '../../shared/worktree/meta-types' +import type { Repo } from '../../shared/repo-types' +import { getDefaultRepoHookSettings } from '../../shared/constants' import type { RuntimeManagedWorktreeCreateArgs } from './runtime-managed-worktree-create-types' import type { AddWorktreeOptions } from '../git/worktree' import { @@ -32,6 +34,7 @@ const mocks = vi.hoisted(() => ({ resolveShared: vi.fn<() => Promise>(), resolveInclude: vi.fn<() => Promise>(), copyPaths: vi.fn<() => Promise>(), + effectiveHooks: vi.fn(), created: { path: '', head: 'abc123', @@ -52,6 +55,7 @@ vi.mock('../git/repo', () => ({ getBranchConflictKind: mocks.branchConflict })) vi.mock('../git/git-username', () => ({ resolveLocalGitUsername: async () => '' })) +vi.mock('../hooks', () => ({ getEffectiveHooks: mocks.effectiveHooks })) vi.mock('../git/worktree-base-ref-probe', () => ({ hasLocalWorktreeBaseRef: mocks.hasBase })) vi.mock('./runtime-worktree-create-git', () => ({ resolveCreateBranchName: mocks.branchName, @@ -92,7 +96,8 @@ const worktreePath = resolve('/worktrees', 'app') function createWorktree( request: Partial = {}, rearm: PreparationRearmHolder = { fire: () => {} }, - timing: WorktreeCreateTimingRecorder = createWorktreeCreateTimingRecorder() + timing: WorktreeCreateTimingRecorder = createWorktreeCreateTimingRecorder(), + repoOverrides: Partial = {} ) { const store = { getSettings: () => ({ @@ -105,7 +110,14 @@ function createWorktree( } return createRuntimeLocalManagedWorktree({ request: { repoSelector: 'repo-1', name: 'app', baseBranch: 'main', ...request }, - repo: { id: 'repo-1', path: '/repo', displayName: 'Repo', badgeColor: '#000000', addedAt: 0 }, + repo: { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000000', + addedAt: 0, + ...repoOverrides + }, // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: All store methods reached by this isolated create path are supplied above. store: store as Store, createdWithAgent: undefined, @@ -149,6 +161,7 @@ beforeEach(() => { mocks.resolveShared.mockResolvedValue([]) mocks.resolveInclude.mockResolvedValue(['.env']) mocks.copyPaths.mockResolvedValue([]) + mocks.effectiveHooks.mockReturnValue(null) }) describe('runtime prepared-worktree replenishment', () => { @@ -347,3 +360,41 @@ describe('runtime create Git priority', () => { expect(mocks.consume).not.toHaveBeenCalled() }) }) + +describe('runtime create setup decision', () => { + const askRepo: Partial = { + hookSettings: { ...getDefaultRepoHookSettings(), setupRunPolicy: 'ask' } + } + + beforeEach(() => { + mocks.effectiveHooks.mockReturnValue({ scripts: { setup: 'pnpm install' } }) + }) + + it('refuses an ask repo with no decision before any git work', async () => { + const timing = createWorktreeCreateTimingRecorder() + // No requested base, so a create that got past the check would resolve the default base. + const request = { baseBranch: undefined } + await expect(createWorktree(request, undefined, timing, askRepo)).rejects.toThrow( + 'Setup decision required for this repository' + ) + // Hooks come from the main checkout (no worktree path): the new worktree doesn't exist yet. + expect(mocks.effectiveHooks.mock.calls).toEqual([[expect.objectContaining({ path: '/repo' })]]) + for (const gitWork of [mocks.defaultBase, mocks.remoteBase, mocks.hasBase, mocks.refresh]) { + expect(gitWork).not.toHaveBeenCalled() + } + expect(mocks.fetch).not.toHaveBeenCalled() + expect(mocks.branchName).not.toHaveBeenCalled() + expect(mocks.consume).not.toHaveBeenCalled() + expect(mocks.add).not.toHaveBeenCalled() + // The refusal's failure telemetry still says where the create ran. + expect(timing.finish().executionHost).toBe('local') + }) + + it.each([ + ['an explicit decision', { setupDecision: 'skip' as const }], + ['runHooks', { runHooks: true }] + ])('creates an ask repo once the request carries %s', async (_label, request) => { + await createWorktree(request, undefined, undefined, askRepo) + expect(mocks.consume).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/runtime/runtime-local-worktree-create.ts b/src/main/runtime/runtime-local-worktree-create.ts index a19a62ee29e..c78ff86d41f 100644 --- a/src/main/runtime/runtime-local-worktree-create.ts +++ b/src/main/runtime/runtime-local-worktree-create.ts @@ -1,4 +1,6 @@ import { worktreeCreateGit } from '../git/worktree-create-git-executor' +import { shouldRunSetupForCreate } from '../effective-hook-config' +import { getEffectiveHooks } from '../hooks' import type { Repo } from '../../shared/repo-types' import type { Worktree } from '../../shared/worktree/types' import type { Store } from '../persistence' @@ -19,6 +21,7 @@ import { hasLocalWorktreeBaseRef } from '../git/worktree-base-ref-probe' import { resolveRuntimeLocalWorktreeCreateCandidate } from './runtime-local-worktree-create-candidate' import { createRuntimeLocalGitWorktree } from './runtime-local-git-worktree-create' import { materializeRuntimeLocalWorktree } from './runtime-local-worktree-materialization' +import { resolveRuntimeSetupDecision } from './runtime-local-worktree-setup' import type { PreparationRearmHolder } from '../worktree-create-preparation' import { localWorktreeCreateExecutionHost, @@ -63,6 +66,11 @@ async function performRuntimeLocalWorktreeCreate(args: RuntimeLocalWorktreeCr const gitExecOptions = getLocalProjectGitExecOptions(store, repo) const worktreeGitOptions = getLocalProjectWorktreeGitOptions(store, repo) args.timing.recordExecutionHost(localWorktreeCreateExecutionHost(gitExecOptions)) + // Why before any git work: an `ask` repo with no decision must refuse with nothing created, as + // the desktop create does; checked after the add, it left an orphan worktree behind. + if (getEffectiveHooks(repo)?.scripts.setup) { + shouldRunSetupForCreate(repo, resolveRuntimeSetupDecision(request)) + } // Username and base resolution are independent read-only probes. Starting // both before awaiting removes one serial git/config round trip from create. const usernamePromise = diff --git a/src/main/runtime/runtime-local-worktree-setup.test.ts b/src/main/runtime/runtime-local-worktree-setup.test.ts new file mode 100644 index 00000000000..87716eec98f --- /dev/null +++ b/src/main/runtime/runtime-local-worktree-setup.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' +import { getDefaultRepoHookSettings } from '../../shared/constants' + +const mocks = vi.hoisted(() => ({ + effectiveHooks: vi.fn(), + runHook: vi.fn() +})) + +vi.mock('../hooks', () => ({ + getEffectiveHooks: mocks.effectiveHooks, + loadHooks: () => null, + runHook: mocks.runHook +})) + +import { prepareRuntimeLocalWorktreeSetup } from './runtime-local-worktree-setup' + +const askRepo: Repo = { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000000', + addedAt: 0, + hookSettings: { ...getDefaultRepoHookSettings(), setupRunPolicy: 'ask' } +} + +function prepare(request: { setupDecision?: 'run' | 'skip' | 'inherit' } = {}) { + return prepareRuntimeLocalWorktreeSetup({ + request: { repoSelector: 'repo-1', name: 'app', ...request }, + repo: askRepo, + worktreePath: '/worktrees/app', + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: settings are read only by the setup runner, which this in-process path never builds. + settings: {} as never, + runtimeTarget: undefined, + shouldUseSetupRunner: false + }) +} + +describe('runtime create setup for a hook the new branch added', () => { + beforeEach(() => { + vi.resetAllMocks() + mocks.runHook.mockResolvedValue({ success: true, output: '' }) + mocks.effectiveHooks.mockReturnValue({ scripts: { setup: 'pnpm install' } }) + }) + + it('skips setup with a warning instead of failing a create that already added the worktree', async () => { + const result = await prepare() + + expect(result.shouldRunSetup).toBe(false) + expect(result.warning).toContain('pass --setup run to run it') + expect(mocks.runHook).not.toHaveBeenCalled() + }) + + it('still runs setup when the caller decided to', async () => { + const result = await prepare({ setupDecision: 'run' }) + + expect(result.shouldRunSetup).toBe(true) + expect(mocks.runHook).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/runtime/runtime-local-worktree-setup.ts b/src/main/runtime/runtime-local-worktree-setup.ts index eb01b180660..f9da4312c34 100644 --- a/src/main/runtime/runtime-local-worktree-setup.ts +++ b/src/main/runtime/runtime-local-worktree-setup.ts @@ -1,4 +1,4 @@ -import type { CreateWorktreeResult } from '../../shared/worktree/create-types' +import type { CreateWorktreeResult, SetupDecision } from '../../shared/worktree/create-types' import type { Repo } from '../../shared/repo-types' import { getEffectiveHooks, loadHooks, runHook } from '../hooks' import { createSetupRunnerScript, resolveSetupRunnerShell } from '../worktree-runner-script' @@ -6,6 +6,14 @@ import { getDefaultTabsLaunch, shouldRunSetupForCreate } from '../effective-hook import type { RuntimeManagedWorktreeCreateArgs } from './runtime-managed-worktree-create-types' import type { RuntimeStore } from './runtime-store-contract' +/** Why one place: the pre-add refusal and the post-add setup must read the same decision, or a + * create the first allowed could fail the second with the worktree already on disk. */ +export function resolveRuntimeSetupDecision( + request: Pick +): SetupDecision { + return request.runHooks ? 'run' : (request.setupDecision ?? 'inherit') +} + export async function prepareRuntimeLocalWorktreeSetup(args: { request: RuntimeManagedWorktreeCreateArgs repo: Repo @@ -18,7 +26,7 @@ export async function prepareRuntimeLocalWorktreeSetup(args: { setup?: CreateWorktreeResult['setup'] defaultTabs?: CreateWorktreeResult['defaultTabs'] warning?: string - effectiveDecision: 'run' | 'skip' | 'inherit' + effectiveDecision: SetupDecision hookFound: boolean shouldRunSetup: boolean didStartInProcessSetupHook: boolean @@ -28,7 +36,7 @@ export async function prepareRuntimeLocalWorktreeSetup(args: { let setup: CreateWorktreeResult['setup'] const yamlHooks = loadHooks(worktreePath) const hooks = getEffectiveHooks(repo, worktreePath) - const effectiveDecision = request.runHooks ? 'run' : (request.setupDecision ?? 'inherit') + const effectiveDecision = resolveRuntimeSetupDecision(request) let defaultTabs: CreateWorktreeResult['defaultTabs'] try { defaultTabs = getDefaultTabsLaunch(yamlHooks, repo, effectiveDecision) @@ -38,9 +46,16 @@ export async function prepareRuntimeLocalWorktreeSetup(args: { ? { tabs: yamlHooks.defaultTabs, runCommands: false } : undefined } - const shouldRunSetup = Boolean( - hooks?.scripts.setup && shouldRunSetupForCreate(repo, effectiveDecision) - ) + let shouldRunSetup = false + if (hooks?.scripts.setup) { + try { + shouldRunSetup = shouldRunSetupForCreate(repo, effectiveDecision) + } catch { + // Why: the new branch may add a setup hook the caller never decided on; the worktree exists, + // so skip setup rather than fail the create, as the desktop create does. The skipped-hook + // branch below logs and returns the warning. + } + } let didStartInProcessSetupHook = false if (shouldRunSetup && hooks?.scripts.setup) { if (args.shouldUseSetupRunner) {