Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
31 changes: 31 additions & 0 deletions apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,37 @@ describe("pull request toolkit handlers", () => {
}),
);

it.effect("canonicalizes GitCafe links independently of the thread's project host", () =>
Effect.gen(function* () {
for (const host of ["git.cafe", "staging.git.cafe"]) {
for (const target of [
{ host, repository: "PingDotGG/T3Code", number: 21353 },
{ url: `https://${host}/PingDotGG/T3Code/pull/21353` },
{ url: `https://${host}/PingDotGG/T3Code/pulls/21353` },
]) {
const harness = yield* makeHarness();
const result = yield* harness.call("link_pull_request", target);
const url = `https://${host}/pingdotgg/t3code/pulls/21353`;
expect(result).toMatchObject({
host,
repository: "pingdotgg/t3code",
number: 21353,
url,
});
expect(yield* Ref.get(harness.commands)).toMatchObject([
{
type: "thread.pull-request.link",
host,
repository: "pingdotgg/t3code",
number: 21353,
url,
},
]);
}
}
}),
);

it.effect("builds the URL in the project host's own shape", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
Expand Down
13 changes: 10 additions & 3 deletions apps/server/src/mcp/toolkits/pullRequests/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import {
type ThreadId,
type ThreadPullRequestLink,
} from "@t3tools/contracts";
import { changeRequestUrlFor, parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl";
import {
canonicalizeChangeRequestUrl,
changeRequestUrlFor,
parseChangeRequestUrl,
} from "@t3tools/shared/changeRequestUrl";
import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl";
import {
normalizeThreadPullRequestKey,
Expand Down Expand Up @@ -79,7 +83,10 @@ const resolveTarget = Effect.fn("PullRequestsToolkit.resolveTarget")(function* (
if (parsed === null) {
return yield* new PullRequestUrlInvalidError({});
}
return { ...normalizeThreadPullRequestKey(parsed), url: input.url } satisfies ResolvedTarget;
return {
...normalizeThreadPullRequestKey(parsed),
url: canonicalizeChangeRequestUrl(input.url),
} satisfies ResolvedTarget;
}
if (input.repository === undefined || input.number === undefined) {
return yield* new PullRequestTargetIncompleteError({});
Expand Down Expand Up @@ -110,7 +117,7 @@ const resolveTarget = Effect.fn("PullRequestsToolkit.resolveTarget")(function* (
`https://${host}/${repository}/pull/${input.number}`;
return {
...normalizeThreadPullRequestKey({ host, repository, number: input.number, url }),
url,
url: canonicalizeChangeRequestUrl(url),
} satisfies ResolvedTarget;
});

Expand Down
9 changes: 5 additions & 4 deletions apps/server/src/orchestration-v2/Orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
usageLimitBlockedRun,
} from "@t3tools/shared/orchestrationV2ThreadError";
import { threadPullRequestsOf } from "@t3tools/shared/threadPullRequests";
import { canonicalizeChangeRequestUrl } from "@t3tools/shared/changeRequestUrl";
import {
normalizeThreadPullRequestKey,
visibleThreadPullRequests,
Expand Down Expand Up @@ -579,7 +580,7 @@ function legacyPullRequestLink(
return withPullRequestWatch(
{
...key,
url: linked.url,
url: canonicalizeChangeRequestUrl(linked.url),
source: "manual",
linkedAt: DateTime.formatIso(now),
snapshot: null,
Expand Down Expand Up @@ -3227,14 +3228,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
? {
...existing,
source: command.source,
url: command.url,
url: canonicalizeChangeRequestUrl(command.url),
...(command.repositoryWebUrl !== undefined && {
repositoryWebUrl: command.repositoryWebUrl,
}),
}
: {
...key,
url: command.url,
url: canonicalizeChangeRequestUrl(command.url),
...(command.repositoryWebUrl !== undefined && {
repositoryWebUrl: command.repositoryWebUrl,
}),
Expand Down Expand Up @@ -3330,7 +3331,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
...linked.filter((link) => !threadPullRequestKeysEqual(link, key)),
{
...key,
url: command.link.url,
url: canonicalizeChangeRequestUrl(command.link.url),
...((command.link.repositoryWebUrl ?? dismissed?.repositoryWebUrl) !==
undefined && {
repositoryWebUrl:
Expand Down
19 changes: 19 additions & 0 deletions apps/server/src/orchestration-v2/runtimeLayer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2259,6 +2259,25 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => {
(yield* orchestrator.getThreadShell(threadId))?.pullRequests?.map((link) => link.number),
[2, 3, 4],
);
for (const [index, route] of ["pull", "pulls"].entries()) {
yield* orchestrator.dispatch({
type: "thread.pull-request.link",
commandId: CommandId.make(`gitcafe-pr-link-${index}`),
threadId,
host: "git.cafe",
repository: "pingdotgg/t3code",
number: 21353 + index,
url: `https://git.cafe/pingdotgg/t3code/${route}/${21353 + index}`,
source: "manual",
});
}
assert.isTrue((yield* maintenance.rebuild).valid);
assert.deepEqual(
(yield* orchestrator.getThreadShell(threadId))?.pullRequests
?.filter((link) => link.host === "git.cafe")
.map((link) => link.url),
[21353, 21354].map((number) => `https://git.cafe/pingdotgg/t3code/pulls/${number}`),
);
}),
);

Expand Down
25 changes: 25 additions & 0 deletions packages/client-runtime/src/pullRequestLinkInput.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,31 @@ const project = {
};

describe("resolveLinkPullRequestInput", () => {
it.each(["git.cafe", "staging.git.cafe"])("canonicalizes both GitCafe routes on %s", (host) => {
for (const [repository, route] of [
["web", "pull"],
["web", "pulls"],
["pull", "pull"],
["pull", "pulls"],
["pulls", "pull"],
["pulls", "pulls"],
]) {
const url = `https://${host}/acme/${repository}/${route}/42`;
expect(sourceControlClients.get("gitcafe").isChangeRequestUrl(url)).toBe(true);
expect(parsePullRequestReference(url)).toBe(url);
expect(
resolveLinkPullRequestInput({ reference: url, project: null, hasProject: () => true }),
).toEqual({
link: {
host,
repository: `acme/${repository}`,
number: 42,
url: `https://${host}/acme/${repository}/pulls/42`,
},
});
}
});

it.each([
["https://bitbucket.org/acme/web/pull-requests/42", "bitbucket.org"],
["https://github.acme.test/acme/web/pull/42", "github.acme.test"],
Expand Down
12 changes: 10 additions & 2 deletions packages/client-runtime/src/pullRequestLinkInput.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { pullRequestHostOf, type RepositoryIdentity } from "@t3tools/contracts";
import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl";
import {
canonicalizeChangeRequestUrl,
parseChangeRequestUrl,
} from "@t3tools/shared/changeRequestUrl";

import { sourceControlClients } from "./sourceControlClients.ts";

Expand Down Expand Up @@ -85,7 +88,12 @@ export function resolveLinkPullRequestInput(input: {
return { error: `No project in this environment can read ${url.host}/${url.repository}.` };
}
return {
link: { host: url.host, repository: url.repository, number: url.number, url: parsed },
link: {
host: url.host,
repository: url.repository,
number: url.number,
url: canonicalizeChangeRequestUrl(parsed),
},
};
}
const number = Number(parsed);
Expand Down
22 changes: 22 additions & 0 deletions packages/shared/src/changeRequestUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,31 @@ import { describe, expect, it } from "vite-plus/test";

import {
changeRequestUrlFor,
canonicalizeChangeRequestUrl,
matchesLinkedPullRequestUrl,
parseChangeRequestUrl,
parseGitCafePullRequestUrl,
pullRequestCandidateUrlFromReferenceAutolink,
siblingPullRequestUrl,
} from "./changeRequestUrl.ts";
import { normalizeThreadPullRequestKey } from "./threadPullRequests.ts";

describe("parseChangeRequestUrl", () => {
it.each(["git.cafe", "staging.git.cafe"])("reads both GitCafe routes on %s", (host) => {
for (const route of ["pull", "pulls"]) {
const url = `https://${host}/PingDotGG/T3Code/${route}/21353/files?w=1#diff`;
expect(parseChangeRequestUrl(url)).toEqual({
host,
repository: "pingdotgg/t3code",
number: 21353,
});
expect(canonicalizeChangeRequestUrl(url)).toBe(
`https://${host}/pingdotgg/t3code/pulls/21353`,
);
expect(parseGitCafePullRequestUrl(url)).toEqual(parseChangeRequestUrl(url));
}
});

it("reads a GitHub pull request, lower-casing the repository", () => {
expect(parseChangeRequestUrl("https://github.com/T3Tools/T3Code/pull/123")).toEqual({
host: "github.com",
Expand Down Expand Up @@ -81,6 +98,8 @@ describe("parseChangeRequestUrl", () => {
repository,
number: 123,
});
expect.soft(parseGitCafePullRequestUrl(linked.url), host).toBeNull();
expect.soft(canonicalizeChangeRequestUrl(linked.url), host).toBe(linked.url);
expect.soft(matchesLinkedPullRequestUrl(linked, `${linked.url}?view=files`), host).toBe(true);
expect.soft(matchesLinkedPullRequestUrl(linked, other.url), host).toBe(false);
expect.soft([linked, other].map(normalizeThreadPullRequestKey), host).toEqual([
Expand Down Expand Up @@ -111,6 +130,9 @@ describe("parseChangeRequestUrl", () => {
"https://github.com/t3tools/t3code/pull/abc",
"https://gitlab.com/t3tools/t3code/-/issues/12",
"https://blog.example.test/2026/updates/pull/3",
"https://git.cafe/owner/repo/pull/0",
"https://git.cafe/owner/repo/pull/9007199254740992",
"https://staging.git.cafe/owner/repo/pull/7suffix",
"https://scm.example.test/tfs/Collection/Project/_git/Repo",
"https://scm.example.test/tfs/Collection/Project/pullrequest/123",
"https://scm.example.test/tfs/Collection/Project/_git/Repo/pullrequest/abc",
Expand Down
18 changes: 17 additions & 1 deletion packages/shared/src/changeRequestUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ function isHostOf(hostname: string, apex: string, label?: string): boolean {
return label !== undefined && hostname.split(".").includes(label);
}

const gitCafePullRequestPath = /^\/([^/]+\/[^/]+)\/pulls?\/(\d+)(?:\/|$)/u;

export function parseChangeRequestUrl(targetUrl: string): ChangeRequestLink | null {
let url: URL;
try {
Expand All @@ -29,7 +31,7 @@ export function parseChangeRequestUrl(targetUrl: string): ChangeRequestLink | nu
const host = url.hostname.toLowerCase();

if (host === "git.cafe" || host === "staging.git.cafe") {
const match = /^\/([^/]+\/[^/]+)\/pulls\/(\d+)(?:\/|$)/u.exec(url.pathname);
const match = gitCafePullRequestPath.exec(url.pathname);
if (match) return claim(host, match);
} else {
if (isHostOf(host, "github.com", "github")) {
Expand All @@ -54,6 +56,20 @@ export function parseChangeRequestUrl(targetUrl: string): ChangeRequestLink | nu
return link === null || url.port === "" ? link : { ...link, authority: url.host.toLowerCase() };
}

export function parseGitCafePullRequestUrl(targetUrl: string): ChangeRequestLink | null {
const link = parseChangeRequestUrl(targetUrl);
return link !== null &&
(link.host === "git.cafe" || link.host === "staging.git.cafe") &&
gitCafePullRequestPath.test(new URL(targetUrl).pathname)
? link
: null;
}

export function canonicalizeChangeRequestUrl(targetUrl: string): string {
const link = parseGitCafePullRequestUrl(targetUrl);
return link === null ? targetUrl : `https://${link.host}/${link.repository}/pulls/${link.number}`;
}

function claim(host: string, match: RegExpExecArray | null): ChangeRequestLink | null {
const repository = match?.[1];
const number = Number(match?.[2]);
Expand Down
20 changes: 6 additions & 14 deletions packages/source-control-gitcafe/src/client/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ import {
} from "@t3tools/contracts";
import * as Effect from "effect/Effect";
import * as Schema from "effect/Schema";
import { parseGitCafePullRequestUrl } from "@t3tools/shared/changeRequestUrl";
import {
defineSourceControlClient,
isChangeRequestInProjectRepository,
isChangeRequestOnProjectHost,
isChangeRequestPath,
} from "@t3tools/source-control-core/client/definition";

/** GitCafe has no self-hosted installs: production and staging are its only hosts. */
const GITCAFE_HOSTS = new Set(["git.cafe", "staging.git.cafe"]);
function isGitCafePullRequestUrl(url: string): boolean {
return parseGitCafePullRequestUrl(url) !== null;
}

const KIND = SourceControlProviderKind.make("gitcafe");

Expand Down Expand Up @@ -60,22 +61,13 @@ export const definition = defineSourceControlClient({
reviewSummaryRequired: () => false,
// `/pulls/` is Forgejo's shape too, so only GitCafe's own hosts claim it.
checkoutCommandArgument: () => null,
isChangeRequestReference: () => false,
isChangeRequestReference: isGitCafePullRequestUrl,
changeRequestUrlHost: (url) => url.hostname,
checkoutChangeRequestHost: () => null,
isChangeRequestInRepository: (identity, link) =>
isChangeRequestInProjectRepository(KIND, identity, link),
canReadChangeRequestOnHost: (identity, link) =>
isChangeRequestOnProjectHost(KIND, identity, link),
isChangeRequestUrl: (url) => {
try {
return (
GITCAFE_HOSTS.has(new URL(url).hostname.toLowerCase()) &&
isChangeRequestPath(url, "/pulls/")
);
} catch {
return false;
}
},
isChangeRequestUrl: isGitCafePullRequestUrl,
settings,
});
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,14 @@ describe("GitCafeSourceControlProvider", () => {
};
return Effect.gen(function* () {
const provider = yield* GitCafeSourceControlProvider.make;
yield* provider.checkoutChangeRequest({
cwd: "/repo",
reference: "https://git.cafe/team/project/pulls/7",
...(context === undefined ? {} : { context }),
});
assert.deepStrictEqual(remotes, [expected]);
for (const route of ["pull", "pulls"]) {
yield* provider.checkoutChangeRequest({
cwd: "/repo",
reference: `https://git.cafe/team/project/${route}/7`,
...(context === undefined ? {} : { context }),
});
}
assert.deepStrictEqual(remotes, [expected, expected]);
}).pipe(
Effect.provide(
Layer.merge(GitCafeApi.layer, NoFiles).pipe(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { SourceControlProviderKind } from "@t3tools/contracts";
import { parseGitCafePullRequestUrl } from "@t3tools/shared/changeRequestUrl";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Option from "effect/Option";
Expand Down Expand Up @@ -166,8 +167,6 @@ const cloneUrls = (target: Target) => ({
const isSshRemote = (remoteUrl: string | undefined) =>
remoteUrl !== undefined && (/^ssh:\/\//iu.test(remoteUrl) || /^[^/:]+@[^/:]+:/u.test(remoteUrl));

const pullUrlPattern = /^https:\/\/([^/]+)\/([^/]+\/[^/]+)\/pulls\/(\d+)\/?(?:[?#].*)?$/u;

export const make = Effect.gen(function* () {
const api = yield* GitCafeApi.GitCafeApi;
const host = yield* SourceControlHost.SourceControlHost;
Expand Down Expand Up @@ -242,15 +241,15 @@ export const make = Effect.gen(function* () {
readonly context?: SourceControlProvider.SourceControlProviderContext;
readonly reference: string;
}) {
const match = pullUrlPattern.exec(input.reference.trim());
const urlHost = match?.[1] ? GitCafeHosts.gitCafeHost(match[1]) : null;
const match = parseGitCafePullRequestUrl(input.reference.trim());
const urlHost = match === null ? null : GitCafeHosts.gitCafeHost(match.host);
const target: Target =
urlHost && match?.[2]
? { host: urlHost, repository: match[2] }
urlHost && match
? { host: urlHost, repository: match.repository }
: yield* resolveTarget("getChangeRequest", input);
const number = match
? urlHost
? match[3]
? String(match.number)
: undefined
: /^#?([1-9]\d*)$/u.exec(input.reference.trim())?.[1];
if (number === undefined)
Expand Down
Loading