Uh oh!
There was an error while loading. Please reload this page.
fix(pr-followup): detect merge conflicts in scheduled sync - #600
Conversation
The conflict-fix pipeline (ingestMergeConflict -> MERGE_CONFLICT queue item -> bridge rebase workload -> reconcile) was fully built but never fired: the sync reads pr.mergeable from the PR *list* endpoint, which GitHub does not populate (mergeability is computed only on the per-PR GET). So pr.mergeable was always undefined and conflicting bot PRs (green + approved but behind main) sat forever with no signal enqueued. Add fetchPullRequestMergeState (per-PR detail GET) and call it per open bot PR in the sync loop, mapping mergeable_state to the enum the existing conflict ingestion consumes: dirty -> CONFLICTING (enqueue), clean/etc -> MERGEABLE (clear a resolved item), unknown/null -> leave unset (GitHub still computing; neither enqueue nor prematurely clear). Tests: dirty PR -> ingestMergeConflict called with CONFLICTING; unknown -> neither enqueue nor clear.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: public_route_changes)
Recommendation: Approve
This PR fixes a bug in the PR-followup scheduled sync where merge conflicts were silently undetected. The root cause: the GitHub PR list endpoint doesn't populate mergeable/mergeable_state, so the existing conflict detection branch never fired in production.
Change-by-Change Findings
src/lib/github.ts (+26 lines)
- Added
fetchPullRequestMergeState(repoFullName, prNumber)— a per-PR detail GET that returnsmergeable_stateandmergeable. - Transient failures degrade to
{ mergeableState: null, mergeable: null }with a silent catch, letting the next sync retry. - The function is used only by the sync route; not exported as a standalone route.
src/app/api/pr-followup/sync/route.ts (+17/-2 lines)
- Added
fetchPullRequestMergeStateimport. - Added
mergeStateto the existingPromise.allconcurrent fetch alongside comments, reviews, and check-runs — one extra per-PR GET per open bot PR per sync (every ~15 min). - Maps
mergeable_stateto the conflict ingestion enum:"dirty"→pr.mergeable = "CONFLICTING"→ triggersingestMergeConflict"clean"/other →pr.mergeable = "MERGEABLE"→ triggersclearResolvedConflictItems"unknown"/null → leavespr.mergeableunset → neither enqueue nor premature clear; resolves on next sync.
src/app/api/pr-followup/sync/route.test.ts (+64 lines)
- Two new test cases covering the bug fix scenarios.
Required Checks
✅ verify route access controls are in place
The route was already protected by authorizeRequest (bearer token via DISPATCH_AGENT_TOKEN per the codebase's auth model). The PR does not remove, weaken, or bypass any auth. It only adds a new server-side GitHub API call. The existing authentication pattern is preserved.
✅ check for unintended public endpoints
No new API routes were introduced. The PR modifies only the existing POST /api/pr-followup/sync endpoint. The new fetchPullRequestMergeState helper is a plain function called within the sync route, not a route handler. No new HTTP listeners or endpoints are exposed.
Standards Compliance
- API routes return appropriate HTTP status codes: ✅
200on success; the existing error handling pattern is unchanged. - Error handling: ✅
fetchPullRequestMergeStateuses a silentcatchthat degrades to null rather than throwing, consistent with the adjacent fetches that also.catch(() => []). - Validation: ✅ The
mergeableStatestring is normalized with.toLowerCase()before comparison. - No commit of secrets: ✅
GITHUB_TOKENis used from env; not logged or persisted. - Code style: ✅ Matches surrounding code;
npm run typecheckandeslintpass.
Linked Issue Fit
No linked issues are present in the corpus. The PR body provides sufficient context: the merge-conflict pipeline was built and unit-tested but never fired because pr.mergeable was always undefined in the scheduled sync path. The fix brings the pull-based sync (active mechanism) to parity with the webhook path, which already carried mergeable_state.
Evidence Provider Findings
No evidence providers are configured for this PR.
Tool Harness Findings
The tool harness issued no tool calls; reviewed corpus directly.
CI Check Results
| Check | State |
|---|---|
| Typecheck | ✅ success |
| Build | ✅ success |
| Lint | ✅ success |
| Tests | ✅ success |
| Docker Build | ✅ success |
Unknowns / Needs Verification
None identified. The PR is fully test-covered and passes all CI gates.
Summary
ingestMergeConflict→MERGE_CONFLICTqueue item → bridge rebase workload → reconcile) was fully built and unit-tested, but never fired in production: the scheduled sync readspr.mergeablefrom the PR list endpoint, which GitHub does not populate (mergeability is computed only on the per-PR GET). Sopr.mergeablewas alwaysundefined, the conflict branch never triggered, and conflicting bot PRs (green + approved but behind main) sat forever with no signal enqueued — e.g. windowstead fix: deduplicate safeEqual using timingSafeEqual from dispatch-env #256/feat: add optional GitHub App authentication for Dispatch #263/fix(ci): use --no-engine for prisma validate in docker run #265.Change
github.ts: addfetchPullRequestMergeState(repo, pr)— a per-PR detail GET returningmergeable_state/mergeable(tolerates transient failure).sync/route.ts: call it per open bot PR (concurrently with comments/reviews/checks) and setpr.mergeableso the existing conflict ingestion gets its signal:dirty→CONFLICTING→ingestMergeConflictenqueuesclean/other →MERGEABLE→clearResolvedConflictItemsclears a resolved itemunknown/null → leave unset — GitHub is still computing; neither enqueue nor prematurely clear (resolves on a later sync)Verification
npm run typecheckclean;eslintclean on changed files.vitestsync route: 8 passed (2 new — dirty→CONFLICTING enqueue; unknown→no-op); github lib: 66 passed.Notes
mergeable_state; this brings the pull-based sync (the active mechanism) to parity.