From 921674c5e1339732f1b61abb84f0af79df61542c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 30 Oct 2026 12:30:26 +0000 Subject: [PATCH 1/3] test(bitbucket): sync summary reads resend unchanged pull request data (reproduces #27578) Co-Authored-By: Claude Opus 5.5 --- .../BitbucketPullRequestProvider.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts index 9bdf13598d9e2..a98222ac19303 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts @@ -178,6 +178,51 @@ it.effect("reads checks and PR state without diff, mergeability, or permission r }), ); +it.effect("rereads only the pull request when sync reads an unchanged pull request again", () => + Effect.gen(function* () { + const pullRequest = Result.getOrThrow( + decodePullRequestJson(`{ + "id": 1, "title": "Sync", "state": "OPEN", + "source": { "branch": { "name": "feature" }, "commit": { "hash": "abc123" } }, + "destination": { "branch": { "name": "main" } }, + "created_on": "2026-09-16T00:00:00Z", "updated_on": "2026-09-16T00:00:00Z", + "links": { "html": { "href": "https://bitbucket.org/acme/web/pull-requests/1" } } + }`), + ); + const calls: Array = []; + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({ + getPullRequest: () => + Effect.sync(() => calls.push("pullRequest")).pipe(Effect.as(pullRequest)), + getDiffStat: () => + Effect.sync(() => calls.push("diffstat")).pipe( + Effect.as({ additions: 12, deletions: 3, changedFiles: 2 }), + ), + getMergeability: () => + Effect.sync(() => calls.push("conflicts")).pipe(Effect.as("conflicting" as const)), + listChecks: () => Effect.sync(() => calls.push("statuses")).pipe(Effect.as([])), + getRepositoryPermission: () => + Effect.sync(() => calls.push("permission")).pipe(Effect.as(true)), + }), + ), + ); + const readSummary = provider.getChangeRequestSummary ?? provider.getChangeRequest; + const input = { cwd: "/repo", repository: "acme/web", host: "bitbucket.org", number: 1 }; + const expected = { additions: 12, deletions: 3, changedFiles: 2, mergeability: "conflicting" }; + + expect(yield* readSummary(input)).toMatchObject(expected); + const firstRead = calls.splice(0).toSorted(); + expect(yield* readSummary(input)).toMatchObject(expected); + const secondRead = calls.splice(0).toSorted(); + + expect({ firstRead, secondRead }).toEqual({ + firstRead: ["conflicts", "diffstat", "pullRequest"], + secondRead: ["pullRequest"], + }); + }), +); + describe("bitbucketProviderFailure", () => { it("treats only an HTTP 401 as unusable credentials", () => { const responseError = (status: number) => From aa286d6a950587e0d72f433bdd503f7bdad6678d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 30 Oct 2026 12:38:18 +0000 Subject: [PATCH 2/3] fix(bitbucket): PR sync rereads only the pull request while its commits 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 --- .../BitbucketPullRequestProvider.test.ts | 55 +++++++++++- .../server/BitbucketPullRequestProvider.ts | 84 +++++++++++++++++++ .../src/server/bitbucketPullRequestJson.ts | 2 + 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts index a98222ac19303..d4ade92cb20d8 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts @@ -184,7 +184,7 @@ it.effect("rereads only the pull request when sync reads an unchanged pull reque decodePullRequestJson(`{ "id": 1, "title": "Sync", "state": "OPEN", "source": { "branch": { "name": "feature" }, "commit": { "hash": "abc123" } }, - "destination": { "branch": { "name": "main" } }, + "destination": { "branch": { "name": "main" }, "commit": { "hash": "def456" } }, "created_on": "2026-09-16T00:00:00Z", "updated_on": "2026-09-16T00:00:00Z", "links": { "html": { "href": "https://bitbucket.org/acme/web/pull-requests/1" } } }`), @@ -223,6 +223,59 @@ it.effect("rereads only the pull request when sync reads an unchanged pull reque }), ); +it.effect( + "rereads diffstat and conflicts when either commit moves or the reads are forgotten", + () => + Effect.gen(function* () { + const pullRequestAt = (head: string, base: string) => + Result.getOrThrow( + decodePullRequestJson( + JSON.stringify({ + id: 1, + title: "Sync", + state: "OPEN", + source: { branch: { name: "feature" }, commit: { hash: head } }, + destination: { branch: { name: "main" }, commit: { hash: base } }, + created_on: "2026-09-16T00:00:00Z", + updated_on: "2026-09-16T00:00:00Z", + links: { html: { href: "https://bitbucket.org/acme/web/pull-requests/1" } }, + }), + ), + ); + let pullRequest = pullRequestAt("abc123", "def456"); + let statReads = 0; + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({ + getPullRequest: () => Effect.sync(() => pullRequest), + getDiffStat: () => + Effect.sync(() => ++statReads).pipe( + Effect.as({ additions: 1, deletions: 0, changedFiles: 1 }), + ), + getMergeability: () => Effect.succeed("mergeable" as const), + }), + ), + ); + const readSummary = provider.getChangeRequestSummary; + const forget = provider.forgetChangeRequestReads; + if (readSummary === undefined || forget === undefined) { + return yield* Effect.die("summary reads missing"); + } + const input = { cwd: "/repo", repository: "acme/web", host: "bitbucket.org", number: 1 }; + + yield* readSummary(input); + pullRequest = pullRequestAt("abc123", "fed789"); + yield* readSummary(input); + pullRequest = pullRequestAt("cba321", "fed789"); + yield* readSummary(input); + yield* readSummary(input); + yield* forget({ repository: "Acme/Web", number: 1 }); + yield* readSummary(input); + + expect(statReads).toBe(4); + }), +); + describe("bitbucketProviderFailure", () => { it("treats only an HTTP 401 as unusable credentials", () => { const responseError = (status: number) => diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts index 0e134af8572df..2d4e12b68a112 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts @@ -1,5 +1,8 @@ import { resolvePullRequestAuthorFilter, SourceControlProviderKind } from "@t3tools/contracts"; +import * as Cache from "effect/Cache"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import type { PullRequestCapabilities, PullRequestViewerPermissions } from "@t3tools/contracts"; import * as BitbucketPullRequestApi from "./BitbucketPullRequestApi.ts"; @@ -33,6 +36,8 @@ const CAPABILITIES: PullRequestCapabilities = { viewedFiles: "environment", }; +const COMMIT_READ_CAPACITY = 256; + const UNAVAILABLE_REQUIREMENT = { missing: "Bitbucket needs API credentials on the server. Add them in Settings → Source Control.", unauthenticated: @@ -145,6 +150,27 @@ export const make = Effect.gen(function* () { }); }; + // Null marks a conflicts read that failed and was recovered, so it is not held. + const readCommitStats = (target: { readonly repository: string; readonly number: number }) => + Effect.all([api.getDiffStat(target), recoverRead(api.getMergeability(target), null)], { + concurrency: 2, + }); + + // Diffstat and conflicts follow from the source and destination commits alone, so sync rereads + // them only when either moves; a refresh or a finished turn drops them through + // `forgetChangeRequestReads`. + const commitStats = yield* Cache.makeWith( + (key: string) => { + const [repository, number] = JSON.parse(key) as [string, number, string, string]; + return readCommitStats({ repository, number }); + }, + { + capacity: COMMIT_READ_CAPACITY, + timeToLive: (exit) => + Exit.isSuccess(exit) && exit.value[1] !== null ? Duration.infinity : Duration.zero, + }, + ); + const provider: PullRequestProviderApi = { kind: SourceControlProviderKind.make("bitbucket"), capabilities: CAPABILITIES, @@ -235,6 +261,64 @@ export const make = Effect.gen(function* () { ); }, + getChangeRequestSummary: (input) => { + const target = { repository: input.repository, number: input.number }; + return api.getPullRequest(target).pipe( + Effect.flatMap((pullRequest) => + (pullRequest.headSha === null || pullRequest.baseSha === null + ? readCommitStats(target) + : Cache.get( + commitStats, + JSON.stringify([ + input.repository, + input.number, + pullRequest.headSha, + pullRequest.baseSha, + ]), + ) + ).pipe( + Effect.map(([diffStat, mergeability]) => ({ + number: pullRequest.number, + title: pullRequest.title, + url: pullRequest.url, + author: pullRequest.author, + headBranch: pullRequest.headBranch, + baseBranch: pullRequest.baseBranch, + state: pullRequest.state, + isDraft: pullRequest.isDraft, + mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null, + closedAt: pullRequest.state === "closed" ? pullRequest.closedAt : null, + updatedAt: pullRequest.updatedAt, + additions: diffStat.additions, + deletions: diffStat.deletions, + changedFiles: diffStat.changedFiles, + mergeability: mergeability ?? "unknown", + })), + ), + ), + Effect.mapError(fail("getChangeRequestSummary")), + ); + }, + + forgetChangeRequestReads: (scope) => + scope === undefined + ? Cache.invalidateAll(commitStats) + : Cache.keys(commitStats).pipe( + Effect.flatMap((keys) => + Effect.forEach( + Array.from(keys).filter((key) => { + const [repository, number] = JSON.parse(key) as [string, number]; + return ( + repository.toLowerCase() === scope.repository.toLowerCase() && + (scope.number === undefined || number === scope.number) + ); + }), + (key) => Cache.invalidate(commitStats, key), + { discard: true }, + ), + ), + ), + getChangeRequestActivity: (input) => { const target = { repository: input.repository, number: input.number }; return Effect.all( diff --git a/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts b/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts index 42d48f76eeafb..7f74859b8e5be 100644 --- a/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts +++ b/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts @@ -163,6 +163,7 @@ export interface BitbucketPullRequest { readonly headSha: string | null; readonly headRepositoryNameWithOwner: string | null; readonly baseBranch: string; + readonly baseSha: string | null; readonly state: PullRequestState; readonly isDraft: boolean; /** Listings omit conflict state; detail reads use the conflicts endpoint. */ @@ -254,6 +255,7 @@ function toPullRequest(raw: Schema.Schema.Type): Bi headSha: trimmed(raw.source.commit?.hash), headRepositoryNameWithOwner: raw.source.repository?.full_name ?? null, baseBranch: raw.destination.branch.name, + baseSha: trimmed(raw.destination.commit?.hash), state: toState(raw), isDraft: raw.draft ?? false, mergeability: "unknown", From a5fc36dcfbfe464cb0b334495cd3c863cbb09227 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 30 Oct 2026 12:53:08 +0000 Subject: [PATCH 3/3] fix(bitbucket): held diffstat and conflicts expire after 15 minutes 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 --- .../BitbucketPullRequestProvider.test.ts | 25 ++++++++++++++++--- .../server/BitbucketPullRequestProvider.ts | 9 ++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts index d4ade92cb20d8..7c57196ca2973 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts @@ -1,8 +1,10 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import { describe, expect, it } from "@effect/vitest"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Result from "effect/Result"; +import * as TestClock from "effect/testing/TestClock"; import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; import * as BitbucketApi from "./BitbucketApi.ts"; @@ -224,7 +226,7 @@ it.effect("rereads only the pull request when sync reads an unchanged pull reque ); it.effect( - "rereads diffstat and conflicts when either commit moves or the reads are forgotten", + "rereads diffstat and conflicts when a commit moves, the reads expire or are forgotten, or conflicts failed", () => Effect.gen(function* () { const pullRequestAt = (head: string, base: string) => @@ -244,6 +246,7 @@ it.effect( ); let pullRequest = pullRequestAt("abc123", "def456"); let statReads = 0; + let conflictsFail = false; const provider = yield* make.pipe( Effect.provide( Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({ @@ -252,7 +255,16 @@ it.effect( Effect.sync(() => ++statReads).pipe( Effect.as({ additions: 1, deletions: 0, changedFiles: 1 }), ), - getMergeability: () => Effect.succeed("mergeable" as const), + getMergeability: () => + conflictsFail + ? Effect.fail( + new BitbucketApi.BitbucketResponseError({ + operation: "request", + status: 503, + responseBodyLength: 0, + }), + ) + : Effect.succeed("mergeable" as const), }), ), ); @@ -271,8 +283,15 @@ it.effect( yield* readSummary(input); yield* forget({ repository: "Acme/Web", number: 1 }); yield* readSummary(input); + yield* TestClock.adjust(Duration.minutes(15)); + yield* readSummary(input); + pullRequest = pullRequestAt("cba321", "0a0b0c"); + conflictsFail = true; + expect((yield* readSummary(input)).mergeability).toBe("unknown"); + conflictsFail = false; + expect((yield* readSummary(input)).mergeability).toBe("mergeable"); - expect(statReads).toBe(4); + expect(statReads).toBe(7); }), ); diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts index 2d4e12b68a112..b97a1bcce91d1 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts @@ -37,6 +37,7 @@ const CAPABILITIES: PullRequestCapabilities = { }; const COMMIT_READ_CAPACITY = 256; +const COMMIT_READ_TTL = Duration.minutes(15); const UNAVAILABLE_REQUIREMENT = { missing: "Bitbucket needs API credentials on the server. Add them in Settings → Source Control.", @@ -156,9 +157,9 @@ export const make = Effect.gen(function* () { concurrency: 2, }); - // Diffstat and conflicts follow from the source and destination commits alone, so sync rereads - // them only when either moves; a refresh or a finished turn drops them through - // `forgetChangeRequestReads`. + // Diffstat and conflicts follow from the source and destination commits, so sync rereads them + // when either moves. The expiry catches a destination branch that moved without the pull + // request's destination commit following; a refresh or a finished turn drops them sooner. const commitStats = yield* Cache.makeWith( (key: string) => { const [repository, number] = JSON.parse(key) as [string, number, string, string]; @@ -167,7 +168,7 @@ export const make = Effect.gen(function* () { { capacity: COMMIT_READ_CAPACITY, timeToLive: (exit) => - Exit.isSuccess(exit) && exit.value[1] !== null ? Duration.infinity : Duration.zero, + Exit.isSuccess(exit) && exit.value[1] !== null ? COMMIT_READ_TTL : Duration.zero, }, );