fix(git): never discard committed local work in rebase_onto_base (#683)

The shared rebase primitive (dev sync_branch verb, PM/CEO rebase path,
submit-freshen, merge-conflict resolver) opened with fetch -> checkout ->
unconditional reset --hard origin/<head_branch>. The dirty-tree gate
protects uncommitted edits only; the reset silently rewound past every
committed-but-unpushed commit — and the commit do-verb never pushes, so
mid-rework a dev routinely has exactly that. The force-with-lease push
then republished the truncated branch as authoritative (the lease
matched the freshly-fetched, never-moved origin ref).

rebase_onto_base now classifies local vs origin/<head> post-fetch:
- behind/equal: reset --hard origin as before (origin loses nothing)
- strictly ahead: reset skipped — the rebase runs from the local tip and
  the lease'd push publishes the previously-doomed commits
- diverged: a patch-equivalence probe (rev-list --right-only
  --cherry-pick) first rescues the self-inflicted residue of a prior
  rebase whose force-push failed (treated as ahead, self-heals on
  retry); only genuine two-sided divergence returns a new
  {status: diverged, local_only, origin_only} — no reset, no rebase,
  no push, neither side silently discarded
- an absent local ref is recovered from origin (branch + checkout,
  never reset)

Callers: the sync_branch verb maps diverged to an actionable envelope
steering to i_am_blocked (stash-preserved note included); the
submit-freshen hard-rejects it like conflicts; the merge-conflict
resolver already escalates any non-rebased/superseded status to the
CEO and degrades gracefully (pinned by test, no code change).

New real-git suite (bare origin + clone, no subprocess mocking)
asserts origin-side outcomes: ahead-publishes, behind-adopts,
diverged-refuses-untouched, absent-ref recovery, superseded,
conflicts, and wedge self-heal on retry via a rejecting pre-receive
hook.

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
2026-07-24 14:34:25 +02:00
committed by GitHub
co-authored by Renn F
parent 3516d925fe
commit 71f5426e40
9 changed files with 744 additions and 63 deletions
+8 -2
View File
@@ -1463,7 +1463,10 @@ async def test_rebase_onto_base_stash_true_auto_stashes_and_pops() -> None:
if args[:2] == ["status", "--porcelain"]:
res.stdout = " M dirty.py\n"
elif args[:2] == ["rev-list", "--count"]:
res.stdout = "1"
# Only the post-rebase unique-vs-base count is non-zero; the
# pre-rebase local-vs-origin(HEAD) classification reads as
# "nothing unique on either side" (behind/equal, not diverged).
res.stdout = "1" if args[2] == "origin/master..HEAD" else "0"
else:
res.stdout = ""
return res
@@ -1499,7 +1502,10 @@ async def test_rebase_onto_base_stash_pop_conflict_preserves_stash() -> None:
if args[:2] == ["status", "--porcelain"]:
res.stdout = " M dirty.py\n"
elif args[:2] == ["rev-list", "--count"]:
res.stdout = "1"
# Only the post-rebase unique-vs-base count is non-zero; the
# pre-rebase local-vs-origin(HEAD) classification reads as
# "nothing unique on either side" (behind/equal, not diverged).
res.stdout = "1" if args[2] == "origin/master..HEAD" else "0"
elif args == ["stash", "pop"]:
res.returncode = 1 # pop conflicted — stash is NOT dropped by git
else:
+99 -30
View File
@@ -94,21 +94,27 @@ async def test_success_path_returns_rebased_and_does_not_call_abort(
Call sequence for the success path (rebase OK, 2 unique commits):
[0] status --porcelain clean (the H8 dirty-tree gate)
[1] fetch origin
[2] checkout HEAD branch
[3] reset --hard origin/HEAD
[4] rebase origin/BASE exits 0
[5] rev-list --count returns "2"
[6] push --force-with-lease pushes the rebased branch
[2] rev-parse --verify --quiet local HEAD ref exists
[3] checkout HEAD branch
[4] rev-list --count origin/H..HEAD local_only=0
[5] rev-list --count HEAD..origin/H origin_only=0 (not diverged)
[6] reset --hard origin/HEAD local has nothing unique
[7] rebase origin/BASE exits 0
[8] rev-list --count returns "2"
[9] push --force-with-lease pushes the rebased branch
"""
run = AsyncMock(
side_effect=[
_result(stdout=""), # [0] status --porcelain → clean
_result(), # [1] fetch
_result(), # [2] checkout
_result(), # [3] reset
_result(), # [4] rebase ← success
_result(stdout="2\n"), # [5] rev-list
_result(), # [6] push
_result(returncode=0), # [2] rev-parse --verify (local ref exists)
_result(), # [3] checkout
_result(stdout="0\n"), # [4] rev-list origin/H..HEAD → local_only=0
_result(stdout="0\n"), # [5] rev-list HEAD..origin/H → origin_only=0
_result(), # [6] reset (local not ahead → reset)
_result(), # [7] rebase ← success
_result(stdout="2\n"), # [8] rev-list origin/BASE..HEAD
_result(), # [9] push
]
)
monkeypatch.setattr(GitService, "_run_git", run)
@@ -148,21 +154,27 @@ async def test_conflict_path_calls_diff_then_abort_and_returns_conflict_files(
Call sequence:
[0] status --porcelain clean (the H8 dirty-tree gate)
[1] fetch origin
[2] checkout HEAD branch
[3] reset --hard origin/HEAD
[4] rebase origin/BASE exits 1 (conflict)
[5] diff --name-only lists conflicted files
[6] rebase --abort exits 0
[2] rev-parse --verify --quiet local HEAD ref exists
[3] checkout HEAD branch
[4] rev-list --count origin/H..HEAD local_only=0
[5] rev-list --count HEAD..origin/H origin_only=0 (not diverged)
[6] reset --hard origin/HEAD local has nothing unique
[7] rebase origin/BASE exits 1 (conflict)
[8] diff --name-only lists conflicted files
[9] rebase --abort exits 0
"""
run = AsyncMock(
side_effect=[
_result(stdout=""), # [0] status --porcelain → clean
_result(), # [1] fetch
_result(), # [2] checkout
_result(), # [3] reset
_result(returncode=1), # [4] rebase ← conflict
_result(stdout="src/a.py\nsrc/b.py\n"), # [5] diff
_result(), # [6] rebase --abort
_result(returncode=0), # [2] rev-parse --verify (local ref exists)
_result(), # [3] checkout
_result(stdout="0\n"), # [4] rev-list origin/H..HEAD → local_only=0
_result(stdout="0\n"), # [5] rev-list HEAD..origin/H → origin_only=0
_result(), # [6] reset (local not ahead → reset)
_result(returncode=1), # [7] rebase ← conflict
_result(stdout="src/a.py\nsrc/b.py\n"), # [8] diff
_result(), # [9] rebase --abort
]
)
monkeypatch.setattr(GitService, "_run_git", run)
@@ -214,21 +226,27 @@ async def test_resilience_when_both_rebase_and_abort_fail_returns_conflict_no_ex
Call sequence:
[0] status --porcelain clean (the H8 dirty-tree gate)
[1] fetch origin
[2] checkout HEAD branch
[3] reset --hard origin/HEAD
[4] rebase origin/BASE exits 1 (conflict)
[5] diff --name-only lists conflicted files
[6] rebase --abort exits 1 (abort also fails)
[2] rev-parse --verify --quiet local HEAD ref exists
[3] checkout HEAD branch
[4] rev-list --count origin/H..HEAD local_only=0
[5] rev-list --count HEAD..origin/H origin_only=0 (not diverged)
[6] reset --hard origin/HEAD local has nothing unique
[7] rebase origin/BASE exits 1 (conflict)
[8] diff --name-only lists conflicted files
[9] rebase --abort exits 1 (abort also fails)
"""
run = AsyncMock(
side_effect=[
_result(stdout=""), # [0] status --porcelain → clean
_result(), # [1] fetch
_result(), # [2] checkout
_result(), # [3] reset
_result(returncode=1), # [4] rebase ← conflict
_result(stdout="src/conflict.py\n"), # [5] diff
_result(returncode=1), # [6] rebase --abort ← also fails
_result(returncode=0), # [2] rev-parse --verify (local ref exists)
_result(), # [3] checkout
_result(stdout="0\n"), # [4] rev-list origin/H..HEAD → local_only=0
_result(stdout="0\n"), # [5] rev-list HEAD..origin/H → origin_only=0
_result(), # [6] reset (local not ahead → reset)
_result(returncode=1), # [7] rebase ← conflict
_result(stdout="src/conflict.py\n"), # [8] diff
_result(returncode=1), # [9] rebase --abort ← also fails
]
)
monkeypatch.setattr(GitService, "_run_git", run)
@@ -246,6 +264,57 @@ async def test_resilience_when_both_rebase_and_abort_fail_returns_conflict_no_ex
assert result == {"status": "conflicts", "files": ["src/conflict.py"]}
# ---------------------------------------------------------------------------
# Test 4 — diverged: both sides carry unique commits, neither is touched
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_diverged_path_returns_diverged_and_touches_nothing(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""When local HEAD and origin/HEAD each carry commits the other lacks
AND the patch-equivalence probe finds a real origin-only commit (no
local match), the method refuses immediately no reset, no rebase, no
push.
Call sequence:
[0] status --porcelain clean (the H8 dirty-tree gate)
[1] fetch origin
[2] rev-parse --verify --quiet local HEAD ref exists
[3] checkout HEAD branch
[4] rev-list --count origin/H..HEAD local_only=2
[5] rev-list --count HEAD..origin/H origin_only=1
[6] rev-list --count --right-only --cherry-pick HEAD...origin/H
1 origin-only commit survives patch-equivalence DIVERGED
"""
run = AsyncMock(
side_effect=[
_result(stdout=""), # [0] status --porcelain → clean
_result(), # [1] fetch
_result(returncode=0), # [2] rev-parse --verify (local ref exists)
_result(), # [3] checkout
_result(stdout="2\n"), # [4] rev-list origin/H..HEAD → local_only=2
_result(stdout="1\n"), # [5] rev-list HEAD..origin/H → origin_only=1
_result(stdout="1\n"), # [6] cherry-pick probe → 1 genuinely unmatched
]
)
monkeypatch.setattr(GitService, "_run_git", run)
svc = _git_service()
result = await svc.rebase_onto_base(
_WORKSPACE,
head_branch=_HEAD,
base_branch=_BASE,
git_token=_TOKEN,
)
assert result == {"status": "diverged", "local_only": 2, "origin_only": 1}
# Nothing past the classification ran: no reset, no rebase, no push.
subcommands = {c.args[1][0] for c in run.call_args_list}
assert subcommands.isdisjoint({"reset", "rebase", "push"})
# ---------------------------------------------------------------------------
# Safety gate tests for rebase() — protected-branch guard
# ---------------------------------------------------------------------------
@@ -0,0 +1,305 @@
"""Real-git regression tests for the committed-work-preserving reconciliation
in ``rebase_onto_base``.
The historical bug (live, struck repeatedly): the preamble ran an
unconditional ``git reset --hard origin/<head_branch>`` right after checkout,
which rewound the local branch to the last-PUSHED tip silently discarding
every committed-but-unpushed commit, since the ``commit`` do-verb never
pushes. The subsequent ``push --force-with-lease`` then succeeded (the lease
matches the freshly-fetched origin ref, which never moved) and republished
the truncated branch as authoritative.
These run against a REAL bare origin + real clones (no mocked ``_run_git``)
the bug lived exactly in real-git ref semantics, and a mock can't
reproduce a fast-forward/divergence classification bug like this one.
"""
from __future__ import annotations
import subprocess
from typing import TYPE_CHECKING
from unittest.mock import MagicMock
import pytest
from roboco.exceptions import GitCommandError
from roboco.services.git import GitService
if TYPE_CHECKING:
from pathlib import Path
_HEAD = "feature/backend/task"
_BASE = "master"
def _git(repo: Path, *args: str) -> subprocess.CompletedProcess[str]:
return subprocess.run(
["git", *args], cwd=repo, check=True, capture_output=True, text=True
)
def _git_ok(repo: Path, *args: str) -> subprocess.CompletedProcess[str]:
"""Non-raising variant for existence probes."""
return subprocess.run(
["git", *args], cwd=repo, check=False, capture_output=True, text=True
)
def _init_bare(path: Path) -> None:
path.mkdir(parents=True, exist_ok=True)
subprocess.run(
["git", "init", "--bare", "--initial-branch=master", str(path)],
check=True,
capture_output=True,
)
def _configure(repo: Path) -> None:
_git(repo, "config", "user.email", "t@example.com")
_git(repo, "config", "user.name", "T")
_git(repo, "config", "commit.gpgsign", "false")
def _clone(origin: Path, dest: Path) -> None:
subprocess.run(
["git", "clone", str(origin), str(dest)], check=True, capture_output=True
)
_configure(dest)
def _commit(repo: Path, name: str, content: str) -> None:
(repo / name).write_text(content)
_git(repo, "add", name)
_git(repo, "commit", "-m", f"add {name}")
def _log_subjects(repo: Path, ref: str) -> list[str]:
out = _git(repo, "log", "--format=%s", ref)
return out.stdout.splitlines()
def _rev(repo: Path, ref: str) -> str:
return _git(repo, "rev-parse", ref).stdout.strip()
def _service() -> GitService:
svc = GitService.__new__(GitService)
svc.log = MagicMock()
svc.session = MagicMock()
return svc
@pytest.fixture
def repo_pair(tmp_path: Path) -> tuple[Path, Path]:
"""A bare origin plus a working clone, both carrying a pushed task branch
(``master`` with one root commit, ``_HEAD`` branched off it with one more)."""
origin = tmp_path / "origin.git"
_init_bare(origin)
seed = tmp_path / "seed"
_clone(origin, seed)
_commit(seed, "README.md", "root\n")
_git(seed, "push", "origin", "master")
_git(seed, "checkout", "-b", _HEAD)
_commit(seed, "feature.py", "v1\n")
_git(seed, "push", "origin", _HEAD)
work = tmp_path / "work"
_clone(origin, work)
_git(work, "checkout", _HEAD)
return origin, work
@pytest.mark.asyncio
async def test_local_ahead_survives_sync_and_reaches_origin(
repo_pair: tuple[Path, Path],
) -> None:
"""(a) Committed-but-unpushed local work is a superset of origin — the
reset is skipped and the rebase + force-push PUBLISHES it instead of
discarding it (the historical data-loss bug)."""
origin, work = repo_pair
_commit(work, "fix.py", "unpushed fix\n") # committed locally, never pushed
svc = _service()
result = await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result["status"] == "rebased"
assert "add fix.py" in _log_subjects(origin, _HEAD)
@pytest.mark.asyncio
async def test_local_behind_adopts_origin_unchanged(
repo_pair: tuple[Path, Path],
) -> None:
"""(b) Local has nothing origin lacks (a sibling pushed while this clone
sat idle) resets to origin exactly as before the fix."""
origin, work = repo_pair
other = origin.parent / "other"
_clone(origin, other)
_git(other, "checkout", _HEAD)
_commit(other, "sibling.py", "from another clone\n")
_git(other, "push", "origin", _HEAD)
svc = _service()
result = await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result["status"] == "rebased"
assert "add sibling.py" in _log_subjects(work, _HEAD)
assert _rev(work, _HEAD) == _rev(work, f"origin/{_HEAD}")
@pytest.mark.asyncio
async def test_diverged_refuses_and_touches_neither_side(
repo_pair: tuple[Path, Path],
) -> None:
"""(c) Both local and origin carry unique commits — refuse outright;
neither branch nor origin is touched, nothing is pushed."""
origin, work = repo_pair
other = origin.parent / "other"
_clone(origin, other)
_git(other, "checkout", _HEAD)
_commit(other, "sibling.py", "from another clone\n")
_git(other, "push", "origin", _HEAD)
_commit(work, "local.py", "local-only work\n") # unpushed local commit
local_before = _rev(work, _HEAD)
origin_before = _rev(origin, _HEAD)
svc = _service()
result = await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result == {"status": "diverged", "local_only": 1, "origin_only": 1}
assert _rev(work, _HEAD) == local_before
assert _rev(origin, _HEAD) == origin_before
@pytest.mark.asyncio
async def test_local_ref_absent_recovers_from_origin(
repo_pair: tuple[Path, Path],
) -> None:
"""A workspace whose local ref for ``head_branch`` doesn't exist yet
(only fetched/tracked from origin, e.g. a bare clone-root caller) is
recovered checkout, never reset, since there's nothing local to lose."""
origin, _work = repo_pair
fresh = origin.parent / "fresh"
_clone(origin, fresh) # only master is checked out; _HEAD is origin-only
assert _git_ok(fresh, "rev-parse", "--verify", "--quiet", _HEAD).returncode != 0
svc = _service()
result = await svc.rebase_onto_base(
fresh, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result["status"] == "rebased"
assert _git_ok(fresh, "rev-parse", "--verify", "--quiet", _HEAD).returncode == 0
@pytest.mark.asyncio
async def test_superseded_still_returns_superseded(
repo_pair: tuple[Path, Path],
) -> None:
"""(d) A head whose work is already in base is still 'superseded'
byte-for-byte unchanged by the reconciliation step."""
origin, work = repo_pair
# Fast-forward base past the branch's own tip so the rebase leaves it
# with zero unique commits over base.
other = origin.parent / "other-base"
_clone(origin, other)
_git(other, "merge", "--no-ff", "-m", "merge feature", f"origin/{_HEAD}")
_git(other, "push", "origin", "master")
svc = _service()
result = await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result == {"status": "superseded"}
def _install_rejecting_hook(bare: Path) -> None:
"""Make every push to ``bare`` fail, simulating a push-side failure
(network blip, flow-verb timeout kill, container reap) after a rebase
already succeeded locally."""
hook = bare / "hooks" / "pre-receive"
hook.write_text("#!/bin/sh\nexit 1\n")
hook.chmod(0o755)
def _remove_rejecting_hook(bare: Path) -> None:
(bare / "hooks" / "pre-receive").unlink()
@pytest.mark.asyncio
async def test_self_inflicted_wedge_self_heals_on_retry(
repo_pair: tuple[Path, Path],
) -> None:
"""(e) A prior rebase that succeeded locally but whose force-push then
failed leaves local=rebased-history, origin=old-history both rev-list
counts positive by raw SHA. The patch-equivalence probe recognizes
origin's tip as fully rewritten into local's exclusive commits, so the
retry self-heals as 'rebased' instead of refusing as 'diverged' forever."""
origin, work = repo_pair
# Advance master so the rebase actually rewrites HEAD's commits' SHAs.
other = origin.parent / "other-base"
_clone(origin, other)
_commit(other, "base2.py", "advance master\n")
_git(other, "push", "origin", "master")
_commit(work, "fix.py", "unpushed fix\n") # local commit to be rebased
svc = _service()
_install_rejecting_hook(origin)
with pytest.raises(GitCommandError):
await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
_remove_rejecting_hook(origin)
result = await svc.rebase_onto_base(
work, head_branch=_HEAD, base_branch=_BASE, git_token=""
)
assert result["status"] == "rebased"
assert "add fix.py" in _log_subjects(origin, _HEAD)
@pytest.mark.asyncio
async def test_conflicts_still_refuses_and_reports_files(
repo_pair: tuple[Path, Path],
) -> None:
"""(d) A genuine rebase conflict is still reported exactly as before —
aborted, files listed, nothing pushed."""
origin, _work = repo_pair
# Branch off the ORIGINAL master and edit README.md.
conflict_head = "feature/backend/conflict"
conflicting = origin.parent / "conflicting"
_clone(origin, conflicting)
_git(conflicting, "checkout", "-b", conflict_head)
(conflicting / "README.md").write_text("branch change\n")
_git(conflicting, "add", "README.md")
_git(conflicting, "commit", "-m", "branch edits README")
_git(conflicting, "push", "origin", conflict_head)
# Move master with a COMPETING edit to the same line, so rebasing the
# branch onto the new master tip collides.
other = origin.parent / "other-base"
_clone(origin, other)
(other / "README.md").write_text("master change\n")
_git(other, "add", "README.md")
_git(other, "commit", "-m", "master edits README")
_git(other, "push", "origin", "master")
svc = _service()
result = await svc.rebase_onto_base(
conflicting, head_branch=conflict_head, base_branch=_BASE, git_token=""
)
assert result["status"] == "conflicts"
assert result["files"] == ["README.md"]
@@ -64,7 +64,14 @@ async def test_rebase_rebased_force_pushes_when_unique_commits() -> None:
pushed.append(args)
return _result()
if args[:2] == ["rev-list", "--count"]:
return _result(stdout="3\n")
# Only the post-rebase unique-vs-base count is non-zero; the
# pre-rebase local-vs-origin(HEAD) classification must read as
# "nothing unique on either side" or this would misclassify as
# diverged before the rebase ever runs.
range_spec = next(iter(args[2:]), "")
if range_spec == f"origin/{_BASE}..HEAD":
return _result(stdout="3\n")
return _result(stdout="0\n")
return _result()
with patch.object(svc, "_run_git", new=fake_run):