diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index 4775353df..3f24cfdd3 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -46,7 +46,8 @@ export const WORKTREE_HANDLERS: Record = { name: getRequiredStringFlag(flags, 'name'), baseBranch: getOptionalStringFlag(flags, 'base-branch'), linkedIssue: getOptionalNumberFlag(flags, 'issue'), - comment: getOptionalStringFlag(flags, 'comment') + comment: getOptionalStringFlag(flags, 'comment'), + runHooks: flags.get('run-hooks') === true }) printResult(result, json, formatWorktreeShow) }, @@ -62,7 +63,8 @@ export const WORKTREE_HANDLERS: Record = { 'worktree rm': async ({ flags, client, cwd, json }) => { const result = await client.call<{ removed: boolean }>('worktree.rm', { worktree: await getRequiredWorktreeSelector(flags, 'worktree', cwd, client), - force: flags.get('force') === true + force: flags.get('force') === true, + runHooks: flags.get('run-hooks') === true }) printResult(result, json, (value) => `removed: ${value.removed}`) } diff --git a/src/cli/help.ts b/src/cli/help.ts index 864d5e526..2e8f6ddb9 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -115,11 +115,11 @@ Common Commands: orca open [--json] orca status [--json] orca worktree list [--repo ] [--limit ] [--json] - orca worktree create --repo --name [--base-branch ] [--issue ] [--comment ] [--json] + orca worktree create --repo --name [--base-branch ] [--issue ] [--comment ] [--run-hooks] [--json] orca worktree show --worktree [--json] orca worktree current [--json] orca worktree set --worktree [--display-name ] [--issue ] [--comment ] [--json] - orca worktree rm --worktree [--force] [--json] + orca worktree rm --worktree [--force] [--run-hooks] [--json] orca worktree ps [--limit ] [--json] orca terminal list [--worktree ] [--limit ] [--json] orca terminal show [--terminal ] [--json] diff --git a/src/cli/specs/core.ts b/src/cli/specs/core.ts index 95f8f66f0..ca40b6bd8 100644 --- a/src/cli/specs/core.ts +++ b/src/cli/specs/core.ts @@ -72,9 +72,12 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ path: ['worktree', 'create'], summary: 'Create a new Orca-managed worktree', usage: - 'orca worktree create --repo --name [--base-branch ] [--issue ] [--comment ] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'repo', 'name', 'base-branch', 'issue', 'comment'], - notes: ['By default this matches the Orca UI flow and activates the new worktree in the app.'] + 'orca worktree create --repo --name [--base-branch ] [--issue ] [--comment ] [--run-hooks] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'repo', 'name', 'base-branch', 'issue', 'comment', 'run-hooks'], + notes: [ + 'By default this matches the Orca UI flow and activates the new worktree in the app.', + 'Repo-defined orca.yaml hooks are skipped unless --run-hooks is passed.' + ] }, { path: ['worktree', 'set'], @@ -86,8 +89,9 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ { path: ['worktree', 'rm'], summary: 'Remove a worktree from Orca and git', - usage: 'orca worktree rm --worktree [--force] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'force'] + usage: 'orca worktree rm --worktree [--force] [--run-hooks] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'force', 'run-hooks'], + notes: ['Repo-defined orca.yaml archive hooks are skipped unless --run-hooks is passed.'] }, { path: ['worktree', 'ps'], diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index f02e8c756..0bbba1a6a 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -524,6 +524,55 @@ describe('registerWorktreeHandlers', () => { }) }) + it('runs the archive hook on remove when skipArchive is not set', async () => { + listWorktreesMock.mockResolvedValue([]) + removeWorktreeMock.mockResolvedValue(undefined) + getEffectiveHooksMock.mockReturnValue({ + scripts: { + archive: 'echo archived' + } + }) + runHookMock.mockResolvedValue({ success: true, output: '' }) + + await handlers['worktrees:remove'](null, { + worktreeId: 'repo-1::/workspace/feature-wt' + }) + + expect(runHookMock).toHaveBeenCalledWith( + 'archive', + '/workspace/feature-wt', + expect.objectContaining({ id: 'repo-1' }) + ) + expect(removeWorktreeMock).toHaveBeenCalledWith( + '/workspace/repo', + '/workspace/feature-wt', + false + ) + }) + + it('skips the archive hook on remove when skipArchive is true', async () => { + listWorktreesMock.mockResolvedValue([]) + removeWorktreeMock.mockResolvedValue(undefined) + getEffectiveHooksMock.mockReturnValue({ + scripts: { + archive: 'echo archived' + } + }) + runHookMock.mockResolvedValue({ success: true, output: '' }) + + await handlers['worktrees:remove'](null, { + worktreeId: 'repo-1::/workspace/feature-wt', + skipArchive: true + }) + + expect(runHookMock).not.toHaveBeenCalled() + expect(removeWorktreeMock).toHaveBeenCalledWith( + '/workspace/repo', + '/workspace/feature-wt', + false + ) + }) + it('rejects ask-policy creates before mutating git state when setup decision is missing', async () => { getEffectiveHooksMock.mockReturnValue({ scripts: { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index fc2fba573..141b4c155 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -249,7 +249,7 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store ipcMain.handle( 'worktrees:remove', - async (_event, args: { worktreeId: string; force?: boolean }) => { + async (_event, args: { worktreeId: string; force?: boolean; skipArchive?: boolean }) => { const { repoId, worktreePath } = parseWorktreeId(args.worktreeId) const repo = store.getRepo(repoId) if (!repo) { @@ -273,7 +273,7 @@ export function registerWorktreeHandlers(mainWindow: BrowserWindow, store: Store // Run archive hook before removal const hooks = getEffectiveHooks(repo) - if (hooks?.scripts.archive) { + if (hooks?.scripts.archive && !args.skipArchive) { const result = await runHook('archive', worktreePath, repo) if (!result.success) { console.error(`[hooks] archive hook failed for ${worktreePath}:`, result.output) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 310a20c76..3a50b194e 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines -- Why: runtime behavior is stateful and cross-cutting, so these tests stay in one file to preserve the end-to-end invariants around handles, waits, and graph sync. */ import { afterEach, describe, expect, it, vi } from 'vitest' import type { WorktreeMeta } from '../../shared/types' -import { addWorktree, listWorktrees } from '../git/worktree' +import { addWorktree, listWorktrees, removeWorktree } from '../git/worktree' import { createSetupRunnerScript, getEffectiveHooks, runHook } from '../hooks' import { OrchestrationDb } from './orchestration/db' import { OrcaRuntimeService } from './orca-runtime' @@ -9,6 +9,7 @@ import { OrcaRuntimeService } from './orca-runtime' const { MOCK_GIT_WORKTREES, addWorktreeMock, + removeWorktreeMock, computeWorktreePathMock, ensurePathWithinWorkspaceMock, invalidateAuthorizedRootsCacheMock @@ -23,6 +24,7 @@ const { } ], addWorktreeMock: vi.fn(), + removeWorktreeMock: vi.fn(), computeWorktreePathMock: vi.fn(), ensurePathWithinWorkspaceMock: vi.fn(), invalidateAuthorizedRootsCacheMock: vi.fn() @@ -30,7 +32,8 @@ const { vi.mock('../git/worktree', () => ({ listWorktrees: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES), - addWorktree: addWorktreeMock + addWorktree: addWorktreeMock, + removeWorktree: removeWorktreeMock })) vi.mock('../hooks', () => ({ @@ -70,6 +73,7 @@ vi.mock('../git/repo', async (importOriginal) => { afterEach(() => { vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES) vi.mocked(addWorktree).mockReset() + vi.mocked(removeWorktree).mockReset() vi.mocked(createSetupRunnerScript).mockReset() vi.mocked(getEffectiveHooks).mockReset() vi.mocked(runHook).mockReset() @@ -899,7 +903,7 @@ describe('OrcaRuntimeService', () => { await expect(runtime.searchRepoRefs('id:repo-1', 'main', -5)).rejects.toThrow('invalid_limit') }) - it('returns a setup launch payload for CLI-created worktrees when orca.yaml defines setup', async () => { + it('returns a setup launch payload for CLI-created worktrees when hooks are explicitly enabled', async () => { const runtime = new OrcaRuntimeService(store) const activateWorktree = vi.fn() runtime.setNotifier({ @@ -940,7 +944,8 @@ describe('OrcaRuntimeService', () => { const result = await runtime.createManagedWorktree({ repoSelector: 'id:repo-1', - name: 'runtime-hook-test' + name: 'runtime-hook-test', + runHooks: true }) expect(createSetupRunnerScript).toHaveBeenCalledWith( @@ -973,6 +978,90 @@ describe('OrcaRuntimeService', () => { expect(activateWorktree).toHaveBeenCalledWith('repo-1', expect.any(String), result.setup) }) + it('skips setup hooks for CLI-created worktrees by default', async () => { + const runtime = new OrcaRuntimeService(store) + const activateWorktree = vi.fn() + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree, + createTerminal: vi.fn(), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn() + }) + runtime.attachWindow(1) + + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-hook-skip') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-hook-skip') + vi.mocked(getEffectiveHooks).mockReturnValue({ + scripts: { + setup: 'pnpm worktree:setup' + } + }) + vi.mocked(listWorktrees).mockResolvedValueOnce([ + { + path: '/tmp/workspaces/runtime-hook-skip', + head: 'def', + branch: 'runtime-hook-skip', + isBare: false, + isMainWorktree: false + } + ]) + + const result = await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'runtime-hook-skip' + }) + + expect(createSetupRunnerScript).not.toHaveBeenCalled() + expect(runHook).not.toHaveBeenCalled() + expect(result).toEqual({ + worktree: expect.objectContaining({ + repoId: 'repo-1', + path: '/tmp/workspaces/runtime-hook-skip', + branch: 'runtime-hook-skip' + }) + }) + expect(activateWorktree).toHaveBeenCalledWith('repo-1', expect.any(String), undefined) + }) + + it('skips archive hooks for CLI worktree removal by default', async () => { + const runtime = new OrcaRuntimeService(store) + vi.mocked(getEffectiveHooks).mockReturnValue({ + scripts: { + archive: 'pnpm worktree:archive' + } + }) + vi.mocked(removeWorktree).mockResolvedValue(undefined) + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + expect(runHook).not.toHaveBeenCalled() + expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false) + }) + + it('runs archive hooks for CLI worktree removal when hooks are explicitly enabled', async () => { + const runtime = new OrcaRuntimeService(store) + vi.mocked(getEffectiveHooks).mockReturnValue({ + scripts: { + archive: 'pnpm worktree:archive' + } + }) + vi.mocked(runHook).mockResolvedValue({ success: true, output: '' }) + vi.mocked(removeWorktree).mockResolvedValue(undefined) + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID, false, true) + + expect(runHook).toHaveBeenCalledWith( + 'archive', + TEST_WORKTREE_PATH, + expect.objectContaining({ id: TEST_REPO_ID, path: TEST_REPO_PATH }) + ) + expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false) + }) + it('invalidates the filesystem-auth cache after CLI worktree creation', async () => { // Reproduces: CLI-created worktrees fail with "Access denied: unknown // repository or worktree path" because the filesystem-auth cache was diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index b221afcff..216f21e17 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -949,6 +949,7 @@ export class OrcaRuntimeService { baseBranch?: string linkedIssue?: number | null comment?: string + runHooks?: boolean }): Promise { if (!this.store) { throw new Error('runtime_unavailable') @@ -1035,7 +1036,7 @@ export class OrcaRuntimeService { let setup: CreateWorktreeResult['setup'] const hooks = getEffectiveHooks(repo) - if (hooks?.scripts.setup) { + if (hooks?.scripts.setup && args.runHooks === true) { if (this.authoritativeWindowId !== null) { try { // Why: CLI-created worktrees must use the same runner-script path as the @@ -1056,6 +1057,9 @@ export class OrcaRuntimeService { } }) } + } else if (hooks?.scripts.setup) { + // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in. + console.info(`[hooks] setup hook skipped for ${worktreePath}; pass --run-hooks to run it`) } this.notifier?.worktreesChanged(repo.id) @@ -1101,7 +1105,11 @@ export class OrcaRuntimeService { return mergeWorktree(worktree.repoId, worktree.git, meta) } - async removeManagedWorktree(worktreeSelector: string, force = false): Promise { + async removeManagedWorktree( + worktreeSelector: string, + force = false, + runHooks = false + ): Promise { if (!this.store) { throw new Error('runtime_unavailable') } @@ -1115,11 +1123,14 @@ export class OrcaRuntimeService { } const hooks = getEffectiveHooks(repo) - if (hooks?.scripts.archive) { + if (hooks?.scripts.archive && runHooks) { const result = await runHook('archive', worktree.path, repo) if (!result.success) { console.error(`[hooks] archive hook failed for ${worktree.path}:`, result.output) } + } else if (hooks?.scripts.archive) { + // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in. + console.info(`[hooks] archive hook skipped for ${worktree.path}; pass --run-hooks to run it`) } try { diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index acc588d04..fafd86585 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -34,7 +34,8 @@ const WorktreeCreate = z.object({ .pipe(z.string().min(1, 'Missing worktree name')), baseBranch: OptionalString, linkedIssue: TriStateLinkedIssue, - comment: OptionalString + comment: OptionalString, + runHooks: OptionalBoolean }) const WorktreeSet = WorktreeSelector.extend({ @@ -44,7 +45,8 @@ const WorktreeSet = WorktreeSelector.extend({ }) const WorktreeRemove = WorktreeSelector.extend({ - force: OptionalBoolean + force: OptionalBoolean, + runHooks: OptionalBoolean }) export const WORKTREE_METHODS: RpcMethod[] = [ @@ -74,7 +76,8 @@ export const WORKTREE_METHODS: RpcMethod[] = [ name: params.name, baseBranch: params.baseBranch, linkedIssue: params.linkedIssue, - comment: params.comment + comment: params.comment, + runHooks: params.runHooks === true }) }), defineMethod({ @@ -92,7 +95,11 @@ export const WORKTREE_METHODS: RpcMethod[] = [ name: 'worktree.rm', params: WorktreeRemove, handler: async (params, { runtime }) => { - await runtime.removeManagedWorktree(params.worktree, params.force === true) + await runtime.removeManagedWorktree( + params.worktree, + params.force === true, + params.runHooks === true + ) return { removed: true } } }) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index f20d74a1c..828500265 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -342,7 +342,7 @@ export type PreloadApi = { headRefName?: string isCrossRepository?: boolean }) => Promise<{ baseBranch: string } | { error: string }> - remove: (args: { worktreeId: string; force?: boolean }) => Promise + remove: (args: { worktreeId: string; force?: boolean; skipArchive?: boolean }) => Promise updateMeta: (args: { worktreeId: string; updates: Partial }) => Promise persistSortOrder: (args: { orderedIds: string[] }) => Promise onChanged: (callback: (data: { repoId: string }) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index f85c5f359..f621d5164 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -265,7 +265,7 @@ const api = { }): Promise<{ baseBranch: string } | { error: string }> => ipcRenderer.invoke('worktrees:resolvePrBase', args), - remove: (args: { worktreeId: string; force?: boolean }): Promise => + remove: (args: { worktreeId: string; force?: boolean; skipArchive?: boolean }): Promise => ipcRenderer.invoke('worktrees:remove', args), updateMeta: (args: { diff --git a/src/renderer/src/components/sidebar/AddRepoSteps.tsx b/src/renderer/src/components/sidebar/AddRepoSteps.tsx index 7e21f4e69..438ce8585 100644 --- a/src/renderer/src/components/sidebar/AddRepoSteps.tsx +++ b/src/renderer/src/components/sidebar/AddRepoSteps.tsx @@ -113,6 +113,9 @@ export function useRemoteRepo( const state = useAppStore.getState() const existingIdx = state.repos.findIndex((r) => r.id === repo.id) + if (existingIdx !== -1) { + state.clearOrcaHookTrustForRepo(repo.id) + } if (existingIdx === -1) { useAppStore.setState({ repos: [...state.repos, repo] }) } else { diff --git a/src/renderer/src/components/sidebar/OrcaYamlTrustDialog.tsx b/src/renderer/src/components/sidebar/OrcaYamlTrustDialog.tsx new file mode 100644 index 000000000..1cbd7fefc --- /dev/null +++ b/src/renderer/src/components/sidebar/OrcaYamlTrustDialog.tsx @@ -0,0 +1,107 @@ +import React, { useCallback } from 'react' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' +import { useAppStore } from '@/store' +import type { OrcaHookScriptKind } from '@/lib/orca-hook-trust' + +type ScriptKind = OrcaHookScriptKind + +const SCRIPT_KIND_LABEL: Record = { + setup: 'setup script', + archive: 'archive script', + issueCommand: 'issue command' +} + +const SCRIPT_KIND_TRIGGER: Record = { + setup: 'when this workspace is created', + archive: 'when this workspace is removed', + issueCommand: 'when this workspace launches with a linked issue' +} + +const OrcaYamlTrustDialog = React.memo(function OrcaYamlTrustDialog() { + const activeModal = useAppStore((s) => s.activeModal) + const modalData = useAppStore((s) => s.modalData) + const closeModal = useAppStore((s) => s.closeModal) + const markOrcaHookScriptConfirmed = useAppStore((s) => s.markOrcaHookScriptConfirmed) + + const isOpen = activeModal === 'confirm-orca-yaml-hooks' + + const repoId = typeof modalData.repoId === 'string' ? modalData.repoId : '' + const repoName = typeof modalData.repoName === 'string' ? modalData.repoName : 'this repository' + const scriptKind: ScriptKind = + modalData.scriptKind === 'archive' + ? 'archive' + : modalData.scriptKind === 'issueCommand' + ? 'issueCommand' + : 'setup' + const scriptContent = typeof modalData.scriptContent === 'string' ? modalData.scriptContent : '' + const contentHash = typeof modalData.contentHash === 'string' ? modalData.contentHash : '' + const onResolve = + typeof modalData.onResolve === 'function' + ? (modalData.onResolve as (decision: 'run' | 'skip') => void) + : null + + const resolveAndClose = useCallback( + (decision: 'run' | 'skip') => { + if (decision === 'run' && repoId && contentHash) { + markOrcaHookScriptConfirmed(repoId, scriptKind, contentHash) + } + onResolve?.(decision) + closeModal() + }, + [closeModal, contentHash, markOrcaHookScriptConfirmed, onResolve, repoId, scriptKind] + ) + + const handleOpenChange = useCallback( + (open: boolean) => { + if (!open) { + resolveAndClose('skip') + } + }, + [resolveAndClose] + ) + + return ( + + + + + Run {SCRIPT_KIND_LABEL[scriptKind]} from {repoName}? + + + This repository's orca.yaml defines a {SCRIPT_KIND_LABEL[scriptKind]}{' '} + that will execute on your machine {SCRIPT_KIND_TRIGGER[scriptKind]}. Only run it if you + trust the contents of this repository. + + + + {scriptContent && ( +
+
+ {scriptKind} script +
+
+              {scriptContent}
+            
+
+ )} + + + + + +
+
+ ) +}) + +export default OrcaYamlTrustDialog diff --git a/src/renderer/src/components/sidebar/index.tsx b/src/renderer/src/components/sidebar/index.tsx index e2ea1c5e5..d29ec8282 100644 --- a/src/renderer/src/components/sidebar/index.tsx +++ b/src/renderer/src/components/sidebar/index.tsx @@ -12,6 +12,7 @@ import DeleteWorktreeDialog from './DeleteWorktreeDialog' import NonGitFolderDialog from './NonGitFolderDialog' import RemoveFolderDialog from './RemoveFolderDialog' import AddRepoDialog from './AddRepoDialog' +import OrcaYamlTrustDialog from './OrcaYamlTrustDialog' const MIN_WIDTH = 220 const MAX_WIDTH = 500 @@ -70,6 +71,7 @@ function Sidebar(): React.JSX.Element { + ) } diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 35c6054a9..de8ebd2fb 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -34,6 +34,7 @@ import { type LinkedWorkItemSummary } from '@/lib/new-workspace' import { getSuggestedCreatureName } from '@/components/sidebar/worktree-name-suggestions' +import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' export type UseComposerStateOptions = { initialRepoId?: string @@ -979,12 +980,21 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS setCreateError(null) setCreating(true) try { - const result = await createWorktree( - repoId, - workspaceName, - baseBranch, - (resolvedSetupDecision ?? 'inherit') as SetupDecision - ) + const setupTrustDecision = await ensureHooksConfirmed(useAppStore.getState(), repoId, 'setup') + const effectiveSetupDecision: SetupDecision = + setupTrustDecision === 'skip' + ? 'skip' + : ((resolvedSetupDecision ?? 'inherit') as SetupDecision) + + let issueCommandTrustDecision: 'run' | 'skip' = 'run' + if (shouldRunIssueAutomation) { + issueCommandTrustDecision = + setupTrustDecision === 'skip' + ? 'skip' + : await ensureHooksConfirmed(useAppStore.getState(), repoId, 'issueCommand') + } + + const result = await createWorktree(repoId, workspaceName, baseBranch, effectiveSetupDecision) const worktree = result.worktree await applyWorktreeMeta(worktree.id, { @@ -993,14 +1003,15 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS ...(note.trim() ? { comment: note.trim() } : {}) }) - const issueCommand = shouldRunIssueAutomation - ? { - command: renderIssueCommandTemplate(issueCommandTemplate, { - issueNumber: parsedLinkedIssueNumber, - artifactUrl: linkedWorkItem?.url ?? null - }) - } - : undefined + const issueCommand = + shouldRunIssueAutomation && issueCommandTrustDecision === 'run' + ? { + command: renderIssueCommandTemplate(issueCommandTemplate, { + issueNumber: parsedLinkedIssueNumber, + artifactUrl: linkedWorkItem?.url ?? null + }) + } + : undefined const startupPlan = buildAgentStartupPlan({ agent: tuiAgent, prompt: startupPrompt, @@ -1087,11 +1098,17 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS setCreateError(null) setCreating(true) try { + const trustDecision = await ensureHooksConfirmed(useAppStore.getState(), repoId, 'setup') + const effectiveSetupDecision: SetupDecision = + trustDecision === 'skip' + ? 'skip' + : ((resolvedSetupDecision ?? 'inherit') as SetupDecision) + const result = await createWorktree( repoId, workspaceName, baseBranch, - (resolvedSetupDecision ?? 'inherit') as SetupDecision + effectiveSetupDecision ) const worktree = result.worktree diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.test.ts b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts new file mode 100644 index 000000000..3a185897e --- /dev/null +++ b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts @@ -0,0 +1,186 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '@/store/types' +import type { PersistedTrustedOrcaHooks } from '../../../shared/types' +import { __resetTrustPromptChainForTests, ensureHooksConfirmed } from './ensure-hooks-confirmed' +import { hashOrcaHookScript } from './orca-hook-trust' + +const hooksCheckMock = vi.fn() +const readIssueCommandMock = vi.fn() + +;(globalThis as { window: unknown }).window = { + api: { + hooks: { + check: hooksCheckMock, + readIssueCommand: readIssueCommandMock + } + } +} + +type PendingPrompt = { + modal: string + data: Record + resolve: (decision: 'run' | 'skip') => void +} + +function createTestState(overrides?: Partial): { + state: AppState + pending: PendingPrompt[] +} { + const pending: PendingPrompt[] = [] + const trust: PersistedTrustedOrcaHooks = {} + const state = { + trustedOrcaHooks: trust, + repos: [{ id: 'repo-1', displayName: 'Repo One' }], + openModal: (modal: string, data: Record) => { + pending.push({ modal, data, resolve: data.onResolve as (d: 'run' | 'skip') => void }) + }, + ...overrides + } as unknown as AppState + return { state, pending } +} + +async function flush(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)) + await new Promise((resolve) => setTimeout(resolve, 0)) +} + +describe('ensureHooksConfirmed', () => { + beforeEach(() => { + hooksCheckMock.mockReset() + readIssueCommandMock.mockReset() + __resetTrustPromptChainForTests() + }) + + it('short-circuits to run when the persisted content hash matches the current script', async () => { + const { state, pending } = createTestState() + const script = 'pnpm install' + const hash = await hashOrcaHookScript(script) + state.trustedOrcaHooks['repo-1'] = { + setup: { contentHash: hash, approvedAt: 1 } + } + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: script } }, + mayNeedUpdate: false + }) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'setup') + + expect(decision).toBe('run') + expect(pending).toHaveLength(0) + }) + + it('re-prompts when the script content differs from the persisted hash', async () => { + const { state, pending } = createTestState() + const staleHash = await hashOrcaHookScript('old script') + state.trustedOrcaHooks['repo-1'] = { + setup: { contentHash: staleHash, approvedAt: 1 } + } + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'new script' } }, + mayNeedUpdate: false + }) + + const promise = ensureHooksConfirmed(state, 'repo-1', 'setup') + await flush() + + expect(pending).toHaveLength(1) + expect(pending[0].data.scriptContent).toBe('new script') + + pending[0].resolve('run') + await expect(promise).resolves.toBe('run') + }) + + it('returns run without prompting when no script of that kind is configured', async () => { + const { state, pending } = createTestState() + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: {} }, + mayNeedUpdate: false + }) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'archive') + + expect(decision).toBe('run') + expect(pending).toHaveLength(0) + }) + + it('returns run without prompting when issueCommand source is local (user-owned)', async () => { + const { state, pending } = createTestState() + readIssueCommandMock.mockResolvedValue({ + source: 'local', + sharedContent: null, + localContent: 'user content', + effectiveContent: 'user content', + localFilePath: '' + }) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'issueCommand') + + expect(decision).toBe('run') + expect(pending).toHaveLength(0) + }) + + it('opens a modal with the computed content hash and resolves with the user decision', async () => { + const { state, pending } = createTestState() + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'pnpm install' } }, + mayNeedUpdate: false + }) + + const promise = ensureHooksConfirmed(state, 'repo-1', 'setup') + await flush() + + expect(pending).toHaveLength(1) + expect(pending[0].data).toMatchObject({ + repoId: 'repo-1', + repoName: 'Repo One', + scriptKind: 'setup', + scriptContent: 'pnpm install', + contentHash: await hashOrcaHookScript('pnpm install') + }) + + pending[0].resolve('run') + await expect(promise).resolves.toBe('run') + }) + + it('serializes overlapping prompts so a second call waits for the first to resolve', async () => { + const { state, pending } = createTestState() + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'pnpm install', archive: 'echo bye' } }, + mayNeedUpdate: false + }) + + const first = ensureHooksConfirmed(state, 'repo-1', 'setup') + const second = ensureHooksConfirmed(state, 'repo-1', 'archive') + + await flush() + + expect(pending).toHaveLength(1) + expect(pending[0].data.scriptKind).toBe('setup') + + pending[0].resolve('skip') + await expect(first).resolves.toBe('skip') + + await flush() + + expect(pending).toHaveLength(2) + expect(pending[1].data.scriptKind).toBe('archive') + + pending[1].resolve('run') + await expect(second).resolves.toBe('run') + }) + + it('fails closed when window.api.hooks.check throws', async () => { + const { state, pending } = createTestState() + hooksCheckMock.mockRejectedValue(new Error('boom')) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'setup') + + expect(decision).toBe('skip') + expect(pending).toHaveLength(0) + }) +}) diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts new file mode 100644 index 000000000..276c905df --- /dev/null +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -0,0 +1,69 @@ +import type { AppState } from '@/store/types' +import type { OrcaHooks } from '../../../shared/types' +import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust' + +export type HookScriptKind = OrcaHookScriptKind + +// Serialize the singleton modal callback so overlapping worktree actions cannot replace it. +let trustPromptChain: Promise = Promise.resolve() + +function enqueueTrustPrompt(task: () => Promise): Promise { + const next = trustPromptChain.then(task, task) + trustPromptChain = next.catch(() => undefined) + return next +} + +export function __resetTrustPromptChainForTests(): void { + trustPromptChain = Promise.resolve() +} + +export async function ensureHooksConfirmed( + state: AppState, + repoId: string, + scriptKind: HookScriptKind +): Promise<'run' | 'skip'> { + return enqueueTrustPrompt(async () => { + let scriptContent = '' + try { + if (scriptKind === 'issueCommand') { + // Local overrides are user-owned; only shared orca.yaml commands need repo trust. + const result = await window.api.hooks.readIssueCommand({ repoId }) + if (result.source !== 'shared') { + return 'run' + } + scriptContent = (result.sharedContent ?? '').trim() + } else { + const result = await window.api.hooks.check({ repoId }) + const yamlHooks = (result.hooks as OrcaHooks | null) ?? null + scriptContent = (yamlHooks?.scripts?.[scriptKind] ?? '').trim() + } + } catch { + // Fail closed: if we cannot inspect the script, we cannot trust it. + return 'skip' + } + + if (!scriptContent) { + return 'run' + } + + const contentHash = await hashOrcaHookScript(scriptContent) + const existingHash = state.trustedOrcaHooks[repoId]?.[scriptKind]?.contentHash + if (existingHash === contentHash) { + return 'run' + } + + const repo = state.repos.find((r) => r.id === repoId) + const repoName = repo?.displayName ?? 'this repository' + + return new Promise<'run' | 'skip'>((resolve) => { + state.openModal('confirm-orca-yaml-hooks', { + repoId, + repoName, + scriptKind, + scriptContent, + contentHash, + onResolve: (decision: 'run' | 'skip') => resolve(decision) + }) + }) + }) +} diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index eb645d53b..decb9219d 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -11,6 +11,7 @@ import { getWorkspaceSeedName } from '@/lib/new-workspace' import { getSuggestedCreatureName } from '@/components/sidebar/worktree-name-suggestions' +import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' import type { OrcaHooks, RepoHookSettings, SetupDecision, TuiAgent } from '../../../shared/types' export type LaunchableWorkItem = { @@ -164,6 +165,10 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom return } + const trustDecision = await ensureHooksConfirmed(useAppStore.getState(), repoId, 'setup') + const finalSetupDecision: SetupDecision = + trustDecision === 'skip' ? 'skip' : setupResolution.decision + const workspaceName = getWorkspaceSeedName({ explicitName: getLinkedWorkItemSuggestedName(item), prompt: '', @@ -175,12 +180,7 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom let primaryTabId: string | null let startupPlan: ReturnType = null try { - const result = await store.createWorktree( - repoId, - workspaceName, - baseBranch, - setupResolution.decision - ) + const result = await store.createWorktree(repoId, workspaceName, baseBranch, finalSetupDecision) worktreeId = result.worktree.id const detectedIds = new Set(await detectedAgentsPromise) @@ -289,6 +289,10 @@ export async function launchFromBranch(args: LaunchFromBranchArgs): Promise { + const normalized = content.trim() + const bytes = new TextEncoder().encode(normalized) + const digest = await crypto.subtle.digest('SHA-256', bytes) + const hex: string[] = [] + const view = new Uint8Array(digest) + for (let i = 0; i < view.length; i += 1) { + hex.push(view[i].toString(16).padStart(2, '0')) + } + return hex.join('') +} diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 62bba77aa..8ab45b17b 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -69,6 +69,9 @@ export const createRepoSlice: StateCreator = (set, return null } const alreadyAdded = get().repos.some((r) => r.id === repo.id) + if (alreadyAdded) { + get().clearOrcaHookTrustForRepo(repo.id) + } set((s) => { if (s.repos.some((r) => r.id === repo.id)) { return s @@ -103,6 +106,9 @@ export const createRepoSlice: StateCreator = (set, } const repo = result.repo const alreadyAdded = get().repos.some((r) => r.id === repo.id) + if (alreadyAdded) { + get().clearOrcaHookTrustForRepo(repo.id) + } set((s) => { if (s.repos.some((r) => r.id === repo.id)) { return s @@ -139,6 +145,8 @@ export const createRepoSlice: StateCreator = (set, try { await window.api.repos.remove({ repoId }) + get().clearOrcaHookTrustForRepo(repoId) + // Kill PTYs for all worktrees belonging to this repo const worktreeIds = (get().worktreesByRepo[repoId] ?? []).map((w) => w.id) const killedTabIds = new Set() diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 6e0306139..6adc7d3ec 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -4,6 +4,7 @@ import type { AppState } from '../types' import { findPrevLiveWorktreeHistoryIndex } from './worktree-nav-history' import type { ChangelogData, + PersistedTrustedOrcaHooks, PersistedUIState, StatusBarItem, TaskViewPresetId, @@ -13,10 +14,9 @@ import type { } from '../../../../shared/types' import { PER_REPO_FETCH_LIMIT } from '../../../../shared/work-items' -// Why: mirrors the preset→query mapping in getTaskPresetQuery (new-workspace.ts). +// Why: mirrors the preset→query mapping used by TaskPage's preset buttons. // Keeping a local copy here avoids a store ↔ lib circular import while letting // openTaskPage warm exactly the cache key the page will read on mount. -// Must stay in sync with getTaskPresetQuery — see DESIGN-gh-issues-improve.md. function presetToQuery(presetId: TaskViewPresetId | null): string { switch (presetId) { case 'issues': @@ -25,10 +25,10 @@ function presetToQuery(presetId: TaskViewPresetId | null): string { return 'assignee:@me is:issue is:open' case 'prs': return 'is:pr is:open' - case 'my-prs': - return 'author:@me is:pr is:open' case 'review': return 'review-requested:@me is:pr is:open' + case 'my-prs': + return 'author:@me is:pr is:open' default: return 'is:open' } @@ -37,6 +37,7 @@ import { DEFAULT_STATUS_BAR_ITEMS, DEFAULT_WORKTREE_CARD_PROPERTIES } from '../../../../shared/constants' +import type { OrcaHookScriptKind } from '../../lib/orca-hook-trust' const MIN_SIDEBAR_WIDTH = 220 const MAX_LEFT_SIDEBAR_WIDTH = 500 @@ -46,6 +47,19 @@ const MAX_LEFT_SIDEBAR_WIDTH = 500 // corrupted/manually-edited values rather than as a product limit. const MAX_RIGHT_SIDEBAR_WIDTH = 4000 +function filterTrustedOrcaHooksToValidRepos( + trust: PersistedTrustedOrcaHooks, + validRepoIds: Set +): PersistedTrustedOrcaHooks { + const next: PersistedTrustedOrcaHooks = {} + for (const [repoId, entry] of Object.entries(trust)) { + if (validRepoIds.has(repoId)) { + next[repoId] = entry + } + } + return next +} + function sanitizePersistedSidebarWidth(width: unknown, fallback: number, maxWidth: number): number { if (typeof width !== 'number' || !Number.isFinite(width)) { return fallback @@ -131,6 +145,7 @@ export type UISlice = { | 'quick-open' | 'worktree-palette' | 'new-workspace-composer' + | 'confirm-orca-yaml-hooks' modalData: Record openModal: (modal: UISlice['activeModal'], data?: Record) => void closeModal: () => void @@ -145,6 +160,13 @@ export type UISlice = { * tab every time. */ createFromSubTab: 'prs' | 'issues' | 'branches' | 'linear' setCreateFromSubTab: (tab: 'prs' | 'issues' | 'branches' | 'linear') => void + trustedOrcaHooks: PersistedTrustedOrcaHooks + markOrcaHookScriptConfirmed: ( + repoId: string, + kind: OrcaHookScriptKind, + contentHash: string + ) => void + clearOrcaHookTrustForRepo: (repoId: string) => void searchQuery: string setSearchQuery: (q: string) => void groupBy: 'none' | 'repo' | 'pr-status' @@ -333,6 +355,33 @@ export const createUISlice: StateCreator = (set, get) createFromSubTab: 'prs', setCreateFromSubTab: (tab) => set({ createFromSubTab: tab }), + trustedOrcaHooks: {}, + markOrcaHookScriptConfirmed: (repoId, kind, contentHash) => + set((s) => { + const existing = s.trustedOrcaHooks[repoId] + const currentEntry = existing?.[kind] + if (currentEntry?.contentHash === contentHash) { + return s + } + const nextRepo = { + ...existing, + [kind]: { contentHash, approvedAt: Date.now() } + } + const next = { ...s.trustedOrcaHooks, [repoId]: nextRepo } + window.api.ui.set({ trustedOrcaHooks: next }).catch(console.error) + return { trustedOrcaHooks: next } + }), + clearOrcaHookTrustForRepo: (repoId) => + set((s) => { + if (!(repoId in s.trustedOrcaHooks)) { + return s + } + const next = { ...s.trustedOrcaHooks } + delete next[repoId] + window.api.ui.set({ trustedOrcaHooks: next }).catch(console.error) + return { trustedOrcaHooks: next } + }), + searchQuery: '', setSearchQuery: (q) => set({ searchQuery: q }), @@ -449,6 +498,10 @@ export const createUISlice: StateCreator = (set, get) updateReassuranceSeen: ui.updateReassuranceSeen ?? false, browserDefaultUrl: ui.browserDefaultUrl ?? null, browserDefaultSearchEngine: ui.browserDefaultSearchEngine ?? null, + trustedOrcaHooks: filterTrustedOrcaHooksToValidRepos( + ui.trustedOrcaHooks ?? {}, + validRepoIds + ), persistedUIReady: true } }), diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 46672086d..eeec95cd2 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -15,6 +15,9 @@ const mockApi = { }, pty: { kill: vi.fn().mockResolvedValue(undefined) + }, + hooks: { + check: vi.fn().mockResolvedValue({ hasHooks: false, hooks: null, mayNeedUpdate: false }) } } @@ -30,6 +33,9 @@ function createTestStore() { // Why: this test isolates the worktree slice, so it only provides the // state surface that `createWorktreeSlice` reads and writes. ...createWorktreeSlice(...a), + trustedOrcaHooks: {}, + repos: [], + openModal: vi.fn(), shutdownWorktreeTerminals: vi.fn().mockResolvedValue(undefined), tabsByWorktree: {}, tabBarOrderByWorktree: {}, diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index f3279d6cf..55d9cae08 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -8,6 +8,7 @@ import { getRepoIdFromWorktreeId, type WorktreeSlice } from './worktree-helpers' +import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers' function areWorktreesEqual(current: Worktree[] | undefined, next: Worktree[]): boolean { @@ -141,12 +142,16 @@ export const createWorktreeSlice: StateCreator })) try { + const repoIdForTrust = getRepoIdFromWorktreeId(worktreeId) + const trustDecision = await ensureHooksConfirmed(get(), repoIdForTrust, 'archive') + const skipArchive = trustDecision === 'skip' + // Why: setup-enabled worktrees now commonly have a live shell open as soon as // they are created. We must tear those PTYs down before asking Git to remove // the working tree or Windows and some shells can keep the directory in use // and make delete look broken even though the git state itself is fine. await get().shutdownWorktreeTerminals(worktreeId) - await window.api.worktrees.remove({ worktreeId, force }) + await window.api.worktrees.remove({ worktreeId, force, skipArchive }) const tabs = get().tabsByWorktree[worktreeId] ?? [] const tabIds = new Set(tabs.map((t) => t.id)) diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 5ab06f31a..69d4332c6 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -232,7 +232,8 @@ export function getDefaultUIState(): PersistedUIState { statusBarItems: [...DEFAULT_STATUS_BAR_ITEMS], statusBarVisible: true, dismissedUpdateVersion: null, - lastUpdateCheckAt: null + lastUpdateCheckAt: null, + trustedOrcaHooks: {} } } diff --git a/src/shared/types.ts b/src/shared/types.ts index 9b22a1f3a..d9c5d30ee 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1117,8 +1117,22 @@ export type PersistedUIState = { /** Once the user has starred Orca (from any entry point) we permanently * suppress the nag — no further thresholds, no notifications. */ starNagCompleted?: boolean + trustedOrcaHooks?: PersistedTrustedOrcaHooks } +export type PersistedTrustedOrcaHookEntry = { + contentHash: string + approvedAt: number +} + +export type PersistedTrustedOrcaHookRepo = { + setup?: PersistedTrustedOrcaHookEntry + archive?: PersistedTrustedOrcaHookEntry + issueCommand?: PersistedTrustedOrcaHookEntry +} + +export type PersistedTrustedOrcaHooks = Record + // ─── Persistence shape ────────────────────────────────────────────── export type PersistedState = { schemaVersion: number