fix(review): stop paging Sentry on an expected update_branch merge conflict - #8077
Conversation
…nflict update_branch performs a real merge internally, so it fails with the same "merge conflict" shape a MERGE action does -- but unlike a merge's terminal hold (a PR permanently blocked until a human intervenes), this is not a stuck state: forceUpdateBranch's caller (prReadyForReview) already falls through to reviewing the PR on its current, non-rebased head when this returns false. The branch owner, not the bot, needs to resolve the conflict; paging on every naturally- diverged PR this happens to hit was pure noise, still recorded via the existing audit() call either way.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-22 19:43:02 UTC
Review summary Nits — 3 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8077 +/- ##
==========================================
+ Coverage 91.98% 97.39% +5.40%
==========================================
Files 748 2 -746
Lines 76616 345 -76271
Branches 23238 143 -23095
==========================================
- Hits 70473 336 -70137
+ Misses 5041 4 -5037
+ Partials 1102 5 -1097
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
update_branchperforms a real merge internally, so it fails with the same "merge conflict" shape a MERGE action does -- but unlike a merge's terminal hold (a PR permanently blocked until a human intervenes), this is NOT a stuck state.forceUpdateBranch's caller (prReadyForReview) already falls through to reviewing the PR on its current, non-rebased head when this returnsfalse-- exactly like every other "couldn't rebase, review anyway" path in that function.audit()call either way.What Changed
src/services/merge-failure.ts: exportedisMergeConflictMessagefor reuse (previously module-private, only used byclassifyMergeFailure's merge-action classification).src/services/agent-action-executor.ts: theupdate_branchaction class now skips the SentrycaptureErrorcall specifically when the failure is a genuine merge conflict; every other non-merge action class (and every OTHER update_branch failure reason) is unaffected.Test plan
update_branchfailure does not callcaptureError.update_branchfailure still pages Sentry exactly as before (proves the existingagent_action_execution_failedpath is unchanged for real failures).npx vitest run test/unit/agent-action-executor.test.ts-- 183/183 passing.npm run test:ci-- green (one unrelated, pre-existing environmental gap:packages/discovery-index's@sentry/clidevDependency was missing fromnode_modules, unrelated to this change -- confirmed by an additive, non-destructive install and a clean re-run of the affected test file in isolation).