diff --git a/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx index 11e094472a6..916abbdcb69 100644 --- a/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx +++ b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx @@ -307,6 +307,8 @@ describe('Activity portal pane switching', () => { }) it('releases a latched readiness once the terminal attaches', async () => { + // Drive rAF explicitly — wall-clock waits flake under CI load. + const frames = installAnimationFrameController() const target = document.createElement('div') document.body.append(target) const buildRoot = (mode: 'hidden' | 'sibling' | 'ready'): void => { @@ -352,15 +354,22 @@ describe('Activity portal pane switching', () => { root = createRoot(document.createElement('div')) await act(async () => { root.render() - await new Promise((resolve) => setTimeout(resolve, 180)) }) + for (let frame = 0; frame < 40; frame += 1) { + if (frames.pending() === 0 && statuses.at(-1) === 'unavailable') { + break + } + await frames.flush() + } expect(statuses.at(-1)).toBe('unavailable') churning = false await act(async () => { buildRoot('ready') - await new Promise((resolve) => setTimeout(resolve, 40)) }) + for (let frame = 0; frame < 10 && statuses.at(-1) !== 'ready'; frame += 1) { + await frames.flush() + } expect(statuses.at(-1)).toBe('ready') }) }) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index d349d789aa0..6f5a4cc5fb1 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -265,11 +265,14 @@ import { getCreatePrIntentStagePaths, resolveCreatePrIntentReviewBase, resolveCreatePrIntentRemoteStep, + shouldAttemptCreateHostedReviewForIntent, + shouldGenerateHostedReviewDetailsForIntent, type CreatePrIntentRunToken } from './source-control-create-pr-intent-flow' import { resolveVisibleCreatePrHeaderAction } from './source-control-create-pr-intent-state' import { resolveBlockedCreateReviewNoticeMessage } from './source-control-create-review-blocked-action' import { + buildCreatePrIntentUnavailableEligibility, buildLoadingHostedReviewCreationEligibility, buildLocalBlockerHostedReviewCreationEligibility, resolveHostedReviewCreationProviderForTarget @@ -1580,6 +1583,7 @@ function SourceControlInner(): React.JSX.Element { resolveHostedReviewCreationProviderForTarget( hostedReviewCreationProviderHintRef.current, { repoId: activeRepoId, worktreeId: activeWorktreeId ?? null, branch: branchName }, + // Why: provisional already infers the remote host and defaults to github; never fall back to unsupported mid-load. provisionalHostedReviewProvider ) ) @@ -3409,7 +3413,7 @@ function SourceControlInner(): React.JSX.Element { token: CreatePrIntentRunToken, eligibility: HostedReviewCreationEligibility ): Promise => { - if (!activeRepo || !token.branch || !eligibility.canCreate) { + if (!activeRepo || !token.branch || !shouldAttemptCreateHostedReviewForIntent(eligibility)) { return false } @@ -3441,6 +3445,7 @@ function SourceControlInner(): React.JSX.Element { } if ( + shouldGenerateHostedReviewDetailsForIntent(eligibility) && hasConfiguredSourceControlTextGenerationDefaults({ actionId: 'pullRequest', settings, @@ -3651,23 +3656,43 @@ function SourceControlInner(): React.JSX.Element { if (!activeRepo || !token.branch) { return null } - const result = await getHostedReviewCreationEligibility({ - repoPath: activeRepo.path, - repoId: activeRepo.id, - worktreePath: token.worktreePath, - branch: token.branch, - base: token.baseRef ?? null, - hasUncommittedChanges, - hasUpstream: upstreamStatus?.hasUpstream, - ahead: upstreamStatus?.ahead, - behind: upstreamStatus?.behind, - linkedGitHubPR, - fallbackGitHubPR: fallbackGitHubPRNumber, - linkedGitLabMR, - linkedBitbucketPR, - linkedAzureDevOpsPR, - linkedGiteaPR - }) + let result: HostedReviewCreationEligibility + try { + result = await getHostedReviewCreationEligibility({ + repoPath: activeRepo.path, + repoId: activeRepo.id, + worktreePath: token.worktreePath, + branch: token.branch, + base: token.baseRef ?? null, + hasUncommittedChanges, + hasUpstream: upstreamStatus?.hasUpstream, + ahead: upstreamStatus?.ahead, + behind: upstreamStatus?.behind, + linkedGitHubPR, + fallbackGitHubPR: fallbackGitHubPRNumber, + linkedGitLabMR, + linkedBitbucketPR, + linkedAzureDevOpsPR, + linkedGiteaPR + }) + } catch (error) { + console.warn('[SourceControl] Create PR intent eligibility failed', error) + // Why: when local status still yields a prep step (dirty/push/sync), keep the intent + // moving. If nothing actionable can be synthesized, rethrow so the outer intent + // catch surfaces a retry notice instead of leaving "Preparing…" stuck forever. + const fallback = buildCreatePrIntentUnavailableEligibility(token.provider, { + branch: token.branch, + baseRef: token.baseRef, + hasUncommittedChanges, + hasUpstream: upstreamStatus?.hasUpstream, + ahead: upstreamStatus?.ahead, + behind: upstreamStatus?.behind + }) + if (!fallback) { + throw error + } + result = fallback + } setHostedReviewCreationState({ repoId: activeRepo.id, worktreeId: token.worktreeId, @@ -3739,6 +3764,9 @@ function SourceControlInner(): React.JSX.Element { worktreeId: activeWorktreeId, worktreePath, branch: branchName, + // Why: token carries the same provisional provider used for UI copy so a failed + // eligibility IPC can synthesize local prep steps for the correct host. + provider: provisionalHostedReviewProvider, // Why: intent crosses async commit/push steps, so the base stays tied to what was selected when the run started. baseRef: effectiveBaseRef ?? null }) @@ -3939,19 +3967,25 @@ function SourceControlInner(): React.JSX.Element { } } - const branchAhead = await refreshBranchCompareForCreatePrIntent(token) - if (abortIfStale()) { - return - } let eligibility = await readHostedReviewCreationEligibilityForIntent({ token, hasUncommittedChanges: latestStatusEntries.length > 0, upstreamStatus: latestUpstreamStatus }) - if (abortIfStale() || !eligibility) { + if (abortIfStale()) { return } - if (eligibility.canCreate) { + if (!eligibility) { + setCreatePrIntentNoticeForWorktree(token.worktreeId, { + tone: 'destructive', + message: translate( + 'auto.components.right.sidebar.SourceControl.d7492cafce', + 'Could not refresh Source Control. Retry Create PR.' + ) + }) + return + } + if (shouldAttemptCreateHostedReviewForIntent(eligibility)) { await createHostedReviewForCreatePrIntent(token, eligibility) if (abortIfStale()) { return @@ -3963,6 +3997,13 @@ function SourceControlInner(): React.JSX.Element { return } + const branchAhead = + eligibility.blockedReason === 'no_upstream' + ? await refreshBranchCompareForCreatePrIntent(token) + : undefined + if (abortIfStale()) { + return + } const remoteStep = resolveCreatePrIntentRemoteStep({ upstreamStatus: latestUpstreamStatus, hostedReviewCreation: eligibility, @@ -4047,19 +4088,23 @@ function SourceControlInner(): React.JSX.Element { if (abortIfStale()) { return } - if (eligibility?.canCreate) { + if (eligibility && shouldAttemptCreateHostedReviewForIntent(eligibility)) { await createHostedReviewForCreatePrIntent(token, eligibility) if (abortIfStale()) { return } return } + // Why: prefer the blocked-reason notice (incl. unavailable lookup) over a generic stop. + const blockedNotice = resolveBlockedCreateReviewNoticeMessage(eligibility) setCreatePrIntentNoticeForWorktree(token.worktreeId, { - tone: 'muted', - message: translate( - 'auto.components.right.sidebar.SourceControl.995c5e67ec', - 'Review setup needs attention.' - ) + tone: blockedNotice ? 'destructive' : 'muted', + message: + blockedNotice ?? + translate( + 'auto.components.right.sidebar.SourceControl.995c5e67ec', + 'Review setup needs attention.' + ) }) } catch (error) { console.warn('[SourceControl] Create PR intent failed', error) @@ -4106,6 +4151,7 @@ function SourceControlInner(): React.JSX.Element { readHostedReviewCreationEligibilityForIntent, refreshGitStatusForCreatePrIntent, refreshBranchCompareForCreatePrIntent, + provisionalHostedReviewProvider, remoteStatus, runRemoteAction, setCreatePrIntentNoticeForWorktree, 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 753ebaaa750..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 @@ -8,7 +8,9 @@ import { getCreatePrIntentCommitFailureNoticeMessage, getCreatePrIntentStagePaths, resolveCreatePrIntentReviewBase, - resolveCreatePrIntentRemoteStep + resolveCreatePrIntentRemoteStep, + shouldAttemptCreateHostedReviewForIntent, + shouldGenerateHostedReviewDetailsForIntent } from './source-control-create-pr-intent-flow' import type { GitStatusEntry } from '../../../../shared/types' @@ -21,6 +23,7 @@ describe('source-control Create PR intent flow helpers', () => { worktreeId: 'wt-1', worktreePath: '/repo', branch: 'feature', + provider: 'github', baseRef: 'origin/main' }) @@ -44,7 +47,8 @@ describe('source-control Create PR intent flow helpers', () => { repoId: 'repo-1', worktreeId: 'wt-1', worktreePath: '/repo', - branch: 'feature/pr' + branch: 'feature/pr', + provider: 'github' }) expect(createPrIntentGitStatusMatchesToken(token, { branch: 'refs/heads/feature/pr' })).toBe( @@ -63,7 +67,8 @@ describe('source-control Create PR intent flow helpers', () => { repoId: 'repo-1', worktreeId: 'wt-1', worktreePath: wt1Path, - branch: 'feature/pr' + branch: 'feature/pr', + provider: 'github' }) expect( @@ -92,6 +97,7 @@ describe('source-control Create PR intent flow helpers', () => { worktreeId: 'wt-1', worktreePath, branch: 'feature/pr', + provider: 'github', baseRef: 'refs/remotes/origin/main' }) @@ -283,6 +289,41 @@ describe('source-control Create PR intent flow helpers', () => { ).toBe('blocked') }) + it('uses main preflight as the final lookup authority after preparation', () => { + const unavailable = { + provider: 'github' as const, + review: null, + canCreate: false, + blockedReason: null, + nextAction: null, + 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({ + ...unavailable, + canCreate: true, + reviewLookupOutcome: 'not_found' + }) + ).toBe(true) + expect( + shouldAttemptCreateHostedReviewForIntent({ + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push', + reviewLookupOutcome: 'unavailable' + }) + ).toBe(false) + }) + it('surfaces the commit failure summary in the Create PR intent notice', () => { expect( getCreatePrIntentCommitFailureNoticeMessage( 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 bafba4c8504..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 @@ -2,7 +2,10 @@ import { isBehindOnlyUpstream, shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status' -import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review' +import type { + HostedReviewCreationEligibility, + HostedReviewProvider +} from '../../../../shared/hosted-review' import { normalizeHostedReviewBaseRef, normalizeHostedReviewHeadRef @@ -24,6 +27,7 @@ export type CreatePrIntentRunToken = { worktreeId: string worktreePath: string branch: string + provider: HostedReviewProvider baseRef?: string | null startedAt: number } @@ -162,6 +166,25 @@ export function resolveCreatePrIntentRemoteStep({ return 'none' } +export function shouldAttemptCreateHostedReviewForIntent( + eligibility: HostedReviewCreationEligibility +): boolean { + return ( + eligibility.canCreate || + // 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())) + ) +} + +export function shouldGenerateHostedReviewDetailsForIntent( + eligibility: HostedReviewCreationEligibility +): boolean { + return eligibility.canCreate +} + export function getCreatePrIntentCommitFailureNoticeMessage( commitError: string | null | undefined, copy: { diff --git a/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.test.ts b/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.test.ts index e2bd937f1c2..c40468b9e49 100644 --- a/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it } from 'vitest' import { canClickBlockedCreateReviewReason, - resolveBlockedCreateReviewNoticeMessage + resolveBlockedCreateReviewNoticeMessage, + resolveUnavailableCreateReviewLookupNoticeMessage } from './source-control-create-review-blocked-action' import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review' @@ -62,6 +63,51 @@ describe('source-control-create-review-blocked-action', () => { ) }) + it('preserves a known dirty-tree prerequisite when review lookup is unavailable', () => { + expect( + resolveBlockedCreateReviewNoticeMessage( + eligibility({ + blockedReason: 'dirty', + nextAction: 'commit', + reviewLookupOutcome: 'unavailable' + }) + ) + ).toBe('Create PR failed: commit or discard local changes before creating a pull request.') + }) + + it('preserves authentication guidance when review lookup is unavailable', () => { + expect( + resolveBlockedCreateReviewNoticeMessage( + eligibility({ + provider: 'gitlab', + blockedReason: 'auth_required', + nextAction: 'authenticate', + reviewLookupOutcome: 'unavailable' + }) + ) + ).toBe( + 'Create MR failed: GitLab is not authenticated. Next step: Run glab auth login in this environment.' + ) + }) + + it('reports unavailable review lookup authority when no local blocker is known', () => { + expect(resolveUnavailableCreateReviewLookupNoticeMessage('gitlab')).toBe( + 'Create MR failed: Orca could not confirm whether this branch already has a merge request. Retry once the GitLab lookup succeeds.' + ) + expect( + resolveBlockedCreateReviewNoticeMessage( + eligibility({ + provider: 'gitlab', + blockedReason: null, + nextAction: null, + reviewLookupOutcome: 'unavailable' + }) + ) + ).toBe( + 'Create MR failed: Orca could not confirm whether this branch already has a merge request. Retry once the GitLab lookup succeeds.' + ) + }) + it('returns null when the blocked reason should remain non-clickable', () => { expect( resolveBlockedCreateReviewNoticeMessage( diff --git a/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.ts b/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.ts index 4bae174619b..1e5c02efb84 100644 --- a/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.ts +++ b/src/renderer/src/components/right-sidebar/source-control-create-review-blocked-action.ts @@ -36,6 +36,13 @@ export function resolveHostedReviewAuthInstruction(provider: HostedReviewProvide return 'Run gh auth login' } +export function resolveUnavailableCreateReviewLookupNoticeMessage( + provider: HostedReviewProvider +): string { + const copy = localizedHostedReviewCopy(resolveSupportedHostedReviewCopyProvider(provider)) + return `Create ${copy.shortLabel} failed: Orca could not confirm whether this branch already has a ${copy.reviewLabel}. Retry once the ${copy.providerName} lookup succeeds.` +} + export function resolveBlockedCreateReviewNoticeMessage( eligibility: HostedReviewCreationEligibility | null | undefined ): string | null { @@ -43,6 +50,9 @@ export function resolveBlockedCreateReviewNoticeMessage( return null } const reason = eligibility.blockedReason + if (eligibility.reviewLookupOutcome === 'unavailable' && reason === null) { + return resolveUnavailableCreateReviewLookupNoticeMessage(eligibility.provider) + } if (!canClickBlockedCreateReviewReason(reason)) { return null } diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.create-pr-intent.test.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.create-pr-intent.test.ts new file mode 100644 index 00000000000..be66db37a06 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.create-pr-intent.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, it } from 'vitest' +import { + resolveDropdownItems, + type DropdownActionInputs, + type DropdownItem +} from './source-control-dropdown-items' + +// Why: a shared defaults object keeps each case row terse while making the +// "this is the one knob that differs from the baseline" intent obvious. +function inputs(overrides: Partial = {}): DropdownActionInputs { + return { + stagedCount: 0, + hasUnstagedChanges: false, + hasStageableChanges: false, + hasPartiallyStagedChanges: false, + hasMessage: false, + hasUnresolvedConflicts: false, + isCommitting: false, + isRemoteOperationActive: false, + upstreamStatus: undefined, + ...overrides + } +} + +describe('resolveDropdownItems Create PR intent', () => { + it('enables the push-before-PR recovery action when review creation is only blocked by unpushed commits', () => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push', + reviewLookupOutcome: 'not_found' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.disabled).toBe(false) + expect(byKind.create_pr.hint).toBe('Push first') + expect(byKind.push_create_pr.label).toBe('Push before PR') + expect(byKind.push_create_pr.disabled).toBe(false) + }) + + it.each([ + { + name: 'push', + provider: 'github' as const, + upstreamStatus: { hasUpstream: true as const, ahead: 2, behind: 0 }, + blockedReason: 'needs_push' as const, + expectedTitle: 'Push local commits before creating a pull request' + }, + { + name: 'force push', + provider: 'gitlab' as const, + upstreamStatus: { + hasUpstream: true as const, + upstreamName: 'origin/feature', + ahead: 2, + behind: 1, + behindCommitsArePatchEquivalent: true + }, + blockedReason: 'needs_sync' as const, + expectedTitle: 'Force push with lease before creating a merge request' + } + ])( + 'keeps $name-before-review recovery available when review lookup is unavailable', + ({ provider, upstreamStatus, blockedReason, expectedTitle }) => { + const items = resolveDropdownItems( + inputs({ + branchCommitsAhead: 2, + upstreamStatus, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason, + nextAction: blockedReason === 'needs_push' ? 'push' : 'sync', + reviewLookupOutcome: 'unavailable' + } + }) + ) + const pushCreate = items.find((item): item is DropdownItem => item.kind === 'push_create_pr') + + expect(pushCreate?.disabled).toBe(false) + expect(pushCreate?.title).toBe(expectedTitle) + expect(pushCreate?.hint).toBeUndefined() + } + ) + + it('uses GitLab MR copy for create and push-before-create rows', () => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider: 'gitlab', + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push', + reviewLookupOutcome: 'not_found' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.label).toBe('Create MR') + expect(byKind.create_pr.hint).toBe('Push first') + expect(byKind.create_pr.disabled).toBe(false) + expect(byKind.push_create_pr.label).toBe('Push before MR') + expect(byKind.push_create_pr.title).toBe('Push local commits before creating a merge request') + expect(byKind.push_create_pr.disabled).toBe(false) + }) + + it.each(['azure-devops', 'gitea'] as const)( + 'enables push-before-PR recovery for %s review creation', + (provider) => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason: 'needs_push', + nextAction: 'push', + reviewLookupOutcome: 'not_found' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.label).toBe('Create PR') + expect(byKind.create_pr.hint).toBe('Push first') + expect(byKind.create_pr.disabled).toBe(false) + expect(byKind.push_create_pr.label).toBe('Push before PR') + expect(byKind.push_create_pr.title).toBe('Push local commits before creating a pull request') + expect(byKind.push_create_pr.disabled).toBe(false) + } + ) + + it.each([ + ['azure-devops', 'Set ORCA_AZURE_DEVOPS_TOKEN in this environment'], + ['gitea', 'Set ORCA_GITEA_TOKEN in this environment'] + ] as const)('uses token auth copy when %s PR creation needs authentication', (provider, hint) => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + provider, + review: null, + canCreate: false, + blockedReason: 'auth_required', + nextAction: 'authenticate', + reviewLookupOutcome: 'not_found' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.hint).toBe(hint) + }) + + it('uses GitLab auth copy when MR creation needs authentication', () => { + const items = resolveDropdownItems( + inputs({ + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + provider: 'gitlab', + review: null, + canCreate: false, + blockedReason: 'auth_required', + nextAction: 'authenticate', + reviewLookupOutcome: 'not_found' + } + }) + ) + const byKind = Object.fromEntries( + items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) + ) + expect(byKind.create_pr.hint).toBe('Run glab auth login in this environment') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts index d97707dd273..14573a50ec2 100644 --- a/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-dropdown-items.test.ts @@ -663,125 +663,6 @@ describe('resolveDropdownItems', () => { 'Try a fast-forward pull; git may reject local commits' ) }) - - it('enables the push-before-PR recovery action when review creation is only blocked by unpushed commits', () => { - const items = resolveDropdownItems( - inputs({ - upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, - hostedReviewCreation: { - provider: 'github', - review: null, - canCreate: false, - blockedReason: 'needs_push', - nextAction: 'push', - reviewLookupOutcome: 'not_found' - } - }) - ) - const byKind = Object.fromEntries( - items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) - ) - expect(byKind.create_pr.disabled).toBe(false) - expect(byKind.create_pr.hint).toBe('Push first') - expect(byKind.push_create_pr.label).toBe('Push before PR') - expect(byKind.push_create_pr.disabled).toBe(false) - }) - - it('uses GitLab MR copy for create and push-before-create rows', () => { - const items = resolveDropdownItems( - inputs({ - upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, - hostedReviewCreation: { - provider: 'gitlab', - review: null, - canCreate: false, - blockedReason: 'needs_push', - nextAction: 'push', - reviewLookupOutcome: 'not_found' - } - }) - ) - const byKind = Object.fromEntries( - items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) - ) - expect(byKind.create_pr.label).toBe('Create MR') - expect(byKind.create_pr.hint).toBe('Push first') - expect(byKind.create_pr.disabled).toBe(false) - expect(byKind.push_create_pr.label).toBe('Push before MR') - expect(byKind.push_create_pr.title).toBe('Push local commits before creating a merge request') - expect(byKind.push_create_pr.disabled).toBe(false) - }) - - it.each(['azure-devops', 'gitea'] as const)( - 'enables push-before-PR recovery for %s review creation', - (provider) => { - const items = resolveDropdownItems( - inputs({ - upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 }, - hostedReviewCreation: { - provider, - review: null, - canCreate: false, - blockedReason: 'needs_push', - nextAction: 'push', - reviewLookupOutcome: 'not_found' - } - }) - ) - const byKind = Object.fromEntries( - items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) - ) - expect(byKind.create_pr.label).toBe('Create PR') - expect(byKind.create_pr.hint).toBe('Push first') - expect(byKind.create_pr.disabled).toBe(false) - expect(byKind.push_create_pr.label).toBe('Push before PR') - expect(byKind.push_create_pr.title).toBe('Push local commits before creating a pull request') - expect(byKind.push_create_pr.disabled).toBe(false) - } - ) - - it.each([ - ['azure-devops', 'Set ORCA_AZURE_DEVOPS_TOKEN in this environment'], - ['gitea', 'Set ORCA_GITEA_TOKEN in this environment'] - ] as const)('uses token auth copy when %s PR creation needs authentication', (provider, hint) => { - const items = resolveDropdownItems( - inputs({ - upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, - hostedReviewCreation: { - provider, - review: null, - canCreate: false, - blockedReason: 'auth_required', - nextAction: 'authenticate', - reviewLookupOutcome: 'not_found' - } - }) - ) - const byKind = Object.fromEntries( - items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) - ) - expect(byKind.create_pr.hint).toBe(hint) - }) - - it('uses GitLab auth copy when MR creation needs authentication', () => { - const items = resolveDropdownItems( - inputs({ - upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, - hostedReviewCreation: { - provider: 'gitlab', - review: null, - canCreate: false, - blockedReason: 'auth_required', - nextAction: 'authenticate', - reviewLookupOutcome: 'not_found' - } - }) - ) - const byKind = Object.fromEntries( - items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e]) - ) - expect(byKind.create_pr.hint).toBe('Run glab auth login in this environment') - }) }) // Why: PR #8196 — drive the real push-target resolution the component uses so diff --git a/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.test.ts b/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.test.ts index 13d7bc5dac9..ec4031c7aed 100644 --- a/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + buildCreatePrIntentUnavailableEligibility, buildLocalBlockerHostedReviewCreationEligibility, resolveHostedReviewCreationProviderForTarget } from './source-control-hosted-review-creation-eligibility-snapshot' @@ -32,7 +33,7 @@ describe('resolveHostedReviewCreationProviderForTarget', () => { }) describe('buildLocalBlockerHostedReviewCreationEligibility', () => { - it('reports dirty without offering create intent when the review lookup failed', () => { + it('reports dirty with unavailable lookup while still allowing prepare-only Create PR intent', () => { const eligibility = buildLocalBlockerHostedReviewCreationEligibility('github', { ...featureBranch, hasUncommittedChanges: true, @@ -45,7 +46,7 @@ describe('buildLocalBlockerHostedReviewCreationEligibility', () => { nextAction: 'commit', reviewLookupOutcome: 'unavailable' }) - // Why: branch guidance can remain specific, but a failed lookup cannot authorize creation. + // Why: branch prep is safe without lookup authority; final create stays fail-closed in main. expect( resolveCreatePrIntentEligibility({ stagedCount: 1, @@ -56,7 +57,7 @@ describe('buildLocalBlockerHostedReviewCreationEligibility', () => { hostedReviewCreation: eligibility, branchCommitsAhead: 0 }) - ).toEqual({ eligible: false, kind: null }) + ).toEqual({ eligible: true, kind: 'dirty' }) }) it('prefers dirty over no_upstream when both apply, matching main-process ordering', () => { @@ -122,6 +123,19 @@ describe('buildLocalBlockerHostedReviewCreationEligibility', () => { ).toBeNull() }) + it('returns null when the default branch is unknown', () => { + expect( + buildLocalBlockerHostedReviewCreationEligibility('github', { + branch: 'main', + baseRef: null, + hasUncommittedChanges: true, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).toBeNull() + }) + it('returns null for a detached HEAD', () => { expect( buildLocalBlockerHostedReviewCreationEligibility('github', { @@ -185,3 +199,72 @@ describe('buildLocalBlockerHostedReviewCreationEligibility', () => { ).toBeNull() }) }) + +describe('buildCreatePrIntentUnavailableEligibility', () => { + it('keeps a rejected click-time probe moving through push and final preflight', () => { + expect( + buildCreatePrIntentUnavailableEligibility('github', { + ...featureBranch, + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 2, + behind: 0 + }) + ).toMatchObject({ + blockedReason: 'needs_push', + nextAction: 'push', + reviewLookupOutcome: 'unavailable' + }) + expect( + buildCreatePrIntentUnavailableEligibility('github', { + ...featureBranch, + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).toMatchObject({ + blockedReason: null, + nextAction: null, + reviewLookupOutcome: 'unavailable' + }) + }) + + it('never synthesizes final intent eligibility for the default branch', () => { + expect( + buildCreatePrIntentUnavailableEligibility('github', { + branch: 'main', + baseRef: 'origin/main', + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 1, + behind: 0 + }) + ).toBeNull() + }) + + it('never synthesizes intent eligibility when the default branch is unknown', () => { + expect( + buildCreatePrIntentUnavailableEligibility('github', { + branch: 'main', + baseRef: null, + hasUncommittedChanges: true, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).toBeNull() + }) + + it('never synthesizes intent eligibility for an unsupported remote provider', () => { + expect( + buildCreatePrIntentUnavailableEligibility('bitbucket', { + ...featureBranch, + hasUncommittedChanges: true, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + ).toBeNull() + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.ts b/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.ts index 0857f94f6a0..b964323830e 100644 --- a/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.ts +++ b/src/renderer/src/components/right-sidebar/source-control-hosted-review-creation-eligibility-snapshot.ts @@ -5,6 +5,33 @@ import type { HostedReviewProvider } from '../../../../shared/hosted-review' +type UnavailableHostedReviewStatus = { + branch: string | null | undefined + baseRef: string | null | undefined + hasUncommittedChanges: boolean + hasUpstream: boolean | undefined + ahead: number | undefined + behind: number | undefined +} + +function resolveUnavailableHostedReviewBranch( + provider: HostedReviewProvider, + status: UnavailableHostedReviewStatus +): string | null { + const branch = status.branch?.trim() ?? '' + const baseBranch = normalizeHostedReviewBaseRef(status.baseRef ?? '').trim() + if ( + branch === '' || + branch === 'HEAD' || + baseBranch === '' || + !supportsHostedReviewCreation(provider) || + branch.toLowerCase() === baseBranch.toLowerCase() + ) { + return null + } + return branch +} + export function buildLoadingHostedReviewCreationEligibility( provider: HostedReviewProvider ): HostedReviewCreationEligibility { @@ -41,44 +68,31 @@ export function resolveHostedReviewCreationProviderForTarget( * times out, so the UI can show branch guidance without treating the failed * lookup as authority to create a review. * - * Mirrors the main process's own lookup-failure fallback exactly — both its - * `canReturnLocalBlocker` guard and its blocker ordering - * (`src/main/source-control/hosted-review-creation.ts`). The guard is - * load-bearing: without it a failed probe on the default branch or a detached - * HEAD would synthesize `dirty`/`commit` and surface an *enabled* Create PR - * that commits onto the base branch, where the real probe returns - * `default_branch`/`detached_head` (disabled). Returns null when no local - * blocker is determinable (unknown upstream, ahead-only, fully synced) so the - * caller can surface the retryable state — matching main, which won't offer a - * push it can't first auth-check. + * Blocker ordering matches main (`dirty` → `no_upstream` → `needs_sync`). The + * branch/base guard is load-bearing and intentionally stricter than main when + * base is unknown: without a known base, a failed probe must not synthesize + * `dirty`/`commit` on what might be the default branch (main would return + * `default_branch`/`detached_head` and keep Create disabled). Returns null + * when no local blocker is determinable (unknown base, unknown upstream, + * ahead-only, fully synced) so the caller can surface the retryable state — + * matching main, which won't offer a push it can't first auth-check. */ export function buildLocalBlockerHostedReviewCreationEligibility( provider: HostedReviewProvider, - status: { - branch: string | null | undefined - baseRef: string | null | undefined - hasUncommittedChanges: boolean - hasUpstream: boolean | undefined - ahead: number | undefined - behind: number | undefined - } + status: UnavailableHostedReviewStatus ): HostedReviewCreationEligibility | null { - const branch = status.branch?.trim() ?? '' - const baseBranch = normalizeHostedReviewBaseRef(status.baseRef ?? '').trim() - const canReturnLocalBlocker = - branch !== '' && - branch !== 'HEAD' && - supportsHostedReviewCreation(provider) && - (baseBranch === '' || branch.toLowerCase() !== baseBranch.toLowerCase()) && - (status.hasUncommittedChanges || status.hasUpstream !== true || (status.behind ?? 0) > 0) - if (!canReturnLocalBlocker) { + const branch = resolveUnavailableHostedReviewBranch(provider, status) + if ( + !branch || + (!status.hasUncommittedChanges && status.hasUpstream === true && (status.behind ?? 0) === 0) + ) { return null } const base = { provider, review: null, canCreate: false as const, - defaultBaseRef: null, + defaultBaseRef: normalizeHostedReviewBaseRef(status.baseRef ?? '').trim(), head: branch, // Why: local Git blockers cannot prove that a hosted review does not exist. reviewLookupOutcome: 'unavailable' as const @@ -96,3 +110,29 @@ export function buildLocalBlockerHostedReviewCreationEligibility( } return null } + +export function buildCreatePrIntentUnavailableEligibility( + provider: HostedReviewProvider, + status: UnavailableHostedReviewStatus +): HostedReviewCreationEligibility | null { + const localBlocker = buildLocalBlockerHostedReviewCreationEligibility(provider, status) + if (localBlocker) { + return localBlocker + } + const branch = resolveUnavailableHostedReviewBranch(provider, status) + if (!branch || status.hasUpstream !== true) { + return null + } + const base = { + provider, + review: null, + canCreate: false as const, + defaultBaseRef: normalizeHostedReviewBaseRef(status.baseRef ?? '').trim(), + head: branch, + reviewLookupOutcome: 'unavailable' as const + } + if ((status.ahead ?? 0) > 0) { + return { ...base, blockedReason: 'needs_push', nextAction: 'push' } + } + return { ...base, blockedReason: null, nextAction: null } +} diff --git a/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts b/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts index c56892b71ec..5c526e047b6 100644 --- a/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-primary-action.create-pr-intent.test.ts @@ -177,6 +177,32 @@ describe('resolvePrimaryAction Create PR intent', () => { }) }) + it('returns Create PR intent for a dirty tree when review lookup is unavailable', () => { + const input = inputs({ + hasUnstagedChanges: true, + hasStageableChanges: true, + upstreamStatus: upstreamInSync, + hostedReviewCreation: { + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit', + defaultBaseRef: 'main', + reviewLookupOutcome: 'unavailable' + } + }) + + expect(resolvePrimaryAction(input)).toMatchObject({ + kind: 'create_pr_intent', + disabled: false + }) + expect(resolveCreatePrHeaderAction(input)).toMatchObject({ + kind: 'create_pr_intent', + disabled: false + }) + }) + it('returns Create PR intent for staged changes without a message so the flow can request one', () => { const input = inputs({ stagedCount: 1, diff --git a/src/shared/source-control-create-review-intent.test.ts b/src/shared/source-control-create-review-intent.test.ts new file mode 100644 index 00000000000..4763117a5be --- /dev/null +++ b/src/shared/source-control-create-review-intent.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest' +import { + resolveCreateReviewIntentEligibility, + type CreateReviewIntentKind +} from './source-control-create-review-intent' +import type { HostedReviewCreationBlockedReason } from './hosted-review' +import type { GitUpstreamStatus } from './git-status-types' + +function unavailableEligibility(blockedReason: HostedReviewCreationBlockedReason) { + return { + provider: 'github' as const, + review: null, + canCreate: false, + blockedReason, + nextAction: null, + defaultBaseRef: 'main', + reviewLookupOutcome: 'unavailable' as const + } +} + +describe('resolveCreateReviewIntentEligibility', () => { + it('rejects unavailable eligibility when the default branch is unknown', () => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount: 1, + hasStageableChanges: true, + hasMessage: true, + hasUnresolvedConflicts: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + ...unavailableEligibility('dirty'), + defaultBaseRef: null + } + }) + ).toEqual({ eligible: false, kind: null }) + }) + + it('rejects unavailable eligibility when the default branch is blank', () => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount: 1, + hasStageableChanges: true, + hasMessage: true, + hasUnresolvedConflicts: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + ...unavailableEligibility('dirty'), + defaultBaseRef: ' ' + } + }) + ).toEqual({ eligible: false, kind: null }) + }) + + it('stays ineligible when no local blocker remains under unavailable lookup', () => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount: 0, + hasStageableChanges: false, + hasMessage: true, + hasUnresolvedConflicts: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: { + ...unavailableEligibility('dirty'), + blockedReason: null + } + }) + ).toEqual({ eligible: false, kind: null }) + }) + + it('keeps dirty local preparation eligible when review lookup is unavailable', () => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount: 0, + hasStageableChanges: true, + hasMessage: true, + hasUnresolvedConflicts: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: unavailableEligibility('dirty') + }) + ).toEqual({ eligible: true, kind: 'dirty' }) + }) + + it('still requires a message before committing staged changes without lookup authority', () => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount: 1, + hasStageableChanges: false, + hasMessage: false, + hasUnresolvedConflicts: false, + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 }, + hostedReviewCreation: unavailableEligibility('dirty') + }) + ).toEqual({ eligible: true, kind: 'message_required' }) + }) + + it.each<{ + blockedReason: HostedReviewCreationBlockedReason + blockedKind: CreateReviewIntentKind + stagedCount?: number + hasStageableChanges?: boolean + branchCommitsAhead?: number + upstreamStatus?: GitUpstreamStatus + }>([ + { + blockedReason: 'no_upstream', + blockedKind: 'no_upstream', + branchCommitsAhead: 1, + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 } + }, + { + blockedReason: 'needs_push', + blockedKind: 'needs_push', + upstreamStatus: { + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 1, + behind: 0 + } + }, + { + blockedReason: 'needs_sync', + blockedKind: 'needs_sync', + upstreamStatus: { + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 0, + behind: 1 + } + }, + { + blockedReason: 'needs_sync', + blockedKind: 'force_push', + branchCommitsAhead: 1, + upstreamStatus: { + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 2, + behind: 1, + behindCommitsArePatchEquivalent: true + } + } + ])( + 'keeps recoverable $blockedKind preparation eligible when review lookup is unavailable', + ({ + blockedReason, + blockedKind, + stagedCount = 0, + hasStageableChanges = false, + branchCommitsAhead, + upstreamStatus + }) => { + expect( + resolveCreateReviewIntentEligibility({ + stagedCount, + hasStageableChanges, + hasMessage: true, + hasUnresolvedConflicts: false, + upstreamStatus, + hostedReviewCreation: unavailableEligibility(blockedReason), + branchCommitsAhead + }) + ).toEqual({ eligible: true, kind: blockedKind }) + } + ) +}) diff --git a/src/shared/source-control-create-review-intent.ts b/src/shared/source-control-create-review-intent.ts index 5d1f517280b..b7266708b25 100644 --- a/src/shared/source-control-create-review-intent.ts +++ b/src/shared/source-control-create-review-intent.ts @@ -41,16 +41,20 @@ export function resolveCreateReviewIntentEligibility({ !hasCurrentBranch || !hostedReviewCreation || hostedReviewCreation.canCreate || - // Fail closed when the existing-review lookup could not prove there is no - // review: a local blocker (e.g. needs_push) returned after a failed lookup - // must not offer a Create PR intent that would push under a false promise — - // the main preflight would refuse the create anyway (invariant 8). - hostedReviewCreation.reviewLookupOutcome === 'unavailable' || !supportsHostedReviewCreation(hostedReviewCreation.provider) ) { return { eligible: false, kind: null } } + if ( + hostedReviewCreation.reviewLookupOutcome === 'unavailable' && + !hostedReviewCreation.defaultBaseRef?.trim() + ) { + return { eligible: false, kind: null } + } + + // Why: safe branch preparation can continue without lookup authority; the + // main create preflight still fails closed before creating a duplicate. if (hostedReviewCreation.blockedReason === 'dirty') { if (stagedCount > 0 && !hasMessage) { return { eligible: true, kind: 'message_required' } diff --git a/tests/e2e/source-control-create-pr-intent-switch.spec.ts b/tests/e2e/source-control-create-pr-intent-switch.spec.ts new file mode 100644 index 00000000000..816cf396d3c --- /dev/null +++ b/tests/e2e/source-control-create-pr-intent-switch.spec.ts @@ -0,0 +1,344 @@ +import type { TestInfo } from '@stablyai/playwright-test' +import { execFileSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + createStagedCommitMessageChange, + openSourceControl, + seedCreatePrComposer +} from './helpers/source-control-ai-generation' + +async function writeEvidence( + testInfo: TestInfo, + screenshotDir: string, + filename: string, + evidence: unknown +): Promise { + const evidencePath = path.join(screenshotDir, filename) + writeFileSync(evidencePath, `${JSON.stringify(evidence, null, 2)}\n`) + await testInfo.attach(filename, { + path: evidencePath, + contentType: 'application/json' + }) +} + +function removeOriginRemoteIfPresent(cwd: string): void { + // Why: check presence instead of swallowing errors, so real Git failures still surface. + const remotes = execFileSync('git', ['remote'], { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'] + }) + .split('\n') + .map((line) => line.trim()) + if (!remotes.includes('origin')) { + return + } + execFileSync('git', ['remote', 'remove', 'origin'], { cwd, stdio: 'pipe' }) +} + +test.describe('Source Control Create PR intent worktree switching', () => { + test.describe.configure({ mode: 'serial' }) + + test('keeps Create PR intent running after switching worktrees', async ({ + orcaPage + }, testInfo) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const { primaryWorktreeId, prWorktreeId, prWorktreePath, primaryBranch } = + await seedCreatePrComposer(orcaPage) + + const screenshotDir = path.join( + process.cwd(), + 'validation-screenshots', + `create-pr-intent-switch-${Date.now()}` + ) + mkdirSync(screenshotDir, { recursive: true }) + await testInfo.attach('validation-screenshot-dir', { + body: screenshotDir, + contentType: 'text/plain' + }) + + await orcaPage.evaluate( + ({ prWorktreeId, primaryBranch }) => { + const store = + window.__store ?? + (() => { + throw new Error('window.__store is not available') + })() + const state = store.getState() + const worktree = Object.values(state.worktreesByRepo) + .flat() + .find((entry) => entry.id === prWorktreeId) + if (!worktree) { + throw new Error('Create PR intent worktree not found') + } + const repo = state.repos.find((entry) => entry.id === worktree.repoId) + if (!repo) { + throw new Error('Create PR intent repo not found') + } + const branch = worktree.branch.replace(/^refs\/heads\//, '') + + type CreatePrIntentHostedReviewCall = { + repoPath: string + input: { + base?: string + head?: string + worktreePath?: string + } + } + const testWindow = window as unknown as { + __createPRIntentPayloads: CreatePrIntentHostedReviewCall[] + __createPRIntentPushStarted: boolean + __createPRIntentPushFinished: boolean + } + testWindow.__createPRIntentPayloads = [] + testWindow.__createPRIntentPushStarted = false + testWindow.__createPRIntentPushFinished = false + store.setState((current) => ({ + getHostedReviewCreationEligibility: async () => { + // Why: eligibility stays blocked until the delayed push completes, + // so this test exercises navigation during an in-flight intent run. + if (!testWindow.__createPRIntentPushFinished) { + return { + provider: 'github' as const, + review: null, + canCreate: false, + blockedReason: 'needs_push' as const, + nextAction: 'push' as const, + defaultBaseRef: primaryBranch, + head: branch + } + } + return { + provider: 'github' as const, + review: null, + canCreate: true, + blockedReason: null, + nextAction: null, + defaultBaseRef: primaryBranch, + title: 'Create PR intent after switching worktrees', + body: 'The intent flow should continue after navigation.', + head: branch + } + }, + fetchHostedReviewForBranch: async () => null, + fetchPRForBranch: async () => null, + pushBranch: async (worktreeId) => { + if (worktreeId !== prWorktreeId) { + throw new Error(`Create PR intent pushed unexpected worktree ${worktreeId}`) + } + testWindow.__createPRIntentPushStarted = true + await new Promise((resolve) => setTimeout(resolve, 1500)) + testWindow.__createPRIntentPushFinished = true + }, + createHostedReview: async (repoPath, input) => { + testWindow.__createPRIntentPayloads.push({ repoPath, input }) + return { + ok: true as const, + number: 74, + url: 'https://github.com/acme/orca/pull/74' + } + }, + gitStatusByWorktree: { + ...current.gitStatusByWorktree, + [worktree.id]: [] + }, + remoteStatusesByWorktree: { + ...current.remoteStatusesByWorktree, + [worktree.id]: { + hasUpstream: true, + upstreamName: `origin/${branch}`, + ahead: 1, + behind: 0 + } + } + })) + }, + { prWorktreeId, primaryBranch } + ) + + await openSourceControl(orcaPage, prWorktreeId) + const createPr = orcaPage.getByRole('button', { name: 'Create PR' }).first() + await expect(createPr).toBeVisible({ timeout: 10_000 }) + await expect(createPr).toBeEnabled() + await createPr.click() + + await expect + .poll( + () => + orcaPage.evaluate( + () => + (window as unknown as { __createPRIntentPushStarted: boolean }) + .__createPRIntentPushStarted + ), + { timeout: 10_000 } + ) + .toBe(true) + await openSourceControl(orcaPage, primaryWorktreeId) + + await expect + .poll( + () => + orcaPage.evaluate( + () => + (window as unknown as { __createPRIntentPayloads: unknown[] }) + .__createPRIntentPayloads.length + ), + { timeout: 10_000 } + ) + .toBe(1) + + const completedWhileSwitchedEvidence = await orcaPage.evaluate(() => { + const state = window.__store?.getState() + return { + activeWorktreeId: state?.activeWorktreeId, + rightSidebarTab: state?.rightSidebarTab + } + }) + expect(completedWhileSwitchedEvidence.activeWorktreeId).toBe(primaryWorktreeId) + expect(completedWhileSwitchedEvidence.rightSidebarTab).toBe('source-control') + + await openSourceControl(orcaPage, prWorktreeId) + const payloads = await orcaPage.evaluate( + () => + ( + window as unknown as { + __createPRIntentPayloads: { + repoPath: string + input: { base?: string; head?: string; worktreePath?: string } + }[] + } + ).__createPRIntentPayloads + ) + expect(payloads).toHaveLength(1) + expect(payloads[0]).toMatchObject({ + input: { + base: primaryBranch, + head: 'e2e-secondary', + worktreePath: prWorktreePath + } + }) + await orcaPage.screenshot({ + path: path.join(screenshotDir, '01-create-pr-intent-completed-after-switch.png') + }) + await writeEvidence(testInfo, screenshotDir, 'create-pr-intent-switch-evidence.json', { + expectedOriginalWorktreeId: prWorktreeId, + expectedOtherWorktreeId: primaryWorktreeId, + completedWhileSwitched: completedWhileSwitchedEvidence, + payloads + }) + }) + + test('carries unavailable dirty intent through push to the final create preflight', async ({ + orcaPage, + registerPostElectronShutdownCleanup + }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const { prWorktreeId, prWorktreePath } = await seedCreatePrComposer(orcaPage) + 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 () => { + removeOriginRemoteIfPresent(prWorktreePath) + rmSync(remoteRoot, { recursive: true, force: true }) + }) + createStagedCommitMessageChange(prWorktreePath) + + const finalCreateError = 'Unavailable lookup intent reached final create preflight' + await orcaPage.evaluate( + ({ prWorktreeId, finalCreateError }) => { + const store = + window.__store ?? + (() => { + throw new Error('window.__store is not available') + })() + const state = store.getState() + const worktree = Object.values(state.worktreesByRepo) + .flat() + .find((entry) => entry.id === prWorktreeId) + if (!worktree) { + throw new Error('Create PR intent worktree not found') + } + const branch = worktree.branch.replace(/^refs\/heads\//, '') + const pushBranchAction = state.pushBranch + const testWindow = window as unknown as { + __unavailableIntentPushFinished: boolean + } + testWindow.__unavailableIntentPushFinished = false + + store.setState((current) => ({ + repos: current.repos.map((repo) => + repo.id === worktree.repoId + ? { + ...repo, + gitRemoteIdentity: { + canonicalKey: 'github.com/acme/orca', + remoteName: 'origin', + remoteUrl: 'https://github.com/acme/orca.git' + } + } + : repo + ), + remoteStatusesByWorktree: { + ...current.remoteStatusesByWorktree, + [prWorktreeId]: { + hasUpstream: true, + upstreamName: `origin/${branch}`, + ahead: 1, + behind: 0 + } + }, + getHostedReviewCreationEligibility: async () => { + throw new Error('Hosted review eligibility timed out') + }, + pushBranch: async (...args: Parameters) => { + const [worktreeId] = args + if (worktreeId !== prWorktreeId) { + throw new Error(`Create PR intent pushed unexpected worktree ${worktreeId}`) + } + await pushBranchAction(...args) + testWindow.__unavailableIntentPushFinished = true + }, + createHostedReview: async () => ({ + ok: false as const, + code: 'validation' as const, + error: finalCreateError + }) + })) + }, + { prWorktreeId, finalCreateError } + ) + + await openSourceControl(orcaPage, prWorktreeId) + await expect(orcaPage.getByText('e2e-commit-message-generation.txt')).toBeVisible({ + timeout: 10_000 + }) + await orcaPage + .getByRole('textbox', { name: 'Commit message' }) + .fill('Exercise unavailable Create PR intent') + const createPr = orcaPage.getByRole('button', { name: 'Create PR' }).first() + await expect(createPr).toBeEnabled() + await createPr.click() + + await expect + .poll( + () => + orcaPage.evaluate( + () => + (window as unknown as { __unavailableIntentPushFinished: boolean }) + .__unavailableIntentPushFinished + ), + { timeout: 10_000 } + ) + .toBe(true) + await expect(orcaPage.getByText(finalCreateError)).toBeVisible({ timeout: 10_000 }) + }) +}) diff --git a/tests/e2e/source-control-pr-generation-switch.spec.ts b/tests/e2e/source-control-pr-generation-switch.spec.ts index 2cf0a3e9bb7..283b47416eb 100644 --- a/tests/e2e/source-control-pr-generation-switch.spec.ts +++ b/tests/e2e/source-control-pr-generation-switch.spec.ts @@ -314,196 +314,6 @@ test.describe('Source Control AI PR generation worktree switching', () => { }) }) - test('keeps Create PR intent running after switching worktrees', async ({ - orcaPage - }, testInfo) => { - await waitForSessionReady(orcaPage) - await waitForActiveWorktree(orcaPage) - const { primaryWorktreeId, prWorktreeId, prWorktreePath, primaryBranch } = - await seedCreatePrComposer(orcaPage) - - const screenshotDir = path.join( - process.cwd(), - 'validation-screenshots', - `create-pr-intent-switch-${Date.now()}` - ) - mkdirSync(screenshotDir, { recursive: true }) - await testInfo.attach('validation-screenshot-dir', { - body: screenshotDir, - contentType: 'text/plain' - }) - - await orcaPage.evaluate( - ({ prWorktreeId, primaryBranch }) => { - const store = - window.__store ?? - (() => { - throw new Error('window.__store is not available') - })() - const state = store.getState() - const worktree = Object.values(state.worktreesByRepo) - .flat() - .find((entry) => entry.id === prWorktreeId) - if (!worktree) { - throw new Error('Create PR intent worktree not found') - } - const repo = state.repos.find((entry) => entry.id === worktree.repoId) - if (!repo) { - throw new Error('Create PR intent repo not found') - } - const branch = worktree.branch.replace(/^refs\/heads\//, '') - - type CreatePrIntentHostedReviewCall = { - repoPath: string - input: { - base?: string - head?: string - worktreePath?: string - } - } - const testWindow = window as unknown as { - __createPRIntentPayloads: CreatePrIntentHostedReviewCall[] - __createPRIntentPushStarted: boolean - __createPRIntentPushFinished: boolean - } - testWindow.__createPRIntentPayloads = [] - testWindow.__createPRIntentPushStarted = false - testWindow.__createPRIntentPushFinished = false - store.setState((current) => ({ - getHostedReviewCreationEligibility: async () => { - // Why: eligibility stays blocked until the delayed push completes, - // so this test exercises navigation during an in-flight intent run. - if (!testWindow.__createPRIntentPushFinished) { - return { - provider: 'github' as const, - review: null, - canCreate: false, - blockedReason: 'needs_push' as const, - nextAction: 'push' as const, - defaultBaseRef: primaryBranch, - head: branch - } - } - return { - provider: 'github' as const, - review: null, - canCreate: true, - blockedReason: null, - nextAction: null, - defaultBaseRef: primaryBranch, - title: 'Create PR intent after switching worktrees', - body: 'The intent flow should continue after navigation.', - head: branch - } - }, - fetchHostedReviewForBranch: async () => null, - fetchPRForBranch: async () => null, - pushBranch: async (worktreeId) => { - if (worktreeId !== prWorktreeId) { - throw new Error(`Create PR intent pushed unexpected worktree ${worktreeId}`) - } - testWindow.__createPRIntentPushStarted = true - await new Promise((resolve) => setTimeout(resolve, 1500)) - testWindow.__createPRIntentPushFinished = true - }, - createHostedReview: async (repoPath, input) => { - testWindow.__createPRIntentPayloads.push({ repoPath, input }) - return { - ok: true as const, - number: 74, - url: 'https://github.com/acme/orca/pull/74' - } - }, - gitStatusByWorktree: { - ...current.gitStatusByWorktree, - [worktree.id]: [] - }, - remoteStatusesByWorktree: { - ...current.remoteStatusesByWorktree, - [worktree.id]: { - hasUpstream: true, - upstreamName: `origin/${branch}`, - ahead: 1, - behind: 0 - } - } - })) - }, - { prWorktreeId, primaryBranch } - ) - - await openSourceControl(orcaPage, prWorktreeId) - const createPr = orcaPage.getByRole('button', { name: 'Create PR' }).first() - await expect(createPr).toBeVisible({ timeout: 10_000 }) - await expect(createPr).toBeEnabled() - await createPr.click() - - await expect - .poll( - () => - orcaPage.evaluate( - () => - (window as unknown as { __createPRIntentPushStarted: boolean }) - .__createPRIntentPushStarted - ), - { timeout: 10_000 } - ) - .toBe(true) - await openSourceControl(orcaPage, primaryWorktreeId) - - await expect - .poll( - () => - orcaPage.evaluate( - () => - (window as unknown as { __createPRIntentPayloads: unknown[] }) - .__createPRIntentPayloads.length - ), - { timeout: 10_000 } - ) - .toBe(1) - - const completedWhileSwitchedEvidence = await orcaPage.evaluate(() => { - const state = window.__store?.getState() - return { - activeWorktreeId: state?.activeWorktreeId, - rightSidebarTab: state?.rightSidebarTab - } - }) - expect(completedWhileSwitchedEvidence.activeWorktreeId).toBe(primaryWorktreeId) - expect(completedWhileSwitchedEvidence.rightSidebarTab).toBe('source-control') - - await openSourceControl(orcaPage, prWorktreeId) - const payloads = await orcaPage.evaluate( - () => - ( - window as unknown as { - __createPRIntentPayloads: { - repoPath: string - input: { base?: string; head?: string; worktreePath?: string } - }[] - } - ).__createPRIntentPayloads - ) - expect(payloads).toHaveLength(1) - expect(payloads[0]).toMatchObject({ - input: { - base: primaryBranch, - head: 'e2e-secondary', - worktreePath: prWorktreePath - } - }) - await orcaPage.screenshot({ - path: path.join(screenshotDir, '01-create-pr-intent-completed-after-switch.png') - }) - await writeEvidence(testInfo, screenshotDir, 'create-pr-intent-switch-evidence.json', { - expectedOriginalWorktreeId: prWorktreeId, - expectedOtherWorktreeId: primaryWorktreeId, - completedWhileSwitched: completedWhileSwitchedEvidence, - payloads - }) - }) - test('hydrates pending PR generation after Source Control remounts', async ({ orcaPage }, testInfo) => {