fix(git): show Sync only when branch diverges from origin
This commit is contained in:
@@ -0,0 +1,62 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { shouldShowMergeFromBaseAction } from "./git-action-visibility";
|
||||
|
||||
describe("git-action-visibility", () => {
|
||||
describe("shouldShowMergeFromBaseAction", () => {
|
||||
it("shows on non-base branches", () => {
|
||||
expect(
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch: false,
|
||||
hasRemote: false,
|
||||
aheadOfOrigin: 0,
|
||||
behindOfOrigin: 0,
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("hides on base branch when no remote exists", () => {
|
||||
expect(
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch: true,
|
||||
hasRemote: false,
|
||||
aheadOfOrigin: 0,
|
||||
behindOfOrigin: 0,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("hides on base branch when local is in sync with origin", () => {
|
||||
expect(
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch: true,
|
||||
hasRemote: true,
|
||||
aheadOfOrigin: 0,
|
||||
behindOfOrigin: 0,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("shows on base branch when ahead of origin", () => {
|
||||
expect(
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch: true,
|
||||
hasRemote: true,
|
||||
aheadOfOrigin: 1,
|
||||
behindOfOrigin: 0,
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("shows on base branch when behind origin", () => {
|
||||
expect(
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch: true,
|
||||
hasRemote: true,
|
||||
aheadOfOrigin: 0,
|
||||
behindOfOrigin: 2,
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,14 @@
|
||||
export function shouldShowMergeFromBaseAction(input: {
|
||||
isOnBaseBranch: boolean;
|
||||
hasRemote: boolean;
|
||||
aheadOfOrigin: number;
|
||||
behindOfOrigin: number;
|
||||
}): boolean {
|
||||
if (!input.isOnBaseBranch) {
|
||||
return true;
|
||||
}
|
||||
if (!input.hasRemote) {
|
||||
return false;
|
||||
}
|
||||
return input.aheadOfOrigin > 0 || input.behindOfOrigin > 0;
|
||||
}
|
||||
@@ -56,6 +56,7 @@ import {
|
||||
} from "@/components/web-desktop-scrollbar";
|
||||
import { buildNewAgentRoute, resolveNewAgentWorkingDir } from "@/utils/new-agent-routing";
|
||||
import { openExternalUrl } from "@/utils/open-external-url";
|
||||
import { shouldShowMergeFromBaseAction } from "./git-action-visibility";
|
||||
|
||||
// =============================================================================
|
||||
// Git Actions Data Structure
|
||||
@@ -900,6 +901,7 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
const actionsDisabled = !isGit || Boolean(status?.error) || isStatusLoading;
|
||||
const aheadCount = gitStatus?.aheadBehind?.ahead ?? 0;
|
||||
const aheadOfOrigin = gitStatus?.aheadOfOrigin ?? 0;
|
||||
const behindOfOrigin = gitStatus?.behindOfOrigin ?? 0;
|
||||
const baseRefLabel = useMemo(() => {
|
||||
if (!baseRef) return "base";
|
||||
const trimmed = baseRef.replace(/^refs\/(heads|remotes)\//, "").trim();
|
||||
@@ -1109,7 +1111,14 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
}
|
||||
|
||||
// Update/sync from base
|
||||
if (!isOnBaseBranch || hasRemote) {
|
||||
if (
|
||||
shouldShowMergeFromBaseAction({
|
||||
isOnBaseBranch,
|
||||
hasRemote,
|
||||
aheadOfOrigin,
|
||||
behindOfOrigin,
|
||||
})
|
||||
) {
|
||||
allActions.set("merge-from-base", {
|
||||
id: "merge-from-base",
|
||||
label: isOnBaseBranch ? "Sync" : `Update from ${baseRefLabel}`,
|
||||
@@ -1203,7 +1212,7 @@ export function GitDiffPane({ serverId, agentId, cwd }: GitDiffPaneProps) {
|
||||
return { primary, secondary, menu };
|
||||
}, [
|
||||
isGit, hasRemote, hasPullRequest, prStatus?.url, aheadCount, isPaseoOwnedWorktree, isOnBaseBranch, githubFeaturesEnabled,
|
||||
hasUncommittedChanges, aheadOfOrigin, shipDefault, baseRefLabel, shouldPromoteArchive,
|
||||
hasUncommittedChanges, aheadOfOrigin, behindOfOrigin, shipDefault, baseRefLabel, shouldPromoteArchive,
|
||||
commitDisabled, pushDisabled, prDisabled, mergeDisabled, mergeFromBaseDisabled, archiveDisabled,
|
||||
commitStatus, pushStatus, prCreateStatus, mergeStatus, mergeFromBaseStatus, archiveStatus,
|
||||
handleCommit, handlePush, handleCreatePr, handleMergeBranch, handleMergeFromBase, handleArchiveWorktree,
|
||||
|
||||
@@ -120,6 +120,7 @@ describe('DaemonClient', () => {
|
||||
baseRef: null,
|
||||
aheadBehind: null,
|
||||
aheadOfOrigin: null,
|
||||
behindOfOrigin: null,
|
||||
hasRemote: false,
|
||||
remoteUrl: null,
|
||||
},
|
||||
|
||||
@@ -3437,6 +3437,7 @@ export class Session {
|
||||
baseRef: null,
|
||||
aheadBehind: null,
|
||||
aheadOfOrigin: null,
|
||||
behindOfOrigin: null,
|
||||
hasRemote: false,
|
||||
remoteUrl: null,
|
||||
isPaseoOwnedWorktree: false,
|
||||
@@ -3460,6 +3461,7 @@ export class Session {
|
||||
baseRef: status.baseRef,
|
||||
aheadBehind: status.aheadBehind ?? null,
|
||||
aheadOfOrigin: status.aheadOfOrigin ?? null,
|
||||
behindOfOrigin: status.behindOfOrigin ?? null,
|
||||
hasRemote: status.hasRemote,
|
||||
remoteUrl: status.remoteUrl,
|
||||
isPaseoOwnedWorktree: true,
|
||||
@@ -3481,6 +3483,7 @@ export class Session {
|
||||
baseRef: status.baseRef ?? null,
|
||||
aheadBehind: status.aheadBehind ?? null,
|
||||
aheadOfOrigin: status.aheadOfOrigin ?? null,
|
||||
behindOfOrigin: status.behindOfOrigin ?? null,
|
||||
hasRemote: status.hasRemote,
|
||||
remoteUrl: status.remoteUrl,
|
||||
isPaseoOwnedWorktree: false,
|
||||
@@ -3500,6 +3503,7 @@ export class Session {
|
||||
baseRef: null,
|
||||
aheadBehind: null,
|
||||
aheadOfOrigin: null,
|
||||
behindOfOrigin: null,
|
||||
hasRemote: false,
|
||||
remoteUrl: null,
|
||||
isPaseoOwnedWorktree: false,
|
||||
|
||||
@@ -1562,6 +1562,7 @@ const CheckoutStatusNotGitSchema = CheckoutStatusCommonSchema.extend({
|
||||
baseRef: z.null(),
|
||||
aheadBehind: z.null(),
|
||||
aheadOfOrigin: z.null(),
|
||||
behindOfOrigin: z.null(),
|
||||
hasRemote: z.boolean(),
|
||||
remoteUrl: z.null(),
|
||||
})
|
||||
@@ -1575,6 +1576,7 @@ const CheckoutStatusGitNonPaseoSchema = CheckoutStatusCommonSchema.extend({
|
||||
baseRef: z.string().nullable(),
|
||||
aheadBehind: AheadBehindSchema.nullable(),
|
||||
aheadOfOrigin: z.number().nullable(),
|
||||
behindOfOrigin: z.number().nullable(),
|
||||
hasRemote: z.boolean(),
|
||||
remoteUrl: z.string().nullable(),
|
||||
})
|
||||
@@ -1589,6 +1591,7 @@ const CheckoutStatusGitPaseoSchema = CheckoutStatusCommonSchema.extend({
|
||||
baseRef: z.string(),
|
||||
aheadBehind: AheadBehindSchema.nullable(),
|
||||
aheadOfOrigin: z.number().nullable(),
|
||||
behindOfOrigin: z.number().nullable(),
|
||||
hasRemote: z.boolean(),
|
||||
remoteUrl: z.string().nullable(),
|
||||
})
|
||||
|
||||
@@ -101,6 +101,43 @@ describe("checkout git utilities", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("reports ahead/behind relative to origin on the base branch", async () => {
|
||||
const remoteDir = join(tempDir, "remote.git");
|
||||
const cloneDir = join(tempDir, "clone");
|
||||
execSync(`git init --bare -b main ${remoteDir}`);
|
||||
execSync(`git remote add origin ${remoteDir}`, { cwd: repoDir });
|
||||
execSync("git push -u origin main", { cwd: repoDir });
|
||||
|
||||
execSync(`git clone ${remoteDir} ${cloneDir}`);
|
||||
execSync("git config user.email 'test@test.com'", { cwd: cloneDir });
|
||||
execSync("git config user.name 'Test'", { cwd: cloneDir });
|
||||
writeFileSync(join(cloneDir, "file.txt"), "remote\n");
|
||||
execSync("git add file.txt", { cwd: cloneDir });
|
||||
execSync("git -c commit.gpgsign=false commit -m 'remote update'", { cwd: cloneDir });
|
||||
execSync("git push", { cwd: cloneDir });
|
||||
|
||||
execSync("git fetch origin", { cwd: repoDir });
|
||||
const behindStatus = await getCheckoutStatus(repoDir);
|
||||
expect(behindStatus.isGit).toBe(true);
|
||||
if (!behindStatus.isGit) {
|
||||
return;
|
||||
}
|
||||
expect(behindStatus.aheadOfOrigin).toBe(0);
|
||||
expect(behindStatus.behindOfOrigin).toBe(1);
|
||||
|
||||
writeFileSync(join(repoDir, "local.txt"), "local\n");
|
||||
execSync("git add local.txt", { cwd: repoDir });
|
||||
execSync("git -c commit.gpgsign=false commit -m 'local update'", { cwd: repoDir });
|
||||
|
||||
const divergedStatus = await getCheckoutStatus(repoDir);
|
||||
expect(divergedStatus.isGit).toBe(true);
|
||||
if (!divergedStatus.isGit) {
|
||||
return;
|
||||
}
|
||||
expect(divergedStatus.aheadOfOrigin).toBe(1);
|
||||
expect(divergedStatus.behindOfOrigin).toBe(1);
|
||||
});
|
||||
|
||||
it("commits messages with quotes safely", async () => {
|
||||
const message = `He said "hello" and it's fine`;
|
||||
writeFileSync(join(repoDir, "file.txt"), "quoted\n");
|
||||
|
||||
@@ -441,6 +441,7 @@ export type CheckoutStatusGitNonPaseo = {
|
||||
baseRef: string | null;
|
||||
aheadBehind: AheadBehind | null;
|
||||
aheadOfOrigin: number | null;
|
||||
behindOfOrigin: number | null;
|
||||
hasRemote: boolean;
|
||||
remoteUrl: string | null;
|
||||
isPaseoOwnedWorktree: false;
|
||||
@@ -455,6 +456,7 @@ export type CheckoutStatusGitPaseo = {
|
||||
baseRef: string;
|
||||
aheadBehind: AheadBehind | null;
|
||||
aheadOfOrigin: number | null;
|
||||
behindOfOrigin: number | null;
|
||||
hasRemote: boolean;
|
||||
remoteUrl: string | null;
|
||||
isPaseoOwnedWorktree: true;
|
||||
@@ -838,6 +840,22 @@ async function getAheadOfOrigin(cwd: string, currentBranch: string): Promise<num
|
||||
}
|
||||
}
|
||||
|
||||
async function getBehindOfOrigin(cwd: string, currentBranch: string): Promise<number | null> {
|
||||
if (!currentBranch) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const { stdout } = await execAsync(
|
||||
`git rev-list --count ${currentBranch}..origin/${currentBranch}`,
|
||||
{ cwd, env: READ_ONLY_GIT_ENV }
|
||||
);
|
||||
const count = Number.parseInt(stdout.trim(), 10);
|
||||
return Number.isNaN(count) ? null : count;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
type CheckoutInspectionContext = {
|
||||
worktreeRoot: string;
|
||||
currentBranch: string | null;
|
||||
@@ -996,10 +1014,11 @@ export async function getCheckoutStatus(
|
||||
const isDirty = await isWorkingTreeDirty(cwd);
|
||||
const hasRemote = remoteUrl !== null;
|
||||
const baseRef = configured.baseRef ?? (await resolveBaseRef(cwd));
|
||||
const aheadBehind =
|
||||
baseRef && currentBranch ? await getAheadBehind(cwd, baseRef, currentBranch) : null;
|
||||
const aheadOfOrigin =
|
||||
hasRemote && currentBranch ? await getAheadOfOrigin(cwd, currentBranch) : null;
|
||||
const [aheadBehind, aheadOfOrigin, behindOfOrigin] = await Promise.all([
|
||||
baseRef && currentBranch ? getAheadBehind(cwd, baseRef, currentBranch) : Promise.resolve(null),
|
||||
hasRemote && currentBranch ? getAheadOfOrigin(cwd, currentBranch) : Promise.resolve(null),
|
||||
hasRemote && currentBranch ? getBehindOfOrigin(cwd, currentBranch) : Promise.resolve(null),
|
||||
]);
|
||||
|
||||
if (configured.isPaseoOwnedWorktree) {
|
||||
const mainRepoRoot = await getMainRepoRoot(cwd);
|
||||
@@ -1012,6 +1031,7 @@ export async function getCheckoutStatus(
|
||||
baseRef: configured.baseRef,
|
||||
aheadBehind,
|
||||
aheadOfOrigin,
|
||||
behindOfOrigin,
|
||||
hasRemote,
|
||||
remoteUrl,
|
||||
isPaseoOwnedWorktree: true,
|
||||
@@ -1026,6 +1046,7 @@ export async function getCheckoutStatus(
|
||||
baseRef,
|
||||
aheadBehind,
|
||||
aheadOfOrigin,
|
||||
behindOfOrigin,
|
||||
hasRemote,
|
||||
remoteUrl,
|
||||
isPaseoOwnedWorktree: false,
|
||||
|
||||
Reference in New Issue
Block a user