From 4a61d95fdfae1fa865ced5183559b29a197bbc1e Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 12:13:37 -0700 Subject: [PATCH 1/7] fix(desktop): prevent [gone] from being stored as workspace branch name The porcelain status parser incorrectly extracted "[gone]" as the branch name when git reported "No commits yet on BRANCH...origin/BRANCH [gone]". The old code split by space and took the last element; the fix strips the prefix and applies the same tracking-info regex used for normal branches. Also adds syncBranch validation to reject obviously invalid names and a one-time startup repair that reads the real branch from the worktree HEAD file for any existing corrupted records. --- .../routers/workspaces/procedures/status.ts | 7 ++- .../lib/trpc/routers/workspaces/utils/git.ts | 16 ++++-- apps/desktop/src/main/lib/local-db/index.ts | 49 ++++++++++++++++++- 3 files changed, 67 insertions(+), 5 deletions(-) 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.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index 0df864005b5..d1df670cbed 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -216,9 +216,19 @@ function parsePortelainStatus(stdout: string): StatusResult { 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; + const prefix = branchInfo.startsWith("No commits yet on ") + ? "No commits yet on " + : "Initial commit on "; + const rest = branchInfo.slice(prefix.length); + + // Parse tracking info if present (e.g., "branch...origin/branch [gone]") + const trackingMatch = rest.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); + if (trackingMatch) { + current = trackingMatch[1]; + tracking = trackingMatch[2].split(" ")[0] || null; + } else { + current = rest.split(" ")[0] || null; + } } else { // Check for tracking info: "branch...origin/branch [ahead 1, behind 2]" const trackingMatch = branchInfo.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); diff --git a/apps/desktop/src/main/lib/local-db/index.ts b/apps/desktop/src/main/lib/local-db/index.ts index cf7dc89cab1..12886bcb6dd 100644 --- a/apps/desktop/src/main/lib/local-db/index.ts +++ b/apps/desktop/src/main/lib/local-db/index.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { chmodSync, existsSync } from "node:fs"; +import { chmodSync, existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import * as schema from "@superset/local-db"; @@ -103,4 +103,51 @@ try { console.log("[local-db] Migrations complete"); +// Repair worktrees/workspaces where branch was incorrectly set to "[gone]" +// due to a parser bug in parsePortelainStatus (see git.ts fix). +try { + const corruptedWorktrees = sqlite + .prepare( + "SELECT id, path FROM worktrees WHERE branch = '[gone]'", + ) + .all() as { id: string; path: string }[]; + + for (const wt of corruptedWorktrees) { + let realBranch: string | null = null; + try { + const dotGit = readFileSync(join(wt.path, ".git"), "utf-8").trim(); + const gitdir = dotGit.replace("gitdir: ", ""); + const head = readFileSync(join(gitdir, "HEAD"), "utf-8").trim(); + const match = head.match(/^ref: refs\/heads\/(.+)$/); + if (match) { + realBranch = match[1]; + } + } catch { + // Worktree may no longer exist on disk — skip + } + + if (realBranch) { + sqlite + .prepare("UPDATE worktrees SET branch = ? WHERE id = ?") + .run(realBranch, wt.id); + sqlite + .prepare( + "UPDATE workspaces SET branch = ? WHERE worktree_id = ? AND branch = '[gone]'", + ) + .run(realBranch, wt.id); + console.log( + `[local-db] Repaired branch for worktree ${wt.id}: [gone] → ${realBranch}`, + ); + } + } + + if (corruptedWorktrees.length > 0) { + console.log( + `[local-db] Repaired ${corruptedWorktrees.length} corrupted worktree branch(es)`, + ); + } +} catch (error) { + console.warn("[local-db] Branch repair failed (non-fatal):", error); +} + export type LocalDb = typeof localDb; From 0e560239c0d1757c3290613486e9a369bbc474db Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 14:40:21 -0700 Subject: [PATCH 2/7] fix(desktop): remove startup branch repair --- apps/desktop/src/main/lib/local-db/index.ts | 49 +-------------------- 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/apps/desktop/src/main/lib/local-db/index.ts b/apps/desktop/src/main/lib/local-db/index.ts index 12886bcb6dd..cf7dc89cab1 100644 --- a/apps/desktop/src/main/lib/local-db/index.ts +++ b/apps/desktop/src/main/lib/local-db/index.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { chmodSync, existsSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync } from "node:fs"; import { join } from "node:path"; import * as schema from "@superset/local-db"; @@ -103,51 +103,4 @@ try { console.log("[local-db] Migrations complete"); -// Repair worktrees/workspaces where branch was incorrectly set to "[gone]" -// due to a parser bug in parsePortelainStatus (see git.ts fix). -try { - const corruptedWorktrees = sqlite - .prepare( - "SELECT id, path FROM worktrees WHERE branch = '[gone]'", - ) - .all() as { id: string; path: string }[]; - - for (const wt of corruptedWorktrees) { - let realBranch: string | null = null; - try { - const dotGit = readFileSync(join(wt.path, ".git"), "utf-8").trim(); - const gitdir = dotGit.replace("gitdir: ", ""); - const head = readFileSync(join(gitdir, "HEAD"), "utf-8").trim(); - const match = head.match(/^ref: refs\/heads\/(.+)$/); - if (match) { - realBranch = match[1]; - } - } catch { - // Worktree may no longer exist on disk — skip - } - - if (realBranch) { - sqlite - .prepare("UPDATE worktrees SET branch = ? WHERE id = ?") - .run(realBranch, wt.id); - sqlite - .prepare( - "UPDATE workspaces SET branch = ? WHERE worktree_id = ? AND branch = '[gone]'", - ) - .run(realBranch, wt.id); - console.log( - `[local-db] Repaired branch for worktree ${wt.id}: [gone] → ${realBranch}`, - ); - } - } - - if (corruptedWorktrees.length > 0) { - console.log( - `[local-db] Repaired ${corruptedWorktrees.length} corrupted worktree branch(es)`, - ); - } -} catch (error) { - console.warn("[local-db] Branch repair failed (non-fatal):", error); -} - export type LocalDb = typeof localDb; From 07efd483bc5df711f71cb78913b9b36262d68f79 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 17:06:14 -0700 Subject: [PATCH 3/7] Refactor porcelain use gh --- .../trpc/routers/changes/git-operations.ts | 64 ++++-- .../routers/changes/security/git-commands.ts | 4 +- .../changes/utils/merge-pull-request.ts | 16 +- .../trpc/routers/workspaces/utils/git.test.ts | 65 ++++++ .../lib/trpc/routers/workspaces/utils/git.ts | 210 +++++++++++------- .../routers/workspaces/utils/github/github.ts | 22 +- .../runtime/pull-requests/pull-requests.ts | 24 +- 7 files changed, 288 insertions(+), 117 deletions(-) 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.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts index 0d563a467ee..70f2b05f13a 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,4 @@ +import { getCurrentBranch } from "../../workspaces/utils/git"; import { execGitWithShellPath } from "../../workspaces/utils/git-client"; import { getPRForBranch, @@ -36,14 +37,13 @@ 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 [localBranch, { stdout: headOutput }] = await Promise.all([ + getCurrentBranch(worktreePath), + execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), + ]); + if (!localBranch) { + return runMerge(legacyMergeArgs); + } const headSha = headOutput.trim(); pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha); 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..3b97e947a3f 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,7 @@ import { createWorktree, getCurrentBranch, hasUnpushedCommits, + parsePorcelainStatusV2, parsePrUrl, } from "./git"; @@ -504,6 +505,70 @@ 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"]); + }); +}); + 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 d1df670cbed..687c77ba5f6 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -135,25 +135,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 +172,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 +197,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,94 +248,99 @@ 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 ") - ) { - const prefix = branchInfo.startsWith("No commits yet on ") - ? "No commits yet on " - : "Initial commit on "; - const rest = branchInfo.slice(prefix.length); - - // Parse tracking info if present (e.g., "branch...origin/branch [gone]") - const trackingMatch = rest.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 { - current = rest.split(" ")[0] || null; + current = branchHead || 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; - } else { - // No tracking branch, just get branch name (before any space) - current = branchInfo.split(" ")[0] || 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) { + addFile({ + path, + indexStatus: normalizeStatusCode(xy[0] || "."), + workingStatus: normalizeStatusCode(xy[1] || "."), + }); + } + } } i++; @@ -306,8 +356,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..7cfaa1a731a 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,5 @@ import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; -import { branchExistsOnRemote } from "../git"; +import { branchExistsOnRemote, getCurrentBranch } from "../git"; import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; import { parseUpstreamRef } from "../upstream-ref"; @@ -44,15 +44,15 @@ async function resolvePullRequestCommentsTarget( return null; } - const [branchResult, shaResult] = await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: worktreePath, - }), + const [branchName, shaResult] = await Promise.all([ + getCurrentBranch(worktreePath), execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath, }), ]); - const branchName = branchResult.stdout.trim(); + if (!branchName) { + return null; + } const headSha = shaResult.stdout.trim(); const prInfo = await getPRForBranch( worktreePath, @@ -93,16 +93,16 @@ async function refreshGitHubPRStatus( return null; } - const [branchResult, shaResult, upstreamResult] = await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: worktreePath, - }), + const [branchName, shaResult, upstreamResult] = await Promise.all([ + getCurrentBranch(worktreePath), execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { cwd: worktreePath, }).catch(() => ({ stdout: "", stderr: "" })), ]); - const branchName = branchResult.stdout.trim(); + if (!branchName) { + return null; + } const headSha = shaResult.stdout.trim(); const parsedUpstreamRef = parseUpstreamRef(upstreamResult.stdout.trim()); const trackingRemote = parsedUpstreamRef?.remoteName ?? "origin"; 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..1587828a78a 100644 --- a/packages/host-service/src/runtime/pull-requests/pull-requests.ts +++ b/packages/host-service/src/runtime/pull-requests/pull-requests.ts @@ -24,6 +24,22 @@ import { const BRANCH_SYNC_INTERVAL_MS = 10_000; const PROJECT_REFRESH_INTERVAL_MS = 15_000; +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; + } + } +} + type RepoProvider = "github"; export interface PullRequestStateSnapshot { @@ -165,11 +181,13 @@ 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"]), + const [branch, rawHeadSha] = await Promise.all([ + getCurrentBranchName(git), git.revparse(["HEAD"]), ]); - const branch = rawBranch.trim(); + if (!branch) { + continue; + } const headSha = rawHeadSha.trim(); if (branch === workspace.branch && headSha === workspace.headSha) { From dc3957a7d70cdad53a820815ee3af0a162a1f000 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 17:14:10 -0700 Subject: [PATCH 4/7] test(desktop): cover porcelain v2 branch handling --- .../changes/utils/merge-pull-request.test.ts | 112 ++++++++++++++++++ .../trpc/routers/workspaces/utils/git.test.ts | 21 ++++ 2 files changed, 133 insertions(+) create mode 100644 apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts 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..e8a026351cc --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.test.ts @@ -0,0 +1,112 @@ +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).toHaveBeenCalledWith( + ["rev-parse", "HEAD"], + { cwd: "/tmp/detached-worktree" }, + ); + 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); + }); +}); + +afterAll(() => { + mock.restore(); +}); 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 3b97e947a3f..7371f427b33 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 @@ -567,6 +567,27 @@ describe("parsePorcelainStatusV2", () => { 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", + }, + ]); + }); }); describe("branchExistsOnRemote", () => { From 3e6cdbe9e6a83e4c2f387339404b74c9e1c2ce36 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 18:50:06 -0700 Subject: [PATCH 5/7] fix(desktop): handle missing HEAD during branch sync --- .../changes/utils/merge-pull-request.test.ts | 38 ++++++++-- .../changes/utils/merge-pull-request.ts | 11 +-- .../trpc/routers/workspaces/utils/git.test.ts | 20 ++++++ .../lib/trpc/routers/workspaces/utils/git.ts | 1 + .../routers/workspaces/utils/github/github.ts | 45 +++++++----- .../runtime/pull-requests/pull-requests.ts | 17 +++-- .../host-service/test/pull-requests.test.ts | 70 +++++++++++++++++++ 7 files changed, 169 insertions(+), 33 deletions(-) create mode 100644 packages/host-service/test/pull-requests.test.ts 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 index e8a026351cc..f15eaba1922 100644 --- 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 @@ -89,10 +89,7 @@ describe("mergePullRequest", () => { expect(getRepoContextMock).toHaveBeenCalledWith("/tmp/detached-worktree"); expect(getCurrentBranchMock).toHaveBeenCalledWith("/tmp/detached-worktree"); - expect(execGitWithShellPathMock).toHaveBeenCalledWith( - ["rev-parse", "HEAD"], - { cwd: "/tmp/detached-worktree" }, - ); + expect(execGitWithShellPathMock).not.toHaveBeenCalled(); expect(getPRForBranchMock).not.toHaveBeenCalled(); expect(execWithShellEnvMock).toHaveBeenCalledWith( "gh", @@ -105,6 +102,39 @@ describe("mergePullRequest", () => { 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); + }); }); afterAll(() => { 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 70f2b05f13a..038df331520 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 @@ -37,14 +37,15 @@ export async function mergePullRequest({ let pr: Awaited> = null; try { - const [localBranch, { stdout: headOutput }] = await Promise.all([ - getCurrentBranch(worktreePath), - execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), - ]); + const localBranch = await getCurrentBranch(worktreePath); if (!localBranch) { return runMerge(legacyMergeArgs); } - const headSha = headOutput.trim(); + const { stdout: headOutput } = await execGitWithShellPath( + ["rev-parse", "HEAD"], + { cwd: worktreePath }, + ).catch(() => ({ stdout: "", stderr: "" })); + const headSha = headOutput.trim() || undefined; pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha); } catch (error) { 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 7371f427b33..3f54b5f9692 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 @@ -588,6 +588,26 @@ describe("parsePorcelainStatusV2", () => { }, ]); }); + + 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("branchExistsOnRemote", () => { 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 687c77ba5f6..be7ec9dccd2 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -334,6 +334,7 @@ export function parsePorcelainStatusV2(stdout: string): StatusResult { const xy = match[1] || ".."; const path = match[2]; if (path) { + conflictedSet.add(path); addFile({ path, indexStatus: normalizeStatusCode(xy[0] || "."), 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 7cfaa1a731a..1fc55e26943 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 @@ -44,16 +44,14 @@ async function resolvePullRequestCommentsTarget( return null; } - const [branchName, shaResult] = await Promise.all([ - getCurrentBranch(worktreePath), - execGitWithShellPath(["rev-parse", "HEAD"], { - cwd: worktreePath, - }), - ]); + const branchName = await getCurrentBranch(worktreePath); if (!branchName) { return null; } - const headSha = shaResult.stdout.trim(); + const shaResult = await execGitWithShellPath(["rev-parse", "HEAD"], { + cwd: worktreePath, + }).catch(() => ({ stdout: "", stderr: "" })); + const headSha = shaResult.stdout.trim() || undefined; const prInfo = await getPRForBranch( worktreePath, branchName, @@ -93,17 +91,20 @@ async function refreshGitHubPRStatus( return null; } - const [branchName, shaResult, upstreamResult] = await Promise.all([ - getCurrentBranch(worktreePath), - execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), + const branchName = await getCurrentBranch(worktreePath); + if (!branchName) { + return null; + } + + const [shaResult, upstreamResult] = await Promise.all([ + execGitWithShellPath(["rev-parse", "HEAD"], { + cwd: worktreePath, + }).catch(() => ({ stdout: "", stderr: "" })), execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { cwd: worktreePath, }).catch(() => ({ stdout: "", stderr: "" })), ]); - if (!branchName) { - return null; - } - 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 +327,7 @@ async function queryDeploymentUrl( */ async function fetchPreviewDeploymentUrl( worktreePath: string, - headSha: string, + headSha: string | undefined, branchName: string, repoContext: RepoContext, ): Promise { @@ -339,10 +340,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 1587828a78a..605787f01bb 100644 --- a/packages/host-service/src/runtime/pull-requests/pull-requests.ts +++ b/packages/host-service/src/runtime/pull-requests/pull-requests.ts @@ -40,6 +40,16 @@ async function getCurrentBranchName(git: Awaited>) { } } +async function getHeadSha(git: Awaited>) { + try { + const branch = await git.revparse(["HEAD"]); + const trimmed = branch.trim(); + return trimmed || null; + } catch { + return null; + } +} + type RepoProvider = "github"; export interface PullRequestStateSnapshot { @@ -181,14 +191,11 @@ export class PullRequestRuntimeManager { for (const workspace of allWorkspaces) { try { const git = await this.git(workspace.worktreePath); - const [branch, rawHeadSha] = await Promise.all([ - getCurrentBranchName(git), - git.revparse(["HEAD"]), - ]); + const branch = await getCurrentBranchName(git); if (!branch) { continue; } - const headSha = rawHeadSha.trim(); + 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..77e0ac73823 --- /dev/null +++ b/packages/host-service/test/pull-requests.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, mock, 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); + }); +}); From bd08307a9ddb33dc4e16dabedefe6f9d25801e2c Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 19:09:31 -0700 Subject: [PATCH 6/7] fix(desktop): narrow unborn HEAD recovery --- .../changes/utils/merge-pull-request.test.ts | 20 +++++++ .../changes/utils/merge-pull-request.ts | 9 ++- .../trpc/routers/workspaces/utils/git.test.ts | 19 +++++++ .../lib/trpc/routers/workspaces/utils/git.ts | 23 ++++++++ .../routers/workspaces/utils/github/github.ts | 20 ++++++- .../runtime/pull-requests/pull-requests.ts | 21 ++++++- .../host-service/test/pull-requests.test.ts | 57 ++++++++++++++++++- 7 files changed, 161 insertions(+), 8 deletions(-) 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 index f15eaba1922..c4e115ccd20 100644 --- 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 @@ -135,6 +135,26 @@ describe("mergePullRequest", () => { ); 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(() => { 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 038df331520..be551afa261 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,4 +1,4 @@ -import { getCurrentBranch } from "../../workspaces/utils/git"; +import { getCurrentBranch, isUnbornHeadError } from "../../workspaces/utils/git"; import { execGitWithShellPath } from "../../workspaces/utils/git-client"; import { getPRForBranch, @@ -44,7 +44,12 @@ export async function mergePullRequest({ const { stdout: headOutput } = await execGitWithShellPath( ["rev-parse", "HEAD"], { cwd: worktreePath }, - ).catch(() => ({ stdout: "", stderr: "" })); + ).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; + }); const headSha = headOutput.trim() || undefined; pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha); 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 3f54b5f9692..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,7 @@ import { createWorktree, getCurrentBranch, hasUnpushedCommits, + isUnbornHeadError, parsePorcelainStatusV2, parsePrUrl, } from "./git"; @@ -610,6 +611,24 @@ describe("parsePorcelainStatusV2", () => { }); }); +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 be7ec9dccd2..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, 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 1fc55e26943..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, getCurrentBranch } from "../git"; +import { + branchExistsOnRemote, + getCurrentBranch, + isUnbornHeadError, +} from "../git"; import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; import { parseUpstreamRef } from "../upstream-ref"; @@ -50,7 +54,12 @@ async function resolvePullRequestCommentsTarget( } const shaResult = await execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath, - }).catch(() => ({ stdout: "", stderr: "" })); + }).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; + }); const headSha = shaResult.stdout.trim() || undefined; const prInfo = await getPRForBranch( worktreePath, @@ -99,7 +108,12 @@ async function refreshGitHubPRStatus( const [shaResult, upstreamResult] = await Promise.all([ execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath, - }).catch(() => ({ stdout: "", stderr: "" })), + }).catch((error) => { + if (isUnbornHeadError(error)) { + return { stdout: "", stderr: "" }; + } + throw error; + }), execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { cwd: worktreePath, }).catch(() => ({ stdout: "", stderr: "" })), 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 605787f01bb..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,13 @@ 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 { @@ -45,8 +52,18 @@ async function getHeadSha(git: Awaited>) { const branch = await git.revparse(["HEAD"]); const trimmed = branch.trim(); return trimmed || null; - } catch { - return 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; } } diff --git a/packages/host-service/test/pull-requests.test.ts b/packages/host-service/test/pull-requests.test.ts index 77e0ac73823..685eda3ed30 100644 --- a/packages/host-service/test/pull-requests.test.ts +++ b/packages/host-service/test/pull-requests.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, mock, test } from "bun:test"; +import { describe, expect, mock, spyOn, test } from "bun:test"; import { PullRequestRuntimeManager } from "../src/runtime/pull-requests/pull-requests"; describe("PullRequestRuntimeManager branch sync", () => { @@ -67,4 +67,59 @@ describe("PullRequestRuntimeManager branch sync", () => { }); 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(); + }); }); From 2c11c514afbbf028d788e59543efe61fd69621f2 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 25 Mar 2026 19:15:22 -0700 Subject: [PATCH 7/7] lint --- .../src/lib/trpc/routers/changes/utils/merge-pull-request.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 be551afa261..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,4 +1,7 @@ -import { getCurrentBranch, isUnbornHeadError } from "../../workspaces/utils/git"; +import { + getCurrentBranch, + isUnbornHeadError, +} from "../../workspaces/utils/git"; import { execGitWithShellPath } from "../../workspaces/utils/git-client"; import { getPRForBranch,