feat(bitbucket): add read-only PR discussion fetch

[KatalKavya96]

Summary

• Adds magpie-bitbucket pr discussion <id> as a comments-only, read-only get_discussion subset so Magpie can fetch PR discussion without adding write paths.
• Fetches Bitbucket Cloud PR comments and Bitbucket Data Center comment-bearing PR activities with pagination, while rejecting unsafe or repeated Cloud pagination URLs before reusing credentials.
• Normalizes discussion output into contract-shaped comments[], participants[], unresolved_count, kind: comment, common inline fields, and updates README/spec wording for external PR content and privacy handling.

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: verified targeted Bitbucket tests with PYTHONPATH=src uv run --group dev pytest tests/test_bitbucket.py

RFC-AI-0004 compliance

• HITL — any new mutation is gated on explicit user confirmation
• Sandbox — no new unrestricted host access; network reach declared in the adapter
• Vendor neutrality — extends the Bitbucket adapter using the existing contract:change-request + Coverage: partial-read-only model without introducing a Bitbucket-specific capability.
• Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
• Write-access discipline — this PR adds read-only discussion fetching only; it does not add comment creation, review, approve, decline, merge, or other mutation paths.
• Privacy LLM — README/spec wording states that fetched PR descriptions, comments, and raw Bitbucket payloads are external data, never agent instructions, and that private or embargoed repository content must follow the approved-LLM/privacy gate before model use.

Linked issues

Refs #606

Notes for reviewers (optional)

This is the next read-only step after the initial Bitbucket bridge and the partial-read-only coverage clarification.

The new command is magpie-bitbucket pr discussion <id>, intended to represent a comments-only subset of the get_discussion surface while keeping the bridge inside Coverage: partial-read-only. Bitbucket Cloud uses PR comments. Bitbucket Data Center uses PR activities, but filters to comment-bearing / COMMENTED activities only.

Normalized discussion output now includes comments[], participants[], and unresolved_count. Each comment includes id, author, date, created, updated, body, kind: comment, deleted, inline, and raw. Inline locations are normalized to a common subset such as path, from_line, and to_line.

Out of scope for this PR: posting comments, reviews, approve/decline/merge operations, Jira handoff, tracker operations, branch permissions, Pipelines status, full activity/event mapping, and unresolved-thread accounting.

[KatalKavya96]

Hi @potiuk I opened the next Bitbucket follow-up PR for the read-only get_discussion surface.

This adds magpie-bitbucket pr discussion <id> as a comments-only subset under the existing Coverage: partial-read-only model. Bitbucket Cloud uses PR comments, while Bitbucket Data Center uses PR activities but filters to comment-bearing / COMMENTED activities only, so lifecycle events like approvals or merges are not normalised as comments.

I also tightened the normalised output to match the discussion contract shape more closely: top-level comments[], participants[], and unresolved_count, with each comment carrying author, date, body, and kind: comment, plus useful extensions like id, created, updated, deleted, inline, and raw.

After a local review pass, I also added guarded Cloud pagination so response-controlled next URLs cannot switch host/scheme or repeat before credentials are reused. README/spec wording now calls this a partial comments-only discussion subset and notes that fetched PR descriptions, comments, and raw Bitbucket payloads are external data, never agent instructions.

prek run --all-files is passing.

[justinmclean]

I ran the review skill over this - looks like there one thing that needs to be fixed:

Robustness / API correctness — tools/bitbucket/src/magpie_bitbucket/datacenter.py get_pull_request_discussion: the DC while True loop has no guard against a non-advancing cursor. If /activities returns isLastPage: false with a nextPageStart that never increases, it re-requests the same ?start=N forever. The Cloud path in this same PR added seen_urls repeat-detection for exactly this; the DC path has no analog. Suggest a next_start <= start → break/raise guard to keep the two backends symmetric.

Docs / code quality — tools/spec-loop/specs/adapters.md: the new security note was wedged mid-sentence, so the bullet now reads "tools/bitbucket/ Fetched Bitbucket descriptions… before model use. currently provides read-only repository metadata…". Split it into its own sentence/bullet after the coverage description.

