Skip to content
Closed
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
190 changes: 190 additions & 0 deletions apps/server/src/git/GitManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,27 @@ function configureVisibleRemoteUrlWithLocalRewrite(
});
}

// Adds origin as the base repository that a fork remote's PRs target.
function addBaseOriginRemote(
cwd: string,
): Effect.Effect<
void,
PlatformError.PlatformError | GitCommandError,
FileSystem.FileSystem | Scope.Scope | GitVcsDriver.GitVcsDriver
> {
return Effect.gen(function* () {
const originDir = yield* createBareRemote();
yield* runGit(cwd, ["remote", "add", "origin", originDir]);
yield* runGit(cwd, ["push", "origin", "main"]);
yield* configureVisibleRemoteUrlWithLocalRewrite(
cwd,
"origin",
"git@github.com:pingdotgg/codething-mvp.git",
originDir,
);
});
}

function createTextGeneration(
overrides: Partial<FakeGitTextGeneration> = {},
): TextGeneration.TextGeneration["Service"] {
Expand Down Expand Up @@ -3922,6 +3943,172 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
12_000,
);

it.effect.each(["status", "commit_push_pr"] as const)(
"finds the open same-repository PR via %s when the only remote is not named origin",
(lookup) =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "dev"]);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork", remoteDir]);
yield* runGit(repoDir, ["push", "-u", "fork", "dev"]);
yield* configureVisibleRemoteUrlWithLocalRewrite(
repoDir,
"fork",
"git@github.com:pingdotgg/codething-mvp.git",
remoteDir,
);
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");

const sameRepositoryPr = {
number: 3,
title: "Dev",
url: "https://github.com/pingdotgg/codething-mvp/pull/3",
baseRefName: "main",
headRefName: "dev",
state: "OPEN",
isCrossRepository: false,
headRepository: { nameWithOwner: "pingdotgg/codething-mvp" },
headRepositoryOwner: { login: "pingdotgg" },
};
const { manager, ghCalls } = yield* makeManager({
ghScenario: {
prListByHeadSelector: { dev: JSON.stringify([sameRepositoryPr]) },
},
});

if (lookup === "status") {
const status = yield* manager.status({ cwd: repoDir });
expect(status.pr?.number).toBe(3);
return;
}

const result = yield* runStackedAction(manager, {
cwd: repoDir,
action: "commit_push_pr",
});

expect(result.commit.status).toBe("created");
expect(result.push.status).toBe("pushed");
expect(result.pr.status).toBe("opened_existing");
expect(result.pr.number).toBe(3);
expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false);
}),
);

it.effect("preserves fork PR selectors with fork and upstream remotes but no origin", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "dev"]);
const forkDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork", forkDir]);
yield* runGit(repoDir, ["push", "-u", "fork", "dev"]);
yield* configureVisibleRemoteUrlWithLocalRewrite(
repoDir,
"fork",
"https://github.com/contributor/demo.git",
forkDir,
);
const upstreamDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "upstream", upstreamDir]);
yield* runGit(repoDir, ["push", "upstream", "main"]);
yield* configureVisibleRemoteUrlWithLocalRewrite(
repoDir,
"upstream",
"https://github.com/acme/demo.git",
upstreamDir,
);
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");
const sameRepositoryPr = {
number: 3,
title: "Same repository",
url: "https://github.com/acme/demo/pull/3",
baseRefName: "main",
headRefName: "dev",
state: "OPEN",
isCrossRepository: false,
headRepository: { nameWithOwner: "contributor/demo" },
headRepositoryOwner: { login: "contributor" },
};
const forkPr = {
...sameRepositoryPr,
number: 4,
title: "Fork",
url: "https://github.com/acme/demo/pull/4",
isCrossRepository: true,
headRepository: { nameWithOwner: "contributor/demo" },
headRepositoryOwner: { login: "contributor" },
};
const { manager, ghCalls } = yield* makeManager({
ghScenario: {
prListByHeadSelector: {
dev: encodeCliJson([sameRepositoryPr, forkPr]),
},
},
});
const status = yield* manager.status({ cwd: repoDir });
expect(status.pr?.number).toBe(4);
// GitHub lists the bare branch and filters the same-repository decoy by its cross-repo flag.
expect(
ghCalls
.filter((call) => call.startsWith("pr list "))
.map((call) => call.split(" --state")[0]),
).toEqual(["pr list --head dev"]);
ghCalls.length = 0;
const result = yield* runStackedAction(manager, { cwd: repoDir, action: "commit_push_pr" });
expect(result.pr.status).toBe("opened_existing");
expect(result.pr.number).toBe(4);
expect(
ghCalls
.filter((call) => call.startsWith("pr list "))
.map((call) => call.split(" --state")[0]),
).toEqual(["pr list --head dev"]);
expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false);
const fresh = yield* makeManager();
const created = yield* runStackedAction(fresh.manager, { cwd: repoDir, action: "create_pr" });
expect(created.pr.status).toBe("created");
expect(
fresh.ghCalls.some((call) =>
call.startsWith("pr create --base main --head contributor:dev "),
),
).toBe(true);
}),
);

