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
23 changes: 20 additions & 3 deletions apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getRepoContext,
} from "../workspaces/utils/github/github";
import { execWithShellEnv } from "../workspaces/utils/shell-env";
import { resolveTrackingRemoteName } from "../workspaces/utils/upstream-ref";
import { isUpstreamMissingError } from "./git-utils";
import { assertRegisteredWorktree } from "./security/path-validation";
import {
Expand All @@ -31,15 +32,27 @@ async function hasUpstreamBranch(git: SimpleGit): Promise<boolean> {
}
}

async function getTrackingRemote(git: SimpleGit): Promise<string> {
try {
const upstream = (
await git.raw(["rev-parse", "--abbrev-ref", "@{upstream}"])
).trim();
return resolveTrackingRemoteName(upstream);
} catch {
return "origin";
}
}

async function fetchCurrentBranch(git: SimpleGit): Promise<void> {
const branch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim();
const remote = await getTrackingRemote(git);
try {
await git.fetch(["origin", branch]);
await git.fetch([remote, branch]);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
if (isUpstreamMissingError(message)) {
try {
await git.fetch(["origin"]);
await git.fetch([remote]);
} catch (fallbackError) {
const fallbackMessage =
fallbackError instanceof Error
Expand All @@ -62,9 +75,11 @@ async function fetchCurrentBranch(git: SimpleGit): Promise<void> {
async function pushWithSetUpstream({
git,
branch,
remote,
}: {
git: SimpleGit;
branch: string;
remote?: string;
}): Promise<void> {
const trimmedBranch = branch.trim();
if (!trimmedBranch || trimmedBranch === "HEAD") {
Expand All @@ -75,11 +90,13 @@ async function pushWithSetUpstream({
});
}

const targetRemote = remote ?? (await getTrackingRemote(git));

// Use HEAD refspec to avoid resolving the branch name as a local ref.
// This is more reliable for worktrees where upstream tracking isn't set yet.
await git.push([
"--set-upstream",
"origin",
targetRemote,
`HEAD:refs/heads/${trimmedBranch}`,
]);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export interface ParsedUpstreamRef {
remoteName: string;
branchName: string;
}
export {
type ParsedUpstreamRef,
parseUpstreamRef,
} from "../../workspaces/utils/upstream-ref";

export interface PullRequestCompareUrlInput {
baseRepoUrl: string;
Expand All @@ -27,20 +27,6 @@ export function normalizeGitHubRepoUrl(remoteUrl: string): string | null {
return `https://github.com/${match.groups.owner}/${match.groups.repo}`;
}

export function parseUpstreamRef(
upstreamRef: string,
): ParsedUpstreamRef | null {
const separatorIndex = upstreamRef.indexOf("/");
if (separatorIndex <= 0 || separatorIndex === upstreamRef.length - 1) {
return null;
}

return {
remoteName: upstreamRef.slice(0, separatorIndex),
branchName: upstreamRef.slice(separatorIndex + 1),
};
}

export function buildPullRequestCompareUrl({
baseRepoUrl,
baseBranch,
Expand Down
47 changes: 46 additions & 1 deletion apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import {
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createWorktree, getCurrentBranch, parsePrUrl } from "./git";
import {
branchExistsOnRemote,
createWorktree,
getCurrentBranch,
parsePrUrl,
} from "./git";

const TEST_DIR = join(
realpathSync(tmpdir()),
Expand Down Expand Up @@ -498,6 +503,46 @@ describe("getCurrentBranch", () => {
});
});

describe("branchExistsOnRemote", () => {
test("checks the requested remote instead of always origin", async () => {
const repoPath = createTestRepo("branch-exists-on-remote");
seedCommit(repoPath);

const originRemotePath = join(TEST_DIR, "branch-exists-origin.git");
const forkRemotePath = join(TEST_DIR, "branch-exists-fork.git");

execSync(`git init --bare "${originRemotePath}"`, { stdio: "ignore" });
execSync(`git init --bare "${forkRemotePath}"`, { stdio: "ignore" });

execSync(`git remote add origin "${originRemotePath}"`, {
cwd: repoPath,
stdio: "ignore",
});
execSync(`git remote add contributor "${forkRemotePath}"`, {
cwd: repoPath,
stdio: "ignore",
});
execSync(
"git push contributor HEAD:refs/heads/feature/fork-tracking-remote",
{
cwd: repoPath,
stdio: "ignore",
},
);

await expect(
branchExistsOnRemote(repoPath, "feature/fork-tracking-remote"),
).resolves.toEqual({ status: "not_found" });
await expect(
branchExistsOnRemote(
repoPath,
"feature/fork-tracking-remote",
"contributor",
),
).resolves.toEqual({ status: "exists" });
});
});

describe("parsePrUrl", () => {
test("parses canonical GitHub PR URL", () => {
expect(
Expand Down
29 changes: 24 additions & 5 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { StatusResult } from "simple-git";
import { runWithPostCheckoutHookTolerance } from "../../utils/git-hook-tolerance";
import { execGitWithShellPath, getSimpleGitWithShellPath } from "./git-client";
import { execWithShellEnv, getProcessEnvWithShellPath } from "./shell-env";
import { resolveTrackingRemoteName } from "./upstream-ref";

const execFileAsync = promisify(execFile);

Expand Down Expand Up @@ -1064,11 +1065,15 @@ const GIT_ERROR_PATTERNS = {
"does not appear to be a git repository",
"no such remote",
"repository not found",
"remote not found",
"remote origin not found",
],
} as const;

function categorizeGitError(errorMessage: string): BranchExistsResult {
function categorizeGitError(
errorMessage: string,
remoteName: string,
): BranchExistsResult {
const lowerMessage = errorMessage.toLowerCase();

if (GIT_ERROR_PATTERNS.network.some((p) => lowerMessage.includes(p))) {
Expand All @@ -1090,8 +1095,7 @@ function categorizeGitError(errorMessage: string): BranchExistsResult {
) {
return {
status: "error",
message:
"Remote 'origin' is not configured or the repository was not found.",
message: `Remote '${remoteName}' is not configured or the repository was not found.`,
};
}

Expand All @@ -1104,6 +1108,7 @@ function categorizeGitError(errorMessage: string): BranchExistsResult {
export async function branchExistsOnRemote(
worktreePath: string,
branchName: string,
remoteName = "origin",
): Promise<BranchExistsResult> {
const env = await getGitEnv();

Expand All @@ -1117,7 +1122,7 @@ export async function branchExistsOnRemote(
"ls-remote",
"--exit-code",
"--heads",
"origin",
remoteName,
branchName,
],
{ env, timeout: 30_000 },
Expand Down Expand Up @@ -1170,7 +1175,21 @@ export async function branchExistsOnRemote(
// For fatal errors (128) or other codes, categorize using stderr (preferred) or message
// stderr contains the actual git error; message may include wrapper text
const errorText = error.stderr || error.message || "";
return categorizeGitError(errorText);
return categorizeGitError(errorText, remoteName);
}
}

export async function getTrackingRemoteNameForWorktree(
worktreePath: string,
): Promise<string> {
try {
const { stdout } = await execGitWithShellPath(
["rev-parse", "--abbrev-ref", "@{upstream}"],
{ cwd: worktreePath },
);
return resolveTrackingRemoteName(stdout);
} catch {
return "origin";
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CheckItem, GitHubStatus } from "@superset/local-db";
import { branchExistsOnRemote } from "../git";
import { branchExistsOnRemote, getTrackingRemoteNameForWorktree } from "../git";
import { execGitWithShellPath } from "../git-client";
import { execWithShellEnv } from "../shell-env";
import {
Expand Down Expand Up @@ -32,19 +32,19 @@ export async function fetchGitHubPRStatus(
return null;
}

const [{ stdout: branchOutput }, { stdout: shaOutput }] = await Promise.all(
[
const [{ stdout: branchOutput }, { stdout: shaOutput }, trackingRemote] =
await Promise.all([
execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], {
cwd: worktreePath,
}),
execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }),
],
);
getTrackingRemoteNameForWorktree(worktreePath),
]);
const branchName = branchOutput.trim();
const headSha = shaOutput.trim();

const [branchCheck, prInfo, previewUrl] = await Promise.all([
branchExistsOnRemote(worktreePath, branchName),
branchExistsOnRemote(worktreePath, branchName, trackingRemote),
getPRForBranch(worktreePath, branchName, repoContext, headSha),
fetchPreviewDeploymentUrl(worktreePath, headSha, branchName, repoContext),
]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { describe, expect, test } from "bun:test";
import { parseUpstreamRef, resolveTrackingRemoteName } from "./upstream-ref";

describe("upstream-ref", () => {
test("parses upstream refs with slashes in branch names", () => {
expect(parseUpstreamRef("kitenite/kitenite/halved-position")).toEqual({
remoteName: "kitenite",
branchName: "kitenite/halved-position",
});
});

test("returns null for invalid upstream refs", () => {
expect(parseUpstreamRef("")).toBeNull();
expect(parseUpstreamRef("no-slash")).toBeNull();
expect(parseUpstreamRef("/leading-slash")).toBeNull();
expect(parseUpstreamRef("trailing/")).toBeNull();
});

test("resolves the tracking remote from upstream refs", () => {
expect(resolveTrackingRemoteName("contributor-fork/feature-branch")).toBe(
"contributor-fork",
);
expect(resolveTrackingRemoteName(" origin/main ")).toBe("origin");
});

test("falls back to origin when no upstream is configured", () => {
expect(resolveTrackingRemoteName("")).toBe("origin");
expect(resolveTrackingRemoteName(null)).toBe("origin");
expect(resolveTrackingRemoteName(undefined)).toBe("origin");
});
});
29 changes: 29 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
export interface ParsedUpstreamRef {
remoteName: string;
branchName: string;
}

export function parseUpstreamRef(
upstreamRef: string,
): ParsedUpstreamRef | null {
const separatorIndex = upstreamRef.indexOf("/");
if (separatorIndex <= 0 || separatorIndex === upstreamRef.length - 1) {
return null;
}

return {
remoteName: upstreamRef.slice(0, separatorIndex),
branchName: upstreamRef.slice(separatorIndex + 1),
};
}

export function resolveTrackingRemoteName(
upstreamRef: string | null | undefined,
fallback = "origin",
): string {
if (!upstreamRef) {
return fallback;
}

return parseUpstreamRef(upstreamRef.trim())?.remoteName ?? fallback;
}
Loading