diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index e1924c03ade..adeba057e77 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -9,6 +9,7 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; +import * as Result from "effect/Result"; import * as Scope from "effect/Scope"; import { ChildProcessSpawner } from "effect/unstable/process"; import { expect } from "vite-plus/test"; @@ -21,6 +22,7 @@ import type { import { GitCommandError, TextGenerationError } from "@t3tools/contracts"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import { decodeGitHubPullRequestListJson } from "../sourceControl/gitHubPullRequests.ts"; import * as TextGeneration from "../textGeneration/TextGeneration.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; @@ -37,6 +39,8 @@ interface FakeGhScenario { prListSequenceByHeadSelector?: Record; createdPrUrl?: string; defaultBranch?: string; + baseRepository?: string; + headRepositoryOwner?: string; pullRequest?: { number: number; title: string; @@ -97,72 +101,6 @@ interface FakeGitTextGeneration { type FakePullRequest = NonNullable; -function normalizeFakePullRequestSummary(raw: unknown): GitHubCli.GitHubPullRequestSummary | null { - if (!raw || typeof raw !== "object") { - return null; - } - - const record = raw as Record; - const number = record.number; - const title = record.title; - const url = record.url; - const baseRefName = record.baseRefName; - const headRefName = record.headRefName; - const headRepository = - typeof record.headRepository === "object" && record.headRepository !== null - ? (record.headRepository as Record) - : null; - const headRepositoryOwner = - typeof record.headRepositoryOwner === "object" && record.headRepositoryOwner !== null - ? (record.headRepositoryOwner as Record) - : null; - - if ( - typeof number !== "number" || - typeof title !== "string" || - typeof url !== "string" || - typeof baseRefName !== "string" || - typeof headRefName !== "string" - ) { - return null; - } - - const state = - typeof record.state === "string" - ? record.state === "OPEN" || record.state === "open" - ? "open" - : record.state === "CLOSED" || record.state === "closed" - ? "closed" - : "merged" - : undefined; - const isCrossRepository = - typeof record.isCrossRepository === "boolean" ? record.isCrossRepository : undefined; - const headRepositoryNameWithOwner = - typeof record.headRepositoryNameWithOwner === "string" - ? record.headRepositoryNameWithOwner - : typeof headRepository?.nameWithOwner === "string" - ? headRepository.nameWithOwner - : undefined; - const headRepositoryOwnerLogin = - typeof record.headRepositoryOwnerLogin === "string" - ? record.headRepositoryOwnerLogin - : typeof headRepositoryOwner?.login === "string" - ? headRepositoryOwner.login - : undefined; - - return { - number, - title, - url, - baseRefName, - headRefName, - ...(state ? { state } : {}), - ...(isCrossRepository !== undefined ? { isCrossRepository } : {}), - ...(headRepositoryNameWithOwner ? { headRepositoryNameWithOwner } : {}), - ...(headRepositoryOwnerLogin ? { headRepositoryOwnerLogin } : {}), - }; -} - function runGitSyncForFakeGh(cwd: string, args: readonly string[]): void { const result = NodeChildProcess.spawnSync("git", args, { cwd, @@ -377,6 +315,10 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { ]), ); const ghCalls: string[] = []; + const qualifyHeadSelector = (headSelector: string) => + scenario.headRepositoryOwner && !headSelector.includes(":") + ? `${scenario.headRepositoryOwner}:${headSelector}` + : headSelector; const execute: GitHubCli.GitHubCli["Service"]["execute"] = (input) => { const args = [...input.args]; @@ -514,28 +456,36 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return { service: { execute, - listOpenPullRequests: (input) => + listPullRequests: (input) => execute({ cwd: input.cwd, args: [ "pr", "list", "--head", - input.headSelector, + qualifyHeadSelector(input.headSelector), "--state", - "open", + input.state, "--limit", - String(input.limit ?? 1), + String(input.limit ?? (input.state === "open" ? 1 : 20)), + "--repo", + scenario.baseRepository ?? "pingdotgg/codething-mvp", "--json", - "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", + "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ], }).pipe( - Effect.map((result) => JSON.parse(result.stdout) as unknown[]), - Effect.map((raw) => - raw - .map((entry) => normalizeFakePullRequestSummary(entry)) - .filter((entry): entry is GitHubCli.GitHubPullRequestSummary => entry !== null), - ), + Effect.flatMap((result) => { + const decoded = decodeGitHubPullRequestListJson(result.stdout); + return Result.isSuccess(decoded) + ? Effect.succeed(decoded.success) + : Effect.fail( + new GitHubCli.GitHubPullRequestListDecodeError({ + command: "gh", + cwd: input.cwd, + cause: decoded.failure, + }), + ); + }), ), createPullRequest: (input) => execute({ @@ -546,17 +496,27 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { "--base", input.baseBranch, "--head", - input.headSelector, + qualifyHeadSelector(input.headSelector), "--title", input.title, "--body-file", input.bodyFile, + "--repo", + scenario.baseRepository ?? "pingdotgg/codething-mvp", ], }).pipe(Effect.asVoid), getDefaultBranch: (input) => execute({ cwd: input.cwd, - args: ["repo", "view", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"], + args: [ + "repo", + "view", + scenario.baseRepository ?? "pingdotgg/codething-mvp", + "--json", + "defaultBranchRef", + "--jq", + ".defaultBranchRef.name", + ], }).pipe( Effect.map((result) => { const value = result.stdout.trim(); @@ -570,6 +530,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { "pr", "view", input.reference, + "--repo", + scenario.baseRepository ?? "pingdotgg/codething-mvp", "--json", "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", ], @@ -581,6 +543,17 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { cwd: input.cwd, args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], }).pipe(Effect.map((result) => JSON.parse(result.stdout))), + getPullRequestBaseRepositoryCloneUrls: (input) => + execute({ + cwd: input.cwd, + args: [ + "repo", + "view", + scenario.baseRepository ?? "pingdotgg/codething-mvp", + "--json", + "nameWithOwner,url,sshUrl", + ], + }).pipe(Effect.map((result) => JSON.parse(result.stdout))), createRepository: (input) => Effect.fail( new GitHubCli.GitHubCliCommandError({ @@ -592,7 +565,14 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { checkoutPullRequest: (input) => execute({ cwd: input.cwd, - args: ["pr", "checkout", input.reference, ...(input.force ? ["--force"] : [])], + args: [ + "pr", + "checkout", + input.reference, + ...(input.force ? ["--force"] : []), + "--repo", + scenario.baseRepository ?? "pingdotgg/codething-mvp", + ], }).pipe(Effect.asVoid), }, ghCalls, @@ -1071,7 +1051,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { state: "open", }); expect(ghCalls).toContain( - "pr list --head jasonLaster:statemachine --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + "pr list --head jasonLaster:statemachine --state all --limit 20 --repo pingdotgg/codething-mvp --json number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ); }), 20_000, @@ -1794,6 +1774,183 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("create_pr targets the upstream repository when origin is a fork", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", forkDir]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:octocat/t3code.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.origin.pushurl", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/upstream-pr"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "upstream-pr.txt"), "upstream pr\n"); + yield* runGit(repoDir, ["add", "upstream-pr.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Target upstream repository"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/upstream-pr"]); + + const { manager, ghCalls } = yield* makeManager({ + ghScenario: { + baseRepository: "pingdotgg/t3code", + headRepositoryOwner: "octocat", + prListSequence: ["[]", "[]"], + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }); + + expect(result.pr.status).toBe("created"); + expect( + ghCalls.some((call) => + call.includes( + "pr create --base main --head octocat:feature/upstream-pr --title Add stacked git actions", + ), + ), + ).toBe(true); + expect(ghCalls.some((call) => call.includes("--repo pingdotgg/t3code"))).toBe(true); + expect(ghCalls.some((call) => call.includes("--repo octocat/t3code"))).toBe(false); + }), + ); + + it.effect("create_pr reuses an existing upstream PR from the origin fork", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", forkDir]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:octocat/t3code.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.origin.pushurl", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/existing-upstream-pr"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/existing-upstream-pr"]); + + const { manager, ghCalls } = yield* makeManager({ + ghScenario: { + baseRepository: "pingdotgg/t3code", + headRepositoryOwner: "octocat", + prListByHeadSelector: { + // @effect-diagnostics-next-line preferSchemaOverJson:off + "feature/existing-upstream-pr": JSON.stringify([]), + // @effect-diagnostics-next-line preferSchemaOverJson:off + "octocat:feature/existing-upstream-pr": JSON.stringify([ + { + number: 3899, + title: "Another fork's PR", + url: "https://github.com/pingdotgg/t3code/pull/3899", + baseRefName: "main", + headRefName: "feature/existing-upstream-pr", + state: "OPEN", + isCrossRepository: true, + headRepository: { + nameWithOwner: "somebody-else/t3code", + }, + headRepositoryOwner: { + login: "somebody-else", + }, + }, + { + number: 3900, + title: "Existing upstream PR", + url: "https://github.com/pingdotgg/t3code/pull/3900", + baseRefName: "main", + headRefName: "feature/existing-upstream-pr", + state: "OPEN", + isCrossRepository: true, + headRepository: { + nameWithOwner: "octocat/t3code", + }, + headRepositoryOwner: { + login: "octocat", + }, + }, + ]), + }, + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }); + + expect(result.pr.status).toBe("opened_existing"); + expect(result.pr.number).toBe(3900); + expect( + ghCalls.some((call) => + call.includes( + "pr list --head octocat:feature/existing-upstream-pr --state open --limit 100", + ), + ), + ).toBe(true); + expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); + }), + ); + + it.effect("create_pr reuses an existing upstream PR from a GitHub Enterprise fork", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", forkDir]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.company.com:octocat/t3code.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.origin.pushurl", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/enterprise-pr"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/enterprise-pr"]); + + const { manager, ghCalls } = yield* makeManager({ + ghScenario: { + baseRepository: "github.company.com/pingdotgg/t3code", + headRepositoryOwner: "octocat", + prListByHeadSelector: { + // @effect-diagnostics-next-line preferSchemaOverJson:off + "octocat:feature/enterprise-pr": JSON.stringify([ + { + number: 3901, + title: "Existing enterprise PR", + url: "https://github.company.com/pingdotgg/t3code/pull/3901", + baseRefName: "main", + headRefName: "feature/enterprise-pr", + state: "OPEN", + isCrossRepository: true, + headRepository: { + nameWithOwner: "octocat/t3code", + }, + headRepositoryOwner: { + login: "octocat", + }, + }, + ]), + }, + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }); + + expect(result.pr.status).toBe("opened_existing"); + expect(result.pr.number).toBe(3901); + expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); + }), + ); + it.effect("create_pr falls back to main when source control provider detection fails", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -1937,7 +2094,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.number).toBe(142); expect( ghCalls.some((call) => - call.includes("pr list --head octocat:statemachine --state open --limit 1"), + call.includes("pr list --head octocat:statemachine --state open --limit 100"), ), ).toBe(true); expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); @@ -2112,7 +2269,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.number).toBe(142); const ownerSelectorCallIndex = ghCalls.findIndex((call) => - call.includes("pr list --head octocat:statemachine --state open --limit 1"), + call.includes("pr list --head octocat:statemachine --state open --limit 100"), ); expect(ownerSelectorCallIndex).toBeGreaterThanOrEqual(0); expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); @@ -2178,10 +2335,10 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.status).toBe("opened_existing"); expect(result.pr.number).toBe(142); - const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 1")); + const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 100")); expect(openLookupCalls).toHaveLength(1); expect(openLookupCalls[0]).toContain( - "pr list --head octocat:statemachine --state open --limit 1", + "pr list --head octocat:statemachine --state open --limit 100", ); }), 12_000, @@ -2233,7 +2390,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); - it.effect("generates PR content against the remote base when the local base is stale", () => + it.effect("falls back to the tracking ref when the target repository fetch fails", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); yield* initRepo(repoDir); @@ -2268,6 +2425,12 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { let generatedCommitSummary = ""; const { manager } = yield* makeManager({ ghScenario: { + repositoryCloneUrls: { + "pingdotgg/codething-mvp": { + url: `${remoteDir}.git`, + sshUrl: `${remoteDir}.git`, + }, + }, prListSequence: ["[]", "[]"], }, textGeneration: { @@ -2289,6 +2452,121 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("generates PR content against upstream when origin is a stale fork", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const forkDir = yield* createBareRemote(); + const upstreamDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", forkDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "seed-upstream", upstreamDir]); + yield* runGit(repoDir, ["push", "seed-upstream", "main"]); + yield* runGit(upstreamDir, ["symbolic-ref", "HEAD", "refs/heads/main"]); + + const upstreamPeerDir = yield* makeTempDir("t3code-upstream-peer-"); + yield* runGit(upstreamPeerDir, ["clone", upstreamDir, "."]); + yield* runGit(upstreamPeerDir, ["config", "user.email", "peer@example.com"]); + yield* runGit(upstreamPeerDir, ["config", "user.name", "Peer User"]); + NodeFS.writeFileSync(NodePath.join(upstreamPeerDir, "upstream.txt"), "upstream\n"); + yield* runGit(upstreamPeerDir, ["add", "upstream.txt"]); + yield* runGit(upstreamPeerDir, ["commit", "-m", "Upstream base commit"]); + yield* runGit(upstreamPeerDir, ["push", "origin", "main"]); + + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:octocat/t3code.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.origin.pushurl", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/upstream-range"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "feature.txt"), "feature\n"); + yield* runGit(repoDir, ["add", "feature.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Feature commit"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/upstream-range"]); + + let generatedCommitSummary = ""; + const { manager } = yield* makeManager({ + ghScenario: { + baseRepository: "pingdotgg/t3code", + repositoryCloneUrls: { + "pingdotgg/t3code": { + url: upstreamDir, + sshUrl: upstreamDir, + }, + }, + prListSequence: ["[]", "[]"], + }, + textGeneration: { + generatePrContent: (input) => { + generatedCommitSummary = input.commitSummary; + return Effect.succeed({ title: "Feature PR", body: "Feature body" }); + }, + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }); + + expect(result.pr.status).toBe("created"); + expect(generatedCommitSummary).toContain("Feature commit"); + expect(generatedCommitSummary).not.toContain("Upstream base commit"); + }), + ); + + it.effect("does not generate fork PR content from origin when upstream fetch fails", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", forkDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:octocat/t3code.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.origin.pushurl", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/upstream-fetch-failure"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "feature.txt"), "feature\n"); + yield* runGit(repoDir, ["add", "feature.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Feature commit"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/upstream-fetch-failure"]); + + let generatedPrContent = false; + const { manager } = yield* makeManager({ + ghScenario: { + baseRepository: "pingdotgg/t3code", + repositoryCloneUrls: { + "pingdotgg/t3code": { + url: "/missing/upstream-repository", + sshUrl: "/missing/upstream-repository", + }, + }, + prListSequence: ["[]", "[]"], + }, + textGeneration: { + generatePrContent: () => { + generatedPrContent = true; + return Effect.succeed({ title: "Feature PR", body: "Feature body" }); + }, + }, + }); + + const error = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }).pipe(Effect.flip); + + expect(error._tag).toBe("GitCommandError"); + expect(generatedPrContent).toBe(false); + }), + ); + it.effect( "creates a new PR instead of reusing an unrelated fork PR with the same head branch", () => @@ -2579,7 +2857,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.worktreePath).toBeNull(); const branch = (yield* runGit(repoDir, ["branch", "--show-current"])).stdout.trim(); expect(branch).toBe("feature/pr-local"); - expect(ghCalls).toContain("pr checkout 64 --force"); + expect(ghCalls).toContain("pr checkout 64 --force --repo pingdotgg/codething-mvp"); }), ); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index f1fb03e7e45..c63865a57d8 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -32,6 +32,8 @@ import { import { detectSourceControlProviderFromGitRemoteUrl, mergeGitStatusParts, + normalizeGitRemoteUrl, + parseGitHubRepositoryNameWithOwnerFromRemoteUrl, resolveAutoFeatureBranchName, sanitizeBranchFragment, sanitizeFeatureBranchName, @@ -93,6 +95,7 @@ const COMMIT_TIMEOUT_MS = 10 * 60_000; const MAX_PROGRESS_TEXT_LENGTH = 500; const SHORT_SHA_LENGTH = 7; const TOAST_DESCRIPTION_MAX = 72; +const OPEN_PULL_REQUEST_CANDIDATE_LIMIT = 100; const STATUS_RESULT_CACHE_TTL = Duration.seconds(1); const STATUS_RESULT_CACHE_CAPACITY = 2_048; type StripProgressContext = T extends any ? Omit : never; @@ -187,20 +190,6 @@ function resolvePullRequestWorktreeLocalBranchName( return `t3code/pr-${pullRequest.number}/${suffix}`; } -function parseGitHubRepositoryNameWithOwnerFromRemoteUrl(url: string | null): string | null { - const trimmed = url?.trim() ?? ""; - if (trimmed.length === 0) { - return null; - } - - const match = - /^(?:git@github\.com:|ssh:\/\/git@github\.com\/|https:\/\/github\.com\/|git:\/\/github\.com\/)([^/\s]+\/[^/\s]+?)(?:\.git)?\/?$/i.exec( - trimmed, - ); - const repositoryNameWithOwner = match?.[1]?.trim() ?? ""; - return repositoryNameWithOwner.length > 0 ? repositoryNameWithOwner : null; -} - function parseRepositoryOwnerLogin(nameWithOwner: string | null): string | null { const trimmed = nameWithOwner?.trim() ?? ""; if (trimmed.length === 0) { @@ -278,17 +267,10 @@ function matchesBranchHeadContext( if ((expectedHeadRepository || expectedHeadOwner) && !prHeadRepository && !prHeadOwner) { return false; } - if (expectedHeadRepository && prHeadRepository && expectedHeadRepository !== prHeadRepository) { + } else if (pr.isCrossRepository === true) { + if (!expectedHeadRepository || !prHeadRepository) { return false; } - if (expectedHeadOwner && prHeadOwner && expectedHeadOwner !== prHeadOwner) { - return false; - } - return true; - } - - if (pr.isCrossRepository === true) { - return false; } if (expectedHeadRepository && prHeadRepository && expectedHeadRepository !== prHeadRepository) { return false; @@ -937,7 +919,7 @@ export const make = Effect.gen(function* () { cwd, headSelector, state: "open", - limit: 1, + limit: OPEN_PULL_REQUEST_CANDIDATE_LIMIT, }); const normalizedPullRequests = pullRequests.map(toPullRequestInfo); @@ -1119,6 +1101,45 @@ export const make = Effect.gen(function* () { cwd: string, baseBranch: string, ) { + const provider = yield* sourceControlProvider(cwd); + const targetCloneUrls = provider.getTargetRepositoryCloneUrls + ? yield* provider.getTargetRepositoryCloneUrls({ cwd }).pipe(Effect.orElseSucceed(() => null)) + : null; + if (targetCloneUrls) { + const originUrl = yield* readConfigValueNullable(cwd, "remote.origin.url"); + const targetMatchesOrigin = + originUrl !== null && + [targetCloneUrls.url, targetCloneUrls.sshUrl].some( + (targetUrl) => normalizeGitRemoteUrl(targetUrl) === normalizeGitRemoteUrl(originUrl), + ); + const resolveTargetBaseRangeRef = Effect.gen(function* () { + const targetUrl = shouldPreferSshRemote(originUrl) + ? targetCloneUrls.sshUrl + : targetCloneUrls.url; + const remoteName = yield* gitCore.ensureRemote({ + cwd, + preferredName: "upstream", + url: targetUrl, + }); + yield* gitCore.fetchRemoteTrackingBranch({ + cwd, + remoteName, + remoteBranch: baseBranch, + }); + return yield* gitCore + .resolveRemoteTrackingCommit({ + cwd, + refName: baseBranch, + fallbackRemoteName: remoteName, + }) + .pipe(Effect.map((resolved) => resolved.commitSha)); + }); + const targetBaseRangeRef = targetMatchesOrigin + ? yield* resolveTargetBaseRangeRef.pipe(Effect.orElseSucceed(() => null)) + : yield* resolveTargetBaseRangeRef; + if (targetBaseRangeRef) return targetBaseRangeRef; + } + const remoteName = yield* gitCore .resolvePrimaryRemoteName(cwd) .pipe(Effect.orElseSucceed(() => null)); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 5df4862b409..3d7b5ff76a5 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -1,6 +1,7 @@ import { assert, it, afterEach, describe, expect, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import { ChildProcessSpawner } from "effect/unstable/process"; import { VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; @@ -52,8 +53,32 @@ describe("GitHubCli.layer", () => { assert.notProperty(commandFailure, "operation"); }); + it.effect("reports malformed repository context without a synthetic cause", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("invalid repository context\n"))); + + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .listPullRequests({ + cwd: "/repo", + headSelector: "feature/pr-list", + state: "open", + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubRepositoryContextDecodeError"); + assert.equal(error.detail, "GitHub CLI returned invalid pull request repository context."); + assert.notProperty(error, "cause"); + }).pipe(Effect.provide(layer)), + ); + it.effect("parses pull request view output", () => Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput("github.com/pingdotgg/codething-mvp\tgithub.com/pingdotgg/codething-mvp\n"), + ), + ); mockRun.mockReturnValueOnce( Effect.succeed( processOutput( @@ -95,13 +120,15 @@ describe("GitHubCli.layer", () => { headRepositoryNameWithOwner: "octocat/codething-mvp", headRepositoryOwnerLogin: "octocat", }); - expect(mockRun).toHaveBeenCalledWith({ + expect(mockRun).toHaveBeenNthCalledWith(2, { operation: "GitHubCli.execute", command: "gh", args: [ "pr", "view", "#42", + "--repo", + "github.com/pingdotgg/codething-mvp", "--json", "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", ], @@ -113,6 +140,11 @@ describe("GitHubCli.layer", () => { it.effect("trims pull request fields decoded from gh json", () => Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput("github.com/pingdotgg/codething-mvp\tgithub.com/pingdotgg/codething-mvp\n"), + ), + ); mockRun.mockReturnValueOnce( Effect.succeed( processOutput( @@ -159,6 +191,11 @@ describe("GitHubCli.layer", () => { it.effect("skips invalid entries when parsing pr lists", () => Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput("github.com/pingdotgg/codething-mvp\tgithub.com/pingdotgg/codething-mvp\n"), + ), + ); mockRun.mockReturnValueOnce( Effect.succeed( processOutput( @@ -190,9 +227,10 @@ describe("GitHubCli.layer", () => { ); const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.listOpenPullRequests({ + const result = yield* gh.listPullRequests({ cwd: "/repo", headSelector: "feature/pr-list", + state: "open", }); assert.deepStrictEqual(result, [ @@ -203,8 +241,231 @@ describe("GitHubCli.layer", () => { baseRefName: "main", headRefName: "feature/pr-list", state: "open", + updatedAt: Option.none(), }, ]); + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: [ + "pr", + "list", + "--head", + "feature/pr-list", + "--state", + "open", + "--limit", + "1", + "--repo", + "github.com/pingdotgg/codething-mvp", + "--json", + "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + cwd: "/repo", + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(layer)), + ); + + it.effect("creates pull requests against a fork's parent repository", () => + Effect.gen(function* () { + mockRun + .mockReturnValueOnce( + Effect.succeed(processOutput("github.com/pingdotgg/t3code\tgithub.com/octocat/t3code\n")), + ) + .mockReturnValueOnce( + Effect.succeed(processOutput("https://github.com/pingdotgg/t3code/pull/42\n")), + ); + + const gh = yield* GitHubCli.GitHubCli; + yield* gh.createPullRequest({ + cwd: "/repo", + baseBranch: "main", + headSelector: "feature/fork-pr", + title: "Target upstream", + bodyFile: "/tmp/pr-body.md", + }); + + expect(mockRun).toHaveBeenNthCalledWith(1, { + operation: "GitHubCli.execute", + command: "gh", + args: [ + "repo", + "view", + "--json", + "nameWithOwner,parent,url", + "--jq", + '. as $repo | (.url | capture("^https?://(?[^/]+)").host) as $host | [if $repo.parent then "\\($host)/\\($repo.parent.owner.login)/\\($repo.parent.name)" else "\\($host)/\\($repo.nameWithOwner)" end, "\\($host)/\\($repo.nameWithOwner)"] | @tsv', + ], + cwd: "/repo", + timeoutMs: 30_000, + }); + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: [ + "api", + "--hostname", + "github.com", + "repos/pingdotgg/t3code/pulls", + "--method", + "POST", + "-f", + "title=Target upstream", + "-f", + "head=octocat:feature/fork-pr", + "-f", + "head_repo=t3code", + "-f", + "base=main", + "-F", + "body=@/tmp/pr-body.md", + "--silent", + ], + cwd: "/repo", + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(layer)), + ); + + it.effect("creates pull requests from organization-owned forks through the API", () => + Effect.gen(function* () { + mockRun + .mockReturnValueOnce( + Effect.succeed(processOutput("github.com/pingdotgg/t3code\tgithub.com/acme/t3code\n")), + ) + .mockReturnValueOnce( + Effect.succeed(processOutput("https://github.com/pingdotgg/t3code/pull/42\n")), + ); + + const gh = yield* GitHubCli.GitHubCli; + yield* gh.createPullRequest({ + cwd: "/repo", + headSelector: "acme:feature/fork-pr", + baseBranch: "main", + title: "Target upstream", + bodyFile: "/tmp/pr-body.md", + }); + + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: expect.arrayContaining(["head=acme:feature/fork-pr", "head_repo=t3code"]), + cwd: "/repo", + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(layer)), + ); + + it.effect("keeps fork head selectors unqualified when listing pull requests", () => + Effect.gen(function* () { + mockRun + .mockReturnValueOnce( + Effect.succeed(processOutput("github.com/pingdotgg/t3code\tgithub.com/octocat/t3code\n")), + ) + .mockReturnValueOnce(Effect.succeed(processOutput("[]\n"))); + + const gh = yield* GitHubCli.GitHubCli; + yield* gh.listPullRequests({ + cwd: "/repo", + headSelector: "feature/fork-pr", + state: "open", + }); + + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: expect.arrayContaining(["--head", "feature/fork-pr"]), + cwd: "/repo", + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(layer)), + ); + + it.effect("preserves GitHub Enterprise hosts across pull request operations", () => + Effect.gen(function* () { + mockRun + .mockReturnValueOnce( + Effect.succeed( + processOutput( + "github.company.com/upstream/widgets\tgithub.company.com/octocat/widgets\n", + ), + ), + ) + .mockReturnValueOnce(Effect.succeed(processOutput("[]\n"))) + .mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + number: 42, + title: "Enterprise PR", + url: "https://github.company.com/upstream/widgets/pull/42", + baseRefName: "main", + headRefName: "feature/enterprise", + state: "OPEN", + }), + ), + ), + ) + .mockReturnValueOnce(Effect.succeed(processOutput(""))); + + const gh = yield* GitHubCli.GitHubCli; + yield* gh.listPullRequests({ + cwd: "/enterprise-repo", + headSelector: "octocat:feature/enterprise", + state: "all", + }); + yield* gh.getPullRequest({ cwd: "/enterprise-repo", reference: "42" }); + yield* gh.checkoutPullRequest({ + cwd: "/enterprise-repo", + reference: "42", + force: true, + }); + + expect(mockRun).toHaveBeenCalledTimes(4); + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: [ + "pr", + "list", + "--head", + "octocat:feature/enterprise", + "--state", + "all", + "--limit", + "20", + "--repo", + "github.company.com/upstream/widgets", + "--json", + "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + cwd: "/enterprise-repo", + timeoutMs: 30_000, + }); + expect(mockRun).toHaveBeenNthCalledWith(3, { + operation: "GitHubCli.execute", + command: "gh", + args: [ + "pr", + "view", + "42", + "--repo", + "github.company.com/upstream/widgets", + "--json", + "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + cwd: "/enterprise-repo", + timeoutMs: 30_000, + }); + expect(mockRun).toHaveBeenNthCalledWith(4, { + operation: "GitHubCli.execute", + command: "gh", + args: ["pr", "checkout", "42", "--force", "--repo", "github.company.com/upstream/widgets"], + cwd: "/enterprise-repo", + timeoutMs: 30_000, + }); }).pipe(Effect.provide(layer)), ); @@ -300,7 +561,15 @@ describe("GitHubCli.layer", () => { detail: "GraphQL: Could not resolve to a PullRequest with the number of 4888. (repository.pullRequest)", }); - mockRun.mockReturnValueOnce(Effect.fail(cause)); + mockRun + .mockReturnValueOnce( + Effect.succeed( + processOutput( + "github.com/pingdotgg/codething-mvp\tgithub.com/pingdotgg/codething-mvp\n", + ), + ), + ) + .mockReturnValueOnce(Effect.fail(cause)); const gh = yield* GitHubCli.GitHubCli; const error = yield* gh diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index bf3f27378b5..541258408c4 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -1,12 +1,18 @@ import * as Context from "effect/Context"; +import * as Cache from "effect/Cache"; +import type * as DateTime from "effect/DateTime"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; import { TrimmedNonEmptyString, + type ChangeRequestState, type SourceControlRepositoryVisibility, type VcsError, } from "@t3tools/contracts"; @@ -18,6 +24,54 @@ import { } from "./gitHubPullRequests.ts"; const DEFAULT_TIMEOUT_MS = 30_000; +const BASE_REPOSITORY_CACHE_CAPACITY = 2_048; +const BASE_REPOSITORY_CACHE_TTL = Duration.seconds(5); + +interface PullRequestRepositoryContext { + readonly baseRepository: string; + readonly headRepository: string; +} + +interface GitHubRepositoryCoordinate { + readonly host: string; + readonly owner: string; + readonly name: string; +} + +function parseGitHubRepositoryCoordinate( + repository: string, +): GitHubRepositoryCoordinate | undefined { + const parts = repository.split("/").filter((part) => part.length > 0); + if (parts.length !== 3) return undefined; + const [host, owner, name] = parts; + return host && owner && name ? { host, owner, name } : undefined; +} + +function ownerLoginFromRepository(repository: string): string | undefined { + const parts = repository.split("/").filter((part) => part.length > 0); + if (parts.length >= 3) { + return parts[parts.length - 2]; + } + if (parts.length === 2) { + return parts[0]; + } + return undefined; +} + +function qualifyPullRequestHead( + context: PullRequestRepositoryContext, + headSelector: string, +): string { + if ( + context.baseRepository.toLowerCase() === context.headRepository.toLowerCase() || + /^[^:\s]+:.+$/u.test(headSelector) + ) { + return headSelector; + } + + const owner = ownerLoginFromRepository(context.headRepository); + return owner ? `${owner}:${headSelector}` : headSelector; +} const gitHubCliFailureFields = { command: Schema.Literal("gh"), @@ -77,6 +131,22 @@ export class GitHubCliCommandError extends Schema.TaggedErrorClass()( + "GitHubRepositoryContextDecodeError", + { + command: Schema.Literal("gh"), + cwd: Schema.String, + }, +) { + get detail(): string { + return "GitHub CLI returned invalid pull request repository context."; + } + + override get message(): string { + return `GitHub CLI failed in resolvePullRequestRepositoryContext: ${this.detail}`; + } +} + const gitHubCliDecodeFields = { command: Schema.Literal("gh"), cwd: Schema.String, @@ -92,7 +162,7 @@ export class GitHubPullRequestListDecodeError extends Schema.TaggedErrorClass; readonly isCrossRepository?: boolean; readonly headRepositoryNameWithOwner?: string | null; readonly headRepositoryOwnerLogin?: string | null; @@ -205,9 +277,10 @@ export class GitHubCli extends Context.Service< readonly timeoutMs?: number; }) => Effect.Effect; - readonly listOpenPullRequests: (input: { + readonly listPullRequests: (input: { readonly cwd: string; readonly headSelector: string; + readonly state: ChangeRequestState | "all"; readonly limit?: number; }) => Effect.Effect, GitHubCliError>; @@ -221,6 +294,10 @@ export class GitHubCli extends Context.Service< readonly repository: string; }) => Effect.Effect; + readonly getPullRequestBaseRepositoryCloneUrls: (input: { + readonly cwd: string; + }) => Effect.Effect; + readonly createRepository: (input: { readonly cwd: string; readonly repository: string; @@ -317,24 +394,85 @@ export const make = Effect.gen(function* () { }) .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); - return GitHubCli.of({ - execute, - listOpenPullRequests: (input) => + const pullRequestRepositoryContextCache = yield* Cache.makeWith( + (cwd: string) => execute({ - cwd: input.cwd, + cwd, args: [ - "pr", - "list", - "--head", - input.headSelector, - "--state", - "open", - "--limit", - String(input.limit ?? 1), + "repo", + "view", "--json", - "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", + "nameWithOwner,parent,url", + "--jq", + '. as $repo | (.url | capture("^https?://(?[^/]+)").host) as $host | [if $repo.parent then "\\($host)/\\($repo.parent.owner.login)/\\($repo.parent.name)" else "\\($host)/\\($repo.nameWithOwner)" end, "\\($host)/\\($repo.nameWithOwner)"] | @tsv', ], }).pipe( + Effect.flatMap((result) => { + const [baseRepository, headRepository] = result.stdout.trim().split("\t"); + if (baseRepository && headRepository) { + return Effect.succeed({ baseRepository, headRepository }); + } + return Effect.fail( + new GitHubRepositoryContextDecodeError({ + command: "gh", + cwd, + }), + ); + }), + ), + { + capacity: BASE_REPOSITORY_CACHE_CAPACITY, + timeToLive: (exit) => (Exit.isSuccess(exit) ? BASE_REPOSITORY_CACHE_TTL : Duration.zero), + }, + ); + + const resolvePullRequestRepositoryContext = (cwd: string) => + Cache.get(pullRequestRepositoryContextCache, cwd); + + const getRepositoryCloneUrls: GitHubCli["Service"]["getRepositoryCloneUrls"] = (input) => + execute({ + cwd: input.cwd, + args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], + }).pipe( + Effect.map((result) => result.stdout.trim()), + Effect.flatMap((raw) => + decodeRawGitHubRepositoryCloneUrls(raw).pipe( + Effect.mapError( + (cause) => + new GitHubRepositoryDecodeError({ + command: "gh", + cwd: input.cwd, + cause, + }), + ), + ), + ), + Effect.map(normalizeRepositoryCloneUrls), + ); + + return GitHubCli.of({ + execute, + listPullRequests: (input) => + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => + execute({ + cwd: input.cwd, + args: [ + "pr", + "list", + "--head", + input.headSelector, + "--state", + input.state, + "--limit", + String(input.limit ?? (input.state === "open" ? 1 : 20)), + "--repo", + context.baseRepository, + "--json", + "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + }), + ), Effect.map((result) => result.stdout.trim()), Effect.flatMap((raw) => raw.length === 0 @@ -351,24 +489,27 @@ export const make = Effect.gen(function* () { ); } - return Effect.succeed( - decoded.success.map(({ updatedAt: _updatedAt, ...summary }) => summary), - ); + return Effect.succeed(decoded.success); }), ), ), ), getPullRequest: (input) => - execute({ - cwd: input.cwd, - args: [ - "pr", - "view", - input.reference, - "--json", - "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], - }).pipe( + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => + execute({ + cwd: input.cwd, + args: [ + "pr", + "view", + input.reference, + "--repo", + context.baseRepository, + "--json", + "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + }), + ), Effect.map((result) => result.stdout.trim()), Effect.flatMap((raw) => Effect.sync(() => decodeGitHubPullRequestJson(raw)).pipe( @@ -390,25 +531,12 @@ export const make = Effect.gen(function* () { ), ), ), - getRepositoryCloneUrls: (input) => - execute({ - cwd: input.cwd, - args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], - }).pipe( - Effect.map((result) => result.stdout.trim()), - Effect.flatMap((raw) => - decodeRawGitHubRepositoryCloneUrls(raw).pipe( - Effect.mapError( - (cause) => - new GitHubRepositoryDecodeError({ - command: "gh", - cwd: input.cwd, - cause, - }), - ), - ), + getRepositoryCloneUrls, + getPullRequestBaseRepositoryCloneUrls: (input) => + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => + getRepositoryCloneUrls({ cwd: input.cwd, repository: context.baseRepository }), ), - Effect.map(normalizeRepositoryCloneUrls), ), createRepository: (input) => execute({ @@ -420,36 +548,101 @@ export const make = Effect.gen(function* () { ), ), createPullRequest: (input) => - execute({ - cwd: input.cwd, - args: [ - "pr", - "create", - "--base", - input.baseBranch, - "--head", - input.headSelector, - "--title", - input.title, - "--body-file", - input.bodyFile, - ], - }).pipe(Effect.asVoid), + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => { + const base = parseGitHubRepositoryCoordinate(context.baseRepository); + const head = parseGitHubRepositoryCoordinate(context.headRepository); + if ( + base && + head && + context.baseRepository.toLowerCase() !== context.headRepository.toLowerCase() + ) { + const qualifiedHead = qualifyPullRequestHead(context, input.headSelector); + const separatorIndex = qualifiedHead.indexOf(":"); + const headBranch = + separatorIndex >= 0 ? qualifiedHead.slice(separatorIndex + 1) : qualifiedHead; + return execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + base.host, + `repos/${base.owner}/${base.name}/pulls`, + "--method", + "POST", + "-f", + `title=${input.title}`, + "-f", + `head=${head.owner}:${headBranch}`, + "-f", + `head_repo=${head.name}`, + "-f", + `base=${input.baseBranch}`, + "-F", + `body=@${input.bodyFile}`, + "--silent", + ], + }); + } + + return execute({ + cwd: input.cwd, + args: [ + "pr", + "create", + "--base", + input.baseBranch, + "--head", + qualifyPullRequestHead(context, input.headSelector), + "--title", + input.title, + "--body-file", + input.bodyFile, + "--repo", + context.baseRepository, + ], + }); + }), + Effect.asVoid, + ), getDefaultBranch: (input) => - execute({ - cwd: input.cwd, - args: ["repo", "view", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"], - }).pipe( + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => + execute({ + cwd: input.cwd, + args: [ + "repo", + "view", + context.baseRepository, + "--json", + "defaultBranchRef", + "--jq", + ".defaultBranchRef.name", + ], + }), + ), Effect.map((value) => { const trimmed = value.stdout.trim(); return trimmed.length > 0 ? trimmed : null; }), ), checkoutPullRequest: (input) => - execute({ - cwd: input.cwd, - args: ["pr", "checkout", input.reference, ...(input.force ? ["--force"] : [])], - }).pipe(Effect.asVoid), + resolvePullRequestRepositoryContext(input.cwd).pipe( + Effect.flatMap((context) => + execute({ + cwd: input.cwd, + args: [ + "pr", + "checkout", + input.reference, + ...(input.force ? ["--force"] : []), + "--repo", + context.baseRepository, + ], + }), + ), + Effect.asVoid, + ), }); }); diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 9e8a6829566..6beb96ecb5f 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -109,27 +109,23 @@ it.effect("adds safe request context while retaining GitHub CLI causes", () => }), ); -it.effect("uses gh json listing for non-open change request state queries", () => +it.effect("maps non-open change request state queries", () => Effect.gen(function* () { - let executeArgs: ReadonlyArray = []; + let listInput: Parameters[0] | null = null; const provider = yield* makeProvider({ - execute: (input) => { - executeArgs = input.args; - return Effect.succeed( - processResult( - JSON.stringify([ - { - number: 7, - title: "Merged work", - url: "https://github.com/pingdotgg/t3code/pull/7", - baseRefName: "main", - headRefName: "feature/merged", - state: "merged", - updatedAt: "2026-01-02T00:00:00.000Z", - }, - ]), - ), - ); + listPullRequests: (input) => { + listInput = input; + return Effect.succeed([ + { + number: 7, + title: "Merged work", + url: "https://github.com/pingdotgg/t3code/pull/7", + baseRefName: "main", + headRefName: "feature/merged", + state: "merged", + updatedAt: Option.some(DateTime.makeUnsafe("2026-01-02T00:00:00.000Z")), + }, + ]); }, }); @@ -140,18 +136,12 @@ it.effect("uses gh json listing for non-open change request state queries", () = limit: 10, }); - assert.deepStrictEqual(executeArgs, [ - "pr", - "list", - "--head", - "feature/merged", - "--state", - "all", - "--limit", - "10", - "--json", - "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ]); + assert.deepStrictEqual(listInput, { + cwd: "/repo", + headSelector: "feature/merged", + state: "all", + limit: 10, + }); assert.strictEqual(changeRequests[0]?.provider, "github"); assert.strictEqual(changeRequests[0]?.state, "merged"); assert.deepStrictEqual( @@ -164,7 +154,7 @@ it.effect("uses gh json listing for non-open change request state queries", () = it.effect("treats empty non-open change request listing output as no results", () => Effect.gen(function* () { const provider = yield* makeProvider({ - execute: () => Effect.succeed(processResult("")), + listPullRequests: () => Effect.succeed([]), }); const changeRequests = yield* provider.listChangeRequests({ diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index b5d5d3a55f8..8a95f7f36a1 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -1,16 +1,10 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; -import * as Result from "effect/Result"; -import { - SourceControlProviderError, - type ChangeRequest, - type ChangeRequestState, -} from "@t3tools/contracts"; +import { SourceControlProviderError, type ChangeRequest } from "@t3tools/contracts"; import * as GitHubCli from "./GitHubCli.ts"; import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; -import { decodeGitHubPullRequestListJson } from "./gitHubPullRequests.ts"; import * as SourceControlProvider from "./SourceControlProvider.ts"; import { combinedAuthOutput, @@ -29,7 +23,7 @@ function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeReq baseRefName: summary.baseRefName, headRefName: summary.headRefName, state: summary.state ?? "open", - updatedAt: Option.none(), + updatedAt: summary.updatedAt ?? Option.none(), ...(summary.isCrossRepository !== undefined ? { isCrossRepository: summary.isCrossRepository } : {}), @@ -98,75 +92,16 @@ export const make = Effect.gen(function* () { const github = yield* GitHubCli.GitHubCli; const listChangeRequests: SourceControlProvider.SourceControlProvider["Service"]["listChangeRequests"] = - (input) => { - if (input.state === "open") { - return github - .listOpenPullRequests({ - cwd: input.cwd, - headSelector: input.headSelector, - ...(input.limit !== undefined ? { limit: input.limit } : {}), - }) - .pipe( - Effect.map((items) => items.map(toChangeRequest)), - Effect.mapError( - (error) => - new SourceControlProviderError({ - provider: "github", - operation: "listChangeRequests", - command: error.command, - cwd: input.cwd, - reference: SourceControlProvider.transportSafeSourceControlErrorValue( - input.headSelector, - ), - detail: error.detail, - cause: error, - }), - ), - ); - } - - const stateArg: ChangeRequestState | "all" = input.state; - return github - .execute({ + (input) => + github + .listPullRequests({ cwd: input.cwd, - args: [ - "pr", - "list", - "--head", - input.headSelector, - "--state", - stateArg, - "--limit", - String(input.limit ?? 20), - "--json", - "number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], + headSelector: input.headSelector, + state: input.state, + ...(input.limit !== undefined ? { limit: input.limit } : {}), }) .pipe( - Effect.flatMap((result) => { - const raw = result.stdout.trim(); - if (raw.length === 0) { - return Effect.succeed([]); - } - return Effect.sync(() => decodeGitHubPullRequestListJson(raw)).pipe( - Effect.flatMap((decoded) => - Result.isSuccess(decoded) - ? Effect.succeed( - decoded.success.map((item) => ({ - ...toChangeRequest(item), - updatedAt: item.updatedAt, - })), - ) - : Effect.fail( - new GitHubCli.GitHubChangeRequestListDecodeError({ - command: "gh", - cwd: input.cwd, - cause: decoded.failure, - }), - ), - ), - ); - }), + Effect.map((items) => items.map(toChangeRequest)), Effect.mapError( (error) => new SourceControlProviderError({ @@ -182,7 +117,6 @@ export const make = Effect.gen(function* () { }), ), ); - }; return SourceControlProvider.SourceControlProvider.of({ kind: "github", @@ -247,6 +181,20 @@ export const make = Effect.gen(function* () { }), ), ), + getTargetRepositoryCloneUrls: (input) => + github.getPullRequestBaseRepositoryCloneUrls(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "getRepositoryCloneUrls", + command: error.command, + cwd: input.cwd, + detail: error.detail, + cause: error, + }), + ), + ), createRepository: (input) => github.createRepository(input).pipe( Effect.mapError( diff --git a/apps/server/src/sourceControl/SourceControlProvider.ts b/apps/server/src/sourceControl/SourceControlProvider.ts index 5f93dbcaa42..8a67c69a5a8 100644 --- a/apps/server/src/sourceControl/SourceControlProvider.ts +++ b/apps/server/src/sourceControl/SourceControlProvider.ts @@ -111,6 +111,10 @@ export class SourceControlProvider extends Context.Service< readonly context?: SourceControlProviderContext; readonly repository: string; }) => Effect.Effect; + readonly getTargetRepositoryCloneUrls?: (input: { + readonly cwd: string; + readonly context?: SourceControlProviderContext; + }) => Effect.Effect; readonly createRepository: (input: { readonly cwd: string; readonly repository: string; diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index fb70d677e43..5193dde3c1e 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -164,6 +164,15 @@ function bindProviderContext( ...input, context: input.context ?? context, }), + ...(provider.getTargetRepositoryCloneUrls + ? { + getTargetRepositoryCloneUrls: (input) => + provider.getTargetRepositoryCloneUrls!({ + ...input, + context: input.context ?? context, + }), + } + : {}), getChangeRequest: (input) => provider.getChangeRequest({ ...input, diff --git a/packages/shared/src/git.test.ts b/packages/shared/src/git.test.ts index 80578e262f9..b8e53a468fe 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -50,6 +50,17 @@ describe("parseGitHubRepositoryNameWithOwnerFromRemoteUrl", () => { expect( parseGitHubRepositoryNameWithOwnerFromRemoteUrl("https://github.com/T3Tools/T3Code.git"), ).toBe("T3Tools/T3Code"); + expect( + parseGitHubRepositoryNameWithOwnerFromRemoteUrl( + "ssh://git@github.company.com:2222/T3Tools/T3Code.git", + ), + ).toBe("T3Tools/T3Code"); + expect( + parseGitHubRepositoryNameWithOwnerFromRemoteUrl("git@github.company.com:T3Tools/T3Code.git"), + ).toBe("T3Tools/T3Code"); + expect( + parseGitHubRepositoryNameWithOwnerFromRemoteUrl("git@gitlab.com:T3Tools/T3Code.git"), + ).toBeNull(); }); }); diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index ae50b148835..e6021c86f27 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -135,16 +135,31 @@ export function normalizeGitRemoteUrl(value: string): string { */ export function parseGitHubRepositoryNameWithOwnerFromRemoteUrl(url: string | null): string | null { const trimmed = url?.trim() ?? ""; - if (trimmed.length === 0) { + if ( + trimmed.length === 0 || + detectSourceControlProviderFromRemoteUrl(trimmed)?.kind !== "github" + ) { return null; } - const match = - /^(?:git@github\.com:|ssh:\/\/git@github\.com\/|https:\/\/github\.com\/|git:\/\/github\.com\/)([^/\s]+\/[^/\s]+?)(?:\.git)?\/?$/i.exec( - trimmed, - ); - const repositoryNameWithOwner = match?.[1]?.trim() ?? ""; - return repositoryNameWithOwner.length > 0 ? repositoryNameWithOwner : null; + let repositoryPath = ""; + if (/^(?:ssh|https?|git):\/\//i.test(trimmed)) { + try { + repositoryPath = new URL(trimmed).pathname; + } catch { + return null; + } + } else { + repositoryPath = /^git@[^:/\s]+[:/](.+)$/iu.exec(trimmed)?.[1] ?? ""; + } + + const parts = repositoryPath + .replace(/\/+$/gu, "") + .replace(/\.git$/iu, "") + .split("/"); + const owner = parts.at(-2)?.trim() ?? ""; + const repository = parts.at(-1)?.trim() ?? ""; + return owner && repository ? `${owner}/${repository}` : null; } function deriveLocalBranchNameCandidatesFromRemoteRef(