feat(review): add opt-in exact-head formal GitHub reviews

CHANGELOG.md

@@ -8,6 +8,11 @@ Format: [Keep a Changelog](https://keepachangelog.com/de/1.1.0/), [Semantic Vers
 
 ### Hinzugefügt / Added
 
+- Opt-in-Aktion `pr.review` für formale GitHub-Reviews am exakten Head mit Kontoprüfung,
+  verpflichtendem Event/Text, Duplikatprüfung und unabhängigem Review-/Head-Readback.
+  Standardmäßig deaktiviert; braucht `permissions.enabled` und `policy.publish_reviews`.
+  Keine automatische Aktivierung oder Abnahme eigener Änderungen. / Opt-in formal reviews
+  bound to the exact commit, with account checks, duplicate detection and independent readback.
 - Getypte Schreibaktionen `pr.merge`, `pr.ready`, `actions.dispatch`, `actions.rerun`, `pr.create`
   und `git.push_branch` mit frischem Lesen, exakter Head-Bindung und Rücklesen; in der
   Standard-Obergrenze, aber nie standardmäßig aktiv. Jede braucht zusätzlich ihren

README.md

@@ -112,20 +112,37 @@ not a silent override. `permissions.enabled` must stay inside the ceiling.
 | `api.request` | disabled | any write method on an endpoint that starts with one of `permissions.api_write_prefixes` |
 | `pr.merge` | disabled | re-reads the PR (open, exact `sha`, default base, not draft, `mergeable_state` clean, configured merge account), `PUT …/pulls/{n}/merge` with `sha`, reads `merged` back |
 | `pr.ready` | disabled | re-reads the draft at the exact `sha`, GraphQL `markPullRequestReadyForReview`, reads back |
+| `pr.review` | disabled | submits `COMMENT`, `APPROVE` or `REQUEST_CHANGES` to `POST …/pulls/{n}/reviews` with mandatory `sha`/body; requires the configured account and reads review ID, actor, commit, body and state back |
 | `actions.dispatch` | disabled | `POST …/actions/workflows/{workflow}/dispatches` for an active workflow |
 | `actions.rerun` | disabled | `POST …/actions/runs/{id}/rerun` for a completed, non-fork run of the exact `sha` |
 | `pr.create` | disabled | `POST …/pulls` from a bot-prefixed branch onto the default branch |
 | `git.push_branch` | disabled | `git push origin <sha>:refs/heads/<branch>` without force, bot prefix only (`typed_actions.push_branch_prefix`, default `bot/`), never the default branch, verified with `ls-remote` |
 
-The six typed actions sit in the default ceiling but are never enabled by default. Each one also
+The seven typed actions sit in the default ceiling but are never enabled by default. Each one also
 needs its `multi_bot` switch (`merge_enabled`, `policy.draft_*`, `policy.trigger_missing_ci`,
-`policy.open_fix_prs`); a permission alone opens nothing. One action at a time:
+`policy.open_fix_prs`, `policy.publish_reviews`); a permission alone opens nothing. One action at a time:
 
 ```bash
 githubbot-core --config instance.json act pr.merge --params '{"repo": "o/r", "number": 7, "sha": "<40-hex>"}'
 githubbot-core --config instance.json act pr.merge --params '…' --apply --expect-plan <plan_sha256>
 ```
 
