From 9d3f620b355f1d9e6f18d9e50699a383d1494bf0 Mon Sep 17 00:00:00 2001 From: hellostatpods Date: Sat, 1 Aug 2026 18:15:36 +0800 Subject: [PATCH] fix(desktop): route external PR merges to provider Co-authored-by: hellostatpods Signed-off-by: hellostatpods --- .../projects/lib/projectCloneUrl.test.mjs | 61 +++++++++++++++++- .../features/projects/lib/projectCloneUrl.ts | 47 ++++++++++++++ .../projects/ui/PullRequestReviewCard.tsx | 64 ++++++++++++++++++- desktop/src/testing/e2eBridge.ts | 21 ++++-- desktop/tests/e2e/project-pr-review.spec.ts | 35 ++++++++++ 5 files changed, 221 insertions(+), 7 deletions(-) diff --git a/desktop/src/features/projects/lib/projectCloneUrl.test.mjs b/desktop/src/features/projects/lib/projectCloneUrl.test.mjs index 6179c46a04..d3d232c54d 100644 --- a/desktop/src/features/projects/lib/projectCloneUrl.test.mjs +++ b/desktop/src/features/projects/lib/projectCloneUrl.test.mjs @@ -1,7 +1,11 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { deriveRelayCloneUrl, effectiveCloneUrls } from "./projectCloneUrl.ts"; +import { + deriveRelayCloneUrl, + effectiveCloneUrls, + isActiveRelayCloneUrl, +} from "./projectCloneUrl.ts"; const OWNER = "a".repeat(64); const ORIGIN = "https://relay.example"; @@ -60,3 +64,58 @@ test("effectiveCloneUrls derives a default when none is advertised", () => { test("effectiveCloneUrls returns empty when no default can be derived", () => { assert.deepEqual(effectiveCloneUrls([], null, OWNER, "repo"), []); }); + +test("isActiveRelayCloneUrl accepts the active relay repository shape", () => { + assert.equal( + isActiveRelayCloneUrl(`${ORIGIN}/git/${OWNER}/repo`, ORIGIN, OWNER), + true, + ); + assert.equal( + isActiveRelayCloneUrl( + `${ORIGIN}/prefix/git/${OWNER}/repo`, + `${ORIGIN}/prefix`, + OWNER, + ), + true, + ); + assert.equal( + isActiveRelayCloneUrl(`${ORIGIN}/git/${"b".repeat(64)}/fork`, ORIGIN), + true, + ); +}); + +test("isActiveRelayCloneUrl rejects external and mismatched repositories", () => { + assert.equal( + isActiveRelayCloneUrl( + "https://github.com/octocat/hello.git", + ORIGIN, + OWNER, + ), + false, + ); + assert.equal( + isActiveRelayCloneUrl( + `https://other-relay.example/git/${OWNER}/repo`, + ORIGIN, + OWNER, + ), + false, + ); + assert.equal( + isActiveRelayCloneUrl( + `${ORIGIN}/git/${"b".repeat(64)}/repo`, + ORIGIN, + OWNER, + ), + false, + ); + assert.equal( + isActiveRelayCloneUrl(`${ORIGIN}/git/${OWNER}/repo/extra`, ORIGIN, OWNER), + false, + ); + assert.equal(isActiveRelayCloneUrl("not a URL", ORIGIN, OWNER), false); + assert.equal( + isActiveRelayCloneUrl(`${ORIGIN}/git/${OWNER}/repo`, null, OWNER), + false, + ); +}); diff --git a/desktop/src/features/projects/lib/projectCloneUrl.ts b/desktop/src/features/projects/lib/projectCloneUrl.ts index 78dfb83762..6c31e24ab8 100644 --- a/desktop/src/features/projects/lib/projectCloneUrl.ts +++ b/desktop/src/features/projects/lib/projectCloneUrl.ts @@ -50,3 +50,50 @@ export function effectiveCloneUrls( const derived = deriveRelayCloneUrl(relayOrigin, owner, dtag); return derived ? [derived] : []; } + +/** + * Whether Desktop's native git workflow may operate on this clone URL. + * + * Native project mutations intentionally accept only repositories hosted by + * the active workspace relay. Keep the UI capability check aligned with that + * boundary so external NIP-34 clone URLs never offer an action the backend + * must reject. + */ +export function isActiveRelayCloneUrl( + cloneUrl: string | null | undefined, + relayOrigin: string | null | undefined, + expectedOwner?: string | null, +): boolean { + if ( + !cloneUrl || + !relayOrigin || + (expectedOwner !== undefined && + expectedOwner !== null && + !/^[0-9a-fA-F]{64}$/.test(expectedOwner)) + ) { + return false; + } + + try { + const clone = new URL(cloneUrl); + const relay = new URL(relayOrigin); + if (clone.origin !== relay.origin) return false; + + const relayPath = relay.pathname.replace(/\/+$/, ""); + if (relayPath && !clone.pathname.startsWith(`${relayPath}/`)) return false; + + const segments = clone.pathname.split("/").filter(Boolean); + const gitIndex = segments.lastIndexOf("git"); + const cloneOwner = segments[gitIndex + 1]; + return ( + gitIndex >= 0 && + segments.length === gitIndex + 3 && + /^[0-9a-fA-F]{64}$/.test(cloneOwner ?? "") && + (!expectedOwner || + cloneOwner?.toLowerCase() === expectedOwner.toLowerCase()) && + Boolean(segments[gitIndex + 2]) + ); + } catch { + return false; + } +} diff --git a/desktop/src/features/projects/ui/PullRequestReviewCard.tsx b/desktop/src/features/projects/ui/PullRequestReviewCard.tsx index c903769b7d..d00e992741 100644 --- a/desktop/src/features/projects/ui/PullRequestReviewCard.tsx +++ b/desktop/src/features/projects/ui/PullRequestReviewCard.tsx @@ -1,5 +1,6 @@ import { Check, + ExternalLink, GitPullRequest, GitPullRequestDraft, MoreHorizontal, @@ -10,6 +11,7 @@ import * as React from "react"; import { toast } from "sonner"; import { useIsManagedAgent } from "@/features/agent-memory/hooks"; +import { useCommunities } from "@/features/communities/useCommunities"; import type { Project, ProjectPullRequest } from "@/features/projects/hooks"; import { nextProjectPullRequestReviewCreatedAt } from "@/features/projects/projectPullRequests.mjs"; import { @@ -18,7 +20,10 @@ import { useUpdateProjectPullRequestStatusMutation, } from "@/features/projects/pullRequestReviews"; import { useIdentityQuery } from "@/shared/api/hooks"; +import { relayHttpFromWs } from "@/shared/api/inviteHelpers"; +import { isActiveRelayCloneUrl } from "@/features/projects/lib/projectCloneUrl"; import { normalizePubkey } from "@/shared/lib/pubkey"; +import { isSafeUrl } from "@/shared/lib/url"; import { Button } from "@/shared/ui/button"; import { Dialog, @@ -59,6 +64,15 @@ export function PullRequestReviewCard({ const [approvalSummary, setApprovalSummary] = React.useState(""); const reviewDecisionInFlightRef = React.useRef(false); const lastReviewDecisionCreatedAtRef = React.useRef(0); + const { activeCommunity } = useCommunities(); + const relayOrigin = React.useMemo(() => { + if (!activeCommunity?.relayUrl) return null; + try { + return relayHttpFromWs(activeCommunity.relayUrl); + } catch { + return null; + } + }, [activeCommunity?.relayUrl]); const viewerPubkey = identityQuery.data?.pubkey ?? null; const viewer = viewerPubkey ? normalizePubkey(viewerPubkey) : null; @@ -75,10 +89,39 @@ export function PullRequestReviewCard({ ); const canReview = canReviewProjectPullRequest(project, pullRequest, viewer); const canApprove = canReview && !hasApproved; - const canMerge = + const canActOnOpenPullRequest = (isOwner || isManagedAgentOwner) && pullRequest.status === "Open" && Boolean(pullRequest.branchName && pullRequest.commit); + const nativeTargetMergeSupported = isActiveRelayCloneUrl( + project.cloneUrls[0], + relayOrigin, + project.owner, + ); + const nativeSourceMergeSupported = isActiveRelayCloneUrl( + pullRequest.cloneUrls[0] ?? project.cloneUrls[0], + relayOrigin, + ); + const nativeMergeSupported = + nativeTargetMergeSupported && nativeSourceMergeSupported; + const canMerge = canActOnOpenPullRequest && nativeMergeSupported; + const externalRepositoryUrl = + canActOnOpenPullRequest && + relayOrigin && + !nativeMergeSupported && + isSafeUrl(project.webUrl ?? undefined) + ? project.webUrl + : null; + const externalRepositoryLabel = (() => { + if (!externalRepositoryUrl) return "Open repository"; + try { + return new URL(externalRepositoryUrl).hostname === "github.com" + ? "Open on GitHub" + : "Open repository"; + } catch { + return "Open repository"; + } + })(); const handleStatusChange = React.useCallback( async (status: "open" | "draft" | "closed") => { @@ -168,6 +211,7 @@ export function PullRequestReviewCard({ const hasAvailableAction = canApprove || canMerge || + Boolean(externalRepositoryUrl) || canMarkReady || canConvertToDraft || canClose || @@ -207,6 +251,24 @@ export function PullRequestReviewCard({ pullRequest={pullRequest} /> ) : null} + {externalRepositoryUrl ? ( + + ) : null} {canMarkReady ? (