it.effect(
"keeps fork PR selectors when gh set-default points the only remote at its parent",
() =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "dev"]);
const forkDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork", forkDir]);
yield* runGit(repoDir, ["push", "fork", "main"]);
yield* runGit(repoDir, ["push", "-u", "fork", "dev"]);
yield* configureVisibleRemoteUrlWithLocalRewrite(
repoDir,
"fork",
"https://github.com/contributor/demo.git",
forkDir,
);
yield* runGit(repoDir, ["config", "remote.fork.gh-resolved", "acme/demo"]);
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");

const { manager, ghCalls } = yield* makeManager();
const created = yield* runStackedAction(manager, {
cwd: repoDir,
action: "commit_push_pr",
});
expect(created.pr.status).toBe("created");
expect(
ghCalls.some((call) => call.startsWith("pr create --base main --head contributor:dev ")),
).toBe(true);
}),
);

it.effect(
"returns the correct existing PR when a slash remote checks out to a synthetic local alias",
() =>
Expand Down Expand Up @@ -4005,6 +4192,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
yield* runGit(repoDir, ["checkout", "-b", "statemachine"]);
const forkDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]);
yield* addBaseOriginRemote(repoDir);
yield* runGit(repoDir, ["push", "-u", "fork-seed", "statemachine"]);
yield* runGit(repoDir, ["checkout", "-b", "t3code/pr-142/statemachine"]);
yield* runGit(repoDir, ["branch", "--set-upstream-to", "fork-seed/statemachine"]);
Expand Down Expand Up @@ -4132,6 +4320,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
yield* runGit(repoDir, ["checkout", "-b", "statemachine"]);
const forkDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]);
yield* addBaseOriginRemote(repoDir);
yield* runGit(repoDir, ["push", "-u", "fork-seed", "statemachine"]);
yield* runGit(repoDir, [
"config",
Expand Down Expand Up @@ -4524,6 +4713,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
yield* initRepo(repoDir);
const forkDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]);
yield* addBaseOriginRemote(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "statemachine"]);
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");
yield* runGit(repoDir, ["add", "changes.txt"]);
Expand Down
90 changes: 76 additions & 14 deletions apps/server/src/git/GitManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1404,16 +1404,71 @@ export const make = Effect.gen(function* () {
};
});

