diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts index 25c0142230c69..507381531ac49 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts @@ -353,6 +353,39 @@ 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 project of [makeProject(), makeProject(null)]) { + 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({ project }); + 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({ diff --git a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts index 584abaf4276df..bc5081d52c119 100644 --- a/apps/server/src/mcp/toolkits/pullRequests/handlers.ts +++ b/apps/server/src/mcp/toolkits/pullRequests/handlers.ts @@ -6,6 +6,7 @@ import { type ThreadId, type ThreadPullRequestLink, } from "@t3tools/contracts"; +import { canonicalizeGitCafePullRequestUrl } from "@t3tools/source-control-definitions/registry"; import * as Cause from "effect/Cause"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; @@ -76,7 +77,10 @@ const resolveTarget = Effect.fn("PullRequestsToolkit.resolveTarget")(function* ( if (parsed === undefined) { return yield* new PullRequestUrlInvalidError({}); } - return { ...normalizeThreadPullRequestKey(parsed), url: input.url } satisfies ResolvedTarget; + return { + ...normalizeThreadPullRequestKey(parsed), + url: canonicalizeGitCafePullRequestUrl(input.url), + } satisfies ResolvedTarget; } if (input.repository === undefined || input.number === undefined) { return yield* new PullRequestTargetIncompleteError({}); @@ -89,7 +93,10 @@ const resolveTarget = Effect.fn("PullRequestsToolkit.resolveTarget")(function* ( const repository = input.repository.toLowerCase(); // Without a URL, a public forge host says what shape its change request URLs take; // any other host is taken to run the project's own forge. - const kind = sourceControlDefinitions.findByPublicHost(host)?.kind ?? projectHost.kind; + const recognizedKind = sourceControlDefinitions.recognizeRemote(`https://${host}`)?.kind; + const kind = + sourceControlDefinitions.findByPublicHost(host)?.kind ?? + (recognizedKind === "gitcafe" ? recognizedKind : projectHost.kind); const url = kind === null ? null @@ -104,7 +111,7 @@ const resolveTarget = Effect.fn("PullRequestsToolkit.resolveTarget")(function* ( } return { ...normalizeThreadPullRequestKey({ host, repository, number: input.number, url }), - url, + url: canonicalizeGitCafePullRequestUrl(url), } satisfies ResolvedTarget; }); diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 8f5fad1c735b6..8f3aeaa8bc954 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -9,6 +9,7 @@ import { runRanAfter, usageLimitBlockedRun, } from "@t3tools/shared/orchestrationV2ThreadError"; +import { canonicalizeGitCafePullRequestUrl } from "@t3tools/source-control-definitions/registry"; import { threadPullRequestsOf } from "../sourceControl/threadPullRequests.ts"; import { normalizeThreadPullRequestKey, @@ -622,7 +623,7 @@ function legacyPullRequestLink( return withPullRequestWatch( { ...key, - url: linked.url, + url: canonicalizeGitCafePullRequestUrl(linked.url), source: "manual", linkedAt: DateTime.formatIso(now), snapshot: null, @@ -3301,14 +3302,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ? { ...existing, source: command.source, - url: command.url, + url: canonicalizeGitCafePullRequestUrl(command.url), ...(command.repositoryWebUrl !== undefined && { repositoryWebUrl: command.repositoryWebUrl, }), } : { ...key, - url: command.url, + url: canonicalizeGitCafePullRequestUrl(command.url), ...(command.repositoryWebUrl !== undefined && { repositoryWebUrl: command.repositoryWebUrl, }), @@ -3329,7 +3330,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ? [ { ...branchKey, - url: branch.url, + url: canonicalizeGitCafePullRequestUrl(branch.url), source: "manual" as const, linkedAt: DateTime.formatIso(now), snapshot: null, @@ -3404,7 +3405,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...linked.filter((link) => !threadPullRequestKeysEqual(link, key)), { ...key, - url: command.link.url, + url: canonicalizeGitCafePullRequestUrl(command.link.url), ...((command.link.repositoryWebUrl ?? dismissed?.repositoryWebUrl) !== undefined && { repositoryWebUrl: diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index d09126a8d14be..206d425c9f8e4 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -2247,6 +2247,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}`), + ); }), ); diff --git a/packages/client-runtime/src/pullRequestLinkInput.test.ts b/packages/client-runtime/src/pullRequestLinkInput.test.ts index 5cba57fe84565..3747fdc833a4b 100644 --- a/packages/client-runtime/src/pullRequestLinkInput.test.ts +++ b/packages/client-runtime/src/pullRequestLinkInput.test.ts @@ -15,6 +15,33 @@ 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").parseChangeRequestUrl(new URL(url)), + ).not.toBeNull(); + 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"], diff --git a/packages/client-runtime/src/pullRequestLinkInput.ts b/packages/client-runtime/src/pullRequestLinkInput.ts index 0a5cae49b201f..472790e5d058a 100644 --- a/packages/client-runtime/src/pullRequestLinkInput.ts +++ b/packages/client-runtime/src/pullRequestLinkInput.ts @@ -1,4 +1,5 @@ import type { RepositoryIdentity } from "@t3tools/contracts"; +import { canonicalizeGitCafePullRequestUrl } from "@t3tools/source-control-definitions/registry"; import { sourceControlClients, type ChangeRequestTerminology } from "./sourceControlClients.ts"; @@ -86,7 +87,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: canonicalizeGitCafePullRequestUrl(parsed), + }, }; } const number = Number(parsed); diff --git a/packages/source-control-definitions/src/registry.ts b/packages/source-control-definitions/src/registry.ts index 0f213ab67a9ac..3823a2c3555bc 100644 --- a/packages/source-control-definitions/src/registry.ts +++ b/packages/source-control-definitions/src/registry.ts @@ -6,6 +6,8 @@ import * as GitCafe from "@t3tools/source-control-gitcafe/shared/definition"; import * as GitHub from "@t3tools/source-control-github/shared/definition"; import * as GitLab from "@t3tools/source-control-gitlab/shared/definition"; +export { canonicalizePullRequestUrl as canonicalizeGitCafePullRequestUrl } from "@t3tools/source-control-gitcafe/shared/definition"; + /** * The server and every client read hosts through this one registry, so they agree on which host * owns a URL or remote. GitHub comes first because a repository that has not reported its host diff --git a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.test.ts index 08fee86bc2670..111ffbd1537c3 100644 --- a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.test.ts @@ -242,12 +242,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( diff --git a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts index fe624532f74b3..994835dee1e7f 100644 --- a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts @@ -20,6 +20,7 @@ import { import { randomUuidV4 } from "@t3tools/provider-core/server/randomUuid"; import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import * as SourceControlProvider from "@t3tools/source-control-core/server/SourceControlProvider"; +import { parsePullRequestUrl } from "../shared/definition.ts"; import * as GitCafeApi from "./GitCafeApi.ts"; import * as GitCafeCredentials from "./GitCafeCredentials.ts"; @@ -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; @@ -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 = parsePullRequestUrl(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) diff --git a/packages/source-control-gitcafe/src/shared/definition.test.ts b/packages/source-control-gitcafe/src/shared/definition.test.ts index 5e99537fd96d0..e69ffd364018c 100644 --- a/packages/source-control-gitcafe/src/shared/definition.test.ts +++ b/packages/source-control-gitcafe/src/shared/definition.test.ts @@ -1,11 +1,45 @@ import { recognizeRemoteWith } from "@t3tools/source-control-core/shared/definition"; import { describe, expect, it } from "vite-plus/test"; -import { definition } from "./definition.ts"; +import { canonicalizePullRequestUrl, definition, parsePullRequestUrl } from "./definition.ts"; const parse = (url: string) => definition.parseChangeRequestUrl(new URL(url)); describe("GitCafe change request URLs", () => { + it.each(["git.cafe", "staging.git.cafe"])("normalizes links for storage on %s", (host) => { + for (const repository of ["web", "pull", "pulls"]) { + for (const route of ["pull", "pulls"]) { + const url = `https://${host}/Acme/${repository}/${route}/42/files?w=1#diff`; + expect(parsePullRequestUrl(url)).toMatchObject({ + host, + repository: `acme/${repository}`, + number: 42, + }); + expect(definition.isChangeRequestReference(url)).toBe(true); + expect(canonicalizePullRequestUrl(url)).toBe(`https://${host}/acme/${repository}/pulls/42`); + } + } + }); + + it("preserves other URLs and rejects invalid GitCafe references", () => { + for (const url of [ + "https://github.com/team/repo/pull/7", + "https://forge.example/team/repo/pulls/7", + "https://not.git.cafe/team/repo/pull/7", + "https://git.cafe.evil.test/team/repo/pull/7", + "https://git.cafe/tfs/collection/project/_git/repo/pullrequest/7", + "https://git.cafe/team/repo/pull/0", + "https://git.cafe/team/repo/pulls/9007199254740992", + "https://staging.git.cafe/team/repo/pull/7suffix", + "ftp://git.cafe/team/repo/pull/7", + "not a url", + ]) { + expect(parsePullRequestUrl(url), url).toBeNull(); + expect(definition.isChangeRequestReference(url), url).toBe(false); + expect(canonicalizePullRequestUrl(url), url).toBe(url); + } + }); + it("reads its canonical /pull/ path and the older /pulls/ path on its own hosts", () => { for (const path of ["pull", "pulls"]) { expect(parse(`https://git.cafe/PingDotGG/T3Code/${path}/23897`), path).toEqual({ diff --git a/packages/source-control-gitcafe/src/shared/definition.ts b/packages/source-control-gitcafe/src/shared/definition.ts index 5192c23720d26..dc352be748dc2 100644 --- a/packages/source-control-gitcafe/src/shared/definition.ts +++ b/packages/source-control-gitcafe/src/shared/definition.ts @@ -22,6 +22,12 @@ const GITCAFE_HOSTS = new Set(["git.cafe", "staging.git.cafe"]); /** GitCafe serves pull requests at `/pull/` and redirects the older `/pulls/` there. */ const CHANGE_REQUEST_PATH = /^\/([^/]+\/[^/]+)\/pulls?\/(\d+)(?:\/|$)/u; +function parseChangeRequestUrl(url: URL) { + return GITCAFE_HOSTS.has(url.hostname.toLowerCase()) + ? parseChangeRequestPath(url, CHANGE_REQUEST_PATH) + : null; +} + const KIND = SourceControlProviderKind.make("gitcafe"); export const settings = makeProviderSettingsSchema({ @@ -61,10 +67,7 @@ export const definition = defineSourceControl({ : `https://${host}/${repository}/pulls/${number}`, // `/pull/` and `/pulls/` are GitHub's and Forgejo's shapes too, so only GitCafe's own hosts // claim them. - parseChangeRequestUrl: (url) => - GITCAFE_HOSTS.has(url.hostname.toLowerCase()) - ? parseChangeRequestPath(url, CHANGE_REQUEST_PATH) - : null, + parseChangeRequestUrl, changeRequestUrlFromReferenceAutolink: () => null, changeRequestActions: new Set([ "ready", @@ -79,7 +82,7 @@ export const definition = defineSourceControl({ authorProfileUrl: () => null, reviewSummaryRequired: () => false, checkoutCommandArgument: () => null, - isChangeRequestReference: () => false, + isChangeRequestReference: (url) => parsePullRequestUrl(url) !== null, checkoutChangeRequestHost: () => null, recognizeRemote: ({ hostname }) => GITCAFE_HOSTS.has(hostname) @@ -91,3 +94,16 @@ export const definition = defineSourceControl({ isChangeRequestOnProjectHost(projectChangeRequestHost(identity, KIND), identity, link), settings, }); + +export function parsePullRequestUrl(value: string) { + try { + return parseChangeRequestUrl(new URL(value)); + } catch { + return null; + } +} + +export function canonicalizePullRequestUrl(value: string): string { + const link = parsePullRequestUrl(value); + return link === null ? value : definition.changeRequestUrl(link); +}