diff --git a/.github/VOUCHED.td b/.github/VOUCHED.td deleted file mode 100644 index b64a6908f..000000000 --- a/.github/VOUCHED.td +++ /dev/null @@ -1,262 +0,0 @@ -# The list of vouched (or actively denounced) users for this repository. -# -# Only vouched users can open PRs here; unvouched/denounced PRs are -# auto-closed by .github/workflows/vouch-pr.yml (mitchellh/vouch check-pr). -# Issues are intentionally NOT gated here — robomp triages those. -# -# Write-access collaborators and bots are auto-allowed and need no entry. -# A denounced user ("-handle") is always blocked, even if also listed. -# -# Syntax: -# - One handle per line (without @), sorted alphabetically. -# - Optional platform prefix: `platform:username` (default platform: github). -# - Denounce by prefixing with minus: `-username` / `-platform:username`. -# - Optional free-text reason after a space following the handle. -# -# Maintainers manage this list by commenting `!vouch` / `!denounce [user]` -# on a discussion (see .github/workflows/vouch-manage.yml). -# -# Seed (2026-06-19): authors with >=2 merged PRs in the prior 6 months. -# Audit found 0 denounce-worthy actors; all reverts were maintainer -# technical rollbacks, not abuse. See the PR/commit history for provenance. - -0ttik -21307369 -35844493 -381181295 -a-glapinski -adrastopoulos -ak4153 -alias8818 -alloevil -alvorithm -anasinno -andrew-pynch -antonlvovych -any-victor -apoc -arg3t -art1kz -asafmah -asterisksf -atyrode -audreyt -azais-corentin -aznikline -bannert1337 -barisdemirdelen -basedcorp99 -baylee4 -belchetz -bjin -blockedpath -brainage04 -cagedbird043 -cexll -chan1103 -chrys4lisfag -chuaaron -chuzui -ckumar1 -cloudsmithbrandon -codelonesomest -coderredlab -codertcy -comicchang -corrm -courtgpt -cyjaysong -czxtm -daandden -daaximus -danvincent -danzaio -darkphilosophy -defaceroot -deprecatedluke -derekszen -devnewbie1826 -dexhunter -disco-trooper -djdembeck -dkeken -dmarsh-gusto -dragonbaba -dylanbohlender -echopi -eggpeat -elikoga -enieuwy -eugenelo -eyycheev -fettpl -flare576 -foreveryoungpp -freespace8 -fryuni -gareth-rouse -gratefuldave -h4vc -habibpro1999 -handlecusion -haosenwang1018 -harshav167 -hellisotherpeople -heyitsgilbert -hezhiyang2000 -hheei -hobostay -hoishing -honsunrise -hpost -iacore -ig0rsky -igasmi -incloon -infernix -inprealpha -insodimension -isaac-sun -itertea -itzrnvr -jaaneek -jaeyeopme -jagravnaik -jasonw22 -jchristman -jdavv -jeffscottward -jfblaine -jiwangyihao -jorgoose -joswha -justmao945 -jwmacd -kamafozilov -kamijotoma -kenmege -kevcube -khanetor -korenkrita -korri123 -kukkerem -lance0 -larkinwc -larrygf -ldx -lederniermagicien -lee-si-yoon -lemeb -linqijin -liwuhou -llvm-x86 -loftiskg -loneexile -luceat-lux-vestra -lunarecl -luojiyin1987 -lyc-aon -m-005 -m3ridian-zero -m8than -maatheusgois-dd -makomakogo -masonc15 -mastertyko -mathews-tom -mattwilkinsonn -maximhar -maxvisionai -mayask -memvu -metaphorics -mgpai22 -mikeei -mmkzer0 -mokto -moutazhaq -mouyase -mq1n -mrshu -mumutw -muness -nasko25 -nibblebot -niklasschaeffer -nnk97 -nszceta -ogrodev -oldschoola -ondrejsojka -panosathdbx -paolomazzitti -paralin -parsifa1 -peterrauscher -pgupta-git -phanthh -pidevxplay -piedpiper911 -pppobear -qfrtt -ravshansbox -rburketaylor -rcbran -renstillmann -reqx -riverpilot -romanalexander -rysiuwroc -rznmkx -salmonumbrella -samy-mohsen-111 -scarthread -segmentationf4u1t -selimsandal -serejaris -serverinspector -shauryaswarup -shoucandanghehe -shyndman -silentknight87 -sit -slact -smileynet -sorphwer -sundbp -superhedge22 -svankina -szavadsky -tbui17 -tc97222 -tcf909 -tdiant -techdufus -tjboudreaux -tsagi2045 -turbomolli -tyrliang -unravl -usr-bin-roygbiv -ve3xone -vincent-huang-2000 -vmcall -voidchecksum -voiys -wahidinaji -watzon -will-bogusz will-bogusz -wodenjay -wolfiesch -wonjun3991 -wtergan -wuchengzu -xaviergmail -xiue233 -xoltus -yashnark -yingliang-zhang -zakhar-kogan -zamorakpds -zekdevs -zommiommy diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f4bb7e1a..0a5439da7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,14 +3,12 @@ name: CI on: push: branches: [main] - # Vouch bookkeeping commits (mitchellh/vouch writes VOUCHED.td back to - # main on !vouch/!denounce/!unvouch) only edit the vouch list and need no - # build. Skip CI when a main push changes nothing but the vouch file; a - # push that also touches anything else still runs the full matrix. - paths-ignore: - - .github/VOUCHED.td + paths: + - "packages/**" pull_request: branches: [main] + paths: + - "packages/**" workflow_dispatch: inputs: skip_npm: diff --git a/.github/workflows/vouch-manage.yml b/.github/workflows/vouch-manage.yml deleted file mode 100644 index e7ce41b11..000000000 --- a/.github/workflows/vouch-manage.yml +++ /dev/null @@ -1,44 +0,0 @@ -name: Vouch (manage) - -# Let maintainers vouch/denounce/unvouch by commenting on a Discussion: -# !vouch vouch the discussion author -# !vouch @user [reason] vouch a specific user -# !denounce [@user] [reason] -# !unvouch [@user] -# Only collaborators with admin/maintain/write are honored (triage EXCLUDED; -# upstream's default `roles` includes triage, which we override below). -# -# Commits the VOUCHED.td change back to the default branch using the stock -# GITHUB_TOKEN (no GitHub App needed). NOTE: this works only while the default -# branch is UNPROTECTED — GITHUB_TOKEN cannot bypass branch protection. If you -# protect the branch later, switch back to a GitHub App token on a bypass list. - -on: - discussion_comment: - types: [created] - -# Serialize writes to VOUCHED.td so concurrent vouches don't clobber. -concurrency: - group: vouch-manage - cancel-in-progress: false - -permissions: - contents: write # commit VOUCHED.td - discussions: write # read the comment / acknowledge - -jobs: - manage: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: mitchellh/vouch/action/manage-by-discussion@v1 - with: - discussion-number: ${{ github.event.discussion.number }} - comment-node-id: ${{ github.event.comment.node_id }} - vouch-keyword: "!vouch" - denounce-keyword: "!denounce" - unvouch-keyword: "!unvouch" - roles: admin,maintain,write - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/vouch-pr.yml b/.github/workflows/vouch-pr.yml deleted file mode 100644 index 6afa86cdc..000000000 --- a/.github/workflows/vouch-pr.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: Vouch (PR gate) - -# Auto-close PRs from unvouched or denounced users. Issues are left alone -# (robomp triages those). Runs under `pull_request_target` so the token can -# act on fork PRs; this job does NO checkout and runs NO PR code — it only -# reads .github/VOUCHED.td from the base repo and calls the GitHub API. - -on: - pull_request_target: - types: [opened, reopened, ready_for_review] - -permissions: - contents: read # read VOUCHED.td from the base branch - pull-requests: write # close + comment - issues: write # add the `vouched` label (labels use the Issues API) - -concurrency: - group: vouch-pr-${{ github.event.pull_request.number }} - cancel-in-progress: true - -jobs: - check: - runs-on: ubuntu-latest - steps: - - id: vouch - uses: mitchellh/vouch/action/check-pr@v1 - with: - pr-number: ${{ github.event.pull_request.number }} - auto-close: true - require-vouch: true # block unvouched, not only denounced - # vouched-file: .github/VOUCHED.td (default) - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - # Survivors of the gate (vouched, or auto-allowed collaborators/bots) get - # a FRESH `vouched` label on every (re)open / ready-for-review. robomp - # reviews ONLY on that label event (ROBOMP_PR_REVIEW_TRIGGER=vouched_label), - # so review is always triggered by a just-validated PR, never a stale label. - - name: Label vouched PRs for robomp review - if: ${{ steps.vouch.outputs.status == 'vouched' || steps.vouch.outputs.status == 'allowed' }} - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - run: | - gh label create vouched --repo "$REPO" --color 2da44e --description "Passed the vouch gate" --force - # remove+add so a fresh `labeled` event fires even when the label - # persisted across close/reopen (re-adding an existing label emits no - # event). The check above just re-validated, so trust is never stale. - gh pr edit "$PR" --repo "$REPO" --remove-label vouched || true - gh pr edit "$PR" --repo "$REPO" --add-label vouched diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 66f25b399..0e0930e46 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,54 +1,84 @@ # Contributing to oh-my-pi -Thanks for your interest in contributing. This project uses a lightweight -**vouch** system to decide who can open pull requests. Please read this before -opening a PR. +Pull requests are welcome. Keep them focused, understand the work you submit, +and be prepared to explain and maintain it. -## TL;DR +## Before you start -- **Issues are open to everyone.** File bugs, feature requests, and questions - freely — they are triaged automatically. -- **Pull requests require a vouch.** A PR whose author is not vouched (or is - denounced) is **closed automatically**. If you are not yet vouched, do **not** - open a PR to get noticed — it will be closed on sight. Start a Discussion and - ask to be vouched first (see below). +### Small changes -## Who can open PRs +Bug fixes, documentation updates, and narrowly scoped improvements can go +straight to a pull request. -A pull request is accepted when its author is any of: +### Major changes -- a repository collaborator (write access or above), or a bot; or -- listed — without a leading `-` — in [`.github/VOUCHED.td`](.github/VOUCHED.td). +Discuss major features and broad architectural or behavioral changes in +[Discord](https://discord.gg/4NMW9cdXZa) **before writing the implementation**. +This includes new subsystems, large UI changes, new dependencies, and changes +that span several packages. A GitHub issue is not a substitute for this +discussion, and prior discussion does not guarantee that a pull request will be +merged. -Anyone **denounced** (prefixed with `-` in that file) is always blocked. +### Do not open an issue for work you are about to submit -## Getting vouched +If you intend to implement a change yourself, **do not create an issue for it +first**. robomp treats actionable issues as work to pick up and may start the +same fix in parallel, wasting compute and maintainer time. -1. Open a [Discussion](../../discussions) (or comment on an existing one) - describing what you'd like to contribute. -2. A maintainer vouches you by commenting **`!vouch`** (vouches the discussion - author) or **`!vouch @your-handle`** on that discussion. -3. Once you appear in `.github/VOUCHED.td`, open your PR — it stays open and is - reviewed. +Open an issue when you are reporting a problem or proposing work that you are +not already turning into a pull request. If a relevant issue already exists, +link it from your pull request instead of creating another one. -Maintainers may also `!denounce [@user]` and `!unvouch [@user]`. Only -collaborators with admin/maintain/write can run these commands. +## AI-assisted contributions -## What happens to your PR +AI agents are welcome as tools, not as unattended contributors. Do not give an +agent a vague goal and submit whatever it produces. -| You are… | Result | -| --- | --- | -| Vouched (or a collaborator) | PR stays open → automated review → human review | -| Not vouched | PR closed with a comment — get vouched, then reopen or open a new PR | -| Denounced | PR closed | +Before opening a pull request, you must: -Pushing more commits to an open, vouched PR is fine — it remains vouched. +- constrain the agent to the agreed scope and reject unrelated changes; +- review every changed file and understand the resulting behavior; +- run the relevant checks and exercise the changed behavior yourself; and +- submit the pull request only after that review, rather than letting an agent + publish it autonomously. -## The VOUCHED.td file +You are responsible for the code, regardless of who or what generated it. -[`.github/VOUCHED.td`](.github/VOUCHED.td) is the source of truth: one handle per -line, sorted alphabetically, optionally `platform:handle`, with `-` marking a -denouncement and an optional reason after the handle. The format follows -[mitchellh/vouch](https://github.com/mitchellh/vouch); the denouncement list is -intentionally public so other projects can reuse our prior knowledge of bad -actors. +## Pull request requirements + +Every pull request body **MUST include at least one sentence written by you, in +your own words**, explaining what changed and why. A generated summary, pasted +agent transcript, or checklist alone does not satisfy this requirement. + +One honest line is enough: + +> I reviewed the full diff; this change fixes duplicate PR reviews by reusing +> the existing delivery guard. + +You **MUST verify that the change works as intended**. `bun check` and automated +tests are expected where relevant, but they are not proof that the behavior +works. Exercise the changed path yourself and report the exact scenario and +result in the pull request: + +- for a bug fix, reproduce the bug and confirm the same reproduction no longer + fails; +- for a feature, launch the product and use the feature end to end; and +- for a UI change, interact with it and inspect the rendered result. + +“`bun check` passes” by itself is not sufficient verification. For coding-agent +development commands and repository structure, see +[`packages/coding-agent/DEVELOPMENT.md`](packages/coding-agent/DEVELOPMENT.md). + +Keep each pull request to one logical change. Avoid unrelated cleanup, +drive-by refactors, generated noise, or features that were not part of the +agreed scope. + +## Review + +Maintainers review the submitted behavior and the contributor's understanding +of it—not the volume of generated code. Respond to review feedback yourself, +and only apply suggestions you have checked. + +Pull requests may be closed when they skip required prior discussion, lack the +human-written explanation, contain unreviewed agent output, or mix unrelated +changes. diff --git a/README.md b/README.md index 00e9fba7a..a95d22f32 100644 --- a/README.md +++ b/README.md @@ -592,12 +592,7 @@ For architecture and contribution guidelines, see [packages/coding-agent/DEVELOP ## Contributing -Issues are open to everyone. **Pull requests require a vouch** — PRs from -unvouched or denounced authors are closed automatically. If you're not yet -vouched, open a [Discussion](https://github.com/can1357/oh-my-pi/discussions) -and ask a maintainer to `!vouch` you rather than opening a PR (which would be -closed on sight). See **[CONTRIBUTING.md](CONTRIBUTING.md)** and -[`.github/VOUCHED.td`](.github/VOUCHED.td) for the full policy. +Issues and pull requests are open to everyone. See **[CONTRIBUTING.md](CONTRIBUTING.md)** for guidelines on contributing. --- diff --git a/python/robomp/.env.example b/python/robomp/.env.example index a338cdf82..164c2715b 100644 --- a/python/robomp/.env.example +++ b/python/robomp/.env.example @@ -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] # ============================================================================= diff --git a/python/robomp/docker-compose.yml b/python/robomp/docker-compose.yml index 3d8aed813..7a5d149fd 100644 --- a/python/robomp/docker-compose.yml +++ b/python/robomp/docker-compose.yml @@ -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} diff --git a/python/robomp/src/config.py b/python/robomp/src/config.py index 046e7c656..2679fd5d3 100644 --- a/python/robomp/src/config.py +++ b/python/robomp/src/config.py @@ -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 diff --git a/python/robomp/src/github_events.py b/python/robomp/src/github_events.py index 5ffa9917d..8af7a0e10 100644 --- a/python/robomp/src/github_events.py +++ b/python/robomp/src/github_events.py @@ -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")) diff --git a/python/robomp/src/server.py b/python/robomp/src/server.py index 36aa3d5a5..70e38abe6 100644 --- a/python/robomp/src/server.py +++ b/python/robomp/src/server.py @@ -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, ) diff --git a/python/robomp/tests/test_github_events.py b/python/robomp/tests/test_github_events.py index 861ca1b0b..cbefdc034 100644 --- a/python/robomp/tests/test_github_events.py +++ b/python/robomp/tests/test_github_events.py @@ -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" diff --git a/python/robomp/tests/test_queue_dispatch.py b/python/robomp/tests/test_queue_dispatch.py index 9ffa056be..6d739afce 100644 --- a/python/robomp/tests/test_queue_dispatch.py +++ b/python/robomp/tests/test_queue_dispatch.py @@ -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: