From f45be51cd7096f0eae09db3826bcfb458cdcef70 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 8 Aug 2026 23:09:04 +0000 Subject: [PATCH 1/2] docs(contributing): squash-merge, and why --merge reds main Landing a PR with `gh pr merge --merge` breaks two gates, and it has now happened three times in a row on main: c3db7804 (#172, mine), 3e5072d4 (#174), and the trailer half on da0a9d8d (#157). check-role-discipline.py inspects every commit in the pushed range and wants each to name its row/ branch or its PR as (#N). GitHub's squash-merge writes (#N) into the subject for free. A --merge landing leaves the content commit with the subject it had on the branch, which names neither, so POL-PR-REQUIRED reports "reached main without a reviewed row/* PR" about a commit that came from exactly such a PR. check-commit-trailers.py reads the same range, and GitHub's generated "Merge pull request #N from ..." message carries no FOLLOWING_AGENTS_PROTOCOL paragraph and no trailers, so POL-COMMIT-TRAILERS fails on the merge commit itself. Both gates are scoped over github.event.before..github.sha and each run's before is the previous run's sha, so no later run re-covers a range that already went red. That is the part worth writing down: the failure is not repairable after the push except by rewriting published history or waiving it, and it is invisible at PR time because the PR is checked against its own base. Documented in CONTRIBUTING.md rather than .agents/workflow.md because that file is 7 bytes under its 12288-byte procedure budget, and every paragraph of its non-generated prose is load-bearing; buying room by trimming it would cost more than this note is worth. CONTRIBUTING.md is also where a contributor with merge rights actually looks. This is documentation, not a guard. The permanent fix is a repository setting - allow squash only, disable merge commits - which needs admin and is left to the owner. No record row, no capability claim, no code. Every tree-scoped gate on main was already green before this change; main's red runs come entirely from the diff-scoped range gates described above. Gates: check-policy, check-protocol-consistency, check-agent-record, check-readme-structure, check-public-doc-tables, check-role-discipline, check-doc-checkpoint and check-commit-trailers over the range - all OK on a worktree pinned at 80b08801. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code] --- CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4269b081..f506fbe3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,6 +44,34 @@ An issue, roadmap row, or helper-queue result is a lead. It is not sufficient evidence by itself. If the work has landed, is already claimed, or no longer matches the record, the agent reconciles that state instead of duplicating it. +## Landing a pull request + +Squash-merge it: `gh pr merge --squash`. The merge method is not cosmetic, +because two gates read the commits a push actually lands on `main`, and both +fail *after* the merge, on `main`, not on your PR. + +`scripts/check-role-discipline.py` inspects every commit in the pushed range and +requires each to name either its `row/` branch or its PR as `(#N)` +(`POL-PR-REQUIRED`). A squash-merge writes `(#N)` into the subject for free. A +`--merge` landing does not: the content commit keeps the subject it had on the +branch, which names neither, so the gate reports `reached main without a +reviewed row/* PR` even though a reviewed row PR is exactly where it came from. + +`scripts/check-commit-trailers.py` reads the same range, so the landed message +must itself carry the `FOLLOWING_AGENTS_PROTOCOL` paragraph and the trailers +(`POL-COMMIT-TRAILERS`, `POL-AI-ATTRIBUTION`). Squash bodies are built from the +branch's commit messages, so a correctly trailered commit carries them through; +confirm the composed message in the merge dialog before confirming. A `--merge` +landing fails this too, because GitHub's generated `Merge pull request #N +from ...` message has no marker and no trailers. + +Neither failure is repairable afterwards. Both gates are scoped over +`github.event.before..github.sha`, and each run's `before` is the previous run's +`sha`, so no later run re-covers a range that already went red. The remedies are +rewriting published history or an explicit waiver; getting the merge method +right is much cheaper. A red `main` from this cause does not block your next PR, +which is checked against its own base, but it does hide real regressions. + ## Reference and hardware guardrails Documentation and hardware-independent work can start without a vLLM checkout From 0d5a938edc484c9bce67d16e7b5351a09bfd3f6f Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 8 Aug 2026 23:24:18 +0000 Subject: [PATCH 2/2] fix(ci): unbreak test_agent_role under a pull_request checkout test_agent_role.py::test_landed_detached_commit_remains_strict_without_ pending_evidence failed on PR #176 and would fail on many PRs that have nothing to do with what it asserts. The test's subject is main()'s DECISION: a violation on a commit that has landed, with no --pending-pr-head evidence, is strict (return 1) rather than a REPORT (return 0). But it obtained that violation by relying on the REAL HEAD to be one, and that coupling is live under CI. On a pull_request event GitHub checks out the SYNTHETIC merge commit for refs/pull/N/merge. inspect() sees two parents, so arrives_via_row_pr() switches to the merged_messages branch and scans the PR's own commit bodies with PR_REFERENCE = \(#\d+\)|#\d+, which matches a bare #123 anywhere. Any PR whose commit message cites an issue or PR number therefore stops being a violation, main() returns 0, and the assertion fails with 0 != 1. #176's message quotes (#157) and (#174) while explaining these very gates, which is how it tripped. Fixed by feeding main() a fixed violation through inspect/enforced rather than depending on HEAD's message, plus a regression case pinning that a message mentioning (#157) and #174 cannot flip the decision again. Proven on a reconstructed synthetic merge, the exact shape CI checks out (git merge --no-ff of the PR head into the base, detached): RED old test on that checkout: AssertionError: 0 != 1, 41 tests, FAILED (failures=1) -- the same failure CI reported GREEN new test on the SAME checkout: 42 tests, OK Not changed: PR_REFERENCE matching a bare #123 anywhere in a body also means a direct push whose message merely mentions an issue satisfies POL-PR-REQUIRED. That is a real hole, but tightening a policy gate's strictness is a separate reviewed decision, not a CI repair. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code] --- tests/scripts/test_agent_role.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/scripts/test_agent_role.py b/tests/scripts/test_agent_role.py index be64ceb1..913bae30 100644 --- a/tests/scripts/test_agent_role.py +++ b/tests/scripts/test_agent_role.py @@ -486,10 +486,38 @@ def test_exact_pending_pr_range_is_reportable_in_any_checkout(self) -> None: sys.argv = saved def test_landed_detached_commit_remains_strict_without_pending_evidence(self) -> None: + # The subject here is main()'s DECISION: a violation on a commit that + # has landed, with no --pending-pr-head evidence, is strict (1), not a + # REPORT (0). Feed it a fixed violation instead of relying on the real + # HEAD to be one. It did rely on that, and the coupling was live: under + # a pull_request event CI checks out the SYNTHETIC merge, whose + # merged_messages are the PR's own commit bodies, so any PR whose + # message cites an issue or PR number matched PR_REFERENCE, HEAD stopped + # being a violation, main() returned 0, and this test failed for a + # reason that had nothing to do with what it asserts. saved = sys.argv sys.argv = [saved[0], "--commit", "HEAD"] try: - with mock.patch.object(discipline, "has_reached_main", return_value=True): + with mock.patch.object(discipline, "has_reached_main", return_value=True), \ + mock.patch.object(discipline, "enforced", return_value=True), \ + mock.patch.object(discipline, "inspect", return_value=["x: landed without a row PR"]): + self.assertEqual(discipline.main(), 1) + finally: + sys.argv = saved + + def test_a_pr_number_in_the_body_does_not_decide_this_gate(self) -> None: + """Regression: the case that made the test above fail in CI. + + A commit message that merely MENTIONS `#123` must not change main()'s + landed-vs-pending decision. This pins the decision to the inputs it is + about, so a message quoting PR numbers cannot flip the outcome again. + """ + saved = sys.argv + sys.argv = [saved[0], "--commit", "HEAD"] + try: + with mock.patch.object(discipline, "has_reached_main", return_value=True), \ + mock.patch.object(discipline, "enforced", return_value=True), \ + mock.patch.object(discipline, "inspect", return_value=["x: see (#157) and #174"]): self.assertEqual(discipline.main(), 1) finally: sys.argv = saved