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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ Format: [Keep a Changelog](https://keepachangelog.com/de/1.1.0/), [Semantic Vers

### Hinzugefügt / Added

- Opt-in-Aktion `pr.close` für nachweislich integrierte Heads: explizite Repository-Liste,
eigenes Policy-Opt-in, exakte Head-/Default-SHAs, Kontoprüfung und unabhängiges Rücklesen.
Keine Branch-Löschung, keine pauschalen Schließungen. / Typed opt-in closure of heads
proven contained in the pinned default branch, with explicit repository scope and readback.
- 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`.
Expand Down
18 changes: 16 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,34 @@ not a silent override. `permissions.enabled` must stay inside the ceiling.
| `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 |
| `pr.close` | disabled | closes a PR only after its exact head is proven contained in the pinned current default branch; requires an explicit repository allowlist, configured account and closed-state readback |
| `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 seven typed actions sit in the default ceiling but are never enabled by default. Each one also
The eight 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`, `policy.publish_reviews`); a permission alone opens nothing. One action at a time:
`policy.open_fix_prs`, `policy.publish_reviews`, `policy.close_integrated_prs`); 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>
```

Closing integrated PRs additionally requires `typed_actions.close_repositories`, an explicit list
of exact `owner/repo` names. Enable `pr.close` and `multi_bot.policy.close_integrated_prs`, set
`multi_bot.merge_account`, then preview:
`act pr.close --params '{"repo":"o/r","number":7,"sha":"<40-hex-head>","integration_sha":"<40-hex-current-default>","reason":"integrated"}'`.
Apply only the inspected `--expect-plan`. Drafts and stacked PRs are eligible only when their
head is proven contained in that default-branch commit. The action never deletes a branch,
posts a comment or closes an unintegrated head. Repository/account/head/default branch are
rechecked immediately before PATCH; PR state and head are read again afterwards. GitHub's
[update endpoint](https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request) has no atomic
expected-head condition: a race detected after writing is an unconfirmed failure, not success.
Do not blindly retry an ambiguous write; inspect the current PR first. Scope locks and an
independent review of the integration proof remain operator responsibilities.

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"}'`
Expand Down
19 changes: 17 additions & 2 deletions README_de.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,36 @@ stille Überschreibung. `permissions.enabled` muss innerhalb der Obergrenze blei
| `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 |
| `pr.close` | aus | schließt nur einen PR, dessen exakter Head im gebundenen aktuellen Default-Branch nachweislich enthalten ist; braucht explizite Repository-Liste, konfiguriertes Konto und Rücklesen des geschlossenen Zustands |
| `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 sieben getypten Aktionen stehen in der Standard-Obergrenze, sind aber nie standardmäßig aktiv.
Die acht 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`, `policy.publish_reviews`); eine Berechtigung allein öffnet nichts.
`policy.trigger_missing_ci`, `policy.open_fix_prs`, `policy.publish_reviews`, `policy.close_integrated_prs`); 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>
```