+Formal reviews require an explicit instance opt-in, for example
+`{"permissions":{"enabled":["pr.review"]},"multi_bot":{"merge_account":"example-bot","policy":{"publish_reviews":true}}}`.
+Then preview `act pr.review --params '{"repo":"o/r","number":7,"sha":"<40-hex>","event":"COMMENT","body":"Review scope and evidence"}'`
+and apply that exact plan. Unlike `issue.comment`, this creates a submitted GitHub review.
+An omitted event or empty body is refused; inline comments and pending reviews are not supported.
+The [GitHub review API](https://docs.github.com/en/rest/pulls/reviews#create-a-review-for-a-pull-request)
+binds the review to `commit_id`. Core additionally reads the head before and after publishing.
+Drafts accept only `COMMENT`; stacked PRs can be reviewed without relaxing merge gates.
+An existing review by this account at the same head prevents another POST, including after an
+ambiguous timeout. A failed post/readback may already have created a review: inspect the reported
+review ID or list before retrying. Concurrent publishers still need shared scope locks; this is
+not an atomic server-side uniqueness guarantee. Full diff inspection, author-instance independence,
+scope locks and review evidence remain the caller's responsibility. Account self-approval/change
+requests are refused, but a different GitHub account alone does not prove an independent session.
+Installing this capability does not enable it; existing instance configurations remain unchanged.
+
 ```json
 {"permissions": {"enabled": ["issue.add_labels", "issue.comment", "api.request"],
                  "api_write_prefixes": ["user/starred/"]}}

README_de.md

@@ -114,21 +114,40 @@ stille Überschreibung. `permissions.enabled` muss innerhalb der Obergrenze blei
 | `api.request` | aus | beliebige Schreibmethode auf einem Endpunkt, der mit einem Eintrag aus `permissions.api_write_prefixes` beginnt |
 | `pr.merge` | aus | liest den PR frisch (offen, exakter `sha`, Default-Base, kein Draft, `mergeable_state` clean, konfiguriertes Merge-Konto), `PUT …/pulls/{n}/merge` mit `sha`, liest `merged` zurück |
 | `pr.ready` | aus | liest den Draft am exakten `sha` frisch, GraphQL `markPullRequestReadyForReview`, liest zurück |
+| `pr.review` | aus | veröffentlicht `COMMENT`, `APPROVE` oder `REQUEST_CHANGES` über `POST …/pulls/{n}/reviews`, mit verpflichtendem `sha` und Text; prüft Konto und liest Review-ID, Autor, Commit, Text und Zustand zurück |
 | `actions.dispatch` | aus | `POST …/actions/workflows/{workflow}/dispatches` für einen aktiven Workflow |
 | `actions.rerun` | aus | `POST …/actions/runs/{id}/rerun` für einen abgeschlossenen Lauf des exakten `sha`, nie aus Forks |
 | `pr.create` | aus | `POST …/pulls` von einem Branch mit Bot-Präfix auf den Default-Branch |
 | `git.push_branch` | aus | `git push origin <sha>:refs/heads/<branch>` ohne Force, nur Bot-Präfix (`typed_actions.push_branch_prefix`, Standard `bot/`), nie der Default-Branch, Prüfung per `ls-remote` |
 
-Die sechs getypten Aktionen stehen in der Standard-Obergrenze, sind aber nie standardmäßig aktiv.
+Die sieben getypten Aktionen stehen in der Standard-Obergrenze, sind aber nie standardmäßig aktiv.
 Jede braucht zusätzlich ihren `multi_bot`-Schalter (`merge_enabled`, `policy.draft_*`,
-`policy.trigger_missing_ci`, `policy.open_fix_prs`); eine Berechtigung allein öffnet nichts.
+`policy.trigger_missing_ci`, `policy.open_fix_prs`, `policy.publish_reviews`); eine Berechtigung allein öffnet nichts.
 Eine Aktion je Aufruf:
 
 ```bash
 githubbot-core --config instanz.json act pr.merge --params '{"repo": "o/r", "number": 7, "sha": "<40-hex>"}'
 githubbot-core --config instanz.json act pr.merge --params '…' --apply --expect-plan <plan_sha256>
 ```
 
+Formale Reviews brauchen ein ausdrückliches Instanz-Opt-in, beispielsweise
+`{"permissions":{"enabled":["pr.review"]},"multi_bot":{"merge_account":"example-bot","policy":{"publish_reviews":true}}}`.
+Danach `act pr.review --params '{"repo":"o/r","number":7,"sha":"<40-hex>","event":"COMMENT","body":"Prüfumfang und Belege"}'`
+als Vorschau ausführen und genau diesen Plan anwenden. Anders als `issue.comment` entsteht ein
+veröffentlichtes GitHub-Review. Ein fehlendes Event oder leerer Text wird verweigert;
+Inline-Kommentare und ausstehende Reviews werden nicht unterstützt.
+Die [GitHub-Review-API](https://docs.github.com/en/rest/pulls/reviews#create-a-review-for-a-pull-request)
+bindet das Review an `commit_id`; Core liest den Head zusätzlich vor und nach Veröffentlichung.
+Drafts erlauben nur `COMMENT`; gestapelte PRs dürfen geprüft werden, ohne Merge-Gates zu lockern.
+Ein vorhandenes Review dieses Kontos am selben Head verhindert einen weiteren POST, auch nach
+einem unklaren Timeout. Bei fehlgeschlagenem POST/Readback kann das Review bereits existieren:
+vor einer Wiederholung die gemeldete Review-ID oder Liste prüfen. Gleichzeitige Publisher brauchen
+weiter gemeinsame Scope-Locks; dies ist keine atomare Eindeutigkeitsgarantie des Servers.
+Vollständige Diff-Prüfung, Unabhängigkeit der Autorensitzung, Scope-Locks und Belege bleiben
+Aufgabe des Aufrufers. Approval/Änderungswünsche am eigenen Konto-PR werden verweigert; ein
+anderes GitHub-Konto allein beweist jedoch keine unabhängige Sitzung. Die Installation aktiviert
+die Fähigkeit nicht; bestehende Instanzkonfigurationen bleiben unverändert.
+
 ```json
 {"permissions": {"enabled": ["issue.add_labels", "issue.comment", "api.request"],
                  "api_write_prefixes": ["user/starred/"]}}

src/githubbot_core/default_config.json

@@ -10,6 +10,7 @@
       "api.request",
       "pr.merge",
       "pr.ready",
+      "pr.review",
       "actions.dispatch",
       "actions.rerun",
       "pr.create",

src/githubbot_core/typed_actions.py

@@ -21,7 +21,8 @@
 MERGE_METHODS = ("merge", "squash", "rebase")
 DEFAULT_PUSH_PREFIX = "bot/"
 
-KINDS = ("pr.merge", "pr.ready", "actions.dispatch", "actions.rerun", "pr.create", "git.push_branch")
+KINDS = ("pr.merge", "pr.ready", "pr.review", "actions.dispatch", "actions.rerun", "pr.create", "git.push_branch")
+REVIEW_STATES = {"COMMENT": "COMMENTED", "APPROVE": "APPROVED", "REQUEST_CHANGES": "CHANGES_REQUESTED"}
 
 _git = subprocess.run  # replaced in tests
 
@@ -44,6 +45,7 @@ def switch_denial(kind: str, config: dict) -> Optional[str]:
     allowed = {
         "pr.merge": policy["merge_enabled"],
         "pr.ready": policy["draft_auto_ready_hours"] is not None or policy["draft_ready_when_mature"],
+        "pr.review": policy["publish_reviews"],
         "actions.dispatch": policy["trigger_missing_ci"],
         "actions.rerun": policy["trigger_missing_ci"],
         "pr.create": policy["open_fix_prs"],
@@ -63,11 +65,21 @@ def param_denial(kind: str, params: dict, config: dict) -> Optional[str]:
         return None
     if not REPO.fullmatch(str(params.get("repo", ""))):
         return "repo must be owner/name"
-    need_sha = kind in ("pr.merge", "pr.ready", "actions.rerun", "git.push_branch")
+    need_sha = kind in ("pr.merge", "pr.ready", "pr.review", "actions.rerun", "git.push_branch")
     if need_sha and not SHA.fullmatch(str(params.get("sha", ""))):
         return "sha must be the exact reviewed 40-character lowercase commit"
-    if kind in ("pr.merge", "pr.ready") and not str(params.get("number", "")).isdecimal():
+    if kind in ("pr.merge", "pr.ready", "pr.review") and not str(params.get("number", "")).isdecimal():
         return "number must be a pull request number"
+    if kind == "pr.review":
+        if int(params["number"]) < 1:
+            return "number must be a positive pull request number"
+        if not isinstance(params.get("event"), str) or params["event"] not in REVIEW_STATES:
+            return "event must be COMMENT, APPROVE or REQUEST_CHANGES"
+        body = params.get("body")
+        if not isinstance(body, str) or not body.strip() or len(body) > 65536:
+            return "body must be nonempty review text of at most 65536 characters"
+        if not str((config.get("multi_bot") or {}).get("merge_account") or "").strip():
+            return "pr.review requires the configured multi_bot.merge_account"
     if kind == "pr.merge" and params.get("method", "merge") not in MERGE_METHODS:
         return f"method must be one of {list(MERGE_METHODS)}"
     if kind == "actions.dispatch":
@@ -190,6 +202,87 @@ def exec_ready(transport, params: dict, config: dict):
     return {"readback": {"draft": False, "head": params["sha"]}}, None
 
 
+def exec_review(transport, params: dict, config: dict):
+    """Publish one submitted, exact-head review; never an issue comment or pending draft.
+
+    Scope locks and author-instance independence remain the operator's responsibility.
+    A failed/ambiguous write is not retried: a subsequent attempt must read reviews first.
+    """
+    account = str((config.get("multi_bot") or {}).get("merge_account") or "").strip()
+    if not account:
+        return None, "pr.review requires the configured multi_bot.merge_account"
+    user, error = transport.get("user")
+    if error or not isinstance(user, dict) or user.get("login") != account:
+        return None, f"identity is not the configured review account {account!r}"
+    repo, number, sha = params["repo"], int(params["number"]), params["sha"]
+    endpoint = f"repos/{repo}/pulls/{number}"
+
+    def current_pull():
+        pull, error = _pull(transport, repo, number)
+        if error:
+            return {}, error
+        head = pull.get("head")
+        if pull.get("state") != "open" or not isinstance(head, dict) or head.get("sha") != sha:
+            return {}, "pull is not open at the reviewed sha; nothing written"
+        if params["event"] != "COMMENT":
+            if pull.get("draft") is not False:
+                return {}, "only COMMENT reviews are allowed on drafts or an unknown draft state"
+            author = pull.get("user")
+            if not isinstance(author, dict) or not isinstance(author.get("login"), str) or not author["login"].strip():
+                return {}, "pull request author unreadable; nothing written"
+            if author["login"].casefold() == account.casefold():
+                return {}, "cannot approve or request changes on the account's own pull request"
+        return pull, None
+
+    _, error = current_pull()
+    if error:
+        return None, error
+    reviews, error = transport.get_paginated(f"{endpoint}/reviews?per_page=100")
+    if error or not isinstance(reviews, list):
+        return None, f"existing reviews unreadable: {error or 'no list'}"
+    for review in reviews:
+        if (
+            not isinstance(review, dict)
+            or not isinstance(review.get("user"), dict)
+            or not isinstance(review["user"].get("login"), str)
+            or not review["user"]["login"].strip()
+            or not isinstance(review.get("commit_id"), str)
+            or not SHA.fullmatch(review["commit_id"])
+        ):
+            return None, "existing review record unreadable; nothing written"
+        if review["user"]["login"].casefold() == account.casefold() and review["commit_id"] == sha:
+            return None, f"review already exists for this account/head (id={review.get('id')}); nothing written"
+    # Pagination may take time; bind the write to a fresh head again afterwards.
+    _, error = current_pull()
+    if error:
+        return None, error
+    body = {"commit_id": sha, "event": params["event"], "body": params["body"]}
+    created, error = transport.request("POST", f"{endpoint}/reviews", body)
+    if error:
+        return None, f"review write unconfirmed: {error}; read existing reviews before any retry"
+    review_id = created.get("id") if isinstance(created, dict) else None
+    if type(review_id) is not int or review_id < 1:
+        return None, "review write unconfirmed (no review id); read existing reviews before any retry"
+    after, error = transport.get(f"{endpoint}/reviews/{review_id}")
+    if (
+        error
+        or not isinstance(after, dict)
+        or type(after.get("id")) is not int
+        or after.get("id") != review_id
+        or after.get("commit_id") != sha
+        or after.get("state") != REVIEW_STATES[params["event"]]
+        or after.get("body") != params["body"]
+        or not isinstance(after.get("user"), dict)
+        or after["user"].get("login") != account
+    ):
+        return None, f"review id={review_id} created but readback unconfirmed; do not blindly retry"
+    latest, error = _pull(transport, repo, number)
+    head = latest.get("head")
+    if error or latest.get("state") != "open" or not isinstance(head, dict) or head.get("sha") != sha:
+        return None, f"review id={review_id} confirmed but target changed; re-review current head, do not retry"
+    return {"readback": {"review_id": review_id, "head": sha, "state": after["state"], "account": account}}, None
+
+
 def exec_dispatch(transport, params: dict, config: dict):
     repo, workflow = params["repo"], params["workflow"]
     data, error = transport.get(f"repos/{repo}/actions/workflows/{workflow}")
@@ -277,6 +370,7 @@ def exec_push_branch(transport, params: dict, config: dict):
 EXECUTORS: dict[str, Any] = {
     "pr.merge": exec_merge,
     "pr.ready": exec_ready,
+    "pr.review": exec_review,
     "actions.dispatch": exec_dispatch,
     "actions.rerun": exec_rerun,
     "pr.create": exec_create_pr,