Bitbucket PR Review

Bitbucket PR Review

⚠️ 本檔在契約測試底下：skills/bitbucket-pr-mutation/scripts/tests/test_no_raw_bitbucket_writes.py 會讀本檔做內容斷言，並掃描全檔有無 raw curl POST/PUT/DELETE。改完跑： cd <repo-root>/skills/bitbucket-pr-mutation/scripts && python3 -m unittest discover -s tests -q

Inspect Bitbucket Cloud pull request evidence through read-only REST API calls.

完整 PR review 請改用 /pr-review。本 skill 只處理既有 finding、留言、作者回覆、diff 位置或 PR 證據的定點複查，不產出正式 review 報告，也不派報告 auditor。

Config

• Workspace: $BITBUCKET_WORKSPACE (the Bitbucket workspace slug, e.g. your-workspace)
• Email: $BITBUCKET_EMAIL — your Atlassian account email
• Token: Scoped API token exported as BITBUCKET_API_TOKEN
• API Base: https://api.bitbucket.org/2.0

Authentication

Step 0 — ALWAYS use bb_api.sh, NEVER raw curl with hand-crafted auth. The helper handles token loading + Basic Auth + 302 redirects (-sL) in one place. Raw curl with manual -u flags has caused repeated 401s + diff-endpoint-returns-empty bugs.

bash <skill_dir>/scripts/bb_api.sh "<endpoint>"

That's it for read endpoints. Do not paste your own curl -u ... even "just to test"—the failure modes (401 / empty diff / wrong account) are the exact bugs this skill exists to prevent.

Background (only relevant when bb_api.sh itself misbehaves)

