xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: pr-bug-scan validated finding from #2270 (#2298) 

* 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

* Resolve direct PR launch start point correctly

- Prevent direct "Use PR" launches from skipping PR head resolution
- Avoid legacy local hook fallback whenever an orca.yaml file exists

---------

Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
buf0-bot[bot] 2026-05-18 22:43:58 -07:00 committed by GitHub
parent f8a2f2889e
commit e2fa5e354c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 158 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
src/main/hooks.test.ts
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()
})
@ -438,6 +458,21 @@ describe('getEffectiveHooks', () => {
})
})
it('does not fall back to legacy local hooks 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')
const { getEffectiveHooks } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: 'echo "legacy setup"', archive: 'echo "legacy archive"' }
})
const result = getEffectiveHooks(repo)
expect(result).toBeNull()
})
it('treats legacy shared-first policy as orca.yaml only', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(true)
@ -468,6 +503,50 @@ describe('getEffectiveHooks', () => {
expect(result).toBeNull()
})
it('falls back to legacy local setup source only when yaml is missing', async () => {
const fs = await import('fs')
vi.mocked(fs.existsSync).mockReturnValue(false)
const { getSetupCommandSource } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: 'echo "legacy setup"', archive: '' }
})
const result = getSetupCommandSource(repo)
expect(result).toEqual({ source: 'local', command: 'echo "legacy setup"' })
})
it('does not use legacy local setup source 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')
const { getSetupCommandSource } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: 'echo "legacy setup"', archive: '' }
})
const result = getSetupCommandSource(repo)
expect(result).toBeNull()
})
it('does not use legacy local setup source 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')
const { getSetupCommandSource } = await import('./hooks')
const repo = makeRepo({
mode: 'override',
scripts: { setup: 'echo "legacy setup"', archive: '' }
})
const result = getSetupCommandSource(repo)
expect(result).toBeNull()
})
})
describe('runHook', () => {

47
src/main/hooks.ts
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,38 @@ 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 hooksRoot = worktreePath ?? repo.path
const yamlHooks = loadHooks(hooksRoot)
const yamlFileExists = hasHooksFile(hooksRoot)
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 && !yamlFileExists && 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
@ -330,9 +353,13 @@ export function getSetupCommandSource(
repo: Repo,
worktreePath?: string
): { source: 'yaml' | 'local' | 'both'; command: string } | null {
const yamlSetup = loadHooks(worktreePath ?? repo.path)?.scripts.setup?.trim()
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 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 +373,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 && !yamlFileExists && localSetup && hasLegacyLocalScripts(repo)) {
return { source: 'local', command: localSetup }
}
return null
}

41
src/renderer/src/lib/launch-work-item-direct.ts
View File

@ -1,10 +1,11 @@
import { toast } from 'sonner'
import { useAppStore } from '@/store'
import { useAppStore, type AppState } from '@/store'
import { AGENT_CATALOG } from '@/lib/agent-catalog'
import { pasteDraftWhenAgentReady } from '@/lib/agent-paste-draft'
import { buildAgentDraftLaunchPlan, buildAgentStartupPlan } from '@/lib/tui-agent-startup'
import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config'
import { activateAndRevealWorktree, type AgentStartedTelemetry } from '@/lib/worktree-activation'
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
import {
CLIENT_PLATFORM,
getLinkedWorkItemSuggestedName,
@ -15,6 +16,7 @@ import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed'
import { checkRuntimeHooks } from '@/runtime/runtime-hooks-client'
import { track, tuiAgentToAgentKind } from '@/lib/telemetry'
import type {
GitPushTarget,
OrcaHooks,
RepoHookSettings,
SetupDecision,
@ -86,6 +88,24 @@ function pickAgent(
return null
}
async function resolveDirectPrStartPoint(
repoId: string,
prNumber: number,
settings: AppState['settings']
): Promise<{ baseBranch: string; pushTarget?: GitPushTarget }> {
const target = getActiveRuntimeTarget(settings)
const result =
target.kind === 'local'
? await window.api.worktrees.resolvePrBase({ repoId, prNumber })
: await callRuntimeRpc<
{ baseBranch: string; pushTarget?: GitPushTarget } | { error: string }
>(target, 'worktree.resolvePrBase', { repo: repoId, prNumber }, { timeoutMs: 30_000 })
if ('error' in result) {
throw new Error(result.error)
}
return result
}
async function resolveSetupDecision(
repoId: string,
repo: { hookSettings?: RepoHookSettings }
@ -207,6 +227,21 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom
linkedIssueNumber: item.type === 'issue' ? (item.number ?? null) : null,
linkedPR: item.type === 'pr' ? (item.number ?? null) : null
})
let resolvedBaseBranch = baseBranch
let resolvedPushTarget: GitPushTarget | undefined
if (!resolvedBaseBranch && item.type === 'pr' && item.number) {
try {
// Why: direct "Use PR" launches bypass the Start-from picker, so they
// must still resolve the PR head before `git worktree add`.
const result = await resolveDirectPrStartPoint(repoId, item.number, settings)
resolvedBaseBranch = result.baseBranch
resolvedPushTarget = result.pushTarget
} catch (error) {
toast.error(error instanceof Error ? error.message : 'Failed to resolve PR head.')
openModalFallback()
return
}
}
let worktreeId: string
let primaryTabId: string | null
@ -217,14 +252,14 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom
const result = await store.createWorktree(
repoId,
workspaceName,
baseBranch,
resolvedBaseBranch,
finalSetupDecision,
undefined,
telemetrySource,
item.title,
item.type === 'issue' && item.number ? item.number : undefined,
item.type === 'pr' && item.number ? item.number : undefined,
undefined,
resolvedPushTarget,
undefined,
item.linearIdentifier
)