fix: address pr-bug-scan validated finding from #2270

Restored legacy local hook fallback at hooks.ts:286-307 when commandSourcePolicy is undefined and persisted scripts.setup/archive are non-empty; also patched getSetupCommandSource and updated the regr
This commit is contained in:
orca-bug-scan-bot
2026-05-18 20:10:04 -07:00
parent 993d0ae599
commit f92dfd79c9
2 changed files with 54 additions and 6 deletions
+21 -1
View File
@@ -346,7 +346,7 @@ describe('getEffectiveHooks', () => {
expect(result?.scripts.setup).not.toContain('old-version')
})
it('does not fall back to local settings hooks by default when yaml is missing', async () => {
it('falls back to legacy local hooks when policy is unset and yaml is missing', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(false)
@@ -357,6 +357,26 @@ describe('getEffectiveHooks', () => {
})
const result = getEffectiveHooks(repo)
expect(result).toEqual({
scripts: {
setup: 'echo "local setup"',
archive: 'echo "local archive"'
}
})
})
it('does not fall back to local hooks when policy is explicitly shared-only', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(false)
const { getEffectiveHooks } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
commandSourcePolicy: 'shared-only',
scripts: { setup: 'echo "local setup"', archive: 'echo "local archive"' }
})
const result = getEffectiveHooks(repo)
expect(result).toBeNull()
})
+33 -5
View File
@@ -267,7 +267,8 @@ function ensureOrcaDirIgnored(repoPath: string): void {
function getEffectiveHookScript(
yamlScript: string | undefined,
localScript: string | undefined,
policy: HookCommandSourcePolicy
policy: HookCommandSourcePolicy,
legacyFallback: boolean
): string | undefined {
const shared = yamlScript?.trim()
const local = localScript?.trim()
@@ -280,16 +281,36 @@ 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 getEffectiveHooks(repo: Repo, worktreePath?: string): OrcaHooks | null {
const yamlHooks = loadHooks(worktreePath ?? repo.path)
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)
const rawPolicy = repo.hookSettings?.commandSourcePolicy
const policy = normalizeHookCommandSourcePolicy(rawPolicy)
const legacyFallback = rawPolicy === undefined && hasLegacyLocalScripts(repo)
const setup = getEffectiveHookScript(yamlHooks?.scripts.setup, localSetup, policy, legacyFallback)
const archive = getEffectiveHookScript(
yamlHooks?.scripts.archive,
localArchive,
policy,
legacyFallback
)
if (!setup && !archive) {
return null
@@ -332,7 +353,8 @@ export function getSetupCommandSource(
): { 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)
const rawPolicy = repo.hookSettings?.commandSourcePolicy
const policy = normalizeHookCommandSourcePolicy(rawPolicy)
if (policy === 'local-only') {
return localSetup ? { source: 'local', command: localSetup } : null
@@ -346,6 +368,12 @@ 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 && localSetup && hasLegacyLocalScripts(repo)) {
return { source: 'local', command: localSetup }
}
return null
}