From 7d229f58f1dbb78b59ffe9193f188b495710f2d7 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 27 Jun 2026 14:49:10 -0700 Subject: [PATCH] Implement automated git preparation workflow for mobile PR creation Introduce a structured hosted review intent preparation workflow to handle staging, AI commit message generation, committing, and pushing changes automatically before displaying the pull request composer on mobile. - Map creation block reasons to descriptive user-facing validation errors (e.g., dirty working tree, default branch, detached head) to match desktop. - Decouple hosted-review business logic into a dedicated service helper. - Update source control runner hooks to handle the new preparation flow. --- mobile/app/h/[hostId]/pr/[worktreeId].tsx | 3 +- .../app/h/[hostId]/session/[worktreeId].tsx | 2 + mobile/pnpm-lock.yaml | 26 -- .../components/MobileDiffReviewScreenView.tsx | 3 + mobile/src/components/MobilePRSidebar.tsx | 20 +- .../pr-sidebar/MobilePrComposeForm.tsx | 37 ++- .../pr-sidebar/MobilePrViewPanel.tsx | 4 + .../pr-sidebar/PrSidebarCreateEmptyState.tsx | 49 ++- mobile/src/session/SessionDockColumn.tsx | 6 + .../use-mobile-pr-branch-context.test.ts | 3 +- .../session/use-mobile-pr-branch-context.ts | 12 +- ...mobile-hosted-review-create-intent.test.ts | 150 ++++++++++ .../mobile-hosted-review-create-intent.ts | 279 ++++++++++++++++++ .../mobile-hosted-review-service.ts | 266 +++++++++++++++++ .../mobile-open-pr-prefill.test.ts | 22 +- .../source-control/mobile-open-pr-prefill.ts | 30 +- .../mobile-pr-create-flow.test.ts | 258 ++++++++++++++++ .../source-control/mobile-pr-create.test.ts | 219 +++++++------- mobile/src/source-control/mobile-pr-create.ts | 223 ++++---------- .../use-mobile-open-pr-sheet-runner.ts | 99 +++++++ .../use-mobile-source-control-runners.ts | 56 +--- 21 files changed, 1365 insertions(+), 402 deletions(-) create mode 100644 mobile/src/source-control/mobile-hosted-review-create-intent.test.ts create mode 100644 mobile/src/source-control/mobile-hosted-review-create-intent.ts create mode 100644 mobile/src/source-control/mobile-hosted-review-service.ts create mode 100644 mobile/src/source-control/mobile-pr-create-flow.test.ts create mode 100644 mobile/src/source-control/use-mobile-open-pr-sheet-runner.ts diff --git a/mobile/app/h/[hostId]/pr/[worktreeId].tsx b/mobile/app/h/[hostId]/pr/[worktreeId].tsx index 6db023bee08..2e0530b0f4b 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 8dbfb932c24..5175ba90c98 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/pnpm-lock.yaml b/mobile/pnpm-lock.yaml index 700bcc09d05..a7abf175872 100644 --- a/mobile/pnpm-lock.yaml +++ b/mobile/pnpm-lock.yaml @@ -1721,56 +1721,48 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-arm64-musl@0.52.0': resolution: {integrity: sha512-wZg6bLjDvh2KibyI3QFUYo8GTXneIFsd0JvehtvJiUmQ8WRPERgxd/VM4ctWb86U5FT1FkqgS8/wZKVB+AZScg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@oxfmt/binding-linux-ppc64-gnu@0.52.0': resolution: {integrity: sha512-IngE8uxhNvxcMrLjZNDo9xNLY7rEK33AKnaMd2B46he1e/mz2CfcW6If/U1wUjdRZddm1QzQaciqZkuMkdh1FA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-riscv64-gnu@0.52.0': resolution: {integrity: sha512-H3+DdFMv/efN3Efmhsv18jDrpiWWqKG7wsfAlQBqAt6z/E2Bx+TwEj2Nowe51CPOWB8/mFBC2dAMSgVFLvvowA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-riscv64-musl@0.52.0': resolution: {integrity: sha512-zji+1kb7lJKohSDjzC1IsS+K/cKRs1hdVf0ZH0VbdbiakmtLvN9twBoXo/k8VdjFax7kfo+DyPxS7vv52br1aw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [musl] '@oxfmt/binding-linux-s390x-gnu@0.52.0': resolution: {integrity: sha512-hcLBYedpCy7ToUvvBidWk7+11Yhg1oAZ4+6hKPic/mQI6NaqXJSXMps5nFlwUuX2ewhtLZZDPg63TI042qGKBg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-x64-gnu@0.52.0': resolution: {integrity: sha512-IDO2loXK2OtTOhSPchU9MW25mWL2QCDGdJbjN8MXKZVS80qXe5gMTwQWu/gMJ3juoBHbkuUZNB2N1LHzNT7DoA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@oxfmt/binding-linux-x64-musl@0.52.0': resolution: {integrity: sha512-mAV2Hjn0SatJ+KoAzKUC3eJhdJ8wv+3m1KyuS0dTsbF0c5weq+QrCt/DRZZM+uj/XiKzCDEUKYsBF30e2qkcyw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@oxfmt/binding-openharmony-arm64@0.52.0': resolution: {integrity: sha512-vd4npaUIwChxp7XzkqmepBWTT9YMcSe/NBApVGPC30/lLyOVaV3dvma1SKo03t8O73BPRAG7EyJzGlN5cJM5hQ==} @@ -1843,56 +1835,48 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-arm64-musl@1.67.0': resolution: {integrity: sha512-zB/Tf6sUjmmvvbva9Gj3JTJ8rJ9t4I8/U0o6vSRtd0DRIsIuyegBwJAzhSUFQHdMijIRJkW0exs/yBhpw2S20w==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@oxlint/binding-linux-ppc64-gnu@1.67.0': resolution: {integrity: sha512-kgU40Gt74CK0TCsF51KZymkIwN9U0BajKsMijB52zPqOeZU9NAHkA/NSQkZDHEaCakx42DxhXkODiAqf2b4Gug==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-riscv64-gnu@1.67.0': resolution: {integrity: sha512-tOYhkk/iaG9aD3FvGpBFd1Lrw0x0RaVoJBxjUkfNzS50rC5NS5BteNCwgr8A2zCdADrIIoze6D7u6U5Ic++/iQ==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-riscv64-musl@1.67.0': resolution: {integrity: sha512-sEtywrPb+0b+tHYl1SDCrw903fiC4eyKoNqzP3v+f2JT3Xcv4NEYG+P8rj+eEnX7IWhqV/xj8/JmcmVj21CXaA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] - libc: [musl] '@oxlint/binding-linux-s390x-gnu@1.67.0': resolution: {integrity: sha512-BvR8Moa0zCLxroOx4vZaZN9nUfwAUpSTwjZdxZyKy4bv3PrzrXrxKR/ZQ0L9wNSvlPhnMJeZfa3q5w6ZCTuN6Q==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@oxlint/binding-linux-x64-gnu@1.67.0': resolution: {integrity: sha512-mm2cxM6fksOpq6l0uFws8BUGKAR4dNa/cZCn37Npq7PFbhD5HDJqWfnoIvTaeRKMy5XdS2tO0MA0qbHDrnXAAA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@oxlint/binding-linux-x64-musl@1.67.0': resolution: {integrity: sha512-WmbMuLapKyDlobMkXAaAL0Y+Uczh4LETfIfQsUpbId4Ip8Ai82/jqeYTOoUCkuuhBFapgqP253+d83tLKOksJg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@oxlint/binding-openharmony-arm64@1.67.0': resolution: {integrity: sha512-9g/PqxYJelzzTAOR5Y+RiRqdeydhEuXv2KxNeFcAKQ7UsvnWSY1OP4MsuPMbTO2Pf70tz7mFhl1j13H3fyh+8g==} @@ -2354,42 +2338,36 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-arm64-musl@1.0.0': resolution: {integrity: sha512-EIVjy2cgd7uuMMo94FVkBp7F6DhcZAUwNURkSG3RwUmvAXR6s0ISxM81U+IydcZByPG0pZIHsf1b6kTxoFDgJA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] - libc: [musl] '@rolldown/binding-linux-ppc64-gnu@1.0.0': resolution: {integrity: sha512-JEwwOPcwTLAcpDQlqSmjEmfs63xJnSiUNIGvLcDLUHCWK4XowpS/7c7tUsUH6uT/ct6bMUTdXKfI8967FYj6mg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-s390x-gnu@1.0.0': resolution: {integrity: sha512-0wjCFhLrihtAubnT9iA0N++0pSV0z5Hg7tNGdNJ4RFaINceHadoF+kiFGyY1qSSNVIAZtLotG8Ju1bgDPkjnFA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] - libc: [glibc] '@rolldown/binding-linux-x64-gnu@1.0.0': resolution: {integrity: sha512-Dfn7iak9BcMMePxcoJfpSbWqnEyrp/dRF63/8qW/eHBdOZov6x5aShLLEYGYdIeSJ6vMLK/XCVB+lGIxm41bQA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [glibc] '@rolldown/binding-linux-x64-musl@1.0.0': resolution: {integrity: sha512-5/utzzDmD/pD/bmuaUcbTf/sZYy0aztwIVlfpoW1fTjCZ0BaPOMVWGZL1zvgxyi7ZIVYWlxKONHmSbHuiOh8Jw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] - libc: [musl] '@rolldown/binding-openharmony-arm64@1.0.0': resolution: {integrity: sha512-ouJs8VcUomfLfpbUECqFMRqdV4x6aeAK3MA4m6vTrJJjKyWTV5KnxZx7Jd9G+GlDaQQxubcba00x16OyJ1meig==} @@ -4651,28 +4629,24 @@ packages: engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] - libc: [glibc] lightningcss-linux-arm64-musl@1.32.0: resolution: {integrity: sha512-UpQkoenr4UJEzgVIYpI80lDFvRmPVg6oqboNHfoH4CQIfNA+HOrZ7Mo7KZP02dC6LjghPQJeBsvXhJod/wnIBg==} engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] - libc: [musl] lightningcss-linux-x64-gnu@1.32.0: resolution: {integrity: sha512-V7Qr52IhZmdKPVr+Vtw8o+WLsQJYCTd8loIfpDaMRWGUZfBOYEJeyJIkqGIDMZPwPx24pUMfwSxxI8phr/MbOA==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] - libc: [glibc] lightningcss-linux-x64-musl@1.32.0: resolution: {integrity: sha512-bYcLp+Vb0awsiXg/80uCRezCYHNg1/l3mt0gzHnWV9XP1W5sKa5/TCdGWaR/zBM2PeF/HbsQv/j2URNOiVuxWg==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] - libc: [musl] lightningcss-win32-arm64-msvc@1.32.0: resolution: {integrity: sha512-8SbC8BR40pS6baCM8sbtYDSwEVQd4JlFTOlaD3gWGHfThTcABnNDBda6eTZeqbofalIJhFx0qKzgHJmcPTnGdw==} diff --git a/mobile/src/components/MobileDiffReviewScreenView.tsx b/mobile/src/components/MobileDiffReviewScreenView.tsx index b63cd3d6b9e..c472d1482db 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 2dc90611a98..71cc33d5afe 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 765a30dba7e..6837686907e 100644 --- a/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx @@ -8,12 +8,15 @@ 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, + shouldPushBeforeMobilePrCreate, + type MobilePrPrefill +} from '../../source-control/mobile-pr-create' import { hostedReviewCopy } from '../../source-control/hosted-review-copy' import { getPrComposeDisabledReason, @@ -22,12 +25,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 +59,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 +127,18 @@ 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 = outcome.existing + ? outcome.number + ? `${copy.titleLabel} #${outcome.number} is already open.` + : `${copy.titleLabel} is already open.` + : outcome.linkError + ? `${copy.titleLabel} created, but Orca could not refresh it yet.` + : undefined if (warning) { setError(warning) } @@ -156,6 +160,7 @@ export function MobilePrComposeForm({ head, onCreated, prefill.provider, + pushBeforeCreate, submitting, title, worktreeId @@ -279,7 +284,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 b733c5f4f13..e1b19263f54 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 3f3b74eeb7a..0a70ae0a22d 100644 --- a/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx +++ b/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx @@ -3,7 +3,15 @@ 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 { + getMobilePrCreateBlockMessage, + type MobilePrPrefill +} from '../../source-control/mobile-pr-create' +import type { MobileGitStatusResult } from '../../source-control/mobile-git-status' +import { + mobileHostedReviewCreateIntentProgressMessage, + prepareMobileHostedReviewCreateIntent +} from '../../source-control/mobile-hosted-review-create-intent' import { fetchWorktreeLinkedPR } from '../../source-control/mobile-pr-link' import { openMobilePrUrl } from '../MobilePrComposeSheet' import { MobilePrComposeForm } from './MobilePrComposeForm' @@ -14,6 +22,7 @@ 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 } @@ -23,7 +32,13 @@ type Mode = 'choose' | 'create' | '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) { +export function PrSidebarCreateEmptyState({ + client, + worktreeId, + gitBranch, + gitStatus, + onCreated +}: Props) { const [prefill, setPrefill] = useState(null) const [mode, setMode] = useState('choose') const [loading, setLoading] = useState(false) @@ -61,17 +76,27 @@ 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 + } + const prepared = await prepareMobileHostedReviewCreateIntent(client, worktreeId, { + branch: gitBranch, + title: gitBranch, + status: gitStatus, + onProgress: (progress) => + setCreateWarning(mobileHostedReviewCreateIntentProgressMessage(progress)) }) + if (!prepared.ok) { + setCreateWarning(prepared.error) + return + } + const resolved = prepared.prefill + const blockedMessage = getMobilePrCreateBlockMessage(resolved) + if (blockedMessage) { + setCreateWarning(blockedMessage) + return + } setPrefill(resolved) setMode('create') } catch { diff --git a/mobile/src/session/SessionDockColumn.tsx b/mobile/src/session/SessionDockColumn.tsx index 9b02b97580a..877973e5d0d 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/use-mobile-pr-branch-context.test.ts b/mobile/src/session/use-mobile-pr-branch-context.test.ts index 4890f887b46..7ac21cf3006 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 b0f4a9baa64..ef2cf1a48d5 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/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 00000000000..5f004f9c239 --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent.test.ts @@ -0,0 +1,150 @@ +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 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('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.' + }) + + expect(client.calls.map((call) => call.method)).toEqual([ + 'git.status', + 'git.generateCommitMessage' + ]) + }) +}) 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 00000000000..2bf01eb509c --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-create-intent.ts @@ -0,0 +1,279 @@ +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' + +export type MobileHostedReviewCreateIntentOutcome = + | { + ok: true + prefill: MobilePrPrefill + status: MobileGitStatusResult | null + committed: boolean + } + | { ok: false; error: string } + +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...' + } +} + +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 { + return ( + !status?.branch || + status.branch === inputBranch || + status.branch === `refs/heads/${inputBranch}` + ) +} + +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 } +> { + if ((currentStatus?.entries.length ?? 0) === 0) { + return { ok: true, status: currentStatus, committed: false } + } + + 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.' } + } + } + + 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.' } + } + + 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.' + } + } + message = generated.message + } + + input.onProgress?.('committing') + const committed = await commitStagedChanges(client, worktreeId, message) + if (!committed.ok) { + return committed + } + currentStatus = await readStatus(client, worktreeId) + if (!branchStillMatches(input.branch, currentStatus)) { + return { ok: false, error: 'Branch changed while preparing the pull request.' } + } + 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 + } + 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.' } + } + 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 00000000000..68ad9a30d53 --- /dev/null +++ b/mobile/src/source-control/mobile-hosted-review-service.ts @@ -0,0 +1,266 @@ +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 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, + ...(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 a3f67fde7f1..cf977b0b2b2 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 70b203057b8..705b4dc68e5 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 00000000000..d896d5d2e57 --- /dev/null +++ b/mobile/src/source-control/mobile-pr-create-flow.test.ts @@ -0,0 +1,258 @@ +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('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 e82fb4eed55..ce877dc3c41 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 5dba2167d87..49d1a50e6e8 100644 --- a/mobile/src/source-control/mobile-pr-create.ts +++ b/mobile/src/source-control/mobile-pr-create.ts @@ -1,179 +1,58 @@ -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 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-open-pr-sheet-runner.ts b/mobile/src/source-control/use-mobile-open-pr-sheet-runner.ts new file mode 100644 index 00000000000..dc8a9793cd3 --- /dev/null +++ b/mobile/src/source-control/use-mobile-open-pr-sheet-runner.ts @@ -0,0 +1,99 @@ +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 { getMobilePrCreateBlockMessage, type MobilePrPrefill } from './mobile-pr-create' +import { + mobileHostedReviewCreateIntentProgressMessage, + prepareMobileHostedReviewCreateIntent, + type MobileHostedReviewCreateIntentOutcome +} from './mobile-hosted-review-create-intent' + +type RunGitWorkflow = (actionId: string, runner: () => Promise) => Promise + +type Params = { + client: RpcClient | null + worktreeId: string + status: MobileGitStatusResult | null + branchLabel: string + commitMessage: string + mountedRef: MutableRefObject + runGitWorkflow: RunGitWorkflow + setActionError: (next: string | null) => void + setCommitMessage: (next: string) => void + setShowActionSheet: (next: boolean) => void + setPrPrefill: (next: MobilePrPrefill | null) => void + setShowPrSheet: (next: boolean) => void +} + +export function useMobileOpenPrSheetRunner({ + client, + worktreeId, + status, + branchLabel, + commitMessage, + mountedRef, + runGitWorkflow, + setActionError, + setCommitMessage, + setShowActionSheet, + setPrPrefill, + setShowPrSheet +}: 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 prepared: { current: MobileHostedReviewCreateIntentOutcome | null } = { current: null } + const ran = await runGitWorkflow(pushFirst ? 'push-create-pr' : 'create-pr', async () => { + prepared.current = await prepareMobileHostedReviewCreateIntent(client, worktreeId, { + branch, + title: branchLabel, + status, + commitMessage, + onProgress: (progress) => + setActionError(mobileHostedReviewCreateIntentProgressMessage(progress)) + }) + if (!prepared.current.ok) { + throw new Error(prepared.current.error) + } + }) + const outcome = prepared.current + if (!ran || !mountedRef.current || !outcome || !outcome.ok) { + return + } + const prefill = outcome.prefill + if (outcome.committed) { + setCommitMessage('') + } + const blockedMessage = getMobilePrCreateBlockMessage(prefill) + if (blockedMessage) { + triggerError() + setActionError(blockedMessage) + return + } + setActionError(null) + setPrPrefill(prefill) + setShowPrSheet(true) + }, + [ + branchLabel, + client, + commitMessage, + mountedRef, + runGitWorkflow, + setActionError, + setCommitMessage, + setPrPrefill, + setShowActionSheet, + setShowPrSheet, + status, + worktreeId + ] + ) +} 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 79c22c3b170..8f543838eb3 100644 --- a/mobile/src/source-control/use-mobile-source-control-runners.ts +++ b/mobile/src/source-control/use-mobile-source-control-runners.ts @@ -3,10 +3,10 @@ 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 { useMobileOpenPrSheetRunner } from './use-mobile-open-pr-sheet-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' @@ -184,46 +184,20 @@ 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 openPrSheet = useMobileOpenPrSheetRunner({ + client, + worktreeId, + status, + branchLabel, + commitMessage, + mountedRef, + runGitWorkflow, + setActionError, + setCommitMessage, + setShowActionSheet, + setPrPrefill, + setShowPrSheet + }) const openBranchPicker = useCallback(() => { setShowActionSheet(false)