Minor (non-blocking) — normalize.py: _cloud_inline and _datacenter_inline are near-identical copy-paste (DC only adds a line fallback); a shared helper would remove the duplication.

Everything else checks out against the surrounding code: the _datacenter_user fallback, empty-raw-body preservation, the COMMENTED action filter, and the Cloud host/scheme + repeat-URL guards.

This review was AI-generated so usual caution apply.

[KatalKavya96]

@justinmclean Thanks, fixed both required points.

I added a Data Center pagination guard for get_pull_request_discussion so a non-advancing nextPageStart cannot loop forever, and added a regression test covering that case.

I also cleaned up the tools/spec-loop/specs/adapters.md Bitbucket note so the external-data/privacy warning is its own bullet instead of being wedged into the coverage sentence, and updated the adapter inventory wording to include the comments-only discussion fetch.

prek run --all-files is passing.

[potiuk]

Approving — a clean, security-conscious, read-only addition that follows the existing bitbucket adapter conventions and is well-tested. Particular credit for cloud.py:_validated_next_url: rejecting scheme/host-changing and repeated pagination next URLs before reattaching credentials is real defense-in-depth against redirect-based credential leakage and pagination loops.

A few non-blocking follow-ups you might fold in now or in a fast follow — none block the merge, left inline:

• Data Center threaded replies (nested comment.comments[]) are silently dropped — a Cloud-parity gap.
• _cloud_comment is typed … | None but never returns None.
• The DC discussion path fetches the full /activities feed and filters to comments afterward.
• Test symmetry: the CLI dispatch test only covers Cloud; a DC CLI test would close the gap.

Nice work — thanks @KatalKavya96.

---

This review was drafted by an AI-assisted tool and confirmed by a 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 Magpie handles maintainer review: CONTRIBUTING.md.

[KatalKavya96]

@potiuk Thanks for the review and approval.

I folded in the non-blocking follow-ups while keeping the PR read-only and within the existing partial-read-only scope:

• Data Center nested comment.comments[] replies are now normalized instead of being silently dropped.
• _cloud_comment() now has a non-optional return type since it always returns a normalized comment.
• Added a code comment in the Data Center discussion fetch path explaining that /activities is the broader feed and comment filtering happens during normalization.
• Added a Data Center CLI dispatch test for pr discussion <id>.

prek run --all-files is passing.

[KatalKavya96]

Hi @potiuk Could you please confirm whether the PR is good to merge?, so I can proceed with the next implementation. Thanks!

[potiuk]

Re-approving on the updated code. The 053c3ef5 fix resolves my earlier finding: Data Center threaded replies are now normalized via _datacenter_comment_tree (recurses comment.comments[], flattens the tree with a parent_id link), with tests — so DC reply coverage now matches Cloud. The _cloud_comment Optional-return and single-element list-comprehension nits are also cleaned up. Nice work @KatalKavya96.

One non-blocking nit for a follow-up: in _datacenter_comment_tree, raw.get("comments", []) still crashes if the API returns an explicit "comments": null (the [] default only applies to a missing key) — raw.get("comments") or [] is safer.

---

This review was drafted by an AI-assisted tool and confirmed by a Magpie maintainer. The maintainer approving this PR has read the findings and signed off.

More on how Magpie handles maintainer review: CONTRIBUTING.md.

[potiuk]

Merged — thanks @KatalKavya96, and thanks for turning the review around so quickly (the Data Center threaded-reply handling via _datacenter_comment_tree is a clean fix).

Two optional, non-blocking follow-ups for whenever you're next in this code — neither blocked the merge:

• Optimization: the Data Center discussion path fetches the full /activities feed and filters down to comments afterward, so a PR with lots of non-comment activity pulls every page regardless. If the DC API supports a server-side comment filter, using it would avoid the wasted pagination.
• Robustness: in _datacenter_comment_tree, raw.get("comments", []) would still raise on an explicit "comments": null (the [] default only applies to a missing key) — raw.get("comments") or [] handles both.