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
129 changes: 96 additions & 33 deletions apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
} from "../workspaces/utils/shell-env";
import { isUpstreamMissingError } from "./git-utils";
import { assertRegisteredWorktree } from "./security";
import {
buildPullRequestCompareUrl,
normalizeGitHubRepoUrl,
parseUpstreamRef,
} from "./utils/pull-request-url";
import { clearStatusCacheForWorktree } from "./utils/status-cache";

export { isUpstreamMissingError };
Expand Down Expand Up @@ -147,11 +152,6 @@ async function getTrackingBranchStatus(
}
}

function extractPRUrl(text: string): string | null {
const match = text.match(/https:\/\/github\.com\/\S+\/pull\/\d+/);
return match?.[0] ?? null;
}

async function findExistingOpenPRUrl(
worktreePath: string,
): Promise<string | null> {
Expand Down Expand Up @@ -242,22 +242,92 @@ async function findOpenPRByHeadCommit(
}
}

async function openPRInBrowser(
worktreePath: string,
prUrl: string,
): Promise<void> {
const ghRepoMetadataSchema = z.object({
url: z.string().url(),
isFork: z.boolean(),
parent: z
.object({
url: z.string().url(),
})
.nullable(),
defaultBranchRef: z.object({
name: z.string().min(1),
}),
});

async function getMergeBaseBranch(
git: ReturnType<typeof simpleGit>,
branch: string,
): Promise<string | null> {
try {
await execWithShellEnv("gh", ["pr", "view", prUrl, "--web"], {
cwd: worktreePath,
const configuredBaseBranch = await git.raw([
"config",
"--get",
`branch.${branch}.gh-merge-base`,
]);
return configuredBaseBranch.trim() || null;
} catch {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 6, 2026

Choose a reason for hiding this comment

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

P2: Empty catch block silently swallows errors from multiple git operations (upstream ref resolution and remote URL lookup). At minimum, log a warning with context so fork/upstream debugging is possible.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/changes/git-operations.ts, line 269:

<comment>Empty catch block silently swallows errors from multiple git operations (upstream ref resolution and remote URL lookup). At minimum, log a warning with context so fork/upstream debugging is possible.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>

<file context>
@@ -242,22 +242,92 @@ async function findOpenPRByHeadCommit(
+			`branch.${branch}.gh-merge-base`,
+		]);
+		return configuredBaseBranch.trim() || null;
+	} catch {
+		return null;
+	}
</file context>
Fix with Cubic

return null;
}
}

async function buildNewPullRequestUrl(
worktreePath: string,
git: ReturnType<typeof simpleGit>,
branch: string,
): Promise<string> {
const { stdout } = await execWithShellEnv(
"gh",
["repo", "view", "--json", "url,isFork,parent,defaultBranchRef"],
{ cwd: worktreePath },
);
const repoMetadata = ghRepoMetadataSchema.parse(JSON.parse(stdout));
const currentRepoUrl = normalizeGitHubRepoUrl(repoMetadata.url);
const baseRepoUrl = normalizeGitHubRepoUrl(
repoMetadata.isFork && repoMetadata.parent?.url
? repoMetadata.parent.url
: repoMetadata.url,
);

if (!currentRepoUrl || !baseRepoUrl) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "GitHub is not available for this workspace.",
});
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
console.warn(
`[git/openPRInBrowser] Failed to open PR URL ${prUrl}:`,
message,
);
// Opening in browser is best-effort; return URL either way.
}

const configuredBaseBranch = await getMergeBaseBranch(git, branch);
const baseBranch = configuredBaseBranch ?? repoMetadata.defaultBranchRef.name;
let headRepoOwner = currentRepoUrl.split("/").at(-2) ?? "";
let headBranch = branch;

try {
const upstreamRef = (
await git.raw(["rev-parse", "--abbrev-ref", "@{upstream}"])
).trim();
const parsedUpstreamRef = parseUpstreamRef(upstreamRef);

if (parsedUpstreamRef) {
headBranch = parsedUpstreamRef.branchName;
const upstreamRemoteUrl = await git.raw([
"remote",
"get-url",
parsedUpstreamRef.remoteName,
]);
headRepoOwner =
normalizeGitHubRepoUrl(upstreamRemoteUrl)?.split("/").at(-2) ??
headRepoOwner;
}
} catch {
// Fall back to the current repository owner and local branch name.
}

return buildPullRequestCompareUrl({
baseRepoUrl,
baseBranch,
headRepoOwner,
headBranch,
});
}

