[KatalKavya96]

Summary

• Add Bitbucket Data Center support for pull-request approval and change-request state writes.
• Reuse the existing backend-neutral commands:
  • pr approve
  • pr unapprove
  • pr request-changes
  • pr remove-request-changes
• Use the Data Center participant-status API for the authenticated reviewer.
• Add direct API and CLI coverage for all four operations.
• Update Bitbucket capability documentation to reflect Cloud + Data Center support.

Type of change

• Skill change (.claude/skills/<name>/)
• Tool / bridge contract (tools/<system>/*.md)
• Python package (tools/*/ with pyproject.toml)
• Groovy reference impl
• Cross-cutting
• Documentation
• Project template
• CI / dev loop
• Other

Implementation

Bitbucket Data Center review state is updated through the pull-request participant resource for the authenticated user.

The command mappings are:

• pr approve → APPROVED
• pr unapprove → UNAPPROVED
• pr request-changes → NEEDS_WORK
• pr remove-request-changes → UNAPPROVED

The bridge first resolves the authenticated Data Center user's slug, then updates that user's participant state on the target pull request.

This avoids the deprecated approval-specific endpoints and keeps the existing normalized approval/change-request output contracts unchanged across Cloud and Data Center.

Test plan

• Ruff checks pass
• Ruff formatting passes
• Mypy passes
• Focused Data Center review-state tests pass
• Full tools/bitbucket/tests/test_bitbucket.py suite passes
• CLI coverage added for all four Data Center commands
• git diff --check passes
• Repository hooks pass

Scope

This PR adds standalone Data Center review-state mutations only.

The combined:

pr review <id> --verdict {comment,approve,request-changes} --body-file <path>

path remains Cloud-only because Data Center review-body + verdict semantics are a separate follow-up.

Data Center decline/reject and merge writes also remain separate follow-ups.

Refs #606

[KatalKavya96]

Hi @potiuk,

This is the next focused #606 Data Center parity follow-up.

It enables the existing standalone review-state commands on Data Center through the participant-status API:

• approve → APPROVED
• unapprove → UNAPPROVED
• request-changes → NEEDS_WORK
• remove-request-changes → UNAPPROVED

The implementation keeps the existing backend-neutral CLI and normalizers, adds API + CLI coverage for all four operations, and avoids the deprecated approval-specific endpoints.

The combined pr review --body-file path remains Cloud-only and is intentionally left for a separate follow-up.

Refs #606

[Kaap10]

Thank you for implementing Bitbucket Data Center pull request approval writes for #606!

Review Summary

The participant status mapping (PUT .../participants/{userSlug} with APPROVED, UNAPPROVED, NEEDS_WORK) is the correct modern REST API approach for Bitbucket Data Center (Server 4.2+). Output normalization and capability documentation updates in README.md and docs/labels-and-capabilities.md are also well-structured.

However, there is a blocker regarding _current_user_slug that will cause review state writes to fail closed on live Data Center instances:

---

Major Blocker: Schema & Endpoint Mismatch on _current_user_slug

In tools/bitbucket/src/magpie_bitbucket/datacenter.py (lines 361-369):

def _current_user_slug(config: BitbucketConfig) -> str:
    """Return the authenticated Bitbucket Data Center user's URL-safe slug."""
    user = get_json(f"{_api_base(config)}/users", config)
    slug = user.get("slug")

    if not isinstance(slug, str) or not slug.strip():
        raise BitbucketError("Bitbucket Data Center current-user response did not include a user slug")

    return quote_path(slug)

1. Atlassian Data Center REST API v1.0 Contract:

   • In Bitbucket Server / Data Center, GET /rest/api/1.0/users is the paged directory search endpoint (Page<User>).
   • The response schema is:

     {
       "size": 1,
       "limit": 25,
       "isLastPage": true,
       "values": [
         {
           "name": "alice",
           "displayName": "Alice",
           "slug": "alice"
         }
       ],
       "start": 0
     }

   • It contains no top-level slug key. user.get("slug") evaluates to None, which will always raise:
     BitbucketError: Bitbucket Data Center current-user response did not include a user slug
   • Furthermore, /rest/api/1.0/users returns server-wide users alphabetically rather than the caller.

