diff --git a/apps/mobile/src/components/agents/chat-markdown-text.mounted.test.tsx b/apps/mobile/src/components/agents/chat-markdown-text.mounted.test.tsx new file mode 100644 index 00000000000..f01a21342fd --- /dev/null +++ b/apps/mobile/src/components/agents/chat-markdown-text.mounted.test.tsx @@ -0,0 +1,145 @@ +/* eslint-disable max-lines -- one mounted harness covers the tap and long-press review-link states for all three providers. */ +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ChatMarkdownText } from './chat-markdown-text'; + +const mocks = vi.hoisted(() => ({ + showActionSheetWithOptions: vi.fn(), + push: vi.fn(), + openExternalUrl: vi.fn(), + markdownProps: [] as Record[], +})); + +vi.mock('react-native', () => ({ Share: { share: vi.fn() } })); +vi.mock('expo-clipboard', () => ({ setStringAsync: vi.fn() })); +vi.mock('sonner-native', () => ({ toast: { error: vi.fn(), success: vi.fn() } })); +vi.mock('@expo/react-native-action-sheet', () => ({ + useActionSheet: () => ({ showActionSheetWithOptions: mocks.showActionSheetWithOptions }), +})); +vi.mock('react-native-safe-area-context', () => ({ useSafeAreaInsets: () => ({ bottom: 0 }) })); +vi.mock('expo-router', () => ({ useRouter: () => ({ push: mocks.push }) })); +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })); +vi.mock('@/i18n', () => ({ i18n: { language: 'en', t: (key: string) => key } })); +vi.mock('@/lib/analytics/posthog', () => ({ + FEATURE_FLAG_PR_REVIEW: 'pr-review', + useFeatureFlag: () => true, +})); +vi.mock('@/lib/external-link', () => ({ openExternalUrl: mocks.openExternalUrl })); +vi.mock('@/components/agents/use-message-copy', () => ({ performCopy: vi.fn() })); +vi.mock('./markdown-link-confirm', () => ({ formatLinkHost: (href: string) => href })); +vi.mock('./markdown-text', () => ({ + MarkdownText: (props: Record) => { + mocks.markdownProps.push(props); + return null; + }, +})); + +function mountChatMarkdownText(): { + onPressLink: (href: string) => boolean; + onLongPressLink: (href: string) => void; +} { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create(createElement(ChatMarkdownText, { value: 'text' })); + }); + const props = mocks.markdownProps.at(-1); + if (!props) { + throw new Error('ChatMarkdownText did not render its markdown surface'); + } + return { + onPressLink: props.onPressLink as (href: string) => boolean, + onLongPressLink: props.onLongPressLink as (href: string) => void, + }; +} + +/** The options array handed to the native action sheet on the last call. */ +function lastSheetOptions(): string[] { + const call = mocks.showActionSheetWithOptions.mock.calls.at(-1); + const sheet = call?.[0] as { options?: string[] } | undefined; + return sheet?.options ?? []; +} + +/** Invoke the sheet's pressed index through the handler the sheet received. */ +function pressSheetOption(index: number): void { + const call = mocks.showActionSheetWithOptions.mock.calls.at(-1); + const onSelect = call?.[1] as ((index: number) => void) | undefined; + onSelect?.(index); +} + +describe('ChatMarkdownText review-link recognition', () => { + beforeEach(() => { + mocks.showActionSheetWithOptions.mockReset(); + mocks.push.mockReset(); + mocks.openExternalUrl.mockReset(); + mocks.markdownProps = []; + }); + + it('offers Review PR for a github.com pull request and routes in-app', () => { + const { onPressLink } = mountChatMarkdownText(); + + const handled = onPressLink('https://github.com/octocat/hello-world/pull/42'); + + expect(handled).toBe(true); + expect(lastSheetOptions()).toContain('common.reviewPr'); + pressSheetOption(0); + expect(mocks.push).toHaveBeenCalledWith('/(app)/pr-review/octocat/hello-world/42'); + }); + + it('offers Review PR for a gitlab.com merge request and routes to the provider route', () => { + const { onPressLink } = mountChatMarkdownText(); + + const handled = onPressLink('https://gitlab.com/group/sub/repo/-/merge_requests/7'); + + expect(handled).toBe(true); + expect(lastSheetOptions()).toContain('common.reviewPr'); + pressSheetOption(0); + expect(mocks.push).toHaveBeenCalledWith( + '/(app)/pr-review/gitlab/group/sub/repo/7?instance=https%3A%2F%2Fgitlab.com' + ); + }); + + it('offers Review PR for a self-managed GitLab merge request', () => { + const { onPressLink } = mountChatMarkdownText(); + + const handled = onPressLink('https://gitlab.example.com/team/repo/-/merge_requests/9'); + + expect(handled).toBe(true); + pressSheetOption(0); + expect(mocks.push).toHaveBeenCalledWith( + '/(app)/pr-review/gitlab/team/repo/9?instance=https%3A%2F%2Fgitlab.example.com' + ); + }); + + it('offers Review PR for a Bitbucket pull request and routes to the provider route', () => { + const { onPressLink } = mountChatMarkdownText(); + + const handled = onPressLink('https://bitbucket.org/acme/api/pull-requests/42/overview'); + + expect(handled).toBe(true); + pressSheetOption(0); + expect(mocks.push).toHaveBeenCalledWith('/(app)/pr-review/bitbucket/acme/api/42'); + }); + + it('leaves a plain link to the default open-in-browser path', () => { + const { onPressLink } = mountChatMarkdownText(); + + expect(onPressLink('https://example.com/docs')).toBe(false); + expect(mocks.showActionSheetWithOptions).not.toHaveBeenCalled(); + }); + + it('adds the long-press Review PR option for GitLab and Bitbucket, not for plain links', () => { + const { onLongPressLink } = mountChatMarkdownText(); + + onLongPressLink('https://gitlab.com/group/repo/-/merge_requests/7'); + expect(lastSheetOptions()).toContain('common.reviewPr'); + pressSheetOption(0); + expect(mocks.push).toHaveBeenCalledWith( + '/(app)/pr-review/gitlab/group/repo/7?instance=https%3A%2F%2Fgitlab.com' + ); + + onLongPressLink('https://example.com/docs'); + expect(lastSheetOptions()).not.toContain('common.reviewPr'); + }); +}); diff --git a/apps/mobile/src/components/agents/chat-markdown-text.tsx b/apps/mobile/src/components/agents/chat-markdown-text.tsx index 7037c7649ba..751b1ef8ab3 100644 --- a/apps/mobile/src/components/agents/chat-markdown-text.tsx +++ b/apps/mobile/src/components/agents/chat-markdown-text.tsx @@ -7,7 +7,8 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { FEATURE_FLAG_PR_REVIEW, useFeatureFlag } from '@/lib/analytics/posthog'; import { openExternalUrl } from '@/lib/external-link'; -import { parseGitHubPrUrl } from '@/lib/github-pr-url'; +import { providerPrRoutePath } from '@/lib/pr-review/provider-pr-ref'; +import { parseProviderPrUrl } from '@/lib/pr-review/provider-pr-url'; import { buildChatLinkActionSheet, @@ -31,18 +32,8 @@ function sheetMessage(href: string): string { } function buildPrReviewHref(href: string): Href | null { - const parsed = parseGitHubPrUrl(href); - if (!parsed) { - return null; - } - return { - pathname: '/(app)/pr-review/[owner]/[repo]/[number]', - params: { - owner: parsed.owner, - repo: parsed.repo, - number: String(parsed.number), - }, - }; + const ref = parseProviderPrUrl(href); + return ref ? providerPrRoutePath(ref) : null; } export function ChatMarkdownText(props: Readonly) { @@ -56,7 +47,7 @@ export function ChatMarkdownText(props: Readonly) { (href: string) => { // When PR Review is off, PR links behave like any other link (default // open-in-browser) instead of showing the Review-PR tap sheet. - if (!prReviewEnabled || !parseGitHubPrUrl(href)) { + if (!prReviewEnabled || !parseProviderPrUrl(href)) { return false; } // Tap on a PR link shows exactly four options: Review PR / Open in @@ -96,7 +87,7 @@ export function ChatMarkdownText(props: Readonly) { const handleLongPressLink = useCallback( (href: string, event?: GestureResponderEvent) => { event?.stopPropagation(); - const isPrLink = prReviewEnabled && parseGitHubPrUrl(href) !== null; + const isPrLink = prReviewEnabled && parseProviderPrUrl(href) !== null; const sheet = buildChatLinkActionSheet({ isPrLink }); showActionSheetWithOptions( { diff --git a/apps/mobile/src/components/agents/session-pr-badge.tsx b/apps/mobile/src/components/agents/session-pr-badge.tsx index b557d3004f8..4629f8344c7 100644 --- a/apps/mobile/src/components/agents/session-pr-badge.tsx +++ b/apps/mobile/src/components/agents/session-pr-badge.tsx @@ -60,7 +60,9 @@ export type SessionPrBadgeProps = Readonly<{ * Shows `#N` with an icon/accent derived from the PR state and review * decision. While `loading` is true it reserves a fixed-width skeleton; after * the fetch it renders the badge or nothing (never a guess from `git_branch`). - * Tapping navigates in-app for GitHub PRs and opens the browser otherwise. + * Tapping routes through the one provider URL resolver: a GitHub pull request, + * a GitLab merge request (any host) and a Bitbucket pull request all open the + * in-app review route, and anything else opens the browser. */ export function SessionPrBadge(props: SessionPrBadgeProps) { const { loading } = props; diff --git a/apps/mobile/src/components/code-reviewer/review-detail-screen.mounted.test.tsx b/apps/mobile/src/components/code-reviewer/review-detail-screen.mounted.test.tsx index e12c32767f4..29d2855d797 100644 --- a/apps/mobile/src/components/code-reviewer/review-detail-screen.mounted.test.tsx +++ b/apps/mobile/src/components/code-reviewer/review-detail-screen.mounted.test.tsx @@ -145,7 +145,7 @@ vi.mock('@/lib/analytics/posthog', () => ({ useFeatureFlag: () => true, })); vi.mock('@/lib/code-reviewer-open-pr-destination', () => ({ - resolveCodeReviewerOpenPrDestination: () => ({ kind: 'external' }), + resolveCodeReviewerOpenPrDestination: () => ({ kind: 'browser' }), })); vi.mock('@/lib/code-reviewer-config', () => ({ reviewerPlatformLabel: () => 'GitHub', @@ -156,7 +156,6 @@ vi.mock('@/lib/hooks/use-code-reviews', () => ({ useCancelReview: () => ({ isPending: false, mutate: vi.fn() }), useRetriggerReview: () => ({ isPending: false, mutate: vi.fn() }), })); -vi.mock('@/lib/profile-agent-navigation', () => ({ getPrReviewPath: vi.fn() })); vi.mock('@/lib/utils', () => ({ cn: (...args: unknown[]) => args.filter(Boolean).join(' '), parseTimestamp: (value: unknown) => value, diff --git a/apps/mobile/src/components/code-reviewer/review-detail-screen.tsx b/apps/mobile/src/components/code-reviewer/review-detail-screen.tsx index 23f4e3eee52..fe595750f06 100644 --- a/apps/mobile/src/components/code-reviewer/review-detail-screen.tsx +++ b/apps/mobile/src/components/code-reviewer/review-detail-screen.tsx @@ -34,7 +34,6 @@ import { reviewerPlatformLabel } from '@/lib/code-reviewer-config'; import { openExternalUrl } from '@/lib/external-link'; import { formatMoney, formatNumber } from '@/lib/format'; import { useCancelReview, useRetriggerReview, useReviewDetail } from '@/lib/hooks/use-code-reviews'; -import { getPrReviewPath } from '@/lib/profile-agent-navigation'; import { cn, parseTimestamp, timeAgo } from '@/lib/utils'; const FINDINGS_PAGE_SIZE = 20; @@ -268,9 +267,7 @@ export function ReviewDetailScreen({ prReviewEnabled ); if (destination.kind === 'in-app') { - router.push( - getPrReviewPath(destination.owner, destination.repo, destination.number) - ); + router.push(destination.href); return; } void openExternalUrl(review.pr_url, { diff --git a/apps/mobile/src/components/security-agent/finding-remediation-panel.mounted.test.tsx b/apps/mobile/src/components/security-agent/finding-remediation-panel.mounted.test.tsx index 92fd2c05371..1ec607d1b5e 100644 --- a/apps/mobile/src/components/security-agent/finding-remediation-panel.mounted.test.tsx +++ b/apps/mobile/src/components/security-agent/finding-remediation-panel.mounted.test.tsx @@ -274,16 +274,26 @@ describe('FindingRemediationPanel pull request navigation', () => { }); it.each([ - ['https://github.com/kilo/kilo/pull/123', true, true], - ['https://github.com/kilo/kilo/pull/123', false, false], - ['https://gitlab.com/kilo/kilo/-/merge_requests/123', true, false], - ] as const)('opens %s with PR review=%s in-app=%s', (prUrl, enabled, inApp) => { + ['https://github.com/kilo/kilo/pull/123', true, '/(app)/pr-review/kilo/kilo/123'], + ['https://github.com/kilo/kilo/pull/123', false, null], + [ + 'https://gitlab.com/kilo/kilo/-/merge_requests/123', + true, + '/(app)/pr-review/gitlab/kilo/kilo/123?instance=https%3A%2F%2Fgitlab.com', + ], + [ + 'https://bitbucket.org/kilo/kilo/pull-requests/123', + true, + '/(app)/pr-review/bitbucket/kilo/kilo/123', + ], + ['https://github.example.com/kilo/kilo/pull/123', true, null], + ] as const)('opens %s with PR review=%s', (prUrl, enabled, expectedHref) => { mocks.prReviewEnabled = enabled; pressButtons( renderPanel(analysisFixture({ remediationSummary: { status: 'pr_opened', prUrl } })) ); - if (inApp) { - expect(mocks.routerPush).toHaveBeenCalledWith('/(app)/pr-review/kilo/kilo/123'); + if (expectedHref) { + expect(mocks.routerPush).toHaveBeenCalledWith(expectedHref); expect(mocks.openExternalUrl).not.toHaveBeenCalled(); } else { expect(mocks.routerPush).not.toHaveBeenCalled(); diff --git a/apps/mobile/src/components/security-agent/finding-remediation-panel.tsx b/apps/mobile/src/components/security-agent/finding-remediation-panel.tsx index f3b83fe8deb..8231dcd09d8 100644 --- a/apps/mobile/src/components/security-agent/finding-remediation-panel.tsx +++ b/apps/mobile/src/components/security-agent/finding-remediation-panel.tsx @@ -28,7 +28,6 @@ import { useStartSecurityRemediation, } from '@/lib/hooks/use-security-remediation'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; -import { getPrReviewPath } from '@/lib/profile-agent-navigation'; import { type SecurityAnalysis } from '@/lib/security-agent'; import { firstNonEmpty, parseTimestamp, timeAgo } from '@/lib/utils'; @@ -221,7 +220,7 @@ export function FindingRemediationPanel({ const openPullRequest = (url: string) => { const destination = resolveCodeReviewerOpenPrDestination(url, prReviewEnabled); if (destination.kind === 'in-app') { - router.push(getPrReviewPath(destination.owner, destination.repo, destination.number)); + router.push(destination.href); return; } void openExternalUrl(url, { label: t('common.pullRequest') }); diff --git a/apps/mobile/src/components/share/share-gate-sheet.mounted.test.tsx b/apps/mobile/src/components/share/share-gate-sheet.mounted.test.tsx index d982899d192..6bedf1d7003 100644 --- a/apps/mobile/src/components/share/share-gate-sheet.mounted.test.tsx +++ b/apps/mobile/src/components/share/share-gate-sheet.mounted.test.tsx @@ -405,4 +405,24 @@ describe('ShareGateSheet spawn operationKey wiring', () => { renderer.unmount(); }); }); + + it('numbers a staged GitLab merge request with the provider separator', async () => { + const shareId = putSharePayload({ + text: 'https://gitlab.com/group/sub/repo/-/merge_requests/7', + files: [], + failedFiles: [], + }); + const renderer = await mountGate(shareId); + + // The Review PR row is the row whose subtitle is the destination identity. + const subtitles = renderer.root + .findAllByType('Text') + .filter(node => node.props.className === 'text-sm text-muted-foreground') + .map(node => node.children.filter(child => typeof child === 'string').join('')); + expect(subtitles).toEqual(['group/sub/repo!7']); + + act(() => { + renderer.unmount(); + }); + }); }); diff --git a/apps/mobile/src/components/share/share-gate-sheet.tsx b/apps/mobile/src/components/share/share-gate-sheet.tsx index 4b9deb6000b..68f5667a418 100644 --- a/apps/mobile/src/components/share/share-gate-sheet.tsx +++ b/apps/mobile/src/components/share/share-gate-sheet.tsx @@ -23,7 +23,7 @@ import { useRemoteInstanceSpawn } from '@/lib/hooks/use-remote-instance-spawn'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { useOrganization } from '@/lib/organization-context'; import { useHoistedOperationKey } from '@/lib/operation-key'; -import { getPrReviewPath } from '@/lib/profile-agent-navigation'; +import { providerPrRoutePath } from '@/lib/pr-review/provider-pr-ref'; import { resolveRemoteSubmitOutcome } from '@/lib/remote-submit-outcome'; import { appendShareParams, setPendingShareNavigation } from '@/lib/share-navigation'; import { @@ -50,7 +50,7 @@ import { ShareDestinationList } from './share-destination-list'; import { isShareCommitEnabled, selectShareGateState } from './share-gate-state'; import { SharePayloadPreview } from './share-payload-preview'; import { type SharePayloadValidation, validateSharePayload } from './share-payload-validation'; -import { selectShareReviewPr } from './share-review-pr'; +import { selectShareReviewPr, selectShareReviewPrSubtitle } from './share-review-pr'; type ShareGateSheetProps = { shareId: string | undefined; @@ -218,7 +218,7 @@ export function ShareGateSheet({ shareId }: Readonly) { } void Haptics.selectionAsync(); setPendingShareNavigation({ - href: getPrReviewPath(reviewPr.owner, reviewPr.repo, reviewPr.number) as string, + href: providerPrRoutePath(reviewPr) as string, shareId: null, }); dismiss(); @@ -409,11 +409,7 @@ export function ShareGateSheet({ shareId }: Readonly) { diff --git a/apps/mobile/src/components/share/share-review-pr.test.ts b/apps/mobile/src/components/share/share-review-pr.test.ts index 3f7a79d81c0..39d8860d486 100644 --- a/apps/mobile/src/components/share/share-review-pr.test.ts +++ b/apps/mobile/src/components/share/share-review-pr.test.ts @@ -1,20 +1,57 @@ +import '@/i18n'; + import { describe, expect, it } from 'vitest'; -import { selectShareReviewPr } from './share-review-pr'; +import { selectShareReviewPr, selectShareReviewPrSubtitle } from './share-review-pr'; const PR_URL = 'https://github.com/octocat/hello-world/pull/42'; +const GITLAB_MR_URL = 'https://gitlab.com/group/sub/repo/-/merge_requests/7'; +const BITBUCKET_PR_URL = 'https://bitbucket.org/acme/api/pull-requests/42/overview'; describe('selectShareReviewPr', () => { it('returns the parsed destination when flag, new-session and PR text all hold', () => { expect( selectShareReviewPr({ text: PR_URL, prReviewEnabled: true, showNewSession: true }) ).toEqual({ + platform: 'github', owner: 'octocat', repo: 'hello-world', number: 42, }); }); + it('recognises a GitLab merge request', () => { + expect( + selectShareReviewPr({ text: GITLAB_MR_URL, prReviewEnabled: true, showNewSession: true }) + ).toEqual({ + platform: 'gitlab', + projectPath: 'group/sub/repo', + mrIid: 7, + instanceHint: 'https://gitlab.com', + }); + }); + + it('recognises a self-managed GitLab merge request', () => { + expect( + selectShareReviewPr({ + text: 'https://gitlab.example.com/team/repo/-/merge_requests/9', + prReviewEnabled: true, + showNewSession: true, + }) + ).toEqual({ + platform: 'gitlab', + projectPath: 'team/repo', + mrIid: 9, + instanceHint: 'https://gitlab.example.com', + }); + }); + + it('recognises a Bitbucket pull request', () => { + expect( + selectShareReviewPr({ text: BITBUCKET_PR_URL, prReviewEnabled: true, showNewSession: true }) + ).toEqual({ platform: 'bitbucket', workspace: 'acme', repoSlug: 'api', prId: 42 }); + }); + it('returns null when the flag is off', () => { expect( selectShareReviewPr({ text: PR_URL, prReviewEnabled: false, showNewSession: true }) @@ -29,6 +66,13 @@ describe('selectShareReviewPr', () => { showNewSession: true, }) ).toBeNull(); + expect( + selectShareReviewPr({ + text: 'https://example.com/group/repo', + prReviewEnabled: true, + showNewSession: true, + }) + ).toBeNull(); }); it('returns null when showNewSession is false even for a PR URL', () => { @@ -45,6 +89,7 @@ describe('selectShareReviewPr', () => { showNewSession: true, }) ).toEqual({ + platform: 'github', owner: 'octocat', repo: 'hello-world', number: 42, @@ -59,9 +104,49 @@ describe('selectShareReviewPr', () => { showNewSession: true, }) ).toEqual({ + platform: 'github', owner: 'octocat', repo: 'hello-world', number: 42, }); }); }); + +describe('selectShareReviewPrSubtitle', () => { + it('numbers a GitLab merge request with the provider separator', () => { + expect( + selectShareReviewPrSubtitle({ platform: 'gitlab', projectPath: 'group/sub/repo', mrIid: 7 }) + ).toBe('group/sub/repo!7'); + }); + + it('numbers a pasted GitLab merge request with the provider separator', () => { + const ref = selectShareReviewPr({ + text: GITLAB_MR_URL, + prReviewEnabled: true, + showNewSession: true, + }); + if (ref === null) { + throw new Error('expected the GitLab MR URL to parse'); + } + expect(selectShareReviewPrSubtitle(ref)).toBe('group/sub/repo!7'); + }); + + it('keeps the GitHub and Bitbucket identity line', () => { + expect( + selectShareReviewPrSubtitle({ + platform: 'github', + owner: 'octocat', + repo: 'hello-world', + number: 42, + }) + ).toBe('octocat/hello-world #42'); + expect( + selectShareReviewPrSubtitle({ + platform: 'bitbucket', + workspace: 'acme', + repoSlug: 'api', + prId: 42, + }) + ).toBe('acme/api #42'); + }); +}); diff --git a/apps/mobile/src/components/share/share-review-pr.ts b/apps/mobile/src/components/share/share-review-pr.ts index cd261c83d6f..d3ccd1cc165 100644 --- a/apps/mobile/src/components/share/share-review-pr.ts +++ b/apps/mobile/src/components/share/share-review-pr.ts @@ -1,18 +1,39 @@ -import { findFirstGitHubPrUrl, type GitHubPrUrl } from '@/lib/github-pr-url'; +import { type ProviderPrRef } from '@kilocode/app-shared/provider-review'; + +import { i18n } from '@/i18n'; +import { providerPrRefLabel, providerPrTriple } from '@/lib/pr-review/provider-pr-ref'; +import { findFirstProviderPrUrl } from '@/lib/pr-review/provider-pr-url'; /** * Decide whether the share gate shows the Review PR destination. The option is * visible only when the feature flag is on, the gate would otherwise offer a * New session (not a stale-share / all-rejected terminal state), and the staged - * text contains a parseable github.com PR URL. + * text contains a parseable GitHub, GitLab or Bitbucket review URL. Parsing + * routes through the one provider URL resolver, so a GitLab merge request and + * a Bitbucket pull request are recognised exactly like a GitHub PR. */ export function selectShareReviewPr(input: { text: string; prReviewEnabled: boolean; showNewSession: boolean; -}): GitHubPrUrl | null { +}): ProviderPrRef | null { if (!input.prReviewEnabled || !input.showNewSession) { return null; } - return findFirstGitHubPrUrl(input.text); + return findFirstProviderPrUrl(input.text); +} + +/** + * The identity line under the Review PR row, written in the provider's own + * reference syntax: GitLab numbers merge requests `group/sub/repo!12`, the + * others write `owner/repo #12`. `share.reviewPrSubtitle` is format-only — the + * same placeholder string in every locale — so the GitLab arm composes the + * line from the shared provider label instead of adding a key all 86 catalogs + * would have to carry. + */ +export function selectShareReviewPrSubtitle(ref: ProviderPrRef): string { + if (ref.platform === 'gitlab') { + return providerPrRefLabel(ref); + } + return i18n.t('share.reviewPrSubtitle', providerPrTriple(ref)); } diff --git a/apps/mobile/src/lib/code-reviewer-open-pr-destination.test.ts b/apps/mobile/src/lib/code-reviewer-open-pr-destination.test.ts index 4984f79dd8d..d75f781f71d 100644 --- a/apps/mobile/src/lib/code-reviewer-open-pr-destination.test.ts +++ b/apps/mobile/src/lib/code-reviewer-open-pr-destination.test.ts @@ -8,25 +8,44 @@ describe('resolveCodeReviewerOpenPrDestination', () => { resolveCodeReviewerOpenPrDestination('https://github.com/octocat/hello-world/pull/42', true) ).toEqual({ kind: 'in-app', - owner: 'octocat', - repo: 'hello-world', - number: 42, + href: '/(app)/pr-review/octocat/hello-world/42', }); }); - it('opens the browser for a GitLab PR URL', () => { + it('routes a gitlab.com merge request in-app to the provider route with its instance', () => { expect( - resolveCodeReviewerOpenPrDestination('https://gitlab.com/octocat/hello-world/pull/42', true) - ).toEqual({ kind: 'browser' }); + resolveCodeReviewerOpenPrDestination( + 'https://gitlab.com/group/sub/repo/-/merge_requests/7', + true + ) + ).toEqual({ + kind: 'in-app', + href: '/(app)/pr-review/gitlab/group/sub/repo/7?instance=https%3A%2F%2Fgitlab.com', + }); }); - it('opens the browser for a Bitbucket PR URL', () => { + it('routes a self-managed GitLab merge request in-app', () => { expect( resolveCodeReviewerOpenPrDestination( - 'https://bitbucket.org/octocat/hello-world/pull-requests/42', + 'https://gitlab.example.com/team/repo/-/merge_requests/9', true ) - ).toEqual({ kind: 'browser' }); + ).toEqual({ + kind: 'in-app', + href: '/(app)/pr-review/gitlab/team/repo/9?instance=https%3A%2F%2Fgitlab.example.com', + }); + }); + + it('routes a Bitbucket pull request in-app', () => { + expect( + resolveCodeReviewerOpenPrDestination( + 'https://bitbucket.org/acme/api/pull-requests/42/overview', + true + ) + ).toEqual({ + kind: 'in-app', + href: '/(app)/pr-review/bitbucket/acme/api/42', + }); }); it('opens the browser for a GitHub Enterprise host', () => { diff --git a/apps/mobile/src/lib/code-reviewer-open-pr-destination.ts b/apps/mobile/src/lib/code-reviewer-open-pr-destination.ts index 891175d125e..e894c13717d 100644 --- a/apps/mobile/src/lib/code-reviewer-open-pr-destination.ts +++ b/apps/mobile/src/lib/code-reviewer-open-pr-destination.ts @@ -1,15 +1,19 @@ -import { parseGitHubPrUrl } from '@/lib/github-pr-url'; +import { type Href } from 'expo-router'; -type CodeReviewerOpenPrDestination = - | { kind: 'in-app'; owner: string; repo: string; number: number } - | { kind: 'browser' }; +import { providerPrRoutePath } from '@/lib/pr-review/provider-pr-ref'; +import { parseProviderPrUrl } from '@/lib/pr-review/provider-pr-url'; + +type CodeReviewerOpenPrDestination = { kind: 'in-app'; href: Href } | { kind: 'browser' }; /** * Decide whether "Open pull request" should navigate in-app or open the browser. * - * In-app only when the PR-review feature flag is on and `prUrl` is a parseable - * github.com PR URL. Everything else (flag off, Enterprise/GitLab/Bitbucket, - * malformed) keeps today's browser path. + * In-app only when the PR-review feature flag is on and `prUrl` names a review + * the provider routes serve: a github.com pull request, a GitLab merge request + * (gitlab.com or a self-managed host) or a Bitbucket pull request. The URL + * parses through the one provider resolver and the path comes from the one + * provider route builder, so all three providers land on the same screen tree. + * Anything else (flag off, GitHub Enterprise, malformed) keeps the browser path. */ export function resolveCodeReviewerOpenPrDestination( prUrl: string, @@ -18,9 +22,9 @@ export function resolveCodeReviewerOpenPrDestination( if (!prReviewEnabled) { return { kind: 'browser' }; } - const parsed = parseGitHubPrUrl(prUrl); - if (!parsed) { + const ref = parseProviderPrUrl(prUrl); + if (!ref) { return { kind: 'browser' }; } - return { kind: 'in-app', owner: parsed.owner, repo: parsed.repo, number: parsed.number }; + return { kind: 'in-app', href: providerPrRoutePath(ref) }; } diff --git a/apps/mobile/src/lib/github-pr-url.test.ts b/apps/mobile/src/lib/github-pr-url.test.ts index 077f91cfdad..c121f191392 100644 --- a/apps/mobile/src/lib/github-pr-url.test.ts +++ b/apps/mobile/src/lib/github-pr-url.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { findFirstGitHubPrUrl, parseGitHubPrUrl } from './github-pr-url'; +import { parseGitHubPrUrl } from './github-pr-url'; describe('parseGitHubPrUrl', () => { it('parses a canonical PR URL', () => { @@ -104,61 +104,3 @@ describe('parseGitHubPrUrl', () => { }); }); }); - -describe('findFirstGitHubPrUrl', () => { - it('parses a bare canonical PR URL', () => { - expect(findFirstGitHubPrUrl('https://github.com/octocat/hello-world/pull/42')).toEqual({ - owner: 'octocat', - repo: 'hello-world', - number: 42, - }); - }); - - it('parses a title plus URL', () => { - expect( - findFirstGitHubPrUrl('Fix the thing\nhttps://github.com/octocat/hello-world/pull/42') - ).toEqual({ - owner: 'octocat', - repo: 'hello-world', - number: 42, - }); - }); - - it('returns the first URL when two PR URLs are present', () => { - expect( - findFirstGitHubPrUrl( - 'https://github.com/octocat/hello-world/pull/42 https://github.com/octocat/hello-world/pull/7' - ) - ).toEqual({ - owner: 'octocat', - repo: 'hello-world', - number: 42, - }); - }); - - it('returns null for an issue URL', () => { - expect(findFirstGitHubPrUrl('https://github.com/octocat/hello-world/issues/42')).toBeNull(); - }); - - it('returns null for a GitLab URL', () => { - expect(findFirstGitHubPrUrl('https://gitlab.com/octocat/hello-world/pull/42')).toBeNull(); - }); - - it('returns null for plain text', () => { - expect(findFirstGitHubPrUrl('no url here at all')).toBeNull(); - }); - - it('returns null for an empty string', () => { - expect(findFirstGitHubPrUrl('')).toBeNull(); - }); - - it('parses a PR URL with extra words on one line', () => { - expect( - findFirstGitHubPrUrl('See https://github.com/octocat/hello-world/pull/42 please') - ).toEqual({ - owner: 'octocat', - repo: 'hello-world', - number: 42, - }); - }); -}); diff --git a/apps/mobile/src/lib/github-pr-url.ts b/apps/mobile/src/lib/github-pr-url.ts index bbc67fbdfaa..6a609c213ca 100644 --- a/apps/mobile/src/lib/github-pr-url.ts +++ b/apps/mobile/src/lib/github-pr-url.ts @@ -43,26 +43,3 @@ export function parseGitHubPrUrl(href: string): GitHubPrUrl | null { } return { owner, repo, number }; } - -/** - * Find the first GitHub PR URL in free text. Tries the whole trimmed string - * first, then each whitespace-separated token, so a title or surrounding words - * do not hide the URL. Returns `null` when no token parses as a PR URL. - */ -export function findFirstGitHubPrUrl(text: string): GitHubPrUrl | null { - if (text.length === 0) { - return null; - } - const trimmed = text.trim(); - const whole = parseGitHubPrUrl(trimmed); - if (whole) { - return whole; - } - for (const token of trimmed.split(/\s+/)) { - const parsed = parseGitHubPrUrl(token); - if (parsed) { - return parsed; - } - } - return null; -} diff --git a/apps/mobile/src/lib/pr-review/provider-pr-url.test.ts b/apps/mobile/src/lib/pr-review/provider-pr-url.test.ts index 78248b93065..e6d41e77574 100644 --- a/apps/mobile/src/lib/pr-review/provider-pr-url.test.ts +++ b/apps/mobile/src/lib/pr-review/provider-pr-url.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { parseProviderPrUrl } from './provider-pr-url'; +import { findFirstProviderPrUrl, parseProviderPrUrl } from './provider-pr-url'; describe('parseProviderPrUrl — GitHub', () => { it('parses a github.com pull request into the GitHub ref', () => { @@ -178,3 +178,54 @@ describe('parseProviderPrUrl — garbage input', () => { expect(parseProviderPrUrl('https://')).toBeNull(); }); }); + +describe('findFirstProviderPrUrl', () => { + it('finds a GitHub pull request in free text', () => { + expect( + findFirstProviderPrUrl('See https://github.com/octocat/hello-world/pull/42 please') + ).toEqual({ platform: 'github', owner: 'octocat', repo: 'hello-world', number: 42 }); + }); + + it('finds a gitlab.com merge request in free text', () => { + expect( + findFirstProviderPrUrl('Fix the thing\nhttps://gitlab.com/group/repo/-/merge_requests/7') + ).toEqual({ + platform: 'gitlab', + projectPath: 'group/repo', + mrIid: 7, + instanceHint: 'https://gitlab.com', + }); + }); + + it('finds a self-managed GitLab merge request in free text', () => { + expect( + findFirstProviderPrUrl('https://gitlab.example.com:8443/team/repo/-/merge_requests/9') + ).toEqual({ + platform: 'gitlab', + projectPath: 'team/repo', + mrIid: 9, + instanceHint: 'https://gitlab.example.com:8443', + }); + }); + + it('finds a Bitbucket pull request in free text', () => { + expect( + findFirstProviderPrUrl('https://bitbucket.org/acme/api/pull-requests/42/overview') + ).toEqual({ platform: 'bitbucket', workspace: 'acme', repoSlug: 'api', prId: 42 }); + }); + + it('returns the first review URL when several are present', () => { + expect( + findFirstProviderPrUrl( + 'https://github.com/octocat/hello-world/pull/42 https://gitlab.com/group/repo/-/merge_requests/7' + ) + ).toEqual({ platform: 'github', owner: 'octocat', repo: 'hello-world', number: 42 }); + }); + + it('returns null when nothing matches a provider', () => { + expect(findFirstProviderPrUrl('no url here at all')).toBeNull(); + expect(findFirstProviderPrUrl('https://example.com/group/repo')).toBeNull(); + expect(findFirstProviderPrUrl('https://github.com/octocat/hello-world/issues/42')).toBeNull(); + expect(findFirstProviderPrUrl('')).toBeNull(); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/provider-pr-url.ts b/apps/mobile/src/lib/pr-review/provider-pr-url.ts index 3c4db09b9b1..46d2ef0c317 100644 --- a/apps/mobile/src/lib/pr-review/provider-pr-url.ts +++ b/apps/mobile/src/lib/pr-review/provider-pr-url.ts @@ -185,3 +185,28 @@ export function parseProviderPrUrl(href: string): ProviderPrRef | null { } return parseGitLabMrUrl(url) ?? parseBitbucketPrUrl(url); } + +/** + * Find the first provider review URL in free text — a shared link, a title + * with the URL appended, or a message body. The whole trimmed string is tried + * first, then each whitespace-separated token, so surrounding words do not + * hide the URL. Returns `null` when nothing parses, so a URL that matches no + * provider keeps rendering as plain text. + */ +export function findFirstProviderPrUrl(text: string): ProviderPrRef | null { + if (text.length === 0) { + return null; + } + const trimmed = text.trim(); + const whole = parseProviderPrUrl(trimmed); + if (whole) { + return whole; + } + for (const token of trimmed.split(/\s+/)) { + const parsed = parseProviderPrUrl(token); + if (parsed) { + return parsed; + } + } + return null; +} diff --git a/apps/mobile/src/lib/profile-agent-navigation.ts b/apps/mobile/src/lib/profile-agent-navigation.ts index 115a75857ee..90504e0ca5b 100644 --- a/apps/mobile/src/lib/profile-agent-navigation.ts +++ b/apps/mobile/src/lib/profile-agent-navigation.ts @@ -26,7 +26,3 @@ export function getCodeReviewerProfilePath(scope: string): Href { export function getPrReviewEntryPath(): Href { return '/(app)/pr-review' as Href; } - -export function getPrReviewPath(owner: string, repo: string, number: number): Href { - return `/(app)/pr-review/${owner}/${repo}/${number}` as Href; -}