Repository navigation
fix(bitbucket): PR sync stops resending diffstat, conflicts, statuses and permission every sweep - #27596
fix(bitbucket): PR sync stops resending diffstat, conflicts, statuses and permission every sweep#27596juliusmarminge wants to merge 4 commits into
Conversation
…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>
ApprovabilityVerdict: Approved at 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. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 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.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
No actionable comments were generated in the recent review.
|
| 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:
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
Possibly related PRs
- pingdotgg/t3code#27100: Also reuses pull-request statistics for unchanged commits.
- pingdotgg/t3code#24392: Adds the provider invalidation operation implemented here for Bitbucket.
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: 
4
-
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
left a comment
There was a problem hiding this comment.
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.
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_URLat 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.forgetChangeRequestReads.unknown. A 429 or the host-wide pause still fails the read as rate-limited.The detail panel's
getChangeRequestis 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 againfails 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 failedcovers 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.T3CODE_DEV_SEED=0 vp run dev --no-seed, fresh database), with the fake Bitbucket API logging every request.link_pull_request, recently updated so they are read every sweep.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.
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