Skip to content

fix(bitbucket): PR sync stops resending diffstat, conflicts, statuses and permission every sweep - #27596

Open
juliusmarminge wants to merge 4 commits into
mainfrom
fix/issue-27578
Open

juliusmarminge wants to merge 4 commits into
mainfrom
fix/issue-27578

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

Pull request sync sent 5 Bitbucket API requests for every linked pull request on every sweep: the pull request, diffstat, conflicts, statuses and repository permission. Bitbucket has no getChangeRequestSummary, so the sync summary read fell back to the full detail read, including statuses and permission, which a summary never uses. With 12 recently updated pull requests on the one-minute sweep that is about 3,600 requests an hour. Bitbucket allows 1,000, so the host stays paused and every Bitbucket feature fails.

Reproduced on main with the real dev server, pointed through T3CODE_BITBUCKET_API_BASE_URL at a local fake Bitbucket API that logs requests. With 12 linked pull requests, every sweep sent 60 requests.

Change

The Bitbucket provider now has its own getChangeRequestSummary. It reads the pull request on every sweep. It reuses diffstat and conflicts while the pull request's source and destination commits are unchanged, and never requests statuses or repository permission. The summary carries the same fields as before.

  • What is held. Diffstat and conflicts, per pull request and commit pair, for at most 15 minutes. The time limit covers a destination branch that moves without the pull request's destination commit following it.
  • What drops a held read early. A refresh or a finished turn, through forgetChangeRequestReads.
  • What is never held. Failed reads, and a conflicts read that failed and was recovered as unknown. A 429 or the host-wide pause still fails the read as rate-limited.
  • No commit hash. A pull request without either hash reads diffstat and conflicts directly.

The detail panel's getChangeRequest is unchanged.

Scope and approval

Fixes #27578

Verification

  • vp test run packages/source-control-bitbucket: 6 files, 92 tests pass.
    • rereads only the pull request when sync reads an unchanged pull request again fails on main: the first and second reads each also call conflicts, diffstat, permission and statuses.
    • rereads diffstat and conflicts when a commit moves, the reads expire or are forgotten, or conflicts failed covers a destination commit move, a source commit move, a forget with a differently cased repository, the 15-minute expiry (TestClock), and a failed conflicts read not being held. With the expiry removed it fails, 6 reads instead of 7.
  • vp run check:changed: passes.
  • Runtime transcript, identical setup on both sides:
    • The real dev server (T3CODE_DEV_SEED=0 vp run dev --no-seed, fresh database), with the fake Bitbucket API logging every request.
    • 12 synthetic pull requests linked to 4 threads through the server's MCP link_pull_request, recently updated so they are read every sweep.
    • The first line of each side is the link-time read.
== Before: main 32ecd71726 ==
12:39:31 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:41:29 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:42:29 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:43:30 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:44:30 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12

