Make bedevere work outside of the CPython repo

README.md

@@ -5,6 +5,110 @@
 
 This bot is meant to help identify issues with a CPython pull request.
 
+## Which repositories it runs on
+
+Most of bedevere assumes CPython's workflow: `gh-NNNNN` issue numbers,
+`Misc/NEWS.d` entries, `X.Y` maintenance branches and the awaiting-review
+labels. So each repository the app is installed on runs only the features
+enabled for it in [`bedevere/config.py`](bedevere/config.py).
+
+### How it works
+
+Every webhook goes to `dispatch()` in `bedevere/__main__.py`. It looks up the
+repository the event came from with `config.features_for()`, then passes the
+event to the router of each enabled feature, in the order of
+`FEATURE_ROUTERS`. Features that aren't enabled never see the event.
+
+- `python/cpython` runs every feature.
+- A `python` org repository with no entry in `REPO_FEATURES` runs only
+  `policy`, the AI policy labels. This is a safe default for a repository
+  added to the installation before anyone has decided its features; it is
+  not a limit, and an entry can turn on any of the features below.
+- A repository outside the `python` org runs nothing unless it has an entry.
+  Bedevere is meant for python org repositories only.
+- Bedevere logs the first event it gets from a repository with no entry.
+- Repository names are matched without regard to case.
+
+| Feature     | What it does                                                                 |
+| ----------- | ---------------------------------------------------------------------------- |
+| `backport`  | `needs backport to X.Y` labels, the `[X.Y]` title status, copies labels to backports |
+| `close_pr`  | Closes PRs that merge a maintenance branch into `main`                       |
+| `filepaths` | `docs` and `tests` labels based on the files changed                         |
+| `gh_issue`  | The `bedevere/issue-number` status for `gh-NNNNN` in the title               |
+| `news`      | The `bedevere/news` status for `Misc/NEWS.d` entries                         |
+| `policy`    | Comments on issues and PRs labeled `AI-needs-explanation` or `AI-guidelines-not-met`; the second label also closes the item and removes the first |
+| `stage`     | The awaiting-review stage labels and comments                                |
+
+`filepaths` also runs the NEWS check when a PR is opened or updated. It skips
+that check when `news` is off, so a repository can have `filepaths` without
+`news`.
+
+### Which features fit a repository
+
+Features are turned on or off per repository. None of them have per-repository
+settings: a feature behaves the same on every repository that runs it.
+
+| Feature     | Fits other python org repositories? |
+| ----------- | ----------------------------------- |
+| `policy`    | Yes. |
+| `stage`     | Yes, if PRs are reviewed by members of the python org's **Python core** team. A review from anyone else counts as a non-core review, so a repository whose reviewers aren't in that team would leave PRs at `awaiting core review`. Its comments say "A Python core developer". |
+| `filepaths` | Partly. It goes by file name, so it works on any repository: when a PR is opened, one that changes only docs files (`.rst`, `.md`, `.nitignore`) gets `docs`, and one that changes only test files (`test_*`, `_test*`), or tests mixed with docs, gets `tests`. But a docs-only PR with no `Misc/NEWS.d` entry also gets CPython's `skip news` label. |
+| `close_pr`  | Only if `X.Y` branches are maintenance branches that must never be merged into `main`. Its comment links to CPython. |
+| `gh_issue`, `news`, `backport` | No. They expect CPython's `gh-NNNNN` titles, `Misc/NEWS.d` entries and backport workflow, and post failing statuses or wrong labels elsewhere. |
+
+### Setting up a repository
+
+1. **Agree with the repository's maintainers** that they want bedevere, and
+   which features from the table above.
+2. **If they want more than `policy`, add the repository to `REPO_FEATURES`**
+   in `bedevere/config.py`, with every feature it should run:
+
+   ```python
+   REPO_FEATURES = {
+       "python/cpython": ALL_FEATURES,
+       "python/devguide": frozenset({"policy", "stage"}),
+   }
+   ```
+
+   Use the lowercase `owner/name`. An entry replaces the default, so list
+   `policy` too unless the repository shouldn't have the AI policy labels.
+   Open a pull request; the features take effect once it's merged and
+   deployed. A repository that wants `policy` only needs no entry.
+3. **Create the labels the features use** in the repository (Issues, Labels,
+   New label). Copy each description from
+   [CPython's labels](https://github.com/python/cpython/labels), since GitHub
+   shows it in the label picker and it tells triagers what applying the label
+   does. The colour doesn't matter to bedevere; copying it only keeps the
+   labels looking the same across repositories.
+   - `policy`: `AI-needs-explanation` (bedevere asks the author to explain
+     the contribution in their own words) and `AI-guidelines-not-met`
+     (bedevere comments, closes the issue or PR, and removes
+     `AI-needs-explanation` if it's there). Either label does nothing on a
+     closed item, and adding `AI-needs-explanation` to an item that already
+     has `AI-guidelines-not-met` does nothing. People apply these, and
+     bedevere acts only when the name matches exactly, including case. Until
+     they exist bedevere does nothing, and a different spelling
+     (`AI needs explanation`) is ignored without any error.
+   - `stage`: `awaiting review`, `awaiting core review`, `awaiting changes`,
+     `awaiting change review` and `awaiting merge`. Bedevere applies these
+     itself; if one is missing, GitHub creates it with a default colour and
+     no description.
+4. **Add the repository to the installation.** A python org admin opens the
+   org's settings, GitHub Apps, bedevere, Configure, and adds the repository
+   under Repository access. If step 2 applies, do this after the deploy, or
+   the repository runs `policy` only until then.
+5. **Check that it works.** Open a test issue, apply `AI-needs-explanation`,
+   and check that bedevere comments within a minute or so, then delete or
+   close the issue. With `stage`, open a test PR and check it gets an
+   `awaiting` label. If nothing happens, check the label names and the app's
+   Recent Deliveries in its settings.
+6. **Tell the repository's triagers** which features are on and what each
+   label does.
+
+To change a repository's features later, edit its entry and open a pull
+request. To turn bedevere off on a repository, remove the repository from the
+installation.
+
 ## What the bot does:
 - ### Identifies missing GitHub issue numbers in the title
 If no GitHub issue number is found the status fails and the

bedevere/__main__.py

@@ -12,21 +12,39 @@
 from gidgethub import apps, routing, sansio
 
 from . import backport, close_pr, filepaths, gh_issue, news, policy, stage
+from .config import features_for
 
-router = routing.Router(
-    backport.router,
-    gh_issue.router,
-    close_pr.router,
-    filepaths.router,
-    news.router,
-    policy.router,
-    stage.router,
-)
+# App-level handlers that run for every event, whatever the repository.
+router = routing.Router()
+
+# Feature routers, in dispatch order. Each repository runs only the features
+# that features_for() enables for it.
+FEATURE_ROUTERS = {
+    "backport": backport.router,
+    "gh_issue": gh_issue.router,
+    "close_pr": close_pr.router,
+    "filepaths": filepaths.router,
+    "news": news.router,
+    "policy": policy.router,
+    "stage": stage.router,
+}
 cache = cachetools.LRUCache(maxsize=500)
 
 sentry_sdk.init(os.environ.get("SENTRY_DSN"))
 
 
+async def dispatch(event, gh, **kwargs):
+    """Dispatch an event to the app-level router and the repo's features."""
+    await router.dispatch(event, gh, **kwargs)
+    repo = event.data.get("repository")
+    if repo is None:
+        return
+    enabled = features_for(repo["full_name"])
+    for name, feature_router in FEATURE_ROUTERS.items():
+        if name in enabled:
+            await feature_router.dispatch(event, gh, features=enabled, **kwargs)
+
+
 async def main(request):
     try:
         body = await request.read()
@@ -51,7 +69,7 @@ async def main(request):
 
             # Give GitHub some time to reach internal consistency.
             await asyncio.sleep(1)
-            await router.dispatch(event, gh, session=session)
+            await dispatch(event, gh, session=session)
         try:
             print("GH requests remaining:", gh.rate_limit.remaining)
         except AttributeError:

bedevere/config.py

@@ -0,0 +1,38 @@
+"""Which bedevere features run on each repository."""
+
+import sys
+
+ALL_FEATURES = frozenset(
+    {"backport", "close_pr", "filepaths", "gh_issue", "news", "policy", "stage"}
+)
+DEFAULT_FEATURES = frozenset({"policy"})
+DEFAULT_OWNER = "python"
+
+# Keys are lowercase "owner/name". Unlisted repos owned by DEFAULT_OWNER get
+# DEFAULT_FEATURES, and unlisted repos owned by anyone else get no features.
+REPO_FEATURES = {
+    "python/cpython": ALL_FEATURES,
+}
+
+_logged_unknown_repos = set()
+
+
+def features_for(repo_full_name):
+    """Return the features enabled for a repository.
+
+    Repositories without an entry are logged the first time they are seen.
+    """
+    repo = repo_full_name.lower()
+    try:
+        return REPO_FEATURES[repo]
+    except KeyError:
+        owner = repo.partition("/")[0]
+        features = DEFAULT_FEATURES if owner == DEFAULT_OWNER else frozenset()
+        if repo not in _logged_unknown_repos:
+            _logged_unknown_repos.add(repo)
+            print(
+                f"{repo_full_name} has no feature config, running only: "
+                f"{', '.join(sorted(features)) or 'nothing'}",
+                file=sys.stderr,
+            )
+        return features

bedevere/filepaths.py

@@ -10,13 +10,15 @@
 @router.register("pull_request", action="opened")
 @router.register("pull_request", action="synchronize")
 @router.register("pull_request", action="reopened")
-async def check_file_paths(event, gh, *args, **kwargs):
+async def check_file_paths(event, gh, *args, features=None, **kwargs):
+    # Callers that don't pass features (such as tests) get the NEWS check.
+    run_news = features is None or "news" in features
     pull_request = event.data["pull_request"]
     files = await util.files_for_PR(gh, pull_request)
     filenames = [file["file_name"] for file in files]
     if event.data["action"] == "opened":
         labels = await prtype.classify_by_filepaths(gh, pull_request, filenames)
-        if prtype.Labels.skip_news not in labels:
+        if run_news and prtype.Labels.skip_news not in labels:
             await news.check_news(gh, pull_request, files)
-    else:
+    elif run_news:
         await news.check_news(gh, pull_request, files)

tests/test___main__.py

@@ -4,6 +4,7 @@
 from gidgethub import sansio
 
 from bedevere import __main__ as main
+from bedevere import config
 
 app_installation_payload = {
     "installation": {
@@ -92,3 +93,60 @@ async def test_repo_installation_added(capfd):
         f"App installed by {event.data['installation']['account']['login']}, installation_id: {event.data['installation']['id']}"
         in out
     )
+
+
+def pr_opened_event(repo):
+    return sansio.Event(
+        {
+            "action": "opened",
+            "number": 1,
+            "pull_request": {
+                "url": f"https://api.github.com/repos/{repo}/pulls/1",
+                "statuses_url": f"https://api.github.com/repos/{repo}/statuses/abc",
+                "issue_url": f"https://api.github.com/repos/{repo}/issues/1",
+                "title": "Fix a typo",
+                "body": "",
+                "user": {"login": "contributor"},
+                "author_association": "NONE",
+                "draft": False,
+                "base": {"ref": "main"},
+                "head": {"ref": "fix-typo", "label": "contributor:fix-typo"},
+            },
+            "repository": {"full_name": repo},
+        },
+        event="pull_request",
+        delivery_id="1",
+    )
+
+
+async def test_unconfigured_repo_runs_no_cpython_features():
+    # FakeGH has no API methods, so any status, label or comment call fails.
+    await main.dispatch(pr_opened_event("python/devguide"), FakeGH())
+
+
+async def test_configured_repo_runs_its_features_in_order():
+    dispatched = []
+
+    def spy(name):
+        class SpyRouter:
+            async def dispatch(self, event, gh, **kwargs):
+                dispatched.append((name, kwargs["features"]))
+
+        return SpyRouter()
+
+    spies = {name: spy(name) for name in main.FEATURE_ROUTERS}
+    with mock.patch.dict(main.FEATURE_ROUTERS, spies):
+        await main.dispatch(pr_opened_event("python/cpython"), FakeGH())
+
+    assert [name for name, _ in dispatched] == list(main.FEATURE_ROUTERS)
+    assert all(features == config.ALL_FEATURES for _, features in dispatched)
+
+
+async def test_event_without_repository_runs_no_features():
+    event_data = {"action": "created"}
+    event_data.update(app_installation_payload)
+    event = sansio.Event(event_data, event="installation", delivery_id="1")
+    with mock.patch.dict(main.FEATURE_ROUTERS, {}, clear=True):
+        main.FEATURE_ROUTERS["policy"] = mock.AsyncMock()
+        await main.dispatch(event, FakeGH())
+        main.FEATURE_ROUTERS["policy"].dispatch.assert_not_called()

tests/test_config.py

@@ -0,0 +1,39 @@
+import pytest
+
+from bedevere import config
+
+
+@pytest.fixture(autouse=True)
+def reset_logged_repos(monkeypatch):
+    monkeypatch.setattr(config, "_logged_unknown_repos", set())
+
+
+def test_cpython_gets_every_feature():
+    assert config.features_for("python/cpython") == config.ALL_FEATURES
+
+
+def test_unknown_python_repo_gets_policy_only():
+    assert config.features_for("python/devguide") == {"policy"}
+
+
+@pytest.mark.parametrize("repo", ["mariatta/devguide", "pythonista/cpython"])
+def test_repo_outside_python_org_gets_nothing(repo):
+    assert config.features_for(repo) == frozenset()
+
+
+def test_lookup_ignores_case():
+    assert config.features_for("Python/CPython") == config.ALL_FEATURES
+    assert config.features_for("Python/DevGuide") == {"policy"}
+
+
+def test_unknown_repo_logged_once(capfd):
+    config.features_for("python/devguide")
+    config.features_for("python/devguide")
+    out, err = capfd.readouterr()
+    assert err.count("python/devguide has no feature config") == 1
+
+
+def test_repo_outside_python_org_logged_as_running_nothing(capfd):
+    config.features_for("mariatta/devguide")
+    out, err = capfd.readouterr()
+    assert "mariatta/devguide has no feature config, running only: nothing" in err

tests/test_filepaths.py

@@ -261,3 +261,34 @@ async def test_synchronize():
     assert len(gh.post_url) == 1
     assert gh.post_url[0] == "https://api.github.com/some/status"
     assert gh.post_data[0]["state"] == "success"
+
+
+@pytest.mark.parametrize("action", ["opened", "synchronize"])
+async def test_no_news_check_when_news_disabled(action):
+    filenames = [
+        {
+            "filename": "test_docs2.py",
+            "patch": "@@ -31,3 +31,7 @@ # Licensed to PSF under a Contributor Agreement.",
+        },
+    ]
+    issue = {"labels": [], "labels_url": "https://api.github.com/some/label"}
+    gh = FakeGH(getiter=filenames, getitem=issue)
+    event_data = {
+        "action": action,
+        "number": 1234,
+        "pull_request": {
+            "url": "https://api.github.com/repos/cpython/python/pulls/1234",
+            "statuses_url": "https://api.github.com/some/status",
+            "issue_url": "https://api.github.com/repos/cpython/python/issue/1234",
+            "issue_comment_url": "https://api.github.com/repos/cpython/python/issue/1234/comments",
+            "author_association": "NONE",
+        },
+    }
+    event = sansio.Event(event_data, event="pull_request", delivery_id="1")
+    await filepaths.router.dispatch(event, gh, features=frozenset({"filepaths"}))
+    if action == "opened":
+        # The type label is still applied, but there's no NEWS status or help.
+        assert gh.post_url == ["https://api.github.com/some/label"]
+        assert gh.post_data == [[Labels.tests.value]]
+    else:
+        assert gh.post_url == []