diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts index b514633654a..658b0b8b102 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts @@ -2,6 +2,7 @@ import { TRPCError } from "@trpc/server"; import type { RemoteWithRefs, SimpleGit } from "simple-git"; import { z } from "zod"; import { publicProcedure, router } from "../.."; +import { getCurrentBranch } from "../workspaces/utils/git"; import { execGitWithShellPath, getSimpleGitWithShellPath, @@ -61,10 +62,16 @@ async function getTrackingRemote(git: SimpleGit): Promise { return trackingRef?.remoteName ?? "origin"; } -async function fetchCurrentBranch(git: SimpleGit): Promise { - const localBranch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim(); +async function fetchCurrentBranch( + git: SimpleGit, + worktreePath: string, +): Promise { + const localBranch = await getCurrentBranch(worktreePath); const trackingRef = await getTrackingRef(git); const branch = trackingRef?.branchName ?? localBranch; + if (!branch) { + return; + } const remote = trackingRef?.remoteName ?? resolveTrackingRemoteName(null); try { await git.fetch([remote, branch]); @@ -498,7 +505,14 @@ export const createGitOperationsRouter = () => { const hasUpstream = await hasUpstreamBranch(git); if (input.setUpstream && !hasUpstream) { - const localBranch = await git.revparse(["--abbrev-ref", "HEAD"]); + const localBranch = await getCurrentBranch(input.worktreePath); + if (!localBranch) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Cannot push from detached HEAD. Please checkout a branch and try again.", + }); + } await pushWithResolvedUpstream({ git, worktreePath: input.worktreePath, @@ -511,7 +525,14 @@ export const createGitOperationsRouter = () => { const message = error instanceof Error ? error.message : String(error); if (shouldRetryPushWithUpstream(message)) { - const localBranch = await git.revparse(["--abbrev-ref", "HEAD"]); + const localBranch = await getCurrentBranch(input.worktreePath); + if (!localBranch) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Cannot push from detached HEAD. Please checkout a branch and try again.", + }); + } await pushWithResolvedUpstream({ git, worktreePath: input.worktreePath, @@ -522,7 +543,8 @@ export const createGitOperationsRouter = () => { } } } - await fetchCurrentBranch(git); + + await fetchCurrentBranch(git, input.worktreePath); clearStatusCacheForWorktree(input.worktreePath); return { success: true }; }), @@ -569,20 +591,28 @@ export const createGitOperationsRouter = () => { const message = error instanceof Error ? error.message : String(error); if (isUpstreamMissingError(message)) { - const localBranch = await git.revparse(["--abbrev-ref", "HEAD"]); + const localBranch = await getCurrentBranch(input.worktreePath); + if (!localBranch) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Cannot push from detached HEAD. Please checkout a branch and try again.", + }); + } await pushWithResolvedUpstream({ git, worktreePath: input.worktreePath, localBranch, }); - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearStatusCacheForWorktree(input.worktreePath); return { success: true }; } throw error; } + await git.push(); - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearStatusCacheForWorktree(input.worktreePath); return { success: true }; }), @@ -592,7 +622,7 @@ export const createGitOperationsRouter = () => { .mutation(async ({ input }): Promise<{ success: boolean }> => { assertRegisteredWorktree(input.worktreePath); const git = await getGitWithShellPath(input.worktreePath); - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearStatusCacheForWorktree(input.worktreePath); return { success: true }; }), @@ -609,7 +639,15 @@ export const createGitOperationsRouter = () => { assertRegisteredWorktree(input.worktreePath); const git = await getGitWithShellPath(input.worktreePath); - const branch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim(); + const branch = await getCurrentBranch(input.worktreePath); + if (!branch) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Cannot create a pull request from detached HEAD. Please checkout a branch and try again.", + }); + } + const trackingStatus = await getTrackingBranchStatus(git); const hasUpstream = trackingStatus.hasUpstream; const isBehindUpstream = @@ -664,7 +702,7 @@ export const createGitOperationsRouter = () => { const existingPRUrl = await findExistingOpenPRUrl(input.worktreePath); if (existingPRUrl) { - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearWorktreeStatusCaches(input.worktreePath); return { success: true, url: existingPRUrl }; } @@ -675,7 +713,7 @@ export const createGitOperationsRouter = () => { git, branch, ); - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearWorktreeStatusCaches(input.worktreePath); return { success: true, url }; @@ -686,7 +724,7 @@ export const createGitOperationsRouter = () => { input.worktreePath, ); if (recoveredPRUrl) { - await fetchCurrentBranch(git); + await fetchCurrentBranch(git, input.worktreePath); clearWorktreeStatusCaches(input.worktreePath); return { success: true, url: recoveredPRUrl }; } diff --git a/apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts b/apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts index 6fac9cfc5b9..230ea918154 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts @@ -1,4 +1,5 @@ import { runWithPostCheckoutHookTolerance } from "../../utils/git-hook-tolerance"; +import { getCurrentBranch } from "../../workspaces/utils/git"; import { getSimpleGitWithShellPath } from "../../workspaces/utils/git-client"; import { assertRegisteredWorktree, @@ -29,8 +30,7 @@ async function isCurrentBranch({ expectedBranch: string; }): Promise { try { - const git = await getGitWithShellPath(worktreePath); - const currentBranch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim(); + const currentBranch = await getCurrentBranch(worktreePath); return currentBranch === expectedBranch; } catch { return false; diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts new file mode 100644 index 00000000000..c4e115ccd20 --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts @@ -0,0 +1,162 @@ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; + +const getCurrentBranchMock = mock( + (async () => null) as (...args: unknown[]) => Promise, +); +const execGitWithShellPathMock = mock((async () => ({ + stdout: "", + stderr: "", +})) as (...args: unknown[]) => Promise<{ stdout: string; stderr: string }>); +const getRepoContextMock = mock( + (async () => null) as (...args: unknown[]) => Promise<{ + isFork: boolean; + repoUrl: string; + upstreamUrl: string; + } | null>, +); +const getPRForBranchMock = mock( + (async () => null) as (...args: unknown[]) => Promise<{ + number: number; + state: "open" | "closed" | "merged"; + } | null>, +); +const getPullRequestRepoArgsMock = mock(() => [] as string[]); +const execWithShellEnvMock = mock( + (async () => undefined) as (...args: unknown[]) => Promise, +); +const isNoPullRequestFoundMessageMock = mock(() => false); +const clearWorktreeStatusCachesMock = mock(() => undefined); + +mock.module("../../workspaces/utils/git", () => ({ + getCurrentBranch: getCurrentBranchMock, +})); + +mock.module("../../workspaces/utils/git-client", () => ({ + execGitWithShellPath: execGitWithShellPathMock, +})); + +mock.module("../../workspaces/utils/github", () => ({ + getPRForBranch: getPRForBranchMock, + getPullRequestRepoArgs: getPullRequestRepoArgsMock, + getRepoContext: getRepoContextMock, +})); + +mock.module("../../workspaces/utils/shell-env", () => ({ + execWithShellEnv: execWithShellEnvMock, +})); + +mock.module("../git-utils", () => ({ + isNoPullRequestFoundMessage: isNoPullRequestFoundMessageMock, +})); + +mock.module("./worktree-status-caches", () => ({ + clearWorktreeStatusCaches: clearWorktreeStatusCachesMock, +})); + +const { mergePullRequest } = await import("./merge-pull-request"); + +describe("mergePullRequest", () => { + beforeEach(() => { + getCurrentBranchMock.mockReset(); + getCurrentBranchMock.mockResolvedValue(null); + execGitWithShellPathMock.mockReset(); + execGitWithShellPathMock.mockResolvedValue({ + stdout: "abc123\n", + stderr: "", + }); + getRepoContextMock.mockReset(); + getRepoContextMock.mockResolvedValue({ + isFork: false, + repoUrl: "https://github.com/superset-sh/superset", + upstreamUrl: "https://github.com/superset-sh/superset", + }); + getPRForBranchMock.mockReset(); + getPRForBranchMock.mockResolvedValue(null); + getPullRequestRepoArgsMock.mockReset(); + getPullRequestRepoArgsMock.mockReturnValue([]); + execWithShellEnvMock.mockReset(); + execWithShellEnvMock.mockResolvedValue(undefined); + isNoPullRequestFoundMessageMock.mockReset(); + isNoPullRequestFoundMessageMock.mockReturnValue(false); + clearWorktreeStatusCachesMock.mockReset(); + }); + + test("falls back to legacy gh merge when HEAD is detached", async () => { + const result = await mergePullRequest({ + worktreePath: "/tmp/detached-worktree", + strategy: "squash", + }); + + expect(getRepoContextMock).toHaveBeenCalledWith("/tmp/detached-worktree"); + expect(getCurrentBranchMock).toHaveBeenCalledWith("/tmp/detached-worktree"); + expect(execGitWithShellPathMock).not.toHaveBeenCalled(); + expect(getPRForBranchMock).not.toHaveBeenCalled(); + expect(execWithShellEnvMock).toHaveBeenCalledWith( + "gh", + ["pr", "merge", "--squash"], + { cwd: "/tmp/detached-worktree" }, + ); + expect(clearWorktreeStatusCachesMock).toHaveBeenCalledWith( + "/tmp/detached-worktree", + ); + expect(result.success).toBe(true); + expect(Number.isNaN(Date.parse(result.mergedAt))).toBe(false); + }); + + test("resolves the PR by branch when HEAD has no commit yet", async () => { + getCurrentBranchMock.mockResolvedValue("feature/unborn"); + execGitWithShellPathMock.mockRejectedValueOnce( + new Error("fatal: ambiguous argument 'HEAD'"), + ); + getPRForBranchMock.mockResolvedValue({ + number: 42, + state: "open", + }); + + const result = await mergePullRequest({ + worktreePath: "/tmp/unborn-worktree", + strategy: "rebase", + }); + + expect(execWithShellEnvMock).toHaveBeenCalledWith( + "gh", + ["pr", "merge", "42", "--rebase"], + { cwd: "/tmp/unborn-worktree" }, + ); + expect(getPRForBranchMock).toHaveBeenCalledWith( + "/tmp/unborn-worktree", + "feature/unborn", + { + isFork: false, + repoUrl: "https://github.com/superset-sh/superset", + upstreamUrl: "https://github.com/superset-sh/superset", + }, + undefined, + ); + expect(result.success).toBe(true); + }); + + test("falls back to legacy merge on unexpected HEAD lookup failures", async () => { + getCurrentBranchMock.mockResolvedValue("feature/branch"); + execGitWithShellPathMock.mockRejectedValueOnce( + new Error("fatal: permission denied"), + ); + + const result = await mergePullRequest({ + worktreePath: "/tmp/broken-worktree", + strategy: "merge", + }); + + expect(getPRForBranchMock).not.toHaveBeenCalled(); + expect(execWithShellEnvMock).toHaveBeenCalledWith( + "gh", + ["pr", "merge", "--merge"], + { cwd: "/tmp/broken-worktree" }, + ); + expect(result.success).toBe(true); + }); +}); + +afterAll(() => { + mock.restore(); +}); diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts index 0d563a467ee..0fd984db33c 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts @@ -1,3 +1,7 @@ +import { + getCurrentBranch, + isUnbornHeadError, +} from "../../workspaces/utils/git"; import { execGitWithShellPath } from "../../workspaces/utils/git-client"; import { getPRForBranch, @@ -36,15 +40,20 @@ export async function mergePullRequest({ let pr: Awaited> = null; try { - const [{ stdout: branchOutput }, { stdout: headOutput }] = - await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: worktreePath, - }), - execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), - ]); - const localBranch = branchOutput.trim(); - const headSha = headOutput.trim(); + const localBranch = await getCurrentBranch(worktreePath); + if (!localBranch) { + return runMerge(legacyMergeArgs); + } + const { stdout: headOutput } = await execGitWithShellPath( + ["rev-parse", "HEAD"], + { cwd: worktreePath }, + ).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; + }); + const headSha = headOutput.trim() || undefined; pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha); } catch (error) { diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts index 4e0ab490faf..6ec9cabb5e6 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/status.ts @@ -199,7 +199,12 @@ export const createStatusProcedures = () => { .mutation(({ input }) => { const { workspaceId, branch } = input; - if (!branch || branch === "HEAD") { + if ( + !branch || + branch === "HEAD" || + branch.startsWith("[") || + branch.includes(" ") + ) { return { success: false as const, reason: "invalid-branch" as const }; } diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts index e92716919e1..24db36461ad 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts @@ -15,6 +15,8 @@ import { createWorktree, getCurrentBranch, hasUnpushedCommits, + isUnbornHeadError, + parsePorcelainStatusV2, parsePrUrl, } from "./git"; @@ -504,6 +506,129 @@ describe("getCurrentBranch", () => { }); }); +describe("parsePorcelainStatusV2", () => { + test("parses branch headers for unborn branches with upstream tracking", () => { + const status = parsePorcelainStatusV2( + [ + "# branch.oid (initial)", + "# branch.head feature/gone-fix", + "# branch.upstream origin/feature/gone-fix", + "# branch.ab +0 -0", + "? path with spaces.txt", + ].join("\0"), + ); + + expect(status.current).toBe("feature/gone-fix"); + expect(status.tracking).toBe("origin/feature/gone-fix"); + expect(status.ahead).toBe(0); + expect(status.behind).toBe(0); + expect(status.not_added).toEqual(["path with spaces.txt"]); + expect(status.files).toEqual([ + { + path: "path with spaces.txt", + from: "path with spaces.txt", + index: "?", + working_dir: "?", + }, + ]); + }); + + test("parses rename and modified entries from porcelain v2 output", () => { + const status = parsePorcelainStatusV2( + [ + "# branch.oid abcdef1234567890", + "# branch.head feature/rename", + "# branch.upstream origin/feature/rename", + "# branch.ab +2 -3", + "2 R. N... 100644 100644 100644 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 R100 new.txt", + "old.txt", + "1 .M N... 100644 100644 100644 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 edited.txt", + ].join("\0"), + ); + + expect(status.current).toBe("feature/rename"); + expect(status.tracking).toBe("origin/feature/rename"); + expect(status.ahead).toBe(2); + expect(status.behind).toBe(3); + expect(status.renamed).toEqual([{ from: "old.txt", to: "new.txt" }]); + expect(status.files).toEqual([ + { + path: "new.txt", + from: "old.txt", + index: "R", + working_dir: " ", + }, + { + path: "edited.txt", + from: "edited.txt", + index: " ", + working_dir: "M", + }, + ]); + expect(status.staged).toEqual(["new.txt"]); + expect(status.modified).toEqual(["edited.txt"]); + }); + + test("parses unmerged conflict entries from porcelain v2 output", () => { + const status = parsePorcelainStatusV2( + [ + "# branch.oid abcdef1234567890", + "# branch.head main", + "u UU N... 100644 100644 100644 100644 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 conflict.txt", + ].join("\0"), + ); + + expect(status.current).toBe("main"); + expect(status.conflicted).toEqual(["conflict.txt"]); + expect(status.files).toEqual([ + { + path: "conflict.txt", + from: "conflict.txt", + index: "U", + working_dir: "U", + }, + ]); + }); + + test("marks all porcelain v2 unmerged records as conflicted", () => { + const status = parsePorcelainStatusV2( + [ + "# branch.oid abcdef1234567890", + "# branch.head main", + "u AA N... 100644 100644 100644 100644 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 43dd47ea691c90a5fa7827892c70241913351963 both-added.txt", + ].join("\0"), + ); + + expect(status.conflicted).toEqual(["both-added.txt"]); + expect(status.files).toEqual([ + { + path: "both-added.txt", + from: "both-added.txt", + index: "A", + working_dir: "A", + }, + ]); + }); +}); + +describe("isUnbornHeadError", () => { + test("matches the standard unborn HEAD rev-parse failure", () => { + expect( + isUnbornHeadError( + new Error( + "fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.", + ), + ), + ).toBe(true); + }); + + test("does not hide unrelated git failures", () => { + expect(isUnbornHeadError(new Error("fatal: not a git repository"))).toBe( + false, + ); + }); +}); + describe("branchExistsOnRemote", () => { test("checks the requested remote instead of always origin", async () => { const repoPath = createTestRepo("branch-exists-on-remote"); diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index 0df864005b5..84033314620 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -25,6 +25,14 @@ export class NotGitRepoError extends Error { } } +const UNBORN_HEAD_ERROR_PATTERNS = [ + "ambiguous argument 'head'", + "unknown revision or path not in the working tree", + "bad revision 'head'", + "not a valid object name head", + "needed a single revision", +]; + /** * Error thrown by execFile when the command fails. * `code` can be a number (exit code) or string (spawn error like "ENOENT"). @@ -45,6 +53,21 @@ function isExecFileException(error: unknown): error is ExecFileException { ); } +export function isUnbornHeadError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + + const stderr = + isExecFileException(error) && typeof error.stderr === "string" + ? error.stderr + : ""; + const message = `${error.message}\n${stderr}`.toLowerCase(); + return UNBORN_HEAD_ERROR_PATTERNS.some((pattern) => + message.includes(pattern), + ); +} + async function isWorktreeRegistered({ mainRepoPath, worktreePath, @@ -135,25 +158,25 @@ export async function getStatusNoLock(repoPath: string): Promise { try { // Run git status with --no-optional-locks to avoid holding locks - // Use porcelain=v1 for machine-parseable output, -b for branch info + // Use porcelain=v2 for stable machine-parseable output with branch headers // Use -z for NUL-terminated output (handles filenames with special chars) // Use -uall to show individual files in untracked directories (not just the directory) - // Note: porcelain=v1 already includes rename info (R/C status codes) without needing -M + // Note: porcelain=v2 includes structured rename/copy records without needing -M const { stdout } = await execGitWithShellPath( [ "--no-optional-locks", "-C", repoPath, "status", - "--porcelain=v1", - "-b", + "--porcelain=v2", + "--branch", "-z", "-uall", ], { env, timeout: 30_000, maxBuffer: 10 * 1024 * 1024 }, ); - return parsePortelainStatus(stdout); + return parsePorcelainStatusV2(stdout); } catch (error) { // Provide more descriptive error messages if (isExecFileException(error)) { @@ -172,17 +195,19 @@ export async function getStatusNoLock(repoPath: string): Promise { } /** - * Parses git status --porcelain=v1 -z output into a StatusResult-compatible object. + * Parses git status --porcelain=v2 --branch -z output into a StatusResult-compatible object. * The -z format uses NUL characters to separate entries, which safely handles * filenames containing spaces, newlines, or other special characters. */ -function parsePortelainStatus(stdout: string): StatusResult { +export function parsePorcelainStatusV2(stdout: string): StatusResult { // Split by NUL character - the -z format separates entries with NUL const entries = stdout.split("\0").filter(Boolean); let current: string | null = null; let tracking: string | null = null; let isDetached = false; + let ahead = 0; + let behind = 0; // Parse file status entries const files: StatusResult["files"] = []; @@ -195,6 +220,49 @@ function parsePortelainStatus(stdout: string): StatusResult { const conflictedSet = new Set(); const notAddedSet = new Set(); + const normalizeStatusCode = (code: string): string => + code === "." ? " " : code; + const addFile = ({ + path, + indexStatus, + workingStatus, + from, + }: { + path: string; + indexStatus: string; + workingStatus: string; + from?: string; + }) => { + files.push({ + path, + from: from ?? path, + index: indexStatus, + working_dir: workingStatus, + }); + + if (indexStatus === "?" && workingStatus === "?") { + notAddedSet.add(path); + return; + } + + // Index status (staged changes) + if (indexStatus === "A") createdSet.add(path); + else if (indexStatus === "M") { + stagedSet.add(path); + modifiedSet.add(path); + } else if (indexStatus === "D") { + stagedSet.add(path); + deletedSet.add(path); + } else if (indexStatus === "R" || indexStatus === "C") stagedSet.add(path); + else if (indexStatus === "U") conflictedSet.add(path); + else if (indexStatus !== " " && indexStatus !== "?") stagedSet.add(path); + + // Working tree status (unstaged changes) + if (workingStatus === "M") modifiedSet.add(path); + else if (workingStatus === "D") deletedSet.add(path); + else if (workingStatus === "U") conflictedSet.add(path); + }; + let i = 0; while (i < entries.length) { const entry = entries[i]; @@ -203,84 +271,100 @@ function parsePortelainStatus(stdout: string): StatusResult { continue; } - // Parse branch line: ## branch...tracking or ## branch - if (entry.startsWith("## ")) { - const branchInfo = entry.slice(3); - - // Check for detached HEAD states - if (branchInfo.startsWith("HEAD (no branch)") || branchInfo === "HEAD") { - isDetached = true; - current = "HEAD"; - } else if ( - // Handle empty repo: "No commits yet on BRANCH" or "Initial commit on BRANCH" - branchInfo.startsWith("No commits yet on ") || - branchInfo.startsWith("Initial commit on ") - ) { - // Extract branch name from the end - const parts = branchInfo.split(" "); - current = parts[parts.length - 1] || null; - } else { - // Check for tracking info: "branch...origin/branch [ahead 1, behind 2]" - const trackingMatch = branchInfo.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); - if (trackingMatch) { - current = trackingMatch[1]; - tracking = trackingMatch[2].split(" ")[0] || null; + if (entry.startsWith("# ")) { + const header = entry.slice(2); + if (header.startsWith("branch.head ")) { + const branchHead = header.slice("branch.head ".length); + if (branchHead === "(detached)") { + isDetached = true; + current = "HEAD"; } else { - // No tracking branch, just get branch name (before any space) - current = branchInfo.split(" ")[0] || null; + current = branchHead || null; + } + } else if (header.startsWith("branch.upstream ")) { + tracking = header.slice("branch.upstream ".length) || null; + } else if (header.startsWith("branch.ab ")) { + const match = header.match(/^branch\.ab \+(\d+) -(\d+)$/); + if (match) { + ahead = Number.parseInt(match[1] || "0", 10); + behind = Number.parseInt(match[2] || "0", 10); } } i++; continue; } - // Parse file status: "XY path" where X=index, Y=working tree - if (entry.length < 3) { + if (entry.startsWith("? ")) { + const path = entry.slice(2); + addFile({ + path, + indexStatus: "?", + workingStatus: "?", + }); i++; continue; } - const indexStatus = entry[0]; - const workingStatus = entry[1]; - // entry[2] is a space separator - const path = entry.slice(3); - let from: string | undefined; + // Ignored entries should not affect clean status. + if (entry.startsWith("! ")) { + i++; + continue; + } - // For renames/copies, the next entry is the original path - if (indexStatus === "R" || indexStatus === "C") { + if (entry.startsWith("1 ")) { + const match = entry.match(/^1 (\S{2}) \S+ \S+ \S+ \S+ \S+ \S+ (.+)$/); + if (match) { + const xy = match[1] || ".."; + const path = match[2]; + if (path) { + addFile({ + path, + indexStatus: normalizeStatusCode(xy[0] || "."), + workingStatus: normalizeStatusCode(xy[1] || "."), + }); + } + } i++; - from = entries[i]; - renamed.push({ from: from || path, to: path }); + continue; } - files.push({ - path, - from: from ?? path, - index: indexStatus, - working_dir: workingStatus, - }); + if (entry.startsWith("2 ")) { + const match = entry.match(/^2 (\S{2}) \S+ \S+ \S+ \S+ \S+ \S+ \S+ (.+)$/); + const from = entries[i + 1]; + if (match) { + const xy = match[1] || ".."; + const path = match[2]; + if (path) { + const originalPath = from || path; + renamed.push({ from: originalPath, to: path }); + addFile({ + path, + from: originalPath, + indexStatus: normalizeStatusCode(xy[0] || "."), + workingStatus: normalizeStatusCode(xy[1] || "."), + }); + } + } + i += 2; + continue; + } - // Populate convenience arrays for checkBranchCheckoutSafety compatibility - if (indexStatus === "?" && workingStatus === "?") { - notAddedSet.add(path); - } else { - // Index status (staged changes) - if (indexStatus === "A") createdSet.add(path); - else if (indexStatus === "M") { - stagedSet.add(path); - modifiedSet.add(path); - } else if (indexStatus === "D") { - stagedSet.add(path); - deletedSet.add(path); - } else if (indexStatus === "R" || indexStatus === "C") - stagedSet.add(path); - else if (indexStatus === "U") conflictedSet.add(path); - else if (indexStatus !== " " && indexStatus !== "?") stagedSet.add(path); - - // Working tree status (unstaged changes) - if (workingStatus === "M") modifiedSet.add(path); - else if (workingStatus === "D") deletedSet.add(path); - else if (workingStatus === "U") conflictedSet.add(path); + if (entry.startsWith("u ")) { + const match = entry.match( + /^u (\S{2}) \S+ \S+ \S+ \S+ \S+ \S+ \S+ \S+ (.+)$/, + ); + if (match) { + const xy = match[1] || ".."; + const path = match[2]; + if (path) { + conflictedSet.add(path); + addFile({ + path, + indexStatus: normalizeStatusCode(xy[0] || "."), + workingStatus: normalizeStatusCode(xy[1] || "."), + }); + } + } } i++; @@ -296,8 +380,8 @@ function parsePortelainStatus(stdout: string): StatusResult { renamed, files, staged: [...stagedSet], - ahead: 0, - behind: 0, + ahead, + behind, current, tracking, detached: isDetached, diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts index 962a5c3c237..3d6edc15e56 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts @@ -1,5 +1,9 @@ import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; -import { branchExistsOnRemote } from "../git"; +import { + branchExistsOnRemote, + getCurrentBranch, + isUnbornHeadError, +} from "../git"; import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; import { parseUpstreamRef } from "../upstream-ref"; @@ -44,16 +48,19 @@ async function resolvePullRequestCommentsTarget( return null; } - const [branchResult, shaResult] = await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: worktreePath, - }), - execGitWithShellPath(["rev-parse", "HEAD"], { - cwd: worktreePath, - }), - ]); - const branchName = branchResult.stdout.trim(); - const headSha = shaResult.stdout.trim(); + const branchName = await getCurrentBranch(worktreePath); + if (!branchName) { + return null; + } + const shaResult = await execGitWithShellPath(["rev-parse", "HEAD"], { + cwd: worktreePath, + }).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; + }); + const headSha = shaResult.stdout.trim() || undefined; const prInfo = await getPRForBranch( worktreePath, branchName, @@ -93,17 +100,25 @@ async function refreshGitHubPRStatus( return null; } - const [branchResult, shaResult, upstreamResult] = await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { + const branchName = await getCurrentBranch(worktreePath); + if (!branchName) { + return null; + } + + const [shaResult, upstreamResult] = await Promise.all([ + execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath, + }).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; }), - execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { cwd: worktreePath, }).catch(() => ({ stdout: "", stderr: "" })), ]); - const branchName = branchResult.stdout.trim(); - const headSha = shaResult.stdout.trim(); + const headSha = shaResult.stdout.trim() || undefined; const parsedUpstreamRef = parseUpstreamRef(upstreamResult.stdout.trim()); const trackingRemote = parsedUpstreamRef?.remoteName ?? "origin"; const previewBranchName = resolveRemoteBranchNameForGitHubStatus({ @@ -326,7 +341,7 @@ async function queryDeploymentUrl( */ async function fetchPreviewDeploymentUrl( worktreePath: string, - headSha: string, + headSha: string | undefined, branchName: string, repoContext: RepoContext, ): Promise { @@ -339,10 +354,16 @@ async function fetchPreviewDeploymentUrl( return undefined; } - // Try by commit SHA (works for Vercel, Netlify official integrations) - const bySha = await queryDeploymentUrl(worktreePath, nwo, `sha=${headSha}`); - if (bySha) { - return bySha; + if (headSha) { + // Try by commit SHA (works for Vercel, Netlify official integrations) + const bySha = await queryDeploymentUrl( + worktreePath, + nwo, + `sha=${headSha}`, + ); + if (bySha) { + return bySha; + } } // Fall back to branch name (works for some CI configurations) diff --git a/packages/host-service/src/runtime/pull-requests/pull-requests.ts b/packages/host-service/src/runtime/pull-requests/pull-requests.ts index 86fdbbed4c9..e5824a7e037 100644 --- a/packages/host-service/src/runtime/pull-requests/pull-requests.ts +++ b/packages/host-service/src/runtime/pull-requests/pull-requests.ts @@ -23,6 +23,49 @@ import { const BRANCH_SYNC_INTERVAL_MS = 10_000; const PROJECT_REFRESH_INTERVAL_MS = 15_000; +const UNBORN_HEAD_ERROR_PATTERNS = [ + "ambiguous argument 'head'", + "unknown revision or path not in the working tree", + "bad revision 'head'", + "not a valid object name head", + "needed a single revision", +]; + +async function getCurrentBranchName(git: Awaited>) { + try { + const branch = await git.raw(["symbolic-ref", "--short", "HEAD"]); + const trimmed = branch.trim(); + return trimmed || null; + } catch { + try { + const branch = await git.revparse(["--abbrev-ref", "HEAD"]); + const trimmed = branch.trim(); + return trimmed && trimmed !== "HEAD" ? trimmed : null; + } catch { + return null; + } + } +} + +async function getHeadSha(git: Awaited>) { + try { + const branch = await git.revparse(["HEAD"]); + const trimmed = branch.trim(); + return trimmed || null; + } catch (error) { + const message = + error instanceof Error + ? error.message.toLowerCase() + : String(error).toLowerCase(); + if ( + UNBORN_HEAD_ERROR_PATTERNS.some((pattern) => message.includes(pattern)) + ) { + return null; + } + + throw error; + } +} type RepoProvider = "github"; @@ -165,12 +208,11 @@ export class PullRequestRuntimeManager { for (const workspace of allWorkspaces) { try { const git = await this.git(workspace.worktreePath); - const [rawBranch, rawHeadSha] = await Promise.all([ - git.revparse(["--abbrev-ref", "HEAD"]), - git.revparse(["HEAD"]), - ]); - const branch = rawBranch.trim(); - const headSha = rawHeadSha.trim(); + const branch = await getCurrentBranchName(git); + if (!branch) { + continue; + } + const headSha = await getHeadSha(git); if (branch === workspace.branch && headSha === workspace.headSha) { continue; diff --git a/packages/host-service/test/pull-requests.test.ts b/packages/host-service/test/pull-requests.test.ts new file mode 100644 index 00000000000..685eda3ed30 --- /dev/null +++ b/packages/host-service/test/pull-requests.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, mock, spyOn, test } from "bun:test"; +import { PullRequestRuntimeManager } from "../src/runtime/pull-requests/pull-requests"; + +describe("PullRequestRuntimeManager branch sync", () => { + test("persists unborn branches even when HEAD has no commit", async () => { + const workspace = { + id: "ws-1", + projectId: "project-1", + worktreePath: "/tmp/unborn-worktree", + branch: "stale-branch", + headSha: "stale-sha", + pullRequestId: null, + createdAt: Date.now(), + }; + + const runMock = mock(() => undefined); + const whereMock = mock(() => ({ run: runMock })); + const setMock = mock(() => ({ where: whereMock })); + const updateMock = mock(() => ({ set: setMock })); + const allMock = mock(() => [workspace]); + const db = { + select: mock(() => ({ + from: mock(() => ({ + all: allMock, + })), + })), + update: updateMock, + }; + + const git = mock(async () => ({ + raw: mock(async (args: string[]) => { + if ( + args[0] === "symbolic-ref" && + args[1] === "--short" && + args[2] === "HEAD" + ) { + return "feature/unborn\n"; + } + throw new Error(`Unexpected raw args: ${args.join(" ")}`); + }), + revparse: mock(async (args: string[]) => { + if (args[0] === "HEAD") { + throw new Error("fatal: ambiguous argument 'HEAD'"); + } + throw new Error(`Unexpected revparse args: ${args.join(" ")}`); + }), + })); + + const manager = new PullRequestRuntimeManager({ + db: db as never, + git: git as never, + github: async () => ({}) as never, + }); + const refreshProjectMock = mock(async () => undefined); + ( + manager as unknown as { refreshProject: typeof refreshProjectMock } + ).refreshProject = refreshProjectMock; + + await ( + manager as unknown as { syncWorkspaceBranches: () => Promise } + ).syncWorkspaceBranches(); + + expect(git).toHaveBeenCalledWith("/tmp/unborn-worktree"); + expect(setMock).toHaveBeenCalledWith({ + branch: "feature/unborn", + headSha: null, + }); + expect(refreshProjectMock).toHaveBeenCalledWith("project-1", true); + }); + + test("logs and skips unexpected HEAD lookup failures", async () => { + const workspace = { + id: "ws-2", + projectId: "project-2", + worktreePath: "/tmp/broken-worktree", + branch: "stale-branch", + headSha: "stale-sha", + pullRequestId: null, + createdAt: Date.now(), + }; + + const runMock = mock(() => undefined); + const whereMock = mock(() => ({ run: runMock })); + const setMock = mock(() => ({ where: whereMock })); + const updateMock = mock(() => ({ set: setMock })); + const allMock = mock(() => [workspace]); + const db = { + select: mock(() => ({ + from: mock(() => ({ + all: allMock, + })), + })), + update: updateMock, + }; + + const git = mock(async () => ({ + raw: mock(async () => "feature/broken\n"), + revparse: mock(async (args: string[]) => { + if (args[0] === "HEAD") { + throw new Error("fatal: permission denied"); + } + throw new Error(`Unexpected revparse args: ${args.join(" ")}`); + }), + })); + + const manager = new PullRequestRuntimeManager({ + db: db as never, + git: git as never, + github: async () => ({}) as never, + }); + const refreshProjectMock = mock(async () => undefined); + ( + manager as unknown as { refreshProject: typeof refreshProjectMock } + ).refreshProject = refreshProjectMock; + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + + await ( + manager as unknown as { syncWorkspaceBranches: () => Promise } + ).syncWorkspaceBranches(); + + expect(setMock).not.toHaveBeenCalled(); + expect(refreshProjectMock).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalled(); + }); +});