diff --git a/src/main/hooks-runner.test.ts b/src/main/hooks-runner.test.ts index da98c8ddb..f8e4ffb90 100644 --- a/src/main/hooks-runner.test.ts +++ b/src/main/hooks-runner.test.ts @@ -47,7 +47,7 @@ describe('createSetupRunnerScript', () => { const { createSetupRunnerScript } = await import('./hooks') const result = createSetupRunnerScript( makeRepo(), - 'C:\\repo\\feature', + 'C:\\repo\\feature\\', 'pnpm install\npnpm build' ) @@ -55,7 +55,8 @@ describe('createSetupRunnerScript', () => { runnerScriptPath: 'C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.cmd', envVars: expect.objectContaining({ ORCA_ROOT_PATH: '/test/repo', - ORCA_WORKTREE_PATH: 'C:\\repo\\feature' + ORCA_WORKTREE_PATH: 'C:\\repo\\feature\\', + ORCA_WORKSPACE_NAME: 'feature' }) }) expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith( @@ -79,6 +80,33 @@ describe('createSetupRunnerScript', () => { } }) + it('derives ORCA_WORKSPACE_NAME from a POSIX worktree path', async () => { + const originalPlatform = process.platform + + execFileSyncMock.mockReturnValue('/test/repo/.git/worktrees/feature/orca/setup-runner.sh') + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'linux' + }) + + try { + const { createSetupRunnerScript } = await import('./hooks') + const result = createSetupRunnerScript(makeRepo(), '/test/repo-feature', 'pnpm install') + + expect(result.envVars).toEqual( + expect.objectContaining({ + ORCA_WORKTREE_PATH: '/test/repo-feature', + ORCA_WORKSPACE_NAME: 'repo-feature' + }) + ) + } finally { + Object.defineProperty(process, 'platform', { + configurable: true, + value: originalPlatform + }) + } + }) + it('translates WSL runner paths and env vars to Linux form on Windows', async () => { const fs = await import('fs') const originalPlatform = process.platform @@ -106,6 +134,7 @@ describe('createSetupRunnerScript', () => { envVars: expect.objectContaining({ ORCA_ROOT_PATH: '/mnt/c/Users/jinwo/git/orca', ORCA_WORKTREE_PATH: '/home/jin/feature', + ORCA_WORKSPACE_NAME: 'feature', CONDUCTOR_ROOT_PATH: '/mnt/c/Users/jinwo/git/orca', GHOSTX_ROOT_PATH: '/mnt/c/Users/jinwo/git/orca' }) @@ -151,6 +180,7 @@ describe('createSetupRunnerScript', () => { envVars: expect.objectContaining({ ORCA_ROOT_PATH: '/test/repo', ORCA_WORKTREE_PATH: '/home/jin/repo/feature', + ORCA_WORKSPACE_NAME: 'feature', CONDUCTOR_ROOT_PATH: '/test/repo', GHOSTX_ROOT_PATH: '/test/repo' }) diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index 647a01dbd..f8744474f 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -291,6 +291,7 @@ describe('getEffectiveHooks', () => { const makeRepo = (hookSettings?: { mode?: 'auto' | 'override' setupRunPolicy?: 'ask' | 'run-by-default' | 'skip-by-default' + commandSourcePolicy?: 'shared-only' | 'local-only' | 'run-both' scripts?: { setup: string; archive: string } }) => ({ @@ -345,26 +346,21 @@ describe('getEffectiveHooks', () => { expect(result?.scripts.setup).not.toContain('old-version') }) - it('falls back to legacy UI hooks when yaml is missing', async () => { + it('does not fall back to local settings hooks by default when yaml is missing', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(false) const { getEffectiveHooks } = await import('./hooks') const repo = makeRepo({ mode: 'override', - scripts: { setup: 'echo "legacy ui setup"', archive: 'echo "legacy archive"' } + scripts: { setup: 'echo "local setup"', archive: 'echo "local archive"' } }) const result = getEffectiveHooks(repo) - expect(result).toEqual({ - scripts: { - setup: 'echo "legacy ui setup"', - archive: 'echo "legacy archive"' - } - }) + expect(result).toBeNull() }) - it('ignores legacy UI override settings when yaml exists', async () => { + it('uses shared yaml settings over local settings by default', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(true) vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo "yaml setup"\n') @@ -383,7 +379,47 @@ describe('getEffectiveHooks', () => { }) }) - it('falls back per hook when orca.yaml defines only one command', async () => { + it('uses only local settings when command source policy is local-only', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo "yaml setup"\n') + + const { getEffectiveHooks } = await import('./hooks') + const repo = makeRepo({ + mode: 'override', + commandSourcePolicy: 'local-only', + scripts: { setup: 'echo "local setup"', archive: '' } + }) + const result = getEffectiveHooks(repo) + + expect(result).toEqual({ + scripts: { + setup: 'echo "local setup"' + } + }) + }) + + it('runs yaml before local settings when command source policy is run-both', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n setup: |\n echo "yaml setup"\n') + + const { getEffectiveHooks } = await import('./hooks') + const repo = makeRepo({ + mode: 'override', + commandSourcePolicy: 'run-both', + scripts: { setup: 'echo "local setup"', archive: '' } + }) + const result = getEffectiveHooks(repo) + + expect(result).toEqual({ + scripts: { + setup: 'echo "yaml setup"\necho "local setup"' + } + }) + }) + + it('treats orca.yaml as authoritative by default when it defines only one command', async () => { const fs = await import('fs') vi.mocked(fs.existsSync).mockReturnValue(true) vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n archive: |\n echo "yaml archive"\n') @@ -397,7 +433,26 @@ describe('getEffectiveHooks', () => { expect(result).toEqual({ scripts: { - setup: 'echo "legacy setup"', + archive: 'echo "yaml archive"' + } + }) + }) + + it('treats legacy shared-first policy as orca.yaml only', async () => { + const fs = await import('fs') + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.readFileSync).mockReturnValue('scripts:\n archive: |\n echo "yaml archive"\n') + + const { getEffectiveHooks } = await import('./hooks') + const repo = makeRepo({ + mode: 'override', + commandSourcePolicy: 'shared-first' as never, + scripts: { setup: 'echo "legacy setup"', archive: 'echo "legacy archive"' } + }) + const result = getEffectiveHooks(repo) + + expect(result).toEqual({ + scripts: { archive: 'echo "yaml archive"' } }) diff --git a/src/main/hooks.ts b/src/main/hooks.ts index 0beb03a64..32c9eb540 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -3,9 +3,12 @@ import { readFileSync, existsSync, mkdirSync, writeFileSync, chmodSync, rmSync } import { dirname, join } from 'path' import { exec, execFile } from 'child_process' import { getDefaultRepoHookSettings } from '../shared/constants' +import { getRuntimePathBasename } from '../shared/cross-platform-path' +import { normalizeHookCommandSourcePolicy } from '../shared/hook-command-source-policy' import { gitExecFileSync } from './git/runner' import { isWslPath, parseWslPath, toWindowsWslPath, toLinuxPath } from './wsl' import type { + HookCommandSourcePolicy, OrcaHooks, Repo, SetupDecision, @@ -261,21 +264,40 @@ function ensureOrcaDirIgnored(repoPath: string): void { } } +function getEffectiveHookScript( + yamlScript: string | undefined, + localScript: string | undefined, + policy: HookCommandSourcePolicy +): string | undefined { + const shared = yamlScript?.trim() + const local = localScript?.trim() + + if (policy === 'local-only') { + return local || undefined + } + + if (policy === 'run-both') { + return [shared, local].filter(Boolean).join('\n') || undefined + } + + return shared || undefined +} + 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 - const archive = yamlHooks?.scripts.archive?.trim() || legacyArchive + const localSetup = repo.hookSettings?.scripts.setup + const localArchive = repo.hookSettings?.scripts.archive + const policy = normalizeHookCommandSourcePolicy(repo.hookSettings?.commandSourcePolicy) + const setup = getEffectiveHookScript(yamlHooks?.scripts.setup, localSetup, policy) + const archive = getEffectiveHookScript(yamlHooks?.scripts.archive, localArchive, policy) if (!setup && !archive) { return null } - // Why: `orca.yaml` is the preferred source going forward, but existing users may - // still have setup/archive commands persisted only in repo settings. Resolve each - // hook independently so a repo that has only migrated one command into `orca.yaml` - // does not silently lose the other legacy hook until the migration is complete. + // Why: committed `orca.yaml` and local Settings commands can intentionally + // coexist, but the source policy defines whether the committed file is an + // authoritative boundary, local settings are authoritative, or both run. return { scripts: { ...(setup ? { setup } : {}), @@ -307,8 +329,18 @@ export function shouldRunSetupForCreate(repo: Repo, decision: SetupDecision = 'i export function getSetupCommandSource( repo: Repo, worktreePath?: string -): { source: 'yaml'; command: string } | null { +): { source: 'yaml' | 'local' | 'both'; command: string } | null { const yamlSetup = loadHooks(worktreePath ?? repo.path)?.scripts.setup?.trim() + const localSetup = repo.hookSettings?.scripts.setup?.trim() + const policy = normalizeHookCommandSourcePolicy(repo.hookSettings?.commandSourcePolicy) + + if (policy === 'local-only') { + return localSetup ? { source: 'local', command: localSetup } : null + } + + if (policy === 'run-both' && yamlSetup && localSetup) { + return { source: 'both', command: `${yamlSetup}\n${localSetup}` } + } if (yamlSetup) { return { source: 'yaml', command: yamlSetup } @@ -321,6 +353,7 @@ function getSetupEnvVars(repo: Repo, worktreePath: string): Record void note: string onNoteChange: (value: string) => void - setupConfig: { source: 'yaml' | 'legacy'; command: string } | null + setupConfig: SetupConfig | null requiresExplicitSetupChoice: boolean setupDecision: 'run' | 'skip' | null onSetupDecisionChange: (value: 'run' | 'skip') => void @@ -96,7 +97,7 @@ function SetupCommandPreview({ setupConfig, headerAction }: { - setupConfig: { source: 'yaml' | 'legacy'; command: string } + setupConfig: SetupConfig headerAction?: React.ReactNode }): React.JSX.Element { if (setupConfig.source === 'yaml') { @@ -117,7 +118,7 @@ function SetupCommandPreview({
- Legacy setup command + {setupConfig.source === 'both' ? 'Combined setup command' : 'Local setup command'}
{headerAction}
@@ -519,7 +520,11 @@ export default function NewWorkspaceComposerCard({ Setup script - {setupConfig.source === 'yaml' ? 'orca.yaml' : 'legacy hooks'} + {setupConfig.source === 'yaml' + ? 'orca.yaml' + : setupConfig.source === 'both' + ? 'orca.yaml + local' + : 'local settings'}
diff --git a/src/renderer/src/components/settings/RepositoryHooksSection.test.ts b/src/renderer/src/components/settings/RepositoryHooksSection.test.ts new file mode 100644 index 000000000..0f2900706 --- /dev/null +++ b/src/renderer/src/components/settings/RepositoryHooksSection.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import { + commandRowsToScript, + localCommandDraftToScripts, + scriptToCommandRows, + type LocalCommandDraft, + type LocalCommandRow +} from './RepositoryHooksSection' + +describe('RepositoryHooksSection command row serialization', () => { + it('round-trips blank lines and trailing whitespace in existing scripts', () => { + const script = 'echo before \n\ncat < { + const rows: LocalCommandRow[] = [ + ...scriptToCommandRows('echo before\n\n echo after '), + { value: '', isPlaceholder: true } + ] + + expect(commandRowsToScript(rows)).toBe('echo before\n\n echo after ') + }) + + it('serializes local command drafts with the same placeholder pruning used by commits', () => { + const draft: LocalCommandDraft = { + setup: [...scriptToCommandRows('echo setup\n'), { value: '', isPlaceholder: true }], + archive: [ + { value: '', isPlaceholder: false }, + { value: 'echo archive', isPlaceholder: false }, + { value: '', isPlaceholder: true } + ] + } + + expect(localCommandDraftToScripts(draft)).toEqual({ + setup: 'echo setup\n', + archive: '\necho archive' + }) + }) +}) diff --git a/src/renderer/src/components/settings/RepositoryHooksSection.tsx b/src/renderer/src/components/settings/RepositoryHooksSection.tsx index 6e6cbf23f..23d00b55d 100644 --- a/src/renderer/src/components/settings/RepositoryHooksSection.tsx +++ b/src/renderer/src/components/settings/RepositoryHooksSection.tsx @@ -1,12 +1,21 @@ /* eslint-disable max-lines -- Why: the YAML status card, issue-command editor, policy grid, and legacy-hook section form one cohesive settings surface; splitting them across files would scatter tightly coupled state and prop drilling. */ import { useCallback, useEffect, useRef, useState } from 'react' -import type { OrcaHooks, Repo, SetupRunPolicy } from '../../../../shared/types' -import { AlertTriangle } from 'lucide-react' +import type { + HookCommandSourcePolicy, + OrcaHooks, + Repo, + RepoHookSettings, + SetupRunPolicy +} from '../../../../shared/types' +import { AlertTriangle, Plus, Trash2 } from 'lucide-react' import { toast } from 'sonner' import { Button } from '../ui/button' +import { Input } from '../ui/input' import { SearchableSetting } from './SearchableSetting' import { useAppStore } from '@/store' import { readRuntimeIssueCommand, writeRuntimeIssueCommand } from '@/runtime/runtime-hooks-client' +import { DEFAULT_REPO_HOOK_SETTINGS } from './SettingsConstants' +import { normalizeHookCommandSourcePolicy } from '../../../../shared/hook-command-source-policy' type RepositoryHooksSectionProps = { repo: Repo @@ -15,11 +24,17 @@ type RepositoryHooksSectionProps = { mayNeedUpdate: boolean copiedTemplate: boolean onCopyTemplate: () => void - onClearLegacyHooks: () => void - onUpdateSetupRunPolicy: (policy: SetupRunPolicy) => void + onUpdateHookSettings: (settings: RepoHookSettings) => void } type PolicyOption

= { policy: P; label: string; description: string } +export type LocalCommandRow = { value: string; isPlaceholder: boolean } +const LOCAL_HOOK_NAMES = ['setup', 'archive'] as const +type LocalHookName = (typeof LOCAL_HOOK_NAMES)[number] +export type LocalCommandDraft = Record +type HookSettingsPolicyDraft = Partial< + Pick +> const SETUP_RUN_POLICY_OPTIONS: PolicyOption[] = [ { policy: 'ask', label: 'Ask every time', description: 'Prompt before running setup.' }, @@ -31,6 +46,92 @@ const SETUP_RUN_POLICY_OPTIONS: PolicyOption[] = [ } ] +const COMMAND_SOURCE_POLICY_OPTIONS: PolicyOption[] = [ + { + policy: 'shared-only', + label: 'Use orca.yaml only', + description: 'Run only committed repo commands; ignore local Settings commands.' + }, + { + policy: 'local-only', + label: 'Use local only', + description: 'Ignore repo commands and run only your local Settings commands.' + }, + { + policy: 'run-both', + label: 'Run both', + description: 'Run orca.yaml first, then your local Settings command.' + } +] + +const LOCAL_HOOK_FIELDS: { + name: LocalHookName + label: string + description: string + placeholder: string +}[] = [ + { + name: 'setup', + label: 'Local setup command', + description: 'Runs after a new workspace is created when the source policy includes local.', + placeholder: 'cp "$ORCA_ROOT_PATH/.env" "$ORCA_WORKTREE_PATH/.env"' + }, + { + name: 'archive', + label: 'Local archive command', + description: 'Runs before a local worktree is archived or removed.', + placeholder: 'echo "Cleaning up $ORCA_WORKSPACE_NAME"' + } +] + +export function scriptToCommandRows(script: string | undefined): LocalCommandRow[] { + if (!script) { + return [] + } + + return script.split('\n').map((line) => ({ + value: line.endsWith('\r') ? line.slice(0, -1) : line, + isPlaceholder: false + })) +} + +export function commandRowsToScript(commands: LocalCommandRow[]): string { + return commands + .filter((command) => !(command.isPlaceholder && command.value.length === 0)) + .map((command) => command.value) + .join('\n') +} + +function pruneLocalCommandPlaceholders(commands: LocalCommandRow[]): LocalCommandRow[] { + return commands.filter((command) => !(command.isPlaceholder && command.value.length === 0)) +} + +export function localCommandDraftToScripts(draft: LocalCommandDraft): RepoHookSettings['scripts'] { + return { + setup: commandRowsToScript(pruneLocalCommandPlaceholders(draft.setup)), + archive: commandRowsToScript(pruneLocalCommandPlaceholders(draft.archive)) + } +} + +function getHookSettingsDraft(hookSettings: Repo['hookSettings']): RepoHookSettings { + return { + ...DEFAULT_REPO_HOOK_SETTINGS, + ...hookSettings, + scripts: { + ...DEFAULT_REPO_HOOK_SETTINGS.scripts, + ...hookSettings?.scripts + } + } +} + +function getLocalCommandsDraft(hookSettings: Repo['hookSettings']): LocalCommandDraft { + const draft = getHookSettingsDraft(hookSettings) + return { + setup: scriptToCommandRows(draft.scripts.setup), + archive: scriptToCommandRows(draft.scripts.archive) + } +} + const EXAMPLE_TEMPLATE = `scripts: setup: | pnpm worktree:setup @@ -153,8 +254,7 @@ export function RepositoryHooksSection({ mayNeedUpdate, copiedTemplate, onCopyTemplate, - onClearLegacyHooks, - onUpdateSetupRunPolicy + onUpdateHookSettings }: RepositoryHooksSectionProps): React.JSX.Element { const settings = useAppStore((s) => s.settings) // Why: distinguish "file has unrecognised top-level keys" from "file is @@ -167,14 +267,31 @@ export function RepositoryHooksSection({ ? 'update-available' : 'invalid' : 'missing' - const hs = repo.hookSettings - const legacyHookEntries = (['setup', 'archive'] as const) - .map((hookName) => [hookName, hs?.scripts[hookName]?.trim() ?? ''] as const) + const [hookSettingsDraft, setHookSettingsDraft] = useState(() => + getHookSettingsDraft(repo.hookSettings) + ) + const hookSettingsDraftRef = useRef(hookSettingsDraft) + hookSettingsDraftRef.current = hookSettingsDraft + const [localCommandsDraft, setLocalCommandsDraft] = useState(() => + getLocalCommandsDraft(repo.hookSettings) + ) + const localCommandsDraftRef = useRef(localCommandsDraft) + localCommandsDraftRef.current = localCommandsDraft + const localCommandsRepoHookSettingsRef = useRef(repo.hookSettings) + const localCommandsDraftDirtyRef = useRef(false) + const localCommandsPersistForRepoRef = useRef(onUpdateHookSettings) + const localHookEntries = (['setup', 'archive'] as const) + .map((hookName) => [hookName, hookSettingsDraft.scripts[hookName] ?? ''] as const) .filter(([, script]) => Boolean(script)) // Why: the type allows `undefined` in persisted settings for backward compatibility, // but the UI always needs a concrete value so the policy grid has an active selection. - const selectedSetupRunPolicy: SetupRunPolicy = hs?.setupRunPolicy ?? 'run-by-default' + const selectedSetupRunPolicy: SetupRunPolicy = + hookSettingsDraft.setupRunPolicy ?? 'run-by-default' + const selectedCommandSourcePolicy: HookCommandSourcePolicy = normalizeHookCommandSourcePolicy( + hookSettingsDraft.commandSourcePolicy + ) const [issueCommandDraft, setIssueCommandDraft] = useState('') + const localCommandsRepoIdRef = useRef(repo.id) const [hasSharedIssueCommand, setHasSharedIssueCommand] = useState(false) const [issueCommandSaveError, setIssueCommandSaveError] = useState(null) // Why: track the latest draft across blur/unmount so repo switches still @@ -183,6 +300,122 @@ export function RepositoryHooksSection({ issueCommandDraftRef.current = issueCommandDraft const lastCommittedIssueCommandRef = useRef('') + localCommandsRepoHookSettingsRef.current = repo.hookSettings + + const setAndMaybePersistHookSettings = useCallback( + (nextSettings: RepoHookSettings, shouldPersist: boolean) => { + hookSettingsDraftRef.current = nextSettings + setHookSettingsDraft(nextSettings) + if (shouldPersist) { + localCommandsDraftDirtyRef.current = false + onUpdateHookSettings(nextSettings) + } + }, + [onUpdateHookSettings] + ) + + const updateLocalCommandsDraft = useCallback( + (hookName: LocalHookName, commands: LocalCommandRow[], shouldPersist: boolean) => { + const nextCommandsDraft = { ...localCommandsDraftRef.current, [hookName]: commands } + localCommandsDraftRef.current = nextCommandsDraft + setLocalCommandsDraft(nextCommandsDraft) + if (!shouldPersist) { + localCommandsDraftDirtyRef.current = true + } + + const nextSettings = { + ...hookSettingsDraftRef.current, + scripts: { + ...hookSettingsDraftRef.current.scripts, + [hookName]: commandRowsToScript(commands) + } + } + setAndMaybePersistHookSettings(nextSettings, shouldPersist) + }, + [setAndMaybePersistHookSettings] + ) + + const commitLocalCommandsDraft = useCallback( + (hookName: LocalHookName) => { + // Why: Add Command creates an unsaved empty editor row. Existing blank script + // lines are real rows and must round-trip, so only placeholder blanks are pruned. + const next = pruneLocalCommandPlaceholders(localCommandsDraftRef.current[hookName]) + updateLocalCommandsDraft(hookName, next, true) + }, + [updateLocalCommandsDraft] + ) + + const flushDirtyLocalCommandsDraft = useCallback( + (persistHookSettings: (settings: RepoHookSettings) => void) => { + if (!localCommandsDraftDirtyRef.current) { + return + } + + const nextSettings = { + ...hookSettingsDraftRef.current, + scripts: { + ...hookSettingsDraftRef.current.scripts, + ...localCommandDraftToScripts(localCommandsDraftRef.current) + } + } + hookSettingsDraftRef.current = nextSettings + localCommandsDraftDirtyRef.current = false + persistHookSettings(nextSettings) + }, + [] + ) + + const updateHookSettingsPolicyDraft = useCallback( + (updates: HookSettingsPolicyDraft) => { + const nextSettings = { + ...hookSettingsDraftRef.current, + ...updates + } + setAndMaybePersistHookSettings(nextSettings, true) + }, + [setAndMaybePersistHookSettings] + ) + + const handleClearLocalCommands = useCallback(() => { + const nextCommandsDraft = { setup: [], archive: [] } + localCommandsDraftRef.current = nextCommandsDraft + setLocalCommandsDraft(nextCommandsDraft) + const nextSettings = { + ...hookSettingsDraftRef.current, + scripts: { + ...hookSettingsDraftRef.current.scripts, + setup: '', + archive: '' + } + } + setAndMaybePersistHookSettings(nextSettings, true) + }, [setAndMaybePersistHookSettings]) + + useEffect(() => { + if (localCommandsRepoIdRef.current === repo.id) { + localCommandsPersistForRepoRef.current = onUpdateHookSettings + return + } + // Why: repo switches reset the local editor state before inputs can blur, + // so flush dirty row drafts through the previous repo's captured updater. + flushDirtyLocalCommandsDraft(localCommandsPersistForRepoRef.current) + localCommandsRepoIdRef.current = repo.id + const nextSettingsDraft = getHookSettingsDraft(localCommandsRepoHookSettingsRef.current) + const nextCommandsDraft = getLocalCommandsDraft(localCommandsRepoHookSettingsRef.current) + hookSettingsDraftRef.current = nextSettingsDraft + localCommandsDraftRef.current = nextCommandsDraft + localCommandsDraftDirtyRef.current = false + localCommandsPersistForRepoRef.current = onUpdateHookSettings + setHookSettingsDraft(nextSettingsDraft) + setLocalCommandsDraft(nextCommandsDraft) + }, [flushDirtyLocalCommandsDraft, onUpdateHookSettings, repo.id]) + + useEffect(() => { + return () => { + flushDirtyLocalCommandsDraft(localCommandsPersistForRepoRef.current) + } + }, [flushDirtyLocalCommandsDraft]) + // Keep the local override editor in sync with the selected repo and flush unsaved edits on exit. useEffect(() => { let cancelled = false @@ -244,8 +477,8 @@ export function RepositoryHooksSection({

Worktree Hooks

- Orca prefers shared hooks from `orca.yaml` and still honors older repo-local hook scripts - until you clear them. + Configure shared repo hooks from `orca.yaml` and personal commands stored locally on this + machine.

@@ -326,45 +559,156 @@ export function RepositoryHooksSection({ - {legacyHookEntries.length > 0 ? ( - -
-
-
-
- Legacy Repo-Local Hooks -
-

- These older commands still run as a fallback when `orca.yaml` does not provide a - hook. Clear them after you migrate the behavior into `orca.yaml`. -

-
- + +
+
+
+
Local Settings Commands
+

+ Stored in Orca on this machine. These commands are not committed to the repository. +

+ {localHookEntries.length > 0 ? ( + + ) : null} +
- {legacyHookEntries.map(([hookName, script]) => ( -
+ {['$ORCA_ROOT_PATH', '$ORCA_WORKTREE_PATH', '$ORCA_WORKSPACE_NAME'].map((name) => ( + -
-

{hookName}

- Compatibility fallback -
- 
-                  {script}
-
-
+ {name} + ))}
- - ) : null} + +
+ {LOCAL_HOOK_FIELDS.map((field) => { + const commands = localCommandsDraft[field.name] + return ( +
+
+
+ +

{field.description}

+
+ +
+ +
+ {commands.length === 0 ? ( +
+ No local {field.name} commands configured. +
+ ) : ( +
+ {commands.map((command, index) => ( +
+ + {index + 1} + + { + const next = [...commands] + next[index] = { + value: event.target.value, + isPlaceholder: false + } + updateLocalCommandsDraft(field.name, next, false) + }} + onBlur={() => commitLocalCommandsDraft(field.name)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + event.preventDefault() + updateLocalCommandsDraft( + field.name, + [ + ...commands.slice(0, index + 1), + { value: '', isPlaceholder: true }, + ...commands.slice(index + 1) + ], + false + ) + } + }} + placeholder={index === 0 ? field.placeholder : 'Command'} + className="h-8 font-mono text-xs" + /> + +
+ ))} +
+ )} +
+
+ ) + })} +
+
+ + + +
+
+
Command Source
+

+ Choose whether Orca runs commands from `orca.yaml`, local Settings, or both. +

+
+ + updateHookSettingsPolicyDraft({ commandSourcePolicy: policy })} + columns="md:grid-cols-3" + /> +
+ updateHookSettingsPolicyDraft({ setupRunPolicy: policy })} columns="md:grid-cols-3" />
diff --git a/src/renderer/src/components/settings/RepositoryPane.tsx b/src/renderer/src/components/settings/RepositoryPane.tsx index 297963ced..3934fabf8 100644 --- a/src/renderer/src/components/settings/RepositoryPane.tsx +++ b/src/renderer/src/components/settings/RepositoryPane.tsx @@ -1,5 +1,5 @@ import { useState } from 'react' -import type { OrcaHooks, Repo, RepoHookSettings, SetupRunPolicy } from '../../../../shared/types' +import type { OrcaHooks, Repo, RepoHookSettings } from '../../../../shared/types' import { getRepoKindLabel, isFolderRepo } from '../../../../shared/repo-kind' import { REPO_COLORS } from '../../../../shared/constants' import { Button } from '../ui/button' @@ -7,7 +7,6 @@ import { Input } from '../ui/input' import { Label } from '../ui/label' import { Separator } from '../ui/separator' import { Trash2 } from 'lucide-react' -import { DEFAULT_REPO_HOOK_SETTINGS } from './SettingsConstants' import { BaseRefPicker } from './BaseRefPicker' import { RepositoryHooksSection } from './RepositoryHooksSection' import { McpConfigSection } from './McpConfigSection' @@ -103,9 +102,23 @@ export function getRepositoryPaneSearchEntries(repo: Repo): SettingsSearchEntry[ keywords: [repo.displayName, 'hooks', 'setup', 'archive', 'yaml'] }, { - title: 'Legacy Repo-Local Hooks', - description: 'Older setup and archive hook scripts stored in local repo settings.', - keywords: [repo.displayName, 'legacy', 'fallback', 'hooks'] + title: 'Local Settings Commands', + description: 'Personal setup and archive commands stored locally on this machine.', + keywords: [repo.displayName, 'local', 'personal', 'hooks'] + }, + { + title: 'Command Source', + description: + 'Choose whether Orca runs commands from `orca.yaml`, local Settings, or both.', + keywords: [ + repo.displayName, + 'local', + 'orca.yaml', + 'shared', + 'both', + 'source', + 'authoritative' + ] }, { title: 'When to Run Setup', @@ -160,18 +173,7 @@ export function RepositoryPane({ setConfirmingRemove(repoId) } - const updateSelectedRepoHookSettings = ( - updates: Partial> - ) => { - // Why: persisted repos may still carry legacy UI hook fields from the old dual-source - // design. We preserve them when saving so existing local state stays loadable, but the - // product now treats `orca.yaml` as the only supported hook definition surface. - const nextSettings: RepoHookSettings = { - ...DEFAULT_REPO_HOOK_SETTINGS, - ...repo.hookSettings, - ...updates - } - + const updateSelectedRepoHookSettings = (nextSettings: RepoHookSettings) => { updateRepo(repo.id, { hookSettings: nextSettings }) @@ -189,22 +191,6 @@ export function RepositoryPane({ window.setTimeout(() => setCopiedTemplate(false), 1500) } - const handleClearLegacyHooks = () => { - // Why: legacy repo-local commands are still honored as a compatibility fallback. - // Keep them visible and removable here so the settings surface matches runtime behavior. - updateRepo(repo.id, { - hookSettings: { - ...DEFAULT_REPO_HOOK_SETTINGS, - ...repo.hookSettings, - scripts: { - ...DEFAULT_REPO_HOOK_SETTINGS.scripts, - setup: '', - archive: '' - } - } - }) - } - const allEntries = getRepositoryPaneSearchEntries(repo) const identityEntries = allEntries.filter((entry) => ['Display Name', 'Badge Color', 'Default Worktree Base', 'Remove Repo'].includes(entry.title) @@ -215,7 +201,8 @@ export function RepositoryPane({ const hooksEntries = allEntries.filter((entry) => [ 'orca.yaml hooks', - 'Legacy Repo-Local Hooks', + 'Local Settings Commands', + 'Command Source', 'When to Run Setup', 'Custom GitHub Issue Command' ].includes(entry.title) @@ -340,10 +327,7 @@ export function RepositoryPane({ mayNeedUpdate={mayNeedUpdate} copiedTemplate={copiedTemplate} onCopyTemplate={() => void handleCopyTemplate()} - onClearLegacyHooks={handleClearLegacyHooks} - onUpdateSetupRunPolicy={(policy) => - updateSelectedRepoHookSettings({ setupRunPolicy: policy as SetupRunPolicy }) - } + onUpdateHookSettings={updateSelectedRepoHookSettings} /> ) : null ].filter(Boolean) diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index efeadc8c1..214ded3f4 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -46,7 +46,8 @@ import { getWorkspaceSeedName, PER_REPO_FETCH_LIMIT, renderIssueCommandTemplate, - type LinkedWorkItemSummary + type LinkedWorkItemSummary, + type SetupConfig } from '@/lib/new-workspace' import { getFullComposerCreateDisabled, @@ -181,7 +182,7 @@ export type ComposerCardProps = { /** Transient inline hint shown next to the Start-from trigger after a repo * switch resets a prior selection (e.g. "was PR #8778"). Null when none. */ startFromResetHint: string | null - setupConfig: { source: 'yaml' | 'legacy'; command: string } | null + setupConfig: SetupConfig | null requiresExplicitSetupChoice: boolean setupDecision: 'run' | 'skip' | null onSetupDecisionChange: (value: 'run' | 'skip') => void diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.test.ts b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts index cf2546f18..d1eee7718 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.test.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts @@ -120,6 +120,33 @@ describe('ensureHooksConfirmed', () => { expect(pending).toHaveLength(0) }) + it('does not prompt for orca.yaml when the repo uses local commands only', async () => { + const { state, pending } = createTestState({ + repos: [ + { + id: 'repo-1', + displayName: 'Repo One', + hookSettings: { + mode: 'auto', + commandSourcePolicy: 'local-only', + scripts: { setup: 'echo local', archive: '' } + } + } + ] + } as Partial) + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'echo shared' } }, + mayNeedUpdate: false + }) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'setup') + + expect(decision).toBe('run') + expect(hooksCheckMock).not.toHaveBeenCalled() + expect(pending).toHaveLength(0) + }) + it('returns run without prompting when issueCommand source is local (user-owned)', async () => { const { state, pending } = createTestState() readIssueCommandMock.mockResolvedValue({ diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts index eeb1f4a71..5232e8a56 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -38,6 +38,10 @@ export async function ensureHooksConfirmed( } scriptContent = (result.sharedContent ?? '').trim() } else { + const repo = state.repos.find((r) => r.id === repoId) + if (repo?.hookSettings?.commandSourcePolicy === 'local-only') { + return 'run' + } const result = await checkRuntimeHooks(state.settings, repoId) const yamlHooks = (result.hooks as OrcaHooks | null) ?? null scriptContent = (yamlHooks?.scripts?.[scriptKind] ?? '').trim() diff --git a/src/renderer/src/lib/new-workspace.ts b/src/renderer/src/lib/new-workspace.ts index 2fb5f3727..06a534ab9 100644 --- a/src/renderer/src/lib/new-workspace.ts +++ b/src/renderer/src/lib/new-workspace.ts @@ -7,6 +7,7 @@ import { import type { AgentStartupPlan } from '@/lib/tui-agent-startup' import { isShellProcess } from '@/lib/tui-agent-startup' import type { OrcaHooks, TaskViewPresetId } from '../../../shared/types' +import { normalizeHookCommandSourcePolicy } from '../../../shared/hook-command-source-policy' /** * Why: the TaskPage's preset buttons and the openTaskPage prefetcher both need @@ -61,6 +62,8 @@ export type LinkedWorkItemSummary = { // is the minimum viable instruction that always produces a coherent agent task. export const DEFAULT_ISSUE_COMMAND_TEMPLATE = 'Complete {{artifact_url}}' +export type SetupConfig = { source: 'yaml' | 'local' | 'both'; command: string } + /** * Substitute the issue-command template variables. Prefers `{{artifact_url}}` * and keeps `{{issue}}` working silently for repos that have not migrated @@ -115,17 +118,31 @@ export function getAttachmentLabel(pathValue: string): string { } export function getSetupConfig( - repo: { hookSettings?: { scripts?: { setup?: string } } } | undefined, + repo: + | { + hookSettings?: { + commandSourcePolicy?: unknown + scripts?: { setup?: string } + } + } + | undefined, yamlHooks: OrcaHooks | null -): { source: 'yaml' | 'legacy'; command: string } | null { +): SetupConfig | null { const yamlSetup = yamlHooks?.scripts?.setup?.trim() + const localSetup = repo?.hookSettings?.scripts?.setup?.trim() + const sourcePolicy = normalizeHookCommandSourcePolicy(repo?.hookSettings?.commandSourcePolicy) + + if (sourcePolicy === 'local-only') { + return localSetup ? { source: 'local', command: localSetup } : null + } + + if (sourcePolicy === 'run-both' && yamlSetup && localSetup) { + return { source: 'both', command: `${yamlSetup}\n${localSetup}` } + } + if (yamlSetup) { return { source: 'yaml', command: yamlSetup } } - const legacySetup = repo?.hookSettings?.scripts?.setup?.trim() - if (legacySetup) { - return { source: 'legacy', command: legacySetup } - } return null } diff --git a/src/renderer/src/store/slices/repos-update-serialization.test.ts b/src/renderer/src/store/slices/repos-update-serialization.test.ts new file mode 100644 index 000000000..4f536be66 --- /dev/null +++ b/src/renderer/src/store/slices/repos-update-serialization.test.ts @@ -0,0 +1,130 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createTestStore } from './store-test-helpers' +import type { Repo } from '../../../../shared/types' + +const localRepo: Repo = { + id: 'local-repo', + path: '/local', + displayName: 'Local', + badgeColor: '#000', + addedAt: 1 +} + +const secondRepo: Repo = { + id: 'second-repo', + path: '/second', + displayName: 'Second', + badgeColor: '#111', + addedAt: 2 +} + +const reposUpdate = vi.fn() + +function deferred() { + let resolve!: (value: T | PromiseLike) => void + let reject!: (reason?: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +beforeEach(() => { + reposUpdate.mockReset() + vi.stubGlobal('window', { + api: { + repos: { + update: reposUpdate + } + } + }) +}) + +describe('repo update serialization', () => { + it('serializes local repo updates for the same repo before applying state', async () => { + const firstUpdate = deferred() + const secondUpdate = deferred() + const firstHookSettings: NonNullable = { + mode: 'override', + setupRunPolicy: 'ask', + commandSourcePolicy: 'local-only', + scripts: { setup: 'first setup', archive: '' } + } + const secondHookSettings: NonNullable = { + mode: 'override', + setupRunPolicy: 'skip-by-default', + commandSourcePolicy: 'run-both', + scripts: { setup: 'second setup', archive: 'second archive' } + } + reposUpdate.mockImplementationOnce(() => firstUpdate.promise) + reposUpdate.mockImplementationOnce(() => secondUpdate.promise) + const store = createTestStore() + store.setState({ repos: [localRepo] }) + + const first = store.getState().updateRepo(localRepo.id, { hookSettings: firstHookSettings }) + const second = store.getState().updateRepo(localRepo.id, { hookSettings: secondHookSettings }) + + expect(reposUpdate).toHaveBeenCalledTimes(1) + expect(store.getState().repos[0]?.hookSettings).toBeUndefined() + + firstUpdate.resolve() + await first + await Promise.resolve() + + expect(reposUpdate).toHaveBeenCalledTimes(2) + expect(store.getState().repos[0]?.hookSettings).toEqual(firstHookSettings) + + secondUpdate.resolve() + await second + + expect(store.getState().repos[0]?.hookSettings).toEqual(secondHookSettings) + }) + + it('does not serialize updates for different repos', async () => { + const slowLocalUpdate = deferred() + reposUpdate.mockImplementationOnce(() => slowLocalUpdate.promise) + reposUpdate.mockResolvedValueOnce(undefined) + const store = createTestStore() + store.setState({ repos: [localRepo, secondRepo] }) + + const local = store.getState().updateRepo(localRepo.id, { displayName: 'Local slow' }) + const second = store.getState().updateRepo(secondRepo.id, { displayName: 'Second fast' }) + + expect(reposUpdate).toHaveBeenCalledTimes(2) + await second + expect(store.getState().repos.find((repo) => repo.id === secondRepo.id)?.displayName).toBe( + 'Second fast' + ) + expect(store.getState().repos.find((repo) => repo.id === localRepo.id)?.displayName).toBe( + 'Local' + ) + + slowLocalUpdate.resolve() + await local + + expect(store.getState().repos.find((repo) => repo.id === localRepo.id)?.displayName).toBe( + 'Local slow' + ) + }) + + it('continues a repo update chain after a failed update', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + try { + reposUpdate.mockRejectedValueOnce(new Error('update failed')) + reposUpdate.mockResolvedValueOnce(undefined) + const store = createTestStore() + store.setState({ repos: [localRepo] }) + + const failed = store.getState().updateRepo(localRepo.id, { displayName: 'Failed' }) + const recovered = store.getState().updateRepo(localRepo.id, { displayName: 'Recovered' }) + + await Promise.all([failed, recovered]) + + expect(reposUpdate).toHaveBeenCalledTimes(2) + expect(store.getState().repos[0]?.displayName).toBe('Recovered') + } finally { + errorSpy.mockRestore() + } + }) +}) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 7f658ba74..18745a7c6 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -13,6 +13,30 @@ import { buildDismissedOnboardingFolderAgentStartup } from '@/lib/onboarding-fol const ERROR_TOAST_DURATION = 60_000 +type RepoUpdate = Partial< + Pick< + Repo, + | 'displayName' + | 'badgeColor' + | 'hookSettings' + | 'worktreeBaseRef' + | 'kind' + | 'symlinkPaths' + | 'issueSourcePreference' + > +> + +const updateRepoChainsByStore = new WeakMap<() => AppState, Map>>() + +function getRepoUpdateChains(get: () => AppState) { + let chains = updateRepoChainsByStore.get(get) + if (!chains) { + chains = new Map>() + updateRepoChainsByStore.set(get, chains) + } + return chains +} + export type RepoSlice = { repos: Repo[] activeRepoId: string | null @@ -21,21 +45,7 @@ export type RepoSlice = { addRepoPath: (path: string, kind?: 'git' | 'folder') => Promise addNonGitFolder: (path: string) => Promise removeRepo: (repoId: string) => Promise - updateRepo: ( - repoId: string, - updates: Partial< - Pick< - Repo, - | 'displayName' - | 'badgeColor' - | 'hookSettings' - | 'worktreeBaseRef' - | 'kind' - | 'symlinkPaths' - | 'issueSourcePreference' - > - > - ) => Promise + updateRepo: (repoId: string, updates: RepoUpdate) => Promise setActiveRepo: (repoId: string | null) => void reorderRepos: (orderedIds: string[]) => Promise } @@ -312,17 +322,34 @@ export const createRepoSlice: StateCreator = (set, }, updateRepo: async (repoId, updates) => { - try { - const target = getActiveRuntimeTarget(get().settings) - await (target.kind === 'local' - ? window.api.repos.update({ repoId, updates }) - : callRuntimeRpc(target, 'repo.update', { repo: repoId, updates }, { timeoutMs: 15_000 })) - set((s) => ({ - repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...updates } : r)) - })) - } catch (err) { - console.error('Failed to update repo:', err) + const updateRepoChains = getRepoUpdateChains(get) + const applyRepoUpdate = async () => { + try { + const target = getActiveRuntimeTarget(get().settings) + await (target.kind === 'local' + ? window.api.repos.update({ repoId, updates }) + : callRuntimeRpc(target, 'repo.update', { repo: repoId, updates }, { timeoutMs: 15_000 })) + set((s) => ({ + repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...updates } : r)) + })) + } catch (err) { + console.error('Failed to update repo:', err) + } } + const previous = updateRepoChains.get(repoId) + // Why: repo settings are persisted as full nested values. Preserve call + // order per repo so a slower IPC/RPC response cannot overwrite newer state. + const next = previous + ? previous.catch(() => undefined).then(applyRepoUpdate) + : applyRepoUpdate() + updateRepoChains.set(repoId, next) + const cleanup = () => { + if (updateRepoChains.get(repoId) === next) { + updateRepoChains.delete(repoId) + } + } + void next.then(cleanup, cleanup) + await next }, setActiveRepo: (repoId) => set({ activeRepoId: repoId }), diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 507562bfe..cffe1dfac 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -299,6 +299,7 @@ export function getDefaultRepoHookSettings(): RepoHookSettings { return { mode: 'auto', setupRunPolicy: 'run-by-default', + commandSourcePolicy: 'shared-only', scripts: { setup: '', archive: '' diff --git a/src/shared/cross-platform-path.ts b/src/shared/cross-platform-path.ts index d2645a342..b40d6e7b0 100644 --- a/src/shared/cross-platform-path.ts +++ b/src/shared/cross-platform-path.ts @@ -15,6 +15,14 @@ export function normalizeRuntimePathForComparison(value: string): string { return isWindowsAbsolutePathLike(value) ? normalized.toLowerCase() : normalized } +export function getRuntimePathBasename(value: string): string { + const trimmed = value.replace(/[\\/]+$/g, '') + if (!trimmed) { + return '' + } + return trimmed.split(/[\\/]/).filter(Boolean).at(-1) ?? '' +} + export function isPathInsideOrEqual(rootPath: string, candidatePath: string): boolean { const root = normalizeRuntimePathForComparison(rootPath) const candidate = normalizeRuntimePathForComparison(candidatePath) diff --git a/src/shared/hook-command-source-policy.ts b/src/shared/hook-command-source-policy.ts new file mode 100644 index 000000000..537659350 --- /dev/null +++ b/src/shared/hook-command-source-policy.ts @@ -0,0 +1,11 @@ +import type { HookCommandSourcePolicy } from './types' + +export function normalizeHookCommandSourcePolicy(policy: unknown): HookCommandSourcePolicy { + if (policy === 'local-only' || policy === 'run-both' || policy === 'shared-only') { + return policy + } + + // Why: old persisted settings may still contain the removed shared-first mode. + // Treat any unknown value as the authoritative committed config policy. + return 'shared-only' +} diff --git a/src/shared/types.ts b/src/shared/types.ts index cae4f3d02..8741ef2e8 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -94,6 +94,7 @@ export type Repo = { export type SetupRunPolicy = 'ask' | 'run-by-default' | 'skip-by-default' export type SetupDecision = 'inherit' | 'run' | 'skip' +export type HookCommandSourcePolicy = 'shared-only' | 'local-only' | 'run-both' /** * Envelope returned by the `repos:getBaseRefDefault` IPC handler. @@ -1049,11 +1050,11 @@ export type OrcaHooks = { } export type RepoHookSettings = { - // Why: legacy persisted data may still include the old UI-hook fields. Orca no longer - // treats them as an active config surface, but we keep them in the stored shape so - // existing local state can still be read without migrations. + // Why: persisted data may still include the old mode field from the earlier + // hook UI. Keep it in the shape so existing local state reads without a migration. mode: 'auto' | 'override' setupRunPolicy?: SetupRunPolicy + commandSourcePolicy?: HookCommandSourcePolicy scripts: { setup: string archive: string