Refuse to start against a base branch that has fallen behind

src/agent_harness/__main__.py

@@ -681,6 +681,22 @@ def live_routes() -> dict[str, Chain]:
         if not clean:
             log.warning("--allow-dirty: %s", why)
 
+    # Applies to BOTH modes, unlike the check above. A worktree cut from a base
+    # that has fallen behind is the rdpapp failure exactly: session mode gives
+    # each item a pristine worktree, and a pristine worktree of the wrong
+    # lineage is still the wrong lineage (#180).
+    if args.work and args.base and (Path(args.work) / ".git").exists():
+        from .preflight import _base_is_current
+
+        current, where = _base_is_current(str(args.work), args.base)
+        if not current and not args.allow_stale_base:
+            print(f"refusing to start: {where}", file=sys.stderr)
+            return 2
+        if not current:
+            log.warning("--allow-stale-base: %s", where)
+        else:
+            log.info("base: %s", where)
+
     if demo_mode:
         from .demo import demo_transport
 
@@ -1264,6 +1280,15 @@ def main(argv: list[str] | None = None) -> int:
         "neither recoverably — so a dirty checkout is refused by default. Pass "
         "this only when the tree is genuinely disposable.",
     )
+    p_run.add_argument(
+        "--allow-stale-base",
+        action="store_true",
+        help="run against a base branch that has fallen well behind its upstream. "
+        "A stale base is the one wrong setting every later stage reports as a "
+        "success — the agent works, the checks pass, the reviewer approves, and "
+        "the commit lands on a lineage nobody develops on any more — so it is "
+        "refused by default. Pass this when the base really is the line of work.",
+    )
     p_run.add_argument(
         "--context-budget",
         type=int,

src/agent_harness/preflight.py

@@ -136,6 +136,129 @@ def _is_clean_tree(path: str) -> tuple[bool, str]:
     )
 
 
