From b3578736edd953444f00f1d87bb0c01fd9e4c2f0 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 18 May 2026 22:33:07 -0700 Subject: [PATCH] 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 --- src/main/hooks.test.ts | 59 +++++++++++++++++++ src/main/hooks.ts | 13 ++-- .../src/lib/launch-work-item-direct.ts | 41 ++++++++++++- 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index a0b14b26cb2..a6ce1718c9d 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -458,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) @@ -488,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', () => { diff --git a/src/main/hooks.ts b/src/main/hooks.ts index 44578cd541f..08bf04695dd 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -298,12 +298,14 @@ function hasLegacyLocalScripts(repo: Repo): boolean { } 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 rawPolicy = repo.hookSettings?.commandSourcePolicy const policy = normalizeHookCommandSourcePolicy(rawPolicy) - const legacyFallback = rawPolicy === undefined && hasLegacyLocalScripts(repo) + const legacyFallback = rawPolicy === undefined && !yamlFileExists && hasLegacyLocalScripts(repo) const setup = getEffectiveHookScript(yamlHooks?.scripts.setup, localSetup, policy, legacyFallback) const archive = getEffectiveHookScript( yamlHooks?.scripts.archive, @@ -351,7 +353,10 @@ 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 rawPolicy = repo.hookSettings?.commandSourcePolicy const policy = normalizeHookCommandSourcePolicy(rawPolicy) @@ -370,7 +375,7 @@ export function getSetupCommandSource( // 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)) { + if (rawPolicy === undefined && !yamlFileExists && localSetup && hasLegacyLocalScripts(repo)) { return { source: 'local', command: localSetup } } diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index 1914f8d7879..cbff5b379c0 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -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 )