2. Why Unit Tests Passed:

   • In tools/bitbucket/tests/test_bitbucket.py:3050, the test mock configured a flat dictionary:

     mock_opener(
         mock_build_opener,
         {
             "slug": "alice",
             "displayName": "Alice",
             "name": "alice",
         },
         ...
     )

   • This masked the endpoint mismatch.

3. Remediation Options:

   • Extract X-AUSERNAME header: In Bitbucket Server / Data Center, all HTTP responses return the X-AUSERNAME: <username> header identifying the authenticated caller.
   • Support config.username: Allow reading the slug from config.username / BITBUCKET_DATACENTER_USER, with fallback to X-AUSERNAME.

[potiuk]

The participant-status mapping and the docs look right, but the current-user lookup cannot work against a real Data Center instance, so the two open threads from Kaap10 need addressing before this can merge.

current-user lookup (tools/bitbucket/src/magpie_bitbucket/datacenter.py:363)

user = get_json(f"{_api_base(config)}/users", config)
slug = user.get("slug")

Agreeing with the open thread from Kaap10: GET /rest/api/1.0/users is the paged user-directory search ({"values": [...]}), not a current-user endpoint, so slug is always missing and all four new commands fail on a live Data Center instance. Read the authenticated username from the X-AUSERNAME response header (Bitbucket Server/DC sets it on every authenticated response), or from a configured username, and resolve the slug from that.

Smaller observations

• tools/bitbucket/tests/test_bitbucket.py:3050 — the mock returns a flat user object, which is why the suite passes; mock the real response shape of whatever endpoint or header the fix uses

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The findings
below are observations, not blockers; an Apache Magpie
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

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

[potiuk]

Marking this as changes requested, so the PR's status matches where it stands: the points in my earlier review (#1746 (review)) are still open and need a change, or a reply on their threads. This review adds nothing new to them.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. After you've
addressed the points above and pushed an update, an Apache Magpie
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

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

[potiuk]

@KatalKavya96, checking in: the head hasn't moved since 2026-10-21, and the two review threads on the user lookup are still open. On Data Center, GET /rest/api/1.0/users is a paged search that returns {"values": [...], ...}, not a single user, so the code needs to read the match out of values (and handle no match or several). The test mock needs the same paged shape, or it keeps hiding the bug. Please push a fix or reply on the threads, and I'll take another look once the head moves.

---

This comment was drafted by an AI-assisted tool and confirmed by an Apache Magpie maintainer.

[potiuk]

@KatalKavya96, I merged main into this branch (e504c74) to clear the conflict in docs/labels-and-capabilities.md: the Bitbucket row now lists Data Center approve/unapprove and request-changes alongside main's Cloud-only review, decline, reject and merge writes. Please pull before pushing again. The user-lookup point from the earlier review is still open.

---

This comment was drafted by an AI-assisted tool and confirmed by an Apache Magpie maintainer.

[KatalKavya96]

@potiuk @Kaap10 Addressed the current-user lookup review in 52623cb.

The implementation now uses X-AUSERNAME to identify the authenticated Data Center user, resolves the slug through the paged /users response, handles zero/multiple matches, and updates the tests to model the real Page schema with pagination coverage.

I’ve also resolved the two related review threads. Thanks for the detailed review.

[potiuk]

Thanks, the current-user lookup now fails closed: an exact name match over the paged /users search, errors on zero or several matches or a missing slug, and the lookup runs before the participant PUT. The Page<User> test fixtures with the two-page walk make this much more convincing than before.

Two non-blocking follow-ups, fine as a separate PR:

• A test that runs get_json_with_header against a mocked response carrying X-AUSERNAME, and one showing that a failed lookup sends no PUT. Right now the lookup tests patch the helper and the approve tests patch _current_user_slug out.
• Worth checking whether Data Center URL-encodes X-AUSERNAME for usernames with @ or spaces (as Jira does). If it does, an unquote() before the comparison would keep those users working instead of failing closed.

---

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.