From ef7da3b57518e005cbbfd0ecabc6f9742ed14bfc Mon Sep 17 00:00:00 2001 From: Kavya Katal Date: Sun, 11 Oct 2026 03:38:50 +0530 Subject: [PATCH] feat(bitbucket): add reject contract support --- tools/bitbucket/README.md | 5 +- tools/bitbucket/src/magpie_bitbucket/cli.py | 26 ++ tools/bitbucket/src/magpie_bitbucket/cloud.py | 37 +++ .../src/magpie_bitbucket/datacenter.py | 12 + .../src/magpie_bitbucket/normalize.py | 22 ++ tools/bitbucket/tests/test_bitbucket.py | 256 ++++++++++++++++++ tools/spec-loop/specs/adapters.md | 8 +- 7 files changed, 361 insertions(+), 5 deletions(-) diff --git a/tools/bitbucket/README.md b/tools/bitbucket/README.md index f72a51cc8..ae6abeedf 100644 --- a/tools/bitbucket/README.md +++ b/tools/bitbucket/README.md @@ -79,6 +79,7 @@ Implemented read-only commands: - `magpie-bitbucket pr request-changes ` (Cloud-only write) - `magpie-bitbucket pr remove-request-changes ` (Cloud-only write) - `magpie-bitbucket pr decline ` (Cloud-only write) +- `magpie-bitbucket pr reject --reason-file ` (Cloud-only write) - `magpie-bitbucket pr merge --strategy {merge,squash,rebase} --expected-source-commit ` (Cloud-only write) - `magpie-bitbucket pr merge-task-status ` (Cloud-only read) - `magpie-bitbucket pr tasks ` @@ -99,7 +100,7 @@ activity where exposed by the configured Bitbucket backend. Write coverage is intentionally narrow. The bridge supports confirmed Bitbucket Cloud issue-comment creation, top-level pull-request comment creation, -and pull-request approve/unapprove, request-changes/remove-request-changes, decline, and merge actions after the calling skill has obtained +and pull-request approve/unapprove, request-changes/remove-request-changes, decline, backend-neutral reject, and merge actions after the calling skill has obtained explicit user confirmation. Other writes, such as editing/deleting comments, creating/updating issues, changing branches, or triggering builds, remain out of scope and should be added separately with narrow command surfaces and @@ -163,7 +164,7 @@ surface: | Change requests | `post_review` / `pr review --verdict {comment,approve,request-changes} --body-file ` | Partial write, Cloud only | Implements the backend-neutral review contract for Bitbucket Cloud after explicit caller-side confirmation. `comment` posts the review body only. `approve` and `request-changes` post the body first and then apply the corresponding Bitbucket verdict. If the second mutation fails after the body was posted, the command fails with an explicit partial-outcome warning so callers inspect the PR before retrying. Data Center post-review writes remain unsupported. | | Change requests | `land` / `pr merge --strategy {merge,squash,rebase} --expected-source-commit ` | Partial write, Cloud only | Submits a Bitbucket Cloud pull-request merge after explicit caller-side confirmation. The caller must run and inspect `pr merge-checks ` before invoking this command; `pr merge` does not independently enforce approval, build-status, or merge-check gates. The expected source commit (7–40 hexadecimal characters, rejected before any request otherwise) is checked immediately before the merge POST so a changed PR head fails closed. The requested strategy is mapped to Bitbucket's merge strategy (reported as `backend_strategy`) and the resulting merge commit is returned as `landed_ref` when available. An asynchronous merge may be accepted before a `landed_ref` is available; `merge_status` is then `submitted`, with Bitbucket's own task state in `task_status` and the task URL in `task_url`. A timeout on the merge POST is reported as "outcome unknown" — check `pr get ` before retrying, since the merge may already be running. Data Center merge writes remain unsupported. | | Change requests | `pr merge-task-status ` | Partial read-only, Cloud only | Fetches the status of an asynchronous Bitbucket Cloud pull-request merge task. Pending tasks normalize to `merge_status=submitted`; successful tasks expose `merge_result.merge_commit.hash` as `landed_ref`; failed tasks normalize to `merge_status=failed`. Data Center merge task-status reads remain unsupported. | -| Change requests | `reject` | Not implemented | Follow-up work for #606. | +| Change requests | `reject` / `pr reject --reason-file ` | Partial write, Cloud only | Implements the backend-neutral reject contract for Bitbucket Cloud after explicit caller-side confirmation. Posts the confirmed rejection reason as a pull-request comment, then declines the pull request. If the decline fails after the reason was posted, the command reports the partial outcome and directs the caller to inspect the PR before retrying to avoid duplicating the rejection comment. Data Center reject writes remain unsupported. | | Tracker | `issue list-open` / `issue get ` / `issue comments ` / `issue attachments ` | Partial read-only, Cloud only | Lists and fetches Bitbucket Cloud issues, issue comments, and issue attachment metadata/links where the repository issue tracker is enabled. Bitbucket Data Center native issue reads/comments/attachments are unsupported; linked Jira handoff remains separate follow-up work. | | Tracker | `issue comment --body-file ` | Partial write, Cloud only | Creates one Bitbucket Cloud issue comment from a caller-supplied body file. The calling skill must obtain explicit user confirmation before invoking this mutation. Bitbucket Data Center native issue comment writes are unsupported; linked Jira coverage remains separate. | | CI | `pr status ` | Partial read-only | Fetches build/status checks for a pull request. This does not trigger, retry, or mutate Pipelines/builds. | diff --git a/tools/bitbucket/src/magpie_bitbucket/cli.py b/tools/bitbucket/src/magpie_bitbucket/cli.py index 042498e8f..e09cde90d 100644 --- a/tools/bitbucket/src/magpie_bitbucket/cli.py +++ b/tools/bitbucket/src/magpie_bitbucket/cli.py @@ -193,6 +193,20 @@ def _build_parser() -> argparse.ArgumentParser: help="Pull request ID whose change request to remove.", ) + pr_reject = pr_subparsers.add_parser( + "reject", + help="Reject a pull request with a reason after caller-side confirmation.", + ) + pr_reject.add_argument( + "pull_request_id", + help="Pull request ID to reject.", + ) + pr_reject.add_argument( + "--reason-file", + required=True, + help="Path to the confirmed rejection reason.", + ) + pr_decline = pr_subparsers.add_parser( "decline", help="Decline a pull request after caller-side confirmation.", @@ -376,6 +390,18 @@ def _dispatch(args: argparse.Namespace, config: BitbucketConfig) -> dict[str, An requested=False, ) + if args.subcommand == "pr" and args.pr_action == "reject": + reason = _read_body_file(args.reason_file) + raw = backend.reject_pull_request( + config, + args.pull_request_id, + reason, + ) + return normalize.rejected_pull_request( + config.kind, + raw, + ) + if args.subcommand == "pr" and args.pr_action == "decline": raw = backend.decline_pull_request( config, diff --git a/tools/bitbucket/src/magpie_bitbucket/cloud.py b/tools/bitbucket/src/magpie_bitbucket/cloud.py index ba26edc46..4b2f76cfe 100644 --- a/tools/bitbucket/src/magpie_bitbucket/cloud.py +++ b/tools/bitbucket/src/magpie_bitbucket/cloud.py @@ -736,6 +736,43 @@ def post_pull_request_review( } +def reject_pull_request( + config: BitbucketConfig, + pull_request_id: str, + reason: str, +) -> dict[str, Any]: + """Reject one Bitbucket Cloud pull request with a confirmed reason.""" + if not reason.strip(): + raise BitbucketError("Pull request rejection reason must not be empty") + + comment_result = create_pull_request_comment( + config, + pull_request_id, + reason, + ) + comment = comment_result.get("comment") + + try: + decline_result = decline_pull_request( + config, + pull_request_id, + ) + except BitbucketError as exc: + raise BitbucketError( + "Bitbucket pull request rejection reason was posted, but the " + f"decline failed. Inspect `pr discussion {pull_request_id}` and " + f"`pr get {pull_request_id}` before retrying to avoid duplicating " + "the rejection comment." + ) from exc + + return { + "pull_request_id": pull_request_id, + "reason": reason, + "comment": comment, + "pull_request": decline_result.get("pull_request"), + } + + def get_pull_request_discussion(config: BitbucketConfig, pull_request_id: str) -> dict[str, Any]: """Fetch pull request comments from Bitbucket Cloud.""" workspace = quote_path(require(config.workspace, "BITBUCKET_WORKSPACE")) diff --git a/tools/bitbucket/src/magpie_bitbucket/datacenter.py b/tools/bitbucket/src/magpie_bitbucket/datacenter.py index 14f0c5950..71ea1fbb8 100644 --- a/tools/bitbucket/src/magpie_bitbucket/datacenter.py +++ b/tools/bitbucket/src/magpie_bitbucket/datacenter.py @@ -464,6 +464,18 @@ def post_pull_request_review( ) +def reject_pull_request( + config: BitbucketConfig, + pull_request_id: str, + reason: str, +) -> dict[str, Any]: + """Reject pull-request reject writes for Bitbucket Data Center for now.""" + _ = (config, pull_request_id, reason) + raise BitbucketError( + "Bitbucket Data Center pull request reject writes are not supported by this command yet" + ) + + def get_pull_request_discussion(config: BitbucketConfig, pull_request_id: str) -> dict[str, Any]: """Fetch pull request activities from Bitbucket Data Center.""" project_key = quote_path(require(config.project_key, "BITBUCKET_PROJECT_KEY")) diff --git a/tools/bitbucket/src/magpie_bitbucket/normalize.py b/tools/bitbucket/src/magpie_bitbucket/normalize.py index c7c9cf6f1..ccebf1b45 100644 --- a/tools/bitbucket/src/magpie_bitbucket/normalize.py +++ b/tools/bitbucket/src/magpie_bitbucket/normalize.py @@ -299,6 +299,28 @@ def posted_pull_request_review( } +def rejected_pull_request( + kind: str, + raw: dict[str, Any], +) -> dict[str, Any]: + """Normalize a backend-neutral pull-request rejection.""" + comment = raw.get("comment") + normalized_comment = _cloud_comment(comment) if kind == "cloud" and isinstance(comment, dict) else {} + + pull_request = raw.get("pull_request") + + return { + "ok": bool(normalized_comment) and isinstance(pull_request, dict), + "backend": "bitbucket-cloud" if kind == "cloud" else "bitbucket-datacenter", + "operation": "pull-request-reject", + "pull_request_id": _string(raw.get("pull_request_id")), + "reason": _string(raw.get("reason")), + "comment": normalized_comment, + "pull_request": pull_request if isinstance(pull_request, dict) else None, + "raw": raw, + } + + def pull_request_discussion(kind: str, raw: dict[str, Any]) -> dict[str, Any]: """Normalize pull request discussion/comments from Bitbucket.""" values = raw.get("values") diff --git a/tools/bitbucket/tests/test_bitbucket.py b/tools/bitbucket/tests/test_bitbucket.py index 83f96832c..6896d357f 100644 --- a/tools/bitbucket/tests/test_bitbucket.py +++ b/tools/bitbucket/tests/test_bitbucket.py @@ -61,6 +61,7 @@ pull_request_status, pull_request_task, pull_request_tasks, + rejected_pull_request, repository, repository_restrictions, ) @@ -4641,3 +4642,258 @@ def test_cli_pr_review_rejects_invalid_verdict_before_write( assert exc.value.code == 2 mock_post_review.assert_not_called() + + +def test_cloud_reject_pull_request_posts_reason_before_decline( + cloud_env: None, +) -> None: + order: list[str] = [] + + def create_comment(*args: Any) -> dict[str, Any]: + order.append("comment") + return { + "pull_request_id": "7", + "comment": { + "id": 601, + "content": {"raw": "Superseded by #8."}, + "deleted": False, + }, + } + + def decline(*args: Any) -> dict[str, Any]: + order.append("decline") + return { + "pull_request_id": "7", + "pull_request": { + "id": 7, + "state": "DECLINED", + "title": "Example PR", + }, + } + + with ( + patch( + "magpie_bitbucket.cloud.create_pull_request_comment", + side_effect=create_comment, + ), + patch( + "magpie_bitbucket.cloud.decline_pull_request", + side_effect=decline, + ), + ): + result = cloud.reject_pull_request( + load_config(), + "7", + "Superseded by #8.", + ) + + assert order == ["comment", "decline"] + assert result["pull_request_id"] == "7" + assert result["reason"] == "Superseded by #8." + assert result["comment"]["id"] == 601 + assert result["pull_request"]["state"] == "DECLINED" + + +def test_cloud_reject_pull_request_rejects_empty_reason_before_write( + cloud_env: None, +) -> None: + with ( + patch("magpie_bitbucket.cloud.create_pull_request_comment") as mock_comment, + patch("magpie_bitbucket.cloud.decline_pull_request") as mock_decline, + ): + with pytest.raises( + BitbucketError, + match="rejection reason must not be empty", + ): + cloud.reject_pull_request( + load_config(), + "7", + " ", + ) + + mock_comment.assert_not_called() + mock_decline.assert_not_called() + + +def test_cloud_reject_pull_request_reports_partial_failure( + cloud_env: None, +) -> None: + with ( + patch( + "magpie_bitbucket.cloud.create_pull_request_comment", + return_value={ + "pull_request_id": "7", + "comment": { + "id": 601, + "content": {"raw": "Out of scope."}, + "deleted": False, + }, + }, + ), + patch( + "magpie_bitbucket.cloud.decline_pull_request", + side_effect=BitbucketError("decline failed"), + ), + pytest.raises( + BitbucketError, + match="rejection reason was posted", + ) as exc, + ): + cloud.reject_pull_request( + load_config(), + "7", + "Out of scope.", + ) + + assert "before retrying" in str(exc.value) + assert "pr discussion 7" in str(exc.value) + assert "pr get 7" in str(exc.value) + + +def test_datacenter_reject_pull_request_unsupported( + datacenter_env: None, +) -> None: + with pytest.raises( + BitbucketError, + match="Data Center pull request reject writes are not supported", + ): + datacenter.reject_pull_request( + load_config(), + "9", + "Out of scope.", + ) + + +def test_normalize_rejected_pull_request() -> None: + normalized = rejected_pull_request( + "cloud", + { + "pull_request_id": "7", + "reason": "Superseded by #8.", + "comment": { + "id": 601, + "content": {"raw": "Superseded by #8."}, + "user": {"display_name": "Alice"}, + "deleted": False, + }, + "pull_request": { + "id": 7, + "state": "DECLINED", + "title": "Example PR", + }, + }, + ) + + assert normalized["ok"] is True + assert normalized["backend"] == "bitbucket-cloud" + assert normalized["operation"] == "pull-request-reject" + assert normalized["pull_request_id"] == "7" + assert normalized["reason"] == "Superseded by #8." + assert normalized["comment"]["body"] == "Superseded by #8." + assert normalized["pull_request"]["state"] == "DECLINED" + + +@patch("magpie_bitbucket.cloud.reject_pull_request") +def test_cli_pr_reject_cloud( + mock_reject_pull_request: MagicMock, + cloud_env: None, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + reason_file = tmp_path / "reason.md" + reason_file.write_text("Superseded by #8.", encoding="utf-8") + + mock_reject_pull_request.return_value = { + "pull_request_id": "7", + "reason": "Superseded by #8.", + "comment": { + "id": 601, + "content": {"raw": "Superseded by #8."}, + "user": {"display_name": "Alice"}, + "deleted": False, + }, + "pull_request": { + "id": 7, + "state": "DECLINED", + "title": "Example PR", + }, + } + + exit_code = main( + [ + "pr", + "reject", + "7", + "--reason-file", + str(reason_file), + ] + ) + + assert exit_code == 0 + + mock_reject_pull_request.assert_called_once_with( + load_config(), + "7", + "Superseded by #8.", + ) + + output = json.loads(capsys.readouterr().out) + + assert output["ok"] is True + assert output["operation"] == "pull-request-reject" + assert output["reason"] == "Superseded by #8." + assert output["comment"]["body"] == "Superseded by #8." + assert output["pull_request"]["state"] == "DECLINED" + + +@patch("magpie_bitbucket.cloud.reject_pull_request") +def test_cli_pr_reject_rejects_empty_reason_before_write( + mock_reject_pull_request: MagicMock, + cloud_env: None, + tmp_path: Path, +) -> None: + reason_file = tmp_path / "empty-reason.md" + reason_file.write_text(" ", encoding="utf-8") + + with pytest.raises( + BitbucketError, + match="Comment body file must not be empty", + ): + main( + [ + "pr", + "reject", + "7", + "--reason-file", + str(reason_file), + ] + ) + + mock_reject_pull_request.assert_not_called() + + +@patch("magpie_bitbucket.client.urllib.request.build_opener") +def test_cli_pr_reject_datacenter_fails_without_request( + mock_build_opener: MagicMock, + datacenter_env: None, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + reason_file = tmp_path / "reason.md" + reason_file.write_text("Out of scope.", encoding="utf-8") + + exit_code = package_main( + [ + "pr", + "reject", + "9", + "--reason-file", + str(reason_file), + ] + ) + + assert exit_code == 1 + mock_build_opener.assert_not_called() + + stderr = capsys.readouterr().err + assert "reject writes are not supported" in stderr diff --git a/tools/spec-loop/specs/adapters.md b/tools/spec-loop/specs/adapters.md index 5333cd42f..2a70831a7 100644 --- a/tools/spec-loop/specs/adapters.md +++ b/tools/spec-loop/specs/adapters.md @@ -81,7 +81,8 @@ by swapping the adapter, not the skill. state and source commit alongside the `/statuses` build checks, matching the Data Center payload (#1526). Confirmed Cloud-only pull-request writes cover comments, - approve/unapprove, request-changes, decline, and `pr merge --strategy + approve/unapprove, request-changes, decline, backend-neutral `reject`, + and `pr merge --strategy {merge,squash,rebase} --expected-source-commit `, which re-checks the head commit immediately before the merge POST so a moved head fails closed (#1471); it does not enforce approval or build gates itself. @@ -315,8 +316,9 @@ uv run --all-packages --group dev pytest tools/github-rollup/tests mutation. The bridge executes only the confirmed action; current write coverage is Bitbucket Cloud issue-comment creation, Bitbucket Cloud pull-request comment creation, backend-neutral `post_review` through - comment/approve/request-changes, and Bitbucket Cloud pull-request - approve/unapprove, request-changes/remove-request-changes, decline, and + comment/approve/request-changes, backend-neutral `reject`, and Bitbucket + Cloud pull-request approve/unapprove, request-changes/remove-request-changes, + decline, and strategy-aware merge actions pinned to a caller-confirmed source commit (7–40 hex characters); gate checks before a merge are the caller's responsibility via `pr merge-checks`.