diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 226b7d703d0b..338b9e1d9aa1 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -5022,6 +5022,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio passed: existing.endedWatch.passed, passedChecks: existing.endedWatch.passedChecks, conflicting: existing.endedWatch.conflicting, + rebaseRequired: existing.endedWatch.rebaseRequired, }), awaitingBaseline: existing.endedWatch ? existing.endedWatch.awaitingBaseline diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts index ad134daab0f3..bb0b3380c38c 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts @@ -613,6 +613,39 @@ describe("PullRequestSyncReactor", () => { }).pipe(Effect.provide(fixture.layer)); }), ); + it.effect("syncs a rebase blocker even when the PR timestamp is unchanged", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const required = yield* Ref.make(false); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([makeThread("rebase", { pullRequests: [makeLink(7)] })]), + summary: (input) => + Ref.get(required).pipe( + Effect.map((rebaseRequired) => makeSummary(input, { rebaseRequired })), + ), + }); + yield* Effect.gen(function* () { + const reactor = yield* startAndSweep(fixture); + yield* applyRecordedSyncs(fixture); + yield* Ref.set(required, true); + yield* reactor.requestSync({ + host: "github.com", + repository: "owner/repository", + number: 7, + }); + yield* Queue.take(fixture.snapshotReads); + yield* reactor.drain; + const commands = yield* Ref.get(fixture.syncCommands); + assert.equal(commands.length, 2); + assert.equal(commands[0]?.snapshot?.rebaseRequired, false); + assert.equal(commands[1]?.snapshot?.rebaseRequired, true); + assert.equal(commands[0]?.snapshot?.updatedAt, commands[1]?.snapshot?.updatedAt); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); + it.effect("snapshots an unsynced link once and writes it to the thread", () => Effect.gen(function* () { yield* TestClock.setTime(Date.parse(NOW)); diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts index 46648e13b4b5..3d785967b7b2 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts @@ -234,6 +234,7 @@ function snapshotFieldsOf(summary: PullRequestSummary): SnapshotFields { ...(summary.reviewDecision === undefined ? {} : { reviewDecision: summary.reviewDecision }), ...(summary.checksState === undefined ? {} : { checksState: summary.checksState }), ...(summary.mergeability === undefined ? {} : { mergeability: summary.mergeability }), + ...(summary.rebaseRequired === undefined ? {} : { rebaseRequired: summary.rebaseRequired }), }; } @@ -282,7 +283,8 @@ function snapshotFieldsEqual(left: SnapshotFields, right: SnapshotFields): boole left.changedFiles === right.changedFiles && (left.reviewDecision ?? null) === (right.reviewDecision ?? null) && (left.checksState ?? null) === (right.checksState ?? null) && - left.mergeability === right.mergeability + left.mergeability === right.mergeability && + left.rebaseRequired === right.rebaseRequired ); } diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 4f67f5cb25b2..5b69a7efa337 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -151,6 +151,7 @@ const snapshotFingerprint = ({ link }: WatchTarget) => { snapshot.updatedAt, snapshot.checksState, snapshot.mergeability, + snapshot.rebaseRequired, snapshot.reviewDecision, snapshot.isDraft, ].join(" "); @@ -212,6 +213,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.remarksThrough === right.remarksThrough && left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && + left.rebaseRequired === right.rebaseRequired && left.mergeQueueRemovalId === right.mergeQueueRemovalId && left.wakes === right.wakes && left.awaitingBaseline === right.awaitingBaseline && diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index eb2ae2756d54..5a836898bb66 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -81,6 +81,45 @@ const remark = ( const noRemarks: ReadonlyArray = []; describe("evaluatePullRequestWatch", () => { + it.each(["pending", "success"] as const)( + "reports a required rebase once with %s CI", + (status) => { + const blocked = detail({ + mergeability: "unknown", + rebaseRequired: true, + checks: [check("test", status)], + }); + const first = evaluatePullRequestWatch(watch(), blocked, noRemarks); + assert.include( + first.changes.map((change) => change.kind), + "needs-rebase", + ); + assert.deepEqual(evaluatePullRequestWatch(first.next, blocked, noRemarks).changes, []); + const unknown = evaluatePullRequestWatch( + first.next, + { ...blocked, rebaseRequired: undefined }, + noRemarks, + ); + assert.deepEqual(evaluatePullRequestWatch(unknown.next, blocked, noRemarks).changes, []); + const clean = evaluatePullRequestWatch( + first.next, + { ...blocked, rebaseRequired: false }, + noRemarks, + ); + assert.deepEqual(evaluatePullRequestWatch(clean.next, blocked, noRemarks).changes, [ + { kind: "needs-rebase" }, + ]); + assert.include( + evaluatePullRequestWatch( + first.next, + { ...blocked, headSha: "bbbbbbbbbb" }, + noRemarks, + ).changes.map((change) => change.kind), + "needs-rebase", + ); + }, + ); + it.each([false, true])("baselines already-passed checks (required: %s)", (required) => { const green = detail({ checks: [{ ...check("test", "success"), required }] }); const baseline = evaluatePullRequestWatch(watch({ awaitingBaseline: "all" }), green, noRemarks); @@ -115,6 +154,7 @@ describe("evaluatePullRequestWatch", () => { const broken = detail({ checks: [check("lint", "failure"), check("test", "pending")], mergeability: "conflicting", + rebaseRequired: true, }); const baseline = evaluatePullRequestWatch( watch({ awaitingBaseline: "all" }), @@ -125,6 +165,7 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(baseline.next.failedChecks, ["lint"]); assert.isFalse(baseline.next.passed); assert.isTrue(baseline.next.conflicting); + assert.isTrue(baseline.next.rebaseRequired); assert.deepEqual(evaluatePullRequestWatch(baseline.next, broken, noRemarks).changes, []); const failed = evaluatePullRequestWatch( @@ -135,11 +176,16 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(failed.changes, [ { kind: "checks-failed", failed: [check("test", "failure")] }, ]); - const clean = evaluatePullRequestWatch(failed.next, detail(), noRemarks); + const clean = evaluatePullRequestWatch( + failed.next, + detail({ rebaseRequired: false }), + noRemarks, + ); assert.deepEqual(clean.changes, []); assert.deepEqual(evaluatePullRequestWatch(clean.next, broken, noRemarks).changes, [ { kind: "checks-failed", failed: [check("lint", "failure")] }, { kind: "conflicting" }, + { kind: "needs-rebase" }, ]); }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index e12d9a124df8..a6603aad97c3 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -21,6 +21,7 @@ export type PullRequestWatchChange = | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "no-checks"; readonly minutes: number } | { readonly kind: "conflicting" } + | { readonly kind: "needs-rebase" } | { readonly kind: "merge-queue-removed"; readonly reason: string | null }; export interface PullRequestWatchReport { @@ -46,6 +47,7 @@ export function evaluatePullRequestWatch( | "mergeability" | "viewer" | "author" + | "rebaseRequired" | "mergeQueueRemoval" >, remarks: ReadonlyArray | null, @@ -196,11 +198,16 @@ export function evaluatePullRequestWatch( ...atLatest.map((remark) => remark.id), ]; - // Like a failed check, a conflict a push did not resolve is news for the new head. + // Like a failed check, a conflict or rebase a push did not resolve is news for the new head. const toldConflict = !headMoved && watch.conflicting; if (watch.awaitingBaseline !== "all" && detail.mergeability === "conflicting" && !toldConflict) { changes.push({ kind: "conflicting" }); } + const toldRebase = !headMoved && watch.rebaseRequired === true; + if (watch.awaitingBaseline !== "all" && detail.rebaseRequired === true && !toldRebase) { + changes.push({ kind: "needs-rebase" }); + } + const rebaseRequired = detail.rebaseRequired ?? watch.rebaseRequired; // "unknown" is a host still computing after a push; only a clean answer clears a conflict. const conflicting = detail.mergeability === "unknown" ? toldConflict : detail.mergeability === "conflicting"; @@ -239,6 +246,7 @@ export function evaluatePullRequestWatch( remarksThrough, remarkIds, conflicting, + rebaseRequired, mergeQueueRemovalId, wakes, // Only a read that cannot be trusted to show the checks leaves the baseline to the next one. @@ -308,6 +316,8 @@ function changeLines( return [ `- No checks have reported${context.commit} after ${change.minutes} minutes. None may be configured to run on this change.`, ]; + case "needs-rebase": + return [`- The branch requires a rebase onto ${context.baseBranch}.`]; case "conflicting": return [`- The branch now conflicts with ${context.baseBranch}.`]; case "merge-queue-removed": @@ -324,6 +334,7 @@ const SUMMARY: Record = { remarks: "new comments", "no-checks": "no checks reported", conflicting: "merge conflict", + "needs-rebase": "rebase required", "merge-queue-removed": "removed from merge queue", }; @@ -388,6 +399,7 @@ export function pullRequestWatchMessage(input: { (change) => change.kind === "checks-failed" || change.kind === "conflicting" || + change.kind === "needs-rebase" || change.kind === "merge-queue-removed", ); const summary = changes.map((change) => SUMMARY[change.kind]); diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 4693b953a647..74106b14cd40 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -3458,6 +3458,7 @@ it.effect( Effect.succeed({ ...changeRequest(1, "2026-07-02T00:00:00Z"), isDraft: true, + rebaseRequired: false, reviewDecision: "approved", checksState: "passing", }), @@ -3467,6 +3468,7 @@ it.effect( deletions: 3, changedFiles: 5, mergeability: "conflicting", + rebaseRequired: true, }), }), ], @@ -3474,6 +3476,8 @@ it.effect( yield* service.summary(reference); const detail = yield* service.detail(reference); const summary = yield* service.summary(reference); + assert.strictEqual(detail.rebaseRequired, true); + assert.strictEqual(summary.rebaseRequired, true); assert.strictEqual(summary.isDraft, false); assert.deepStrictEqual(summary.author, detail.author); assert.strictEqual(summary.additions, 14); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 4e0c648cf153..85a9349b7af8 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1309,6 +1309,9 @@ export const make = Effect.gen(function* () { state: input.item.state, isDraft: input.item.isDraft, mergeability: input.item.mergeability, + ...(input.item.rebaseRequired === undefined + ? {} + : { rebaseRequired: input.item.rebaseRequired }), additions: input.item.additions, deletions: input.item.deletions, createdAt: input.item.createdAt, @@ -1792,6 +1795,9 @@ export const make = Effect.gen(function* () { ...(changeRequest.mergeability === undefined ? {} : { mergeability: changeRequest.mergeability }), + ...(changeRequest.rebaseRequired === undefined + ? {} + : { rebaseRequired: changeRequest.rebaseRequired }), ...(changeRequest.stack === undefined ? {} : { stack: changeRequest.stack }), ...(changeRequest.headSha?.trim() ? { headSha: changeRequest.headSha.trim() } : {}), ...(changeRequest.autoMergeEnabled === undefined @@ -1891,6 +1897,9 @@ export const make = Effect.gen(function* () { state: changeRequest.state, isDraft: changeRequest.isDraft, mergeability: changeRequest.mergeability, + ...(changeRequest.rebaseRequired === undefined + ? {} + : { rebaseRequired: changeRequest.rebaseRequired }), additions: changeRequest.additions, deletions: changeRequest.deletions, changedFiles: changeRequest.changedFiles, @@ -3330,6 +3339,7 @@ export const make = Effect.gen(function* () { deletions: detail.deletions, changedFiles: detail.changedFiles, mergeability: detail.mergeability, + ...(detail.rebaseRequired === undefined ? {} : { rebaseRequired: detail.rebaseRequired }), headBranch: detail.headBranch, baseBranch: detail.baseBranch, closedAt: detail.closedAt, diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 3f1198c1212e..df5447f21df6 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -370,6 +370,7 @@ export const PullRequestListEntry = Schema.Struct({ state: PullRequestState, isDraft: Schema.Boolean, mergeability: PullRequestMergeability, + rebaseRequired: Schema.optional(Schema.Boolean), /** Zero until fetched through `PullRequestListStatsInput`; GitHub listings omit counts. */ additions: NonNegativeInt, deletions: NonNegativeInt, @@ -528,6 +529,7 @@ export const PullRequestSummary = Schema.Struct({ reviewDecision: Schema.optional(Schema.NullOr(PullRequestReviewDecision)), checksState: Schema.optional(Schema.NullOr(PullRequestChecksState)), mergeability: Schema.optional(PullRequestMergeability), + rebaseRequired: Schema.optional(Schema.Boolean), /** Null when the host says the pull request is in no stack; absent when the read did not ask. */ stack: Schema.optional(Schema.NullOr(PullRequestStackMembership)), headSha: Schema.optional(TrimmedNonEmptyString), @@ -651,6 +653,7 @@ export const PullRequestDetail = Schema.Struct({ state: PullRequestState, isDraft: Schema.Boolean, mergeability: PullRequestMergeability, + rebaseRequired: Schema.optional(Schema.Boolean), additions: NonNegativeInt, deletions: NonNegativeInt, changedFiles: NonNegativeInt, diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 7775f0755ff0..0c8d7a61fe92 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -51,6 +51,7 @@ export const ThreadPullRequestSnapshot = Schema.Struct({ reviewDecision: Schema.optional(Schema.NullOr(PullRequestReviewDecision)), checksState: Schema.optional(Schema.NullOr(PullRequestChecksState)), mergeability: Schema.optional(PullRequestMergeability), + rebaseRequired: Schema.optional(Schema.Boolean), }); export type ThreadPullRequestSnapshot = typeof ThreadPullRequestSnapshot.Type; @@ -97,6 +98,7 @@ export const ThreadPullRequestWatch = Schema.Struct({ /** Reported remarks at remarksThrough, so late arrivals at that timestamp still count. */ remarkIds: Schema.Array(TrimmedNonEmptyString), conflicting: Schema.Boolean, + rebaseRequired: Schema.optional(Schema.Boolean), /** Last removal seen; null after an empty read, absent before the first read. */ mergeQueueRemovalId: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), /** Consecutive comment-only wakes; limited to prevent bot loops. */ diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 8ea9eaae216b..21f9cab137d7 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -97,6 +97,7 @@ export interface ProviderChangeRequest { readonly state: PullRequestState; readonly isDraft: boolean; readonly mergeability: PullRequestMergeability; + readonly rebaseRequired?: boolean | undefined; readonly additions: number; readonly deletions: number; readonly createdAt: string; @@ -127,6 +128,7 @@ export interface ProviderChangeRequestSummary { readonly reviewDecision?: PullRequestReviewDecision | null | undefined; readonly checksState?: PullRequestChecksState | null | undefined; readonly mergeability?: PullRequestMergeability | undefined; + readonly rebaseRequired?: boolean | undefined; /** Null means no stack; absent means not requested. */ readonly stack?: PullRequestStackMembership | null | undefined; readonly headSha?: string | undefined; diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts index f66302d5b43b..4ec7ce9a33bc 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts @@ -126,6 +126,37 @@ describe("decodeMergeRequestListJson", () => { }); describe("decodeMergeRequestDetailJson", () => { + it("preserves a required rebase while CI is running or successful", () => { + for (const status of ["running", "success"]) { + const detail = expectSuccess( + decodeMergeRequestDetailJson( + detailJson({ + merge_status: "can_be_merged", + detailed_merge_status: "need_rebase", + has_conflicts: false, + head_pipeline: { status }, + }), + ), + ); + expect(detail.rebaseRequired).toBe(true); + expect(detail.mergeability).toBe("mergeable"); + } + }); + + it("distinguishes a cleared rebase blocker from an inconclusive merge check", () => { + expect( + expectSuccess( + decodeMergeRequestDetailJson(detailJson({ detailed_merge_status: "mergeable" })), + ).rebaseRequired, + ).toBe(false); + for (const detailed_merge_status of [undefined, "checking", "ci_still_running"]) { + expect( + expectSuccess(decodeMergeRequestDetailJson(detailJson({ detailed_merge_status }))) + .rebaseRequired, + ).toBeUndefined(); + } + }); + it("reads the description, file count and pipeline", () => { const detail = expectSuccess( decodeMergeRequestDetailJson( @@ -1266,6 +1297,7 @@ describe("decodeMergeRequestWatchFingerprintJson", () => { mergeRequest: { state: "opened", mergeStatusEnum: "CAN_BE_MERGED", + detailedMergeStatus: "MERGEABLE", conflicts: false, diffHeadSha: "a".repeat(40), userPermissions: { updateMergeRequest: true }, @@ -1307,6 +1339,8 @@ describe("decodeMergeRequestWatchFingerprintJson", () => { expect(edited?.remarks).not.toBe(base?.remarks); expect(resolved?.remarks).toBe(base?.remarks); expect(read({ userNotesCount: 3 })?.remarks).not.toBe(base?.remarks); + // A target branch moving on leaves merge_status at can_be_merged on fast-forward projects. + expect(read({ detailedMergeStatus: "NEED_REBASE" })?.status).not.toBe(base?.status); }); // Only the REST read asks GitLab to recheck mergeability, and only for viewers who can update. diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts index 685f920f0765..24ad19248eaf 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts @@ -73,6 +73,7 @@ const RawMergeRequestSchema = Schema.Struct({ state: Schema.optional(Schema.NullOr(Schema.String)), draft: Schema.optional(Schema.Boolean), work_in_progress: Schema.optional(Schema.Boolean), + detailed_merge_status: Schema.optional(Schema.NullOr(Schema.String)), merge_status: Schema.optional(Schema.NullOr(Schema.String)), has_conflicts: Schema.optional(Schema.NullOr(Schema.Boolean)), created_at: Schema.String, @@ -208,6 +209,7 @@ export interface GitLabMergeRequestListItem { readonly state: PullRequestState; readonly isDraft: boolean; readonly mergeability: PullRequestMergeability; + readonly rebaseRequired?: boolean | undefined; readonly additions: number; readonly deletions: number; readonly createdAt: string; @@ -484,6 +486,7 @@ export function decodePipelineStageChecksJson( function toListItem( raw: Schema.Schema.Type, ): GitLabMergeRequestListItem { + const detailedMergeStatus = raw.detailed_merge_status?.trim().toLowerCase(); return { number: raw.iid, title: raw.title, @@ -494,6 +497,12 @@ function toListItem( state: toState(raw), isDraft: raw.draft ?? raw.work_in_progress ?? false, mergeability: toMergeability(raw), + rebaseRequired: + detailedMergeStatus === "need_rebase" + ? true + : detailedMergeStatus === "mergeable" + ? false + : undefined, additions: 0, deletions: 0, createdAt: raw.created_at, @@ -2063,7 +2072,7 @@ export function decodeMergeRequestFileStatsJson(raw: string): Result.Result< export const MERGE_REQUEST_WATCH_FINGERPRINT_GRAPHQL_QUERY = `query($fullPath: ID!, $iid: String!) { project(fullPath: $fullPath) { mergeRequest(iid: $iid) { - state mergeStatusEnum conflicts diffHeadSha + state mergeStatusEnum detailedMergeStatus conflicts diffHeadSha userPermissions { updateMergeRequest } headPipeline { id status @@ -2097,6 +2106,7 @@ const decodeMergeRequestWatchFingerprint = decodeJsonResult( Schema.Struct({ state: Schema.String, mergeStatusEnum: Schema.NullOr(Schema.String), + detailedMergeStatus: Schema.optional(Schema.NullOr(Schema.String)), conflicts: Schema.Boolean, diffHeadSha: Schema.NullOr(Schema.String), userPermissions: Schema.Struct({ updateMergeRequest: Schema.Boolean }), @@ -2182,6 +2192,7 @@ export function decodeMergeRequestWatchFingerprintJson( status: [ mr.state, mr.mergeStatusEnum ?? "", + mr.detailedMergeStatus ?? "", mr.conflicts, mr.diffHeadSha ?? "", pipeline?.id ?? "",