From 7cfeabc6b8c9f4d004d0c7f23d502fd20dac8628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Pen=CC=83alba?= Date: Tue, 14 Jul 2026 11:18:59 +0200 Subject: [PATCH] Don't offer undo for a pushed commit whose upstream ref was deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Undo of the last commit is withheld once the commit is pushed, so it can't rewrite published history. That check (isUnpushed) asks whether the tip is an ancestor of the branch's upstream — but a configured upstream can be gone from the remote (a stale local branch still tracking a deleted/renamed remote branch, e.g. `master` after the remote renamed its default to `main`). git keeps reporting the upstream, so the ancestor probe errored on the missing ref and the failure was read as "unpushed", wrongly offering to undo an already-published commit. When the upstream ref no longer resolves, fall back to the no-upstream test — "contained in any remote-tracking branch" — which still finds the commit on the remote and correctly reports it as pushed. Also enforce the pushed-commit guard inside the undo mutation's recorded path (not just the snapshot that decides whether to show the affordance), so a stale renderer snapshot or the "Undo Last Action" menu command can't slip a pushed tip past the check. A reset stays exempt: undoing it moves HEAD forward to restore commits and can never rewrite remote history. --- src/main/git/undo.test.ts | 34 ++++++++++++++++++++++++++++++++++ src/main/git/undo.ts | 31 ++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/main/git/undo.test.ts b/src/main/git/undo.test.ts index e05ede8..c965306 100644 --- a/src/main/git/undo.test.ts +++ b/src/main/git/undo.test.ts @@ -286,6 +286,40 @@ describe('readUndoSnapshot validity & snapshot integration', () => { expect((await readUndoSnapshot(dir, tip, 'origin/main'))?.kind).toBe('commit') }) + test('treats a pushed commit as pushed when its configured upstream ref was deleted', async () => { + // A stale branch still tracking a remote branch that no longer exists (the + // remote deleted it, or renamed its default). git keeps reporting the + // upstream (here 'origin/gone'), but the ref doesn't resolve — the tip must + // still count as pushed via any *other* remote-tracking branch that has it. + const dir = seedRepo() + put(dir, 'foo.txt', 'hi') + const tip = rawCommit(dir, 'add foo') + // The commit lives on a real remote branch, but NOT on the (missing) upstream. + git(dir, 'update-ref', 'refs/remotes/origin/main', tip) + + // Configured upstream 'origin/gone' has no ref: don't mistake the ancestor + // probe's failure for "unpushed" — fall back to remote-containment. + expect(await readUndoSnapshot(dir, tip, 'origin/gone')).toBeNull() + // And the mutation refuses too (no record → derived path), rather than + // rewriting published history. + await expect(undo(dir)).rejects.toThrow(/nothing to undo/i) + expect(head(dir)).toBe(tip) + }) + + test('the mutation refuses to undo a recorded op whose tip is already pushed', async () => { + const dir = seedRepo() + put(dir, 'foo.txt', 'hi') + await commitSelection(dir, 'add foo', COMMIT_ALL) // records this commit + const tip = head(dir) + // Configure an upstream that already contains the tip → pushed. + git(dir, 'update-ref', 'refs/remotes/origin/main', tip) + git(dir, 'config', 'branch.main.remote', 'origin') + git(dir, 'config', 'branch.main.merge', 'refs/heads/main') + + await expect(undo(dir)).rejects.toThrow(/already pushed/i) + expect(head(dir)).toBe(tip) // nothing rewritten + }) + test('a reset stays undoable even when its new tip is already pushed', async () => { const dir = seedRepo() put(dir, 'b.txt', 'b') diff --git a/src/main/git/undo.ts b/src/main/git/undo.ts index f1595ca..cb0eb94 100644 --- a/src/main/git/undo.ts +++ b/src/main/git/undo.ts @@ -102,18 +102,33 @@ async function currentUpstream(repoPath: string): Promise { } } +/** Whether `ref` resolves to a commit that actually exists in this repo. */ +async function refExists(repoPath: string, ref: string): Promise { + return runRead(repoPath, ['rev-parse', '--verify', '--quiet', `${ref}^{commit}`]).then( + (out) => out.trim() !== '', + () => false + ) +} + /** * Whether `sha` is unpublished — not yet on the remote. With an upstream, that's * "not an ancestor of the upstream" (ahead of it); without one, "not contained * in any remote-tracking branch". Mirrors GitHub Desktop's local-commits set, * which is exactly what gates whether an undo is offered. + * + * A configured upstream can be gone from the remote — e.g. a stale local branch + * still tracking a remote branch that was deleted (or renamed, like `master` → + * `main`). git keeps reporting that upstream, but its ref no longer resolves, so + * the ancestor probe would error and we'd read the failure as "unpushed" and + * wrongly offer to undo an already-published commit. When the upstream ref is + * missing, fall back to the no-upstream test (contained in *any* remote branch). */ async function isUnpushed( repoPath: string, sha: string, upstream: string | null ): Promise { - if (upstream) { + if (upstream && (await refExists(repoPath, upstream))) { return runRead(repoPath, ['merge-base', '--is-ancestor', sha, upstream]).then( () => false, () => true @@ -266,6 +281,20 @@ export async function undo(repoPath: string): Promise { const record = await readUndoRecord(repoPath) if (record && record.postSha === head) { + // Never rewrite published history. A recorded op is undoable only while its + // tip is unpushed — the same gate readUndoSnapshot uses to decide whether + // to even offer the affordance. This guards the mutation itself, so a stale + // renderer snapshot or the "Undo Last Action" menu command can't slip a + // pushed tip past the check. A reset is exempt: undoing it moves HEAD + // *forward* to restore commits, so it can never rewrite remote history. + if ( + record.kind !== 'reset' && + !(await isUnpushed(repoPath, head, await currentUpstream(repoPath))) + ) { + throw new Error( + 'This commit is already pushed, so undoing it would rewrite published history.' + ) + } if (record.kind === 'commit' && record.preSha === null) { await undoFirstCommit(repoPath) } else if (record.kind === 'commit') {