Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/<ROW-ID>` 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
Expand Down
30 changes: 29 additions & 1 deletion tests/scripts/test_agent_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading