Skip to content
Merged
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
147 changes: 147 additions & 0 deletions apps/server/src/git/Layers/GitCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,44 @@ it.layer(TestLayer)("git integration", (it) => {
}),
);

it.effect(
"computes ahead count against a non-origin remote-prefixed gh-merge-base candidate",
() =>
Effect.gen(function* () {
const remote = yield* makeTmpDir();
const source = yield* makeTmpDir();
const remoteName = "fork-seed";
yield* git(remote, ["init", "--bare"]);

yield* initRepoWithCommit(source);
const initialBranch = (yield* listGitBranches({ cwd: source })).branches.find(
(branch) => branch.current,
)!.name;
yield* git(source, ["remote", "add", remoteName, remote]);
yield* git(source, ["push", "-u", remoteName, initialBranch]);
yield* git(source, ["checkout", "-b", "feature/non-origin-merge-base"]);
yield* git(source, [
"config",
"branch.feature/non-origin-merge-base.gh-merge-base",
`${remoteName}/${initialBranch}`,
]);
yield* writeTextFile(
path.join(source, "feature.txt"),
`ahead of ${remoteName}/${initialBranch}\n`,
);
yield* git(source, ["add", "feature.txt"]);
yield* git(source, ["commit", "-m", "feature commit"]);
yield* git(source, ["branch", "-D", initialBranch]);

const core = yield* GitCore;
const details = yield* core.statusDetails(source);
expect(details.branch).toBe("feature/non-origin-merge-base");
expect(details.hasUpstream).toBe(false);
expect(details.aheadCount).toBe(1);
expect(details.behindCount).toBe(0);
}),
);

