From 3d22ba30047c56fb107570301cb64fcec252a3bf Mon Sep 17 00:00:00 2001 From: phernandez Date: Tue, 9 Jun 2026 16:32:06 -0500 Subject: [PATCH] fix(ci): harden BM Bossbot finalization Signed-off-by: phernandez --- scripts/bm_bossbot_status.py | 20 +++- tests/scripts/test_bm_bossbot_status.py | 118 ++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 5 deletions(-) diff --git a/scripts/bm_bossbot_status.py b/scripts/bm_bossbot_status.py index 479914fd..9d1c78f7 100755 --- a/scripts/bm_bossbot_status.py +++ b/scripts/bm_bossbot_status.py @@ -191,6 +191,17 @@ def update_pull_request_body(*, token: str, repo: str, number: int, body: str) - ) +def get_pull_request_body(*, token: str, repo: str, number: int) -> str: + response = _github_request( + method="GET", + path=f"/repos/{repo}/pulls/{number}", + token=token, + ) + if not isinstance(response, Mapping): + raise SystemExit("GitHub API response for pull request was invalid") + return _string(response.get("body")) + + def mark_pending( *, event_path: Path, @@ -234,7 +245,8 @@ def finalize_review( review = {} result = validate_review(review, expected_head_sha=event.head_sha) - updated_body = upsert_summary_block(event.body, render_summary(review, result)) + current_body = get_pull_request_body(token=token, repo=event.repo, number=event.number) + updated_body = upsert_summary_block(current_body, render_summary(review, result)) update_pull_request_body(token=token, repo=event.repo, number=event.number, body=updated_body) set_commit_status( token=token, @@ -255,9 +267,9 @@ def _github_request( method: str, path: str, token: str, - payload: Mapping[str, Any], + payload: Mapping[str, Any] | None = None, ) -> Any: - data = json.dumps(payload).encode("utf-8") + data = None if payload is None else json.dumps(payload).encode("utf-8") request = urllib.request.Request( f"https://api.github.com{path}", data=data, @@ -343,9 +355,7 @@ def finalize( Path, typer.Option( "--review", - exists=True, dir_okay=False, - readable=True, help="Structured BM Bossbot review JSON.", ), ], diff --git a/tests/scripts/test_bm_bossbot_status.py b/tests/scripts/test_bm_bossbot_status.py index 3e31ddac..c9f327b9 100644 --- a/tests/scripts/test_bm_bossbot_status.py +++ b/tests/scripts/test_bm_bossbot_status.py @@ -1,6 +1,24 @@ +import json +from pathlib import Path +from typing import Mapping + +import pytest +from typer.testing import CliRunner + from scripts import bm_bossbot_status +def _event_payload(body: str = "Event snapshot body") -> dict[str, object]: + return { + "repository": {"full_name": "basicmachines-co/basic-memory"}, + "pull_request": { + "number": 925, + "body": body, + "head": {"sha": "abc123"}, + }, + } + + def test_status_script_is_uv_typer_entrypoint() -> None: source = bm_bossbot_status.__file__ assert source is not None @@ -84,3 +102,103 @@ def test_upsert_summary_block_replaces_existing_block() -> None: assert "New summary" in updated assert updated.startswith("Intro") assert updated.endswith("Footer") + + +def test_finalize_review_fetches_current_pr_body_before_upserting( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + event_path = tmp_path / "event.json" + review_path = tmp_path / "review.json" + event_path.write_text(json.dumps(_event_payload()), encoding="utf-8") + review_path.write_text(json.dumps(_review_payload()), encoding="utf-8") + monkeypatch.setenv("GITHUB_TOKEN", "token") + + updated_bodies: list[str] = [] + statuses: list[Mapping[str, str]] = [] + + def fake_get_pull_request_body(*, token: str, repo: str, number: int) -> str: + assert token == "token" + assert repo == "basicmachines-co/basic-memory" + assert number == 925 + return "Current body edited while the workflow was running" + + def fake_update_pull_request_body(*, token: str, repo: str, number: int, body: str) -> None: + updated_bodies.append(body) + + def fake_set_commit_status( + *, + token: str, + repo: str, + sha: str, + payload: Mapping[str, str], + ) -> None: + statuses.append(payload) + + monkeypatch.setattr(bm_bossbot_status, "get_pull_request_body", fake_get_pull_request_body) + monkeypatch.setattr(bm_bossbot_status, "update_pull_request_body", fake_update_pull_request_body) + monkeypatch.setattr(bm_bossbot_status, "set_commit_status", fake_set_commit_status) + + result = bm_bossbot_status.finalize_review( + event_path=event_path, + review_path=review_path, + repo=None, + run_url="https://github.com/basicmachines-co/basic-memory/actions/runs/1", + token_env="GITHUB_TOKEN", + ) + + assert result.approved is True + assert "Current body edited while the workflow was running" in updated_bodies[0] + assert "Event snapshot body" not in updated_bodies[0] + assert statuses[0]["state"] == "success" + + +def test_finalize_cli_marks_failure_when_review_file_is_missing( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + event_path = tmp_path / "event.json" + missing_review_path = tmp_path / "missing-review.json" + event_path.write_text(json.dumps(_event_payload(body="Current body")), encoding="utf-8") + monkeypatch.setenv("GITHUB_TOKEN", "token") + + updated_bodies: list[str] = [] + statuses: list[Mapping[str, str]] = [] + + def fake_get_pull_request_body(*, token: str, repo: str, number: int) -> str: + return "Current body" + + def fake_update_pull_request_body(*, token: str, repo: str, number: int, body: str) -> None: + updated_bodies.append(body) + + def fake_set_commit_status( + *, + token: str, + repo: str, + sha: str, + payload: Mapping[str, str], + ) -> None: + statuses.append(payload) + + monkeypatch.setattr(bm_bossbot_status, "get_pull_request_body", fake_get_pull_request_body) + monkeypatch.setattr(bm_bossbot_status, "update_pull_request_body", fake_update_pull_request_body) + monkeypatch.setattr(bm_bossbot_status, "set_commit_status", fake_set_commit_status) + + result = CliRunner().invoke( + bm_bossbot_status.app, + [ + "finalize", + "--event", + str(event_path), + "--review", + str(missing_review_path), + "--repo", + "basicmachines-co/basic-memory", + "--run-url", + "https://github.com/basicmachines-co/basic-memory/actions/runs/1", + ], + ) + + assert result.exit_code == 1 + assert "BM Bossbot review output was invalid" in updated_bodies[0] + assert statuses[0]["state"] == "failure"