Link created pull requests to mobile worktree across providers (#5955)

* Automatically link newly created pull requests/merge requests to the
  worktree using the provider-specific metadata key (e.g., linkedPR,
  linkedGitLabMR) and persist the submitted base ref.
* Prefer worktree-specific base ref returned by worktree.show when
  resolving the comparison base ref.
* Handle metadata linking failures gracefully as a non-fatal warning
  without blocking the pull request creation flow.
This commit is contained in:
Jinjing 2026-06-20 21:15:02 -07:00 committed by GitHub
parent 7a04b1e26f
commit 144df671ab
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 301 additions and 23 deletions

View File

@ -11,7 +11,7 @@ type Props = {
// Head branch — enables the base≠head guard and the "from <branch>" hint.
head?: string | null
onClose: () => void
onCreated: (url: string) => void
onCreated: (url: string, warning?: string) => void
}
// BottomDrawer wrapper around the inline compose form, for full-screen roots

View File

@ -36,7 +36,7 @@ type Props = {
// Head branch — enables the base≠head guard and the "from <branch>" hint.
head?: string | null
onCancel: () => void
onCreated: (url: string) => void
onCreated: (url: string, warning?: string) => void
}
// PR compose form body: title/body/base/draft with AI prefill (git.generate
@ -132,7 +132,13 @@ export function MobilePrComposeForm({
})
if (outcome.ok) {
triggerSuccess()
onCreated(outcome.url)
const warning = outcome.linkError
? `${copy.titleLabel} created, but Orca could not refresh it yet.`
: undefined
if (warning) {
setError(warning)
}
onCreated(outcome.url, warning)
} else {
triggerError()
setError(outcome.error)
@ -145,6 +151,7 @@ export function MobilePrComposeForm({
body,
canSubmit,
client,
copy.titleLabel,
draft,
head,
onCreated,

View File

@ -25,6 +25,7 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre
const [prefill, setPrefill] = useState<MobilePrPrefill | null>(null)
const [mode, setMode] = useState<Mode>('choose')
const [loading, setLoading] = useState(false)
const [createWarning, setCreateWarning] = useState<string | null>(null)
// A persisted linkedPR while the branch shows no PR means the linked PR could
// not be resolved. Mention it, but keep link editing out of this desktop-parity
// create surface.
@ -56,6 +57,7 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre
if (!client || loading) {
return
}
setCreateWarning(null)
setLoading(true)
try {
// Git-status fields are best-effort here (the sidebar has no working-tree
@ -90,8 +92,9 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre
prefill={prefill}
head={gitBranch}
onCancel={() => setMode('choose')}
onCreated={(url) => {
onCreated={(url, warning) => {
setMode('choose')
setCreateWarning(warning ?? null)
openMobilePrUrl(url)
onCreated()
}}
@ -144,6 +147,7 @@ export function PrSidebarCreateEmptyState({ client, worktreeId, gitBranch, onCre
? `${gitBranch} is not linked to an open PR.`
: 'The current branch is not linked to an open PR.'}
</Text>
{createWarning ? <Text style={styles.bodyText}>{createWarning}</Text> : null}
</View>
</View>
)

View File

@ -28,6 +28,8 @@ export function MobileSourceControlModals({ state, worktreeId, actionSheetAction
localBranches,
createdPrUrl,
setCreatedPrUrl,
createdPrWarning,
setCreatedPrWarning,
status,
branchLabel,
loadStatus,
@ -79,9 +81,10 @@ export function MobileSourceControlModals({ state, worktreeId, actionSheetAction
prefill={prPrefill ?? { provider: 'github', base: 'main', title: branchLabel, body: '' }}
head={status?.branch ?? null}
onClose={() => setShowPrSheet(false)}
onCreated={(url) => {
onCreated={(url, warning) => {
setShowPrSheet(false)
setCreatedPrUrl(url)
setCreatedPrWarning(warning ?? null)
void loadStatus({ preserveReadyOnFailure: true, force: true })
}}
/>
@ -108,15 +111,23 @@ export function MobileSourceControlModals({ state, worktreeId, actionSheetAction
<ConfirmModal
visible={createdPrUrl !== null}
title="Pull Request Created"
message="Open it in your browser?"
message={
createdPrWarning
? `Open it in your browser?\n\n${createdPrWarning}`
: 'Open it in your browser?'
}
confirmLabel="Open"
onConfirm={() => {
if (createdPrUrl) {
openMobilePrUrl(createdPrUrl)
}
setCreatedPrUrl(null)
setCreatedPrWarning(null)
}}
onCancel={() => {
setCreatedPrUrl(null)
setCreatedPrWarning(null)
}}
onCancel={() => setCreatedPrUrl(null)}
/>
</>
)

View File

@ -0,0 +1,70 @@
import { describe, expect, it } from 'vitest'
import { resolveMobileBranchCompareBaseRef } from './mobile-branch-base-ref'
import type { RpcClient } from '../transport/rpc-client'
type RpcResponse =
| { ok: true; result: unknown }
| { ok: false; error: { code: string; message: string } }
function ok(result: unknown): RpcResponse {
return { ok: true, result }
}
function fail(message: string): RpcResponse {
return { ok: false, error: { code: 'runtime_error', message } }
}
function clientWith(responses: RpcResponse[]) {
const calls: { method: string; params?: Record<string, unknown> }[] = []
return {
calls,
client: {
sendRequest: async (method: string, params?: Record<string, unknown>) => {
calls.push({ method, params })
return responses.shift() ?? fail(`unexpected ${method}`)
}
} as Pick<RpcClient, 'sendRequest'> as RpcClient
}
}
describe('resolveMobileBranchCompareBaseRef', () => {
it('prefers the per-worktree base ref over the repo default', async () => {
const { client, calls } = clientWith([
ok({ worktree: { baseRef: 'origin/release' } }),
ok({ repos: [{ id: 'repo-1', worktreeBaseRef: 'origin/main' }] })
])
await expect(resolveMobileBranchCompareBaseRef(client, 'repo-1::/tmp/wt')).resolves.toBe(
'origin/release'
)
expect(calls.map((call) => call.method)).toEqual(['worktree.show', 'repo.list'])
})
it('falls back to repo worktreeBaseRef when the worktree has no pinned base', async () => {
const { client } = clientWith([
ok({ worktree: { baseRef: null } }),
ok({ repos: [{ id: 'repo-1', worktreeBaseRef: 'origin/main' }] })
])
await expect(resolveMobileBranchCompareBaseRef(client, 'repo-1::/tmp/wt')).resolves.toBe(
'origin/main'
)
})
it('falls back to repo.baseRefDefault when metadata reads have no base', async () => {
const { client, calls } = clientWith([
fail('old host'),
ok({ repos: [{ id: 'repo-1', worktreeBaseRef: null }] }),
ok({ defaultBaseRef: 'origin/main' })
])
await expect(resolveMobileBranchCompareBaseRef(client, 'repo-1::/tmp/wt')).resolves.toBe(
'origin/main'
)
expect(calls.map((call) => call.method)).toEqual([
'worktree.show',
'repo.list',
'repo.baseRefDefault'
])
})
})

View File

@ -6,6 +6,10 @@ type RuntimeRepoSummary = {
worktreeBaseRef?: string | null
}
type RuntimeWorktreeSummary = {
baseRef?: string | null
}
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null
}
@ -40,6 +44,15 @@ function readDefaultBaseRef(value: unknown): string | null {
return typeof value.defaultBaseRef === 'string' ? value.defaultBaseRef.trim() || null : null
}
function readWorktreeSummary(value: unknown): RuntimeWorktreeSummary | null {
if (!isRecord(value) || !isRecord(value.worktree)) {
return null
}
return {
baseRef: typeof value.worktree.baseRef === 'string' ? value.worktree.baseRef : null
}
}
export async function resolveMobileBranchCompareBaseRef(
client: RpcClient,
worktreeId: string
@ -49,15 +62,23 @@ export async function resolveMobileBranchCompareBaseRef(
return null
}
let repoBaseRef: string | null = null
const repoResponse = await client.sendRequest('repo.list')
if (repoResponse.ok) {
const repo = readRepoSummaries(repoResponse.result).find((candidate) => candidate.id === repoId)
repoBaseRef = repo?.worktreeBaseRef?.trim() || null
const [worktreeResponse, repoResponse] = await Promise.all([
client.sendRequest('worktree.show', { worktree: `id:${worktreeId}` }).catch(() => null),
client.sendRequest('repo.list').catch(() => null)
])
if (worktreeResponse?.ok) {
const worktreeBaseRef = readWorktreeSummary(worktreeResponse.result)?.baseRef?.trim() || null
if (worktreeBaseRef) {
return worktreeBaseRef
}
}
if (repoBaseRef) {
return repoBaseRef
if (repoResponse?.ok) {
const repo = readRepoSummaries(repoResponse.result).find((candidate) => candidate.id === repoId)
const repoBaseRef = repo?.worktreeBaseRef?.trim() || null
if (repoBaseRef) {
return repoBaseRef
}
}
const defaultResponse = await client.sendRequest('repo.baseRefDefault', { repo: `id:${repoId}` })

View File

@ -69,7 +69,10 @@ 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' })])
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',
@ -80,6 +83,56 @@ describe('createMobilePr', () => {
})
).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('maps a host failure result to { ok:false }', async () => {

View File

@ -5,6 +5,7 @@ import type {
} from '../../../src/shared/hosted-review'
import type { RpcClient } from '../transport/rpc-client'
import type { RpcSuccess } from '../transport/types'
import { linkMobileHostedReview } from './mobile-pr-link'
// The mobile worktree id is `${repoId}::${path}`; the repo selector the host
// hosted-review RPCs expect is `id:${repoId}`.
@ -130,7 +131,7 @@ export function buildMobilePrCreateParams(
}
export type MobilePrCreateOutcome =
| { ok: true; url: string; number: number }
| { ok: true; url: string; number: number; linkError?: string }
| { ok: false; error: string }
export async function createMobilePr(
@ -148,7 +149,23 @@ export async function createMobilePr(
}
const result = (response as RpcSuccess).result as CreateHostedReviewResult
if (result.ok) {
return { ok: true, url: result.url, number: result.number }
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) {

View File

@ -1,6 +1,11 @@
import { describe, expect, it, vi } from 'vitest'
import type { RpcClient } from '../transport/rpc-client'
import { buildWorktreeSetLinkParams, fetchWorktreeLinkedPR, linkMobilePr } from './mobile-pr-link'
import {
buildWorktreeSetHostedReviewLinkParams,
buildWorktreeSetLinkParams,
fetchWorktreeLinkedPR,
linkMobilePr
} from './mobile-pr-link'
describe('buildWorktreeSetLinkParams', () => {
it('sets linkedPR to a number when linking', () => {
@ -18,6 +23,39 @@ describe('buildWorktreeSetLinkParams', () => {
})
})
describe('buildWorktreeSetHostedReviewLinkParams', () => {
it.each([
['github', 'linkedPR'],
['gitlab', 'linkedGitLabMR'],
['bitbucket', 'linkedBitbucketPR'],
['azure-devops', 'linkedAzureDevOpsPR'],
['gitea', 'linkedGiteaPR']
] as const)('maps %s reviews to the matching worktree field', (provider, key) => {
expect(buildWorktreeSetHostedReviewLinkParams('repo42::/p', provider, 12)).toEqual({
worktree: 'id:repo42::/p',
[key]: 12
})
})
it('includes a submitted base ref so mobile diff review refreshes against the review base', () => {
expect(
buildWorktreeSetHostedReviewLinkParams('repo42::/p', 'github', 12, {
baseRef: ' origin/release '
})
).toEqual({
worktree: 'id:repo42::/p',
baseRef: 'origin/release',
linkedPR: 12
})
})
it('does not invent a metadata field for unsupported providers', () => {
expect(buildWorktreeSetHostedReviewLinkParams('repo42::/p', 'unsupported', 12)).toEqual({
worktree: 'id:repo42::/p'
})
})
})
describe('fetchWorktreeLinkedPR', () => {
const client = (result: unknown, okFlag = true) =>
({

View File

@ -1,11 +1,11 @@
import type { RpcClient } from '../transport/rpc-client'
import type { RpcSuccess } from '../transport/types'
import type { HostedReviewProvider } from '../../../src/shared/hosted-review'
// Link / unlink an existing PR to the current worktree via worktree.set (the same
// path desktop's "Link another PR" uses). GitHub-scoped: it writes the worktree's
// `linkedPR` key, matching desktop where linking is GitHub-only (GitLab/Bitbucket
// use separate linked* keys). linkedPR is tri-state on the host: a number sets the
// link, null clears it. worktree.set is allowlisted for mobile.
// Link / unlink review metadata via worktree.set (the same path desktop uses).
// GitHub's existing manual link flow writes linkedPR; hosted-review creation maps
// each provider to its own linked* field so mobile follow-up reads get the same
// authoritative hint as desktop.
export type MobilePrLinkOutcome = { ok: true } | { ok: false; error: string }
@ -17,6 +17,33 @@ export function buildWorktreeSetLinkParams(
return { worktree: `id:${worktreeId}`, linkedPR }
}
export function buildWorktreeSetHostedReviewLinkParams(
worktreeId: string,
provider: HostedReviewProvider,
number: number | null,
options?: { baseRef?: string | null }
): Record<string, unknown> {
const trimmedBaseRef = options?.baseRef?.trim()
const base = {
worktree: `id:${worktreeId}`,
...(trimmedBaseRef ? { baseRef: trimmedBaseRef } : {})
}
switch (provider) {
case 'github':
return { ...base, linkedPR: number }
case 'gitlab':
return { ...base, linkedGitLabMR: number }
case 'bitbucket':
return { ...base, linkedBitbucketPR: number }
case 'azure-devops':
return { ...base, linkedAzureDevOpsPR: number }
case 'gitea':
return { ...base, linkedGiteaPR: number }
case 'unsupported':
return base
}
}
async function setLinkedPr(
client: Pick<RpcClient, 'sendRequest'>,
worktreeId: string,
@ -49,6 +76,33 @@ export function linkMobilePr(
return setLinkedPr(client, worktreeId, prNumber)
}
export async function linkMobileHostedReview(
client: Pick<RpcClient, 'sendRequest'>,
worktreeId: string,
provider: HostedReviewProvider,
number: number,
options?: { baseRef?: string | null }
): Promise<MobilePrLinkOutcome> {
const params = buildWorktreeSetHostedReviewLinkParams(worktreeId, provider, number, options)
if (Object.keys(params).length === 1) {
return { ok: true }
}
try {
const response = await client.sendRequest('worktree.set', params)
if (!response.ok) {
return { ok: false, error: response.error?.message || 'Failed to update linked review' }
}
return { ok: true }
} catch (err) {
// Why: the review was already created; normalize link failures so callers can
// surface a non-fatal refresh problem instead of losing the created URL.
return {
ok: false,
error: err instanceof Error ? err.message : 'Failed to update linked review'
}
}
}
export function unlinkMobilePr(
client: Pick<RpcClient, 'sendRequest'>,
worktreeId: string

View File

@ -53,6 +53,7 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara
const [showBranchPicker, setShowBranchPicker] = useState(false)
const [localBranches, setLocalBranches] = useState<MobileGitLocalBranches | null>(null)
const [createdPrUrl, setCreatedPrUrl] = useState<string | null>(null)
const [createdPrWarning, setCreatedPrWarning] = useState<string | null>(null)
const [prPrefill, setPrPrefill] = useState<MobilePrPrefill | null>(null)
const [discardTarget, setDiscardTarget] = useState<MobileGitStatusEntry | null>(null)
const [showActionSheet, setShowActionSheet] = useState(false)
@ -224,6 +225,8 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara
localBranches,
createdPrUrl,
setCreatedPrUrl,
createdPrWarning,
setCreatedPrWarning,
prPrefill,
discardTarget,
setDiscardTarget,