// Returns [head remote, origin]. Most branches track origin, so read it once.
const resolveHeadAndOriginContexts = (cwd: string, remoteName: string | null) =>
remoteName === "origin"
? resolveRemoteRepositoryContext(cwd, "origin").pipe(
Effect.map((origin) => [origin, origin] as const),
// The remote holding the repository PRs target: origin, else the only remote,
// which gh also reads as the base. Null when several remotes leave it open.
const resolveTargetRemoteName = (cwd: string) =>
gitCore
.execute({
operation: "GitManager.resolveTargetRemoteName",
cwd,
args: ["remote"],
timeoutMs: 5_000,
})
.pipe(
Effect.map(({ stdout }) => {
const names = stdout
.split("\n")
.map((name) => name.trim())
.filter((name) => name.length > 0);
if (names.includes("origin")) return "origin";
return names.length === 1 ? names[0]! : null;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}),
Effect.orElseSucceed(() => "origin"),
);

// The target remote's repository, or the OWNER/REPO that `gh repo set-default`
// recorded in its gh-resolved key (`base` means the remote itself).
const applyGhResolvedRepository = Effect.fn("applyGhResolvedRepository")(function* (
cwd: string,
remoteName: string | null,
context: {
remoteUrlKey: string | null;
repositoryNameWithOwner: string | null;
ownerLogin: string | null;
},
) {
if (!remoteName) return context;
const resolved = yield* readConfigValueNullable(cwd, `remote.${remoteName}.gh-resolved`);
const [owner, name, ...rest] = resolved?.trim().split("/") ?? [];
if (!owner || !name || rest.length > 0) return context;
const host = context.remoteUrlKey?.split("/")[0];
return {
remoteUrlKey: host ? `${host}/${owner}/${name}`.toLowerCase() : context.remoteUrlKey,
repositoryNameWithOwner: `${owner}/${name}`,
ownerLogin: owner,
};
});

// Returns [head remote, target remote]. Most branches track the target, so read it once.
const resolveHeadAndTargetContexts = (
cwd: string,
remoteName: string | null,
targetRemoteName: string | null,
) =>
remoteName === targetRemoteName
? resolveRemoteRepositoryContext(cwd, remoteName).pipe(
Effect.flatMap((head) =>
applyGhResolvedRepository(cwd, targetRemoteName, head).pipe(
Effect.map((target) => [head, target] as const),
),
),
)
: Effect.all(
[
resolveRemoteRepositoryContext(cwd, remoteName),
resolveRemoteRepositoryContext(cwd, "origin"),
resolveRemoteRepositoryContext(cwd, targetRemoteName).pipe(
Effect.flatMap((target) => applyGhResolvedRepository(cwd, targetRemoteName, target)),
),
],
{ concurrency: "unbounded" },
);
Expand All @@ -1422,7 +1477,12 @@ export const make = Effect.gen(function* () {
function* (cwd: string, branch: string, remoteNameOverride?: string) {
const remoteName =
remoteNameOverride ?? (yield* readConfigValueNullable(cwd, `branch.${branch}.remote`));
const [headRemote, targetRemote] = yield* resolveHeadAndOriginContexts(cwd, remoteName);
const targetRemoteName = yield* resolveTargetRemoteName(cwd);
const [headRemote, targetRemote] = yield* resolveHeadAndTargetContexts(
cwd,
remoteName,
targetRemoteName,
);
return {
remoteName,
headRemoteUrlKey:
Expand All @@ -1446,18 +1506,20 @@ export const make = Effect.gen(function* () {
const shouldProbeLocalBranchSelector =
headBranchFromUpstream.length === 0 || headBranch === details.branch;

const [remoteRepository, originRepository] = yield* resolveHeadAndOriginContexts(
const targetRemoteName = yield* resolveTargetRemoteName(cwd);
const [remoteRepository, targetRepository] = yield* resolveHeadAndTargetContexts(
cwd,
remoteName,
targetRemoteName,
);

const isCrossRepository =
remoteRepository.repositoryNameWithOwner !== null &&
originRepository.repositoryNameWithOwner !== null
targetRepository.repositoryNameWithOwner !== null
? remoteRepository.repositoryNameWithOwner.toLowerCase() !==
originRepository.repositoryNameWithOwner.toLowerCase()
targetRepository.repositoryNameWithOwner.toLowerCase()
: remoteName !== null &&
remoteName !== "origin" &&
remoteName !== targetRemoteName &&
remoteRepository.repositoryNameWithOwner !== null;

const ownerHeadSelector =
Expand All @@ -1467,7 +1529,7 @@ export const make = Effect.gen(function* () {
const remoteAliasHeadSelector =
remoteName && headBranch.length > 0 ? `${remoteName}:${headBranch}` : null;
const shouldProbeRemoteOwnedSelectors =
isCrossRepository || (remoteName !== null && remoteName !== "origin");
isCrossRepository || (remoteName !== null && remoteName !== targetRemoteName);

const headSelectors: string[] = [];
if (isCrossRepository && shouldProbeRemoteOwnedSelectors) {
Expand Down Expand Up @@ -1498,8 +1560,8 @@ export const make = Effect.gen(function* () {
remoteName,
headRemoteUrlKey:
remoteRepository.remoteUrlKey ??
(remoteName === null ? originRepository.remoteUrlKey : null),
targetRemoteUrlKey: originRepository.remoteUrlKey,
(remoteName === null ? targetRepository.remoteUrlKey : null),
targetRemoteUrlKey: targetRepository.remoteUrlKey,
headRepositoryNameWithOwner: remoteRepository.repositoryNameWithOwner,
headRepositoryOwnerLogin: remoteRepository.ownerLogin,
isCrossRepository,
Expand Down
Loading