async function getGitWithShellPath(worktreePath: string) {
Expand Down Expand Up @@ -449,41 +519,34 @@ export const createGitOperationsRouter = () => {

const existingPRUrl = await findExistingOpenPRUrl(input.worktreePath);
if (existingPRUrl) {
await openPRInBrowser(input.worktreePath, existingPRUrl);
await fetchCurrentBranch(git);
clearStatusCacheForWorktree(input.worktreePath);
return { success: true, url: existingPRUrl };
}

let stdout = "";
try {
const result = await execWithShellEnv(
"gh",
["pr", "create", "--web", "--fill", "--head", branch],
{ cwd: input.worktreePath },
const url = await buildNewPullRequestUrl(
input.worktreePath,
git,
branch,
);
stdout = result.stdout;
await fetchCurrentBranch(git);
clearStatusCacheForWorktree(input.worktreePath);

return { success: true, url };
} catch (error) {
// If creation reports branch/tracking mismatch but an open PR exists,
// recover by opening that existing PR instead of failing.
const recoveredPRUrl = await findExistingOpenPRUrl(
input.worktreePath,
);
if (recoveredPRUrl) {
await openPRInBrowser(input.worktreePath, recoveredPRUrl);
await fetchCurrentBranch(git);
clearStatusCacheForWorktree(input.worktreePath);
return { success: true, url: recoveredPRUrl };
}
throw error;
}

const url =
(extractPRUrl(stdout) ?? stdout.trim()) || "https://github.com";
await fetchCurrentBranch(git);
clearStatusCacheForWorktree(input.worktreePath);

return { success: true, url };
},
),

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, expect, test } from "bun:test";
import {
buildPullRequestCompareUrl,
normalizeGitHubRepoUrl,
parseUpstreamRef,
} from "./pull-request-url";

