Repository navigation
feat(bitbucket): add post-review contract support - #1591
Conversation
|
Hi @potiuk — next narrow #606 follow-up after #1577. This adds Bitbucket Cloud 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 |
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
left a comment
There was a problem hiding this comment.
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.
Summary
post_review(id, verdict, body) -> okchange-request contract throughpr review <id> --verdict {comment,approve,request-changes} --body-file <path>.Type of change
.claude/skills/<name>/) — eval fixtures updated belowtools/<system>/*.md)tools/*/withpyproject.toml)docs/,README.md,CONTRIBUTING.md)projects/_template/)prek, workflows, validators)Test plan
prek run --all-filespassesuv run pytest/ruff check/mypypasses(
PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)(a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)
post_reviewbackend / normalization / CLI tests passgit diff --checkpassesprek run --all-filescurrently fails only in the unrelatedskill-script-testshook becauseplugins/magpie-pr-management/skills/stack-review/testsreportsNO TESTS RAN; no files under that area are changed by this PRRFC-AI-0004 compliance
post_reviewchange-request contract withcomment,approve, andrequest-changesverdictsLinked 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
approveandrequest-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 discussionandpr reviewsbefore 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.