Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 60 additions & 1 deletion desktop/src/features/projects/lib/projectCloneUrl.test.mjs
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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,
);
});
47 changes: 47 additions & 0 deletions desktop/src/features/projects/lib/projectCloneUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
64 changes: 63 additions & 1 deletion desktop/src/features/projects/ui/PullRequestReviewCard.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
Check,
ExternalLink,
GitPullRequest,
GitPullRequestDraft,
MoreHorizontal,
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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") => {
Expand Down Expand Up @@ -168,6 +211,7 @@ export function PullRequestReviewCard({
const hasAvailableAction =
canApprove ||
canMerge ||
Boolean(externalRepositoryUrl) ||
canMarkReady ||
canConvertToDraft ||
canClose ||
Expand Down Expand Up @@ -207,6 +251,24 @@ export function PullRequestReviewCard({
pullRequest={pullRequest}
/>
) : null}
{externalRepositoryUrl ? (
<Button
asChild
className="h-8 gap-1.5 px-3.5"
size="xs"
type="button"
variant="secondary"
>
<a
href={externalRepositoryUrl}
rel="noopener noreferrer"
target="_blank"
>
<ExternalLink className="h-3.5 w-3.5" />
{externalRepositoryLabel}
</a>
</Button>
) : null}
{canMarkReady ? (
<Button
className="h-8 gap-1.5 px-3"
Expand Down
21 changes: 16 additions & 5 deletions desktop/src/testing/e2eBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,10 @@ declare global {
};
/** Overrides the first mock repository owner for delegated-owner tests. */
__BUZZ_E2E_PROJECT_OWNER_OVERRIDE__?: string;
/** Overrides the first mock repository clone URL for provider tests. */
__BUZZ_E2E_PROJECT_CLONE_URL_OVERRIDE__?: string;
/** Adds a web URL to the first mock repository for provider tests. */
__BUZZ_E2E_PROJECT_WEB_URL_OVERRIDE__?: string;
/** Project history kinds rejected with CLOSED for aggregate-query tests. */
__BUZZ_E2E_REJECT_PROJECT_QUERY_KINDS__?: number[];
/** Captured aggregate project-history filters for request-count assertions. */
Expand Down Expand Up @@ -5135,6 +5139,15 @@ function buildMockProjectEvents(): RelayEvent[] {
projectIndex === 0
? (window.__BUZZ_E2E_PROJECT_OWNER_OVERRIDE__ ?? seed.owner)
: seed.owner;
const relayCloneUrl = `${getRelayHttpUrl(getConfig())}/git/${owner}/${seed.dtag}`;
const cloneUrl =
projectIndex === 0
? (window.__BUZZ_E2E_PROJECT_CLONE_URL_OVERRIDE__ ?? relayCloneUrl)
: relayCloneUrl;
const webUrl =
projectIndex === 0
? window.__BUZZ_E2E_PROJECT_WEB_URL_OVERRIDE__
: undefined;
const repoAddress = `${KIND_REPO_ANNOUNCEMENT}:${owner}:${seed.dtag}`;
const authors = [seed.owner, ...seed.contributors];
const random = mulberry32(projectIndex + 1);
Expand All @@ -5147,7 +5160,8 @@ function buildMockProjectEvents(): RelayEvent[] {
["d", seed.dtag],
["name", seed.name],
["description", seed.description],
["clone", `https://relay.example.com/git/${owner}/${seed.dtag}`],
["clone", cloneUrl],
...(webUrl ? [["web", webUrl]] : []),
...seed.contributors.map((pubkey) => ["p", pubkey]),
],
owner,
Expand Down Expand Up @@ -5212,10 +5226,7 @@ function buildMockProjectEvents(): RelayEvent[] {
? [
["h", "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"],
["branch-name", `feature/mock-${dayOffset}-${index}`],
[
"clone",
`https://relay.example.com/git/${owner}/${seed.dtag}`,
],
["clone", cloneUrl],
]
: []),
];
Expand Down
35 changes: 35 additions & 0 deletions desktop/tests/e2e/project-pr-review.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,41 @@ test("managed agent repository owner can merge", async ({ page }) => {
});
});

test("external repository owner opens the provider instead of native merge", async ({
page,
}) => {
await enableProjectsFeature(page);
await page.addInitScript(() => {
window.__BUZZ_E2E_PROJECT_CLONE_URL_OVERRIDE__ =
"https://github.com/block/buzz.git";
window.__BUZZ_E2E_PROJECT_WEB_URL_OVERRIDE__ =
"https://github.com/block/buzz";
});
await installMockBridge(page);
await openBuzzProject(page);

await page.getByRole("tab", { name: "Pull Request" }).click();
const aliceRow = page
.getByTestId("project-pull-request-row")
.filter({ hasText: "alice" })
.first();
await aliceRow.getByRole("button", { name: /^#/ }).click();

await expect(
page.getByRole("button", { name: "Merge", exact: true }),
).toHaveCount(0);
await expect(
page.getByRole("link", { name: "Open on GitHub", exact: true }),
).toHaveAttribute("href", "https://github.com/block/buzz");
const mergeCommandCount = await page.evaluate(
() =>
window.__BUZZ_E2E_COMMANDS__?.filter(
(command) => command === "merge_project_pull_request",
).length ?? 0,
);
expect(mergeCommandCount).toBe(0);
});

test("viewer without repository ownership cannot merge", async ({ page }) => {
await enableProjectsFeature(page);
await page.addInitScript((owner) => {
Expand Down