diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a70a45..fde6c48 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 8355d48..c5b9490 100644 --- a/README.md +++ b/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 :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 +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 ``` +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/"]}} diff --git a/README_de.md b/README_de.md index 947c7d9..05ef534 100644 --- a/README_de.md +++ b/README_de.md @@ -114,14 +114,15 @@ 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 :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. +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 @@ -129,6 +130,24 @@ githubbot-core --config instanz.json act pr.merge --params '{"repo": "o/r", "num githubbot-core --config instanz.json act pr.merge --params '…' --apply --expect-plan ``` +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/"]}} diff --git a/src/githubbot_core/default_config.json b/src/githubbot_core/default_config.json index a354ca2..717f5d9 100644 --- a/src/githubbot_core/default_config.json +++ b/src/githubbot_core/default_config.json @@ -10,6 +10,7 @@ "api.request", "pr.merge", "pr.ready", + "pr.review", "actions.dispatch", "actions.rerun", "pr.create", diff --git a/src/githubbot_core/typed_actions.py b/src/githubbot_core/typed_actions.py index 92fc00d..83ade3b 100644 --- a/src/githubbot_core/typed_actions.py +++ b/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, diff --git a/tests/test_pr_review_action.py b/tests/test_pr_review_action.py new file mode 100644 index 0000000..ee65d94 --- /dev/null +++ b/tests/test_pr_review_action.py @@ -0,0 +1,304 @@ +"""Formal reviews stay opt-in, commit-bound and fail closed on ambiguous writes.""" + +from __future__ import annotations + +import copy +import json + +import pytest + +from githubbot_core import actions, cli +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 +ENDPOINT = f"repos/{REPO}/pulls/7" +STATES = {"COMMENT": "COMMENTED", "APPROVE": "APPROVED", "REQUEST_CHANGES": "CHANGES_REQUESTED"} + + +def config(*, enabled=True, publish=True, account="example-bot"): + return merge_layers( + [ + ("default", default_config()), + ( + "instance", + { + "permissions": {"enabled": ["pr.review"] if enabled else []}, + "multi_bot": {"merge_account": account, "policy": {"publish_reviews": publish}}, + }, + ), + ] + ) + + +def action(**overrides): + return Action( + "pr.review", + { + "repo": REPO, + "number": 7, + "sha": SHA, + "event": "APPROVE", + "body": "Complete independent review; tests passed.", + **overrides, + }, + ) + + +class Transport: + def __init__(self): + self.login = "example-bot" + self.pull = {"state": "open", "head": {"sha": SHA}, "draft": False, "user": {"login": "author"}} + self.pull_answers = [] + self.reviews = [] + self.list_error = None + self.write_error = None + self.id = 81 + self.created = None + self.readback_override = {} + self.readback_error = None + self.writes = [] + self.reads = [] + + def get(self, endpoint): + self.reads.append(endpoint) + if endpoint == "user": + return {"login": self.login}, None + if endpoint == ENDPOINT: + return copy.deepcopy(self.pull_answers.pop(0) if self.pull_answers else self.pull), None + if endpoint == f"{ENDPOINT}/reviews/81": + return {**self.created, **self.readback_override}, self.readback_error + raise AssertionError(f"unexpected GET {endpoint}") + + def get_paginated(self, endpoint): + assert endpoint == f"{ENDPOINT}/reviews?per_page=100" + return copy.deepcopy(self.reviews), self.list_error + + def request(self, method, endpoint, body): + assert (method, endpoint) == ("POST", f"{ENDPOINT}/reviews") + self.writes.append((method, endpoint, body)) + self.created = { + "id": self.id, + "commit_id": body["commit_id"], + "body": body["body"], + "state": STATES[body["event"]], + "user": {"login": self.login}, + } + # A timed-out POST can have created a real review. Subsequent calls must discover it. + self.reviews.append(copy.deepcopy(self.created)) + return self.created, self.write_error + + +def apply(transport, review=None, cfg=None): + return actions.apply([review or action()], transport, cfg or config())["receipts"][0] + + +def test_review_requires_permission_and_policy_and_never_runs_from_preview(): + review = action() + assert "pr.review" in default_config()["permissions"]["ceiling"] + assert "pr.review" not in default_config()["permissions"]["enabled"] + assert "not enabled" in actions.denial_reason(review, config(enabled=False)) + assert "switched off" in actions.denial_reason(review, config(publish=False)) + contract = permission_contract(config(), []) + assert contract["permissions"]["effective_enabled"] == ["pr.review"] + assert actions.preview([review], config())["writes"] == 0 + assert actions.denial_reason(review, {"permissions": {"enabled": ["pr.review"], "ceiling": []}}) + + +@pytest.mark.parametrize( + "overrides", + [ + {"sha": "short"}, + {"sha": OTHER.upper()}, + {"number": 0}, + {"number": -1}, + {"repo": "../wrong/repo"}, + {"event": "PENDING"}, + {"event": "approve"}, + {"event": []}, + {"body": " "}, + {"body": None}, + {"body": {}}, + {"body": "x" * 65537}, + ], +) +def test_invalid_review_parameters_are_denied_without_api_calls(overrides): + transport = Transport() + receipt = apply(transport, action(**overrides)) + assert receipt["status"] == "denied" + assert transport.reads == transport.writes == [] + + +@pytest.mark.parametrize("event,state", STATES.items()) +def test_formal_review_is_submitted_at_the_exact_sha_and_independently_read_back(event, state): + transport = Transport() + review = action(event=event, body="Geprüft: Änderungen vollständig gelesen.") + receipt = apply(transport, review) + assert receipt["status"] == "applied" + assert transport.writes == [ + ("POST", f"{ENDPOINT}/reviews", {"commit_id": SHA, "event": event, "body": review.params["body"]}) + ] + assert receipt["readback"] == {"review_id": 81, "head": SHA, "state": state, "account": "example-bot"} + assert f"{ENDPOINT}/reviews/81" in transport.reads + + +@pytest.mark.parametrize( + "pull", + [ + {"state": "closed"}, + {"head": {"sha": OTHER}}, + {"head": None}, + {"draft": True}, + {"user": {"login": "Example-Bot"}}, + {"user": None}, + {"user": {"login": ""}}, + {"draft": None}, + ], +) +def test_approval_refuses_unreviewed_closed_draft_or_own_pr(pull): + transport = Transport() + transport.pull.update(pull) + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +def test_comment_may_review_a_draft_or_stacked_pull_without_enabling_merge(): + transport = Transport() + transport.pull.update(draft=True, base={"ref": "feature"}) + assert apply(transport, action(event="COMMENT"))["status"] == "applied" + assert transport.writes[0][1].endswith("/reviews") + + +def test_review_requires_the_configured_account(): + transport = Transport() + assert apply(transport, cfg=config(account=""))["status"] == "denied" + transport.login = "someone-else" + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +@pytest.mark.parametrize( + "existing", + [ + {"id": 12, "user": {"login": "example-bot"}, "commit_id": SHA, "state": "APPROVED"}, + {"id": 13, "user": {"login": "Example-Bot"}, "commit_id": SHA, "state": "PENDING"}, + {"id": 14, "user": None, "commit_id": SHA}, + ], +) +def test_existing_or_unreadable_reviews_prevent_a_second_write(existing): + transport = Transport() + transport.reviews = [existing] + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +def test_reviews_on_old_heads_or_from_other_actors_do_not_block(): + transport = Transport() + transport.reviews = [ + {"user": {"login": "example-bot"}, "commit_id": OTHER}, + {"user": {"login": "other-reviewer"}, "commit_id": SHA}, + ] + assert apply(transport)["status"] == "applied" + + +def test_review_list_failure_is_not_an_empty_list(): + transport = Transport() + transport.list_error = "403" + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +@pytest.mark.parametrize("reviews", [None, {}, [None], [{"user": {"login": ""}, "commit_id": SHA}]]) +def test_malformed_review_list_is_a_hold(reviews): + transport = Transport() + transport.reviews = reviews + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +@pytest.mark.parametrize("review_id", [None, True, 0, "81"]) +def test_post_without_a_usable_review_id_is_ambiguous_and_never_retried(review_id): + transport = Transport() + transport.id = review_id + receipt = apply(transport) + assert receipt["status"] == "failed" and "read existing reviews" in receipt["detail"] + assert len(transport.writes) == 1 + + +def test_head_moving_during_pagination_prevents_post(): + transport = Transport() + transport.pull_answers = [transport.pull, {**transport.pull, "head": {"sha": OTHER}}] + assert apply(transport)["status"] == "failed" + assert not transport.writes + + +@pytest.mark.parametrize( + "override", + [ + {"commit_id": OTHER}, + {"body": "different"}, + {"state": "PENDING"}, + {"id": 82}, + {"user": {"login": "another"}}, + {"user": None}, + ], +) +def test_readback_must_match_created_review_commit_body_state_and_actor(override): + transport = Transport() + transport.readback_override = override + receipt = apply(transport) + assert receipt["status"] == "failed" and "id=81" in receipt["detail"] + assert len(transport.writes) == 1 + + +def test_unreadable_readback_reports_the_created_id_and_does_not_repeat_post(): + transport = Transport() + transport.readback_error = "Timeout" + receipt = apply(transport) + assert receipt["status"] == "failed" and "id=81" in receipt["detail"] + assert len(transport.writes) == 1 + + +def test_head_moving_after_post_never_reports_a_current_head_approval(): + transport = Transport() + transport.pull_answers = [transport.pull, transport.pull, {**transport.pull, "head": {"sha": OTHER}}] + receipt = apply(transport) + assert receipt["status"] == "failed" and "target changed" in receipt["detail"] + assert len(transport.writes) == 1 + + +def test_timeout_after_creation_is_discovered_on_retry_even_without_receipt_store(): + transport = Transport() + transport.write_error = "Timeout" + assert apply(transport)["status"] == "failed" + transport.write_error = None + assert apply(transport)["status"] == "failed" + assert len(transport.writes) == 1 + + +def test_successful_review_is_deduplicated_by_receipt_store(tmp_path): + transport = Transport() + store = tmp_path / "receipts.jsonl" + first = actions.apply([action()], transport, config(), receipt_store=store) + second = actions.apply([action()], transport, config(), receipt_store=store) + assert first["receipts"][0]["status"] == "applied" + assert second["receipts"][0]["status"] == "skipped_duplicate" + assert len(transport.writes) == 1 + + +def test_cli_preview_plan_mismatch_and_apply_use_the_same_review_contract(tmp_path, capsys): + layer = tmp_path / "instance.json" + layer.write_text(json.dumps(config()), encoding="utf-8") + transport = Transport() + argv = ["--config", str(layer), "--json", "act", "pr.review", "--params", json.dumps(action().params)] + assert cli.main(argv, transport=transport) == 0 + plan = json.loads(capsys.readouterr().out)["data"] + assert plan["actions"][0]["permitted"] and not transport.writes + cli.main([*argv, "--apply", "--expect-plan", "wrong"], transport=transport) + assert not transport.writes + capsys.readouterr() + assert cli.main([*argv, "--apply", "--expect-plan", plan["plan_sha256"]], transport=transport) == 0 + assert json.loads(capsys.readouterr().out)["data"]["receipts"][0]["status"] == "applied"