describe("pull-request-url", () => {
test("normalizes GitHub remote URLs", () => {
expect(
normalizeGitHubRepoUrl("https://github.com/superset-sh/superset.git"),
).toBe("https://github.com/superset-sh/superset");
expect(normalizeGitHubRepoUrl("git@github.com:Kitenite/superset.git")).toBe(
"https://github.com/Kitenite/superset",
);
expect(
normalizeGitHubRepoUrl("ssh://git@github.com/Kitenite/superset.git"),
).toBe("https://github.com/Kitenite/superset");
});

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

test("builds compare URLs for fork branches", () => {
expect(
buildPullRequestCompareUrl({
baseRepoUrl: "https://github.com/superset-sh/superset.git",
baseBranch: "main",
headRepoOwner: "Kitenite",
headBranch: "kitenite/halved-position",
}),
).toBe(
"https://github.com/superset-sh/superset/compare/main...Kitenite:kitenite/halved-position?expand=1",
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
export interface ParsedUpstreamRef {
remoteName: string;
branchName: string;
}

export interface PullRequestCompareUrlInput {
baseRepoUrl: string;
baseBranch: string;
headRepoOwner: string;
headBranch: string;
}

export function normalizeGitHubRepoUrl(remoteUrl: string): string | null {
const trimmedRemoteUrl = remoteUrl.trim();
const match = [
/^git@github\.com:(?<owner>[^/]+)\/(?<repo>[^/]+?)(?:\.git)?$/,
/^ssh:\/\/git@github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+?)(?:\.git)?$/,
/^https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+?)(?:\.git)?\/?$/,
]
.map((pattern) => pattern.exec(trimmedRemoteUrl))
.find((result) => result?.groups?.owner && result.groups.repo);

if (!match?.groups?.owner || !match.groups.repo) {
return 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,
headRepoOwner,
headBranch,
}: PullRequestCompareUrlInput): string {
const normalizedBaseRepoUrl =
normalizeGitHubRepoUrl(baseRepoUrl) ??
baseRepoUrl.replace(/\.git$/, "").replace(/\/$/, "");

return `${normalizedBaseRepoUrl}/compare/${baseBranch}...${headRepoOwner}:${headBranch}?expand=1`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { ChangesHeader } from "./components/ChangesHeader";
import { CommitInput } from "./components/CommitInput";
import { CommitItem } from "./components/CommitItem";
import { FileList } from "./components/FileList";
import { getPRActionState } from "./utils";
import { getPRActionState, shouldAutoCreatePRAfterPublish } from "./utils";

interface ChangesViewProps {
onFileOpen?: (
Expand Down Expand Up @@ -364,8 +364,10 @@ export function ChangesView({ onFileOpen, isExpandedView }: ChangesViewProps) {
pullCount: status.pullCount,
isDefaultBranch,
});
const shouldAutoCreatePRAfterPublish =
hasGitHubRepo && !isDefaultBranch && !hasExistingPR;
const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 6, 2026

Choose a reason for hiding this comment

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

P1: Bug: hasGitHubRepo check was dropped when extracting shouldAutoCreatePRAfterPublish into a utility. The original inline logic was hasGitHubRepo && !isDefaultBranch && !hasExistingPR, but the new function only checks !hasExistingPR && !isDefaultBranch. This means workspaces without a GitHub repo will now incorrectly trigger auto-PR creation after a branch publish.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx, line 367:

<comment>Bug: `hasGitHubRepo` check was dropped when extracting `shouldAutoCreatePRAfterPublish` into a utility. The original inline logic was `hasGitHubRepo && !isDefaultBranch && !hasExistingPR`, but the new function only checks `!hasExistingPR && !isDefaultBranch`. This means workspaces without a GitHub repo will now incorrectly trigger auto-PR creation after a branch publish.</comment>

<file context>
@@ -364,8 +364,10 @@ export function ChangesView({ onFileOpen, isExpandedView }: ChangesViewProps) {
 	});
-	const shouldAutoCreatePRAfterPublish =
-		hasGitHubRepo && !isDefaultBranch && !hasExistingPR;
+	const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({
+		hasExistingPR,
+		isDefaultBranch,
</file context>
Suggested change
const shouldAutoCreatePR = shouldAutoCreatePRAfterPublish({
const shouldAutoCreatePR = hasGitHubRepo && shouldAutoCreatePRAfterPublish({
Fix with Cubic

hasExistingPR,
isDefaultBranch,
});

return (
<div className="flex flex-col flex-1 min-h-0">
Expand Down Expand Up @@ -398,7 +400,7 @@ export function ChangesView({ onFileOpen, isExpandedView }: ChangesViewProps) {
hasUpstream={status.hasUpstream}
hasExistingPR={hasExistingPR}
canCreatePR={prActionState.canCreatePR}
shouldAutoCreatePRAfterPublish={shouldAutoCreatePRAfterPublish}
shouldAutoCreatePRAfterPublish={shouldAutoCreatePR}
prUrl={prUrl}
onRefresh={handleRefresh}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { describe, expect, test } from "bun:test";
import { shouldAutoCreatePRAfterPublish } from "./auto-create-pr-after-publish";

describe("shouldAutoCreatePRAfterPublish", () => {
test("auto-creates after publishing on a non-default branch without an existing PR", () => {
expect(
shouldAutoCreatePRAfterPublish({
hasExistingPR: false,
isDefaultBranch: false,
}),
).toBe(true);
});

test("does not auto-create on the default branch", () => {
expect(
shouldAutoCreatePRAfterPublish({
hasExistingPR: false,
isDefaultBranch: true,
}),
).toBe(false);
});

test("does not auto-create when a PR already exists", () => {
expect(
shouldAutoCreatePRAfterPublish({
hasExistingPR: true,
isDefaultBranch: false,
}),
).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
interface AutoCreatePRAfterPublishInput {
hasExistingPR: boolean;
isDefaultBranch: boolean;
}

export function shouldAutoCreatePRAfterPublish({
hasExistingPR,
isDefaultBranch,
}: AutoCreatePRAfterPublishInput): boolean {
return !hasExistingPR && !isDefaultBranch;
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { shouldAutoCreatePRAfterPublish } from "./auto-create-pr-after-publish";
export { formatRelativeDate } from "./date";
export { getPRActionState } from "./pr-action-state";
export { getStatusColor, getStatusIndicator } from "./status";
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export function useCreateOrOpenPR({

void (async () => {
try {
await mutateAsync({ worktreePath });
const result = await mutateAsync({ worktreePath });
window.open(result.url, "_blank", "noopener,noreferrer");
toast.success("Opening GitHub...");
onSuccess?.();
return;
Expand All @@ -45,7 +46,11 @@ export function useCreateOrOpenPR({
}

try {
await mutateAsync({ worktreePath, allowOutOfDate: true });
const result = await mutateAsync({
worktreePath,
allowOutOfDate: true,
});
window.open(result.url, "_blank", "noopener,noreferrer");
toast.success("Opening GitHub...");
onSuccess?.();
} catch (retryError) {
Expand Down
Loading