diff --git a/src/main/hooks-setup-scripts-match.test.ts b/src/main/hooks-setup-scripts-match.test.ts new file mode 100644 index 00000000000..41338b101bd --- /dev/null +++ b/src/main/hooks-setup-scripts-match.test.ts @@ -0,0 +1,62 @@ +import type { Repo } from '../shared/types' + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('fs', () => ({ + readFileSync: vi.fn(), + existsSync: vi.fn(), + mkdirSync: vi.fn(), + writeFileSync: vi.fn(), + rmSync: vi.fn(), + chmodSync: vi.fn() +})) + +const makeRepo = () => + ({ + id: 'test-id', + path: '/test/repo', + displayName: 'Test Repo', + badgeColor: '#000', + addedAt: Date.now() + }) as unknown as Repo + +describe('setupScriptsMatch', () => { + beforeEach(() => { + vi.resetAllMocks() + }) + + it('returns true when primary and worktree setup scripts are identical', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockImplementation( + (path) => path === '/test/repo/orca.yaml' || path === '/test/worktree/orca.yaml' + ) + vi.mocked(fs.readFileSync).mockImplementation((path) => { + if (path === '/test/repo/orca.yaml' || path === '/test/worktree/orca.yaml') { + return 'scripts:\n setup: |\n pnpm install\n' + } + return '' + }) + + const { setupScriptsMatch } = await import('./hooks') + expect(setupScriptsMatch(makeRepo(), '/test/worktree')).toBe(true) + }) + + it('returns false when the worktree setup script differs from the primary script', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockImplementation( + (path) => path === '/test/repo/orca.yaml' || path === '/test/worktree/orca.yaml' + ) + vi.mocked(fs.readFileSync).mockImplementation((path) => { + if (path === '/test/repo/orca.yaml') { + return 'scripts:\n setup: |\n pnpm install\n' + } + if (path === '/test/worktree/orca.yaml') { + return 'scripts:\n setup: |\n curl https://example.com/install.sh | bash\n' + } + return '' + }) + + const { setupScriptsMatch } = await import('./hooks') + expect(setupScriptsMatch(makeRepo(), '/test/worktree')).toBe(false) + }) +}) diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index 48d6c3dc8b1..647a01dbd18 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -319,6 +319,32 @@ describe('getEffectiveHooks', () => { }) }) + it("loads setup hooks from the target worktree's orca.yaml when a worktree path is provided", async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockImplementation( + (path) => path === '/test/repo/orca.yaml' || path === '/test/worktree/orca.yaml' + ) + vi.mocked(fs.readFileSync).mockImplementation((path) => { + if (path === '/test/repo/orca.yaml') { + return 'scripts:\n setup: |\n echo old-version\n' + } + if (path === '/test/worktree/orca.yaml') { + return 'scripts:\n setup: |\n echo new-version\n' + } + return '' + }) + + const { getEffectiveHooks } = await import('./hooks') + const result = getEffectiveHooks(makeRepo(), '/test/worktree') + + expect(result).toEqual({ + scripts: { + setup: 'echo new-version' + } + }) + expect(result?.scripts.setup).not.toContain('old-version') + }) + it('falls back to legacy UI hooks when yaml is missing', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(false) diff --git a/src/main/hooks.ts b/src/main/hooks.ts index efd61d2b2e0..d83d7951312 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -261,8 +261,8 @@ function ensureOrcaDirIgnored(repoPath: string): void { } } -export function getEffectiveHooks(repo: Repo): OrcaHooks | null { - const yamlHooks = loadHooks(repo.path) +export function getEffectiveHooks(repo: Repo, worktreePath?: string): OrcaHooks | null { + const yamlHooks = loadHooks(worktreePath ?? repo.path) const legacySetup = repo.hookSettings?.scripts.setup?.trim() const legacyArchive = repo.hookSettings?.scripts.archive?.trim() const setup = yamlHooks?.scripts.setup?.trim() || legacySetup @@ -284,6 +284,15 @@ export function getEffectiveHooks(repo: Repo): OrcaHooks | null { } } +export function setupScriptsMatch( + repo: Repo, + worktreePath: string, + primarySetupScript = getEffectiveHooks(repo)?.scripts.setup +): boolean { + const worktreeSetupScript = getEffectiveHooks(repo, worktreePath)?.scripts.setup + return primarySetupScript === worktreeSetupScript +} + export function getEffectiveSetupRunPolicy(repo: Repo): SetupRunPolicy { return repo.hookSettings?.setupRunPolicy ?? getDefaultRepoHookSettings().setupRunPolicy! } @@ -304,8 +313,11 @@ export function shouldRunSetupForCreate(repo: Repo, decision: SetupDecision = 'i return policy === 'run-by-default' } -export function getSetupCommandSource(repo: Repo): { source: 'yaml'; command: string } | null { - const yamlSetup = loadHooks(repo.path)?.scripts.setup?.trim() +export function getSetupCommandSource( + repo: Repo, + worktreePath?: string +): { source: 'yaml'; command: string } | null { + const yamlSetup = loadHooks(worktreePath ?? repo.path)?.scripts.setup?.trim() if (yamlSetup) { return { source: 'yaml', command: yamlSetup } @@ -433,9 +445,10 @@ function createWorktreeRunnerScript( export function runHook( hookName: 'setup' | 'archive', cwd: string, - repo: Repo + repo: Repo, + hooksPath?: string ): Promise<{ success: boolean; output: string }> { - const hooks = getEffectiveHooks(repo) + const hooks = getEffectiveHooks(repo, hooksPath) const script = hooks?.scripts[hookName] if (!script) { diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 31867bbc6b6..79e97e339b5 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -1,6 +1,11 @@ +/* eslint-disable max-lines */ // Why: extracted from worktrees.ts to keep the main IPC module under the // max-lines threshold. Worktree creation helpers (local and remote) live -// here so the IPC dispatch file stays focused on handler wiring. +// here so the IPC dispatch file stays focused on handler wiring. The +// recently added sparse-checkout flow plus the worktree-bound setup-script +// trust gate pushed this file marginally over the per-file limit; matches +// the eslint-disable pattern other files in src/renderer use when a +// cohesive flow would split awkwardly. import type { BrowserWindow } from 'electron' import { join } from 'path' @@ -17,7 +22,12 @@ import { listWorktrees, addWorktree, addSparseWorktree } from '../git/worktree' import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo' import { gitExecFileAsync } from '../git/runner' import { isWslPath, parseWslPath, getWslHome } from '../wsl' -import { createSetupRunnerScript, getEffectiveHooks, shouldRunSetupForCreate } from '../hooks' +import { + createSetupRunnerScript, + getEffectiveHooks, + setupScriptsMatch, + shouldRunSetupForCreate +} from '../hooks' import { getSshGitProvider } from '../providers/ssh-git-dispatch' import { getActiveMultiplexer } from './ssh' import type { SshGitProvider } from '../providers/ssh-git-provider' @@ -307,11 +317,15 @@ export async function createLocalWorktree( 'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.' ) } - const setupScript = getEffectiveHooks(repo)?.scripts.setup + const primarySetupScript = getEffectiveHooks(repo)?.scripts.setup // Why: `ask` is a pre-create choice gate, not a post-create side effect. // Resolve it before mutating git state so missing UI input cannot strand - // a real worktree on disk while the renderer reports "create failed". - const shouldLaunchSetup = setupScript ? shouldRunSetupForCreate(repo, args.setupDecision) : false + // a real worktree on disk while the renderer reports "create failed". The + // actual run/skip decision is recomputed after the worktree exists, gated + // on the worktree's own setup script matching the primary's preview. + if (primarySetupScript) { + shouldRunSetupForCreate(repo, args.setupDecision) + } const sparseDirectories = args.sparseCheckout ? normalizeSparseDirectories(args.sparseCheckout.directories) : [] @@ -401,6 +415,24 @@ export async function createLocalWorktree( invalidateAuthorizedRootsCache() let setup: CreateWorktreeResult['setup'] + const setupScript = getEffectiveHooks(repo, worktreePath)?.scripts.setup + const setupMatchesPreview = setupScriptsMatch(repo, worktreePath, primarySetupScript) + let shouldLaunchSetup = false + if (setupScript && !setupMatchesPreview) { + console.warn( + `[hooks] setup hook skipped for ${worktreePath}: worktree setup script differs from the primary checkout setup script shown to the user` + ) + } else if (setupScript) { + try { + shouldLaunchSetup = shouldRunSetupForCreate(repo, args.setupDecision) + } catch (error) { + // Why: if the target branch introduces setup hooks that the primary + // checkout did not expose, the renderer may not have collected an ask + // decision. The worktree already exists, so skip setup instead of + // turning successful git creation into an IPC failure. + console.warn(`[hooks] setup hook skipped for ${worktreePath}:`, error) + } + } if (setupScript && shouldLaunchSetup) { try { // Why: setup now runs in a visible terminal owned by the renderer so users diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 3c8f6a79164..9d35ecb5765 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -17,6 +17,7 @@ const { runHookMock, hasHooksFileMock, loadHooksMock, + setupScriptsMatchMock, computeWorktreePathMock, ensurePathWithinWorkspaceMock } = vi.hoisted(() => ({ @@ -33,6 +34,7 @@ const { createIssueCommandRunnerScriptMock: vi.fn(), createSetupRunnerScriptMock: vi.fn(), shouldRunSetupForCreateMock: vi.fn(), + setupScriptsMatchMock: vi.fn(() => true), runHookMock: vi.fn(), hasHooksFileMock: vi.fn(), loadHooksMock: vi.fn(), @@ -78,7 +80,8 @@ vi.mock('../hooks', () => ({ loadHooks: loadHooksMock, runHook: runHookMock, hasHooksFile: hasHooksFileMock, - shouldRunSetupForCreate: shouldRunSetupForCreateMock + shouldRunSetupForCreate: shouldRunSetupForCreateMock, + setupScriptsMatch: setupScriptsMatchMock })) vi.mock('./worktree-logic', async (importOriginal) => { diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index f32730d0b33..53057e826b3 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -19,6 +19,7 @@ const { runHookMock, hasHooksFileMock, loadHooksMock, + setupScriptsMatchMock, computeWorktreePathMock, ensurePathWithinWorkspaceMock, gitExecFileAsyncMock @@ -37,6 +38,7 @@ const { createIssueCommandRunnerScriptMock: vi.fn(), createSetupRunnerScriptMock: vi.fn(), shouldRunSetupForCreateMock: vi.fn(), + setupScriptsMatchMock: vi.fn(() => true), runHookMock: vi.fn(), hasHooksFileMock: vi.fn(), loadHooksMock: vi.fn(), @@ -81,7 +83,8 @@ vi.mock('../hooks', () => ({ loadHooks: loadHooksMock, runHook: runHookMock, hasHooksFile: hasHooksFileMock, - shouldRunSetupForCreate: shouldRunSetupForCreateMock + shouldRunSetupForCreate: shouldRunSetupForCreateMock, + setupScriptsMatch: setupScriptsMatchMock })) vi.mock('./worktree-logic', async (importOriginal) => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 4776a555510..cd4a9ea96c3 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1036,7 +1036,10 @@ export class OrcaRuntimeService { let setup: CreateWorktreeResult['setup'] let warning: string | undefined - const hooks = getEffectiveHooks(repo) + // Why: CLI-created worktrees do not have a renderer preview to mismatch + // against. Trust is granted by the direct CLI invocation (`--run-hooks`), + // so loading the setup hook from the created worktree is intentional here. + const hooks = getEffectiveHooks(repo, worktreePath) if (hooks?.scripts.setup && args.runHooks === true) { if (this.authoritativeWindowId !== null) { try { @@ -1052,7 +1055,7 @@ export class OrcaRuntimeService { console.error(`[hooks] Failed to prepare setup runner for ${worktreePath}:`, error) } } else { - void runHook('setup', worktreePath, repo).then((result) => { + void runHook('setup', worktreePath, repo, worktreePath).then((result) => { if (!result.success) { console.error(`[hooks] setup hook failed for ${worktreePath}:`, result.output) }