feat: surface merge conflicts as PR review-fix items with priority ordering#356
Conversation
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: db_or_migration_changes)
Review: PR PR 356 — Surface merge conflicts as PR review-fix items with priority ordering
Recommendation
Approve. The migration is safe, the implementation satisfies all linked issue acceptance criteria, and the code follows repository conventions.
Change-by-Change Findings
Migration (prisma/migrations/20260611000000_add_pr_fix_type/migration.sql)
- New enum
PrFixTypewith valuesMERGE_CONFLICT,CI_FAILURE,REVIEW_FEEDBACK,OTHER— standard Prisma pattern, no data loss risk. - **ADD COLUMN
typeNOT NULL DEFAULT 'OTHER'** — populates existing rows with the catch-allOTHER` value, which is correct since pre-existing queue items had no explicit type. - Two composite indexes on
(type)and(status, type)for efficient priority-ordered queue queries.
Schema (prisma/schema.prisma)
- Enum
PrFixTypeadded;typefield onPrFixQueueItemdefaults toOTHER. - Two new indexes mirror the migration SQL.
Code changes (src/lib/pr-followup-ingestion.ts)
ingestMergeConflict(): detectsmergeable=CONFLICTINGPRs, enqueues withtype: "MERGE_CONFLICT". Correctly checks bot author and repo owner eligibility before enqueueing.clearResolvedConflictItems(): marksFIXEDwhen PR becomes mergeable or closed. Guarded bytype === "MERGE_CONFLICT"andstatus === "QUEUED"— idempotent.ingestMergeStateEvent(): now sets type based onmergeStateStatus("dirty"/"conflicting" →MERGE_CONFLICT).- All existing ingestors (
ingestCommentEvent,ingestReviewEvent,ingestCheckRunEvent) now set type explicitly (REVIEW_FEEDBACK,CI_FAILURE).
Queue logic (src/lib/pr-fix-queue.ts)
normalizePrFixType(): proper normalization with hyphens stripped, uppercase comparison.listQueuedPrFixItems(): sorts in-memory byPR_FIX_TYPE_PRIORITYthenqueuedAtwhenprioritizeByType !== false.toAgentQueueItem(): includesfixTypein the returned item — workers can distinguish conflict from feedback items.
API route (src/app/api/pr-fix-queue/queued/route.ts)
- Added
prioritize_by_typequery param (defaulttrue) — mirrors theprioritizeByTypeoption in the library.
Types (src/types/index.ts)
PrFixTypetype,normalizePrFixType(),VALID_PR_FIX_TYPES, andPR_FIX_TYPE_PRIORITYconstant — consistent with the existingPrFixLane/PrFixStatuspattern in the codebase.
Required Check Responses
✅ review migration for data loss risk
The migration adds a new enum type and a non-null column with a default value. No existing rows are deleted or modified destructively. PostgreSQL handles ADD COLUMN ... DEFAULT 'OTHER' without a table rewrite in modern versions (11+), so this is a low-risk online migration. Existing PrFixQueueItem rows will receive type = 'OTHER' by default, which is semantically correct.
⚠️ test migration on a copy of production schema
Cannot verify from the PR corpus. There is no evidence of testing on a production-schema copy in the provided diff, PR body, or linked issue. This is a best-practice recommendation for production deployments: run prisma migrate deploy on a staging snapshot before applying to production. The migration itself is straightforward, but schema size and concurrent write patterns during deployment could reveal edge cases not visible in a clean diff.
Standards Compliance
- ✅ Enum and type patterns follow existing
PrFixLane/PrFixStatusconventions - ✅ Error handling uses
error instanceof Errorpattern in routes - ✅ No agent-specific names in generic code (
isAllowedBotAuthoruses env-configurable allowlist) - ✅ HTTP status codes used appropriately in API routes
- ✅ Audit trail entries written via
prFixHistoryon status changes - ✅ No hardcoded agent names or repo names —
PR_FOLLOWUP_BOT_IDENTITIESandPR_FOLLOWUP_BRANCH_OWNERSare configurable
Linked Issue Fit (PR 355)
| Acceptance Criterion | Status |
|---|---|
Dispatch detects mergeable=CONFLICTING and creates pr-review-fix items with type: "MERGE_CONFLICT" |
✅ ingestMergeConflict() in sync route |
| Queue returns items in priority order: conflicts > CI fails > requested changes | ✅ PR_FIX_TYPE_PRIORITY + in-memory sort |
Workers can distinguish conflict items via fixType field |
✅ toAgentQueueItem() surfaces fixType |
| Already-existing conflict items are not re-created (idempotent) | ✅ Evidence-key deduplication + existence check |
| Conflict items are cleared when PR becomes mergeable or is closed | ✅ clearResolvedConflictItems() sets status: "FIXED" |
Unknowns / Needs Verification
- Production migration testing — recommend verifying the
ADD COLUMN NOT NULL DEFAULTstep on a production-schema snapshot before deployment, especially if thePrFixQueueItemtable is large or sees high write volume. - In-memory sort at scale —
listQueuedPrFixItems()loads all queued items into memory and sorts in-process. This is acceptable for typical queue sizes but would need pagination or a database-sideORDER BYif the queue grows to thousands of items. No evidence of queue-size stress testing in the corpus.
…dering Refs #355 - Add PrFixType enum (MERGE_CONFLICT, CI_FAILURE, REVIEW_FEEDBACK, OTHER) - Add type field to PrFixQueueItem for categorizing fix items - Implement priority ordering: conflicts > CI failures > review feedback - Add ingestMergeConflict() for detecting CONFLICTING PRs - Add clearResolvedConflictItems() for idempotent cleanup - Update sync route to detect conflicts and clear resolved items - Update queue endpoint to support prioritize_by_type parameter - Workers can distinguish conflict items via fixType field
bc8698a to
89827ff
Compare
Fixes #355
Summary
Merge-conflicting PRs are invisible to the current worker system. This PR adds a sync/ingestion step that detects open PRs with merge conflicts and creates
pr-review-fixitems with priority ordering.Changes
MERGE_CONFLICT,CI_FAILURE,REVIEW_FEEDBACK,OTHERtypesingestMergeConflict()function for detecting CONFLICTING PRsclearResolvedConflictItems()clears items when PR becomes mergeable or closedprioritize_by_typeparameter (default: true)Acceptance Criteria
mergeable=CONFLICTINGand createspr-review-fixitems withtype: "MERGE_CONFLICT"fixTypefieldMigration
Added migration
20260611000000_add_pr_fix_typeto add thePrFixTypeenum andtypefield toPrFixQueueItem.