From 26afeeaa57ffc0864d3162bc7d456402a8c28d0c Mon Sep 17 00:00:00 2001 From: svrf-maintainer Date: Sat, 3 Oct 2026 22:47:25 +0000 Subject: [PATCH 1/3] test: cross-check an optional tree provider against git on family merge steps --- tests/test_tree_provider.py | 241 ++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 tests/test_tree_provider.py diff --git a/tests/test_tree_provider.py b/tests/test_tree_provider.py new file mode 100644 index 0000000..02da048 --- /dev/null +++ b/tests/test_tree_provider.py @@ -0,0 +1,241 @@ +"""An optional tree provider for the family merges: an alternative merge engine whose +proposed tree is cross-checked against git's own merge tree on every merge step the +train builds. Git's tree is always computed; the provider's tree is used only when it +is the identical tree, and anything else (a different tree, a failure, a timeout, +unreadable output) keeps git's tree for that step and records why. Unset, nothing +changes.""" + +from __future__ import annotations + +import json +import tempfile +import time +import unittest +from pathlib import Path + +from fakes import Admission, Clock, DaemonGitHub, DaemonRepo, FakeGate, FakeGitHub, FakeRepo, git, is_union + +from svrf.config import ConfigError, from_dict +from svrf.daemon import Daemon +from svrf.git import RealGit +from svrf.train import Train +from svrf.tree_provider import TreeCommand + +MINIMAL = {"repo": "example/project", "gate": {"commands": ["make test"]}} + + +def train(repo, gh, gate, tmp, **kw): + clock = Clock() + kw.setdefault("jobs", 1) + return Train(repo, gh, gate, receipts=Path(tmp), clock=clock, sleep=clock.sleep, poll_seconds=0, + is_union=is_union, **kw) + + +class FakeProvider: + """Proposes the fake repository's merge tree (the union of both sides' lanes), or a + wrong tree, or fails, and records every merge step it was asked about.""" + + def __init__(self, repo, mode="same"): + self.repo, self.mode, self.calls = repo, mode, [] + + def propose(self, ours, theirs): + self.calls.append((ours, theirs)) + if self.mode == "fail": + raise RuntimeError("engine fell over") + if self.mode == "refuse": + return None, "PROVIDER_EXIT:1" + lanes = self.repo.lanes(ours) | self.repo.lanes(theirs) + if self.mode == "wrong": + lanes = lanes | {999} + return "T" + ",".join(str(n) for n in sorted(lanes)), None + + +class Configuration(unittest.TestCase): + def test_unset_by_default(self): + config = from_dict(MINIMAL) + self.assertEqual(config.merge.tree_command, "") + self.assertEqual(config.merge.tree_timeout_seconds, 120) + + def test_the_merge_section_takes_a_command_and_a_timeout(self): + config = from_dict({**MINIMAL, "merge": {"tree_command": "my-engine", "tree_timeout_seconds": 30}}) + self.assertEqual((config.merge.tree_command, config.merge.tree_timeout_seconds), ("my-engine", 30)) + + def test_an_unknown_merge_key_or_a_bad_timeout_is_refused(self): + with self.assertRaises(ConfigError): + from_dict({**MINIMAL, "merge": {"command": "my-engine"}}) + for bad in (0, -1, "soon", True): + with self.assertRaises(ConfigError): + from_dict({**MINIMAL, "merge": {"tree_command": "x", "tree_timeout_seconds": bad}}) + + def test_build_wires_a_provider_only_when_a_command_is_set(self): + from svrf.app import build + + with tempfile.TemporaryDirectory() as tmp: + base = {**MINIMAL, "clone": str(Path(tmp) / "clone"), "state_dir": str(Path(tmp) / "state")} + daemon = build(from_dict(base), github=object()) + self.assertNotIn("tree_provider", daemon.train_options) + daemon = build(from_dict({**base, "merge": {"tree_command": "my-engine", + "tree_timeout_seconds": 7}}), github=object()) + provider = daemon.train_options["tree_provider"] + self.assertIsInstance(provider, TreeCommand) + self.assertEqual((provider.command, provider.timeout), ("my-engine", 7)) + self.assertEqual(provider.root, (Path(tmp) / "clone").resolve()) + + +class TrainWithoutProvider(unittest.TestCase): + def test_unset_leaves_the_receipt_exactly_as_before(self): + with tempfile.TemporaryDirectory() as tmp: + repo = FakeRepo([11, 12]) + gh = FakeGitHub(repo) + receipt = train(repo, gh, FakeGate(repo), tmp).run([11, 12]) + self.assertEqual(gh.merged, [11, 12]) + self.assertNotIn("tree_provider", receipt) + for family in receipt["families"]: + for step in family["steps"]: + self.assertEqual(set(step), {"number", "commit", "tree"}) + for row in receipt["merges"]: + self.assertNotIn("tree_source", row) + + +class TrainWithProvider(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp() + + def test_an_identical_tree_is_used_and_every_family_merge_step_consults_the_provider(self): + repo = FakeRepo([11, 12]) + gh = FakeGitHub(repo) + provider = FakeProvider(repo) + receipt = train(repo, gh, FakeGate(repo), self.tmp, tree_provider=provider).run([11, 12]) + self.assertEqual(gh.merged, [11, 12]) + self.assertTrue(all(m["identity"] for m in receipt["merges"])) + # two steps folded for the plan, two recomputed while landing; the pair read asks nothing + self.assertEqual(len(provider.calls), 4) + self.assertEqual([ours for ours, _ in provider.calls], ["h11", "h12", "h11", "h12"]) + steps = receipt["families"][0]["steps"] + self.assertEqual([s["tree_source"] for s in steps], ["provider", "provider"]) + self.assertEqual([m["tree_source"] for m in receipt["merges"]], ["provider", "provider"]) + self.assertEqual(receipt["tree_provider"], {"consulted": 4, "provider": 4, "git": 0, "reasons": {}}) + + def test_a_different_tree_keeps_gits_tree_records_the_mismatch_and_still_lands(self): + repo = FakeRepo([11, 12]) + gh = FakeGitHub(repo) + receipt = train(repo, gh, FakeGate(repo), self.tmp, tree_provider=FakeProvider(repo, "wrong")).run([11, 12]) + self.assertEqual(gh.merged, [11, 12]) + self.assertEqual(repo.tree(repo.main), "T0,11,12") + self.assertTrue(all(m["identity"] for m in receipt["merges"])) + self.assertEqual(receipt["alerts"], []) + step = receipt["families"][0]["steps"][0] + self.assertEqual((step["tree_source"], step["tree_provider"]), ("git", "PROVIDER_MISMATCH")) + self.assertEqual(step["tree"], "T0,11") + self.assertEqual(step["proposed_tree"], "T0,11,999") + self.assertEqual(receipt["tree_provider"], + {"consulted": 4, "provider": 0, "git": 4, "reasons": {"PROVIDER_MISMATCH": 4}}) + + def test_a_failing_or_refusing_provider_never_fails_the_landing(self): + for mode, reason in (("fail", "PROVIDER_FAILED:RuntimeError"), ("refuse", "PROVIDER_EXIT:1")): + with self.subTest(mode=mode): + repo = FakeRepo([11, 12]) + gh = FakeGitHub(repo) + receipt = train(repo, gh, FakeGate(repo), tempfile.mkdtemp(), + tree_provider=FakeProvider(repo, mode)).run([11, 12]) + self.assertEqual(gh.merged, [11, 12]) + self.assertEqual([m["tree_source"] for m in receipt["merges"]], ["git", "git"]) + self.assertEqual([m["tree_provider"] for m in receipt["merges"]], [reason, reason]) + self.assertEqual(receipt["tree_provider"]["reasons"], {reason: 4}) + + def test_a_step_git_could_not_merge_is_never_offered_to_the_provider(self): + repo = FakeRepo([11, 12], main_conflicts={11: ["src/x.py"]}) + gh = FakeGitHub(repo) + provider = FakeProvider(repo) + receipt = train(repo, gh, FakeGate(repo), self.tmp, tree_provider=provider).run([11, 12]) + self.assertEqual(gh.merged, [12]) + self.assertEqual([h["number"] for h in receipt["holds"]], [11]) + self.assertNotIn("h11", [ours for ours, _ in provider.calls]) + self.assertEqual(receipt["tree_provider"]["consulted"], 2) + + def test_the_round_summary_and_state_carry_the_counts(self): + repo = DaemonRepo([11, 12]) + gh = DaemonGitHub(repo) + clock = Clock() + d = Daemon(repo, gh, FakeGate(repo), Admission(), state_dir=Path(self.tmp), receipts=Path(self.tmp) / "r", + clock=clock, sleep=clock.sleep, is_union=is_union, + train_options={"poll_seconds": 0, "tree_provider": FakeProvider(repo)}) + out = d.tick() + self.assertEqual(out["merged"], [11, 12]) + self.assertEqual(out["tree_provider"], {"consulted": 4, "provider": 4, "git": 0, "reasons": {}}) + state = json.loads((Path(self.tmp) / "state.json").read_text()) + self.assertEqual(state["last_tick"]["tree_provider"]["provider"], 4) + + def test_without_a_provider_the_round_summary_is_unchanged(self): + repo = DaemonRepo([11]) + gh = DaemonGitHub(repo) + clock = Clock() + d = Daemon(repo, gh, FakeGate(repo), Admission(), state_dir=Path(self.tmp), receipts=Path(self.tmp) / "r", + clock=clock, sleep=clock.sleep, is_union=is_union, train_options={"poll_seconds": 0}) + out = d.tick() + self.assertEqual(out["merged"], [11]) + self.assertNotIn("tree_provider", out) + + +class RealCommand(unittest.TestCase): + """The command against a real clone: what it is given, and how its answer is read.""" + + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()) + self.work = self.tmp / "work" + self.work.mkdir() + git(self.work, "init", "-q", "-b", "main") + (self.work / "base.txt").write_text("base\n") + git(self.work, "add", ".") + git(self.work, "commit", "-qm", "base") + self.base = git(self.work, "rev-parse", "HEAD") + for lane in ("a", "b"): + git(self.work, "checkout", "-q", "-b", lane, self.base) + (self.work / f"{lane}.txt").write_text(lane) + git(self.work, "add", ".") + git(self.work, "commit", "-qm", lane) + self.a = git(self.work, "rev-parse", "a") + self.b = git(self.work, "rev-parse", "b") + self.git_tree = git(self.work, "merge-tree", "--write-tree", self.b, self.a).splitlines()[0] + self.echo_git = 'git merge-tree --write-tree "$SVRF_MERGE_OURS" "$SVRF_MERGE_THEIRS" | head -1' + + def test_the_command_sees_ours_theirs_and_the_merge_bases_in_the_clone(self): + seen = self.tmp / "seen.txt" + command = f'printf "%s\\n" "$SVRF_MERGE_OURS" "$SVRF_MERGE_THEIRS" "$SVRF_MERGE_BASES" "$PWD" > {seen}; ' \ + + self.echo_git + tree, reason = TreeCommand(command, self.work, timeout=30).propose(self.b, self.a) + self.assertEqual((tree, reason), (self.git_tree, None)) + ours, theirs, bases, cwd = seen.read_text().splitlines() + self.assertEqual((ours, theirs, bases), (self.b, self.a, self.base)) + self.assertEqual(Path(cwd).resolve(), self.work.resolve()) + + def test_a_failure_a_timeout_or_unreadable_output_is_a_reason_never_a_tree(self): + cases = (("exit 3", "PROVIDER_EXIT:3"), + ("echo not-a-tree", "PROVIDER_OUTPUT_INVALID"), + ("true", "PROVIDER_OUTPUT_INVALID"), + ("sleep 20", "PROVIDER_TIMEOUT")) + for command, expected in cases: + with self.subTest(command=command): + started = time.monotonic() + tree, reason = TreeCommand(command, self.work, timeout=0.5).propose(self.b, self.a) + self.assertIsNone(tree) + self.assertTrue(reason.startswith(expected), reason) + self.assertLess(time.monotonic() - started, 10) + + def test_through_the_train_git_keeps_the_step_when_the_proposal_differs(self): + mismatch = TreeCommand('git rev-parse "$SVRF_MERGE_OURS^{tree}"', self.work, timeout=30) + same = TreeCommand(self.echo_git, self.work, timeout=30) + for provider, source in ((same, "provider"), (mismatch, "git")): + with self.subTest(source=source): + t = Train(RealGit(self.work), None, None, receipts=self.tmp / source, + pr_comments=False, status_checks=False, tree_provider=provider) + t.rows[2] = {"number": 2, "head_sha": self.b} + family = t.plan([2], self.a) + step = family["steps"][0] + self.assertEqual(step["tree"], self.git_tree) + self.assertEqual(step["tree_source"], source) + self.assertEqual(git(self.work, "rev-parse", f"{step['commit']}^{{tree}}"), self.git_tree) + + +if __name__ == "__main__": + unittest.main() From 4d28c4a4b95f6567a861c1ef4cb940b576c3bac6 Mon Sep 17 00:00:00 2001 From: svrf-maintainer Date: Sat, 3 Oct 2026 22:48:21 +0000 Subject: [PATCH 2/3] feat: optional tree provider for family merge steps, cross-checked against git --- src/svrf/app.py | 15 +++++--- src/svrf/config.py | 15 +++++++- src/svrf/daemon.py | 8 +++- src/svrf/train.py | 45 +++++++++++++++++++--- src/svrf/tree_provider.py | 79 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 13 deletions(-) create mode 100644 src/svrf/tree_provider.py diff --git a/src/svrf/app.py b/src/svrf/app.py index a70172d..50d6f5c 100644 --- a/src/svrf/app.py +++ b/src/svrf/app.py @@ -15,6 +15,7 @@ from .git import RealGit from .github import RealGitHub from .globs import PathSet +from .tree_provider import TreeCommand def ensure_clone(config: Config) -> None: @@ -56,6 +57,14 @@ def build(config: Config, *, github=None, dry_run: bool = False, clock=None, sle extra["clock"] = clock if sleep is not None: extra["sleep"] = sleep + train_options = {"jobs": config.train.jobs, "family_size": config.train.family_size, + "memory": memory, "rate_floor": config.train.rate_floor, + "max_rounds": config.train.max_rounds, "comment": config.train.comment, + "pr_comments": config.ui.pr_comments, "status_checks": config.ui.status_checks, + "dashboard_url": config.ui.dashboard_url} + if config.merge.tree_command: + train_options["tree_provider"] = TreeCommand(config.merge.tree_command, config.clone, + timeout=config.merge.tree_timeout_seconds) if config.demand_driver: try: module, factory = config.demand_driver.split(":", 1) @@ -76,9 +85,5 @@ def build(config: Config, *, github=None, dry_run: bool = False, clock=None, sle # but a command supplies the same class of refusal. reland=config.history.reland, kind=kind, history_verdict=lambda b, h: history.verdict(git, b, h, kind, prefixes), - train_options={"jobs": config.train.jobs, "family_size": config.train.family_size, - "memory": memory, "rate_floor": config.train.rate_floor, - "max_rounds": config.train.max_rounds, "comment": config.train.comment, - "pr_comments": config.ui.pr_comments, "status_checks": config.ui.status_checks, - "dashboard_url": config.ui.dashboard_url}, + train_options=train_options, **extra) diff --git a/src/svrf/config.py b/src/svrf/config.py index 9c2e673..13b9c3e 100644 --- a/src/svrf/config.py +++ b/src/svrf/config.py @@ -49,6 +49,12 @@ class UiConfig: dashboard_url: str = "" +@dataclass +class MergeConfig: + tree_command: str = "" # "": git alone builds every merge step + tree_timeout_seconds: float = 120 + + @dataclass class HistoryConfig: order: str = "off" # "off" or "tests-first" @@ -78,6 +84,7 @@ class Config: train: TrainConfig = field(default_factory=TrainConfig) history: HistoryConfig = field(default_factory=HistoryConfig) ui: UiConfig = field(default_factory=UiConfig) + merge: MergeConfig = field(default_factory=MergeConfig) # ---- derived paths @property @@ -101,7 +108,8 @@ def union_paths(self) -> PathSet: return PathSet(self.union_merge) -_SECTIONS = {"gate": GateConfig, "train": TrainConfig, "history": HistoryConfig, "ui": UiConfig} +_SECTIONS = {"gate": GateConfig, "train": TrainConfig, "history": HistoryConfig, "ui": UiConfig, + "merge": MergeConfig} _TOP = {"repo", "base", "clone", "state_dir", "remote", "git_name", "git_email", "hold_label", "admission_command", "demand_driver"} @@ -157,6 +165,11 @@ def from_dict(value: dict, *, root: Path | None = None) -> Config: raise ConfigError("history.order must be \"off\" or \"tests-first\"") if config.train.family_size < 1 or config.train.jobs < 1: raise ConfigError("train.family_size and train.jobs must be at least 1") + timeout = config.merge.tree_timeout_seconds + if isinstance(timeout, bool) or not isinstance(timeout, (int, float)) or timeout <= 0: + raise ConfigError("[merge] tree_timeout_seconds must be a positive number of seconds") + if not isinstance(config.merge.tree_command, str): + raise ConfigError("[merge] tree_command must be a string") if not config.gate.commands: raise ConfigError("gate.commands must name at least one command") return config diff --git a/src/svrf/daemon.py b/src/svrf/daemon.py index f5db1c7..b31aab3 100644 --- a/src/svrf/daemon.py +++ b/src/svrf/daemon.py @@ -120,7 +120,7 @@ def save(self, state: dict, summary: dict) -> None: return state["last_tick"] = {k: summary[k] for k in ( "tick", "at", "receipt", "receipts", "stopped", "reason", "retry_at", - "admitted", "held", "merged", "skipped", "retry_later", "reasons", + "admitted", "held", "merged", "skipped", "retry_later", "reasons", "tree_provider", ) if k in summary} state["last_tick"]["admission_sources"] = { str(n): {"head": inputs["row"].get("headRefOid"), "base": inputs["admission_base"]} @@ -319,6 +319,12 @@ def land(self, admitted: list[int], rows: list[dict], by_number: dict, state: di summary["merged"].extend(m["number"] for m in receipt["merges"] if m.get("identity")) summary["api_calls"] = receipt.get("api_calls") summary["gates"] = summary.get("gates", 0) + len([g for g in receipt["gates"] if "reused" not in g]) + if "tree_provider" in receipt: + total = summary.setdefault("tree_provider", {"consulted": 0, "provider": 0, "git": 0, "reasons": {}}) + for key in ("consulted", "provider", "git"): + total[key] += receipt["tree_provider"][key] + for reason, count in receipt["tree_provider"]["reasons"].items(): + total["reasons"][reason] = total["reasons"].get(reason, 0) + count repaired: set[int] = set() for hold in receipt["holds"]: n = int(hold["number"]) diff --git a/src/svrf/train.py b/src/svrf/train.py index c6841e7..b4698c1 100644 --- a/src/svrf/train.py +++ b/src/svrf/train.py @@ -49,6 +49,7 @@ from . import pr_surface from .errors import RateLimited, ReadFailed from .rules import choose_families, chunk +from .tree_provider import reason_class SCHEMA = "svrf.receipt/1" @@ -66,7 +67,7 @@ def __init__(self, git, github, gate, *, receipts: Path, jobs: int = 1, family_s poll_seconds: float = 5, poll_tries: int = 36, max_rounds: int = 8, dry_run: bool = False, comment: bool = True, is_union: Callable[[str], bool] = lambda p: False, pr_comments: bool = True, status_checks: bool = True, dashboard_url: str = "", - hold_label: str = "train:hold"): + hold_label: str = "train:hold", tree_provider=None): self.git, self.gh, self.gate = git, github, gate self.jobs, self.family_size, self.memory = max(1, jobs), max(1, family_size), memory self.clock, self.sleep = clock, sleep @@ -75,6 +76,7 @@ def __init__(self, git, github, gate, *, receipts: Path, jobs: int = 1, family_s self.max_rounds, self.dry_run, self.comment = max_rounds, dry_run, comment self.is_union = is_union self.hold_label = hold_label + self.tree_provider = tree_provider self.pr_comments, self.status_checks, self.dashboard_url = pr_comments, status_checks, dashboard_url self.lock = threading.RLock() self.rows: dict[int, dict] = {} @@ -97,6 +99,8 @@ def __init__(self, git, github, gate, *, receipts: Path, jobs: int = 1, family_s "requested": [], "prs": {}, "pairs": None, "out": {}, "rounds": [], "families": [], "gates": [], "merges": [], "holds": [], "retry_later": [], "pending": [], "alerts": [], "rate_waits": [], "api_calls": {}, "surface": dict(self._surface_calls), "stopped": False} + if tree_provider is not None: + self.receipt["tree_provider"] = {"consulted": 0, "provider": 0, "git": 0, "reasons": {}} # ---- receipt @@ -229,6 +233,35 @@ def rate_guard(self) -> None: self._wait_until(float(value.get("reset", self.clock() + 300)), f"RATE_FLOOR:{kind}:{value.get('remaining')}<{self.rate_floor}") + # ---- the merge step + + def merge_step(self, acc: str, head: str, message: str): + """The union step, as always. With a tree provider, a step that git merged into a + new commit is also offered to the provider; its tree is used only when it is + git's own tree. Returns the step and, with a provider, what to record about it.""" + step = self.git.union_step(acc, head, message) + if self.tree_provider is None or step.status != "CLEAN" or step.commit in (acc, head): + return step, {} + try: + proposed, reason = self.tree_provider.propose(head, acc) + except Exception as error: # a provider failure is never a landing failure + proposed, reason = None, f"PROVIDER_FAILED:{type(error).__name__}" + if reason is None and proposed != step.tree: + reason = "PROVIDER_MISMATCH" + record = {"tree_source": "git" if reason else "provider"} + if reason: + record["tree_provider"] = reason + if proposed is not None: + record["proposed_tree"] = proposed + with self.lock: + counts = self.receipt["tree_provider"] + counts["consulted"] += 1 + counts[record["tree_source"]] += 1 + if reason: + key = reason_class(reason) + counts["reasons"][key] = counts["reasons"].get(key, 0) + 1 + return step, record + # ---- planning and gating def plan(self, numbers: list[int], base: str, parent: str | None = None) -> dict | None: @@ -237,7 +270,7 @@ def plan(self, numbers: list[int], base: str, parent: str | None = None) -> dict acc, steps = base, [] for n in numbers: row = self.rows[n] - step = self.git.union_step(acc, row["head_sha"], f"train preview #{n}") + step, record = self.merge_step(acc, row["head_sha"], f"train preview #{n}") if step.status == "CONFLICT": self.hold(n, "CONFLICT", paths=step.conflicts) elif step.status != "CLEAN": @@ -245,7 +278,7 @@ def plan(self, numbers: list[int], base: str, parent: str | None = None) -> dict elif step.commit == acc: self.retry_later(n, "ALREADY_MERGED") else: - steps.append({"number": n, "commit": step.commit, "tree": step.tree}) + steps.append({"number": n, "commit": step.commit, "tree": step.tree, **record}) acc = step.commit if not steps: return None @@ -341,7 +374,7 @@ def land_family(self, family: dict, expected: str, gate_seconds: float = 0.0) -> observed=pull.get("head_sha")) self._stop_family(family, index, "REQUEUED") return False, rest - prepared = self.git.union_step(current, row["head_sha"], f"Merge base into {row['head_ref']}") + prepared, record = self.merge_step(current, row["head_sha"], f"Merge base into {row['head_ref']}") if prepared.status != "CLEAN" or prepared.tree != planned["tree"]: self.alert("PREPARE_DIVERGED", family=family["id"], number=n, status=prepared.status, planned=planned["tree"], prepared=prepared.tree) @@ -366,14 +399,14 @@ def land_family(self, family: dict, expected: str, gate_seconds: float = 0.0) -> self.retry_later(n, f"LANDED_TREE_UNREAD:{unread}") self._add("merges", {"number": n, "family": family["id"], "head": prepared.commit, "merge": merged, "parents": [], "gated_tree": planned["tree"], "observed_tree": None, - "identity": None, "at": self.clock()}) + "identity": None, "at": self.clock(), **record}) family["status"] = "LANDED_TREE_UNREAD" self._save() return False, [s["number"] for s in steps[index + 1:]] identity = observed == planned["tree"] self._add("merges", {"number": n, "family": family["id"], "head": prepared.commit, "merge": merged, "parents": parents, "gated_tree": planned["tree"], "observed_tree": observed, - "identity": identity, "at": self.clock()}) + "identity": identity, "at": self.clock(), **record}) if not identity: self.alert("TREE_MISMATCH", family=family["id"], number=n, planned=planned["tree"], observed=observed, merge=merged) diff --git a/src/svrf/tree_provider.py b/src/svrf/tree_provider.py new file mode 100644 index 0000000..07a3257 --- /dev/null +++ b/src/svrf/tree_provider.py @@ -0,0 +1,79 @@ +"""An optional tree provider for the family merges (`[merge] tree_command`). + +The command is an alternative merge engine. For each merge step the train builds (a pull +request's head merged with the base or the fold so far), it is run in the train's clone +with the step's sides and merge bases in its environment, and prints the tree it would +produce. The train always computes git's own merge tree as well and uses the proposed +tree only when it is the identical tree; this module only runs the command and reads its +answer. Nothing here can fail a landing: every failure is a reason, never an exception. + + SVRF_MERGE_OURS the pull request's head (git's "ours") + SVRF_MERGE_THEIRS the base, or the fold so far (git's "theirs") + SVRF_MERGE_BASES their merge bases (`git merge-base --all`), space separated + SVRF_CLONE the clone the command runs in (also its working directory) +""" + +from __future__ import annotations + +import os +import re +import signal +import subprocess +from pathlib import Path + +from .redact import redact + +TREE_ID = re.compile(r"[0-9a-f]{40}|[0-9a-f]{64}") + + +def reason_class(reason: str) -> str: + """The counted part of a reason: `PROVIDER_EXIT:1:` counts as `PROVIDER_EXIT:1`.""" + return ":".join(reason.split(":")[:2]) + + +class TreeCommand: + def __init__(self, command: str, root: Path | str, *, timeout: float = 120): + self.command = command + self.root = Path(root).expanduser().resolve() + self.timeout = timeout + + def _merge_bases(self, ours: str, theirs: str) -> list[str] | None: + done = subprocess.run(["git", "-C", str(self.root), "merge-base", "--all", ours, theirs], + capture_output=True, text=True) + if done.returncode not in (0, 1): + return None + return done.stdout.split() + + def propose(self, ours: str, theirs: str) -> tuple[str | None, str | None]: + """(tree, None) when the command printed a tree id, else (None, reason).""" + bases = self._merge_bases(ours, theirs) + if bases is None: + return None, "MERGE_BASES_UNREAD" + env = {**os.environ, "SVRF_MERGE_OURS": ours, "SVRF_MERGE_THEIRS": theirs, + "SVRF_MERGE_BASES": " ".join(bases), "SVRF_CLONE": str(self.root)} + try: + # Its own session, so a timeout kills everything the command started. + proc = subprocess.Popen(["bash", "-c", self.command], cwd=str(self.root), env=env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + start_new_session=True) + except OSError as error: + return None, f"PROVIDER_UNAVAILABLE:{type(error).__name__}" + try: + stdout, stderr = proc.communicate(timeout=self.timeout) + except subprocess.TimeoutExpired: + try: + os.killpg(proc.pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + proc.communicate(timeout=5) + except subprocess.TimeoutExpired: + pass + return None, "PROVIDER_TIMEOUT" + if proc.returncode != 0: + tail = [line.strip() for line in redact(stderr, env).splitlines() if line.strip()] + return None, f"PROVIDER_EXIT:{proc.returncode}" + (f":{tail[-1][:160]}" if tail else "") + lines = [line.strip() for line in stdout.splitlines() if line.strip()] + if not lines or not TREE_ID.fullmatch(lines[0]): + return None, "PROVIDER_OUTPUT_INVALID" + return lines[0], None From f7866a0e5deef270afb5bf7e2781e1de1f2f6d8c Mon Sep 17 00:00:00 2001 From: svrf-maintainer Date: Sat, 3 Oct 2026 22:48:58 +0000 Subject: [PATCH 3/3] docs: describe the optional tree provider for family merges --- CHANGELOG.md | 9 ++++++ README.md | 3 ++ docs/TREE_PROVIDER.md | 70 +++++++++++++++++++++++++++++++++++++++++++ svrf.example.toml | 4 +++ 4 files changed, 86 insertions(+) create mode 100644 docs/TREE_PROVIDER.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fc89d0..1f4de8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ project intends to follow [Semantic Versioning](https://semver.org/) once it rea ## [Unreleased] +### Added + +- `[merge] tree_command`: an optional tree provider for the family merges. For every + merge step the train builds, the command proposes a tree; git's own merge tree is always + computed and the proposed tree is used only when identical. A different tree, a failure, + a timeout or unreadable output keeps git's tree and records why. Each step and merge + row carries `tree_source`; the receipt and the round summary carry the totals. Unset, + nothing changes. See `docs/TREE_PROVIDER.md`. + ## [0.2.0] - 2026-09-25 ### Changed diff --git a/README.md b/README.md index c6bdbb7..6383128 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,7 @@ own production train (see [docs/CASE_STUDY.md](docs/CASE_STUDY.md)): | One train per repository: a file lock; a second owner exits 3 without reading anything | `owner_lock` | | | Every git call names its clone (`git -C`) and every `gh` call names its repository | `gh_argv`, a test over the source | | | Branches are updated by refspec push, never forced, never checked out | `RealGit` | | +| A configured tree provider's tree is used only when it is git's own merge tree for that step; anything else keeps git's tree | `Train.merge_step` | | The theorems are about a model of the merge step, not about git. The code never relies on the model being right: it reads every landed tree and compares it with the gated one, @@ -249,6 +250,8 @@ Unknown keys are refused. | `ui.pr_comments` | `true` | one living comment per pull request (see [docs/PR_SURFACE.md](docs/PR_SURFACE.md)) | | `ui.status_checks` | `true` | a `svrf` commit status on each candidate head | | `ui.dashboard_url` | `""` | optional: linked from the status as `target_url` | +| `merge.tree_command` | `""` | optional alternative merge engine for the family merges, cross-checked against git's own merge tree on every step ([docs/TREE_PROVIDER.md](docs/TREE_PROVIDER.md)) | +| `merge.tree_timeout_seconds` | `120` | per merge step; a timeout keeps git's tree | Gate commands see `SVRF_BASE`, `SVRF_COMMIT`, `SVRF_LABEL` and `SVRF_CHANGED_FILES` (a file listing the changed paths), so a gate can build only what changed. diff --git a/docs/TREE_PROVIDER.md b/docs/TREE_PROVIDER.md new file mode 100644 index 0000000..1f4f4ec --- /dev/null +++ b/docs/TREE_PROVIDER.md @@ -0,0 +1,70 @@ +# A tree provider for the family merges + +Every commit SVRF lands is built from one merge step: a pull request's head merged with +the base (or with the fold of the earlier pull requests in its family), union-merge paths +resolved, committed with plumbing. Git builds those trees. If you have another merge +engine you want to run on real traffic, `[merge] tree_command` lets the train ask it for +each step's tree and cross-check the answer against git's, without ever trusting it. + +```toml +[merge] +tree_command = "my-merge-engine --print-tree" # "" (the default): git alone +tree_timeout_seconds = 120 # per step; a timeout falls back to git +``` + +Unset, nothing changes: no command runs and the receipts are exactly as before. + +## What the command is given + +For each merge step the train builds — while folding a family for its gate, and again +while preparing each pull request's branch for its merge — the command is run with +`bash -c` in the train's clone, with: + +| Variable | Value | +| --- | --- | +| `SVRF_MERGE_OURS` | the pull request's head (git's "ours") | +| `SVRF_MERGE_THEIRS` | the base, or the fold so far (git's "theirs") | +| `SVRF_MERGE_BASES` | their merge bases (`git merge-base --all`), space separated | +| `SVRF_CLONE` | the clone; also the working directory | + +It prints the tree id of the merge on the first line of its output and exits 0. Only the +id is read: a step that uses the provider's answer commits the tree git itself wrote, +which is the same tree. + +Steps git does not merge into a new commit are not offered: a head already in the base, +a base already in the head, a conflict, or a failed read. The pairwise conflict read, the +admission check, repairs and re-lands never run the command. + +## How the answer is used + +Git's own merge tree is always computed, exactly as without a provider. Then: + +| The command | The step uses | Recorded | +| --- | --- | --- | +| printed git's tree | that tree | `tree_source: "provider"` | +| printed a different tree | git's tree | `tree_source: "git"`, `tree_provider: "PROVIDER_MISMATCH"`, `proposed_tree` | +| exited non-zero | git's tree | `PROVIDER_EXIT::` | +| ran past `tree_timeout_seconds` | git's tree | `PROVIDER_TIMEOUT` (its whole process group is killed) | +| printed no tree id | git's tree | `PROVIDER_OUTPUT_INVALID` | +| could not be started, or the merge bases could not be read | git's tree | `PROVIDER_UNAVAILABLE:`, `MERGE_BASES_UNREAD` | + +The provider can never put a tree on the base that git did not produce, and nothing it +does can hold a pull request, fail a gate or stop a landing. Every landed tree is still +compared with the gated tree after its merge, as always. + +## Where it shows up + +Each step of a family in the round receipt, and each merge row, carries `tree_source` +(and the reason when git's tree was used). The receipt has the totals: + +```json +"tree_provider": {"consulted": 4, "provider": 3, "git": 1, "reasons": {"PROVIDER_MISMATCH": 1}} +``` + +and `svrf run --once` prints the same totals for the round (also kept in the state's +`last_tick`). A reason's counted part is its first two fields, so exit codes are counted +separately and stderr text is not. + +The command runs once per merge step: twice per landed pull request (once in the gated +fold, once when its branch is prepared), plus once per step of a replanned bisection +half. Keep it well inside the timeout; the gate and the landing wait for it. diff --git a/svrf.example.toml b/svrf.example.toml index a56cf16..238cae7 100644 --- a/svrf.example.toml +++ b/svrf.example.toml @@ -46,3 +46,7 @@ command = "" # optional extra check; exit 0 admits, other codes h pr_comments = true # one living comment per pull request (see docs/PR_SURFACE.md) status_checks = true # a "svrf" commit status on each candidate head dashboard_url = "" # optional: linked from the status as target_url + +[merge] +tree_command = "" # optional alternative merge engine, cross-checked against git (docs/TREE_PROVIDER.md) +tree_timeout_seconds = 120 # per merge step; a timeout keeps git's tree