it.effect("skips push when no upstream is configured and branch is not ahead of base", () =>
Effect.gen(function* () {
const tmp = yield* makeTmpDir();
Expand Down Expand Up @@ -1447,6 +1485,31 @@ it.layer(TestLayer)("git integration", (it) => {
}),
);

it.effect("pushes with upstream setup to the only configured non-origin remote", () =>
Effect.gen(function* () {
const tmp = yield* makeTmpDir();
const remote = yield* makeTmpDir();
yield* git(tmp, ["init", "--initial-branch=main"]);
yield* git(tmp, ["config", "user.email", "test@test.com"]);
yield* git(tmp, ["config", "user.name", "Test"]);
yield* writeTextFile(path.join(tmp, "README.md"), "hello\n");
yield* git(tmp, ["add", "README.md"]);
yield* git(tmp, ["commit", "-m", "initial"]);
yield* git(remote, ["init", "--bare"]);
yield* git(tmp, ["remote", "add", "fork", remote]);
yield* git(tmp, ["checkout", "-b", "feature/fork-only"]);

const core = yield* GitCore;
const pushed = yield* core.pushCurrentBranch(tmp, null);
expect(pushed.status).toBe("pushed");
expect(pushed.setUpstream).toBe(true);
expect(pushed.upstreamBranch).toBe("fork/feature/fork-only");
expect(yield* git(tmp, ["rev-parse", "--abbrev-ref", "@{upstream}"])).toBe(
"fork/feature/fork-only",
);
}),
);

it.effect(
"pushes with upstream setup when comparable base exists but remote branch is missing",
() =>
Expand Down Expand Up @@ -1483,6 +1546,90 @@ it.layer(TestLayer)("git integration", (it) => {
}),
);

it.effect("prefers branch pushRemote over origin when setting upstream", () =>
Effect.gen(function* () {
const tmp = yield* makeTmpDir();
const origin = yield* makeTmpDir();
const fork = yield* makeTmpDir();
yield* git(origin, ["init", "--bare"]);
yield* git(fork, ["init", "--bare"]);

yield* initRepoWithCommit(tmp);
const initialBranch = (yield* listGitBranches({ cwd: tmp })).branches.find(
(branch) => branch.current,
)!.name;
yield* git(tmp, ["remote", "add", "origin", origin]);
yield* git(tmp, ["remote", "add", "fork", fork]);
yield* git(tmp, ["push", "-u", "origin", initialBranch]);

const featureBranch = "feature/push-remote";
yield* git(tmp, ["checkout", "-b", featureBranch]);
yield* git(tmp, ["config", `branch.${featureBranch}.pushRemote`, "fork"]);
yield* writeTextFile(path.join(tmp, "feature.txt"), "push to fork\n");
yield* git(tmp, ["add", "feature.txt"]);
yield* git(tmp, ["commit", "-m", "feature commit"]);

const core = yield* GitCore;
const pushed = yield* core.pushCurrentBranch(tmp, null);
expect(pushed.status).toBe("pushed");
expect(pushed.setUpstream).toBe(true);
expect(pushed.upstreamBranch).toBe(`fork/${featureBranch}`);
expect(yield* git(tmp, ["rev-parse", "--abbrev-ref", "@{upstream}"])).toBe(
`fork/${featureBranch}`,
);
expect(yield* git(tmp, ["ls-remote", "--heads", "fork", featureBranch])).toContain(
featureBranch,
);
}),
);

it.effect(
"pushes renamed PR worktree branches to their tracked upstream branch even when push.default is current",
() =>
Effect.gen(function* () {
const tmp = yield* makeTmpDir();
const fork = yield* makeTmpDir();
yield* git(fork, ["init", "--bare"]);

const { initialBranch } = yield* initRepoWithCommit(tmp);
yield* git(tmp, ["remote", "add", "jasonLaster", fork]);
yield* git(tmp, ["checkout", "-b", "statemachine"]);
yield* writeTextFile(path.join(tmp, "fork.txt"), "fork branch\n");
yield* git(tmp, ["add", "fork.txt"]);
yield* git(tmp, ["commit", "-m", "fork branch"]);
yield* git(tmp, ["push", "-u", "jasonLaster", "statemachine"]);
yield* git(tmp, ["checkout", initialBranch]);
yield* git(tmp, ["branch", "-D", "statemachine"]);
yield* git(tmp, [
"checkout",
"-b",
"t3code/pr-488/statemachine",
"--track",
"jasonLaster/statemachine",
]);
yield* git(tmp, ["config", "push.default", "current"]);
yield* writeTextFile(path.join(tmp, "fork.txt"), "updated fork branch\n");
yield* git(tmp, ["add", "fork.txt"]);
yield* git(tmp, ["commit", "-m", "update reviewed PR branch"]);

const core = yield* GitCore;
const pushed = yield* core.pushCurrentBranch(tmp, null);

expect(pushed.status).toBe("pushed");
expect(pushed.setUpstream).toBe(false);
expect(pushed.upstreamBranch).toBe("jasonLaster/statemachine");
expect(yield* git(tmp, ["rev-parse", "--abbrev-ref", "@{upstream}"])).toBe(
"jasonLaster/statemachine",
);
expect(
yield* git(tmp, ["ls-remote", "--heads", "jasonLaster", "statemachine"]),
).toContain("statemachine");
expect(
yield* git(tmp, ["ls-remote", "--heads", "jasonLaster", "t3code/pr-488/statemachine"]),
).toBe("");
}),
);

it.effect("includes command context when worktree removal fails", () =>
Effect.gen(function* () {
const tmp = yield* makeTmpDir();
Expand Down
101 changes: 85 additions & 16 deletions apps/server/src/git/Layers/GitCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ function commandLabel(args: readonly string[]): string {
return `git ${args.join(" ")}`;
}

function parseDefaultBranchFromRemoteHeadRef(value: string): string | null {
function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null {
const trimmed = value.trim();
const prefix = "refs/remotes/origin/";
const prefix = `refs/remotes/${remoteName}/`;
if (!trimmed.startsWith(prefix)) {
return null;
}
Expand Down Expand Up @@ -417,29 +417,33 @@ const makeGitCore = Effect.gen(function* () {
yield* fetchUpstreamRef(cwd, upstream);
});

const resolveDefaultBranchName = (cwd: string): Effect.Effect<string | null, GitCommandError> =>
const resolveDefaultBranchName = (
cwd: string,
remoteName: string,
): Effect.Effect<string | null, GitCommandError> =>
executeGit(
"GitCore.resolveDefaultBranchName",
cwd,
["symbolic-ref", "refs/remotes/origin/HEAD"],
["symbolic-ref", `refs/remotes/${remoteName}/HEAD`],
{ allowNonZeroExit: true },
).pipe(
Effect.map((result) => {
if (result.code !== 0) {
return null;
}
return parseDefaultBranchFromRemoteHeadRef(result.stdout);
return parseDefaultBranchFromRemoteHeadRef(result.stdout, remoteName);
}),
);

const remoteBranchExists = (
cwd: string,
remoteName: string,
branch: string,
): Effect.Effect<boolean, GitCommandError> =>
executeGit(
"GitCore.remoteBranchExists",
cwd,
["show-ref", "--verify", "--quiet", `refs/remotes/origin/${branch}`],
["show-ref", "--verify", "--quiet", `refs/remotes/${remoteName}/${branch}`],
{
allowNonZeroExit: true,
},
Expand Down Expand Up @@ -473,6 +477,34 @@ const makeGitCore = Effect.gen(function* () {
);
});

const resolvePushRemoteName = (
cwd: string,
branch: string,
): Effect.Effect<string | null, GitCommandError> =>
Effect.gen(function* () {
const branchPushRemote = yield* runGitStdout(
"GitCore.resolvePushRemoteName.branchPushRemote",
cwd,
["config", "--get", `branch.${branch}.pushRemote`],
true,
).pipe(Effect.map((stdout) => stdout.trim()));
if (branchPushRemote.length > 0) {
return branchPushRemote;
}

const pushDefaultRemote = yield* runGitStdout(
"GitCore.resolvePushRemoteName.remotePushDefault",
cwd,
["config", "--get", "remote.pushDefault"],
true,
).pipe(Effect.map((stdout) => stdout.trim()));
if (pushDefaultRemote.length > 0) {
return pushDefaultRemote;
}

return yield* resolvePrimaryRemoteName(cwd).pipe(Effect.catch(() => Effect.succeed(null)));
});

const ensureRemote: GitCoreShape["ensureRemote"] = (input) =>
Effect.gen(function* () {
const preferredName = sanitizeRemoteName(input.preferredName);
Expand Down Expand Up @@ -517,7 +549,11 @@ const makeGitCore = Effect.gen(function* () {
true,
).pipe(Effect.map((stdout) => stdout.trim()));

const defaultBranch = yield* resolveDefaultBranchName(cwd);
const primaryRemoteName = yield* resolvePrimaryRemoteName(cwd).pipe(
Effect.catch(() => Effect.succeed(null)),
);
const defaultBranch =
primaryRemoteName === null ? null : yield* resolveDefaultBranchName(cwd, primaryRemoteName);
const candidates = [
configuredBaseBranch.length > 0 ? configuredBaseBranch : null,
defaultBranch,
Expand All @@ -529,9 +565,13 @@ const makeGitCore = Effect.gen(function* () {
continue;
}

const remotePrefix =
primaryRemoteName && primaryRemoteName !== "origin" ? `${primaryRemoteName}/` : null;
const normalizedCandidate = candidate.startsWith("origin/")
? candidate.slice("origin/".length)
: candidate;
: remotePrefix && candidate.startsWith(remotePrefix)
? candidate.slice(remotePrefix.length)
: candidate;
if (normalizedCandidate.length === 0 || normalizedCandidate === branch) {
continue;
}
Expand All @@ -540,8 +580,11 @@ const makeGitCore = Effect.gen(function* () {
return normalizedCandidate;
}

if (yield* remoteBranchExists(cwd, normalizedCandidate)) {
return `origin/${normalizedCandidate}`;
if (
primaryRemoteName &&
(yield* remoteBranchExists(cwd, primaryRemoteName, normalizedCandidate))
) {
return `${primaryRemoteName}/${normalizedCandidate}`;
}
}

Expand Down Expand Up @@ -792,17 +835,17 @@ const makeGitCore = Effect.gen(function* () {
Effect.catch(() => Effect.succeed(null)),
);
if (comparableBaseBranch) {
const hasOriginRemote = yield* originRemoteExists(cwd).pipe(
Effect.catch(() => Effect.succeed(false)),
const publishRemoteName = yield* resolvePushRemoteName(cwd, branch).pipe(
Effect.catch(() => Effect.succeed(null)),
);
if (!hasOriginRemote) {
if (!publishRemoteName) {
return {
status: "skipped_up_to_date" as const,
branch,
};
}

const hasRemoteBranch = yield* remoteBranchExists(cwd, branch).pipe(
const hasRemoteBranch = yield* remoteBranchExists(cwd, publishRemoteName, branch).pipe(
Effect.catch(() => Effect.succeed(false)),
);
if (hasRemoteBranch) {
Expand All @@ -815,20 +858,46 @@ const makeGitCore = Effect.gen(function* () {
}

if (!details.hasUpstream) {
const publishRemoteName = yield* resolvePushRemoteName(cwd, branch);
if (!publishRemoteName) {
return yield* createGitCommandError(
"GitCore.pushCurrentBranch",
cwd,
["push"],
"Cannot push because no git remote is configured for this repository.",
);
}
yield* runGit("GitCore.pushCurrentBranch.pushWithUpstream", cwd, [
"push",
"-u",
"origin",
publishRemoteName,
branch,
]);
return {
status: "pushed" as const,
branch,
upstreamBranch: `origin/${branch}`,
upstreamBranch: `${publishRemoteName}/${branch}`,
setUpstream: true,
};
}

const currentUpstream = yield* resolveCurrentUpstream(cwd).pipe(
Effect.catch(() => Effect.succeed(null)),
);
if (currentUpstream) {
yield* runGit("GitCore.pushCurrentBranch.pushUpstream", cwd, [
"push",
currentUpstream.remoteName,
`HEAD:${currentUpstream.upstreamBranch}`,
]);
return {
status: "pushed" as const,
branch,
upstreamBranch: currentUpstream.upstreamRef,
setUpstream: false,
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slashed remote names break new explicit push path

Low Severity

The new explicit push path at pushCurrentBranch uses resolveCurrentUpstream, which parses the remote name by splitting the upstream ref on the first /. For remotes with slashes in their names (e.g., my-org/upstream), my-org/upstream/feature is incorrectly split into remoteName=my-org and upstreamBranch=upstream/feature. The old code simply ran git push (which git resolves correctly); the new code constructs git push my-org HEAD:upstream/feature, which fails because no remote named my-org exists. The BranchToolbar tests explicitly exercise slashed remote names, confirming they're a supported configuration.

Additional Locations (1)
Fix in Cursor Fix in Web

}

yield* runGit("GitCore.pushCurrentBranch.push", cwd, ["push"]);
return {
status: "pushed" as const,
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/git/Layers/GitHubCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ const makeGitHubCli = Effect.sync(() => {
"pr",
"list",
"--head",
input.headBranch,
input.headSelector,
"--state",
"open",
"--limit",
Expand Down Expand Up @@ -250,7 +250,7 @@ const makeGitHubCli = Effect.sync(() => {
"--base",
input.baseBranch,
"--head",
input.headBranch,
input.headSelector,
"--title",
input.title,
"--body-file",
Expand Down
Loading