starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-22 00:02:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve setup settings organization (#2503)

Browse Source 
* fix: address review findings

* fix: address review findings
This commit is contained in:
Jinjing
2026-05-21 16:33:30 -07:00
committed by GitHub
parent d89e7f3e34
commit 55e6f4bfcd
17 changed files with 1029 additions and 611 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/hooks.test.ts
+55 -10
View File
@@ -380,7 +380,7 @@ describe('getEffectiveHooks', () => {
expect(result).toBeNull()
})
it('uses shared yaml settings over local settings by default', async () => {
it('uses local settings over shared yaml settings by default when local hooks exist', 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')
@@ -394,7 +394,7 @@ describe('getEffectiveHooks', () => {
expect(result).toEqual({
scripts: {
setup: 'echo "yaml setup"'
setup: 'echo "ui override"'
}
})
})
@@ -439,7 +439,7 @@ describe('getEffectiveHooks', () => {
})
})
it('treats orca.yaml as authoritative by default when it defines only one command', async () => {
it('uses local settings by default even when orca.yaml 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')
@@ -453,12 +453,35 @@ describe('getEffectiveHooks', () => {
expect(result).toEqual({
scripts: {
archive: 'echo "yaml archive"'
setup: 'echo "legacy setup"',
archive: 'echo "legacy archive"'
}
})
})
it('does not fall back to legacy local hooks when yaml exists without supported hooks', async () => {
it('keeps shared setup when only archive has a legacy local script', 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 archive: |\n echo "yaml archive"\n'
)
const { getEffectiveHooks } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: '', archive: 'echo "legacy archive"' }
})
const result = getEffectiveHooks(repo)
expect(result).toEqual({
scripts: {
setup: 'echo "yaml setup"',
archive: 'echo "legacy archive"'
}
})
})
it('uses local settings by default when yaml exists without supported hooks', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(true)
vi.mocked(fs.readFileSync).mockReturnValue('futureFeature: enabled\n')
@@ -470,7 +493,12 @@ describe('getEffectiveHooks', () => {
})
const result = getEffectiveHooks(repo)
expect(result).toBeNull()
expect(result).toEqual({
scripts: {
setup: 'echo "legacy setup"',
archive: 'echo "legacy archive"'
}
})
})
it('treats legacy shared-first policy as orca.yaml only', async () => {
@@ -518,7 +546,7 @@ describe('getEffectiveHooks', () => {
expect(result).toEqual({ source: 'local', command: 'echo "legacy setup"' })
})
it('does not use legacy local setup source when yaml omits setup', async () => {
it('uses local setup source by default when yaml omits setup', 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')
@@ -530,10 +558,10 @@ describe('getEffectiveHooks', () => {
})
const result = getSetupCommandSource(repo)
expect(result).toBeNull()
expect(result).toEqual({ source: 'local', command: 'echo "legacy setup"' })
})
it('does not use legacy local setup source when yaml exists without supported hooks', async () => {
it('uses local setup source by default when yaml exists without supported hooks', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(true)
vi.mocked(fs.readFileSync).mockReturnValue('futureFeature: enabled\n')
@@ -545,7 +573,24 @@ describe('getEffectiveHooks', () => {
})
const result = getSetupCommandSource(repo)
expect(result).toBeNull()
expect(result).toEqual({ source: 'local', command: 'echo "legacy setup"' })
})
it('uses shared setup source when only archive has a legacy local script', 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 archive: |\n echo "yaml archive"\n'
)
const { getSetupCommandSource } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: '', archive: 'echo "legacy archive"' }
})
const result = getSetupCommandSource(repo)
expect(result).toEqual({ source: 'yaml', command: 'echo "yaml setup"' })
})
})
src/main/hooks.ts
+15 -36
View File
@@ -4,7 +4,7 @@ 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 { resolveHookCommandSourcePolicy } from '../shared/hook-command-source-policy'
import { gitExecFileSync } from './git/runner'
import { isWslPath, parseWslPath, toWindowsWslPath, toLinuxPath } from './wsl'
import type {
@@ -267,8 +267,7 @@ function ensureOrcaDirIgnored(repoPath: string): void {
function getEffectiveHookScript(
yamlScript: string | undefined,
localScript: string | undefined,
policy: HookCommandSourcePolicy,
legacyFallback: boolean
policy: HookCommandSourcePolicy
): string | undefined {
const shared = yamlScript?.trim()
const local = localScript?.trim()
@@ -281,39 +280,24 @@ function getEffectiveHookScript(
return [shared, local].filter(Boolean).join('\n') || undefined
}
// Why: existing users persisted local setup/archive scripts before
// commandSourcePolicy existed. Without a fallback the new default
// 'shared-only' would silently drop those scripts when orca.yaml is missing.
if (legacyFallback) {
return shared || local || undefined
}
return shared || undefined
}
function hasLegacyLocalScripts(repo: Repo): boolean {
return Boolean(
repo.hookSettings?.scripts.setup?.trim() || repo.hookSettings?.scripts.archive?.trim()
)
}
export function getEffectiveHooksFromConfig(
repo: Repo,
yamlHooks: OrcaHooks | null,
yamlFileExists: boolean
yamlHooks: OrcaHooks | null
): OrcaHooks | null {
const localSetup = repo.hookSettings?.scripts.setup
const localArchive = repo.hookSettings?.scripts.archive
const rawPolicy = repo.hookSettings?.commandSourcePolicy
const policy = normalizeHookCommandSourcePolicy(rawPolicy)
const legacyFallback = rawPolicy === undefined && !yamlFileExists && hasLegacyLocalScripts(repo)
const setup = getEffectiveHookScript(yamlHooks?.scripts.setup, localSetup, policy, legacyFallback)
const archive = getEffectiveHookScript(
yamlHooks?.scripts.archive,
localArchive,
policy,
legacyFallback
)
const setupPolicy = resolveHookCommandSourcePolicy(rawPolicy, {
hasLocalScript: Boolean(localSetup?.trim())
})
const archivePolicy = resolveHookCommandSourcePolicy(rawPolicy, {
hasLocalScript: Boolean(localArchive?.trim())
})
const setup = getEffectiveHookScript(yamlHooks?.scripts.setup, localSetup, setupPolicy)
const archive = getEffectiveHookScript(yamlHooks?.scripts.archive, localArchive, archivePolicy)
if (!setup && !archive) {
return null
@@ -332,7 +316,7 @@ export function getEffectiveHooksFromConfig(
export function getEffectiveHooks(repo: Repo, worktreePath?: string): OrcaHooks | null {
const hooksRoot = worktreePath ?? repo.path
return getEffectiveHooksFromConfig(repo, loadHooks(hooksRoot), hasHooksFile(hooksRoot))
return getEffectiveHooksFromConfig(repo, loadHooks(hooksRoot))
}
export function getEffectiveSetupRunPolicy(repo: Repo): SetupRunPolicy {
@@ -361,11 +345,12 @@ export function getSetupCommandSource(
): { source: 'yaml' | 'local' | 'both'; command: string } | null {
const hooksRoot = worktreePath ?? repo.path
const yamlHooks = loadHooks(hooksRoot)
const yamlFileExists = hasHooksFile(hooksRoot)
const yamlSetup = yamlHooks?.scripts.setup?.trim()
const localSetup = repo.hookSettings?.scripts.setup?.trim()
const rawPolicy = repo.hookSettings?.commandSourcePolicy
const policy = normalizeHookCommandSourcePolicy(rawPolicy)
const policy = resolveHookCommandSourcePolicy(rawPolicy, {
hasLocalScript: Boolean(localSetup)
})
if (policy === 'local-only') {
return localSetup ? { source: 'local', command: localSetup } : null
@@ -379,12 +364,6 @@ export function getSetupCommandSource(
return { source: 'yaml', command: yamlSetup }
}
// Why: pre-policy persisted local setup scripts must keep working when
// orca.yaml is missing; otherwise upgrading silently drops the legacy hook.
if (rawPolicy === undefined && !yamlFileExists && localSetup && hasLegacyLocalScripts(repo)) {
return { source: 'local', command: localSetup }
}
return null
}
src/main/ipc/worktree-remote.ts
+2 -2
View File
@@ -550,9 +550,9 @@ async function readRemoteEffectiveHooks(
try {
const result = await fsProvider.readFile(joinWorktreeRelativePath(hooksRootPath, 'orca.yaml'))
const yamlHooks = result.isBinary ? null : parseOrcaYaml(result.content)
return getEffectiveHooksFromConfig(repo, yamlHooks, true)
return getEffectiveHooksFromConfig(repo, yamlHooks)
} catch {
return getEffectiveHooksFromConfig(repo, null, false)
return getEffectiveHooksFromConfig(repo, null)
}
}
src/main/startup/run-electron-vite-dev.test.ts
+3 -2
View File
@@ -344,7 +344,7 @@ describe('run-electron-vite-dev', () => {
} catch {
return false
}
})
}, 20000)
const grandchildPid = Number.parseInt(readFileSync(pidFile, 'utf8').trim(), 10)
if (Number.isFinite(grandchildPid)) {
@@ -366,6 +366,7 @@ describe('run-electron-vite-dev', () => {
expect(readlinkSync(join(frameworkPath, 'Versions', 'Current'))).toBe('A')
wrapper.kill('SIGINT')
}
},
30000
)
})
src/renderer/src/components/settings/RepositoryHooksSection.test.ts
+64 -31
View File
@@ -1,41 +1,74 @@
import { describe, expect, it } from 'vitest'
import {
commandRowsToScript,
localCommandDraftToScripts,
scriptToCommandRows,
type LocalCommandDraft,
type LocalCommandRow
} from './RepositoryHooksSection'
import { getLocalCommandSourcePolicyNotice } from './RepositoryHooksSection'
describe('RepositoryHooksSection command row serialization', () => {
it('round-trips blank lines and trailing whitespace in existing scripts', () => {
const script = 'echo before \n\ncat <<EOF\n body \nEOF\n'
expect(commandRowsToScript(scriptToCommandRows(script))).toBe(script)
describe('getLocalCommandSourcePolicyNotice', () => {
it('does not show a notice when no local scripts are saved', () => {
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: true,
currentPolicy: 'shared-only',
setupScript: '',
archiveScript: '',
hasSharedScript: false
})
).toBeNull()
})
it('keeps persisted blank rows distinct from new empty placeholders', () => {
const rows: LocalCommandRow[] = [
...scriptToCommandRows('echo before\n\n echo after '),
{ value: '', isPlaceholder: true }
]
it('does not show a notice when command source already includes local scripts', () => {
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: true,
currentPolicy: 'local-only',
setupScript: 'pnpm install',
archiveScript: '',
hasSharedScript: true
})
).toBeNull()
expect(commandRowsToScript(rows)).toBe('echo before\n\n echo after ')
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: true,
currentPolicy: 'run-both',
setupScript: '',
archiveScript: 'echo archive',
hasSharedScript: true
})
).toBeNull()
})
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 }
]
}
it('waits for hook inspection before recommending a command source', () => {
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: false,
currentPolicy: 'shared-only',
setupScript: 'pnpm install',
archiveScript: '',
hasSharedScript: false
})
).toEqual({ kind: 'checking' })
})
expect(localCommandDraftToScripts(draft)).toEqual({
setup: 'echo setup\n',
archive: '\necho archive'
})
it('recommends local commands when local scripts are saved and no shared script exists', () => {
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: true,
currentPolicy: 'shared-only',
setupScript: 'pnpm install',
archiveScript: '',
hasSharedScript: false
})
).toEqual({ kind: 'action', policy: 'local-only', label: 'Use local commands' })
})
it('recommends run-both when local and shared scripts both exist', () => {
expect(
getLocalCommandSourcePolicyNotice({
hooksInspectionReady: true,
currentPolicy: 'shared-only',
setupScript: '',
archiveScript: 'echo archive',
hasSharedScript: true
})
).toEqual({ kind: 'action', policy: 'run-both', label: 'Run both' })
})
})
src/renderer/src/components/settings/RepositoryHooksSection.tsx
+670 -427
View File
File diff suppressed because it is too large Load Diff
src/renderer/src/components/settings/RepositoryPane.test.ts
+27
View File
@@ -0,0 +1,27 @@
import { describe, expect, it } from 'vitest'
import type { Repo } from '../../../../shared/types'
import { getRepositoryPaneSearchEntries } from './RepositoryPane'
import { matchesSettingsSearch } from './settings-search'
const repo: Repo = {
id: 'repo-1',
path: '/tmp/repo',
displayName: 'Example Repo',
badgeColor: '#000000',
addedAt: 1,
kind: 'git'
}
describe('RepositoryPane search entries', () => {
it('keeps renamed hook sections reachable through settings search', () => {
const entries = getRepositoryPaneSearchEntries(repo)
expect(matchesSettingsSearch('setup script', entries)).toBe(true)
expect(matchesSettingsSearch('archive script', entries)).toBe(true)
expect(matchesSettingsSearch('setup command', entries)).toBe(true)
expect(matchesSettingsSearch('archive command', entries)).toBe(true)
expect(matchesSettingsSearch('advanced', entries)).toBe(true)
expect(matchesSettingsSearch('command source', entries)).toBe(true)
expect(matchesSettingsSearch('local settings scripts', entries)).toBe(true)
})
})
src/renderer/src/components/settings/RepositoryPane.tsx
+83 -62
View File
@@ -20,6 +20,7 @@ type RepositoryPaneProps = {
repo: Repo
yamlHooks: OrcaHooks | null
hasHooksFile: boolean
hooksInspectionReady: boolean
mayNeedUpdate: boolean
updateRepo: (repoId: string, updates: Partial<Repo>) => void
removeRepo: (repoId: string) => void
@@ -97,21 +98,40 @@ export function getRepositoryPaneSearchEntries(repo: Repo): SettingsSearchEntry[
]
},
{
title: 'orca.yaml hooks',
description: 'Shared setup and archive hook commands for this repository.',
keywords: [repo.displayName, 'hooks', 'setup', 'archive', 'yaml']
},
{
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.',
title: 'Setup Script',
description: 'Local and shared scripts that run after a new worktree is created.',
keywords: [
repo.displayName,
'hooks',
'setup',
'setup script',
'setup command',
'local settings scripts',
'orca.yaml hooks',
'yaml'
]
},
{
title: 'Archive Script',
description: 'Local and shared scripts that run before a worktree is archived.',
keywords: [
repo.displayName,
'hooks',
'archive',
'archive script',
'archive command',
'local settings scripts',
'orca.yaml hooks',
'yaml'
]
},
{
title: 'Advanced',
description: 'Command source and orca.yaml details.',
keywords: [
repo.displayName,
'advanced',
'command source',
'local',
'orca.yaml',
'shared',
@@ -122,7 +142,7 @@ export function getRepositoryPaneSearchEntries(repo: Repo): SettingsSearchEntry[
},
{
title: 'When to Run Setup',
description: 'Choose the default behavior when a setup command is available.',
description: 'Choose the default behavior when a setup script is available.',
keywords: [
repo.displayName,
'setup run policy',
@@ -153,6 +173,7 @@ export function RepositoryPane({
repo,
yamlHooks,
hasHooksFile,
hooksInspectionReady,
mayNeedUpdate,
updateRepo,
removeRepo
@@ -200,9 +221,9 @@ export function RepositoryPane({
)
const hooksEntries = allEntries.filter((entry) =>
[
'orca.yaml hooks',
'Local Settings Commands',
'Command Source',
'Setup Script',
'Archive Script',
'Advanced',
'When to Run Setup',
'Custom GitHub Issue Command'
].includes(entry.title)
@@ -210,6 +231,24 @@ export function RepositoryPane({
const mcpEntries = allEntries.filter((entry) => entry.title === 'MCP Configs')
const symlinkEntries = allEntries.filter((entry) => entry.title === 'Worktree Symlinks')
const hooksSection =
!isFolder && matchesSettingsSearch(searchQuery, hooksEntries) ? (
<RepositoryHooksSection
key="hooks"
repo={repo}
yamlHooks={yamlHooks}
hasHooksFile={hasHooksFile}
hooksInspectionReady={hooksInspectionReady}
mayNeedUpdate={mayNeedUpdate}
copiedTemplate={copiedTemplate}
onCopyTemplate={() => void handleCopyTemplate()}
onUpdateHookSettings={updateSelectedRepoHookSettings}
/>
) : null
// Why: Identity (name, color, base ref) stays at the top so it's the first
// thing a user sees. Setup commands follow immediately because they're the
// most-edited surface and should beat MCP/symlinks/sparse-presets.
const visibleSections = [
matchesSettingsSearch(searchQuery, identityEntries) ? (
<section key="identity" className="space-y-8">
@@ -249,42 +288,35 @@ export function RepositoryPane({
<SearchableSetting
title="Display Name"
description="Repo-specific display details for the sidebar and tabs."
keywords={[repo.displayName, repo.path, 'repository name']}
keywords={[repo.displayName, repo.path, 'repository name', 'color', 'badge']}
className="space-y-2"
>
<Label className="text-sm font-semibold">Display Name</Label>
<Input
value={repo.displayName}
onChange={(e) =>
updateRepo(repo.id, {
displayName: e.target.value
})
}
className="h-9 text-sm"
/>
</SearchableSetting>
<SearchableSetting
title="Badge Color"
description="Repo color used in the sidebar and tabs."
keywords={[repo.displayName, 'color', 'badge']}
className="space-y-2"
>
<Label className="text-sm font-semibold">Badge Color</Label>
<div className="flex flex-wrap gap-2">
{REPO_COLORS.map((color) => (
<button
key={color}
onClick={() => updateRepo(repo.id, { badgeColor: color })}
className={`size-7 rounded-full transition-all ${
repo.badgeColor === color
? 'ring-2 ring-foreground ring-offset-2 ring-offset-background'
: 'hover:ring-1 hover:ring-muted-foreground hover:ring-offset-2 hover:ring-offset-background'
}`}
style={{ backgroundColor: color }}
title={color}
/>
))}
<div className="flex items-center gap-3">
<Input
value={repo.displayName}
onChange={(e) =>
updateRepo(repo.id, {
displayName: e.target.value
})
}
className="h-9 flex-1 text-sm"
/>
<div className="flex flex-wrap gap-2">
{REPO_COLORS.map((color) => (
<button
key={color}
onClick={() => updateRepo(repo.id, { badgeColor: color })}
className={`size-7 rounded-full transition-all ${
repo.badgeColor === color
? 'ring-2 ring-foreground ring-offset-2 ring-offset-background'
: 'hover:ring-1 hover:ring-muted-foreground hover:ring-offset-2 hover:ring-offset-background'
}`}
style={{ backgroundColor: color }}
title={color}
/>
))}
</div>
</div>
</SearchableSetting>
@@ -306,6 +338,7 @@ export function RepositoryPane({
) : null}
</section>
) : null,
hooksSection,
!isFolder &&
!repo.connectionId &&
symlinksEnabled &&
@@ -317,18 +350,6 @@ export function RepositoryPane({
) : null,
!isFolder && matchesSettingsSearch(searchQuery, mcpEntries) ? (
<McpConfigSection key="mcp-configs" repo={repo} />
) : null,
!isFolder && matchesSettingsSearch(searchQuery, hooksEntries) ? (
<RepositoryHooksSection
key="hooks"
repo={repo}
yamlHooks={yamlHooks}
hasHooksFile={hasHooksFile}
mayNeedUpdate={mayNeedUpdate}
copiedTemplate={copiedTemplate}
onCopyTemplate={() => void handleCopyTemplate()}
onUpdateHookSettings={updateSelectedRepoHookSettings}
/>
) : null
].filter(Boolean)
src/renderer/src/components/settings/Settings.tsx
+2 -1
View File
@@ -1313,7 +1313,7 @@ function Settings(): React.JSX.Element {
<SettingsSection
key={repo.id}
id={repoSectionId}
title={repo.displayName}
title={`Repo Settings > ${repo.displayName}`}
description={repo.path}
searchEntries={getRepositoryPaneSearchEntries(repo)}
>
@@ -1322,6 +1322,7 @@ function Settings(): React.JSX.Element {
repo={repo}
yamlHooks={repoHooksState?.hooks ?? null}
hasHooksFile={repoHooksState?.hasHooks ?? false}
hooksInspectionReady={Boolean(repoHooksState)}
mayNeedUpdate={repoHooksState?.mayNeedUpdate ?? false}
updateRepo={updateRepo}
removeRepo={removeRepo}
src/renderer/src/components/sidebar/SetupScriptPromptCard.tsx
+4 -8
View File
@@ -14,7 +14,7 @@ import {
type HookCheckResult
} from '@/runtime/runtime-hooks-client'
import { getDefaultRepoHookSettings } from '../../../../shared/constants'
import { normalizeHookCommandSourcePolicy } from '../../../../shared/hook-command-source-policy'
import { resolveHookCommandSourcePolicy } from '../../../../shared/hook-command-source-policy'
import { isGitRepoKind } from '../../../../shared/repo-kind'
import type { Repo, RepoHookSettings } from '../../../../shared/types'
import type { SetupScriptImportCandidate } from '../../../../shared/setup-script-imports'
@@ -34,7 +34,9 @@ function hasEffectiveSetupCommand(repo: Repo, hooksResult: HookCheckResult): boo
const localSetup = repo.hookSettings?.scripts?.setup?.trim()
const sharedSetup = hooksResult.hooks?.scripts?.setup?.trim()
const rawPolicy = repo.hookSettings?.commandSourcePolicy
const sourcePolicy = normalizeHookCommandSourcePolicy(rawPolicy)
const sourcePolicy = resolveHookCommandSourcePolicy(rawPolicy, {
hasLocalScript: Boolean(localSetup)
})
if (sourcePolicy === 'local-only') {
return Boolean(localSetup)
@@ -44,12 +46,6 @@ function hasEffectiveSetupCommand(repo: Repo, hooksResult: HookCheckResult): boo
return Boolean(sharedSetup || localSetup)
}
// Why: local setup commands saved before commandSourcePolicy existed still
// run when there is no tracked hook file; the prompt should respect that.
if (rawPolicy === undefined && !hooksResult.hasHooks) {
return Boolean(localSetup)
}
return Boolean(sharedSetup)
}
src/renderer/src/lib/ensure-hooks-confirmed.test.ts
+26
View File
@@ -147,6 +147,32 @@ describe('ensureHooksConfirmed', () => {
expect(pending).toHaveLength(0)
})
it('does not prompt for orca.yaml when local commands are the implicit default', async () => {
const { state, pending } = createTestState({
repos: [
{
id: 'repo-1',
displayName: 'Repo One',
hookSettings: {
mode: 'auto',
scripts: { setup: 'echo local', archive: '' }
}
}
]
} as Partial<AppState>)
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({
src/renderer/src/lib/ensure-hooks-confirmed.ts
+9 -1
View File
@@ -1,5 +1,6 @@
import type { AppState } from '@/store/types'
import type { OrcaHooks } from '../../../shared/types'
import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-source-policy'
import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust'
import { checkRuntimeHooks, readRuntimeIssueCommand } from '@/runtime/runtime-hooks-client'
@@ -39,7 +40,14 @@ export async function ensureHooksConfirmed(
scriptContent = (result.sharedContent ?? '').trim()
} else {
const repo = state.repos.find((r) => r.id === repoId)
if (repo?.hookSettings?.commandSourcePolicy === 'local-only') {
const localScript = repo?.hookSettings?.scripts?.[scriptKind]?.trim()
const sourcePolicy = resolveHookCommandSourcePolicy(
repo?.hookSettings?.commandSourcePolicy,
{
hasLocalScript: Boolean(localScript)
}
)
if (sourcePolicy === 'local-only') {
return 'run'
}
const result = await checkRuntimeHooks(state.settings, repoId)
src/renderer/src/lib/new-workspace.ts
+4 -2
View File
@@ -7,7 +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'
import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-source-policy'
import { isExpectedAgentProcess } from '../../../shared/agent-process-recognition'
/**
@@ -141,7 +141,9 @@ export function getSetupConfig(
): SetupConfig | null {
const yamlSetup = yamlHooks?.scripts?.setup?.trim()
const localSetup = repo?.hookSettings?.scripts?.setup?.trim()
const sourcePolicy = normalizeHookCommandSourcePolicy(repo?.hookSettings?.commandSourcePolicy)
const sourcePolicy = resolveHookCommandSourcePolicy(repo?.hookSettings?.commandSourcePolicy, {
hasLocalScript: Boolean(localSetup)
})
if (sourcePolicy === 'local-only') {
return localSetup ? { source: 'local', command: localSetup } : null
src/shared/constants.ts
-1
View File
@@ -313,7 +313,6 @@ export function getDefaultRepoHookSettings(): RepoHookSettings {
return {
mode: 'auto',
setupRunPolicy: 'run-by-default',
commandSourcePolicy: 'shared-only',
scripts: {
setup: '',
archive: ''
src/shared/hook-command-source-policy.test.ts
+26
View File
@@ -0,0 +1,26 @@
import { describe, expect, it } from 'vitest'
import {
normalizeHookCommandSourcePolicy,
resolveHookCommandSourcePolicy
} from './hook-command-source-policy'
describe('hook command source policy', () => {
it('normalizes unknown persisted policies to shared-only', () => {
expect(normalizeHookCommandSourcePolicy('shared-first')).toBe('shared-only')
})
it('uses local commands by default when a local script is configured', () => {
expect(resolveHookCommandSourcePolicy(undefined, { hasLocalScript: true })).toBe('local-only')
})
it('uses shared commands by default when no local script is configured', () => {
expect(resolveHookCommandSourcePolicy(undefined, { hasLocalScript: false })).toBe('shared-only')
})
it('preserves explicit command source choices', () => {
expect(resolveHookCommandSourcePolicy('shared-only', { hasLocalScript: true })).toBe(
'shared-only'
)
expect(resolveHookCommandSourcePolicy('run-both', { hasLocalScript: true })).toBe('run-both')
})
})
src/shared/hook-command-source-policy.ts
+15
View File
@@ -9,3 +9,18 @@ export function normalizeHookCommandSourcePolicy(policy: unknown): HookCommandSo
// Treat any unknown value as the authoritative committed config policy.
return 'shared-only'
}
export function resolveHookCommandSourcePolicy(
policy: unknown,
{ hasLocalScript }: { hasLocalScript: boolean }
): HookCommandSourcePolicy {
if (policy === 'local-only' || policy === 'run-both' || policy === 'shared-only') {
return policy
}
if (policy === undefined && hasLocalScript) {
return 'local-only'
}
return 'shared-only'
}
tests/e2e/setup-script-import.spec.ts
+24 -28
View File
@@ -121,7 +121,7 @@ async function openRepoSettings(page: Page, repoId: string): Promise<Locator> {
const repoSettings = page.locator(`[data-settings-section="repo-${repoId}"]`)
await expect(repoSettings).toBeVisible({ timeout: 10_000 })
await expect(repoSettings.getByText('Local Settings Commands').first()).toBeVisible()
await expect(repoSettings.getByText('Setup Script').first()).toBeVisible()
return repoSettings
}
@@ -131,29 +131,19 @@ async function openImportedSetupSettingsFromToast(page: Page, repoId: string): P
// Why: in hidden Electron CI windows, the Sonner action can be laid out just
// outside Playwright's viewport even though the action is mounted and wired.
await viewInSettings.evaluate((button) => (button as HTMLButtonElement).click())
const localCommands = page.locator(`[id="repo-${repoId}-local-commands"]`)
await expect(localCommands).toBeVisible({ timeout: 10_000 })
await expect(localCommands.getByText('Local Settings Commands').first()).toBeVisible()
return localCommands
const setupCommand = page.locator(`[id="repo-${repoId}-local-commands"]`)
await expect(setupCommand).toBeVisible({ timeout: 10_000 })
const repoSettings = page.locator(`[data-settings-section="repo-${repoId}"]`)
await expect(repoSettings.getByText('Setup Script').first()).toBeVisible()
return repoSettings
}
async function expectSettingsInputValue(container: Locator, value: string): Promise<void> {
await expect
.poll(
() =>
container
.locator('input')
.evaluateAll(
(inputs, expectedValue) =>
inputs.some((input) => (input as HTMLInputElement).value === expectedValue),
value
),
{
timeout: 10_000,
message: `Settings input value "${value}" was not visible`
}
)
.toBe(true)
async function expectSettingsCommandValue(
container: Locator,
name: 'Setup Script' | 'Archive Script',
value: string
): Promise<void> {
await expect(container.getByRole('textbox', { name })).toHaveValue(value, { timeout: 10_000 })
}
test.describe('Setup script import prompt', () => {
@@ -178,10 +168,16 @@ test.describe('Setup script import prompt', () => {
).toBeVisible()
const localCommands = await openImportedSetupSettingsFromToast(orcaPage, repoId)
await expectSettingsInputValue(localCommands, 'corepack enable')
await expectSettingsInputValue(localCommands, 'bun install')
await expectSettingsInputValue(localCommands, 'bun run db:migrate')
await expectSettingsInputValue(localCommands, 'docker compose down --remove-orphans')
await expectSettingsCommandValue(
localCommands,
'Setup Script',
'corepack enable\nbun install\nbun run db:migrate'
)
await expectSettingsCommandValue(
localCommands,
'Archive Script',
'docker compose down --remove-orphans'
)
})
test('imports cmux setup commands through the prompt UI', async ({ orcaPage }, testInfo) => {
@@ -197,7 +193,7 @@ test.describe('Setup script import prompt', () => {
await expect(orcaPage.getByText('Setup script imported')).toBeVisible()
const repoSettings = await openRepoSettings(orcaPage, repoId)
await expectSettingsInputValue(repoSettings, './scripts/setup.sh')
await expect(repoSettings.getByText('No local archive commands configured.')).toBeVisible()
await expectSettingsCommandValue(repoSettings, 'Setup Script', './scripts/setup.sh')
await expectSettingsCommandValue(repoSettings, 'Archive Script', '')
})
})