== After: fix/issue-27578 a5fc36dcfb ==
12:54:04 UTC  requests= 36  pull_requests=12  conflicts=12 diffstat=12 pullrequest=12
12:56:01 UTC  requests= 12  pull_requests=12  pullrequest=12
12:57:02 UTC  requests= 12  pull_requests=12  pullrequest=12
12:58:02 UTC  requests= 14  pull_requests=12  conflicts=1 diffstat=1 pullrequest=12   (new source commit pushed to #1 at 12:57:09)
12:59:02 UTC  requests= 12  pull_requests=12  pullrequest=12
13:00:02 UTC  requests= 12  pull_requests=12  pullrequest=12

Steady state drops from 60 requests a sweep to 12, about 720 an hour. The push rereads diffstat and conflicts for that pull request only.

  • Not checked:
    • Real Bitbucket Cloud, since no account is available. The fake API serves the same JSON shapes the decoders read.
    • The fake API, seed driver and request logs are not attached: the upload tool was unavailable on the machine that ran them.

Model and harness: Claude Opus 5.5 (Claude Code) as issue lead, red test and implementation; reviews by GPT-6.1 Sol (Codex) and Claude Opus 5.5.

🤖 Generated with Claude Code

juliusmarminge and others added 4 commits October 30, 2026 12:30
…a (reproduces #27578)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts are unchanged

Bitbucket had no summary read, so every sync sweep ran the full detail read
for each pull request: the pull request, diffstat, conflicts, statuses and
repository permission, though the summary uses neither statuses nor
permission.

Add `getChangeRequestSummary`, which reads the pull request and reuses the
diffstat and conflicts held for the same source and destination commits.
Reads are not held when either commit hash is missing, when they fail, or
when the conflicts read fell back to `unknown`. `forgetChangeRequestReads`
drops them on a refresh or a finished turn. The detail read is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bitbucket may leave a pull request's destination commit in place while the
destination branch moves, so conflicts keyed on the two commits could stay
stale until a refresh. The test also covers a recovered conflicts failure,
which is never held.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 30, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a58901b

Macroscope's review found this PR approvable — Bitbucket sync avoids redundant API requests by caching commit-dependent summary data while leaving full detail reads unchanged. Commit-based keys, a 15-minute expiry and refresh invalidation limit stale summaries without changing permissions or merge behavior.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.6 KiB 21.6 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 6575ce3 · PR result: a58901b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 6575ce3 and a58901b.

Files selected for processing: 3
  • packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts
  • packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts
  • packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

Bitbucket pull-request summaries now reuse diff-stat and mergeability results when both commit SHAs are available. The parsed pull-request model now includes the destination commit SHA as baseSha.

Changes: Bitbucket summary cache

Layer / File(s) Summary
Commit-keyed summary cache and invalidation
packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts, packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts, packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts
The pull-request data includes a trimmed destination commit SHA as baseSha. The provider caches diff statistics and mergeability by repository, pull-request number, and both commit SHAs, with a 15-minute TTL and a 256-entry limit. It reads these values directly when either SHA is unavailable. Cache invalidation can clear all entries or match a repository and optional pull-request number. Tests cover reuse, changed commits, invalidation, expiration, and mergeability read failures.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Sequence Diagram(s)

sequenceDiagram
  participant Provider as BitbucketPullRequestProvider
  participant API as Bitbucket API
  participant Cache as Summary cache
  Provider->>API: Get pull request and commit SHAs
  Provider->>Cache: Look up summary by repository, pull request, and SHAs
  alt Cache hit
    Cache-->>Provider: Return diff statistics and mergeability
  else Cache miss
    Provider->>API: Read diff statistics and mergeability
    API-->>Provider: Return summary data
    Provider->>Cache: Store successful result with expiration
  end
Loading

Possibly related PRs

Suggested labels: bugfix

Suggested reviewers: maria-rcks, macroscopeapp

Merge Risk: Minimal · up to a5890

Summary reads reduce repeated Bitbucket requests. No material merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: reducing repeated Bitbucket pull-request sync reads for diffstat, conflicts, statuses, and permission.
Description check Passed The description includes Problem, Change, Scope and approval, and Verification sections. It explains the issue, implementation, scope, tests, runtime results, and limitations in sufficient detail.
Linked Issues check Passed [#27578] Reduce sync reads per Bitbucket pull request. Implemented. The new getChangeRequestSummary in BitbucketPullRequestProvider.ts reads only the pull request on each sweep. The diff shows no stat…
Out of Scope Changes check Passed All changes support the Bitbucket summary read fix for #27578. In BitbucketPullRequestProvider.ts, the Cache, Duration and Exit imports, COMMIT_READ_CAPACITY and COMMIT_READ_TTL, readCommitStats, and …
Full details: Linked Issues check

Explanation

[#27578] Reduce sync reads per Bitbucket pull request. Implemented. The new getChangeRequestSummary in BitbucketPullRequestProvider.ts reads only the pull request on each sweep. The diff shows no statuses or repository-permission call in the summary path. Diffstat and conflicts are read through a Cache keyed by repository, number, headSha and baseSha, so they reread only when a commit moves. The detail-panel getChangeRequest is not changed in the diff. Author-reported transcript: steady state falls from 60 to 12 requests per sweep for 12 linked pull requests. I did not run the transcript or the tests.

[#27578] Repository permission read once per repository. The summary path reads permission zero times, so no per-pull-request permission read remains on sync. The issue wording asks for one read per repository. This PR removes the sync read instead. I treat this as meeting the intent of the requirement.

[#27578] Diffstat and conflicts reread only when the head commit changes. Partly met. Rereads also occur when the 15-minute time-to-live expires and when forgetChangeRequestReads runs on refresh or a finished turn. The 15-minute expiry is justified in the PR for a destination branch that moves without the pull request's destination commit following. This goes beyond the literal 'only when the head commit changes' wording. The extra rereads are bounded.

[#27578] Few requests per sync through a batched list read (GET /repositories/{workspace}/{repo}/pullrequests). Not implemented. Each linked pull request still receives one pull-request read per sweep (12 per sweep in the transcript). The issue presents the list endpoint as sufficient for change detection. The PR objectives state that the list-endpoint strategy is not implemented. I do not treat this as a failed requirement, but the residual per-pull-request read is a known gap against the 'few requests' wording.

Unverified: real Bitbucket Cloud behavior. The author reports the fake API was used.

Full details: Out of Scope Changes check

Explanation

All changes support the Bitbucket summary read fix for #27578. In BitbucketPullRequestProvider.ts, the Cache, Duration and Exit imports, COMMIT_READ_CAPACITY and COMMIT_READ_TTL, readCommitStats, and commitStats serve the held diffstat and conflicts reads. getChangeRequestSummary implements the new summary read. forgetChangeRequestReads invalidates held reads on refresh or a finished turn. In bitbucketPullRequestJson.ts, the nullable baseSha field supplies the destination commit hash needed for the cache key. The test changes cover the reread, expiry, forget and failed-read rules. The diff shows no unrelated change. The change stays in the Bitbucket provider package. The issue's Area label is apps/server, but the server consumes this provider, so the location does not indicate out-of-scope work.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@aravhawk aravhawk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about the failed-conflicts path in getChangeRequestSummary.

readCommitStats reads diffstat and conflicts together, and the cache's timeToLive returns Duration.zero whenever exit.value[1] === null. recoverRead turns every non-429 error from /conflicts into that null: 403, 404, decode errors, request errors and so on. So if a PR's conflicts read fails every time instead of once, its entry is never held. Sync then goes back to pull request + diffstat (every page) + conflicts on each sweep for that PR, which is 3+ requests instead of 1, and the diffstat that did succeed gets thrown away as well.

Can /conflicts fail persistently on Bitbucket Cloud? For example, merged or declined PRs that sync still reads because they were recently updated. If it can, would it be worth caching diffstat under its own key and leaving only the conflicts retry uncached? Or holding a null for a short TTL (a minute or two), so it still retries without paying for diffstat on every sweep? The new test only covers a single 503 that recovers on the next read, so it wouldn't catch the persistent case either way.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Bitbucket sends 5 requests per pull request on every sync

2 participants