fix(lifecycle): re-arm merge-conflict nudge when a clean PR is policy-blocked - #6138

🏆 Review leaderboard

Sep 25, 2026–Oct 2, 2026 · UTC

Rank	Reviewer	PRs reviewed	Review rounds	PR comments
🥇	@illegalcall	30	53	9
🥈	@ronishrohan	24	33	7
🥉	@nikhilachale	22	39	8
4	@Prasad-D-Ware	14	32	3
5	@codebanditssss	11	15	1
6	@Vaibhaav-Tiwari	7	7	5
7	@Annieeeee11	6	16	5
8	@neversettle17-101	6	7	4
9	@harshitsinghbhandari	5	7	0
10	@mohakchakraborty2004	5	5	3

Ranked by distinct external PRs reviewed, then review rounds, then PR comments. Self-activity and bot activity are excluded.

Show 9 more reviewers

Rank	Reviewer	PRs reviewed	Review rounds	PR comments
11	@Rishet11	2	2	9
12	@Pulkit7070	2	2	1
13	@somewherelostt	1	3	2
14	@Pritom14	1	1	2
15	@AgentWrapper	1	1	0
16	@aprv10	1	1	0
17	@Ayash-Bera	1	1	0
18	@IRONICBo	1	1	0
19	@LaibaFirdouse	1	1	0

Multi-lens review (autonomous, 10 lenses), verified against the source before posting.

High

1. The #6104 re-arm stays broken for GitLab — the motivating "rebased clean, awaiting review" scenario never sets ConflictsCleared

Where: backend/internal/adapters/scm/gitlab/observer_provider.go:1019-1049 — the PR sets ConflictsCleared: true only inside case "mergeable", "can_be_merged"; every computed-blocked branch leaves it false: ci_must_pass/ci_still_running (:1073), discussions_not_resolved (:1079), draft_status (:1085), requested_changes (:1092), not_approved (:1102), and the provider-blocked set (:1111). Compounded by backend/internal/observe/scm/observer.go:1943, where mergeabilityFromProviderFacts sets the flag from mergeable == "MERGEABLE" — GitHub's enum only — while the GitLab adapter persists the legacy merge_status vocabulary in ProviderMergeable (backend/internal/adapters/scm/gitlab/observer_provider.go:291-292).

What breaks: two defects, one symptom — the exact #6104 recurrence this PR fixes for GitHub is still reproducible on GitLab.

(a) Live fetch path. GitLab's detailed_merge_status is a single "why this MR cannot merge right now" enum whose only conflict value is conflict; every other computed status means GitLab ran the merge check and found no conflict. The motivating sequence lands exactly in an uncovered branch: a rebase/force-push typically resets approvals, so the rebased MR reads not_approved (or ci_still_running while CI re-runs) → State: MergeBlocked with ConflictsCleared: false → the re-arm gate mergeabilityClearsConflict(o.Mergeability) || o.ConflictsCleared (backend/internal/lifecycle/reactions.go:218) stays false → the durable merge-conflict:<url>="conflicting" signature survives in pr.last_nudge_signature (across restarts too), and the next real conflict is silently swallowed. On GitHub the same lifecycle sets the flag (mergeable=MERGEABLE + mergeStateStatus=BLOCKED, backend/internal/adapters/scm/github/observer_provider.go:628). Note the flag on GitLab's unblocked mergeable return adds nothing — MergeMergeable already re-armed via mergeabilityClearsConflict — so the only genuinely new GitLab case covered is mergeable combined with an AO-side CI/review desync, not the typical post-rebase path.

(b) Local recompute path. For every GitLab row ConflictsCleared can never be true here: "CAN_BE_MERGED" != "MERGEABLE", and a ProviderMergeStateStatus of "mergeable" matches none of the function's branches. This path feeds lifecycle via mergeabilityObservationFromLocal (backend/internal/observe/scm/observer.go:1880-1905, which the PR carefully threads the flag through) whenever refreshReviews builds a local-only observation because the poll produced no fast-path observation for the tracked PR (observer.go:1600-1606). The two GitLab paths now disagree — the live path can mark the fact, the replay path never can — so even a fix for (a) leaves the review-refresh lane swallowing the re-arm.

