diff --git a/mobile/app/h/[hostId]/pr/[worktreeId].tsx b/mobile/app/h/[hostId]/pr/[worktreeId].tsx index 6db023bee..2e0530b0f 100644 --- a/mobile/app/h/[hostId]/pr/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/pr/[worktreeId].tsx @@ -8,7 +8,7 @@ import { MobilePrViewPanel } from '../../../../src/components/pr-sidebar/MobileP export default function MobilePrViewScreen() { const { hostId, worktreeId } = useLocalSearchParams<{ hostId: string; worktreeId: string }>() const { client, state: connState } = useHostClient(hostId) - const { branch, headSha, isGithubRepo, repoLoaded, loaded } = useMobilePrBranchContext({ + const { branch, headSha, status, isGithubRepo, repoLoaded, loaded } = useMobilePrBranchContext({ client, connState, worktreeId @@ -21,6 +21,7 @@ export default function MobilePrViewScreen() { worktreeId={worktreeId} branch={branch} headSha={headSha} + gitStatus={status} isGithubRepo={isGithubRepo} branchContextLoaded={loaded && repoLoaded} embedded={false} diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 8dbfb932c..5175ba90c 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -897,6 +897,7 @@ export default function SessionScreen() { const { branch: prBranch, headSha: prHeadSha, + status: prStatus, isGithubRepo: prIsGithubRepo, repoLoaded: prRepoContextLoaded, loaded: prContextLoaded @@ -5126,6 +5127,7 @@ export default function SessionScreen() { connState={connState} branch={prBranch} headSha={prHeadSha} + gitStatus={prStatus} isGithubRepo={prIsGithubRepo} branchContextLoaded={prContextLoaded && prRepoContextLoaded} availableWidth={sessionContentRowWidth} diff --git a/mobile/src/components/MobileDiffReviewScreenView.tsx b/mobile/src/components/MobileDiffReviewScreenView.tsx index b63cd3d6b..c472d1482 100644 --- a/mobile/src/components/MobileDiffReviewScreenView.tsx +++ b/mobile/src/components/MobileDiffReviewScreenView.tsx @@ -32,6 +32,7 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { // Inline-dock the sidebar only when wide and the repo is GitHub; otherwise it // lives in the RightDrawer overlay toggled by showPRSidebar. const showInlineDock = presentationMode === 'inline' && controller.prSidebarIsGithubRepo + const gitStatus = controller.screenState.kind === 'ready' ? controller.screenState.status : null // The docked sidebar has no trigger to tap, so load its PR data once it becomes // visible (the overlay loads on trigger press instead). @@ -120,6 +121,7 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { connState={controller.connState} worktreeId={controller.worktreeId} gitBranch={controller.prSidebarBranch} + gitStatus={gitStatus} headSha={controller.prSidebarHeadSha} bottomInset={insets.bottom} /> @@ -140,6 +142,7 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) { connState={controller.connState} worktreeId={controller.worktreeId} gitBranch={controller.prSidebarBranch} + gitStatus={gitStatus} headSha={controller.prSidebarHeadSha} /> diff --git a/mobile/src/components/MobilePRSidebar.tsx b/mobile/src/components/MobilePRSidebar.tsx index 2dc90611a..71cc33d5a 100644 --- a/mobile/src/components/MobilePRSidebar.tsx +++ b/mobile/src/components/MobilePRSidebar.tsx @@ -17,6 +17,7 @@ import { useMobilePrAiTriage, type MobilePrAiTriage } from '../session/use-mobil import { buildFixChecksPrompt, buildResolveConflictsPrompt } from '../session/pr-ai-triage-prompt' import { prSidebarRenderBranch } from './mobile-pr-sidebar-presentation' import { mobilePrSidebarStyles as styles } from './pr-sidebar/mobile-pr-sidebar-styles' +import type { MobileGitStatusResult } from '../source-control/mobile-git-status' import { PRSidebarHeader } from './pr-sidebar/PRSidebarHeader' import { PRConflictingFilesSection } from './pr-sidebar/PRConflictingFilesSection' import { PRActionsSection } from './pr-sidebar/PRActionsSection' @@ -34,17 +35,13 @@ type Props = { client: RpcClient | null connState: ConnectionState worktreeId: string - // Current git branch — feeds the create-PR prefill in the no-PR empty state. gitBranch: string | null + gitStatus: MobileGitStatusResult | null headSha: string | null - // Applied by the docked column so content clears the home indicator (the screen's - // SafeAreaView is edges={['top']} only). bottomInset?: number } -// The shell switches on the controller's state machine and renders the sections -// (header/actions/reviewers/checks). The mutation hook is created here (hooks must -// run unconditionally) and only fires once a PR is ready. Style only from mobile-theme. +// Mutation hooks run unconditionally here and gate internally until a PR is ready. export function MobilePRSidebar({ state, onRetry, @@ -53,6 +50,7 @@ export function MobilePRSidebar({ connState, worktreeId, gitBranch, + gitStatus, headSha, bottomInset = 0 }: Props) { @@ -74,8 +72,6 @@ export function MobilePRSidebar({ prRepo, refetch }) - // Separate hook for the interactive comment timeline (reply/resolve/add). Like - // useMobilePrActions it must run unconditionally; it gates internally on a client. const commentActions = useMobilePrCommentActions({ client, connState, @@ -84,8 +80,6 @@ export function MobilePRSidebar({ prRepo, refetch }) - // Inline title-edit action. Like the others it must run unconditionally and gates - // internally on a client; refetches authoritative PR data after a successful edit. const titleAction = useMobilePrTitleAction({ client, connState, @@ -94,8 +88,6 @@ export function MobilePRSidebar({ prRepo, refetch }) - // AI triage (Fix checks / Resolve conflicts). Like the other hooks it must run - // unconditionally; it gates internally on a connected client. const triage = useMobilePrAiTriage({ client, connState, worktreeId }) return ( @@ -113,6 +105,7 @@ export function MobilePRSidebar({ client={client} worktreeId={worktreeId} gitBranch={gitBranch} + gitStatus={gitStatus} actions={actions} commentActions={commentActions} titleAction={titleAction} @@ -130,6 +123,7 @@ function PrSidebarContent({ client, worktreeId, gitBranch, + gitStatus, actions, commentActions, titleAction, @@ -142,6 +136,7 @@ function PrSidebarContent({ client: RpcClient | null worktreeId: string gitBranch: string | null + gitStatus: MobileGitStatusResult | null actions: MobilePrActions commentActions: MobilePrCommentActions titleAction: MobilePrTitleAction @@ -194,6 +189,7 @@ function PrSidebarContent({ client={client} worktreeId={worktreeId} gitBranch={gitBranch} + gitStatus={gitStatus} onCreated={refetch} /> ) diff --git a/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx b/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx index 765a30dba..618e399d1 100644 --- a/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx @@ -8,12 +8,16 @@ import { TriangleAlert, X } from 'lucide-react-native' -import type { HostedReviewProvider } from '../../../../src/shared/hosted-review' import { colors } from '../../theme/mobile-theme' import type { RpcClient } from '../../transport/rpc-client' import type { RpcSuccess } from '../../transport/types' import { triggerError, triggerSuccess } from '../../platform/haptics' -import { createMobilePr } from '../../source-control/mobile-pr-create' +import { + createMobilePr, + getMobilePrCreateSuccessWarning, + shouldPushBeforeMobilePrCreate, + type MobilePrPrefill +} from '../../source-control/mobile-pr-create' import { hostedReviewCopy } from '../../source-control/hosted-review-copy' import { getPrComposeDisabledReason, @@ -22,12 +26,7 @@ import { import { MobilePrBasePicker } from '../MobilePrBasePicker' import { mobilePrComposeFormStyles as styles } from './mobile-pr-compose-form-styles' -export type PrComposePrefill = { - base: string - title: string - body: string - provider: HostedReviewProvider -} +export type PrComposePrefill = MobilePrPrefill type Props = { client: RpcClient | null @@ -61,6 +60,7 @@ export function MobilePrComposeForm({ const [generating, setGenerating] = useState(false) const [submitting, setSubmitting] = useState(false) const [error, setError] = useState(null) + const pushBeforeCreate = shouldPushBeforeMobilePrCreate(prefill) const generate = useCallback(async () => { if (!client || generating) { @@ -128,13 +128,12 @@ export function MobilePrComposeForm({ ...(head ? { head } : {}), title, body, - draft + draft, + pushBeforeCreate }) if (outcome.ok) { triggerSuccess() - const warning = outcome.linkError - ? `${copy.titleLabel} created, but Orca could not refresh it yet.` - : undefined + const warning = getMobilePrCreateSuccessWarning(outcome, prefill.provider) if (warning) { setError(warning) } @@ -156,6 +155,7 @@ export function MobilePrComposeForm({ head, onCreated, prefill.provider, + pushBeforeCreate, submitting, title, worktreeId @@ -279,7 +279,13 @@ export function MobilePrComposeForm({ )} - {draft ? `Create draft ${copy.shortLabel}` : `Create ${copy.shortLabel}`} + {pushBeforeCreate + ? draft + ? `Push & create draft ${copy.shortLabel}` + : `Push & create ${copy.shortLabel}` + : draft + ? `Create draft ${copy.shortLabel}` + : `Create ${copy.shortLabel}`} diff --git a/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx b/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx index b733c5f4f..e1b19263f 100644 --- a/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrViewPanel.tsx @@ -6,6 +6,7 @@ import { ChevronLeft, X } from 'lucide-react-native' import { colors, radii, spacing, typography } from '../../theme/mobile-theme' import type { ConnectionState } from '../../transport/types' import type { RpcClient } from '../../transport/rpc-client' +import type { MobileGitStatusResult } from '../../source-control/mobile-git-status' import { useMobilePrSidebarController } from '../../session/use-mobile-pr-sidebar-controller' import { MobilePRSidebar } from '../MobilePRSidebar' @@ -15,6 +16,7 @@ type Props = { worktreeId: string branch: string | null headSha: string | null + gitStatus: MobileGitStatusResult | null isGithubRepo?: boolean branchContextLoaded?: boolean // Embedded (docked) drops the full-screen SafeAreaView chrome and shows a close @@ -29,6 +31,7 @@ export function MobilePrViewPanel({ worktreeId, branch, headSha, + gitStatus, isGithubRepo = true, branchContextLoaded = true, embedded = false, @@ -79,6 +82,7 @@ export function MobilePrViewPanel({ connState={connState} worktreeId={worktreeId} gitBranch={branch} + gitStatus={gitStatus} headSha={headSha} bottomInset={insets.bottom} /> diff --git a/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx b/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx index 3f3b74eeb..605eafafa 100644 --- a/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx +++ b/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx @@ -3,10 +3,11 @@ import { ActivityIndicator, Pressable, Text, View } from 'react-native' import { GitPullRequestArrow, Link2, RefreshCw } from 'lucide-react-native' import { colors } from '../../theme/mobile-theme' import type { RpcClient } from '../../transport/rpc-client' -import { resolveMobilePrPrefill, type MobilePrPrefill } from '../../source-control/mobile-pr-create' +import type { MobileGitStatusResult } from '../../source-control/mobile-git-status' +import { mobileHostedReviewCreateIntentProgressMessage } from '../../source-control/mobile-hosted-review-create-intent' +import { runMobileHostedReviewCreateIntent } from '../../source-control/mobile-hosted-review-create-intent-runner' import { fetchWorktreeLinkedPR } from '../../source-control/mobile-pr-link' import { openMobilePrUrl } from '../MobilePrComposeSheet' -import { MobilePrComposeForm } from './MobilePrComposeForm' import { MobileLinkPrForm } from './MobileLinkPrForm' import { prCreateEmptyStateStyles as styles } from './pr-create-empty-state-styles' @@ -14,17 +15,23 @@ type Props = { client: RpcClient | null worktreeId: string gitBranch: string | null + gitStatus: MobileGitStatusResult | null // Refetches the sidebar after create or an explicit empty-state refresh. onCreated: () => void } -type Mode = 'choose' | 'create' | 'link' +type Mode = 'choose' | 'link' // Empty state for a branch with no PR: create a new PR, or link an existing one // (the no-PR surface is the natural home for linking — desktop's link entry lives // on its PR card, but on mobile this is where a user lands with nothing linked). -export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCreated }: Props) { - const [prefill, setPrefill] = useState(null) +export function PrSidebarCreateEmptyState({ + client, + worktreeId, + gitBranch, + gitStatus, + onCreated +}: Props) { const [mode, setMode] = useState('choose') const [loading, setLoading] = useState(false) const [createWarning, setCreateWarning] = useState(null) @@ -61,22 +68,28 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre setCreateWarning(null) setLoading(true) try { - // Git-status fields are best-effort here (the sidebar has no working-tree - // state); base/title/body come from host eligibility regardless, and create - // does the authoritative branch-state validation. - const resolved = await resolveMobilePrPrefill(client, worktreeId, { - branch: gitBranch ?? undefined, - title: gitBranch ?? '', - hasUncommittedChanges: false, - hasUpstream: true, - ahead: 1, - behind: 0 + if (!gitBranch) { + setCreateWarning('Check out a branch before creating a pull request.') + return + } + // Why: mobile skips the local compose step here and runs the hosted create + // flow directly so PR creation matches the automated hosted-review path. + const outcome = await runMobileHostedReviewCreateIntent(client, worktreeId, { + branch: gitBranch, + title: gitBranch, + status: gitStatus, + onProgress: (progress) => + setCreateWarning(mobileHostedReviewCreateIntentProgressMessage(progress)) }) - setPrefill(resolved) - setMode('create') - } catch { - // Best-effort: if prefill resolution rejects, leave the empty state so the - // user can retry rather than surfacing an unhandled rejection. + if (!outcome.ok) { + setCreateWarning(outcome.error) + return + } + setCreateWarning(outcome.warning ?? null) + openMobilePrUrl(outcome.url) + onCreated() + } catch (err) { + setCreateWarning(err instanceof Error ? err.message : 'Failed to create pull request.') } finally { setLoading(false) } @@ -84,26 +97,6 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre const canCreate = !!client && !!gitBranch - if (mode === 'create' && prefill) { - return ( - - setMode('choose')} - onCreated={(url, warning) => { - setMode('choose') - setCreateWarning(warning ?? null) - openMobilePrUrl(url) - onCreated() - }} - /> - - ) - } - if (mode === 'link') { return ( diff --git a/mobile/src/session/SessionDockColumn.tsx b/mobile/src/session/SessionDockColumn.tsx index 9b02b9758..877973e5d 100644 --- a/mobile/src/session/SessionDockColumn.tsx +++ b/mobile/src/session/SessionDockColumn.tsx @@ -8,6 +8,7 @@ import { MobilePrViewPanel } from '../components/pr-sidebar/MobilePrViewPanel' import { mobilePrSidebarStyles } from '../components/pr-sidebar/mobile-pr-sidebar-styles' import { useMobileDockResize } from './use-mobile-dock-resize' import type { ActivePanel } from './session-panel-host' +import type { MobileGitStatusResult } from '../source-control/mobile-git-status' type Props = { activePanel: Exclude @@ -18,6 +19,7 @@ type Props = { connState: ConnectionState branch: string | null headSha: string | null + gitStatus: MobileGitStatusResult | null isGithubRepo: boolean branchContextLoaded: boolean availableWidth: number @@ -40,6 +42,7 @@ export function SessionDockColumn({ connState, branch, headSha, + gitStatus, isGithubRepo, branchContextLoaded, availableWidth, @@ -60,6 +63,7 @@ export function SessionDockColumn({ connState={connState} branch={branch} headSha={headSha} + gitStatus={gitStatus} isGithubRepo={isGithubRepo} branchContextLoaded={branchContextLoaded} onRequestClose={onRequestClose} @@ -79,6 +83,7 @@ const DockPanelContent = memo(function DockPanelContent({ connState, branch, headSha, + gitStatus, isGithubRepo, branchContextLoaded, onRequestClose @@ -113,6 +118,7 @@ const DockPanelContent = memo(function DockPanelContent({ worktreeId={worktreeId} branch={branch} headSha={headSha} + gitStatus={gitStatus} isGithubRepo={isGithubRepo} branchContextLoaded={branchContextLoaded} embedded diff --git a/mobile/src/session/mobile-pr-sidebar-state.ts b/mobile/src/session/mobile-pr-sidebar-state.ts index 16b7533f7..5f5b679b2 100644 --- a/mobile/src/session/mobile-pr-sidebar-state.ts +++ b/mobile/src/session/mobile-pr-sidebar-state.ts @@ -103,7 +103,9 @@ export async function loadPrSidebarData( const pr = prOutcome.result const checksOutcome = await deps.fetchPRChecks(args.worktreeId, { prNumber: pr.number, - headSha: args.headSha ?? pr.headSha ?? null, + // Why: mobile create can commit before opening the review; the fetched PR + // head is fresher than the route's cached git.status head. + headSha: pr.headSha ?? args.headSha ?? null, // Prefer the fetched PR's own repo identity so fork PRs key their cached // checks correctly; fall back to an explicit override then null. prRepo: pr.prRepo ?? args.prRepo ?? null diff --git a/mobile/src/session/use-mobile-pr-branch-context.test.ts b/mobile/src/session/use-mobile-pr-branch-context.test.ts index 4890f887b..7ac21cf30 100644 --- a/mobile/src/session/use-mobile-pr-branch-context.test.ts +++ b/mobile/src/session/use-mobile-pr-branch-context.test.ts @@ -78,7 +78,7 @@ describe('deriveMobilePrBranchContext', () => { it('does not throw on null status and null branchCompare', () => { expect(() => deriveMobilePrBranchContext(null, null)).not.toThrow() const result = deriveMobilePrBranchContext(null, null) - expect(result).toEqual({ branch: null, headSha: null }) + expect(result).toEqual({ branch: null, headSha: null, status: null }) }) }) @@ -109,6 +109,7 @@ describe('loadMobilePrBranchContext', () => { expect(out).toEqual({ branch: 'feat', headSha: 'sha-status', + status: expect.objectContaining({ branch: 'feat', head: 'sha-status' }), isGithubRepo: true, repoLoaded: true, loaded: true diff --git a/mobile/src/session/use-mobile-pr-branch-context.ts b/mobile/src/session/use-mobile-pr-branch-context.ts index b0f4a9baa..ef2cf1a48 100644 --- a/mobile/src/session/use-mobile-pr-branch-context.ts +++ b/mobile/src/session/use-mobile-pr-branch-context.ts @@ -10,6 +10,7 @@ import { readMobileBranchCompareResult, readMobileGitStatusResult } from './mobi export type MobilePrBranchContext = { branch: string | null headSha: string | null + status: MobileGitStatusResult | null isGithubRepo: boolean repoLoaded: boolean loaded: boolean @@ -22,10 +23,11 @@ export type MobilePrBranchContext = { export function deriveMobilePrBranchContext( status: MobileGitStatusResult | null, branchCompare: MobileGitBranchCompareResult | null -): { branch: string | null; headSha: string | null } { +): { branch: string | null; headSha: string | null; status: MobileGitStatusResult | null } { return { branch: status?.branch ?? null, - headSha: status?.head ?? branchCompare?.summary.headOid ?? null + headSha: status?.head ?? branchCompare?.summary.headOid ?? null, + status } } @@ -41,6 +43,7 @@ export function useMobilePrBranchContext(input: { const [context, setContext] = useState({ branch: null, headSha: null, + status: null, isGithubRepo: false, repoLoaded: false, loaded: false @@ -54,6 +57,7 @@ export function useMobilePrBranchContext(input: { setContext({ branch: null, headSha: null, + status: null, isGithubRepo: false, repoLoaded: false, loaded: false @@ -63,6 +67,7 @@ export function useMobilePrBranchContext(input: { setContext({ branch: null, headSha: null, + status: null, isGithubRepo: false, repoLoaded: false, loaded: false @@ -108,6 +113,7 @@ export function useMobilePrBranchContext(input: { ...prev, branch: null, headSha: null, + status: null, loaded: true })) } @@ -142,7 +148,7 @@ export async function loadMobilePrRepoContext( export async function loadMobilePrBranchIdentity( client: RpcClient, worktreeId: string -): Promise> { +): Promise> { const [status, branchCompare] = await Promise.all([ readGitStatus(client, worktreeId), // Why: the standalone PR entry point only needs branchCompare as a head-SHA diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts index 5fc684acf..005c4dc0e 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.test.ts @@ -85,10 +85,10 @@ describe('loadPrSidebarData', () => { expect(d.fetchWorkItemDetails).not.toHaveBeenCalled() // forBranch's PR number is threaded into prForBranch as the linked hint. expect(d.fetchPRForBranch).toHaveBeenCalledWith('w', { branch: 'feat', linkedPRNumber: 7 }) - // headSha forwarded to checks (status SHA wins over pr.headSha). + // The fetched PR head wins over the route's cached status SHA. expect(d.fetchPRChecks).toHaveBeenCalledWith('w', { prNumber: 7, - headSha: 'sha-status', + headSha: 'sha-pr', prRepo: null }) }) diff --git a/mobile/src/source-control/MobileSourceControlModals.tsx b/mobile/src/source-control/MobileSourceControlModals.tsx index bd47a1901..b58d25207 100644 --- a/mobile/src/source-control/MobileSourceControlModals.tsx +++ b/mobile/src/source-control/MobileSourceControlModals.tsx @@ -1,28 +1,23 @@ import { ActionSheetModal, type ActionSheetAction } from '../components/ActionSheetModal' import { ConfirmModal } from '../components/ConfirmModal' import { PickerModal } from '../components/PickerModal' -import { MobilePrComposeSheet, openMobilePrUrl } from '../components/MobilePrComposeSheet' +import { openMobilePrUrl } from '../components/MobilePrComposeSheet' import { MobileBranchDiffPreviewDrawer } from './MobileBranchDiffPreviewDrawer' import type { MobileSourceControlState } from './use-mobile-source-control-state' type Props = { state: MobileSourceControlState - worktreeId: string actionSheetActions: ActionSheetAction[] } -export function MobileSourceControlModals({ state, worktreeId, actionSheetActions }: Props) { +export function MobileSourceControlModals({ state, actionSheetActions }: Props) { const { - client, branchDiffPreview, setBranchDiffPreview, showActionSheet, setShowActionSheet, discardTarget, setDiscardTarget, - showPrSheet, - setShowPrSheet, - prPrefill, showBranchPicker, setShowBranchPicker, localBranches, @@ -30,9 +25,7 @@ export function MobileSourceControlModals({ state, worktreeId, actionSheetAction setCreatedPrUrl, createdPrWarning, setCreatedPrWarning, - status, branchLabel, - loadStatus, checkoutBranch, runGitAction } = state @@ -74,21 +67,6 @@ export function MobileSourceControlModals({ state, worktreeId, actionSheetAction onCancel={() => setDiscardTarget(null)} /> - setShowPrSheet(false)} - onCreated={(url, warning) => { - setShowPrSheet(false) - setCreatedPrUrl(url) - setCreatedPrWarning(warning ?? null) - void loadStatus({ preserveReadyOnFailure: true, force: true }) - }} - /> - )} - + ) } diff --git a/mobile/src/source-control/mobile-hosted-review-create-intent-runner.test.ts b/mobile/src/source-control/mobile-hosted-review-create-intent-runner.test.ts new file mode 100644 index 000000000..bf79182be --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent-runner.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcFailure, RpcResponse, RpcSuccess } from '../transport/types' +import { runMobileHostedReviewCreateIntent } from './mobile-hosted-review-create-intent-runner' + +function ok(result: unknown): RpcSuccess { + return { id: 'r', ok: true, result, _meta: { runtimeId: 'rt' } } +} + +function fail(message: string): RpcFailure { + return { id: 'r', ok: false, error: { code: 'x', message }, _meta: { runtimeId: 'rt' } } +} + +function status(entries: unknown[], upstreamStatus = { hasUpstream: true, ahead: 0, behind: 0 }) { + return { + entries, + conflictOperation: 'unknown', + branch: 'feature/x', + head: 'sha', + upstreamStatus + } +} + +function entry(area: 'unstaged' | 'staged') { + return { path: 'a.ts', status: 'modified', area } +} + +function eligibility(overrides: Record) { + return { + provider: 'github', + review: null, + defaultBaseRef: 'main', + title: 'Ship mobile PR create', + body: 'Generated body', + ...overrides + } +} + +function clientWith(responses: RpcResponse[]): Pick & { + calls: Array<{ method: string; params: unknown }> +} { + const calls: Array<{ method: string; params: unknown }> = [] + return { + calls, + sendRequest: vi.fn(async (method: string, params?: unknown) => { + calls.push({ method, params }) + return responses.shift() ?? fail(`unexpected ${method}`) + }) + } +} + +describe('runMobileHostedReviewCreateIntent', () => { + it('prepares the branch and creates the hosted review in one flow', async () => { + const client = clientWith([ + ok(status([entry('unstaged')])), + ok({ success: true }), + ok(status([entry('staged')])), + ok({ success: true, message: 'Ship mobile PR create' }), + ok({ success: true }), + ok(status([], { hasUpstream: true, ahead: 1, behind: 0 })), + ok(eligibility({ canCreate: false, blockedReason: 'needs_push', nextAction: 'push' })), + ok({ success: true }), + ok(status([], { hasUpstream: true, ahead: 0, behind: 0 })), + ok(eligibility({ canCreate: true, blockedReason: null, nextAction: null })), + ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), + ok({ worktree: { linkedPR: 42 } }) + ]) + const progress: string[] = [] + + const result = await runMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null, + onProgress: (step) => progress.push(step) + }) + + expect(result).toEqual( + expect.objectContaining({ + ok: true, + committed: true, + url: 'https://github.com/o/r/pull/42' + }) + ) + expect(progress).toEqual([ + 'staging', + 'generating_commit_message', + 'committing', + 'pushing', + 'creating_review' + ]) + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.bulkStage', + 'git.status', + 'git.generateCommitMessage', + 'git.commit', + 'git.status', + 'hostedReview.getCreationEligibility', + 'git.push', + 'git.status', + 'hostedReview.getCreationEligibility', + 'hostedReview.create', + 'worktree.set' + ]) + }) + + it('does not create when eligibility remains blocked', async () => { + const client = clientWith([ + ok(status([])), + ok(eligibility({ canCreate: false, blockedReason: 'auth_required', nextAction: 'auth' })) + ]) + + await expect( + runMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null + }) + ).resolves.toEqual({ + ok: false, + error: 'Authenticate before creating a pull request.', + committed: false, + status: expect.objectContaining({ entries: [] }) + }) + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'hostedReview.getCreationEligibility' + ]) + }) + + it('reports committed work when creation fails after the automatic commit', async () => { + const client = clientWith([ + ok(status([entry('staged')])), + ok({ success: true }), + ok(status([])), + ok(eligibility({ canCreate: true, blockedReason: null, nextAction: null })), + ok({ ok: false, code: 'validation', error: 'Create PR failed: bad base' }) + ]) + + await expect( + runMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null, + commitMessage: 'Use my message' + }) + ).resolves.toEqual({ + ok: false, + error: 'Create PR failed: bad base', + committed: true, + status: expect.objectContaining({ entries: [] }) + }) + }) +}) diff --git a/mobile/src/source-control/mobile-hosted-review-create-intent-runner.ts b/mobile/src/source-control/mobile-hosted-review-create-intent-runner.ts new file mode 100644 index 000000000..16e6a117f --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent-runner.ts @@ -0,0 +1,86 @@ +import type { RpcClient } from '../transport/rpc-client' +import type { MobileGitStatusResult } from './mobile-git-status' +import { + createMobilePr, + getMobilePrCreateBlockMessage, + getMobilePrCreateSuccessWarning, + shouldPushBeforeMobilePrCreate, + type MobilePrPrefill +} from './mobile-pr-create' +import { + prepareMobileHostedReviewCreateIntent, + type MobileHostedReviewCreateIntentProgress +} from './mobile-hosted-review-create-intent' + +type RunInput = { + branch: string + title: string + status: MobileGitStatusResult | null + commitMessage?: string + onProgress?: (progress: MobileHostedReviewCreateIntentProgress) => void +} + +export type MobileHostedReviewCreateIntentRunOutcome = + | { + ok: true + url: string + warning?: string + prefill: MobilePrPrefill + status: MobileGitStatusResult | null + committed: boolean + } + | { + ok: false + error: string + committed?: boolean + status?: MobileGitStatusResult | null + } + +export async function runMobileHostedReviewCreateIntent( + client: Pick, + worktreeId: string, + input: RunInput +): Promise { + const prepared = await prepareMobileHostedReviewCreateIntent(client, worktreeId, input) + if (!prepared.ok) { + return prepared + } + + const blockedMessage = getMobilePrCreateBlockMessage(prepared.prefill) + if (blockedMessage) { + return { + ok: false, + error: blockedMessage, + committed: prepared.committed, + status: prepared.status + } + } + + input.onProgress?.('creating_review') + const created = await createMobilePr(client, worktreeId, { + provider: prepared.prefill.provider, + base: prepared.prefill.base, + head: input.branch, + title: prepared.prefill.title, + body: prepared.prefill.body, + draft: false, + pushBeforeCreate: shouldPushBeforeMobilePrCreate(prepared.prefill) + }) + if (!created.ok) { + return { + ok: false, + error: created.error, + committed: prepared.committed, + status: prepared.status + } + } + + return { + ok: true, + url: created.url, + warning: getMobilePrCreateSuccessWarning(created, prepared.prefill.provider), + prefill: prepared.prefill, + status: prepared.status, + committed: prepared.committed + } +} diff --git a/mobile/src/source-control/mobile-hosted-review-create-intent.test.ts b/mobile/src/source-control/mobile-hosted-review-create-intent.test.ts new file mode 100644 index 000000000..b15153a1c --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent.test.ts @@ -0,0 +1,210 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcFailure, RpcResponse, RpcSuccess } from '../transport/types' +import { prepareMobileHostedReviewCreateIntent } from './mobile-hosted-review-create-intent' + +function ok(result: unknown): RpcSuccess { + return { id: 'r', ok: true, result, _meta: { runtimeId: 'rt' } } +} + +function fail(message: string): RpcFailure { + return { id: 'r', ok: false, error: { code: 'x', message }, _meta: { runtimeId: 'rt' } } +} + +function status(entries: unknown[], upstreamStatus = { hasUpstream: true, ahead: 0, behind: 0 }) { + return { + entries, + conflictOperation: 'unknown', + branch: 'feature/x', + head: 'sha', + upstreamStatus + } +} + +function entry(area: 'unstaged' | 'untracked' | 'staged') { + return { path: 'a.ts', status: 'modified', area } +} + +function unresolvedEntry(area: 'unstaged' | 'staged') { + return { path: 'conflicted.ts', status: 'modified', area, conflictStatus: 'unresolved' } +} + +function eligibility(overrides: Record) { + return { + provider: 'github', + review: null, + defaultBaseRef: 'main', + title: 'feature/x', + body: '', + ...overrides + } +} + +function clientWith(responses: RpcResponse[]): Pick & { + calls: Array<{ method: string; params: unknown }> +} { + const calls: Array<{ method: string; params: unknown }> = [] + return { + calls, + sendRequest: vi.fn(async (method: string, params?: unknown) => { + calls.push({ method, params }) + return responses.shift() ?? fail(`unexpected ${method}`) + }) + } +} + +describe('prepareMobileHostedReviewCreateIntent', () => { + it('stages, generates a commit, commits, pushes, then returns an eligible prefill', async () => { + const client = clientWith([ + ok(status([entry('unstaged')])), + ok({ success: true }), + ok(status([entry('staged')])), + ok({ success: true, message: 'Ship mobile PR create' }), + ok({ success: true }), + ok(status([], { hasUpstream: true, ahead: 1, behind: 0 })), + ok(eligibility({ canCreate: false, blockedReason: 'needs_push', nextAction: 'push' })), + ok({ success: true }), + ok(status([], { hasUpstream: true, ahead: 0, behind: 0 })), + ok(eligibility({ canCreate: true, blockedReason: null, nextAction: null })) + ]) + const progress: string[] = [] + + const result = await prepareMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null, + onProgress: (step) => progress.push(step) + }) + + expect(result).toEqual({ + ok: true, + committed: true, + status: expect.objectContaining({ + entries: [], + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } + }), + prefill: expect.objectContaining({ canCreate: true, blockedReason: null }) + }) + expect(progress).toEqual(['staging', 'generating_commit_message', 'committing', 'pushing']) + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.bulkStage', + 'git.status', + 'git.generateCommitMessage', + 'git.commit', + 'git.status', + 'hostedReview.getCreationEligibility', + 'git.push', + 'git.status', + 'hostedReview.getCreationEligibility' + ]) + }) + + it('uses a provided commit message instead of generating one', async () => { + const client = clientWith([ + ok(status([entry('staged')])), + ok({ success: true }), + ok(status([], { hasUpstream: true, ahead: 0, behind: 0 })), + ok(eligibility({ canCreate: true, blockedReason: null, nextAction: null })) + ]) + + await expect( + prepareMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null, + commitMessage: 'Use my draft' + }) + ).resolves.toEqual(expect.objectContaining({ ok: true, committed: true })) + + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.commit', + 'git.status', + 'hostedReview.getCreationEligibility' + ]) + expect(client.calls[1].params).toEqual({ + worktree: 'id:repo-1::/tmp/wt', + message: 'Use my draft' + }) + }) + + it('blocks when refreshed status loses its branch during staging', async () => { + const client = clientWith([ + ok(status([entry('unstaged')])), + ok({ success: true }), + ok({ ...status([entry('staged')]), branch: null }) + ]) + + const result = await prepareMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null + }) + + expect(result).toEqual({ + ok: false, + error: 'Branch changed while preparing the pull request.', + committed: false, + status: expect.objectContaining({ + entries: [expect.objectContaining({ area: 'staged' })] + }) + }) + expect(result.status?.branch).toBeUndefined() + + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.bulkStage', + 'git.status' + ]) + }) + + it('returns an actionable error when commit message generation fails', async () => { + const client = clientWith([ + ok(status([entry('staged')])), + ok({ success: false, error: 'no model configured' }) + ]) + + await expect( + prepareMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null + }) + ).resolves.toEqual({ + ok: false, + error: 'Could not generate a commit message. Add one in Source Control, then retry.', + committed: false, + status: expect.objectContaining({ + entries: [expect.objectContaining({ area: 'staged' })] + }) + }) + + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.generateCommitMessage' + ]) + }) + + it('blocks unresolved conflicts before attempting a commit', async () => { + const client = clientWith([ok(status([entry('staged'), unresolvedEntry('unstaged')]))]) + + await expect( + prepareMobileHostedReviewCreateIntent(client, 'repo-1::/tmp/wt', { + branch: 'feature/x', + title: 'feature/x', + status: null, + commitMessage: 'Use my message' + }) + ).resolves.toEqual({ + ok: false, + error: 'Resolve conflicts before creating a pull request.', + committed: false, + status: expect.objectContaining({ + entries: expect.arrayContaining([expect.objectContaining({ path: 'conflicted.ts' })]) + }) + }) + + expect(client.calls.map((call) => call.method)).toEqual(['git.status']) + }) +}) diff --git a/mobile/src/source-control/mobile-hosted-review-create-intent.ts b/mobile/src/source-control/mobile-hosted-review-create-intent.ts new file mode 100644 index 000000000..2e459e1ee --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent.ts @@ -0,0 +1,316 @@ +import type { RpcClient } from '../transport/rpc-client' +import type { RpcSuccess } from '../transport/types' +import { readMobileGitStatusResult } from '../session/mobile-diff-review-rpc' +import { requestMobileCommitMessage } from './mobile-commit-message-ai' +import { getStageablePaths, type MobileGitStatusResult } from './mobile-git-status' +import { getMobilePrEligibilityReadiness } from './mobile-open-pr-prefill' +import { resolveMobilePrPrefill, type MobilePrPrefill } from './mobile-pr-create' + +export type MobileHostedReviewCreateIntentProgress = + | 'staging' + | 'generating_commit_message' + | 'committing' + | 'publishing' + | 'pushing' + | 'force_pushing' + | 'creating_review' + +export type MobileHostedReviewCreateIntentOutcome = + | { + ok: true + prefill: MobilePrPrefill + status: MobileGitStatusResult | null + committed: boolean + } + | { ok: false; error: string; committed?: boolean; status?: MobileGitStatusResult | null } + +type PrepareInput = { + branch: string + title: string + status: MobileGitStatusResult | null + commitMessage?: string + onProgress?: (progress: MobileHostedReviewCreateIntentProgress) => void +} + +export function mobileHostedReviewCreateIntentProgressMessage( + progress: MobileHostedReviewCreateIntentProgress +): string { + switch (progress) { + case 'staging': + return 'Staging changes...' + case 'generating_commit_message': + return 'Generating commit message...' + case 'committing': + return 'Committing changes...' + case 'publishing': + return 'Publishing branch...' + case 'pushing': + return 'Pushing commits...' + case 'force_pushing': + return 'Force pushing with lease...' + case 'creating_review': + return 'Creating review...' + } +} + +async function readStatus( + client: Pick, + worktreeId: string +): Promise { + const response = await client.sendRequest('git.status', { worktree: `id:${worktreeId}` }) + if (!response.ok) { + return null + } + return readMobileGitStatusResult((response as RpcSuccess).result) +} + +function branchStillMatches(inputBranch: string, status: MobileGitStatusResult | null): boolean { + const branch = status?.branch + if (!branch) { + return false + } + return branch === inputBranch || branch === `refs/heads/${inputBranch}` +} + +function hasUnresolvedConflicts(status: MobileGitStatusResult | null): boolean { + return status?.entries.some((entry) => entry.conflictStatus === 'unresolved') === true +} + +async function sendGitMutation( + client: Pick, + method: string, + params: Record, + fallback: string +): Promise<{ ok: true } | { ok: false; error: string }> { + try { + const response = await client.sendRequest(method, params) + if (!response.ok) { + return { ok: false, error: response.error?.message || fallback } + } + return { ok: true } + } catch (err) { + return { ok: false, error: err instanceof Error ? err.message : fallback } + } +} + +async function commitStagedChanges( + client: Pick, + worktreeId: string, + message: string +): Promise<{ ok: true } | { ok: false; error: string }> { + try { + const response = await client.sendRequest('git.commit', { + worktree: `id:${worktreeId}`, + message + }) + if (!response.ok) { + return { ok: false, error: response.error?.message || 'Commit failed' } + } + const result = (response as RpcSuccess).result as { success?: boolean; error?: string } + if (result?.success !== true) { + return { ok: false, error: result?.error || 'Commit failed' } + } + return { ok: true } + } catch (err) { + return { ok: false, error: err instanceof Error ? err.message : 'Commit failed' } + } +} + +async function resolvePrefillFromStatus( + client: Pick, + worktreeId: string, + branch: string, + title: string, + status: MobileGitStatusResult | null +): Promise { + return resolveMobilePrPrefill(client, worktreeId, { + branch, + title, + ...getMobilePrEligibilityReadiness(status) + }) +} + +async function ensureLocalChangesCommitted( + client: Pick, + worktreeId: string, + input: PrepareInput, + currentStatus: MobileGitStatusResult | null +): Promise< + | { ok: true; status: MobileGitStatusResult | null; committed: boolean } + | { ok: false; error: string; committed?: boolean; status?: MobileGitStatusResult | null } +> { + if ((currentStatus?.entries.length ?? 0) === 0) { + return { ok: true, status: currentStatus, committed: false } + } + if (hasUnresolvedConflicts(currentStatus)) { + return { + ok: false, + error: 'Resolve conflicts before creating a pull request.', + committed: false, + status: currentStatus + } + } + + const stagePaths = getStageablePaths(currentStatus?.entries ?? []) + if (stagePaths.length > 0) { + input.onProgress?.('staging') + const staged = await sendGitMutation( + client, + 'git.bulkStage', + { worktree: `id:${worktreeId}`, filePaths: stagePaths }, + 'Failed to stage changes' + ) + if (!staged.ok) { + return staged + } + currentStatus = await readStatus(client, worktreeId) + if (!branchStillMatches(input.branch, currentStatus)) { + return { + ok: false, + error: 'Branch changed while preparing the pull request.', + committed: false, + status: currentStatus + } + } + } + + const hasStagedChanges = currentStatus?.entries.some((entry) => entry.area === 'staged') === true + if (!hasStagedChanges) { + return { + ok: false, + error: 'Resolve or stage changes before creating a pull request.', + committed: false, + status: currentStatus + } + } + + let message = input.commitMessage?.trim() ?? '' + if (!message) { + input.onProgress?.('generating_commit_message') + const generated = await requestMobileCommitMessage(client, worktreeId) + if (!generated.success) { + return { + ok: false, + error: 'Could not generate a commit message. Add one in Source Control, then retry.', + committed: false, + status: currentStatus + } + } + message = generated.message + } + + input.onProgress?.('committing') + const committed = await commitStagedChanges(client, worktreeId, message) + if (!committed.ok) { + return { ...committed, committed: false, status: currentStatus } + } + currentStatus = await readStatus(client, worktreeId) + if (!branchStillMatches(input.branch, currentStatus)) { + return { + ok: false, + error: 'Branch changed while preparing the pull request.', + committed: true, + status: currentStatus + } + } + return { ok: true, status: currentStatus, committed: true } +} + +async function applyRemotePrerequisite( + client: Pick, + worktreeId: string, + prefill: MobilePrPrefill, + input: PrepareInput +): Promise<{ ok: true; ran: boolean } | { ok: false; error: string }> { + switch (prefill.blockedReason) { + case 'no_upstream': { + input.onProgress?.('publishing') + const result = await sendGitMutation( + client, + 'git.push', + { worktree: `id:${worktreeId}`, publish: true }, + 'Failed to publish branch' + ) + return result.ok ? { ok: true, ran: true } : result + } + case 'needs_push': { + input.onProgress?.('pushing') + const result = await sendGitMutation( + client, + 'git.push', + { worktree: `id:${worktreeId}` }, + 'Failed to push commits' + ) + return result.ok ? { ok: true, ran: true } : result + } + case 'needs_sync': + if (input.status?.upstreamStatus?.behindCommitsArePatchEquivalent !== true) { + return { ok: true, ran: false } + } + input.onProgress?.('force_pushing') + const result = await sendGitMutation( + client, + 'git.push', + { worktree: `id:${worktreeId}`, forceWithLease: true }, + 'Failed to force push with lease' + ) + return result.ok ? { ok: true, ran: true } : result + default: + return { ok: true, ran: false } + } +} + +export async function prepareMobileHostedReviewCreateIntent( + client: Pick, + worktreeId: string, + input: PrepareInput +): Promise { + let currentStatus = (await readStatus(client, worktreeId)) ?? input.status + if (!branchStillMatches(input.branch, currentStatus)) { + return { ok: false, error: 'Branch changed while preparing the pull request.' } + } + + const committed = await ensureLocalChangesCommitted(client, worktreeId, input, currentStatus) + if (!committed.ok) { + return committed + } + currentStatus = committed.status + + let prefill = await resolvePrefillFromStatus( + client, + worktreeId, + input.branch, + input.title, + currentStatus + ) + for (let attempts = 0; attempts < 2; attempts++) { + const remote = await applyRemotePrerequisite(client, worktreeId, prefill, { + ...input, + status: currentStatus + }) + if (!remote.ok) { + return { ...remote, committed: committed.committed, status: currentStatus } + } + if (!remote.ran) { + break + } + currentStatus = await readStatus(client, worktreeId) + if (!branchStillMatches(input.branch, currentStatus)) { + return { + ok: false, + error: 'Branch changed while preparing the pull request.', + committed: committed.committed, + status: currentStatus + } + } + prefill = await resolvePrefillFromStatus( + client, + worktreeId, + input.branch, + input.title, + currentStatus + ) + } + + return { ok: true, prefill, status: currentStatus, committed: committed.committed } +} diff --git a/mobile/src/source-control/mobile-hosted-review-service.ts b/mobile/src/source-control/mobile-hosted-review-service.ts new file mode 100644 index 000000000..c219586ed --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-service.ts @@ -0,0 +1,267 @@ +import type { + CreateHostedReviewResult, + HostedReviewCreationBlockedReason, + HostedReviewCreationEligibility, + HostedReviewCreationNextAction, + HostedReviewProvider +} from '../../../src/shared/hosted-review' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcSuccess } from '../transport/types' +import { hostedReviewCopy } from './hosted-review-copy' +import { linkMobileHostedReview } from './mobile-pr-link' + +// The mobile worktree id is `${repoId}::${path}`; hosted-review RPCs expect the +// repo selector separately, matching the desktop/runtime hosted-review service. +export function mobileRepoSelectorFromWorktreeId(worktreeId: string): string { + const separatorIdx = worktreeId.indexOf('::') + const repoId = separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx) + return `id:${repoId}` +} + +export type MobileHostedReviewEligibilityInput = { + branch: string + base?: string | null + hasUncommittedChanges?: boolean + hasUpstream?: boolean + ahead?: number + behind?: number + linkedGitHubPR?: number | null + linkedGitLabMR?: number | null +} + +export async function fetchMobileHostedReviewEligibility( + client: Pick, + worktreeId: string, + input: MobileHostedReviewEligibilityInput +): Promise { + const response = await client.sendRequest('hostedReview.getCreationEligibility', { + repo: mobileRepoSelectorFromWorktreeId(worktreeId), + worktree: `id:${worktreeId}`, + branch: input.branch, + base: input.base ?? null, + ...(input.hasUncommittedChanges !== undefined + ? { hasUncommittedChanges: input.hasUncommittedChanges } + : {}), + ...(input.hasUpstream !== undefined ? { hasUpstream: input.hasUpstream } : {}), + ...(input.ahead !== undefined ? { ahead: input.ahead } : {}), + ...(input.behind !== undefined ? { behind: input.behind } : {}), + linkedGitHubPR: input.linkedGitHubPR ?? null, + linkedGitLabMR: input.linkedGitLabMR ?? null + }) + if (!response.ok) { + return null + } + return (response as RpcSuccess).result as HostedReviewCreationEligibility +} + +export type MobileHostedReviewPrefill = { + provider: HostedReviewProvider + base: string + title: string + body: string + canCreate?: boolean + blockedReason?: HostedReviewCreationBlockedReason + nextAction?: HostedReviewCreationNextAction +} + +// Resolve the mobile compose prefill from the same hosted-review eligibility +// service desktop uses. If eligibility is unavailable, return a blocked prefill +// instead of inventing a provider/base locally. +export async function resolveMobileHostedReviewPrefill( + client: Pick, + worktreeId: string, + args: { + branch: string | undefined + title: string + hasUncommittedChanges?: boolean + hasUpstream?: boolean + ahead?: number + behind?: number + } +): Promise { + const fallback: MobileHostedReviewPrefill = { + provider: 'github', + base: 'main', + title: args.title, + body: '' + } + if (!args.branch) { + return { ...fallback, canCreate: false, blockedReason: 'detached_head', nextAction: null } + } + try { + const eligibility = await fetchMobileHostedReviewEligibility(client, worktreeId, { + branch: args.branch, + hasUncommittedChanges: args.hasUncommittedChanges, + hasUpstream: args.hasUpstream, + ahead: args.ahead, + behind: args.behind + }) + if (!eligibility) { + return { ...fallback, canCreate: false, blockedReason: null, nextAction: null } + } + return { + provider: eligibility.provider, + base: eligibility.defaultBaseRef || 'main', + title: eligibility.title || args.title, + body: eligibility.body || '', + canCreate: eligibility.canCreate, + blockedReason: eligibility.blockedReason, + nextAction: eligibility.nextAction + } + } catch { + return { ...fallback, canCreate: false, blockedReason: null, nextAction: null } + } +} + +export function shouldPushBeforeMobileHostedReviewCreate( + prefill: Pick +): boolean { + return prefill.blockedReason === 'needs_push' +} + +export type MobileHostedReviewCreateInput = { + provider: HostedReviewProvider + base: string + head?: string + title: string + body: string + draft: boolean + useTemplate?: boolean + pushBeforeCreate?: boolean +} + +// Builds the hostedReview.create params, trimming title/body and dropping empty +// optional fields so the host's required-string validation passes cleanly. +export function buildMobileHostedReviewCreateParams( + worktreeId: string, + input: MobileHostedReviewCreateInput +): Record { + return { + repo: mobileRepoSelectorFromWorktreeId(worktreeId), + worktree: `id:${worktreeId}`, + provider: input.provider, + base: input.base.trim(), + ...(input.head && input.head.trim().length > 0 ? { head: input.head.trim() } : {}), + title: input.title.trim(), + ...(input.body.trim().length > 0 ? { body: input.body.trim() } : {}), + draft: input.draft, + ...(input.useTemplate !== undefined ? { useTemplate: input.useTemplate } : {}) + } +} + +export type MobileHostedReviewCreateOutcome = + | { ok: true; url: string; number?: number; existing?: boolean; linkError?: string } + | { ok: false; error: string } + +async function pushMobileBranchBeforeCreate( + client: Pick, + worktreeId: string +): Promise<{ ok: true } | { ok: false; error: string }> { + try { + const response = await client.sendRequest('git.push', { worktree: `id:${worktreeId}` }) + if (!response.ok) { + return { ok: false, error: 'Push failed. Resolve the push error, then try again.' } + } + return { ok: true } + } catch { + return { ok: false, error: 'Push failed. Resolve the push error, then try again.' } + } +} + +function formatMobileHostedReviewCreateError( + result: CreateHostedReviewResult, + pushed: boolean, + shortLabel: string +): string { + if (result.ok) { + return '' + } + if (!pushed) { + return result.error + } + const prefix = new RegExp(`^Create ${shortLabel} failed:\\s*`, 'i') + return `Push succeeded, but ${shortLabel} creation failed: ${result.error.replace(prefix, '')}` +} + +async function finishMobileHostedReviewCreateSuccess( + client: Pick, + worktreeId: string, + input: MobileHostedReviewCreateInput, + result: { number: number; url: string }, + existing?: boolean +): Promise { + const baseRef = input.base.trim() + const linked = await linkMobileHostedReview(client, worktreeId, input.provider, result.number, { + // Why: mobile branch compare cannot infer the new hosted review's target + // base from renderer cache; persist the submitted base for the refresh. + baseRef + }) + return { + ok: true, + url: result.url, + number: result.number, + ...(existing ? { existing: true } : {}), + ...(linked.ok ? {} : { linkError: linked.error }) + } +} + +export async function createMobileHostedReview( + client: Pick, + worktreeId: string, + input: MobileHostedReviewCreateInput +): Promise { + let pushed = false + try { + if (input.pushBeforeCreate) { + const push = await pushMobileBranchBeforeCreate(client, worktreeId) + if (!push.ok) { + return push + } + pushed = true + } + const response = await client.sendRequest( + 'hostedReview.create', + buildMobileHostedReviewCreateParams(worktreeId, input) + ) + if (!response.ok) { + return { ok: false, error: response.error?.message || 'Failed to create pull request' } + } + const result = (response as RpcSuccess).result as CreateHostedReviewResult + if (result.ok) { + return finishMobileHostedReviewCreateSuccess(client, worktreeId, input, result) + } + if (result.existingReview?.url) { + const number = result.existingReview.number + if (!number) { + return { + ok: true, + url: result.existingReview.url, + existing: true + } + } + return finishMobileHostedReviewCreateSuccess( + client, + worktreeId, + input, + { number, url: result.existingReview.url }, + true + ) + } + return { + ok: false, + error: + formatMobileHostedReviewCreateError( + result, + pushed, + hostedReviewCopy(input.provider).shortLabel + ) || 'Failed to create pull request' + } + } catch (err) { + // Why: create review runs from an inline form; transport drops should surface + // as form errors instead of escaping as unhandled promise rejections. + return { + ok: false, + error: err instanceof Error ? err.message : 'Failed to create pull request' + } + } +} diff --git a/mobile/src/source-control/mobile-open-pr-prefill.test.ts b/mobile/src/source-control/mobile-open-pr-prefill.test.ts index a3f67fde7..cf977b0b2 100644 --- a/mobile/src/source-control/mobile-open-pr-prefill.test.ts +++ b/mobile/src/source-control/mobile-open-pr-prefill.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { readFreshGitStatus } from './mobile-open-pr-prefill' +import { getMobilePrEligibilityReadiness, readFreshGitStatus } from './mobile-open-pr-prefill' import type { MobileGitStatusResult } from './mobile-git-status' const fallback = { branch: 'old', entries: [] } as unknown as MobileGitStatusResult @@ -33,3 +33,23 @@ describe('readFreshGitStatus', () => { expect(out).toBe(fallback) }) }) + +describe('getMobilePrEligibilityReadiness', () => { + it('keeps readiness fields absent when git status is unknown', () => { + expect(getMobilePrEligibilityReadiness(null)).toEqual({}) + }) + + it('derives dirty and upstream readiness from git status', () => { + const status = { + entries: [{ path: 'a.ts' }], + upstreamStatus: { hasUpstream: true, ahead: 2, behind: 1 } + } as unknown as MobileGitStatusResult + + expect(getMobilePrEligibilityReadiness(status)).toEqual({ + hasUncommittedChanges: true, + hasUpstream: true, + ahead: 2, + behind: 1 + }) + }) +}) diff --git a/mobile/src/source-control/mobile-open-pr-prefill.ts b/mobile/src/source-control/mobile-open-pr-prefill.ts index 70b203057..705b4dc68 100644 --- a/mobile/src/source-control/mobile-open-pr-prefill.ts +++ b/mobile/src/source-control/mobile-open-pr-prefill.ts @@ -31,13 +31,33 @@ export async function buildOpenPrPrefill( if (!client) { return { provider: 'github', base: 'main', title: branchLabel, body: '' } } - const up = status?.upstreamStatus + const gitReadiness = getMobilePrEligibilityReadiness(status) return resolveMobilePrPrefill(client, worktreeId, { branch: status?.branch, title: branchLabel, - hasUncommittedChanges: (status?.entries?.length ?? 0) > 0, - hasUpstream: up?.hasUpstream === true, - ahead: up?.ahead ?? 0, - behind: up?.behind ?? 0 + ...gitReadiness }) } + +export function getMobilePrEligibilityReadiness(status: MobileGitStatusResult | null): { + hasUncommittedChanges?: boolean + hasUpstream?: boolean + ahead?: number + behind?: number +} { + if (!status) { + return {} + } + const up = status?.upstreamStatus + const upstreamReadiness = up + ? { + hasUpstream: up.hasUpstream, + ahead: up.ahead, + behind: up.behind + } + : {} + return { + hasUncommittedChanges: (status.entries?.length ?? 0) > 0, + ...upstreamReadiness + } +} diff --git a/mobile/src/source-control/mobile-pr-create-flow.test.ts b/mobile/src/source-control/mobile-pr-create-flow.test.ts new file mode 100644 index 000000000..78a99dcce --- /dev/null +++ b/mobile/src/source-control/mobile-pr-create-flow.test.ts @@ -0,0 +1,280 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcFailure, RpcResponse, RpcSuccess } from '../transport/types' +import { createMobilePr } from './mobile-pr-create' + +function ok(result: unknown): RpcSuccess { + return { id: 'r', ok: true, result, _meta: { runtimeId: 'rt' } } +} + +function fail(message: string): RpcFailure { + return { id: 'r', ok: false, error: { code: 'x', message }, _meta: { runtimeId: 'rt' } } +} + +function clientWith(responses: RpcResponse[]): Pick & { + calls: Array<{ method: string; params: unknown }> +} { + const calls: Array<{ method: string; params: unknown }> = [] + return { + calls, + sendRequest: vi.fn(async (method: string, params?: unknown) => { + calls.push({ method, params }) + return responses.shift() ?? fail('unexpected') + }) + } +} + +describe('createMobilePr', () => { + it('returns the url on success', async () => { + const client = clientWith([ + ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), + ok({ worktree: { linkedPR: 42 } }) + ]) + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }) + expect(client.calls[0].method).toBe('hostedReview.create') + expect(client.calls[1]).toEqual({ + method: 'worktree.set', + params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedPR: 42 } + }) + }) + + it('persists the same trimmed base ref used for creation', async () => { + const client = clientWith([ + ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), + ok({ worktree: { linkedPR: 42 } }) + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: ' main ', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }) + expect(client.calls[0].params).toMatchObject({ base: 'main' }) + expect(client.calls[1]).toEqual({ + method: 'worktree.set', + params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedPR: 42 } + }) + }) + + it('links created merge requests through the provider-specific worktree field', async () => { + const client = clientWith([ + ok({ ok: true, number: 7, url: 'https://gitlab.com/o/r/-/merge_requests/7' }), + ok({ worktree: { linkedGitLabMR: 7 } }) + ]) + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'gitlab', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ + ok: true, + number: 7, + url: 'https://gitlab.com/o/r/-/merge_requests/7' + }) + expect(client.calls[1]).toEqual({ + method: 'worktree.set', + params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedGitLabMR: 7 } + }) + }) + + it('keeps the created url when the metadata link refresh fails', async () => { + const client = clientWith([ + ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), + fail('metadata failed') + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ + ok: true, + number: 42, + url: 'https://github.com/o/r/pull/42', + linkError: 'metadata failed' + }) + }) + + it('pushes before create when eligibility requires it', async () => { + const client = clientWith([ + ok({ success: true }), + ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), + ok({ worktree: { linkedPR: 42 } }) + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false, + pushBeforeCreate: true + }) + ).resolves.toEqual({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }) + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.push', + 'hostedReview.create', + 'worktree.set' + ]) + expect(client.calls[0].params).toEqual({ worktree: 'id:repo-1::/tmp/wt' }) + }) + + it('stops before create when the required push fails', async () => { + const client = clientWith([fail('rejected')]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false, + pushBeforeCreate: true + }) + ).resolves.toEqual({ + ok: false, + error: 'Push failed. Resolve the push error, then try again.' + }) + expect(client.calls.map((call) => call.method)).toEqual(['git.push']) + }) + + it('returns existing reviews as linkable success outcomes', async () => { + const client = clientWith([ + ok({ + ok: false, + code: 'already_exists', + error: 'Already open', + existingReview: { number: 42, url: 'https://github.com/o/r/pull/42' } + }), + ok({ worktree: { linkedPR: 42 } }) + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ + ok: true, + existing: true, + number: 42, + url: 'https://github.com/o/r/pull/42' + }) + expect(client.calls[1]).toEqual({ + method: 'worktree.set', + params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedPR: 42 } + }) + }) + + it('returns existing review urls even when no number is available', async () => { + const client = clientWith([ + ok({ + ok: false, + code: 'already_exists', + error: 'Already open', + existingReview: { url: 'https://github.com/o/r/pull/42' } + }) + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ + ok: true, + existing: true, + url: 'https://github.com/o/r/pull/42' + }) + expect(client.calls).toHaveLength(1) + }) + + it('formats create failures after a successful required push like desktop', async () => { + const client = clientWith([ + ok({ success: true }), + ok({ ok: false, code: 'validation', error: 'Create PR failed: bad base' }) + ]) + + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false, + pushBeforeCreate: true + }) + ).resolves.toEqual({ + ok: false, + error: 'Push succeeded, but PR creation failed: bad base' + }) + }) + + it('maps a host failure result to { ok:false }', async () => { + const client = clientWith([ok({ ok: false, code: 'validation', error: 'Push first' })]) + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ ok: false, error: 'Push first' }) + }) + + it('maps an RPC transport failure to { ok:false }', async () => { + const client = clientWith([fail('disconnected')]) + const result = await createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + expect(result).toEqual({ ok: false, error: 'disconnected' }) + }) + + it('normalizes a thrown sendRequest into { ok:false }', async () => { + const client = { + sendRequest: vi.fn(async () => { + throw new Error('socket hung up') + }) + } as unknown as Pick + await expect( + createMobilePr(client, 'repo-1::/tmp/wt', { + provider: 'github', + base: 'main', + title: 'T', + body: '', + draft: false + }) + ).resolves.toEqual({ ok: false, error: 'socket hung up' }) + }) +}) diff --git a/mobile/src/source-control/mobile-pr-create.test.ts b/mobile/src/source-control/mobile-pr-create.test.ts index e82fb4eed..ce877dc3c 100644 --- a/mobile/src/source-control/mobile-pr-create.test.ts +++ b/mobile/src/source-control/mobile-pr-create.test.ts @@ -1,11 +1,14 @@ import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import type { RpcFailure, RpcResponse, RpcSuccess } from '../transport/types' +import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review' +import { shouldOpenChecksPanelCreateComposer } from '../../../src/renderer/src/components/right-sidebar/checks-panel-review-creation' import { buildMobilePrCreateParams, - createMobilePr, + getMobilePrCreateBlockMessage, mobileRepoSelectorFromWorktreeId, - resolveMobilePrPrefill + resolveMobilePrPrefill, + shouldPushBeforeMobilePrCreate } from './mobile-pr-create' function ok(result: unknown): RpcSuccess { @@ -27,6 +30,22 @@ function clientWith(responses: RpcResponse[]): Pick & } } +function eligibility( + overrides: Partial = {} +): HostedReviewCreationEligibility { + return { + provider: 'github', + review: null, + canCreate: true, + blockedReason: null, + nextAction: null, + defaultBaseRef: 'main', + title: 'Add feature', + body: '', + ...overrides + } +} + describe('mobileRepoSelectorFromWorktreeId', () => { it('extracts the repo id before the :: separator', () => { expect(mobileRepoSelectorFromWorktreeId('repo-1::/tmp/wt')).toBe('id:repo-1') @@ -39,10 +58,11 @@ describe('buildMobilePrCreateParams', () => { expect( buildMobilePrCreateParams('repo-1::/tmp/wt', { provider: 'github', - base: 'main', + base: ' main ', title: ' Add feature ', body: ' ', - draft: false + draft: false, + useTemplate: true }) ).toEqual({ repo: 'id:repo-1', @@ -50,7 +70,8 @@ describe('buildMobilePrCreateParams', () => { provider: 'github', base: 'main', title: 'Add feature', - draft: false + draft: false, + useTemplate: true }) }) @@ -67,114 +88,39 @@ describe('buildMobilePrCreateParams', () => { }) }) -describe('createMobilePr', () => { - it('returns the url on success', async () => { - const client = clientWith([ - ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), - ok({ worktree: { linkedPR: 42 } }) - ]) - await expect( - createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'github', - base: 'main', - title: 'T', - body: '', - draft: false - }) - ).resolves.toEqual({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }) - expect(client.calls[0].method).toBe('hostedReview.create') - expect(client.calls[1]).toEqual({ - method: 'worktree.set', - params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedPR: 42 } +describe('mobile create form gating parity', () => { + it.each([ + { reason: null, canCreate: true }, + { reason: 'dirty', canCreate: false }, + { reason: 'detached_head', canCreate: false }, + { reason: 'default_branch', canCreate: false }, + { reason: 'no_upstream', canCreate: false }, + { reason: 'needs_push', canCreate: false }, + { reason: 'needs_sync', canCreate: false }, + { reason: 'auth_required', canCreate: false }, + { reason: 'unsupported_provider', canCreate: false }, + { reason: 'existing_review', canCreate: false }, + { reason: 'fork_head_unsupported', canCreate: false } + ] as const)('matches desktop composer gating for $reason', ({ reason, canCreate }) => { + const desktopEligibility = eligibility({ canCreate, blockedReason: reason }) + const desktopAllowsComposer = shouldOpenChecksPanelCreateComposer({ + activeReview: null, + isFolder: false, + branch: 'feature/x', + hostedReviewCreation: desktopEligibility }) - }) + const mobileAllowsComposer = + getMobilePrCreateBlockMessage({ + provider: desktopEligibility.provider, + base: desktopEligibility.defaultBaseRef ?? 'main', + title: desktopEligibility.title ?? 'feature/x', + body: desktopEligibility.body ?? '', + canCreate: desktopEligibility.canCreate, + blockedReason: desktopEligibility.blockedReason, + nextAction: desktopEligibility.nextAction + }) === null - it('links created merge requests through the provider-specific worktree field', async () => { - const client = clientWith([ - ok({ ok: true, number: 7, url: 'https://gitlab.com/o/r/-/merge_requests/7' }), - ok({ worktree: { linkedGitLabMR: 7 } }) - ]) - await expect( - createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'gitlab', - base: 'main', - title: 'T', - body: '', - draft: false - }) - ).resolves.toEqual({ - ok: true, - number: 7, - url: 'https://gitlab.com/o/r/-/merge_requests/7' - }) - expect(client.calls[1]).toEqual({ - method: 'worktree.set', - params: { worktree: 'id:repo-1::/tmp/wt', baseRef: 'main', linkedGitLabMR: 7 } - }) - }) - - it('keeps the created url when the metadata link refresh fails', async () => { - const client = clientWith([ - ok({ ok: true, number: 42, url: 'https://github.com/o/r/pull/42' }), - fail('metadata failed') - ]) - - await expect( - createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'github', - base: 'main', - title: 'T', - body: '', - draft: false - }) - ).resolves.toEqual({ - ok: true, - number: 42, - url: 'https://github.com/o/r/pull/42', - linkError: 'metadata failed' - }) - }) - - it('maps a host failure result to { ok:false }', async () => { - const client = clientWith([ok({ ok: false, code: 'needs_push', error: 'Push first' })]) - await expect( - createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'github', - base: 'main', - title: 'T', - body: '', - draft: false - }) - ).resolves.toEqual({ ok: false, error: 'Push first' }) - }) - - it('maps an RPC transport failure to { ok:false }', async () => { - const client = clientWith([fail('disconnected')]) - const result = await createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'github', - base: 'main', - title: 'T', - body: '', - draft: false - }) - expect(result).toEqual({ ok: false, error: 'disconnected' }) - }) - - it('normalizes a thrown sendRequest into { ok:false }', async () => { - const client = { - sendRequest: vi.fn(async () => { - throw new Error('socket hung up') - }) - } as unknown as Pick - await expect( - createMobilePr(client, 'repo-1::/tmp/wt', { - provider: 'github', - base: 'main', - title: 'T', - body: '', - draft: false - }) - ).resolves.toEqual({ ok: false, error: 'socket hung up' }) + expect(mobileAllowsComposer).toBe(desktopAllowsComposer) }) }) @@ -205,27 +151,70 @@ describe('resolveMobilePrPrefill', () => { provider: 'gitlab', base: 'develop', title: 'Add feature', - body: 'Body' + body: 'Body', + canCreate: true, + blockedReason: null, + nextAction: null }) }) - it('falls back to github/main when eligibility is unavailable', async () => { + it('marks needs_push eligibility for submit-time push parity', async () => { + const client = clientWith([ + ok({ + provider: 'github', + canCreate: false, + review: null, + blockedReason: 'needs_push', + nextAction: 'push', + defaultBaseRef: 'main', + title: 'Add feature', + body: '' + }) + ]) + const prefill = await resolveMobilePrPrefill(client, 'repo-1::/tmp/wt', baseArgs) + expect(shouldPushBeforeMobilePrCreate(prefill)).toBe(true) + expect(getMobilePrCreateBlockMessage(prefill)).toBeNull() + }) + + it('returns a mobile block message for desktop-blocked create states', async () => { + const client = clientWith([ + ok({ + provider: 'github', + canCreate: false, + review: null, + blockedReason: 'dirty', + nextAction: 'commit', + defaultBaseRef: 'main' + }) + ]) + const prefill = await resolveMobilePrPrefill(client, 'repo-1::/tmp/wt', baseArgs) + expect(getMobilePrCreateBlockMessage(prefill)).toBe( + 'Commit changes before creating a pull request.' + ) + }) + + it('returns a blocked fallback when eligibility is unavailable', async () => { const client = clientWith([fail('nope')]) await expect(resolveMobilePrPrefill(client, 'repo-1::/tmp/wt', baseArgs)).resolves.toEqual({ provider: 'github', base: 'main', title: 'feature/x', - body: '' + body: '', + canCreate: false, + blockedReason: null, + nextAction: null }) }) - it('falls back without calling the RPC when there is no branch', async () => { + it('blocks without calling the RPC when there is no branch', async () => { const client = clientWith([]) const result = await resolveMobilePrPrefill(client, 'repo-1::/tmp/wt', { ...baseArgs, branch: undefined }) expect(result.provider).toBe('github') + expect(result.canCreate).toBe(false) + expect(result.blockedReason).toBe('detached_head') expect(client.calls).toEqual([]) }) }) diff --git a/mobile/src/source-control/mobile-pr-create.ts b/mobile/src/source-control/mobile-pr-create.ts index 5dba2167d..675df20be 100644 --- a/mobile/src/source-control/mobile-pr-create.ts +++ b/mobile/src/source-control/mobile-pr-create.ts @@ -1,179 +1,74 @@ -import type { - CreateHostedReviewResult, - HostedReviewCreationEligibility, - HostedReviewProvider -} from '../../../src/shared/hosted-review' -import type { RpcClient } from '../transport/rpc-client' -import type { RpcSuccess } from '../transport/types' -import { linkMobileHostedReview } from './mobile-pr-link' +import { hostedReviewCopy } from './hosted-review-copy' +import { + buildMobileHostedReviewCreateParams, + createMobileHostedReview, + fetchMobileHostedReviewEligibility, + mobileRepoSelectorFromWorktreeId, + resolveMobileHostedReviewPrefill, + shouldPushBeforeMobileHostedReviewCreate, + type MobileHostedReviewCreateInput, + type MobileHostedReviewCreateOutcome, + type MobileHostedReviewEligibilityInput, + type MobileHostedReviewPrefill +} from './mobile-hosted-review-service' -// The mobile worktree id is `${repoId}::${path}`; the repo selector the host -// hosted-review RPCs expect is `id:${repoId}`. -export function mobileRepoSelectorFromWorktreeId(worktreeId: string): string { - const separatorIdx = worktreeId.indexOf('::') - const repoId = separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx) - return `id:${repoId}` +export type MobilePrEligibilityInput = MobileHostedReviewEligibilityInput +export type MobilePrPrefill = MobileHostedReviewPrefill +export type MobilePrCreateInput = MobileHostedReviewCreateInput +export type MobilePrCreateOutcome = MobileHostedReviewCreateOutcome + +export { + buildMobileHostedReviewCreateParams as buildMobilePrCreateParams, + createMobileHostedReview as createMobilePr, + fetchMobileHostedReviewEligibility as fetchMobilePrEligibility, + mobileRepoSelectorFromWorktreeId, + resolveMobileHostedReviewPrefill as resolveMobilePrPrefill, + shouldPushBeforeMobileHostedReviewCreate as shouldPushBeforeMobilePrCreate } -export type MobilePrEligibilityInput = { - branch: string - base?: string | null - hasUncommittedChanges: boolean - hasUpstream: boolean - ahead: number - behind: number - linkedGitHubPR?: number | null - linkedGitLabMR?: number | null +export function getMobilePrCreateSuccessWarning( + outcome: Extract, + provider: MobilePrPrefill['provider'] +): string | undefined { + const copy = hostedReviewCopy(provider) + if (outcome.existing) { + return outcome.number + ? `${copy.titleLabel} #${outcome.number} is already open.` + : `${copy.titleLabel} is already open.` + } + if (outcome.linkError) { + return `${copy.titleLabel} created, but Orca could not refresh it yet.` + } + return undefined } -export async function fetchMobilePrEligibility( - client: Pick, - worktreeId: string, - input: MobilePrEligibilityInput -): Promise { - const response = await client.sendRequest('hostedReview.getCreationEligibility', { - repo: mobileRepoSelectorFromWorktreeId(worktreeId), - worktree: `id:${worktreeId}`, - branch: input.branch, - base: input.base ?? null, - hasUncommittedChanges: input.hasUncommittedChanges, - hasUpstream: input.hasUpstream, - ahead: input.ahead, - behind: input.behind, - linkedGitHubPR: input.linkedGitHubPR ?? null, - linkedGitLabMR: input.linkedGitLabMR ?? null - }) - if (!response.ok) { +export function getMobilePrCreateBlockMessage(prefill: MobilePrPrefill): string | null { + if (prefill.canCreate !== false || shouldPushBeforeMobileHostedReviewCreate(prefill)) { return null } - return (response as RpcSuccess).result as HostedReviewCreationEligibility -} - -export type MobilePrPrefill = { - provider: HostedReviewProvider - base: string - title: string - body: string -} - -// Fetches hosted-review eligibility and derives the PR compose prefill from it -// — so non-GitHub repos (e.g. GitLab) get the right provider/base instead of a -// hardcoded one. Falls back to a github/main default (with the branch label as -// title) when branch/eligibility is unavailable. -export async function resolveMobilePrPrefill( - client: Pick, - worktreeId: string, - args: { - branch: string | undefined - title: string - hasUncommittedChanges: boolean - hasUpstream: boolean - ahead: number - behind: number - } -): Promise { - const fallback: MobilePrPrefill = { - provider: 'github', - base: 'main', - title: args.title, - body: '' - } - if (!args.branch) { - return fallback - } - try { - const eligibility = await fetchMobilePrEligibility(client, worktreeId, { - branch: args.branch, - hasUncommittedChanges: args.hasUncommittedChanges, - hasUpstream: args.hasUpstream, - ahead: args.ahead, - behind: args.behind - }) - if (!eligibility) { - return fallback - } - return { - provider: eligibility.provider, - base: eligibility.defaultBaseRef || 'main', - title: eligibility.title || args.title, - body: eligibility.body || '' - } - } catch { - return fallback - } -} - -export type MobilePrCreateInput = { - provider: HostedReviewProvider - base: string - head?: string - title: string - body: string - draft: boolean -} - -// Builds the hostedReview.create params, trimming title/body and dropping empty -// optional fields so the host's required-string validation passes cleanly. -export function buildMobilePrCreateParams( - worktreeId: string, - input: MobilePrCreateInput -): Record { - return { - repo: mobileRepoSelectorFromWorktreeId(worktreeId), - worktree: `id:${worktreeId}`, - provider: input.provider, - base: input.base, - ...(input.head && input.head.length > 0 ? { head: input.head } : {}), - title: input.title.trim(), - ...(input.body.trim().length > 0 ? { body: input.body.trim() } : {}), - draft: input.draft - } -} - -export type MobilePrCreateOutcome = - | { ok: true; url: string; number: number; linkError?: string } - | { ok: false; error: string } - -export async function createMobilePr( - client: Pick, - worktreeId: string, - input: MobilePrCreateInput -): Promise { - try { - const response = await client.sendRequest( - 'hostedReview.create', - buildMobilePrCreateParams(worktreeId, input) - ) - if (!response.ok) { - return { ok: false, error: response.error?.message || 'Failed to create pull request' } - } - const result = (response as RpcSuccess).result as CreateHostedReviewResult - if (result.ok) { - const linked = await linkMobileHostedReview( - client, - worktreeId, - input.provider, - result.number, - { - // Why: mobile branch compare cannot infer the new hosted review's target - // base from renderer cache; persist the submitted base for the refresh. - baseRef: input.base - } - ) - return { - ok: true, - url: result.url, - number: result.number, - ...(linked.ok ? {} : { linkError: linked.error }) - } - } - return { ok: false, error: result.error || 'Failed to create pull request' } - } catch (err) { - // Why: create-PR runs from an inline form; transport drops should surface as - // form errors instead of escaping as unhandled promise rejections. - return { - ok: false, - error: err instanceof Error ? err.message : 'Failed to create pull request' - } + const copy = hostedReviewCopy(prefill.provider) + switch (prefill.blockedReason) { + case 'dirty': + return `Commit changes before creating a ${copy.reviewLabel}.` + case 'detached_head': + return `Check out a branch before creating a ${copy.reviewLabel}.` + case 'default_branch': + return `Switch to a feature branch before creating a ${copy.reviewLabel}.` + case 'no_upstream': + return `Publish commits before creating a ${copy.reviewLabel}.` + case 'needs_sync': + return `Sync this branch before creating a ${copy.reviewLabel}.` + case 'auth_required': + return `Authenticate before creating a ${copy.reviewLabel}.` + case 'unsupported_provider': + return `Creating ${copy.reviewLabel}s is not supported for this repo.` + case 'existing_review': + return `A ${copy.reviewLabel} already exists for this branch.` + case 'fork_head_unsupported': + return `Creating a ${copy.reviewLabel} from this fork is not supported.` + case 'needs_push': + case null: + case undefined: + return `This branch is not ready for a ${copy.reviewLabel} yet.` } } diff --git a/mobile/src/source-control/use-mobile-create-pr-runner.ts b/mobile/src/source-control/use-mobile-create-pr-runner.ts new file mode 100644 index 000000000..9952faaaa --- /dev/null +++ b/mobile/src/source-control/use-mobile-create-pr-runner.ts @@ -0,0 +1,108 @@ +import { useCallback, type MutableRefObject } from 'react' +import type { RpcClient } from '../transport/rpc-client' +import { triggerError } from '../platform/haptics' +import type { MobileGitStatusResult } from './mobile-git-status' +import type { LoadStatusOptions } from './mobile-source-control-screen-state' +import { + mobileHostedReviewCreateIntentProgressMessage, + type MobileHostedReviewCreateIntentProgress +} from './mobile-hosted-review-create-intent' +import { + runMobileHostedReviewCreateIntent, + type MobileHostedReviewCreateIntentRunOutcome +} from './mobile-hosted-review-create-intent-runner' + +type RunGitWorkflow = (actionId: string, runner: () => Promise) => Promise +type LoadStatus = (options?: LoadStatusOptions) => Promise + +type Params = { + client: RpcClient | null + worktreeId: string + status: MobileGitStatusResult | null + branchLabel: string + commitMessage: string + mountedRef: MutableRefObject + runGitWorkflow: RunGitWorkflow + loadStatus: LoadStatus + setActionError: (next: string | null) => void + setCommitMessage: (next: string) => void + setShowActionSheet: (next: boolean) => void + setCreatedPrUrl: (next: string | null) => void + setCreatedPrWarning: (next: string | null) => void +} + +export function useMobileCreatePrRunner({ + client, + worktreeId, + status, + branchLabel, + commitMessage, + mountedRef, + runGitWorkflow, + loadStatus, + setActionError, + setCommitMessage, + setShowActionSheet, + setCreatedPrUrl, + setCreatedPrWarning +}: Params) { + return useCallback( + async (pushFirst: boolean) => { + setShowActionSheet(false) + const branch = status?.branch + if (!client || !branch) { + triggerError() + setActionError('Check out a branch before creating a pull request.') + return + } + const created: { current: MobileHostedReviewCreateIntentRunOutcome | null } = { + current: null + } + const ran = await runGitWorkflow(pushFirst ? 'push-create-pr' : 'create-pr', async () => { + created.current = await runMobileHostedReviewCreateIntent(client, worktreeId, { + branch, + title: branchLabel, + status, + commitMessage, + onProgress: (progress: MobileHostedReviewCreateIntentProgress) => + setActionError(mobileHostedReviewCreateIntentProgressMessage(progress)) + }) + if (!created.current.ok) { + throw new Error(created.current.error) + } + }) + const outcome = created.current + if (outcome?.committed && mountedRef.current) { + setCommitMessage('') + } + if (!ran && outcome?.status !== undefined && mountedRef.current) { + await loadStatus({ + preserveReadyOnFailure: true, + clearActionErrorOnSuccess: false, + force: true + }) + } + if (!ran || !mountedRef.current || !outcome || !outcome.ok) { + return + } + setActionError(null) + setCreatedPrUrl(outcome.url) + setCreatedPrWarning(outcome.warning ?? null) + }, + [ + branchLabel, + client, + commitMessage, + loadStatus, + mountedRef, + runGitWorkflow, + setActionError, + setCommitMessage, + setCreatedPrUrl, + setCreatedPrWarning, + setShowActionSheet, + status, + worktreeId + ] + ) +} diff --git a/mobile/src/source-control/use-mobile-source-control-action-sheet.ts b/mobile/src/source-control/use-mobile-source-control-action-sheet.ts index 8511996fe..cedf3dad5 100644 --- a/mobile/src/source-control/use-mobile-source-control-action-sheet.ts +++ b/mobile/src/source-control/use-mobile-source-control-action-sheet.ts @@ -23,7 +23,7 @@ export function useMobileSourceControlActionSheet( runActionSheetGitSequence, runActionSheetGitSync, runActionSheetRebase, - openPrSheet, + createPr, openBranchPicker, openHistory } = state @@ -38,7 +38,8 @@ export function useMobileSourceControlActionSheet( busyAction, openingPath, openingBranchPath, - prAvailable: upstreamKnown && upstream?.hasUpstream === true, + // Why: the create intent can publish/push before creating, matching desktop. + prAvailable: upstreamKnown, handlers: { commit: () => void runActionSheetCommit(), commitPush: () => @@ -55,8 +56,8 @@ export function useMobileSourceControlActionSheet( fastForward: () => void runActionSheetGitSequence('fast-forward', [{ method: 'git.fastForward' }]), rebase: () => void runActionSheetRebase(), - createPr: () => void openPrSheet(false), - pushAndCreatePr: () => void openPrSheet(true), + createPr: () => void createPr(false), + pushAndCreatePr: () => void createPr(true), checkout: () => void openBranchPicker(), history: () => void openHistory() } @@ -64,11 +65,11 @@ export function useMobileSourceControlActionSheet( [ busyAction, commitMessage, + createPr, openBranchPicker, openHistory, openingBranchPath, openingPath, - openPrSheet, runActionSheetCommit, runActionSheetCommitSequence, runActionSheetCommitSync, diff --git a/mobile/src/source-control/use-mobile-source-control-runners.ts b/mobile/src/source-control/use-mobile-source-control-runners.ts index 79c22c3b1..a04ead696 100644 --- a/mobile/src/source-control/use-mobile-source-control-runners.ts +++ b/mobile/src/source-control/use-mobile-source-control-runners.ts @@ -2,11 +2,10 @@ import { useCallback, type MutableRefObject } from 'react' import { useRouter } from 'expo-router' import type { RpcClient } from '../transport/rpc-client' import { triggerError, triggerSuccess } from '../platform/haptics' -import type { MobilePrPrefill } from './mobile-pr-create' -import { buildOpenPrPrefill, readFreshGitStatus } from './mobile-open-pr-prefill' import { useMobileCommitMessageGeneration } from './use-mobile-commit-message-generation' import { useMobileSourceControlCommitRunners } from './use-mobile-source-control-commit-runners' import { useMobileSourceControlActionSheetRunners } from './use-mobile-source-control-action-sheet-runners' +import { useMobileCreatePrRunner } from './use-mobile-create-pr-runner' import type { RuntimeGitLocalBranches } from '../../../src/shared/runtime-types' import type { MobileGitStatusResult } from './mobile-git-status' import type { LoadStatusOptions } from './mobile-source-control-screen-state' @@ -38,8 +37,8 @@ type Params = { setShowActionSheet: (next: boolean) => void setLocalBranches: (next: RuntimeGitLocalBranches | null) => void setShowBranchPicker: (next: boolean) => void - setPrPrefill: (next: MobilePrPrefill | null) => void - setShowPrSheet: (next: boolean) => void + setCreatedPrUrl: (next: string | null) => void + setCreatedPrWarning: (next: string | null) => void } // All git workflow + action-sheet runners for the source-control panel. Split @@ -70,8 +69,8 @@ export function useMobileSourceControlRunners(params: Params) { setShowActionSheet, setLocalBranches, setShowBranchPicker, - setPrPrefill, - setShowPrSheet + setCreatedPrUrl, + setCreatedPrWarning } = params const runGitWorkflow = useCallback( @@ -184,46 +183,21 @@ export function useMobileSourceControlRunners(params: Params) { setActionError }) - const openPrSheet = useCallback( - async (pushFirst: boolean) => { - setShowActionSheet(false) - let effectiveStatus = status - if (pushFirst) { - const pushed = await runGitWorkflow('push-create-pr', async () => { - await sendGitRequest('git.push') - }) - if (!pushed || !mountedRef.current) { - return - } - // Why: the captured `status` predates the push, so its upstream/ahead data is - // stale; read fresh git.status so the prefill reflects the just-pushed branch. - if (client) { - effectiveStatus = await readFreshGitStatus(worktreeId, status, sendGitRequest) - if (!mountedRef.current) { - return - } - } - } - const prefill = await buildOpenPrPrefill(client, worktreeId, effectiveStatus, branchLabel) - if (!mountedRef.current) { - return - } - setPrPrefill(prefill) - setShowPrSheet(true) - }, - [ - branchLabel, - client, - mountedRef, - runGitWorkflow, - sendGitRequest, - setPrPrefill, - setShowActionSheet, - setShowPrSheet, - status, - worktreeId - ] - ) + const createPr = useMobileCreatePrRunner({ + client, + worktreeId, + status, + branchLabel, + commitMessage, + mountedRef, + runGitWorkflow, + loadStatus, + setActionError, + setCommitMessage, + setShowActionSheet, + setCreatedPrUrl, + setCreatedPrWarning + }) const openBranchPicker = useCallback(() => { setShowActionSheet(false) @@ -304,7 +278,7 @@ export function useMobileSourceControlRunners(params: Params) { commit, generateCommitMessage, cancelGenerateCommitMessage, - openPrSheet, + createPr, openBranchPicker, openHistory, checkoutBranch, diff --git a/mobile/src/source-control/use-mobile-source-control-state.ts b/mobile/src/source-control/use-mobile-source-control-state.ts index 10a7c1813..6ca585a3a 100644 --- a/mobile/src/source-control/use-mobile-source-control-state.ts +++ b/mobile/src/source-control/use-mobile-source-control-state.ts @@ -3,7 +3,6 @@ import { Keyboard, Platform } from 'react-native' import { useSafeAreaInsets } from 'react-native-safe-area-context' import { useHostClient, useForceReconnect } from '../transport/client-context' import { getWorktreeLabel } from '../session/worktree-label' -import type { MobilePrPrefill } from './mobile-pr-create' import { useMobileGitRequests } from './use-mobile-git-requests' import { useMobileSourceControlLoaders } from './use-mobile-source-control-loaders' import { useMobileSourceControlOpeners } from './use-mobile-source-control-openers' @@ -50,12 +49,10 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara const [busyAction, setBusyAction] = useState(null) const [commitMessage, setCommitMessage] = useState('') const [generatingMessage, setGeneratingMessage] = useState(false) - const [showPrSheet, setShowPrSheet] = useState(false) const [showBranchPicker, setShowBranchPicker] = useState(false) const [localBranches, setLocalBranches] = useState(null) const [createdPrUrl, setCreatedPrUrl] = useState(null) const [createdPrWarning, setCreatedPrWarning] = useState(null) - const [prPrefill, setPrPrefill] = useState(null) const [discardTarget, setDiscardTarget] = useState(null) const [showActionSheet, setShowActionSheet] = useState(false) const [actionError, setActionError] = useState(null) @@ -203,8 +200,8 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara setShowActionSheet, setLocalBranches, setShowBranchPicker, - setPrPrefill, - setShowPrSheet + setCreatedPrUrl, + setCreatedPrWarning }) const primaryAction = useMemo( () => @@ -261,8 +258,6 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara commitMessage, setCommitMessage, generatingMessage, - showPrSheet, - setShowPrSheet, showBranchPicker, setShowBranchPicker, localBranches, @@ -270,7 +265,6 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara setCreatedPrUrl, createdPrWarning, setCreatedPrWarning, - prPrefill, discardTarget, setDiscardTarget, showActionSheet,