Das Schließen integrierter PRs braucht zusätzlich `typed_actions.close_repositories`, eine Liste
exakter Namen im Format `owner/repo`. `pr.close` und `multi_bot.policy.close_integrated_prs`
aktivieren, `multi_bot.merge_account` setzen und zunächst die Vorschau ausführen:
`act pr.close --params '{"repo":"o/r","number":7,"sha":"<40-hex-head>","integration_sha":"<40-hex-aktueller-default>","reason":"integrated"}'`.
Nur den geprüften Plan mit `--expect-plan` anwenden. Auch Drafts und gestapelte PRs sind nur
dann geeignet, wenn ihr Head nachweislich in diesem Default-Branch-Commit enthalten ist.
Die Aktion löscht keine Branches, veröffentlicht keine Kommentare und schließt keine noch
nicht integrierten Heads. Konto, Repository, Head und Default-Branch werden unmittelbar vor
PATCH erneut geprüft, PR-Zustand und Head danach zurückgelesen. GitHubs
[Update-Endpunkt](https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request) bietet keine
atomare Head-Bedingung: Ein erst nach dem Schreiben erkanntes Rennen bleibt ein unbestätigter
Fehler. Unklare Schreibversuche nie blind wiederholen; zuerst den aktuellen PR lesen.
Scope-Sperren und unabhängige Prüfung des Integrationsnachweises bleiben Operator-Aufgaben.

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"}'`
Expand Down
4 changes: 4 additions & 0 deletions src/githubbot_core/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@
"pr.merge",
"pr.ready",
"pr.review",
"pr.close",
"actions.dispatch",
"actions.rerun",
"pr.create",
"git.push_branch"
],
"enabled": ["issue.add_labels", "issue.comment"]
},
"typed_actions": {
"close_repositories": []
},
"label_schema": {
"bug": {"color": "d73a4a", "description": "Something is not working", "auto_apply": true},
"feature": {"color": "0075ca", "description": "New feature or enhancement", "auto_apply": true},
Expand Down
9 changes: 8 additions & 1 deletion src/githubbot_core/multibot.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def permission_contract(config, layers):
"draft_ready_when_mature": False,
"trigger_missing_ci": False,
"publish_reviews": False,
"close_integrated_prs": False,
"open_fix_prs": False,
}
_POLICY_CHOICES = {
Expand All @@ -83,7 +84,13 @@ def policy_settings(raw):
hours = policy["draft_auto_ready_hours"]
if hours is not None and (type(hours) not in (int, float) or hours <= 0):
raise ValueError("multi_bot.policy.draft_auto_ready_hours must be null or a positive number")
for key in ("draft_ready_when_mature", "trigger_missing_ci", "publish_reviews", "open_fix_prs"):
for key in (
"draft_ready_when_mature",
"trigger_missing_ci",
"publish_reviews",
"open_fix_prs",
"close_integrated_prs",
):
if type(policy[key]) is not bool:
raise ValueError(f"multi_bot.policy.{key} must be a boolean")
return policy
Expand Down
6 changes: 6 additions & 0 deletions src/githubbot_core/prompts/MULTI-BOT.de.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ und ersetzen weder Merge-Opt-in noch Identitätsprüfung.
Prüfumfang (gelesen/nicht gelesen), Testergebnis mit Befehl, Befunde mit Datei und Zeile.
Ein Beitrag pro (PR, Head); keine Secrets, keine lokalen Pfade. Ohne diesen Schalter bleibt
das Ergebnis im Bericht.
- `close_integrated_prs` (bool, standardmäßig aus): erlaubt ausschließlich `pr.close` für
nachweislich im aktuellen Default-Branch integrierte Heads. Zusätzlich braucht die Aktion
eine explizite Liste `typed_actions.close_repositories`, ihre Berechtigung und das konfigurierte
Konto. Exakte Head-/Integrations-SHAs, Vorschau/Planbindung und frische Integrationsprüfung
sind Pflicht. Keine Branch-Löschung oder pauschalen Schließungen. Bei unklarem Schreibversuch
zuerst unabhängig zurücklesen; niemals blind wiederholen.
- `open_fix_prs` (bool): erlaubt Fix-Branches zu pushen (Core-Aktion `git.push_branch`: nur
Bot-Präfix `typed_actions.push_branch_prefix`, nie Default-Branch, nie Force) und PRs zu
öffnen (`pr.create`, Base immer der Default-Branch). Ohne diesen Schalter bleiben Fixes lokal
Expand Down
133 changes: 130 additions & 3 deletions src/githubbot_core/typed_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import re
import subprocess
from typing import Any, Optional
from urllib.parse import quote

SHA = re.compile(r"[0-9a-f]{40}")
REPO = re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+")
Expand All @@ -21,7 +22,16 @@
MERGE_METHODS = ("merge", "squash", "rebase")
DEFAULT_PUSH_PREFIX = "bot/"

KINDS = ("pr.merge", "pr.ready", "pr.review", "actions.dispatch", "actions.rerun", "pr.create", "git.push_branch")
KINDS = (
"pr.merge",
"pr.ready",
"pr.review",
"pr.close",
"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
Expand All @@ -46,6 +56,7 @@ def switch_denial(kind: str, config: dict) -> Optional[str]:
"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"],
"pr.close": policy["close_integrated_prs"],
"actions.dispatch": policy["trigger_missing_ci"],
"actions.rerun": policy["trigger_missing_ci"],
"pr.create": policy["open_fix_prs"],
Expand All @@ -65,11 +76,33 @@ 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", "pr.review", "actions.rerun", "git.push_branch")
need_sha = kind in ("pr.merge", "pr.ready", "pr.review", "pr.close", "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", "pr.review") and not str(params.get("number", "")).isdecimal():
if kind in ("pr.merge", "pr.ready", "pr.review", "pr.close") and not str(params.get("number", "")).isdecimal():
return "number must be a pull request number"
if kind == "pr.close":
if int(params["number"]) < 1:
return "number must be a positive pull request number"
if params.get("reason") != "integrated":
return "pr.close only supports reason='integrated'"
if not SHA.fullmatch(str(params.get("integration_sha", ""))):
return "integration_sha must be the exact current default-branch commit"
account = (config.get("multi_bot") or {}).get("merge_account")
if not isinstance(account, str) or not account.strip():
return "pr.close requires the configured multi_bot.merge_account"
settings = config.get("typed_actions") or {}
if not isinstance(settings, dict):
return "typed_actions must be an object"
scopes = settings.get("close_repositories")
if (
not isinstance(scopes, list)
or not scopes
or any(not isinstance(repo, str) or not REPO.fullmatch(repo) for repo in scopes)
):
return "pr.close requires explicit typed_actions.close_repositories"
if params["repo"] not in scopes:
return "repository is outside typed_actions.close_repositories"
if kind == "pr.review":
if int(params["number"]) < 1:
return "number must be a positive pull request number"
Expand Down Expand Up @@ -283,6 +316,99 @@ def current_pull():
return {"readback": {"review_id": review_id, "head": sha, "state": after["state"], "account": account}}, None


def exec_close(transport, params: dict, config: dict):
"""Close only heads proven contained in the pinned current default branch.

GitHub's close endpoint has no atomic expected-head argument. Recheck before
PATCH and detect races afterwards; ambiguous results must never be retried
blindly. Locks and independent source review remain operator responsibilities.
"""
permissions = config.get("permissions") or {}
if "pr.close" not in (permissions.get("enabled") or []) or "pr.close" not in (permissions.get("ceiling") or []):
return None, "pr.close is not enabled within the permission ceiling"
denial = switch_denial("pr.close", config) or param_denial("pr.close", params, config)
if denial:
return None, denial
account = config["multi_bot"]["merge_account"].strip()
repo, number, sha = params["repo"], int(params["number"]), params["sha"]
integrated = params["integration_sha"]

def current_target():
user, error = transport.get("user")
if error or not isinstance(user, dict) or user.get("login") != account:
return None, "identity is not the configured close account"
branch, error = _default_branch(transport, repo)
if error or not isinstance(branch, str):
return None, error or "default branch unreadable"
ref, error = transport.get(f"repos/{repo}/git/ref/heads/{quote(branch, safe='')}")
obj = ref.get("object") if isinstance(ref, dict) else None
if error or not isinstance(obj, dict) or obj.get("type") != "commit" or obj.get("sha") != integrated:
return None, "default branch moved or unreadable; nothing written"
pull, error = _pull(transport, repo, number)
head = pull.get("head")
base = pull.get("base")
base_repo = base.get("repo") if isinstance(base, dict) else None
if (
error
or pull.get("number") != number
or pull.get("state") != "open"
or pull.get("merged") is not False
or not isinstance(head, dict)
or head.get("sha") != sha
or not isinstance(base_repo, dict)
or base_repo.get("full_name") != repo
):
return None, "pull is not open and unmerged at the exact head/repository; nothing written"
return branch, None

branch, error = current_target()
if error:
return None, error
proof, error = transport.get(f"repos/{repo}/compare/{sha}...{integrated}")
if (
error
or not isinstance(proof, dict)
or proof.get("status") not in ("ahead", "identical")
or (proof.get("status") == "identical") != (sha == integrated)
or not isinstance(proof.get("base_commit"), dict)
or proof["base_commit"].get("sha") != sha
or not isinstance(proof.get("merge_base_commit"), dict)
or proof["merge_base_commit"].get("sha") != sha
):
return None, "head is not proven integrated in the pinned default branch"
latest_branch, error = current_target()
if error or latest_branch != branch:
return None, error or "default branch name changed; nothing written"
_, error = transport.request("PATCH", f"repos/{repo}/pulls/{number}", {"state": "closed"})
if error:
return None, "close write unconfirmed; independently read PR state before any retry: " + str(error)
after, error = _pull(transport, repo, number)
head = after.get("head")
base = after.get("base")
base_repo = base.get("repo") if isinstance(base, dict) else None
if (
error
or after.get("number") != number
or after.get("state") != "closed"
or after.get("merged") is not False
or not isinstance(head, dict)
or head.get("sha") != sha
or not isinstance(base_repo, dict)
or base_repo.get("full_name") != repo
):
return None, "close readback unconfirmed or head changed; do not blindly retry"
return {
"readback": {
"closed": True,
"merged": False,
"head": sha,
"integration_sha": integrated,
"reason": "integrated",
"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}")
Expand Down Expand Up @@ -371,6 +497,7 @@ def exec_push_branch(transport, params: dict, config: dict):
"pr.merge": exec_merge,
"pr.ready": exec_ready,
"pr.review": exec_review,
"pr.close": exec_close,
"actions.dispatch": exec_dispatch,
"actions.rerun": exec_rerun,
"pr.create": exec_create_pr,
Expand Down
3 changes: 3 additions & 0 deletions tests/test_multibot_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
def env_for(tmp_path, code=23):
package = tmp_path / "githubbot_core"
package.mkdir()
# A namespace package would lose to an installed regular githubbot_core and
# accidentally exercise the real CLI instead of this isolated no-provider stub.
(package / "__init__.py").write_text("", encoding="ascii")
(package / "__main__.py").write_text(
"import sys\nprint('[TEST] no provider started')\nsys.exit(" + str(code) + ")\n", encoding="ascii"
)
Expand Down
Loading
Loading