From 51b374121bbf82dbf8b5f90ae69ec80e5bf624d5 Mon Sep 17 00:00:00 2001 From: Ludovic Marie <136255825+ludovic111@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:46:44 +0100 Subject: [PATCH] Team merge rule: ordinary pull requests merge on CI, risky ones get needs-proof; levzzz5154 trusted The Horizon Team merges ordinary pull requests once verify and parity pass; saves, network, world generation, launcher updates, the harnesses and big changes wait for a local proof. Also names the licence holder with the contributors. Co-Authored-By: Claude Opus 5.5 --- .github/trusted-reviewers.txt | 1 + CONTRIBUTING.md | 3 +++ LICENSE | 2 +- docs/release-flow.md | 43 +++++++++++++++++++++++++++++------ tests/test_nightly_set.py | 2 +- tests/test_triage_command.py | 2 +- 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/.github/trusted-reviewers.txt b/.github/trusted-reviewers.txt index 550c528cc2..48b98391f9 100644 --- a/.github/trusted-reviewers.txt +++ b/.github/trusted-reviewers.txt @@ -6,3 +6,4 @@ kacpherek # added by Ludovic, 2026-10-27 rdbtCVS # added by Ludovic, 2026-10-27 Online5277 # added by Ludovic, 2026-10-27 (his top three by merged pull requests) +levzzz5154 # added by Ludovic, 2026-10-28 (Horizon Team; author of horizon-mixins) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 42959f9504..abd343896b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,6 +86,9 @@ configuration and toolchain changes select the whole workspace; CI also keeps it release notes), `skip-release` when players see no change (tooling, docs, tests), nothing for a fix. `release-now` is for crash fixes that shouldn't wait. pr_ready suggests one. +Who reviews and merges: the Horizon Team, on CI for ordinary pull requests; risky ones (saves, network, world +generation, the launcher's updates) wait for a local proof first ([merging](docs/release-flow.md#merging)). + Every merge to main is released automatically ([release flow](docs/release-flow.md)), so main always stays playable. How agents, checks, releases and signed updates keep this safe: [security](docs/security.md). diff --git a/LICENSE b/LICENSE index df2363379e..e959a076e9 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2026 Ludovic +Copyright (c) 2026 Ludovic Marie and the Horizon contributors Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/docs/release-flow.md b/docs/release-flow.md index 3ea4b3fcb5..e5201d0891 100644 --- a/docs/release-flow.md +++ b/docs/release-flow.md @@ -119,10 +119,39 @@ ones only there. ## Merging -Pull requests merge with a merge commit. The orchestrator sessions review a pull request, then turn on -auto-merge (`gh pr merge --auto --merge`): GitHub merges it once the required checks pass. Branch -protection on main requires `verify`. Branches don't have to be up to date with main: two sessions merge in -parallel, and main's own `checks` run catches the rare semantic conflict. +Pull requests merge with a merge commit, by hand: never `gh pr merge --auto`. Branch protection on main +requires `verify` for everyone, and Ludovic's review for the files `.github/CODEOWNERS` lists (what builds, signs +and ships releases). Branches don't have to be up to date with main: main's own `checks` run catches the rare +semantic conflict. + +**Who merges.** The Horizon Team (GitHub team `horizon-team`: kacpherek, levzzz5154, rdbtCVS, Online5277) and +Ludovic. The orchestrator's agents prove only the risky pull requests (below), so a contributor's ordinary pull +request doesn't wait for them. + +**An ordinary pull request merges on CI.** A Team member: + +1. reads the whole diff (for a fork, before adding `run-parity`: that runs the fork's code on a maintainer's machine); +2. adds `run-parity` to a fork's pull request; our own branches run `parity` by themselves. Since #2448 it tests + the pull request merged into main; +3. merges once `verify` and `parity` pass, nobody's change request is open, and every comment has been read (a + "don't merge" from anyone holds it). + +Never merge your own pull request: another Team member reviews and merges it. Never force-push or rebase a +contributor's branch; push a fixup commit, or merge main into it. + +**A risky pull request gets the `needs-proof` label** and waits for an agent's local proof (both single-player +paths, `lan_e2e`, a screenshot for anything visible) before it merges. Risky means any of: + +- it changes what is saved or loaded (`save.rs`, `entity_nbt.rs`, `block_entity_nbt.rs`, `region.rs`, `datafix/`): + a bug there damages players' worlds; +- the network or the server thread: `horizon-protocol`, `net.rs`, `remote*.rs`, `lan/`, `lan*.rs`, + `integrated.rs`, `levels.rs`, `dedicated*`; +- world generation (`worldgen/`): seeds must keep generating the same terrain; +- the launcher's installs and updates, or the mod runtime; +- the harnesses themselves (`tools/test_*_parity.py`, the Java oracles): a wrong harness hides regressions; +- over ~500 lines, or `parity` failed, was skipped or ran no harness for gameplay code. + +Unsure? Add `needs-proof`: it costs a few hours, a broken release costs more. `parity` becomes required once it has run green on pull requests for a while (a week without a false failure): @@ -131,7 +160,7 @@ gh api -X POST repos/horizon-rust/horizon/branches/main/protection/required_stat ``` A skipped `parity` (a fork's pull request without the label) counts as passed, so requiring it never blocks a -fork's pull request; the maintainer decides whether to run it. +fork's pull request: the Team member decides whether to run it, and a skipped run makes the pull request risky. ## Parity on main @@ -423,7 +452,7 @@ before the gate. ### Trusted reviewers One list, `.github/trusted-reviewers.txt`, governs two things for the people on it (kacpherek, rdbtCVS and -Online5277 since 2026-10-27) and for Ludovic, with no repository role: `/nightly` (below) and triage by comment. +Online5277 since 2026-10-27, levzzz5154 since 2026-10-28) and for Ludovic, with no repository role: `/nightly` (below) and triage by comment. ```text /close closes an issue as completed; a pull request is closed without merging @@ -449,7 +478,7 @@ Online5277 since 2026-10-27) and for Ludovic, with no repository role: `/nightly ### Trusted reviewers pick the Nightly's pull requests The Nightly is where contributors test pull requests before the gate. The trusted reviewers listed in -`.github/trusted-reviewers.txt` (kacpherek, rdbtCVS and Online5277 since 2026-10-27) and Ludovic pick them with a +`.github/trusted-reviewers.txt` (kacpherek, rdbtCVS and Online5277 since 2026-10-27, levzzz5154 since 2026-10-28) and Ludovic pick them with a comment on any issue or pull request (`.github/workflows/nightly-command.yml`, `tools/ci/nightly_set.py`): ```text diff --git a/tests/test_nightly_set.py b/tests/test_nightly_set.py index 4ba8d43d3c..975857668f 100644 --- a/tests/test_nightly_set.py +++ b/tests/test_nightly_set.py @@ -73,7 +73,7 @@ class AllowTests(unittest.TestCase): def test_the_repos_list_names_the_three_reviewers(self): reviewers = ns.read_reviewers((ROOT / ns.REVIEWERS).read_text()) - self.assertEqual(reviewers.logins, {"kacpherek", "rdbtcvs", "online5277"}) + self.assertEqual(reviewers.logins, {"kacpherek", "rdbtcvs", "online5277", "levzzz5154"}) self.assertFalse(reviewers.paused) class RefusalTests(unittest.TestCase): diff --git a/tests/test_triage_command.py b/tests/test_triage_command.py index ce73ea4dea..331f9b803e 100644 --- a/tests/test_triage_command.py +++ b/tests/test_triage_command.py @@ -104,7 +104,7 @@ class AllowTests(unittest.TestCase): def test_the_repos_list_is_the_nightlys(self): self.assertEqual(ns.REVIEWERS, ".github/trusted-reviewers.txt") listed = ns.read_reviewers((ROOT / ns.REVIEWERS).read_text()) - self.assertEqual(listed.logins, {"kacpherek", "rdbtcvs", "online5277"}) + self.assertEqual(listed.logins, {"kacpherek", "rdbtcvs", "online5277", "levzzz5154"}) self.assertFalse(listed.paused)