Conversation
easingthemes
left a comment
easingthemes
left a comment
There was a problem hiding this comment.
Automated review of the Bitbucket support changes. Overall a clean, additive implementation — the ADO path is untouched and the API-difference handling (flat comments grouped by parent.id, Cloud inline.* vs DC anchor.*, vote clamping on all platforms) is accurate. A few robustness notes inline, plus two non-blocking process items:
- PR title is
Fix— this is afeat(new Bitbucket support, minor bump). Since the repo runs semantic-release on conventional-commit prefixes, a squash-merge ofFixwon't trigger the intended minor release. Suggestfeat: add Bitbucket Cloud + DC support to dx-pr-review/answer/init. - Description lists
docs/bitbucket-pr-review-support.mdin the Files Changed table, but it's not in the diff (10 files changed, 11 rows). Verify whether that doc was meant to ship.
Nothing blocking — recommend addressing the unanchored-grep parsing (flagged inline, it recurs in 4 files) before merge.
| 2. `scm.bitbucket-token` from `.ai/config.yaml` | ||
|
|
||
| ```bash | ||
| TOKEN="${BITBUCKET_TOKEN:-$(grep 'bitbucket-token:' .ai/config.yaml 2>/dev/null | awk '{print $2}' | tr -d '"')}" |
There was a problem hiding this comment.
[Medium] Unanchored grep matches commented template lines.
The generated config ships bitbucket-token/bitbucket-host as commented examples (# bitbucket-token: ""). grep 'bitbucket-token:' matches that comment, and awk '{print $2}' on # bitbucket-token: "" returns the literal bitbucket-token:.
Failure mode: a Bitbucket project where the user relies on BITBUCKET_TOKEN but it's unset and the line stays commented → TOKEN becomes the garbage string bitbucket-token: (non-empty), so the [ -z "$TOKEN" ] guard passes and curl sends Authorization: Bearer bitbucket-token: → an opaque 401 instead of the intended clear "token not found, exit 2".
Same hazard for BB_HOST: if a real value coexists with the example comment, grep returns two lines and BB_BASE becomes a malformed multi-line URL.
This grep pattern recurs in dx-pr-review/SKILL.md, dx-pr-answer/SKILL.md, and post-findings.md. Fix it here (the shared source of truth) and reference it:
TOKEN="${BITBUCKET_TOKEN:-$(grep -E '^[[:space:]]*bitbucket-token:' .ai/config.yaml 2>/dev/null | head -1 | awk '{print $2}' | tr -d '\"')}"| ```bash | ||
| # Resolve slug from email | ||
| USER_SLUG=$(curl -s -H "Authorization: Bearer $TOKEN" \ | ||
| "$BASE/users?filter={email}" | jq -r '.values[0].slug') |
There was a problem hiding this comment.
[Low] Email isn't URL-encoded in the ?filter= query param. @ and + are unencoded query values here. Most DC instances tolerate raw @, but it's fragile. Also worth a note that the subsequent participant PUT /participants/{slug} assumes the user is already a participant/reviewer on the PR — some DC versions reject the status change otherwise.
| If no URL was provided (PR ID only), read `scm.provider` from `.ai/config.yaml`: | ||
|
|
||
| ```bash | ||
| PLATFORM=$(grep 'provider:' .ai/config.yaml | head -1 | awk '{print $2}' | tr -d '"') |
There was a problem hiding this comment.
[Low] PLATFORM detection relies on scm.provider being the first provider: in the file. Works today only because scm: precedes tracker: in the template, and grep also matches commented # provider: lines. It silently breaks if blocks are reordered or scm.provider is commented. Suggest anchoring: grep -E '^[[:space:]]*provider:' ... | head -1 (or scope to the scm: block).
| For Bitbucket DC, also read the host: | ||
|
|
||
| ```bash | ||
| BB_HOST=$(grep 'bitbucket-host:' .ai/config.yaml | awk '{print $2}' | tr -d '"') |
There was a problem hiding this comment.
Same unanchored-grep issue as flagged in shared/bitbucket-config.md (line 37): grep 'bitbucket-host:' will match the commented template example. If both the example comment and a real value are present, BB_HOST captures two lines → malformed BB_BASE. Use grep -E '^[[:space:]]*bitbucket-host:' ... | head -1.
2 participants
PR Description
What does this PR do?
Extends
dx-pr-review,dx-pr-answer, anddx-initto support Bitbucket Cloud and Bitbucket Data Center alongside the existing Azure DevOps path. Adds platform detection, per-platform API branching, a shared Bitbucket REST reference, and auto-detection of Bitbucket remotes indetect-env.sh.Why?
dx-pr-reviewanddx-pr-answerwere hardwired to Azure DevOps — every API call usedmcp__ado__*tools. Teams on Bitbucket Cloud or Bitbucket DC got an immediate failure with no fallback. Since AEM projects frequently live on Bitbucket, this blocked a large share of users from the core PR workflow entirely.What changed
Platform detection added to steps 1, 2, 3, 8, 9 of
dx-pr-reviewand steps 1–3, 5, 7 ofdx-pr-answer.Each step that previously made an unconditional ADO MCP call now detects
PLATFORMfrom the URL orscm.providerconfig and branches: ADO keeps its existing MCP path unchanged; Bitbucket usescurlwith a bearer token (BITBUCKET_TOKENenv →scm.bitbucket-tokenconfig).Key API differences handled:
parent.idinline.path/to; DC usesanchor.line/lineType/fileType/pathPOST /approveorPOST /request-changes; DC =POST /approveorPUT /participants/{slug}withstatus: NEEDS_WORKauthor.nicknameonlyType of Change
Checklist
docs/reference/config-reference.mdchmod +xFiles Changed
shared/bitbucket-config.mdskills/dx-pr-review/SKILL.mdskills/dx-pr-review/references/post-findings.mdskills/dx-pr-answer/SKILL.mdskills/dx-pr-answer/references/apply-fixes.mdshared/provider-config.mdtemplates/config.yaml.templaterepos[]workspace/project fieldsskills/dx-init/scripts/detect-env.shskills/dx-init/SKILL.mdscm.providervstracker.providerconflation bugdocs/reference/config-reference.mddocs/bitbucket-pr-review-support.mdKnown Gaps
discover-review-queue.shis ADO-onlydx-doctordoesn't validate Bitbucket configbitbucket-hostorBITBUCKET_TOKENwon't be caughtBITBUCKET_USERNAME/scm.bitbucket-username— Cloud doesn't expose author emaildx-pr/dx-pr-commitPR creationEnvironment Tested