Skip to content

Don't offer undo for a pushed commit whose upstream ref was deleted - #82

Merged
danipen merged 1 commit into
mainfrom
fix-undo-pushed-commit
Jul 14, 2026
Merged

Don't offer undo for a pushed commit whose upstream ref was deleted#82
danipen merged 1 commit into
mainfrom
fix-undo-pushed-commit

Conversation

@danipen

@danipen danipen commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Problem

GitGrove offered to undo the last commit even when that commit was already pushed — it should refuse, so it never rewrites published history.

Seen on the Avalonia repo: on master (tip 842937d, which is an ancestor of origin/main and lives on many remote branches, so it is very much pushed), GitGrove offered the undo.

The pushed-commit protection does exist (isUnpushed), but it has the same root cause as #81: master is configured to track origin/master, and origin/master no longer exists on the remote (default renamed to main). git still reports the configured upstream, so:

git merge-base --is-ancestor 842937d origin/master   # errors: origin/master doesn't exist

The error was interpreted as "not an ancestor → unpushed", so the undo was offered for an already-published commit.

Fix

  1. Detection — when the configured upstream ref no longer resolves, fall back to the no-upstream test (branch -r --contains, i.e. "contained in any remote-tracking branch"). That finds 842937d on origin/main and correctly reports it as pushed. Verified against the real Avalonia repo: readUndoSnapshot now returns null (no affordance).

  2. Defense in depth — enforce the pushed-commit guard inside the undo() mutation's recorded path, not just the snapshot that decides whether to show the affordance. Previously the recorded path reset with no pushed check, so a stale renderer snapshot or the "Undo Last Action" menu command could slip a pushed tip past it. A reset stays exempt (undoing a reset moves HEAD forward to restore commits, and can't rewrite remote history) — matching the existing snapshot logic.

Tests

Two new integration tests in undo.test.ts:

  • A pushed commit whose configured upstream ref was deleted → still detected as pushed (snapshot withheld, mutation refuses).
  • The mutation refuses to undo a recorded op whose tip is already pushed.

bun test (811 pass), typecheck, and lint all green.

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.
@danipen
danipen merged commit aadff41 into main Jul 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant