From 539b0d030076d8492d3aa2b9de9e2db6d9867281 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 17 Oct 2026 18:54:00 +0200 Subject: [PATCH 1/2] feat(multibot): make review independence, draft readiness, CI trigger and publishing switchable multi_bot.policy carries validated switches into the run request and the packaged policy documents them. Public defaults stay conservative (model_class independence, unknown author holds, no draft readying, no CI trigger, no publishing, no fix PRs); an instance opts in per key. Unknown keys or wrong types fail closed before launch. The switches loosen no hold, lock, exclude_writes or permission-ceiling rule. Two tests in test_multibot_resources.py already fail on this branch before the change under Windows checkouts (line endings, path comparison); not touched here. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_013G793TMH12ptWX1f6PYcZU --- src/githubbot_core/multibot.example.json | 11 +++- src/githubbot_core/multibot.py | 43 ++++++++++++++ src/githubbot_core/prompts/MULTI-BOT.de.md | 65 ++++++++++++++++++---- tests/test_multibot_resources.py | 53 ++++++++++++++++++ 4 files changed, 161 insertions(+), 11 deletions(-) diff --git a/src/githubbot_core/multibot.example.json b/src/githubbot_core/multibot.example.json index 90e0cfb..29673c0 100644 --- a/src/githubbot_core/multibot.example.json +++ b/src/githubbot_core/multibot.example.json @@ -8,6 +8,15 @@ "model": "", "effort": "", "root": "", - "instance_prompt": "" + "instance_prompt": "", + "policy": { + "review_independence": "model_class", + "unknown_author": "hold", + "draft_auto_ready_hours": null, + "draft_ready_when_mature": false, + "trigger_missing_ci": false, + "publish_reviews": false, + "open_fix_prs": false + } } } diff --git a/src/githubbot_core/multibot.py b/src/githubbot_core/multibot.py index 02f230e..4d3199c 100644 --- a/src/githubbot_core/multibot.py +++ b/src/githubbot_core/multibot.py @@ -47,6 +47,44 @@ def permission_contract(config, layers): return {"permissions": permissions, "config_layers": [str(Path(path).expanduser().resolve()) for path in layers]} +# Public defaults stay conservative; an instance opts in per key via multi_bot.policy. +POLICY_DEFAULTS = { + "review_independence": "model_class", # provider | model_class | instance + "unknown_author": "hold", # hold | allow + "draft_auto_ready_hours": None, # hours an unchanged head must age before a draft may be readied + "draft_ready_when_mature": False, + "trigger_missing_ci": False, + "publish_reviews": False, + "open_fix_prs": False, +} +_POLICY_CHOICES = { + "review_independence": ("provider", "model_class", "instance"), + "unknown_author": ("hold", "allow"), +} + + +def policy_settings(raw): + """Validate multi_bot.policy; unknown keys or wrong types fail closed instead of being ignored.""" + if raw is None: + raw = {} + if not isinstance(raw, dict): + raise ValueError("multi_bot.policy must be an object") + unknown = sorted(set(raw) - set(POLICY_DEFAULTS)) + if unknown: + raise ValueError(f"unknown multi_bot.policy keys: {unknown}") + policy = {**POLICY_DEFAULTS, **raw} + for key, allowed in _POLICY_CHOICES.items(): + if policy[key] not in allowed: + raise ValueError(f"multi_bot.policy.{key} must be one of {list(allowed)}") + 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"): + if type(policy[key]) is not bool: + raise ValueError(f"multi_bot.policy.{key} must be a boolean") + return policy + + def choose_mode(value="", *, ask=input, chooser=None): value = value.strip().lower() if value in ("1", "2", "3"): @@ -108,6 +146,11 @@ def main(argv=None, *, launch=None): request = f"Start MULTI-BOT in mode {mode}. Read the complete packaged role prompt." request += " Configured merge account: " + (str(settings.get("merge_account", "")) or "none") + "." request += " Merge enabled: " + str(settings.get("merge_enabled") is True).lower() + "." + request += ( + " Policy switches (authoritative, see 'Einstellbare Regeln' in the packaged policy): " + + json.dumps(policy_settings(settings.get("policy")), sort_keys=True) + + "." + ) if instance: request += f" Read the explicitly configured local instance prompt {instance}." if args.request: diff --git a/src/githubbot_core/prompts/MULTI-BOT.de.md b/src/githubbot_core/prompts/MULTI-BOT.de.md index c880197..73d332f 100644 --- a/src/githubbot_core/prompts/MULTI-BOT.de.md +++ b/src/githubbot_core/prompts/MULTI-BOT.de.md @@ -28,9 +28,12 @@ Modus. Keine Unteragenten starten oder zusätzliche kostenpflichtige Dienste bes Projektregeln und der privaten Instanzkonfiguration gelesen. Datum allein hebt nichts auf. 4. `config/orgs.json` und `exclude_writes`/`global_exclude_writes` beachten. Org-Profile (`owner/.github`) nur als eigenes Care-PR mit unabhängiger Prüfung; keine direkten - Bot-Schreibfeatures dort. Keine Draft- oder gestapelten PRs mergen. Keine Force-Pushes, - Admin-/Bypass-Merges, Publikation, Tags, Releases oder Sichtbarkeitsänderungen. Keine - Dritt-Nachrichten, Einladungen, Follow-Aktionen oder Kommentare ohne explizite Autorisierung. + Bot-Schreibfeatures dort. Keine gestapelten PRs mergen; Drafts nur nach den Schaltern + `draft_auto_ready_hours`/`draft_ready_when_mature` (Abschnitt „Einstellbare Regeln“). Keine + Force-Pushes, Admin-/Bypass-Merges, Publikation, Tags, Releases oder Sichtbarkeitsänderungen. + Keine Dritt-Nachrichten, Einladungen, Follow-Aktionen oder Kommentare ohne explizite + Autorisierung; für Reviews und Triage an PRs/Issues der konfigurierten Owner ist das der + Schalter `publish_reviews`. 5. Echte Nutzerentscheidungen mit dem vorhandenen `ticket_writer.py` als Ticket erfassen; nicht in alte `USER-DECIDE-NEEDED.md`-Wrapper schreiben. Fremde Arbeit nie mitcommitten. @@ -113,19 +116,20 @@ Prüfe offene PRs nacheinander, exakt einen PR pro Arbeitseinheit. Eine abgeschl Runde schreibt einen Bericht; bei Leerlauf 60 Sekunden warten, keine heiße Schleife. Jede neue Runde prüft STOP und Kontingent; fehlende Telemetrie sperrt neue Modellstarts. -Die Zwei-Modell-Regel ist verbindlich: Autor anhand Commit-Trailern (`Co-authored-by`, -`Generated-with`), PR-Text und Branch-Präfix ermitteln. Widersprüchliche oder fehlende -Autorinformation bedeutet HOLD. Reviewer ist ein anderer Anbieter oder eine andere -Modellklasse; ein neuer Prozess desselben Modells genügt nicht. Das unabhängige Modell +Die Unabhängigkeitsregel ist verbindlich; ihre Strenge setzt der Schalter `review_independence` +(Abschnitt „Einstellbare Regeln“, Standard `model_class`). Autor anhand Commit-Trailern +(`Co-authored-by`, `Generated-with`, Sitzungslinks), PR-Text und Branch-Präfix ermitteln; +fehlende oder widersprüchliche Autorinformation folgt `unknown_author`. Der unabhängige Reviewer liest den vollständigen Diff im isolierten Worktree und prüft Verhalten, Tests und CI. -Autor-Modell repariert und öffnet PRs; Reviewer darf erst nach belegter Prüfung mergen. +Der Autor repariert und öffnet PRs; der Reviewer darf erst nach belegter Prüfung mergen. `core/review_gate.py` und `docs/REVIEW_GATE_PILOT.md` nutzen, wenn das Repo den Pilot verlangt; Receipt muss zum exakten aktuellen `headRefOid` gehören. Niemals eine fehlende Receipt ersetzen. Vor Merge: `gh pr view --json headRefOid,baseRefName,headRefName,isDraft,mergeable, mergeStateStatus,statusCheckRollup,reviewDecision,commits` neu lesen. Default-Base, keine Stapelung, vollständig grüne erforderliche CI, keine offenen Konflikte oder -Review-Blocker; bei unbekanntem Checkstatus HOLD. Nach jeder Änderung des Heads erneut prüfen. +Review-Blocker; bei unbekanntem Checkstatus HOLD (mit `trigger_missing_ci` darf der Bot +fehlende Läufe zuerst anstoßen). Nach jeder Änderung des Heads erneut prüfen. Merge ausschließlich mit `--match-head-commit ` und ohne `--admin` oder `--auto`. Wo Branch-Schutz zusätzliche Approvals verlangt, diesen normal erfüllen. @@ -162,6 +166,47 @@ full-after-care-/surface-after-care-Skills passend; keine Versionsbump-/Release- Die neuen Rollenblueprints verweisen auf ihre expliziten Scopes; Legacy-Blueprints bleiben begrenzt. Deren API- Registrierung ist getrennt von diesem Starter und bleibt bis zum API-Readback unbestätigt. +## Einstellbare Regeln (`multi_bot.policy`) + +Der Startauftrag nennt „Policy switches (authoritative …)“ als JSON. Diese Werte, nicht +Gewohnheit oder ältere Annahmen, bestimmen die folgenden Regeln. Fehlt eine Angabe, gilt der +konservative Standard. Sie lockern keine Hold-, Lock-, `exclude_writes`- oder Permission-Grenze +und ersetzen weder Merge-Opt-in noch Identitätsprüfung. + +- `review_independence`: `provider` = Reviewer ist ein anderer Anbieter als der Autor. + `model_class` (Standard) = anderer Anbieter oder andere Modellklasse; ein neuer Prozess + desselben Modells genügt nicht. `instance` = Reviewer ist eine andere Instanz als der Autor, + also eine andere Sitzung mit eigenem Kontext; gleiches Modell und gleicher Anbieter sind + zulässig (zwei Sitzungen desselben Modells). Nicht zulässig ist, dass dieselbe Sitzung + eigene Arbeit prüft oder mergt. Eigenarbeit erkennst du an Sitzungs-Trailern/-Links, Branch, + PR-Text und deinem eigenen Protokoll. +- `unknown_author`: `hold` (Standard) = fehlende oder widersprüchliche Autorinformation ist + HOLD. `allow` = fehlende Autorinformation allein ist kein HOLD, solange nichts auf + Eigenarbeit dieser Sitzung hinweist; Hinweise auf Eigenarbeit oder echte Widersprüche + bleiben HOLD. +- `draft_auto_ready_hours` (Zahl > 0 oder `null`, Standard `null`): Ein Draft, dessen Head seit + mindestens so vielen Stunden unverändert ist, darf der Bot mit `gh pr ready` zum regulären PR + machen. `draft_ready_when_mature` (bool): das gilt auch früher, wenn der Draft sichtbar + ausgereift ist: alle erforderlichen Checks am Head grün, keine offenen Review-Threads oder + Änderungswünsche, kein WIP-/Draft-/Hold-Hinweis in Titel, Text oder Kommentaren, Beschreibung + nennt Problem und Tests, keine Konflikte. Danach gilt der normale Ablauf am dann frisch + gelesenen Head (unabhängige Prüfung, alle Merge-Gates). Gestapelte Drafts (Base ist nicht der + Default-Branch) und Drafts mit aktivem Hold oder Lock bleiben unberührt. +- `trigger_missing_ci` (bool): Hat der exakte Head keine Check-Läufe, darf der Bot sie + anstoßen: `gh workflow run --ref ` für Workflows mit `workflow_dispatch` + oder `gh run rerun` eines früheren Laufs desselben PRs. Verboten bleiben leere Commits, + Pushs auf fremde Branches, Änderungen an Workflow-Dateien, das Freigeben wartender + Fork-Läufe und jede Umgehung des Branch-Schutzes. Entsteht kein Lauf (kein passender + Workflow, Actions aus, Abrechnung), ist das HOLD mit Ticket. Ein Lauf ist erst nach + Abschluss und grün am gleichen Head ein Beleg; in der nächsten Runde erneut lesen. +- `publish_reviews` (bool): erlaubt Reviews, Review-Kommentare, Triage-Kommentare und Labels an + PRs und Issues der konfigurierten Owner. Inhalt: exakter Head, tatsächliches Modell, + 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. +- `open_fix_prs` (bool): erlaubt Fix-Branches zu pushen (nie Default-Branch, nie Force) und PRs + zu öffnen. Ohne diesen Schalter bleiben Fixes lokal vorbereitet und werden gemeldet. + ## Paket- und Instanzkonfiguration Diese neutrale Rolle wird mit `githubbot-core multi-bot` gestartet. Der vorhandene @@ -172,6 +217,6 @@ Care-/Probe-Helfer benutzen den ausdrücklich gewählten `--root`, standardmäß Verzeichnis. Ihre Dateikonventionen (`config/orgs.json`, `config/repo_registry.json`, Hostlogs) müssen als Instanzdaten vorhanden sein; bei fehlendem Bestand sperren sie die Auswahl. Die Core-Konfiguration kommt über wiederholbares `--config PFAD`. `multi_bot` enthält -Root, optionalen Instanzprompt, Merge-Konto, Merge-Opt-in und optional trusted_automation. +Root, optionalen Instanzprompt, Merge-Konto, Merge-Opt-in, `policy`-Schalter und optional trusted_automation. Die berechnete Core-Permission-Decke bleibt bindend; ein Rollenprompt erweitert sie nicht. Ohne Merge-Aktionsfähigkeit/Permission keine Merge-Mutation, auch bei aktivem Merge-Opt-in. diff --git a/tests/test_multibot_resources.py b/tests/test_multibot_resources.py index 4efc294..9ff54f3 100644 --- a/tests/test_multibot_resources.py +++ b/tests/test_multibot_resources.py @@ -189,3 +189,56 @@ def test_explicit_layer_order_is_preserved(tmp_path): ) assert contract["config_layers"] == [str(first.resolve()), str(second.resolve())] assert contract["permissions"]["enabled"] == [] + + +def _launch_with_policy(tmp_path, policy): + config = tmp_path / "policy.json" + config.write_text(json.dumps({"multi_bot": {"root": str(tmp_path), "policy": policy}})) + launch = Mock(return_value=0) + code = multibot.main(["--config", str(config), "--mode", "1", "--no-interactive"], launch=launch) + return code, launch + + +def test_policy_switches_default_to_conservative_and_reach_the_request(tmp_path): + launch = Mock(return_value=0) + assert multibot.main(["--root", str(tmp_path), "--mode", "1", "--no-interactive"], launch=launch) == 0 + request = launch.call_args.kwargs["request"] + assert json.dumps(multibot.policy_settings(None), sort_keys=True) in request + assert '"review_independence": "model_class"' in request and '"publish_reviews": false' in request + + +def test_policy_switches_can_be_opted_in_per_instance(tmp_path): + code, launch = _launch_with_policy( + tmp_path, + { + "review_independence": "instance", + "unknown_author": "allow", + "draft_auto_ready_hours": 24, + "trigger_missing_ci": True, + "publish_reviews": True, + }, + ) + assert code == 0 + request = launch.call_args.kwargs["request"] + assert '"review_independence": "instance"' in request and '"draft_auto_ready_hours": 24' in request + assert '"open_fix_prs": false' in request # unlisted keys keep the conservative default + + +def test_invalid_policy_switches_fail_closed_before_any_launch(tmp_path, capsys): + for bad in ( + {"review_independence": "whatever"}, + {"unknown_key": True}, + {"draft_auto_ready_hours": 0}, + {"draft_auto_ready_hours": "24"}, + {"publish_reviews": "yes"}, + {"unknown_author": None}, + ): + code, launch = _launch_with_policy(tmp_path, bad) + assert code != 0 and not launch.called, bad + assert "[HOLD]" in capsys.readouterr().err + + +def test_packaged_policy_documents_every_switch(): + prompt = resources.files("githubbot_core").joinpath("prompts/MULTI-BOT.de.md").read_text(encoding="utf-8") + for key in multibot.POLICY_DEFAULTS: + assert f"`{key}`" in prompt, key From 93864c8f8ed0b64fa90718fbaec74d2e9cfca14c Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 17 Oct 2026 22:54:55 +0200 Subject: [PATCH 2/2] feat(actions): typed merge/ready/CI/PR/push actions gated by multi_bot switches api.request can only be scoped by endpoint prefix; "repos//" would allow every write in every repository, and draft-ready needs GraphQL. Add six narrow actions instead: pr.merge (fresh read, exact sha, default base, not a draft, mergeable_state clean, configured merge account, sha-bound PUT, merged readback), pr.ready (GraphQL markPullRequestReadyForReview at the exact draft head), actions.dispatch (active workflow only), actions.rerun (completed, non-fork run of the exact head), pr.create (bot-prefixed branch onto the default branch) and git.push_branch (no force, bot prefix, never the default branch, ls-remote readback). They are in the default ceiling but never enabled by default, and each also needs its multi_bot switch, so a permission alone opens nothing. The MULTI-BOT run contract now reports effective_enabled. New `act` command for one action through the existing preview/apply/expect-plan contract. The policy prompt routes merges, ready, CI triggers and fix PRs through these actions and treats a job waiting for a (self-hosted) runner as HOLD, never green. Prompt and JSON resources are pinned to LF so the bound policy hash no longer depends on core.autocrlf. Ticket: T-20261017-353754430 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01C4ZiDnWpjFvgsu8AjdPgbN --- .gitattributes | 2 + CHANGELOG.md | 12 + README.md | 15 ++ README_de.md | 16 ++ src/githubbot_core/actions.py | 18 +- src/githubbot_core/cli.py | 18 ++ src/githubbot_core/default_config.json | 12 +- src/githubbot_core/multibot.py | 10 +- src/githubbot_core/prompts/MULTI-BOT.de.md | 27 ++- src/githubbot_core/typed_actions.py | 260 +++++++++++++++++++++ tests/test_multibot_resources.py | 2 +- tests/test_typed_actions.py | 252 ++++++++++++++++++++ 12 files changed, 629 insertions(+), 15 deletions(-) create mode 100644 src/githubbot_core/typed_actions.py create mode 100644 tests/test_typed_actions.py diff --git a/.gitattributes b/.gitattributes index eb378b0..ec19131 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,5 @@ src/githubbot_core/starters/posix/*.sh text eol=lf src/githubbot_core/starters/windows/*.bat text eol=crlf src/githubbot_core/starters/windows/providers/*.bat text eol=crlf +src/githubbot_core/prompts/*.md text eol=lf +src/githubbot_core/*.json text eol=lf diff --git a/CHANGELOG.md b/CHANGELOG.md index 827ac84..7a70a45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ Format: [Keep a Changelog](https://keepachangelog.com/de/1.1.0/), [Semantic Vers ## [Unreleased] +### Hinzugefügt / Added + +- 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 + `multi_bot`-Schalter. Neues Kommando `act` (Vorschau, dann `--apply --expect-plan`). Der + MULTI-BOT-Laufvertrag nennt `effective_enabled`. / Typed write actions with fresh re-read, + exact head binding and readback; in the default ceiling, never enabled by default, each gated + by its `multi_bot` switch. New `act` command; the run contract reports `effective_enabled`. +- Prompt und Rollen-JSON mit festem LF (`.gitattributes`), damit der Policy-Hash nicht vom + Checkout abhängt / prompt and role JSON pinned to LF so the policy hash is checkout-independent. + ### Geändert / Changed - Gesamter Baum mit `ruff format` formatiert (keine Codeänderung, AST identisch); CI prüft jetzt diff --git a/README.md b/README.md index 46cca68..8355d48 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,21 @@ not a silent override. `permissions.enabled` must stay inside the ceiling. | `issue.add_labels` | enabled | `POST /repos/{repo}/issues/{n}/labels` | | `issue.comment` | enabled | `POST /repos/{repo}/issues/{n}/comments` | | `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 | +| `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 :refs/heads/` 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 +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: + +```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 +``` ```json {"permissions": {"enabled": ["issue.add_labels", "issue.comment", "api.request"], diff --git a/README_de.md b/README_de.md index 79457d2..947c7d9 100644 --- a/README_de.md +++ b/README_de.md @@ -112,6 +112,22 @@ stille Überschreibung. `permissions.enabled` muss innerhalb der Obergrenze blei | `issue.add_labels` | aktiv | `POST /repos/{repo}/issues/{n}/labels` | | `issue.comment` | aktiv | `POST /repos/{repo}/issues/{n}/comments` | | `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 | +| `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 :refs/heads/` 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. +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. +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 +``` ```json {"permissions": {"enabled": ["issue.add_labels", "issue.comment", "api.request"], diff --git a/src/githubbot_core/actions.py b/src/githubbot_core/actions.py index 90eb644..284dcd1 100644 --- a/src/githubbot_core/actions.py +++ b/src/githubbot_core/actions.py @@ -17,6 +17,8 @@ from pathlib import Path from typing import Any, Optional +from . import typed_actions + WRITE_METHODS = {"POST", "PUT", "PATCH", "DELETE"} @@ -50,6 +52,11 @@ def denial_reason(action: Action, config: dict) -> Optional[str]: return f"{action.kind} is not enabled in permissions.enabled" if action.kind not in EXECUTORS: return f"{action.kind} has no executor" + reason = typed_actions.switch_denial(action.kind, config) or typed_actions.param_denial( + action.kind, action.params, config + ) + if reason: + return reason if action.kind == "api.request": method = str(action.params.get("method", "")).upper() endpoint = str(action.params.get("endpoint", "")).lstrip("/") @@ -69,19 +76,19 @@ def preview(actions: list[Action], config: dict) -> dict: return {"mode": "preview", "plan_sha256": plan_digest(actions), "actions": items, "writes": 0} -def _exec_add_labels(transport, params: dict): +def _exec_add_labels(transport, params: dict, config: dict): return transport.request( "POST", f"repos/{params['repo']}/issues/{int(params['number'])}/labels", {"labels": list(params["labels"])} ) -def _exec_comment(transport, params: dict): +def _exec_comment(transport, params: dict, config: dict): return transport.request( "POST", f"repos/{params['repo']}/issues/{int(params['number'])}/comments", {"body": str(params["body"])} ) -def _exec_api_request(transport, params: dict): +def _exec_api_request(transport, params: dict, config: dict): return transport.request(str(params["method"]).upper(), str(params["endpoint"]).lstrip("/"), params.get("body")) @@ -89,6 +96,7 @@ def _exec_api_request(transport, params: dict): "issue.add_labels": _exec_add_labels, "issue.comment": _exec_comment, "api.request": _exec_api_request, + **typed_actions.EXECUTORS, } @@ -139,9 +147,11 @@ def apply( elif action.action_id in applied_before: record.update(status="skipped_duplicate", detail="already applied according to the receipt store") else: - _, error = EXECUTORS[action.kind](transport, action.params) + data, error = EXECUTORS[action.kind](transport, action.params, config) writes += 1 record.update(status="failed" if error else "applied", detail=error or "") + if not error and isinstance(data, dict) and "readback" in data: + record["readback"] = data["readback"] receipts.append(record) if receipt_store is not None: receipt_store.parent.mkdir(parents=True, exist_ok=True) diff --git a/src/githubbot_core/cli.py b/src/githubbot_core/cli.py index 8fea537..df15d12 100644 --- a/src/githubbot_core/cli.py +++ b/src/githubbot_core/cli.py @@ -316,6 +316,15 @@ def cmd_run(args, config, transport) -> int: return _plan_output(args, "run", plan, config, transport, {"plugin": args.plugin}) +def cmd_act(args, config, transport) -> int: + """One explicit write action through the same preview/apply contract as every plan.""" + params = json.loads(args.params) + if not isinstance(params, dict): + raise ValueError("--params must be a JSON object") + plan = [actions.Action(args.kind, params, reason=args.reason or args.kind, source="act")] + return _plan_output(args, "act", plan, config, transport, {"kind": args.kind}) + + def cmd_gate_scaffold(args, config, transport) -> int: names = gate_names(args.namespace) package = resources.files("githubbot_core") @@ -420,6 +429,15 @@ def add_apply_flags(p): add_apply_flags(p) p.set_defaults(func=cmd_run) + p = sub.add_parser("act", help="preview or apply one typed write action (pr.merge, pr.ready, ...)") + p.add_argument("kind", help="action kind, for example pr.merge") + p.add_argument( + "--params", required=True, help='JSON object, for example {"repo": "o/r", "number": 1, "sha": "..."}' + ) + p.add_argument("--reason", default="") + add_apply_flags(p) + p.set_defaults(func=cmd_act) + p = sub.add_parser("gate-scaffold", help="copy the review-gate workflow and verifier into a repository") p.add_argument("--dest", required=True, help="root of the target repository checkout") p.add_argument("--namespace", default=DEFAULT_NAMESPACE) diff --git a/src/githubbot_core/default_config.json b/src/githubbot_core/default_config.json index 748c7b2..a354ca2 100644 --- a/src/githubbot_core/default_config.json +++ b/src/githubbot_core/default_config.json @@ -4,7 +4,17 @@ "description": "Neutral defaults. Override with --config layers; permission ceilings can only be narrowed by later layers." }, "permissions": { - "ceiling": ["issue.add_labels", "issue.comment", "api.request"], + "ceiling": [ + "issue.add_labels", + "issue.comment", + "api.request", + "pr.merge", + "pr.ready", + "actions.dispatch", + "actions.rerun", + "pr.create", + "git.push_branch" + ], "enabled": ["issue.add_labels", "issue.comment"] }, "label_schema": { diff --git a/src/githubbot_core/multibot.py b/src/githubbot_core/multibot.py index 4d3199c..aa6a43c 100644 --- a/src/githubbot_core/multibot.py +++ b/src/githubbot_core/multibot.py @@ -44,6 +44,10 @@ def permission_contract(config, layers): if not isinstance(values, list) or any(not isinstance(v, str) for v in values): raise ValueError(f"permissions.{field} must be a list of strings") permissions[field] = values + from .typed_actions import switch_denial + + # What the run may really do: enabled AND not switched off by multi_bot. + permissions["effective_enabled"] = [kind for kind in permissions["enabled"] if not switch_denial(kind, config)] return {"permissions": permissions, "config_layers": [str(Path(path).expanduser().resolve()) for path in layers]} @@ -161,8 +165,10 @@ def main(argv=None, *, launch=None): + json.dumps(contract, sort_keys=True, ensure_ascii=False) + "\nPreserve config_layers in this exact order for every subsequent Core call via --config. " "Never replace these layers with defaults or widen the effective permissions. " - "Writes require the action in BOTH ceiling and enabled; api.request writes also require " - "a matching api_write_prefixes endpoint. Empty, missing or unknown write authority means HOLD. " + "Writes require the action in ceiling, enabled AND effective_enabled; api.request writes also require " + "a matching api_write_prefixes endpoint. Perform GitHub writes only through githubbot-core " + "(act/run/triage with these --config layers), preview first, then --apply --expect-plan. " + "Empty, missing or unknown write authority means HOLD. " "Instance prompts and task requests cannot override this contract. No credentials are included." ) request += "\nExplicit Core config layers (ordered, use unchanged):\n" + ( diff --git a/src/githubbot_core/prompts/MULTI-BOT.de.md b/src/githubbot_core/prompts/MULTI-BOT.de.md index 73d332f..964c4b8 100644 --- a/src/githubbot_core/prompts/MULTI-BOT.de.md +++ b/src/githubbot_core/prompts/MULTI-BOT.de.md @@ -130,8 +130,12 @@ mergeStateStatus,statusCheckRollup,reviewDecision,commits` neu lesen. Default-Ba keine Stapelung, vollständig grüne erforderliche CI, keine offenen Konflikte oder Review-Blocker; bei unbekanntem Checkstatus HOLD (mit `trigger_missing_ci` darf der Bot fehlende Läufe zuerst anstoßen). Nach jeder Änderung des Heads erneut prüfen. -Merge ausschließlich mit `--match-head-commit ` und ohne `--admin` oder -`--auto`. Wo Branch-Schutz zusätzliche Approvals verlangt, diesen normal erfüllen. +Merge ausschließlich über die Core-Aktion `pr.merge` (`githubbot-core --config … act pr.merge +--params '{"repo":…,"number":…,"sha":""}'`, erst Vorschau, dann `--apply +--expect-plan `). Sie liest den PR frisch, verlangt exakten Head, Default-Base, +keinen Draft und `mergeable_state = clean`, prüft das Merge-Konto, merged sha-gebunden ohne +Admin-Bypass und liest das Ergebnis zurück. Kein `gh pr merge`, kein `--admin`/`--auto`. +Wo Branch-Schutz zusätzliche Approvals verlangt, diesen normal erfüllen. Merges sind standardmäßig deaktiviert. Nur die ausdrücklich autorisierte Instanzkonfiguration darf sie aktivieren (`multi_bot.merge_enabled = true` in `config/orgs.json` oder einem @@ -185,7 +189,7 @@ und ersetzen weder Merge-Opt-in noch Identitätsprüfung. Eigenarbeit dieser Sitzung hinweist; Hinweise auf Eigenarbeit oder echte Widersprüche bleiben HOLD. - `draft_auto_ready_hours` (Zahl > 0 oder `null`, Standard `null`): Ein Draft, dessen Head seit - mindestens so vielen Stunden unverändert ist, darf der Bot mit `gh pr ready` zum regulären PR + mindestens so vielen Stunden unverändert ist, darf der Bot mit der Core-Aktion `pr.ready` zum regulären PR machen. `draft_ready_when_mature` (bool): das gilt auch früher, wenn der Draft sichtbar ausgereift ist: alle erforderlichen Checks am Head grün, keine offenen Review-Threads oder Änderungswünsche, kein WIP-/Draft-/Hold-Hinweis in Titel, Text oder Kommentaren, Beschreibung @@ -193,19 +197,25 @@ und ersetzen weder Merge-Opt-in noch Identitätsprüfung. gelesenen Head (unabhängige Prüfung, alle Merge-Gates). Gestapelte Drafts (Base ist nicht der Default-Branch) und Drafts mit aktivem Hold oder Lock bleiben unberührt. - `trigger_missing_ci` (bool): Hat der exakte Head keine Check-Läufe, darf der Bot sie - anstoßen: `gh workflow run --ref ` für Workflows mit `workflow_dispatch` - oder `gh run rerun` eines früheren Laufs desselben PRs. Verboten bleiben leere Commits, + anstoßen: Core-Aktion `actions.dispatch` (Workflow mit `workflow_dispatch`, `--ref `) + oder `actions.rerun` eines abgeschlossenen früheren Laufs desselben Heads. Verboten bleiben leere Commits, Pushs auf fremde Branches, Änderungen an Workflow-Dateien, das Freigeben wartender Fork-Läufe und jede Umgehung des Branch-Schutzes. Entsteht kein Lauf (kein passender Workflow, Actions aus, Abrechnung), ist das HOLD mit Ticket. Ein Lauf ist erst nach Abschluss und grün am gleichen Head ein Beleg; in der nächsten Runde erneut lesen. + Selbst gehostete Runner (z. B. Labels `self-hosted`/`mac-studio`) holen ihre Jobs bei GitHub + ab; angestoßen wird immer über GitHub. Bleibt ein Job länger als eine Runde `queued` oder + „Waiting for a runner“, ist das HOLD (Runner offline?) mit Ticket, nie grün. Lokale Läufe + auf dem Runner-Host sind Prüfbelege für Reviews, aber keine Checks; keine Commit-Status setzen. - `publish_reviews` (bool): erlaubt Reviews, Review-Kommentare, Triage-Kommentare und Labels an PRs und Issues der konfigurierten Owner. Inhalt: exakter Head, tatsächliches Modell, 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. -- `open_fix_prs` (bool): erlaubt Fix-Branches zu pushen (nie Default-Branch, nie Force) und PRs - zu öffnen. Ohne diesen Schalter bleiben Fixes lokal vorbereitet und werden gemeldet. +- `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 + vorbereitet und werden gemeldet. ## Paket- und Instanzkonfiguration @@ -220,3 +230,6 @@ Die Core-Konfiguration kommt über wiederholbares `--config PFAD`. `multi_bot` e Root, optionalen Instanzprompt, Merge-Konto, Merge-Opt-in, `policy`-Schalter und optional trusted_automation. Die berechnete Core-Permission-Decke bleibt bindend; ein Rollenprompt erweitert sie nicht. Ohne Merge-Aktionsfähigkeit/Permission keine Merge-Mutation, auch bei aktivem Merge-Opt-in. +Jede getypte Aktion (`pr.merge`, `pr.ready`, `actions.dispatch`, `actions.rerun`, `pr.create`, +`git.push_branch`) braucht Obergrenze, `permissions.enabled` UND ihren `multi_bot`-Schalter; +der Startauftrag nennt das Ergebnis als `effective_enabled`. Nur diese Liste gilt. diff --git a/src/githubbot_core/typed_actions.py b/src/githubbot_core/typed_actions.py new file mode 100644 index 0000000..7961e55 --- /dev/null +++ b/src/githubbot_core/typed_actions.py @@ -0,0 +1,260 @@ +"""Narrow, typed write actions for pull requests, CI runs and bot branches. + +``api.request`` can only be scoped by endpoint prefix, which is far too coarse for +merges (``repos//`` would allow every write in every repository). These +actions each do exactly one thing, re-read the target before writing, refuse when +the reviewed head moved, and read the result back. They are in the default ceiling +but never in the default ``enabled`` list, and every kind is additionally gated by +its ``multi_bot`` switch, so a permission alone opens nothing. +""" + +from __future__ import annotations + +import re +import subprocess +from typing import Any, Optional + +SHA = re.compile(r"[0-9a-f]{40}") +REPO = re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+") +BRANCH = re.compile(r"[A-Za-z0-9][A-Za-z0-9._/-]{0,199}") +WORKFLOW = re.compile(r"[A-Za-z0-9_.-]+\.ya?ml|[0-9]+") +MERGE_METHODS = ("merge", "squash", "rebase") +DEFAULT_PUSH_PREFIX = "bot/" + +KINDS = ("pr.merge", "pr.ready", "actions.dispatch", "actions.rerun", "pr.create", "git.push_branch") + +_git = subprocess.run # replaced in tests + + +def _policy(config: dict) -> dict: + from .multibot import policy_settings + + settings = config.get("multi_bot") or {} + return {"merge_enabled": settings.get("merge_enabled") is True, **policy_settings(settings.get("policy"))} + + +def switch_denial(kind: str, config: dict) -> Optional[str]: + """A typed action also needs its multi_bot switch; the permission alone is not enough.""" + if kind not in KINDS: + return None + try: + policy = _policy(config) + except ValueError as exc: + return f"multi_bot.policy invalid: {exc}" + allowed = { + "pr.merge": policy["merge_enabled"], + "pr.ready": policy["draft_auto_ready_hours"] is not None or policy["draft_ready_when_mature"], + "actions.dispatch": policy["trigger_missing_ci"], + "actions.rerun": policy["trigger_missing_ci"], + "pr.create": policy["open_fix_prs"], + "git.push_branch": policy["open_fix_prs"], + }[kind] + return None if allowed else f"{kind} is switched off in multi_bot (merge_enabled/policy)" + + +def push_prefix(config: dict) -> str: + prefix = str((config.get("typed_actions") or {}).get("push_branch_prefix") or DEFAULT_PUSH_PREFIX) + return prefix if prefix.endswith("/") else prefix + "/" + + +def param_denial(kind: str, params: dict, config: dict) -> Optional[str]: + """Static parameter checks, shown already in preview.""" + if kind not in KINDS: + 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") + 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(): + return "number must be a pull request number" + 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": + if not WORKFLOW.fullmatch(str(params.get("workflow", ""))): + return "workflow must be a workflow file name or id" + if not BRANCH.fullmatch(str(params.get("ref", ""))) or ".." in str(params.get("ref")): + return "ref must be a branch name" + if kind == "actions.rerun" and not str(params.get("run_id", "")).isdecimal(): + return "run_id must be numeric" + if kind in ("pr.create", "git.push_branch"): + branch = str(params.get("branch", "")) + if not BRANCH.fullmatch(branch) or ".." in branch or branch.endswith((".lock", "/")): + return "branch name is invalid" + if not branch.startswith(push_prefix(config)): + return f"branch must start with the bot prefix {push_prefix(config)!r}" + if kind == "pr.create" and (not str(params.get("title", "")).strip() or not str(params.get("body", "")).strip()): + return "title and body are required" + if kind == "git.push_branch" and not str(params.get("path", "")).strip(): + return "path of the local checkout is required" + return None + + +def _default_branch(transport, repo: str) -> tuple[Optional[str], Optional[str]]: + data, error = transport.get(f"repos/{repo}") + if error or not isinstance(data, dict) or not data.get("default_branch"): + return None, f"default branch unreadable: {error or 'no data'}" + return data["default_branch"], None + + +def _pull(transport, repo: str, number) -> tuple[dict, Optional[str]]: + data, error = transport.get(f"repos/{repo}/pulls/{int(number)}") + if error or not isinstance(data, dict): + return {}, f"pull request unreadable: {error or 'no data'}" + return data, None + + +def _reviewed_pull(transport, params: dict, *, draft: bool) -> tuple[dict, Optional[str]]: + """Fresh read: open, unstacked, exact reviewed head, expected draft state.""" + repo, sha = params["repo"], params["sha"] + default, error = _default_branch(transport, repo) + if error: + return {}, error + pull, error = _pull(transport, repo, params["number"]) + if error: + return {}, error + if pull.get("state") != "open": + return {}, "pull request is not open" + if (pull.get("head") or {}).get("sha") != sha: + return {}, "head moved since review (sha mismatch); nothing written" + if (pull.get("base") or {}).get("ref") != default: + return {}, "stacked pull request (base is not the default branch)" + if bool(pull.get("draft")) is not draft: + return {}, "pull request is a draft" if pull.get("draft") else "pull request is not a draft" + return pull, None + + +def exec_merge(transport, params: dict, config: dict): + account = str((config.get("multi_bot") or {}).get("merge_account") or "") + if account: + user, error = transport.get("user") + if error or (user or {}).get("login") != account: + return None, f"identity is not the configured merge account {account!r}" + pull, error = _reviewed_pull(transport, params, draft=False) + if error: + return None, error + # "clean" means GitHub sees required checks and reviews satisfied. Anything else + # (blocked, unstable, dirty, unknown) is a HOLD, so an admin token cannot bypass. + if pull.get("mergeable_state") != "clean": + return None, f"mergeable_state is {pull.get('mergeable_state')!r}, not 'clean'" + repo, number = params["repo"], int(params["number"]) + body = {"sha": params["sha"], "merge_method": params.get("method", "merge")} + _, error = transport.request("PUT", f"repos/{repo}/pulls/{number}/merge", body) + if error: + return None, error + after, error = _pull(transport, repo, number) + if error or not after.get("merged") or not after.get("merge_commit_sha"): + return None, "merge not confirmed by readback" + return {"readback": {"merged": True, "merge_commit_sha": after["merge_commit_sha"]}}, None + + +READY_MUTATION = ( + "mutation($id: ID!) { markPullRequestReadyForReview(input: {pullRequestId: $id}) { pullRequest { isDraft } } }" +) + + +def exec_ready(transport, params: dict, config: dict): + pull, error = _reviewed_pull(transport, params, draft=True) + if error: + return None, error + data, error = transport.request("POST", "graphql", {"query": READY_MUTATION, "variables": {"id": pull["node_id"]}}) + if error or (data or {}).get("errors"): + return None, error or "GraphQL error" + after, error = _pull(transport, params["repo"], params["number"]) + if error or after.get("draft") or (after.get("head") or {}).get("sha") != params["sha"]: + return None, "ready state not confirmed by readback" + return {"readback": {"draft": False, "head": params["sha"]}}, 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}") + if error or (data or {}).get("state") != "active": + return None, f"workflow not active or unreadable: {error or (data or {}).get('state')}" + _, error = transport.request( + "POST", f"repos/{repo}/actions/workflows/{workflow}/dispatches", {"ref": params["ref"]} + ) + if error: + return None, error + # The run appears asynchronously; queued or still running is never a green result. + return {"readback": {"dispatched": True, "note": "evidence only once completed and green at the same head"}}, None + + +def exec_rerun(transport, params: dict, config: dict): + repo, run_id = params["repo"], int(params["run_id"]) + run, error = transport.get(f"repos/{repo}/actions/runs/{run_id}") + if error or not isinstance(run, dict): + return None, f"run unreadable: {error or 'no data'}" + if run.get("head_sha") != params["sha"]: + return None, "run belongs to another head" + if (run.get("head_repository") or {}).get("full_name") != repo: + return None, "run comes from a fork; fork runs are never re-run or approved by the bot" + if run.get("status") != "completed": + return None, f"run is {run.get('status')!r}, not completed" + _, error = transport.request("POST", f"repos/{repo}/actions/runs/{run_id}/rerun", None) + return (None, error) if error else ({"readback": {"rerun": run_id}}, None) + + +def exec_create_pr(transport, params: dict, config: dict): + repo, branch = params["repo"], params["branch"] + default, error = _default_branch(transport, repo) + if error: + return None, error + if branch == default: + return None, "head must not be the default branch" + body = { + "title": str(params["title"]), + "body": str(params["body"]), + "head": branch, + "base": default, + "draft": bool(params.get("draft", False)), + } + data, error = transport.request("POST", f"repos/{repo}/pulls", body) + if error or not isinstance(data, dict) or not data.get("number"): + return None, error or "pull request not confirmed" + return {"readback": {"number": data["number"], "url": data.get("html_url")}}, None + + +def _run_git(path: str, *args: str) -> tuple[str, Optional[str]]: + result = _git(["git", "-C", path, *args], capture_output=True, text=True, encoding="utf-8", errors="replace") + if result.returncode: + return "", " ".join((result.stderr or result.stdout or "").split())[:500] or f"git exit {result.returncode}" + return result.stdout.strip(), None + + +def _same_repo(url: str, repo: str) -> bool: + url = url.strip().removesuffix(".git").removesuffix("/").casefold() + return url.endswith("github.com/" + repo.casefold()) or url.endswith("github.com:" + repo.casefold()) + + +def exec_push_branch(transport, params: dict, config: dict): + repo, branch, sha, path = params["repo"], params["branch"], params["sha"], params["path"] + default, error = _default_branch(transport, repo) + if error: + return None, error + if branch == default: + return None, "never pushes the default branch" + url, error = _run_git(path, "remote", "get-url", "origin") + if error or not _same_repo(url, repo): + return None, "origin of the checkout is not the target repository" + resolved, error = _run_git(path, "rev-parse", "--verify", f"{sha}^{{commit}}") + if error or resolved != sha: + return None, "commit not present in the checkout" + # No '+' refspec and no --force: a non-fast-forward is rejected by git itself. + _, error = _run_git(path, "push", "origin", f"{sha}:refs/heads/{branch}") + if error: + return None, error + remote, error = _run_git(path, "ls-remote", "origin", f"refs/heads/{branch}") + if error or not remote.startswith(sha): + return None, "push not confirmed by ls-remote" + return {"readback": {"branch": branch, "sha": sha}}, None + + +EXECUTORS: dict[str, Any] = { + "pr.merge": exec_merge, + "pr.ready": exec_ready, + "actions.dispatch": exec_dispatch, + "actions.rerun": exec_rerun, + "pr.create": exec_create_pr, + "git.push_branch": exec_push_branch, +} diff --git a/tests/test_multibot_resources.py b/tests/test_multibot_resources.py index 9ff54f3..39d9f97 100644 --- a/tests/test_multibot_resources.py +++ b/tests/test_multibot_resources.py @@ -155,7 +155,7 @@ def test_readonly_ceiling_and_config_layers_reach_the_run_contract(tmp_path): request.split("AUTHORITATIVE CORE RUN CONTRACT (not credentials):\n")[1].split("\nPreserve config_layers")[0] ) assert contract == { - "permissions": {"ceiling": [], "enabled": [], "api_write_prefixes": []}, + "permissions": {"ceiling": [], "enabled": [], "api_write_prefixes": [], "effective_enabled": []}, "config_layers": [str(path.resolve())], } assert "must-not-copy-secret" not in request and "unknown write authority means HOLD" in request diff --git a/tests/test_typed_actions.py b/tests/test_typed_actions.py new file mode 100644 index 0000000..05c66c3 --- /dev/null +++ b/tests/test_typed_actions.py @@ -0,0 +1,252 @@ +"""Typed write actions: ceiling + enabled + multi_bot switch, fresh re-read, exact head, readback.""" + +from __future__ import annotations + +import json +from types import SimpleNamespace + +from githubbot_core import actions, cli, typed_actions +from githubbot_core.actions import Action +from githubbot_core.config import default_config, merge_layers +from githubbot_core.multibot import permission_contract + +REPO = "example-org/example-repo" +SHA = "a" * 40 +OTHER = "b" * 40 +ALL = list(typed_actions.KINDS) + + +def config(enabled=ALL, merge=True, **policy): + switches = {"trigger_missing_ci": True, "open_fix_prs": True, "draft_ready_when_mature": True, **policy} + layer = { + "permissions": {"enabled": ["issue.comment", *enabled]}, + "multi_bot": {"merge_enabled": merge, "merge_account": "example-bot", "policy": switches}, + } + return merge_layers([("default", default_config()), ("instance", layer)]) + + +def pull(**overrides): + data = { + "state": "open", + "draft": False, + "node_id": "PR_node", + "head": {"sha": SHA}, + "base": {"ref": "main"}, + "mergeable_state": "clean", + "merged": False, + } + data.update(overrides) + return data + + +class Transport: + """GET answers may be lists (one entry per call, the last repeats); writes are recorded.""" + + def __init__(self, gets, write_result=None): + self.gets = {key: value if isinstance(value, list) else [value] for key, value in gets.items()} + self.write_result = write_result if write_result is not None else {"ok": True} + self.writes = [] + + def get(self, endpoint): + answers = self.gets.get(endpoint) + if not answers: + return None, f"404 {endpoint}" + return (answers.pop(0) if len(answers) > 1 else answers[0]), None + + def request(self, method, endpoint, body=None): + self.writes.append((method, endpoint, body)) + return self.write_result, None + + +def base_gets(**pull_overrides): + return { + f"repos/{REPO}": {"default_branch": "main"}, + "user": {"login": "example-bot"}, + f"repos/{REPO}/pulls/7": [pull(**pull_overrides), pull(merged=True, merge_commit_sha=OTHER)], + } + + +def merge_action(sha=SHA): + return Action("pr.merge", {"repo": REPO, "number": 7, "sha": sha}) + + +def test_default_ceiling_lists_typed_actions_but_enables_none(): + cfg = default_config() + assert set(ALL) <= set(cfg["permissions"]["ceiling"]) + assert not set(ALL) & set(cfg["permissions"]["enabled"]) + assert "not enabled" in actions.denial_reason(merge_action(), merge_layers([("default", cfg)])) + + +def test_permission_alone_is_not_enough_without_the_switch(): + assert "switched off" in actions.denial_reason(merge_action(), config(merge=False)) + off = config(trigger_missing_ci=False, open_fix_prs=False, draft_ready_when_mature=False) + assert "switched off" in actions.denial_reason(Action("actions.rerun", {}), off) + assert "switched off" in actions.denial_reason(Action("pr.create", {}), off) + assert "switched off" in actions.denial_reason(Action("pr.ready", {}), off) + assert actions.denial_reason(merge_action(), config()) is None + + +def test_static_parameters_are_checked_in_preview(): + cfg = config() + assert "sha" in actions.denial_reason(merge_action(sha="abc"), cfg) + assert "method" in actions.denial_reason( + Action("pr.merge", {"repo": REPO, "number": 7, "sha": SHA, "method": "x"}), cfg + ) + push = {"repo": REPO, "branch": "feature/x", "sha": SHA, "path": "."} + assert "prefix" in actions.denial_reason(Action("git.push_branch", push), cfg) + assert "invalid" in actions.denial_reason(Action("git.push_branch", {**push, "branch": "bot/../main"}), cfg) + + +def test_merge_is_sha_bound_and_read_back(): + transport = Transport(base_gets()) + result = actions.apply([merge_action()], transport, config()) + receipt = result["receipts"][0] + assert receipt["status"] == "applied" and receipt["readback"]["merge_commit_sha"] == OTHER + assert transport.writes == [("PUT", f"repos/{REPO}/pulls/7/merge", {"sha": SHA, "merge_method": "merge"})] + + +def test_merge_refuses_moved_head_unclean_stacked_draft_and_foreign_identity(): + cases = [ + {"head": {"sha": OTHER}}, + {"mergeable_state": "blocked"}, + {"mergeable_state": "unknown"}, + {"base": {"ref": "feature"}}, + {"draft": True}, + {"state": "closed"}, + ] + for overrides in cases: + transport = Transport(base_gets(**overrides)) + receipt = actions.apply([merge_action()], transport, config())["receipts"][0] + assert receipt["status"] == "failed", overrides + assert transport.writes == [], overrides + gets = base_gets() + gets["user"] = {"login": "someone-else"} + transport = Transport(gets) + assert actions.apply([merge_action()], transport, config())["receipts"][0]["status"] == "failed" + assert transport.writes == [] + + +def test_merge_without_readback_is_failed(): + gets = base_gets() + gets[f"repos/{REPO}/pulls/7"] = pull() # never shows merged + receipt = actions.apply([merge_action()], Transport(gets), config())["receipts"][0] + assert receipt["status"] == "failed" and "readback" in receipt["detail"] + + +def test_ready_uses_graphql_for_the_exact_draft_head(): + gets = base_gets() + gets[f"repos/{REPO}/pulls/7"] = [pull(draft=True), pull(draft=False)] + transport = Transport(gets, write_result={"data": {"markPullRequestReadyForReview": {}}}) + action = Action("pr.ready", {"repo": REPO, "number": 7, "sha": SHA}) + receipt = actions.apply([action], transport, config())["receipts"][0] + assert receipt["status"] == "applied" + method, endpoint, body = transport.writes[0] + assert (method, endpoint, body["variables"]) == ("POST", "graphql", {"id": "PR_node"}) + gets[f"repos/{REPO}/pulls/7"] = pull(draft=True, base={"ref": "feature"}) + transport = Transport(gets) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "failed" + assert transport.writes == [] + + +def test_rerun_refuses_fork_runs_other_heads_and_running_runs(): + run = {"head_sha": SHA, "status": "completed", "head_repository": {"full_name": REPO}} + action = Action("actions.rerun", {"repo": REPO, "run_id": 5, "sha": SHA}) + for overrides in ( + {"head_repository": {"full_name": "fork/example-repo"}}, + {"head_sha": OTHER}, + {"status": "queued"}, + ): + transport = Transport({f"repos/{REPO}/actions/runs/5": {**run, **overrides}}) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "failed" + assert transport.writes == [] + transport = Transport({f"repos/{REPO}/actions/runs/5": run}) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "applied" + assert transport.writes == [("POST", f"repos/{REPO}/actions/runs/5/rerun", None)] + + +def test_dispatch_needs_an_active_workflow(): + action = Action("actions.dispatch", {"repo": REPO, "workflow": "ci.yml", "ref": "bot/fix"}) + transport = Transport({f"repos/{REPO}/actions/workflows/ci.yml": {"state": "disabled_manually"}}) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "failed" + transport = Transport({f"repos/{REPO}/actions/workflows/ci.yml": {"state": "active"}}) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "applied" + assert transport.writes == [("POST", f"repos/{REPO}/actions/workflows/ci.yml/dispatches", {"ref": "bot/fix"})] + + +def test_create_pr_targets_the_default_branch_from_a_bot_branch(): + action = Action("pr.create", {"repo": REPO, "branch": "bot/fix", "title": "fix: x", "body": "Problem, tests"}) + transport = Transport({f"repos/{REPO}": {"default_branch": "main"}}, write_result={"number": 9, "html_url": "u"}) + receipt = actions.apply([action], transport, config())["receipts"][0] + assert receipt["status"] == "applied" and receipt["readback"]["number"] == 9 + assert transport.writes[0][2]["base"] == "main" and transport.writes[0][2]["head"] == "bot/fix" + + +def test_push_branch_never_forces_and_checks_origin(monkeypatch): + calls = [] + + def git(cmd, **kwargs): + calls.append(cmd[3:]) + out = { + "remote": f"https://github.com/{REPO}.git\n", + "rev-parse": SHA + "\n", + "push": "", + "ls-remote": f"{SHA}\trefs/heads/bot/fix\n", + }[cmd[3]] + return SimpleNamespace(returncode=0, stdout=out, stderr="") + + monkeypatch.setattr(typed_actions, "_git", git) + action = Action("git.push_branch", {"repo": REPO, "branch": "bot/fix", "sha": SHA, "path": "."}) + transport = Transport({f"repos/{REPO}": {"default_branch": "main"}}) + receipt = actions.apply([action], transport, config())["receipts"][0] + assert receipt["status"] == "applied" + push = next(c for c in calls if c[0] == "push") + assert push == ["push", "origin", f"{SHA}:refs/heads/bot/fix"] + assert not any(arg.startswith("+") or "force" in arg for c in calls for arg in c) + + calls.clear() + monkeypatch.setattr( + typed_actions, + "_git", + lambda cmd, **k: SimpleNamespace(returncode=0, stdout="https://github.com/other/repo\n", stderr=""), + ) + assert actions.apply([action], transport, config())["receipts"][0]["status"] == "failed" + + +def test_push_branch_refuses_the_default_branch_even_with_bot_prefix(monkeypatch): + monkeypatch.setattr(typed_actions, "_git", lambda *a, **k: (_ for _ in ()).throw(AssertionError("git called"))) + cfg = config() + cfg["typed_actions"] = {"push_branch_prefix": "release"} + action = Action("git.push_branch", {"repo": REPO, "branch": "release/main", "sha": SHA, "path": "."}) + transport = Transport({f"repos/{REPO}": {"default_branch": "release/main"}}) + assert actions.apply([action], transport, cfg)["receipts"][0]["status"] == "failed" + + +def test_run_contract_reports_effective_enabled(tmp_path): + layer = tmp_path / "instance.json" + layer.write_text( + json.dumps( + { + "permissions": {"enabled": ["issue.comment", "pr.merge", "pr.ready"]}, + "multi_bot": {"merge_enabled": True, "policy": {"draft_auto_ready_hours": None}}, + } + ) + ) + from githubbot_core.config import load_config + + contract = permission_contract(load_config([layer]), [layer]) + assert contract["permissions"]["effective_enabled"] == ["issue.comment", "pr.merge"] + + +def test_cli_act_previews_then_applies_with_expected_plan(tmp_path, capsys): + layer = tmp_path / "instance.json" + layer.write_text( + json.dumps({"permissions": {"enabled": ["pr.merge"]}, "multi_bot": {"merge_enabled": True}}), encoding="utf-8" + ) + params = json.dumps({"repo": REPO, "number": 7, "sha": SHA}) + transport = Transport(base_gets()) + argv = ["--config", str(layer), "--json", "act", "pr.merge", "--params", params] + assert cli.main(argv, transport=transport) == 0 + preview = json.loads(capsys.readouterr().out)["data"] + assert preview["actions"][0]["permitted"] and transport.writes == [] + assert cli.main([*argv, "--apply", "--expect-plan", preview["plan_sha256"]], transport=transport) == 0 + assert json.loads(capsys.readouterr().out)["data"]["receipts"][0]["status"] == "applied"