From 5faaaa4f9cdb60bfba143fb31abce22312b6b0af Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:32:06 -0700 Subject: [PATCH] fix(source-control): distinguish loading state from unavailable lookup Require head branch presence in shouldAttemptCreateHostedReviewForIntent to separate real unavailable-lookup results from loading placeholders, which share the same outcome/reason pair but lack a branch name. --- .../source-control-create-pr-intent-flow.test.ts | 7 ++++++- .../source-control-create-pr-intent-flow.ts | 6 +++++- .../source-control-create-pr-intent-switch.spec.ts | 12 +++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts index 75ecc278809..c28e428fb4d 100644 --- a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.test.ts @@ -296,9 +296,14 @@ describe('source-control Create PR intent flow helpers', () => { canCreate: false, blockedReason: null, nextAction: null, - reviewLookupOutcome: 'unavailable' as const + reviewLookupOutcome: 'unavailable' as const, + head: 'feature-branch' } expect(shouldAttemptCreateHostedReviewForIntent(unavailable)).toBe(true) + // Loading placeholders share the unavailable/null-reason shape but carry no branch. + expect(shouldAttemptCreateHostedReviewForIntent({ ...unavailable, head: undefined })).toBe( + false + ) expect(shouldGenerateHostedReviewDetailsForIntent(unavailable)).toBe(false) expect( shouldGenerateHostedReviewDetailsForIntent({ diff --git a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts index 13f337c8544..03abd03a755 100644 --- a/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts +++ b/src/renderer/src/components/right-sidebar/source-control-create-pr-intent-flow.ts @@ -171,7 +171,11 @@ export function shouldAttemptCreateHostedReviewForIntent( ): boolean { return ( eligibility.canCreate || - (eligibility.reviewLookupOutcome === 'unavailable' && eligibility.blockedReason === null) + // Why: `head` separates a real unavailable-lookup result from a loading + // placeholder, which carries the same outcome/reason pair but no branch. + (eligibility.reviewLookupOutcome === 'unavailable' && + eligibility.blockedReason === null && + Boolean(eligibility.head?.trim())) ) } diff --git a/tests/e2e/source-control-create-pr-intent-switch.spec.ts b/tests/e2e/source-control-create-pr-intent-switch.spec.ts index be6115711ec..281cdfccce8 100644 --- a/tests/e2e/source-control-create-pr-intent-switch.spec.ts +++ b/tests/e2e/source-control-create-pr-intent-switch.spec.ts @@ -25,6 +25,14 @@ async function writeEvidence( }) } +function removeOriginRemoteIfPresent(cwd: string): void { + try { + execFileSync('git', ['remote', 'remove', 'origin'], { cwd, stdio: 'pipe' }) + } catch { + // Why: setup and teardown must not fail just because origin is absent. + } +} + test.describe('Source Control Create PR intent worktree switching', () => { test.describe.configure({ mode: 'serial' }) @@ -228,9 +236,11 @@ test.describe('Source Control Create PR intent worktree switching', () => { const remoteRoot = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-create-pr-remote-')) const remotePath = path.join(remoteRoot, 'origin.git') execFileSync('git', ['init', '--bare', remotePath]) + // Why: the seeded worktree may already define origin, so make the add idempotent. + removeOriginRemoteIfPresent(prWorktreePath) execFileSync('git', ['remote', 'add', 'origin', remotePath], { cwd: prWorktreePath }) registerPostElectronShutdownCleanup(async () => { - execFileSync('git', ['remote', 'remove', 'origin'], { cwd: prWorktreePath }) + removeOriginRemoteIfPresent(prWorktreePath) rmSync(remoteRoot, { recursive: true, force: true }) }) createStagedCommitMessageChange(prWorktreePath)