From f7a507a5db22decd85ff376b99c309ad965d1a5b Mon Sep 17 00:00:00 2001 From: Muu Date: Wed, 11 Nov 2026 16:41:45 +0800 Subject: [PATCH] feat(vcs): allow rewriting history of pushed commits (#2644) Edit Commit Message, Fixup, Squash Into, Drop Commit, Squash Commits and Interactively Rebase from Here are no longer disabled for pushed commits. A rewrite that reaches commits already on the upstream asks for confirmation first (pushing afterwards needs a force push); on a protected branch such a rewrite is refused, while its local commits can still be rewritten. Undo Commit follows the same rule. Co-authored-by: muutot <92625353+muutot@users.noreply.github.com> --- docs/en/guide/vcs.mdx | 2 +- docs/zh/guide/vcs.mdx | 2 +- src/features/vcs/GitLog.tsx | 19 +++---- src/features/vcs/__tests__/gitOps.test.ts | 11 +++++ src/features/vcs/__tests__/historyOps.test.ts | 25 +++++----- src/features/vcs/logStore.ts | 7 --- src/features/vcs/rebase.tsx | 49 +++++++++++++++++-- src/features/vcs/rebasePlan.ts | 11 +++++ 8 files changed, 90 insertions(+), 36 deletions(-) diff --git a/docs/en/guide/vcs.mdx b/docs/en/guide/vcs.mdx index 14d6670b7..d8b5be30b 100644 --- a/docs/en/guide/vcs.mdx +++ b/docs/en/guide/vcs.mdx @@ -181,7 +181,7 @@ Right-click a commit for **Copy Revision Number** (`⌥⇧⌘C` / `Ctrl+Alt+Shif ### Rewrite history -From the log you can **Edit Commit Message…**, **Fixup…**, **Squash Into…**, **Drop Commit**, **Squash Commits…** and **Interactively Rebase from Here…**. The interactive rebase dialog lets you pick, reword, edit, squash, fixup, drop and reorder commits. These actions are disabled for commits that are already pushed. +From the log you can **Edit Commit Message…**, **Fixup…**, **Squash Into…**, **Drop Commit**, **Squash Commits…** and **Interactively Rebase from Here…**. The interactive rebase dialog lets you pick, reword, edit, squash, fixup, drop and reorder commits. These actions also work for commits that are already pushed: rewriting them is confirmed first, and pushing afterwards needs a force push. Commits already pushed from a protected branch cannot be rewritten. Limits: interactive rebase doesn't support ranges with merge commits, and **Undo Commit…** works only on the latest commit (its changes go back to a changelist). diff --git a/docs/zh/guide/vcs.mdx b/docs/zh/guide/vcs.mdx index e19eec7e2..3079719b7 100644 --- a/docs/zh/guide/vcs.mdx +++ b/docs/zh/guide/vcs.mdx @@ -181,7 +181,7 @@ Git 工具窗口的 **日志** 标签页显示提交图。 ### 改写历史 -在日志中可以 **编辑提交消息…**、**修补提交…**、**压缩到…**、**丢弃提交**、**压缩提交…** 和 **从此处进行交互式变基…**。交互式变基对话框可以对提交执行 pick、reword、edit、squash、fixup、drop，并调整顺序。已推送的提交不能使用这些操作。 +在日志中可以 **编辑提交消息…**、**修补提交…**、**压缩到…**、**丢弃提交**、**压缩提交…** 和 **从此处进行交互式变基…**。交互式变基对话框可以对提交执行 pick、reword、edit、squash、fixup、drop，并调整顺序。这些操作也可以用于已推送的提交：改写前会先确认，之后推送需要强制推送。受保护分支上已推送的提交不能被改写。 限制：交互式变基不支持包含合并提交的范围；**撤消提交…** 只能用于最新的提交（其更改会放回某个更改列表）。 diff --git a/src/features/vcs/GitLog.tsx b/src/features/vcs/GitLog.tsx index 40179ad26..a762336e0 100644 --- a/src/features/vcs/GitLog.tsx +++ b/src/features/vcs/GitLog.tsx @@ -60,7 +60,6 @@ import { goToChild, goToParent, initLogBranchFilter, - isPushed, isMine, loadMore, logState, @@ -1155,10 +1154,7 @@ export function commitMenu(c: Commit, selection: Commit[] = [c]): MenuItem[] { const vcs = useVcs.getState(); const head = (repo ? vcs.heads[repo]?.head : vcs.branch?.head) ?? null; const { outgoing } = useGitLog.getState(); - const pushed = (x: Commit) => - commitRepo(x) === undefined ? !!outgoing && !outgoing.has(x.id) : isPushed(commitKey(x)); if (many) { - const anyPushed = selection.some(pushed); return [ { kind: "action", action: "vcs.log.copyRevision", arg: selection, label: "Copy Revision Numbers" }, { kind: "action", action: "vcs.log.createPatch", arg: selection }, @@ -1169,11 +1165,10 @@ export function commitMenu(c: Commit, selection: Commit[] = [c]): MenuItem[] { { kind: "separator" }, { kind: "action", action: "vcs.log.revert", arg: selection, label: "Revert Commits" }, { kind: "separator" }, - { kind: "action", action: "vcs.log.squashCommits", arg: selection, disabled: anyPushed }, - { kind: "action", action: "vcs.log.dropCommit", arg: selection, label: "Drop Commits", disabled: anyPushed }, + { kind: "action", action: "vcs.log.squashCommits", arg: selection }, + { kind: "action", action: "vcs.log.dropCommit", arg: selection, label: "Drop Commits" }, ]; } - const pushedCommit = pushed(c); // Without an upstream there is no outgoing set: allow pushing up to any commit of HEAD. const canPushHere = !!head && @@ -1214,11 +1209,11 @@ export function commitMenu(c: Commit, selection: Commit[] = [c]): MenuItem[] { { kind: "action", action: "vcs.log.revert", arg: c }, { kind: "action", action: "vcs.log.undoCommit", arg: c }, { kind: "separator" }, - { kind: "action", action: "vcs.log.editMessage", arg: c, disabled: pushedCommit }, - { kind: "action", action: "vcs.log.fixup", arg: c, disabled: pushedCommit }, - { kind: "action", action: "vcs.log.squashInto", arg: c, disabled: pushedCommit }, - { kind: "action", action: "vcs.log.dropCommit", arg: c, disabled: pushedCommit }, - { kind: "action", action: "vcs.log.interactiveRebase", arg: c, disabled: pushedCommit }, + { kind: "action", action: "vcs.log.editMessage", arg: c }, + { kind: "action", action: "vcs.log.fixup", arg: c }, + { kind: "action", action: "vcs.log.squashInto", arg: c }, + { kind: "action", action: "vcs.log.dropCommit", arg: c }, + { kind: "action", action: "vcs.log.interactiveRebase", arg: c }, { kind: "action", action: "vcs.log.pushUpToHere", arg: c, disabled: !canPushHere }, { kind: "separator" }, ...(branchActions.length > 2 ? [{ label: "Branches", submenu: branchActions }] : branchActions), diff --git a/src/features/vcs/__tests__/gitOps.test.ts b/src/features/vcs/__tests__/gitOps.test.ts index 7d9b7b37f..ba565c64b 100644 --- a/src/features/vcs/__tests__/gitOps.test.ts +++ b/src/features/vcs/__tests__/gitOps.test.ts @@ -26,6 +26,7 @@ import { moveRow, planIsNoop, planProblem, + protectedBranchProblem, rebaseRange, singleCommitPlan, squashPlan, @@ -131,6 +132,16 @@ describe("Undo Commit", () => { expect(headPushed(info({ upstream: "origin/main", ahead: 1 }))).toBe(false); expect(headPushed(info())).toBe(false); }); + + it("refuses history rewriting on a protected branch with an upstream", () => { + const problem = (head: string | null, upstream: string | null) => + protectedBranchProblem({ head, upstream }, (name) => /^(main|master)$/.test(name)); + expect(problem("main", "origin/main")).toMatch(/protected branch 'main'/); + expect(problem("master", "origin/master")).toMatch(/protected branch 'master'/); + expect(problem("feature", "origin/main")).toBeNull(); + expect(problem("main", null)).toBeNull(); + expect(problem(null, "origin/main")).toBeNull(); + }); }); describe("interactive rebase plan", () => { diff --git a/src/features/vcs/__tests__/historyOps.test.ts b/src/features/vcs/__tests__/historyOps.test.ts index bad25f9cb..09512b549 100644 --- a/src/features/vcs/__tests__/historyOps.test.ts +++ b/src/features/vcs/__tests__/historyOps.test.ts @@ -84,22 +84,23 @@ describe("Git Log commit actions", () => { expect(items.every((i) => !("arg" in i) || i.arg === c)).toBe(true); }); - it("disables history rewriting for pushed commits", () => { + it("offers history rewriting for pushed commits too (IDEA allows it; the run confirms)", () => { const local = commit("Local", "1".repeat(40)); const pushed = commit("Pushed", "2".repeat(40)); useGitLog.setState({ outgoing: new Set([local.id]) }); + const rewriting = (c: Commit) => + commitMenu(c).flatMap((i) => + i.kind === "action" && + i.disabled && + ["editMessage", "fixup", "squashInto", "dropCommit", "interactiveRebase"].some((a) => i.action.endsWith(a)) + ? [i.action] + : [], + ); + expect(rewriting(local)).toEqual([]); + expect(rewriting(pushed)).toEqual([]); + // Push All up to Here keeps its own rule: it cannot push past a pushed commit. const disabled = (c: Commit) => commitMenu(c).flatMap((i) => (i.kind === "action" && i.disabled ? [i.action] : [])); - expect(disabled(local)).not.toContain("vcs.log.editMessage"); - expect(disabled(pushed)).toEqual( - expect.arrayContaining([ - "vcs.log.editMessage", - "vcs.log.fixup", - "vcs.log.squashInto", - "vcs.log.dropCommit", - "vcs.log.interactiveRebase", - "vcs.log.pushUpToHere", - ]), - ); + expect(disabled(pushed)).toContain("vcs.log.pushUpToHere"); useGitLog.setState({ outgoing: null }); }); diff --git a/src/features/vcs/logStore.ts b/src/features/vcs/logStore.ts index 381143115..c0150dfd4 100644 --- a/src/features/vcs/logStore.ts +++ b/src/features/vcs/logStore.ts @@ -819,13 +819,6 @@ async function loadOutgoing(id: string) { if (logState(id).generation === generation) setViewState(id, { outgoing, tracked }); } -/** The commit is on the upstream of the current branch (rewriting it needs a force push). */ -export function isPushed(id: string): boolean { - const { outgoing, tracked, commits } = useGitLog.getState(); - const c = commits.find((c) => commitKey(c) === id); - return !!c && tracked.has(commitRepo(c) ?? "") && !!outgoing && !outgoing.has(id); -} - export function loadMore(id = useGitLog.getState().shownLog): Promise { return loadPage(false, id); } diff --git a/src/features/vcs/rebase.tsx b/src/features/vcs/rebase.tsx index 58a325a1a..c839a7f9c 100644 --- a/src/features/vcs/rebase.tsx +++ b/src/features/vcs/rebase.tsx @@ -4,7 +4,7 @@ import { commitRepo } from "./multiRoot"; * Interactively Rebase from Here (IDEA's Git.Log.ContextMenu). */ import { useState } from "react"; -import type { Commit, RebaseAction, RebaseEntry } from "@/bindings"; +import type { BranchInfo, Commit, RebaseAction, RebaseEntry } from "@/bindings"; import { call, commands } from "@/platform/ipc"; import { Dialog, IconButton, Select, T } from "@/ui"; import { workflowSettings } from "@/platform/workflowSettings"; @@ -24,6 +24,7 @@ import { moveRow, planIsNoop, planProblem, + protectedBranchProblem, rebaseRange, singleCommitPlan, squashPlan, @@ -33,6 +34,7 @@ import { type RebaseRow, } from "./rebasePlan"; import { currentRepo, refreshNow, useVcs } from "./store"; +import { isProtectedBranch } from "./remoteUi"; import { notifications } from "@/workbench/contributions"; import styles from "./common.module.css"; import { tx } from "@/platform/i18n"; @@ -44,9 +46,43 @@ function refuse(title: string, body: string) { void showError(title, body); } +/** Pushed history of a protected branch cannot be rewritten (force push is not allowed there). */ +const protectedProblem = (branch: { head: string | null; upstream: string | null } | null) => + protectedBranchProblem(branch ?? { head: null, upstream: null }, isProtectedBranch); + const pushedWarning = (what: string) => `The commit is already pushed to ${useVcs.getState().branch?.upstream ?? "the remote"}. ${what} rewrites published history; pushing afterwards needs a force push.`; +/** + * Whether rewriting `range` (its oldest commit up to HEAD) touches history already + * pushed to the branch's upstream. Every common ancestor of HEAD and the upstream is + * an ancestor of their merge base, so the range is pushed iff its oldest commit is at + * or below that merge base. + */ +async function rangeTouchesUpstream( + branch: BranchInfo | null, + range: RebaseRange, + repo: string | null, +): Promise { + if (!branch?.upstream) return false; + if (headPushed(branch)) return true; + const base = await call(commands.vcsMergeBase(repo, "HEAD", branch.upstream)).catch(() => null); + if (!base) return false; + const oldest = range.commits[0]!.id; + const below = await call( + commands.vcsLog(repo, { + branch: null, + path: null, + author: null, + text: null, + range: `${base}..${oldest}`, + skip: 0, + limit: 1, + }), + ).catch(() => null); + return below !== null && below.length === 0; +} + // --------------------------------------------------------------------------- // Undo Commit @@ -54,7 +90,8 @@ export async function undoCommit(commit: Commit) { const repo = commitRepo(commit) ?? currentRepo(); await refreshNow(); const branch = useVcs.getState().branch; - const problem = undoCommitProblem(commit, branch); + // Protected branches refuse only rewrites of history that is already pushed. + const problem = undoCommitProblem(commit, branch) ?? (headPushed(branch) ? protectedProblem(branch) : null); if (problem) return refuse("Cannot undo commit", problem); if (headPushed(branch)) { const ok = await confirmDialog({ @@ -114,7 +151,13 @@ async function rangeFrom(target: Commit, verb: string): Promise<(RebaseRange & { refuse(`Cannot ${verb}`, range.error); return null; } - if (headPushed(branch)) { + if (await rangeTouchesUpstream(branch, range, repo)) { + // Protected branches refuse only rewrites of history that is already pushed. + const protectedBranch = protectedProblem(branch); + if (protectedBranch) { + refuse(`Cannot ${verb}`, protectedBranch); + return null; + } const ok = await confirmDialog({ title: verb.replace(/^\w/, (c) => c.toUpperCase()), message: pushedWarning(verb.replace(/^\w/, (c) => c.toUpperCase())), diff --git a/src/features/vcs/rebasePlan.ts b/src/features/vcs/rebasePlan.ts index d2ff2beca..5ad66bce5 100644 --- a/src/features/vcs/rebasePlan.ts +++ b/src/features/vcs/rebasePlan.ts @@ -43,6 +43,17 @@ export function headPushed(branch: BranchInfo | null): boolean { return !!branch?.upstream && branch.ahead === 0; } +/** Pushed history of a protected branch cannot be rewritten (force push is not allowed there). */ +export function protectedBranchProblem( + branch: { head: string | null; upstream: string | null }, + protectedBranch: (name: string) => boolean, +): string | null { + const name = branch.head; + // With no upstream there is no force push to prohibit, so the branch is not protected. + if (!name || !branch.upstream || !protectedBranch(name)) return null; + return `Pushed commits of the protected branch '${name}' cannot be rewritten`; +} + export function initialRows(commits: readonly Commit[]): RebaseRow[] { return autosquash(commits.map((commit) => ({ commit, action: "pick", message: commit.message }))); }