From 209c5d01f84fb22b8469f94cbe59f0974f976101 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:23:06 +0300 Subject: [PATCH] fix: the updater survives an upstream history rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git pull --ff-only cannot cross a rewritten history — every local commit has a new hash, so git reports divergence and the in-app update dead-ends in a git hint. On that failure the updater now asks git cherry whether any local commit's CONTENT is missing upstream: if none is, nothing can be lost, so it realigns to origin and reports that it did; if there is real local work, it refuses and says how many commits would go. A cherry that fails to run counts as 'not proven safe'. Co-Authored-By: Claude Opus 5 --- movie_editor/backend/git_update.py | 33 ++++++++++++++++- movie_editor/tests/test_git_update.py | 53 +++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/movie_editor/backend/git_update.py b/movie_editor/backend/git_update.py index fab02f24..616babee 100644 --- a/movie_editor/backend/git_update.py +++ b/movie_editor/backend/git_update.py @@ -73,6 +73,17 @@ def _ahead_behind(branch: str) -> tuple[int, int]: return 0, 0 +def _unique_local_commits(branch: str) -> list[str]: + """Local commits with no patch-equivalent upstream — i.e. real work that a reset + would destroy. `git cherry` marks those "+" and marks "-" the ones whose CONTENT is + already upstream under a different hash, which is exactly what a rewritten history + leaves behind.""" + proc = _run_git("cherry", f"origin/{branch}", branch) + if proc.returncode != 0: + return [""] # can't prove it's safe → treat as if there were local work + return [ln[2:].strip() for ln in (proc.stdout or "").splitlines() if ln.startswith("+")] + + def _list_branches() -> list[str]: proc = _run_git("branch", "-a", "--format=%(refname:short)") if proc.returncode != 0: @@ -133,16 +144,34 @@ def pull(branch: str | None = None) -> dict: if co.returncode != 0: raise GitUpdateError((co.stderr or co.stdout or "git checkout failed").strip()) pull_proc = _run_git("pull", "--ff-only", "origin", branch) + realigned = False if pull_proc.returncode != 0: msg = (pull_proc.stderr or pull_proc.stdout or "git pull failed").strip() - raise GitUpdateError(msg) + # A fast-forward is impossible once upstream history has been rewritten: every + # local commit is a different hash, so git reports the branches as diverged even + # though the content matches. Only realign when nothing would actually be lost — + # a clean tree (checked above) and no local commit whose content is missing + # upstream. Anything else keeps the original error and the user's work. + unique = _unique_local_commits(branch) + if unique: + raise GitUpdateError( + f"{msg}\n\nThis checkout has {len(unique)} local commit(s) that are not on " + f"origin/{branch}. Push or move them first — updating would discard them.") + reset = _run_git("reset", "--hard", f"origin/{branch}") + if reset.returncode != 0: + raise GitUpdateError(msg) + realigned = True after = _current_commit() return { "branch": branch, "before": before, "after": after, "updated": before != after, - "output": (pull_proc.stdout or "").strip(), + # Surfaced so the update reads as what it was, not a silent jump to another commit. + "realigned": realigned, + "output": ((pull_proc.stdout or "").strip() if not realigned + else f"Upstream history was rewritten — realigned to origin/{branch}. " + "No local commits were lost."), } diff --git a/movie_editor/tests/test_git_update.py b/movie_editor/tests/test_git_update.py index 9b2537b3..281d5079 100644 --- a/movie_editor/tests/test_git_update.py +++ b/movie_editor/tests/test_git_update.py @@ -116,3 +116,56 @@ def test_checkout_switches_branch(monkeypatch): result = git_update.checkout("cutting_edge", pull_after=False) assert result["branch"] == "cutting_edge" assert result["before_branch"] == "dev" + + +def _rewritten_upstream(cherry_stdout, *, reset_ok=True): + """A checkout whose branch diverged because upstream history was rewritten.""" + def fake_run(*args, **kwargs): + if args == ("rev-parse", "--abbrev-ref", "HEAD"): + return _proc(stdout="dev\n") + if args == ("rev-parse", "--short", "HEAD"): + return _proc(stdout="old1234\n") + if args == ("status", "--porcelain"): + return _proc(stdout="") + if args == ("fetch", "--prune", "origin"): + return _proc() + if args == ("pull", "--ff-only", "origin", "dev"): + return _proc(code=1, stderr="fatal: Not possible to fast-forward, aborting.") + if args == ("cherry", "origin/dev", "dev"): + return _proc(stdout=cherry_stdout) + if args == ("reset", "--hard", "origin/dev"): + return _proc() if reset_ok else _proc(code=1, stderr="reset failed") + return _proc(code=1, stderr="unexpected") + return fake_run + + +def test_pull_realigns_after_upstream_history_rewrite(monkeypatch): + """History was rewritten upstream, so every local commit has a new hash and no + fast-forward exists. `git cherry` marks them "-" (content already upstream), which + means a reset loses nothing — update instead of dead-ending the user in git.""" + monkeypatch.setattr(git_update, "_run_git", _rewritten_upstream("- aaa111\n- bbb222\n")) + res = git_update.pull("dev") + assert res["realigned"] is True + assert res["updated"] is False # same short hash from the stub; the reset ran + assert "rewritten" in res["output"] and "No local commits were lost" in res["output"] + + +def test_pull_refuses_to_realign_over_real_local_work(monkeypatch): + """A "+" from `git cherry` is a local commit whose content is NOT upstream. Resetting + would destroy it, so the update has to stop and say so.""" + monkeypatch.setattr(git_update, "_run_git", _rewritten_upstream("- aaa111\n+ ccc333\n")) + with pytest.raises(git_update.GitUpdateError) as e: + git_update.pull("dev") + assert "1 local commit" in str(e.value) + assert "discard" in str(e.value) + + +def test_pull_keeps_the_original_error_when_it_cannot_prove_safety(monkeypatch): + """`git cherry` itself failing is not evidence of safety — keep the plain failure.""" + def fake_run(*args, **kwargs): + if args == ("cherry", "origin/dev", "dev"): + return _proc(code=1, stderr="unknown revision") + return _rewritten_upstream("")(*args, **kwargs) + monkeypatch.setattr(git_update, "_run_git", fake_run) + with pytest.raises(git_update.GitUpdateError): + git_update.pull("dev")