Skip to content
Merged
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
101 changes: 101 additions & 0 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2770,6 +2770,107 @@ it("falls back to the path's last segment where an Azure identity has no name",
assert.strictEqual(selector, "checkout");
});

it("lists origin and upstream so a fork PR is still this project's", () => {
const names = PullRequestService.repositoriesOf({
repositoryIdentity: {
provider: "github",
displayName: "pingdotgg/t3code",
owner: "pingdotgg",
name: "t3code",
remotes: [
{
remoteName: "origin",
remoteUrl: "https://github.com/patroza/t3code.git",
owner: "patroza",
name: "t3code",
canonicalKey: "github.com/patroza/t3code",
},
{
remoteName: "upstream",
remoteUrl: "https://github.com/pingdotgg/t3code.git",
owner: "pingdotgg",
name: "t3code",
canonicalKey: "github.com/pingdotgg/t3code",
},
],
},
} as never);
assert.deepStrictEqual(names.toSorted(), ["patroza/t3code", "pingdotgg/t3code"]);
});

it.effect("reads a fork pull request against origin, not the upstream identity", () =>
Effect.gen(function* () {
let requested: string | null = null;
const fork = project({
id: "p1",
title: "t3code",
workspaceRoot: "/t3code",
repository: "pingdotgg/t3code",
});
const identity = fork.repositoryIdentity!;
const withRemotes = {
...fork,
repositoryIdentity: {
...identity,
remotes: [
{
remoteName: "origin",
remoteUrl: "https://github.com/patroza/t3code.git",
owner: "patroza",
name: "t3code",
canonicalKey: "github.com/patroza/t3code",
},
{
remoteName: "upstream",
remoteUrl: "https://github.com/pingdotgg/t3code.git",
owner: "pingdotgg",
name: "t3code",
canonicalKey: "github.com/pingdotgg/t3code",
},
],
},
};
const service = yield* makeService({
projects: [withRemotes],
providers: [
fakeProvider("github", {
getChangeRequest: (input) => {
requested = input.repository;
return Effect.succeed({
...changeRequest(410, "2026-08-15T00:00:00Z"),
url: "https://github.com/patroza/t3code/pull/410",
body: "",
changedFiles: 1,
mergedAt: null,
closedAt: null,
reviewers: [],
checks: [],
mergeCapabilities: { merge: true, squash: true, rebase: true },
viewerPermissions: {
actions: ["merge"],
comment: true,
resolve: true,
verdicts: ["comment", "approve", "request-changes"],
requestReviewers: true,
},
});
},
}),
],
});

const detail = yield* service.detail({
projectId: "p1" as ProjectId,
repository: "patroza/t3code",
number: 410,
});

assert.strictEqual(requested, "patroza/t3code");
assert.strictEqual(detail.repository, "patroza/t3code");
assert.strictEqual(detail.number, 410);
}),
);

it("keeps a GitLab identity's whole path, because a nested group is part of the name", () => {
const selector = PullRequestService.repositoryIdentityOf({
repositoryIdentity: {
Expand Down
31 changes: 27 additions & 4 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,23 @@ export function repositoryIdentityOf(project: OrchestrationProjectShell): string
return identity.owner && identity.name ? `${identity.owner}/${identity.name}` : null;
}

/**
* Every repository this checkout answers to. A fork records both origin and
* upstream; the primary identity prefers upstream, but a change request on the
* fork is still this project's.
*/
export function repositoriesOf(project: OrchestrationProjectShell): ReadonlyArray<string> {
const names = new Set<string>();
const primary = repositoryIdentityOf(project);
if (primary) names.add(primary.toLowerCase());
for (const remote of project.repositoryIdentity?.remotes ?? []) {
if (remote.owner && remote.name) {
names.add(`${remote.owner}/${remote.name}`.toLowerCase());
}
}
return [...names];
}

export const make = Effect.gen(function* () {
const registry = yield* PullRequestProviderRegistry;
const projections = yield* ProjectionSnapshotQuery.ProjectionSnapshotQuery;
Expand Down Expand Up @@ -524,17 +541,23 @@ export const make = Effect.gen(function* () {
if (!match) {
return Effect.fail(new PullRequestUnavailableError({ reason: "provider-unsupported" }));
}
// The repository travels through the client, so it is checked against the project's
// own remote rather than being handed to a provider verbatim.
if (match.repository.toLowerCase() !== ref.repository.trim().toLowerCase()) {
// The repository travels through the client, so it is checked against the
// checkout's remotes rather than being handed to a provider verbatim.
// Upstream is the primary identity on a fork; origin is still ours.
const requested = ref.repository.trim().toLowerCase();
if (!repositoriesOf(match.project).includes(requested)) {
return Effect.fail(
new PullRequestOperationError({
operation: "resolveRepository",
detail: "The change request does not belong to the selected project.",
}),
);
}
return Effect.succeed(match);
return Effect.succeed(
requested === match.repository.toLowerCase()
? match
: { ...match, repository: ref.repository.trim() },
);
}),
);

Expand Down
23 changes: 15 additions & 8 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ import {
selectThreadPreviewMiniPlayer,
usePreviewMiniPlayerStore,
} from "../previewMiniPlayerStore";
import { isThreadOwnPullRequest } from "./pullRequest/pullRequestDetail.logic";
import {
isThreadOwnPullRequest,
repositoryFromChangeRequestUrl,
} from "./pullRequest/pullRequestDetail.logic";
import { PullRequestDetailPanel } from "./pullRequest/PullRequestDetailPanel";
import { PullRequestDetailGhost } from "./pullRequest/PullRequestGhosts";
import { PullRequestsUnavailableState } from "./pullRequest/PullRequestsUnavailableState";
Expand Down Expand Up @@ -3511,18 +3514,19 @@ function ChatViewContent(props: ChatViewProps) {
// project there is nothing to resolve it against, so the caller falls back to the browser.
const threadRepository = activeProject?.repositoryIdentity?.displayName ?? null;
const openThreadPullRequest = useCallback(
(number: number) => {
(number: number, repository: string | null = threadRepository) => {
const selectedRepository = repository ?? threadRepository;
if (
!supportsPullRequests ||
!activeThreadRef ||
!activeProject ||
threadRepository === null
selectedRepository === null
) {
return;
}
useRightPanelStore.getState().openPullRequest(activeThreadRef, {
projectId: activeProject.id,
repository: threadRepository,
repository: selectedRepository,
number,
});
},
Expand Down Expand Up @@ -4464,12 +4468,15 @@ function ChatViewContent(props: ChatViewProps) {
});
// The right panel offers the thread's own change request, so it can only offer it once the
// branch has one; until then the picker says so rather than opening an empty panel.
const threadPullRequestRepository =
(activeThreadPr !== null ? repositoryFromChangeRequestUrl(activeThreadPr.url) : null) ??
threadRepository;
const addPullRequestSurface = useCallback(() => {
if (activeThreadPr === null) return;
openThreadPullRequest(activeThreadPr.number);
}, [activeThreadPr, openThreadPullRequest]);
openThreadPullRequest(activeThreadPr.number, threadPullRequestRepository);
}, [activeThreadPr, openThreadPullRequest, threadPullRequestRepository]);
const pullRequestSurfaceAvailable =
supportsPullRequests && activeThreadPr !== null && threadRepository !== null;
supportsPullRequests && activeThreadPr !== null && threadPullRequestRepository !== null;
const supportsSettlement = serverConfig?.environment.capabilities.threadSettlement === true;
const supportsSnooze = serverConfig?.environment.capabilities.threadSnooze === true;
const nowMinute = useNowMinute();
Expand Down Expand Up @@ -6739,7 +6746,7 @@ function ChatViewContent(props: ChatViewProps) {
isThreadOwnPullRequest(
{
projectId: activeProject?.id ?? null,
repository: threadRepository,
repository: threadPullRequestRepository,
number: activeThreadPr?.number ?? null,
},
{
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/components/GitActionsControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
} from "lucide-react";
import { Radio as RadioPrimitive } from "@base-ui/react/radio";
import { AzureDevOpsIcon, BitbucketIcon, GitHubIcon, GitLabIcon } from "~/components/Icons";
import { repositoryFromChangeRequestUrl } from "~/components/pullRequest/pullRequestDetail.logic";
import { RadioGroup } from "~/components/ui/radio-group";
import { Spinner } from "~/components/ui/spinner";
import { cn } from "~/lib/utils";
Expand Down Expand Up @@ -105,7 +106,7 @@ interface GitActionsControlProps {
* Opens the thread's own change request beside it. Absent when the thread has no project to
* place it against, in which case it still opens in the browser.
*/
onOpenPullRequest?: ((number: number) => void) | undefined;
onOpenPullRequest?: ((number: number, repository?: string | null) => void) | undefined;
}

interface PendingDefaultBranchAction {
Expand Down Expand Up @@ -1239,7 +1240,7 @@ export default function GitActionsControl({
// Beside the thread where it was made, the way the browser opens beside it. Checked before
// the shell, which opening in the app does not need.
if (openPr && onOpenPullRequest) {
onOpenPullRequest(openPr.number);
onOpenPullRequest(openPr.number, repositoryFromChangeRequestUrl(openPr.url));
return;
}
const api = readLocalApi();
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/components/chat/ChatHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ interface ChatHeaderProps {
/** For showing usage dot on the active thread's model at conversation level. */
activeThreadDriverKind?: ProviderDriverKind | null;
activeThreadModel?: string | null;
readonly onOpenPullRequest?: ((number: number) => void) | undefined;
readonly onOpenPullRequest?: ((number: number, repository?: string | null) => void) | undefined;
onNewThreadInProject: () => void;
onRunProjectScript: (script: ProjectScript) => void;
onAddProjectScript: (input: NewProjectScriptInput) => Promise<ProjectScriptActionResult>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
handoffPrompt,
handoffReviewComments,
isThreadOwnPullRequest,
repositoryFromChangeRequestUrl,
orderPullRequestComments,
pullRequestActionNeedsHostRefresh,
pullRequestActionMenuHasGroup,
Expand Down Expand Up @@ -917,6 +918,27 @@ describe("whether the panel is showing the thread's own pull request", () => {
});
});

describe("repositoryFromChangeRequestUrl", () => {
it("reads owner/name from a GitHub pull request URL", () => {
expect(repositoryFromChangeRequestUrl("https://github.com/patroza/t3code/pull/410")).toBe(
"patroza/t3code",
);
expect(
repositoryFromChangeRequestUrl("https://github.com/pingdotgg/t3code/pull/6613/files"),
).toBe("pingdotgg/t3code");
});

it("reads a nested GitLab path", () => {
expect(
repositoryFromChangeRequestUrl("https://gitlab.com/group/sub/service/-/merge_requests/12"),
).toBe("group/sub/service");
});

it("returns null when the URL is not a change request", () => {
expect(repositoryFromChangeRequestUrl("https://github.com/patroza/t3code")).toBeNull();
});
});

describe("which actions need the host read again after they run", () => {
it("classifies every action the contract knows about", () => {
// Imported from the contract rather than hand-listed, so a new PullRequestAction fails this
Expand Down
10 changes: 10 additions & 0 deletions apps/web/src/components/pullRequest/pullRequestDetail.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/re
* number are not enough: one environment can hold two checkouts of the same repository under
* different projects, and the other project's checkout is somebody else's branch.
*/
/** Owner/name from a change-request URL, so a fork PR is not opened against upstream. */
export function repositoryFromChangeRequestUrl(url: string): string | null {
const trimmed = url.trim();
const github = /^https:\/\/[^/\s]+\/([^/\s]+\/[^/\s]+)\/pull\/\d+(?:[/?#].*)?$/i.exec(trimmed);
if (github?.[1]) return github[1];
const gitlab = /^https:\/\/[^/\s]+\/(.+)\/-\/merge_requests\/\d+(?:[/?#].*)?$/i.exec(trimmed);
if (gitlab?.[1]) return gitlab[1];
return null;
}

export function isThreadOwnPullRequest(
thread: {
readonly projectId: string | null;
Expand Down
Loading