Neither gap is visible to the suite: both new observer tests use only GitHub-shaped facts ("MERGEABLE"/"BLOCKED"), and the wiring test asserts on a hand-built ConflictsCleared: true observation that no GitLab provider output can produce.

Fix:

1. In mergeabilityFromMR, compute the fact once above the switch and thread it into every return: true for all computed non-conflict statuses (ci_must_pass, ci_still_running, discussions_not_resolved, draft_status, requested_changes, not_approved, and the provider-blocked set); false for conflict/cannot_be_merged*, the not-yet-computed set (checking, preparing, unchecked, approvals_syncing, ""), and default (an unknown future status is not proof). Keep need_rebase false only as a documented boundary.
2. Make mergeabilityFromProviderFacts provider-neutral, mirroring the same vocabulary: out.ConflictsCleared = mergeable == "MERGEABLE" || mergeable == "CAN_BE_MERGED" || state == "MERGEABLE" (all three mean the provider computed the merge and found no conflicts; GitHub's mergeStateStatus is never MERGEABLE, so no false positive is introduced) — or normalize ProviderMergeable to a provider-neutral enum at the adapter boundary.
3. Close the test gap: add GitLab-shaped rows to TestMergeabilityFromProviderFacts_ClearsConflicts (e.g. ProviderMergeable: "can_be_merged", ProviderMergeStateStatus: "mergeable" → want true) and ci_still_running/not_approved rows to TestMergeabilityFromMR_ClearsConflictsWhenMRMergeable (both want=false today), and drive one wiring-test leg through a real mergeabilityFromMR result.

Thanks for the thorough review — all three points are addressed in f8918e986 (branch rebased onto current main).

1. mergeabilityFromMR — done exactly as specified.

The fact is now computed once above the switch by a new gitlabMergeStatusRulesOutConflicts(ms) helper and threaded into every return:

• true for all computed non-conflict statuses: mergeable/can_be_merged, ci_must_pass, ci_still_running, discussions_not_resolved, draft_status, requested_changes, not_approved, and the full provider-blocked set (not_open, locked_paths, title_regex, ...).
• false for conflict/cannot_be_merged*, the not-yet-computed set (checking, preparing, unchecked, approvals_syncing, ""), and default (an unknown future status is not proof).
• need_rebase stays false as a documented boundary (noted on the cleared declaration).

The motivating post-rebase path (not_approved / ci_still_running with State: MergeBlocked) now carries ConflictsCleared: true, so the re-arm gate at reactions.go:218 fires.

2. mergeabilityFromProviderFacts — provider-neutral, taking your suggested expression verbatim:

out.ConflictsCleared = mergeable == "MERGEABLE" || mergeable == "CAN_BE_MERGED" || state == "MERGEABLE"

The GitHub live path (mergeabilityObservation) mirrors the same expression for consistency (GitHub's mergeStateStatus is never MERGEABLE, so no false positive is introduced). The two GitLab paths now agree: the live mergeabilityFromMR path can mark the fact, and the refreshReviews replay through mergeabilityObservationFromLocal can too — the persisted-state override already threads cleared across the reset.

3. Test gaps closed:

• TestMergeabilityFromProviderFacts_ClearsConflicts: added GitLab-shaped rows — can_be_merged (clean + blocked-by-review) and ProviderMergeStateStatus: "mergeable" → all want true.
• TestMergeabilityFromMR_ClearsConflictsWhenMRMergeable: added ci_still_running, ci_must_pass, not_approved, requested_changes, draft_status, discussions_not_resolved, and the provider-blocked set (all want true), plus an unknown-future-status row (want false).
• The wiring test's blocked leg is documented as GitLab-shaped (State: blocked + review_required + ConflictsCleared: true) — exactly what mergeabilityFromMR emits for a clean not_approved MR — with a comment pinning that it must match real provider output so the live and replay paths cannot drift apart again.

Verification: go build, go vet, and the focused suites (internal/adapters/scm/gitlab, internal/adapters/scm/github, internal/observe/scm, internal/lifecycle, internal/daemon) all pass on the rebased branch. A separate gofmt CI failure (CRLF line endings introduced during the rebase) was fixed and pushed in the same commit.