+#: How far behind its upstream a base may be before the run is refused. A few
+#: commits behind is the ordinary state of any branch and blocking on it would
+#: make the check noise nobody reads. Two dozen is a different repository.
+STALE_BASE_LIMIT = 25
+
+
+def _git(path: str, *args: str, timeout: float = 60.0) -> tuple[int, str, str]:
+    try:
+        result = subprocess.run(  # noqa: S603
+            ["git", "-C", path, *args],  # noqa: S607
+            capture_output=True,
+            text=True,
+            timeout=timeout,
+        )
+    except (OSError, subprocess.SubprocessError) as exc:
+        return (1, "", str(exc))
+    return (result.returncode, result.stdout.strip(), result.stderr.strip())
+
+
+def _base_is_current(path: str, base: str) -> tuple[bool, str]:
+    """Whether the base branch still resembles the line of work it came from.
+
+    **This is the one wrong configuration every downstream stage reports as a
+    success.** The agent works, the checks pass, the reviewer approves and the
+    commit lands — onto a branch nobody is developing on any more. There is no
+    failure to notice, so nothing notices, and the cost is not one item but
+    every item in the run.
+
+    Measured on `rdpapp`: a base cut from a local working tree turned out to be
+    121 commits behind `origin/master` and 27 ahead, carrying an alternate
+    implementation that was never promoted. Six items were delivered onto it
+    before a human who knew the lineage said so (#180).
+
+    Deliberately quiet about what it cannot answer. A branch with no upstream,
+    a repository with no remote, an unreachable remote — none of those are
+    evidence that the base is stale, and reporting them as failures would train
+    people to pass the override flag by default.
+    """
+    upstreams = _upstreams_for(path, base)
+    if not upstreams:
+        return (True, f"nothing to compare {base} against: no upstream and no remote default")
+
+    # Behind *every* candidate is the finding. A base cut locally for one run
+    # legitimately tracks nothing, and a repository can have several remotes
+    # only one of which is authoritative — so being current with any one of
+    # them is enough to believe the base is on a live line of work.
+    #
+    # The first draft of this returned early when a branch tracked nothing and
+    # the repository had more than one remote, which is exactly the shape of
+    # the case it was written for: `harness/base` tracked nothing, `rdpapp` has
+    # a Forgejo `origin` and a GitHub secondary, and the check said nothing at
+    # all while the base sat 121 commits behind.
+    results: list[tuple[int | None, str]] = []
+    for upstream in upstreams:
+        remote = upstream.split("/", 1)[0]
+        code, _, err = _git(path, "fetch", "--quiet", remote, timeout=120.0)
+        if code == 0 and _git(path, "merge-base", base, upstream)[0] != 0:
+            # No common ancestor: a different project that happens to be a
+            # remote of this checkout, not a line of work this base could have
+            # come from. Counting it would let an unrelated remote with two
+            # commits in it vouch for a base that is a hundred behind the one
+            # that matters.
+            continue
+        if code != 0:
+            # Unreachable is a fact about the network, not about the base.
+            results.append((None, f"could not reach {remote}: {err[:100]}"))
+            continue
+        code, counts, _ = _git(path, "rev-list", "--left-right", "--count", f"{base}...{upstream}")
+        if code != 0 or "\t" not in counts:
+            results.append((None, f"could not compare {base} against {upstream}"))
+            continue
+        ahead_s, _, behind_s = counts.partition("\t")
+        try:
+            ahead, behind = int(ahead_s), int(behind_s.strip())
+        except ValueError:  # pragma: no cover - git's output is two integers
+            results.append((None, f"could not read {base}...{upstream}"))
+            continue
+        results.append(
+            (
+                behind,
+                f"{behind} commit(s) behind {upstream}" + (f" and {ahead} ahead" if ahead else ""),
+            )
+        )
+
+    measured = [(behind, text) for behind, text in results if behind is not None]
+    if not measured:
+        return (True, f"could not compare {base}: " + "; ".join(text for _, text in results))
+
+    closest, detail = min(measured, key=lambda pair: pair[0])
+    if closest == 0:
+        return (True, f"{base} is current with {detail.split(' behind ', 1)[-1]}")
+    where = f"{base} is " + "; ".join(text for _, text in measured)
+    if closest <= STALE_BASE_LIMIT:
+        return (True, where)
+    return (
+        False,
+        f"{where}. Work based here lands on a lineage that has moved on, and every "
+        "stage after this one — the agent, the checks, the reviewer, the commit — "
+        "will report success while it happens. Rebase or cut a new base from the "
+        "current head, or pass --allow-stale-base if this really is the line of work.",
+    )
+
+
+def _upstreams_for(path: str, base: str) -> list[str]:
+    """Every remote ref this base could reasonably be measured against.
+
+    Its own tracking branch when it has one, since that is the answer the
+    person who made the branch already gave. Otherwise every remote's default
+    head — plural on purpose, because "which remote is authoritative" is not a
+    question a preflight check can answer and not one it should guess at.
+    """
+    code, upstream, _ = _git(path, "rev-parse", "--abbrev-ref", f"{base}@{{upstream}}")
+    if code == 0 and upstream:
+        return [upstream]
+    code, remotes, _ = _git(path, "remote")
+    heads = []
+    for remote in (name.strip() for name in remotes.splitlines() if name.strip()):
+        code, head, _ = _git(path, "symbolic-ref", "--short", f"refs/remotes/{remote}/HEAD")
+        if code == 0 and head:
+            heads.append(head)
+    return heads
+
+
 def _gh_can_write(repo: str) -> tuple[bool, str]:
     """Whether `gh` can actually write to the repo.
 
@@ -504,6 +627,8 @@ def preflight_project(
     disk_probe: Callable[[str, float], tuple[bool, str]] = disk_space_probe,
     clean_probe: Callable[[str], tuple[bool, str]] = _is_clean_tree,
     allow_dirty: bool = False,
+    base_probe: Callable[[str, str], tuple[bool, str]] = _base_is_current,
+    allow_stale_base: bool = False,
 ) -> Preflight:
     """Everything that must hold before this project can produce a pull request."""
     checks: list[Check] = []
@@ -555,6 +680,19 @@ def preflight_project(
                     else f"{clean_detail}{' Allowed by --allow-dirty.' if allow_dirty else ''}",
                 )
             )
+            base = str(getattr(project, "base_branch", "") or "")
+            if base:
+                base_ok, base_detail = base_probe(work_dir, base)
+                checks.append(
+                    Check(
+                        "base branch",
+                        base_ok or allow_stale_base,
+                        base_detail
+                        if base_ok
+                        else f"{base_detail}"
+                        + (" Allowed by --allow-stale-base." if allow_stale_base else ""),
+                    )
+                )
         disk_ok, disk_detail = disk_probe(
             work_dir, float(getattr(project, "min_free_disk_gb", 0.0) or 0.0)
         )

tests/test_preflight.py

@@ -16,6 +16,7 @@
 import pytest
 
 from agent_harness.preflight import (
+    STALE_BASE_LIMIT,
     Answer,
     _is_clean_tree,
     clean_checks_probe,
@@ -406,3 +407,123 @@ def test_allow_dirty_lets_it_start_and_still_says_what_is_at_risk(tmp_path: Path
     assert check["ok"]
     assert "3 uncommitted change(s)" in check["detail"]
     assert "Allowed by --allow-dirty" in check["detail"]
+
+
+# --------------------------------------------- a base that has fallen behind
+
+
+def _repo_with_remote(tmp_path: Path, *, behind: int, remote: str = "origin") -> Path:
+    """A checkout whose `work` branch is `behind` commits behind its remote."""
+    upstream = tmp_path / f"{remote}-upstream"
+    upstream.mkdir()
+    _git_init(upstream)
+    (upstream / "f.txt").write_text("0\n")
+    _run(upstream, "add", "-A")
+    _run(upstream, "commit", "-qm", "base")
+
+    local = tmp_path / f"local-{remote}-{behind}"
+    _run(tmp_path, "clone", "-q", str(upstream), str(local))
+    _run(local, "config", "user.email", "t@t")
+    _run(local, "config", "user.name", "t")
+    _run(local, "checkout", "-qb", "work")
+
+    for n in range(behind):
+        (upstream / "f.txt").write_text(f"{n + 1}\n")
+        _run(upstream, "add", "-A")
+        _run(upstream, "commit", "-qm", f"upstream {n}")
+    return local
+
+
+def _git_init(path: Path) -> None:
+    _run(path, "init", "-q", "-b", "main")
+    _run(path, "config", "user.email", "t@t")
+    _run(path, "config", "user.name", "t")
+
+
+def _run(path: Path, *args: str) -> str:
+    import subprocess
+
+    return subprocess.run(
+        ["git", "-C", str(path), *args], capture_output=True, text=True, check=True
+    ).stdout
+
+
+def test_a_base_far_behind_its_upstream_is_refused(tmp_path: Path) -> None:
+    """The rdpapp failure: 121 commits behind, and every later stage said ok."""
+    from agent_harness.preflight import _base_is_current
+
+    repo = _repo_with_remote(tmp_path, behind=STALE_BASE_LIMIT + 5)
+
+    ok, why = _base_is_current(str(repo), "work")
+
+    assert ok is False
+    assert f"{STALE_BASE_LIMIT + 5} commit(s) behind" in why
+    assert "--allow-stale-base" in why
+
+
+def test_a_base_slightly_behind_is_reported_but_not_refused(tmp_path: Path) -> None:
+    """Every branch is a few commits behind. Blocking on that is noise nobody reads."""
+    from agent_harness.preflight import _base_is_current
+
+    repo = _repo_with_remote(tmp_path, behind=2)
+
+    ok, why = _base_is_current(str(repo), "work")
+
+    assert ok is True
+    assert "2 commit(s) behind" in why
+
+
+def test_a_current_base_says_so(tmp_path: Path) -> None:
+    from agent_harness.preflight import _base_is_current
+
+    repo = _repo_with_remote(tmp_path, behind=0)
+
+    ok, why = _base_is_current(str(repo), "work")
+
+    assert ok is True
+    assert "current with" in why
+
+
+def test_a_branch_tracking_nothing_is_still_measured_against_every_remote(
+    tmp_path: Path,
+) -> None:
+    """The bug in the first draft of this check, and the exact rdpapp shape.
+
+    `harness/base` tracked nothing and the repository had two remotes, so the
+    check returned "no single obvious line of work to compare against" and said
+    nothing while the base sat 121 commits behind the authoritative one.
+    """
+    from agent_harness.preflight import _base_is_current
+
+    repo = _repo_with_remote(tmp_path, behind=STALE_BASE_LIMIT + 5)
+    # A second remote, exactly as rdpapp has a Forgejo origin and a GitHub one.
+    other = tmp_path / "second"
+    other.mkdir()
+    _git_init(other)
+    (other / "g.txt").write_text("x\n")
+    _run(other, "add", "-A")
+    _run(other, "commit", "-qm", "unrelated")
+    _run(repo, "remote", "add", "github", str(other))
+    _run(repo, "fetch", "-q", "github")
+    # `work` has no upstream by construction — it was cut locally with
+    # `checkout -b`, which is the ordinary case and the one that went unnoticed.
+
+    ok, why = _base_is_current(str(repo), "work")
+
+    assert ok is False, why
+    assert "behind" in why
+
+
+def test_an_unreachable_remote_is_not_evidence_that_the_base_is_stale(
+    tmp_path: Path,
+) -> None:
+    """A network fact must not be reported as a fact about the lineage."""
+    from agent_harness.preflight import _base_is_current
+
+    repo = _repo_with_remote(tmp_path, behind=0)
+    _run(repo, "remote", "set-url", "origin", str(tmp_path / "does-not-exist"))
+
+    ok, why = _base_is_current(str(repo), "work")
+
+    assert ok is True
+    assert "could not" in why