• bb_api.sh reads BITBUCKET_API_TOKEN and BITBUCKET_EMAIL env-first, then falls back to grepping the shell secrets files listed in BITBUCKET_SECRETS_FILES (default ~/.zsh_secrets then ~/.zshrc — adjust that list to wherever your own setup keeps exported secrets). Sourcing the rc file instead is not an option: interactive-only rc files routinely fail under non-interactive shells (nvm / compdef / prompt-init errors).
• 🪤 If you see ERROR: BITBUCKET_API_TOKEN not in env or in: ..., the token is genuinely unreadable — that is NOT the same as the token being expired. Run echo ${#BITBUCKET_API_TOKEN} first: a non-zero length means the credential is alive and only the read path is broken. Moving the token between secrets files while helpers still grep the old one produces exactly this error and is repeatedly misdiagnosed as an expired token.
• Atlassian killed Bitbucket app passwords (CHANGE-3222, returns 410). Anything still using the older username + app password pair is dead; only account email + scoped API token works for REST.
• For git push/pull over HTTPS the same token uses a different username (x-bitbucket-api-token-auth), not your account email.

Write endpoints

bb_api.sh only does GET. This review skill never calls POST／PUT／DELETE endpoints. Description updates and comment drafts must be passed to bitbucket-pr-mutation, which owns preview, exact proposal confirmation, pre-write recheck, Apply, and GET read-back. Unsupported mutations stay draft-only; do not fall back to raw curl.

If 401, the scoped API token may have expired. Direct user to regenerate at: https://id.atlassian.com/manage-profile/security/api-tokens (Use "建立有範圍的 API 權杖", must select Bitbucket scopes including repository read and pullrequest read)

Steps

1. Parse PR URL

Extract workspace, repo, and PR ID from the URL: https://bitbucket.org/{workspace}/{repo}/pull-requests/{id}

2. Fetch PR Details

bash <skill_dir>/scripts/bb_api.sh "/repositories/{workspace}/{repo}/pullrequests/{id}"

Parse with python3 to extract: title, state, author, source branch, destination branch, description, full source.commit.hash, full destination.commit.hash, source.repository.uuid, and destination.repository.uuid.

Create one immutable review_input_basis before fetching review inputs:

source_repo_uuid: "{source.repository.uuid}"
source_sha: "{full source.commit.hash}"
destination_repo_uuid: "{destination.repository.uuid}"
destination_sha: "{full destination.commit.hash}"
input_binding: verified
reviewed_at: "{timestamp}"

Set input_binding: verified only when both repository UUIDs are present, both SHA values are full 40-character hashes, and every diffstat／diff／src request below uses this exact commit pair. Otherwise set input_binding: unverified; the focused verdict must not claim a Reviewed SHA.

2.1 Load author calibration file (if present)

Slugify author.display_name (lowercase, spaces → hyphens, e.g. Ada Lovelace → ada-lovelace), then try to read from the target repo's local clone:

<repo-root>/docs/pr-review-calibration/<author-slug>.md

• File exists → load it and keep the author's entries in context; they apply at 6d rule 4 (severity calibration).
• File missing → proceed normally; no calibration applies. Do not create the file. The focused verdict still carries one line:「無作者校準檔（<author-slug>.md 不存在）、本輪無套用」— keeps "ran, no file" distinguishable from "step skipped".

Calibration entries record how a specific author historically responds to review comments (which finding types they accept vs reject). They tune severity and comment phrasing for that author only — they never introduce new findings and never upgrade severity.

3. Fetch Diffstat

bash <skill_dir>/scripts/bb_api.sh "/repositories/{workspace}/{repo}/diffstat/{source_commit}%0D{dest_commit}?from_pullrequest_id={id}&topic=true"

Get source_commit and dest_commit from review_input_basis, not from branch names or a later PR response:

• source_commit = full review_input_basis.source_sha
• dest_commit = full review_input_basis.destination_sha

The exact commit-pair path is {source_commit}%0D{dest_commit}. Do not replace either side with a moving branch ref.

Diffstat 只用來定位使用者點名的檔案與 spec／plan；standalone 定點複查不執行全 PR React-doctor，也不從未點名的檔案產生新 finding。完整機械掃描由 /pr-review 負責。

4. Fetch Diff

bash <skill_dir>/scripts/bb_api.sh "/repositories/{workspace}/{repo}/diff/{source_commit}%0D{dest_commit}?from_pullrequest_id={id}&topic=true&context=5"

If diff is too large, fetch per file using diffstat paths.

5. Fetch Comments (if any)

bash <skill_dir>/scripts/bb_api.sh "/repositories/{workspace}/{repo}/pullrequests/{id}/comments"

6. Focused Follow-up

只針對使用者點名的既有 finding、留言、作者回覆或 diff 位置查證。若請求需要完整逐檔 review、多軸比較或正式報告，停止本流程並改用 /pr-review。

下列 6a–6d 是 /pr-review 引用的查證與校準規則；定點複查只套用與目標 finding 直接相關的部分，不展開正式報告。

6a. Detect Spec / Plan Docs in PR

PRs produced via the Superpowers workflow (brainstorming → writing-plans → executing-plans) often ship with a markdown spec/plan/design doc that declares intent, scope, and explicit non-goals. Reviewers should use these as ground truth for "what this PR is supposed to do" before flagging "missing X".

From the diffstat returned in Step 3, flag a .md file as a spec if ANY of:

• Path contains /specs/, /plans/, /brainstorm/, /design/, /proposals/, .claude/plans/
• Filename matches *-spec.md, *-plan.md, *-design.md, *-brainstorm.md, *-requirements.md, *-proposal.md, or YYYY-MM-DD-*.md
• File starts with frontmatter containing type: plan / type: spec / type: design / phase: / goals: / non_goals:

Excluded (not specs): README.md, CHANGELOG.md, CONTRIBUTING.md, LICENSE.md, SECURITY.md, CODE_OF_CONDUCT.md.

For each detected spec, fetch its full content via:

bash <skill_dir>/scripts/bb_api.sh "/repositories/{workspace}/{repo}/src/{source_commit}/{path_to_spec}"

If combined spec content > ~8000 tokens, summarise before feeding into the review context (keep goals, non-goals, decisions, constraints — drop prose).

Use spec content when applying 6b (search-before-flag context) and 6c (design intent ground truth for refactor PRs): if spec marks something as out-of-scope, do NOT uphold the target finding on that basis. Quote the relevant spec passage in the focused verdict.

Spec-mapping self-check — before citing spec as evidence AGAINST the code: quoting a real spec passage is not enough; the passage must describe the same flow as the code being flagged. Ask "does this spec section's subject (the flow/feature it governs) match the code path I am commenting on?" If the spec section governs flow A (e.g. error handling during redemption) and the flagged code implements flow B (e.g. listing already-redeemed items), do not cite it — the citation would be accurate text applied to the wrong target, which is worse than no citation because spec quotes carry authority. Real miss: a review cited spec lines governing redemption-flow errors against fetch-redeemed-list code; the author correctly rejected it as「不一樣的東西」.

If no spec is detected, note it in the focused verdict ("此 PR 未附 spec／plan 文件") and proceed normally.

After this step, continue to 6b — Context-Gathering Discipline.

6b. Context-Gathering Discipline (MANDATORY before flagging)

Before flagging findings of the form "missing X" / "should handle Y" / "no validation" / "not tested" / "reinventing wheel" / "missing auth check", you MUST verify the gap is real by searching the codebase first. Upstream middleware, framework defaults, existing utilities, and tests in other files often cover what looks missing in the diff alone.

Runtime-behavior assertions are in scope too — findings of the form "this will crash" / "this button does nothing" / "this renders undefined" / "會炸 / 按了沒反應". These assert an observable behavior, and the diff alone cannot prove them: the surrounding machinery often makes the code correct. Before flagging, trace the mechanism the assertion depends on — at minimum whichever of these apply:

• Form library defaults: does useForm defaultValues (or equivalent) already supply the value claimed to be undefined? (Real miss: flagged useWatch destructure crash while DEFAULT_FORM_VALUES provided additionalEntries: {})
• Native platform behavior: is the "dead" button inside a <form> where its default type="submit" triggers the parent onSubmit? A sibling button carrying explicit type="button" is a strong signal the omission is deliberate. (Real miss: flagged missing handleSave while the button submitted the enclosing form)
• Framework/library internals: does the claimed failure mode match what the library actually does? Read the library source or docs when the assertion hinges on it (e.g. what a provider renders before activation). (Real miss: described "flash of English" where Lingui actually renders null pre-activate)

The finding must state which mechanism was traced and why the assertion still holds. If tracing shows the behavior is covered → drop the finding silently, same as the "missing X" rule.

Suggested-fix embedded assumptions are in scope too — a finding that survives verification can still get refuted through its suggested fix: fix text routinely asserts behavior of paths / APIs / options the diff never touched（「把 Y 分支也加上 X」隱含「Y 支援 X」）. The finding carries an evidence chain while the fix rides along unverified; when the author refutes the fix half, the whole comment's credibility goes down with it. Before attaching a suggested fix, verify every path / API behavior assumption embedded in it with the same search tools below (or library docs / source); where verification isn't feasible, narrow the fix to the verified branch and phrase the rest as「需確認 X 是否支援 Y」instead of asserting it. (Real miss: finding "classic 登入掉 locale 前綴" was correct and accepted, but the suggested fix also prefixed /customer_authentication/login on the unverified assumption that the path accepts a locale prefix — it 404s, Shopify handles locale itself; the author refuted the fix half)

Mechanism-chain claims must be verified station-by-station — a finding whose argument is a causal chain（「line N 的 filter 會把 X 濾掉 → 判定函式回 false → 卡在未完成」）is only as strong as its weakest link. Citing a file:line is not verification — misreading the cited line is exactly how the chain breaks. Before flagging, read every function the chain passes through and confirm its actual semantics: what a filter predicate actually excludes (the element itself vs a field of the element), what a completion/validation predicate actually compares (hasOwnProperty checks key existence, not value). The finding must quote the chain's key predicates, not just their line numbers. (Real miss: flagged "switching the primary entry leaves the base tier stuck incomplete" citing a filter line, but that filter excludes the base tier from the rewrite path rather than filtering the tier's entries, and the completion predicate uses hasOwnProperty — key existence, not value — which passes for the all-zero structure)