Skip to content

Commit d3014c3

Browse files
only21milclaude
only21mil
and
claude
committed
fix(server): compare the only remote's host the way the provider's context does
The GH_REPO check compared the provider's locator host, which keeps an HTTPS port, with the remote's normalized key, which drops it. A remote at https://github.example.com:8443/contributor/demo with the same URL in GH_REPO was classed as a fork, and the open same-repository PR was missed. The remote's host now comes from the same source as the provider's context: the base URL detectSourceControlProviderFromGitRemoteUrl gives, read with `new URL`. An HTTPS remote keeps a non-default port, `:443` drops, and an SSH remote uses its hostname. The GH_REPO host is read the same way, so `host:443` matches the plain host and a different port stays a fork. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 91d1444 commit d3014c3

2 files changed

Lines changed: 93 additions & 19 deletions

File tree

‎apps/server/src/git/GitManager.test.ts‎

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4472,24 +4472,26 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
44724472
);
44734473

44744474
it.effect.each([
4475-
{ ghRepo: null },
4476-
{ ghRepo: "https://github.com/pingdotgg/codething-mvp?tab=readme#readme" },
4475+
{ remoteUrl: "git@github.com:pingdotgg/codething-mvp.git", ghRepo: null },
4476+
{
4477+
remoteUrl: "git@github.com:pingdotgg/codething-mvp.git",
4478+
ghRepo: "https://github.com/pingdotgg/codething-mvp?tab=readme#readme",
4479+
},
4480+
{
4481+
remoteUrl: "https://github.example.com:8443/pingdotgg/codething-mvp.git",
4482+
ghRepo: "https://github.example.com:8443/pingdotgg/codething-mvp",
4483+
},
44774484
] as const)(
4478-
"reuses the open same-repository PR when the only remote is not named origin (GH_REPO $ghRepo)",
4479-
({ ghRepo }) =>
4485+
"reuses the open same-repository PR when the only remote is not named origin (remote $remoteUrl, GH_REPO $ghRepo)",
4486+
({ remoteUrl, ghRepo }) =>
44804487
Effect.gen(function* () {
44814488
const repoDir = yield* makeTempDir("t3code-git-manager-");
44824489
yield* initRepo(repoDir);
44834490
yield* runGit(repoDir, ["checkout", "-b", "dev"]);
44844491
const remoteDir = yield* createBareRemote();
44854492
yield* runGit(repoDir, ["remote", "add", "fork", remoteDir]);
44864493
yield* runGit(repoDir, ["push", "-u", "fork", "dev"]);
4487-
yield* configureVisibleRemoteUrlWithLocalRewrite(
4488-
repoDir,
4489-
"fork",
4490-
"git@github.com:pingdotgg/codething-mvp.git",
4491-
remoteDir,
4492-
);
4494+
yield* configureVisibleRemoteUrlWithLocalRewrite(repoDir, "fork", remoteUrl, remoteDir);
44934495
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");
44944496

44954497
const { manager, ghCalls } = yield* makeManager({
@@ -4613,6 +4615,61 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
46134615
12_000,
46144616
);
46154617

4618+
it.effect.each([
4619+
{
4620+
remoteUrl: "https://github.example.com:8443/contributor/demo.git",
4621+
ghRepo: "https://github.example.com:9443/contributor/demo",
4622+
head: "contributor:dev",
4623+
},
4624+
{
4625+
remoteUrl: "https://github.example.com:8443/contributor/demo.git",
4626+
ghRepo: "github.example.com/contributor/demo",
4627+
head: "contributor:dev",
4628+
},
4629+
{
4630+
remoteUrl: "https://github.example.com:8443/contributor/demo.git",
4631+
ghRepo: "github.example.com:8443/contributor/demo",
4632+
head: "dev",
4633+
},
4634+
{
4635+
remoteUrl: "https://github.example.com/contributor/demo.git",
4636+
ghRepo: "github.example.com:443/contributor/demo",
4637+
head: "dev",
4638+
},
4639+
// An SSH port is not part of the host the provider uses.
4640+
{
4641+
remoteUrl: "ssh://git@github.example.com:2222/contributor/demo.git",
4642+
ghRepo: "github.example.com/contributor/demo",
4643+
head: "dev",
4644+
},
4645+
] as const)(
4646+
"creates the PR with head $head when the only remote is $remoteUrl and GH_REPO is $ghRepo",
4647+
({ remoteUrl, ghRepo, head }) =>
4648+
Effect.gen(function* () {
4649+
const repoDir = yield* makeTempDir("t3code-git-manager-");
4650+
yield* initRepo(repoDir);
4651+
yield* runGit(repoDir, ["checkout", "-b", "dev"]);
4652+
const forkDir = yield* createBareRemote();
4653+
yield* runGit(repoDir, ["remote", "add", "fork", forkDir]);
4654+
yield* runGit(repoDir, ["push", "-u", "fork", "dev"]);
4655+
yield* configureVisibleRemoteUrlWithLocalRewrite(repoDir, "fork", remoteUrl, forkDir);
4656+
NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n");
4657+
4658+
const { manager, ghCalls } = yield* makeManager({ environment: { GH_REPO: ghRepo } });
4659+
4660+
const result = yield* runStackedAction(manager, {
4661+
cwd: repoDir,
4662+
action: "commit_push_pr",
4663+
});
4664+
4665+
expect(result.pr.status).toBe("created");
4666+
expect(
4667+
ghCalls.some((call) => call.startsWith("pr create ") && call.includes(`--head ${head} `)),
4668+
).toBe(true);
4669+
}),
4670+
12_000,
4671+
);
4672+
46164673
it.effect(
46174674
"still treats a fork remote as cross-repository with an upstream remote but no origin",
46184675
() =>

‎apps/server/src/git/GitManager.ts‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,11 @@ function headBranchProbe(
618618
);
619619
}
620620

621+
/** A URL's host as `new URL` reads it: lowercased, any non-default port kept, `:443` dropped. */
622+
function urlHost(url: string): string | null {
623+
return URL.canParse(url) ? new URL(url).host : null;
624+
}
625+
621626
function appendUnique(values: string[], next: string | null | undefined): void {
622627
const trimmed = next?.trim() ?? "";
623628
if (trimmed.length === 0 || values.includes(trimmed)) {
@@ -1376,23 +1381,33 @@ export const make = Effect.gen(function* () {
13761381
const readConfigValueNullable = (cwd: string, key: string) =>
13771382
gitCore.readConfigValue(cwd, key).pipe(Effect.orElseSucceed(() => null));
13781383

1379-
// Whether the GitHub provider creates PRs in the only remote, `remoteUrlKey`. It picks GH_REPO
1380-
// when that parses, else the `OWNER/REPO` that `gh repo set-default` recorded (gh-resolved), else
1381-
// the only remote itself, as for gh-resolved `base`.
1384+
// Whether the GitHub provider creates PRs in the only remote. It picks GH_REPO when that
1385+
// parses, else the `OWNER/REPO` that `gh repo set-default` recorded (gh-resolved), else the
1386+
// only remote itself, as for gh-resolved `base`. The remote's host is the one the provider's
1387+
// context gets from the remote URL, so an HTTPS remote keeps a non-default port.
13821388
const targetsOnlyRemote = Effect.fn("targetsOnlyRemote")(function* (
13831389
cwd: string,
13841390
remoteName: string,
1385-
remoteUrlKey: string | null,
1391+
remote: { readonly remoteUrl: string | null; readonly repositoryNameWithOwner: string | null },
13861392
) {
1387-
const host = remoteUrlKey?.split("/")[0] ?? "";
1388-
const fromEnvironment = parseGitHubRepositorySelector(environment.GH_REPO ?? "", host);
1393+
const baseUrl =
1394+
remote.remoteUrl === null
1395+
? undefined
1396+
: detectSourceControlProviderFromGitRemoteUrl(remote.remoteUrl)?.baseUrl;
1397+
const host = baseUrl === undefined ? null : urlHost(baseUrl);
1398+
const repository = remote.repositoryNameWithOwner?.toLowerCase();
1399+
const fromEnvironment = parseGitHubRepositorySelector(environment.GH_REPO ?? "", host ?? "");
13891400
if (fromEnvironment !== null) {
13901401
const { host: targetHost, owner, name } = fromEnvironment;
1391-
return `${targetHost}/${owner}/${name}`.toLowerCase() === remoteUrlKey;
1402+
return (
1403+
host !== null &&
1404+
urlHost(`https://${targetHost}`) === host &&
1405+
`${owner}/${name}`.toLowerCase() === repository
1406+
);
13921407
}
13931408
const mark = yield* readConfigValueNullable(cwd, `remote.${remoteName}.gh-resolved`);
13941409
const [owner, name, ...rest] = (mark ?? "base").toLowerCase().split("/");
1395-
return !owner || !name || rest.length > 0 || `${host}/${owner}/${name}` === remoteUrlKey;
1410+
return !owner || !name || rest.length > 0 || `${owner}/${name}` === repository;
13961411
});
13971412

13981413
const resolveHostingProvider = Effect.fn("resolveHostingProvider")(function* (
@@ -1424,6 +1439,7 @@ export const make = Effect.gen(function* () {
14241439
) {
14251440
if (!remoteName) {
14261441
return {
1442+
remoteUrl: null,
14271443
remoteUrlKey: null,
14281444
repositoryNameWithOwner: null,
14291445
ownerLogin: null,
@@ -1447,6 +1463,7 @@ export const make = Effect.gen(function* () {
14471463
repositoryNameWithOwner = parseRepositoryNameWithOwnerFromRemoteUrl(remoteUrl, provider);
14481464
}
14491465
return {
1466+
remoteUrl,
14501467
remoteUrlKey: remoteUrl ? normalizeGitRemoteUrl(remoteUrl) : null,
14511468
repositoryNameWithOwner,
14521469
ownerLogin: parseRepositoryOwnerLogin(repositoryNameWithOwner),
@@ -1505,7 +1522,7 @@ export const make = Effect.gen(function* () {
15051522
),
15061523
Effect.orElseSucceed(() => 0),
15071524
)) === 1 &&
1508-
(yield* targetsOnlyRemote(cwd, remoteName, remoteRepository.remoteUrlKey));
1525+
(yield* targetsOnlyRemote(cwd, remoteName, remoteRepository));
15091526
const isForeignRemote = remoteName !== null && remoteName !== "origin" && !isOnlyRemote;
15101527

15111528
const isCrossRepository =

0 commit comments

Comments
 (0)