Skip to content

Commit feac738

Browse files
RealDoc06claude
andcommitted
fix(server): Bitbucket threads settle after their pull request merges
Bitbucket pull request reads never carried a merge or decline time, so auto-settle on merge skipped every Bitbucket thread. Ask for `closed_on`, which Bitbucket omits by default, and map it to mergedAt or closedAt by state. Merged snapshots saved without a merge time keep the slow sync cadence so existing threads pick it up. Closes #16304 Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 98bc998 commit feac738

7 files changed

Lines changed: 116 additions & 5 deletions

File tree

‎apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,11 @@ describe("PullRequestSyncReactor", () => {
635635
yield* TestClock.setTime(Date.parse(NOW));
636636
const fixture = yield* makeHarness({
637637
snapshot: makeSnapshot([
638-
makeThread("merged", { pullRequests: [makeLink(1, { state: "merged" })] }),
638+
makeThread("merged", {
639+
pullRequests: [
640+
makeLink(1, { state: "merged", mergedAt: "2026-08-27T00:00:00.000Z" }),
641+
],
642+
}),
639643
]),
640644
});
641645

@@ -663,6 +667,33 @@ describe("PullRequestSyncReactor", () => {
663667
),
664668
);
665669

670+
it.effect("re-reads a merged pull request until the host supplies its merge time", () =>
671+
Effect.scoped(
672+
Effect.gen(function* () {
673+
yield* TestClock.setTime(Date.parse(NOW));
674+
const fixture = yield* makeHarness({
675+
snapshot: makeSnapshot([
676+
makeThread("merged", { pullRequests: [makeLink(1, { state: "merged" })] }),
677+
]),
678+
summary: (input) =>
679+
Effect.succeed(
680+
makeSummary(input, { state: "merged", mergedAt: "2026-08-27T00:00:00.000Z" }),
681+
),
682+
});
683+
684+
yield* Effect.gen(function* () {
685+
const reactor = yield* startAndSweep(fixture);
686+
const firstSweep = yield* Ref.get(fixture.syncCommands);
687+
assert.strictEqual(firstSweep.at(-1)?.snapshot.mergedAt, "2026-08-27T00:00:00.000Z");
688+
yield* Ref.update(fixture.snapshots, (snapshot) => applySync(snapshot, firstSweep));
689+
690+
for (let index = 0; index < 15; index += 1) yield* sweepAgain(fixture, reactor);
691+
assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 1);
692+
}).pipe(Effect.provide(fixture.layer));
693+
}),
694+
),
695+
);
696+
666697
it.effect("syncs a pull request a reader saw merge without waiting for the sweep", () =>
667698
Effect.scoped(
668699
Effect.gen(function* () {

‎apps/server/src/orchestration-v2/PullRequestSyncReactor.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,15 @@ export const make = Effect.gen(function* () {
177177
// Settled threads stop watching their pull requests. Unsettling one makes its links due on
178178
// the next sweep, since the cadence clock below kept running while it was settled.
179179
const active = entries.filter((entry) => isUnsettled(entry.thread));
180-
if (active.every((entry) => entry.link.snapshot?.state === "merged")) return false;
180+
// A merged snapshot without its merge time cannot settle the thread, so it keeps the slow
181+
// cadence until the host supplies one (Bitbucket snapshots saved before it was read).
182+
if (
183+
active.every(
184+
(entry) => entry.link.snapshot?.state === "merged" && entry.link.snapshot.mergedAt != null,
185+
)
186+
) {
187+
return false;
188+
}
181189
if (active.some((entry) => entry.link.snapshot?.state === "open")) return true;
182190
// Closed requests can reopen on the host.
183191
const last = lastSyncedAt.get(key);

‎packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,24 @@ layer("BitbucketPullRequestApi.layer", (it) => {
730730
}),
731731
);
732732

733+
it.effect("asks for the time a pull request closed, which Bitbucket leaves out by default", () =>
734+
Effect.gen(function* () {
735+
mockedRequest.mockReturnValueOnce(
736+
Effect.succeed(
737+
response(
738+
pullRequestJson({ state: "MERGED", closed_on: "2025-06-16T23:40:46.222238+00:00" }),
739+
),
740+
),
741+
);
742+
const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi;
743+
744+
const pullRequest = yield* api.getPullRequest({ repository: "acme/web", number: 7 });
745+
746+
expect(callAt(0).url).toBe("/repositories/acme/web/pullrequests/7?fields=%2Bclosed_on");
747+
assert.strictEqual(pullRequest.closedAt, "2025-06-16T23:40:46.222Z");
748+
}),
749+
);
750+
733751
it.effect("fails the read when Bitbucket answers with something unreadable", () =>
734752
Effect.gen(function* () {
735753
mockedRequest.mockReturnValueOnce(

‎packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,9 @@ export const make = Effect.gen(function* () {
654654
withRepository(input.repository, (path) =>
655655
readPage({
656656
operation: "getPullRequest",
657-
url: `${path}/pullrequests/${input.number}`,
657+
// `closed_on` is left out of the default representation. It is the only merge or
658+
// decline time Bitbucket states on the pull request itself, and settlement needs it.
659+
url: `${path}/pullrequests/${input.number}?fields=%2Bclosed_on`,
658660
decode: decodePullRequestJson,
659661
}),
660662
),

‎packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,53 @@ it.effect.each([
7070
}),
7171
);
7272

73+
it.effect.each([
74+
{ state: "MERGED", mergedAt: "2025-06-16T23:40:46.222Z", closedAt: null },
75+
{ state: "DECLINED", mergedAt: null, closedAt: "2025-06-16T23:40:46.222Z" },
76+
{ state: "OPEN", mergedAt: null, closedAt: null },
77+
])("reads the $state time from closed_on", ({ state, mergedAt, closedAt }) =>
78+
Effect.gen(function* () {
79+
const pullRequest = Result.getOrThrow(
80+
decodePullRequestJson(
81+
JSON.stringify({
82+
id: 52,
83+
title: "Release",
84+
state,
85+
source: { branch: { name: "release" } },
86+
destination: { branch: { name: "master" } },
87+
created_on: "2025-06-16T06:20:58.415108+00:00",
88+
// Comments after a merge move this, so it is not the merge time.
89+
updated_on: "2025-06-20T10:00:00.000000+00:00",
90+
closed_on: state === "OPEN" ? null : "2025-06-16T23:40:46.222238+00:00",
91+
links: { html: { href: "https://bitbucket.org/acme/web/pull-requests/52" } },
92+
}),
93+
),
94+
);
95+
const provider = yield* make.pipe(
96+
Effect.provide(
97+
Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({
98+
getPullRequest: () => Effect.succeed(pullRequest),
99+
getDiffStat: () => Effect.succeed({ additions: 0, deletions: 0, changedFiles: 0 }),
100+
getMergeability: () => Effect.succeed("unknown"),
101+
listChecks: () => Effect.succeed([]),
102+
getRepositoryPermission: () => Effect.succeed(true),
103+
}),
104+
),
105+
);
106+
const detail = yield* provider.getChangeRequest({
107+
cwd: "/repo",
108+
repository: "acme/web",
109+
host: "bitbucket.org",
110+
number: 52,
111+
});
112+
113+
expect({ mergedAt: detail.mergedAt, closedAt: detail.closedAt }).toEqual({
114+
mergedAt,
115+
closedAt,
116+
});
117+
}),
118+
);
119+
73120
for (const operation of [
74121
"getMergeability",
75122
"listChecks",

‎packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ export const make = Effect.gen(function* () {
221221
deletions: diffStat.deletions,
222222
changedFiles: diffStat.changedFiles,
223223
body: pullRequest.body,
224-
mergedAt: null,
225-
closedAt: null,
224+
mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null,
225+
closedAt: pullRequest.state === "closed" ? pullRequest.closedAt : null,
226226
reviewers: pullRequest.reviewers,
227227
checks,
228228
// Bitbucket publishes no per-repository list of allowed strategies, so the ones it

‎packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ const RawPullRequestSchema = Schema.Struct({
7171
destination: RawBranchSchema,
7272
created_on: Schema.String,
7373
updated_on: Schema.String,
74+
/** Only sent when asked for with `fields=+closed_on`; null while the request is open. */
75+
closed_on: Schema.optional(Schema.NullOr(Schema.String)),
7476
reviewers: Schema.optional(Schema.NullOr(Schema.Array(RawUserSchema))),
7577
participants: Schema.optional(
7678
Schema.NullOr(
@@ -192,6 +194,8 @@ export interface BitbucketPullRequest {
192194
readonly mergeability: PullRequestMergeability;
193195
readonly createdAt: string;
194196
readonly updatedAt: string;
197+
/** When the request was merged or declined, unlike `updatedAt`, which later comments move. */
198+
readonly closedAt: string | null;
195199
readonly body: string;
196200
readonly reviewRequestLogins: ReadonlyArray<string>;
197201
readonly reviewers: ReadonlyArray<PullRequestActor>;
@@ -304,6 +308,7 @@ function toPullRequest(raw: Schema.Schema.Type<typeof RawPullRequestSchema>): Bi
304308
mergeability: "unknown",
305309
createdAt: toIsoUtc(raw.created_on),
306310
updatedAt: toIsoUtc(raw.updated_on),
311+
closedAt: raw.closed_on == null ? null : toIsoUtc(raw.closed_on),
307312
body: raw.description ?? "",
308313
reviewRequestLogins: reviewers.map((reviewer) => reviewer.login),
309314
reviewers,

0 commit comments

Comments
 (0)