smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

**no more vouching! let's see how it goes for a week :)**

Browse Source
This commit is contained in:
can1357
2026-07-23 02:35:36 +02:00
parent 1f7d2ff9ea
commit bb15939414
13 changed files with 79 additions and 673 deletions
Download Patch File Download Diff File Expand all files Collapse all files
python/robomp/.env.example
+2 -15
View File
@@ -117,23 +117,10 @@ ROBOMP_REQUEST_TIMEOUT_SECONDS=120
# =============================================================================
# --- PR review / vouch gate ---
# --- PR review ---
# =============================================================================
# robomp reviews incoming contributor PRs. When PRs are gated by the vouch
# GitHub Action (.github/workflows/vouch-pr.yml), set the trigger to
# `vouched_label` so robomp reviews ONLY the PRs the gate let through (the
# workflow labels survivors with ROBOMP_VOUCH_REVIEW_LABEL) instead of racing
# the gate on PR open. Set ROBOMP_PR_REVIEW_ENABLED=false to disable robomp PR
# review entirely.
# Set ROBOMP_PR_REVIEW_ENABLED=false to disable robomp PR review entirely.
ROBOMP_PR_REVIEW_ENABLED=true
# open | vouched_label
ROBOMP_PR_REVIEW_TRIGGER=open
ROBOMP_VOUCH_REVIEW_LABEL=vouched
# In vouched_label mode only `labeled` events from this actor trigger review,
# so a manual triage/maintainer label cannot bypass the gate. Default matches
# the stock GITHUB_TOKEN actor; set to your App's bot login if the vouch
# workflow labels via a GitHub App token.
ROBOMP_VOUCH_REVIEW_LABELER=github-actions[bot]
# =============================================================================
python/robomp/docker-compose.yml
+1 -8
View File
@@ -44,15 +44,8 @@ services:
ROBOMP_MAINTAINER_LOGINS: ${ROBOMP_MAINTAINER_LOGINS:-}
ROBOMP_REVIEWER_BOTS: ${ROBOMP_REVIEWER_BOTS:-}
# --- PR review / vouch gate ---
# When PRs are gated by the vouch GitHub Action, set the trigger to
# `vouched_label` so robomp reviews ONLY PRs that survive the gate
# (the workflow labels survivors `ROBOMP_VOUCH_REVIEW_LABEL`), instead
# of racing the gate on PR open. `vouched_label` keeps review ENABLED.
# --- PR review ---
ROBOMP_PR_REVIEW_ENABLED: ${ROBOMP_PR_REVIEW_ENABLED:-true}
ROBOMP_PR_REVIEW_TRIGGER: ${ROBOMP_PR_REVIEW_TRIGGER:-open}
ROBOMP_VOUCH_REVIEW_LABEL: ${ROBOMP_VOUCH_REVIEW_LABEL:-vouched}
ROBOMP_VOUCH_REVIEW_LABELER: ${ROBOMP_VOUCH_REVIEW_LABELER:-github-actions[bot]}
# --- model selection ---
ROBOMP_MODEL: ${ROBOMP_MODEL:-anthropic/claude-sonnet-4-6}
python/robomp/src/config.py
-12
View File
@@ -38,18 +38,6 @@ class Settings(BaseSettings):
git_author_email: str = Field(..., alias="ROBOMP_GIT_AUTHOR_EMAIL")
repo_allowlist_raw: str = Field("", alias="ROBOMP_REPO_ALLOWLIST")
pr_review_enabled: bool = Field(True, alias="ROBOMP_PR_REVIEW_ENABLED")
# PR review trigger. "open" (default) reviews incoming PRs on
# opened/reopened/ready_for_review. "vouched_label" DEFERS review until the
# vouch GitHub Action labels the PR `vouch_review_label`, so robomp reviews
# only PRs that survive the vouch gate. `pr_review_enabled` remains the
# master switch (False disables review under either trigger).
pr_review_trigger: Literal["open", "vouched_label"] = Field("open", alias="ROBOMP_PR_REVIEW_TRIGGER")
vouch_review_label: str = Field("vouched", alias="ROBOMP_VOUCH_REVIEW_LABEL")
# In vouched_label mode, only `labeled` events from this actor trigger a
# review, so a manual label by a triage/maintainer cannot bypass the gate.
# Default is the actor for the stock GITHUB_TOKEN; set to your App's bot
# login (e.g. "vouch-bot[bot]") if the vouch workflow labels via an App.
vouch_review_labeler: str = Field("github-actions[bot]", alias="ROBOMP_VOUCH_REVIEW_LABELER")
# gh-proxy. Set BOTH to route GitHub through the proxy; leave both empty
# to keep PAT-on-orchestrator behavior. Mixing the two (PAT + proxy) is
python/robomp/src/github_events.py
-23
View File
@@ -226,9 +226,6 @@ def route(
reviewer_bots: frozenset[str] = frozenset(),
resolve_issue_from_pr: PrIssueResolver = None,
pr_review_enabled: bool = True,
pr_review_trigger: str = "open",
vouch_review_label: str = "vouched",
vouch_review_labeler: str = "github-actions[bot]",
) -> RouteDecision:
"""Decide whether and how to handle a webhook event.
@@ -362,28 +359,8 @@ def route(
if not pr_review_enabled:
return RouteDecision("skip", None, repo, None, "PR review disabled")
pr = payload.get("pull_request") or {}
if pr_review_trigger == "vouched_label":
# Defer to the vouch gate. robomp reviews ONLY on the `labeled`
# event the workflow emits AFTER a fresh check (it re-applies the
# vouch label on every opened/reopened/ready_for_review). Never
# trust a persisted label here: a since-denounced author must not
# slip through on reopen.
return RouteDecision("skip", None, repo, None, "deferred to vouch label")
return _pr_review_pr(pr, repo, action, bot_login)
if event_type == "pull_request" and action == "labeled" and pr_review_trigger == "vouched_label":
if not pr_review_enabled:
return RouteDecision("skip", None, repo, None, "PR review disabled")
label = payload.get("label")
label_name = str(label.get("name") or "") if isinstance(label, Mapping) else ""
if label_name.lower() != vouch_review_label.lower():
return RouteDecision("skip", None, repo, None, f"label {label_name!r} not vouch label")
sender = payload.get("sender")
labeler = str(sender.get("login") or "") if isinstance(sender, Mapping) else ""
if labeler.lower() != vouch_review_labeler.lower():
return RouteDecision("skip", None, repo, None, f"vouch label not from trusted labeler ({labeler!r})")
return _pr_review_pr(payload.get("pull_request") or {}, repo, action, bot_login)
if event_type == "pull_request_review_comment" and action == "created":
comment = payload.get("comment") or {}
rb_login = _reviewer_bot_login(comment.get("user"))
python/robomp/src/server.py
-3
View File
@@ -356,9 +356,6 @@ def create_app(settings: Settings | None = None) -> FastAPI:
maintainers=cfg.maintainer_logins,
reviewer_bots=cfg.reviewer_bots,
pr_review_enabled=cfg.pr_review_enabled,
pr_review_trigger=cfg.pr_review_trigger,
vouch_review_label=cfg.vouch_review_label,
vouch_review_labeler=cfg.vouch_review_labeler,
resolve_issue_from_pr=_resolve,
)
python/robomp/tests/test_github_events.py
-192
View File
@@ -956,195 +956,3 @@ def test_route_non_directive_comment_carries_no_pragmas() -> None:
assert decision.directive_pragmas == ()
# ---------- vouched-label deferred PR review ----------
def test_route_vouched_label_defers_pr_open() -> None:
decision = route(
"pull_request",
{
"action": "opened",
"pull_request": {"number": 9, "draft": False, "user": {"login": "alice", "type": "User"}},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "deferred to vouch label"
def test_route_vouched_label_reviews_on_label() -> None:
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "vouched"},
"pull_request": {
"number": 9,
"draft": False,
"user": {"login": "alice", "type": "User"},
"author_association": "CONTRIBUTOR",
},
"repository": {"full_name": "octo/widget"},
"sender": {"login": "github-actions[bot]"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert decision.should_queue
assert decision.task == "review_pr"
assert decision.issue_key == "octo/widget#9"
assert decision.submitter == "alice"
def test_route_vouched_label_ignores_other_labels() -> None:
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "bug"},
"pull_request": {"number": 9, "user": {"login": "alice"}},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
def test_route_vouched_label_ready_for_review_defers_even_with_label() -> None:
# Persisted labels are NOT trusted; the workflow re-applies the label (a
# fresh `labeled` event) after re-validating, so ready_for_review defers.
decision = route(
"pull_request",
{
"action": "ready_for_review",
"pull_request": {
"number": 9,
"draft": False,
"user": {"login": "alice", "type": "User"},
"labels": [{"name": "vouched"}],
},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "deferred to vouch label"
def test_route_vouched_label_labeled_skips_draft() -> None:
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "vouched"},
"pull_request": {"number": 9, "draft": True, "user": {"login": "alice", "type": "User"}},
"repository": {"full_name": "octo/widget"},
"sender": {"login": "github-actions[bot]"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "draft PR"
def test_route_default_trigger_ignores_labeled() -> None:
# Backward-compat: the default "open" trigger does not route `labeled`.
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "vouched"},
"pull_request": {"number": 9, "user": {"login": "alice"}},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
)
assert not decision.should_queue
def test_route_vouched_label_reopened_defers_even_with_label() -> None:
# A since-denounced author must not slip through on reopen via a stale
# label; reopened always defers to a fresh gate check + re-label.
decision = route(
"pull_request",
{
"action": "reopened",
"pull_request": {
"number": 9,
"draft": False,
"user": {"login": "alice", "type": "User"},
"labels": [{"name": "vouched"}],
},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "deferred to vouch label"
def test_route_vouched_label_reopened_without_label_defers() -> None:
decision = route(
"pull_request",
{
"action": "reopened",
"pull_request": {"number": 9, "draft": False, "user": {"login": "alice", "type": "User"}},
"repository": {"full_name": "octo/widget"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "deferred to vouch label"
def test_route_vouched_label_rejects_manual_labeler() -> None:
# A triage/maintainer hand-adding the label must NOT trigger review.
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "vouched"},
"pull_request": {"number": 9, "draft": False, "user": {"login": "alice", "type": "User"}},
"repository": {"full_name": "octo/widget"},
"sender": {"login": "evilmaintainer"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert "trusted labeler" in decision.reason
def test_route_vouched_label_skips_closed_pr() -> None:
# `labeled` can fire on a closed PR; never review one.
decision = route(
"pull_request",
{
"action": "labeled",
"label": {"name": "vouched"},
"pull_request": {"number": 9, "state": "closed", "user": {"login": "alice", "type": "User"}},
"repository": {"full_name": "octo/widget"},
"sender": {"login": "github-actions[bot]"},
},
allowlist=ALLOWLIST,
bot_login=BOT,
pr_review_trigger="vouched_label",
)
assert not decision.should_queue
assert decision.reason == "PR not open"
python/robomp/tests/test_queue_dispatch.py
+3 -13
View File
@@ -1,11 +1,4 @@
"""Dispatch action -> task mapping in WorkerPool._dispatch.
Regression guard for the route<->dispatch contract: `github_events.route`
queues a `pull_request.labeled` event as a `review_pr` task in `vouched_label`
mode, so `_dispatch` MUST invoke `tasks.review_pr` for that action. It
previously only handled `opened/reopened/ready_for_review`, so every vouched
PR fell through to the no-op branch and was silently marked `done`.
"""
"""Dispatch action -> task mapping in WorkerPool._dispatch."""
from __future__ import annotations
@@ -55,15 +48,12 @@ def _pr_row(action: str, *, delivery: str = "pr1") -> EventRow:
)
@pytest.mark.parametrize("action", ["opened", "reopened", "ready_for_review", "labeled"])
@pytest.mark.parametrize("action", ["opened", "reopened", "ready_for_review"])
@pytest.mark.asyncio
async def test_dispatch_routes_pr_review_actions_to_review_pr(
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch, action: str
) -> None:
"""Every PR action `route` can queue for review MUST reach `tasks.review_pr`.
`labeled` is the vouched-label trigger; the others are the `open` trigger.
"""
"""Every PR action `route` can queue for review MUST reach `tasks.review_pr`."""
seen: list[str] = []
async def fake_review_pr(*, payload, **_kwargs) -> None: