Skip to content

feat(bitbucket): add post-review contract support - #1591

Merged
potiuk merged 2 commits into
apache:mainfrom
KatalKavya96:feat-bitbucket-post-review
Oct 10, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
KatalKavya96:feat-bitbucket-post-review

Conversation

@KatalKavya96

Copy link
Copy Markdown
Contributor

Summary

  • Adds Bitbucket Cloud support for the backend-neutral post_review(id, verdict, body) -> ok change-request contract through pr review <id> --verdict {comment,approve,request-changes} --body-file <path>.
  • Reuses the existing Cloud comment / approve / request-changes primitives while preserving caller-side explicit confirmation and keeping Bitbucket Data Center fail-closed.
  • Handles Cloud's two-step approve/request-changes flow safely: the review body is posted first, and if the verdict mutation then fails, the command reports the partial outcome and tells callers to inspect the PR before retrying to avoid duplicate comments.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other:

Test plan

  • prek run --all-files passes
  • For Python packages touched: uv run pytest / ruff check / mypy passes
  • For Groovy bridges touched: command-line invocation tested end-to-end
  • For skill changes: eval suite passes for the affected skill
    (PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)
  • For skill behaviour changes: a new or updated eval fixture is included in this PR
    (a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)
  • Other:
    • focused post_review backend / normalization / CLI tests pass
    • full Bitbucket test suite passes
    • git diff --check passes
    • workspace Ruff, formatting, mypy, pytest, validators, and spec checks pass
    • prek run --all-files currently fails only in the unrelated skill-script-tests hook because plugins/magpie-pr-management/skills/stack-review/tests reports NO TESTS RAN; no files under that area are changed by this PR

RFC-AI-0004 compliance

  • HITL — the new review mutation remains gated on explicit caller-side user confirmation
  • Sandbox — no new unrestricted host access; the implementation reuses the existing guarded Bitbucket Cloud write path
  • Vendor neutrality — the Bitbucket-specific operations are exposed through the generic post_review change-request contract with comment, approve, and request-changes verdicts
  • Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
  • Write-access discipline — the bridge executes only an already-confirmed review; it does not decide or post reviews autonomously
  • Privacy LLM — private content does not reach a non-approved LLM; redactor invoked where needed

Linked issues

Refs #606

Notes for reviewers (optional)

Bitbucket Cloud does not expose the review body and approval/change-request verdict as one atomic mutation. For approve and request-changes, this adapter therefore posts the confirmed review body first and applies the verdict second.

If that second request fails, the command fails with an explicit partial-outcome warning directing the caller to inspect pr discussion and pr reviews before retrying. This avoids silently retrying the whole operation and duplicating the already-posted review comment.

Bitbucket Data Center has a different review API and remains explicitly unsupported here to keep this follow-up narrow.

@github-actions github-actions Bot added family:tools tools/* family:docs Docs, MISSION.md, READMEs contract:tracker Tool capability: issue / board / label backend contract:change-request Tool capability: proposed-change review + merge gate (PR / MR / Gerrit change) labels Oct 10, 2026
@KatalKavya96

Copy link
Copy Markdown
Contributor Author

Hi @potiuk — next narrow #606 follow-up after #1577.

This adds Bitbucket Cloud post_review(id, verdict, body) support via pr review, covering comment, approve, and request-changes.

For approve/request-changes, the review body is posted first and the verdict second; if the second step fails, the command reports the partial outcome and tells the caller to inspect before retrying to avoid duplicate comments. Data Center remains fail-closed.

Focused/full Bitbucket tests, Ruff, mypy, workspace pytest, validators, and spec checks are green. The only prek --all-files failure is the unrelated stack-review/tests “NO TESTS RAN” sibling-script hook.

Both rows still said they do not implement the post_review contract;
pr review now does, so they point callers at it for a review body
together with a verdict.

Generated-by: Claude Opus 5
Claude-Session: https://claude.ai/code/session_01GTPsZ5aEE5bUVYqp47Dpvv

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — post_review is built from the existing, already-reviewed comment / approve / request-changes primitives, the verdict is validated before any write, and the two-step approve / request-changes case is handled the right way: a failed verdict after the body was posted is reported as a partial outcome with an "inspect before retrying" pointer, so a retry cannot duplicate the comment. Data Center fails closed before any request. CI is green.

One nit, which I pushed as a follow-up commit on your branch (94daf3a) rather than asking for another round: the README's pr approve and pr request-changes rows still said they do "not implement the full post_review contract surface", which this PR now does; they now point at pr review.

On the test-plan note: plugins/magpie-pr-management/skills/stack-review/tests no longer exists on main (#1582 moved those tests into tools/pr-management), so the NO TESTS RAN failure is a leftover untracked directory in your checkout — git clean -ndX there will show it.


This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
Contributing guide.

@potiuk
potiuk merged commit 52398ab into apache:main Oct 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contract:change-request Tool capability: proposed-change review + merge gate (PR / MR / Gerrit change) contract:tracker Tool capability: issue / board / label backend family:docs Docs, MISSION.md, READMEs family:tools tools/*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants