critical: Fix SL/TP exit P&L compounding with atomic deduplication

CRITICAL BUG FIX: Stop loss and take profit exits were sending duplicate
Telegram notifications with compounding P&L (16 duplicates, 796x inflation).

Real Incident (Dec 2, 2025):
- Manual SOL-PERP SHORT position stopped out
- 16 duplicate Telegram notifications received
- P&L compounding: $0.23 → $12.10 → $24.21 → $183.12 (796× multiplication)
- All showed identical: entry $139.64, hold 4h 5-6m, exit reason SL
- First notification: Ghost detected (handled correctly)
- Next 15 notifications: SL exit (all duplicates with compounding P&L)

Root Cause:
- Multiple monitoring loops detect SL condition simultaneously
- All call executeExit() before any can remove position from tracking
- Race condition: check closingInProgress → both true → both proceed
- Database update happens BEFORE activeTrades.delete()
- Each execution sends Telegram notification
- P&L values compound across notifications

Solution:
Applied same atomic delete pattern as ghost detection fix (commit 93dd950):
- Move activeTrades.delete() to START of executeExit() (before any async operations)
- Check wasInMap return value (only true for first caller, false for duplicates)
- Early return if already deleted (atomic deduplication guard)
- Only first loop proceeds to close, save DB, send notification
- Removed redundant removeTrade() call (already deleted at start)

Impact:
- Prevents duplicate notifications for SL, TP1, TP2, emergency stops
- Ensures accurate P&L reporting (no compounding)
- Database receives correct single exit record
- User receives ONE notification per exit (as intended)

Code Changes:
- Line ~1520: Added atomic delete guard for full closes (percentToClose >= 100)
- Line ~1651: Removed redundant removeTrade() call
- Both changes prevent race condition at function entry

Scope:
-  Stop loss exits: Fixed
-  Take profit 2 exits: Fixed
-  Emergency stops: Fixed
-  Trailing stops: Fixed
- ℹ️ Take profit 1: Not affected (partial close keeps position in monitoring)

Related:
- Ghost detection fix: commit 93dd950 (Dec 2, 2025) - same pattern, different function
- Manual trade enhancement: commit 23277b7 (Dec 2, 2025) - unrelated feature
- P&L compounding series: Common Pitfalls #48-49, #59-61, #67 in docs
This commit is contained in:
mindesbunister
2025-12-02 23:32:09 +01:00
parent 23277b7c87
commit 1a5205c289

View File

@@ -1517,6 +1517,17 @@ export class PositionManager {
*
* Rate limit handling: If 429 error occurs, marks trade for retry
* instead of removing it from monitoring (prevents orphaned positions)
*
* CRITICAL FIX (Dec 2, 2025): Atomic deduplication at function entry
* Bug: Multiple monitoring loops detect SL/TP condition simultaneously
* - All call executeExit() before any can mark position as closing
* - Race condition in later removeTrade() call
* - Each execution sends Telegram notification
* - P&L values compound across notifications (16 duplicates, 796x inflation)
* Fix: Delete from activeTrades FIRST using atomic Map.delete()
* - Only first caller gets wasInMap=true, others get false and return
* - Prevents duplicate database updates, notifications, P&L compounding
* - Same pattern as ghost detection fix (handleExternalClosure)
*/
private async executeExit(
trade: ActiveTrade,
@@ -1524,6 +1535,22 @@ export class PositionManager {
reason: ExitResult['reason'],
currentPrice: number
): Promise<void> {
// CRITICAL FIX (Dec 2, 2025): Atomic deduplication for full closes
// For partial closes (TP1), we DON'T delete yet (position still monitored for TP2)
// For full closes (100%), delete FIRST to prevent duplicate execution
if (percentToClose >= 100) {
const tradeId = trade.id
const wasInMap = this.activeTrades.delete(tradeId)
if (!wasInMap) {
console.log(`⚠️ DUPLICATE EXIT PREVENTED: ${tradeId} already processing ${reason}`)
console.log(` This prevents duplicate Telegram notifications with compounding P&L`)
return
}
console.log(`🗑️ Removed ${trade.symbol} from monitoring (${reason}) - atomic deduplication applied`)
}
try {
console.log(`🔴 Executing ${reason} for ${trade.symbol} (${percentToClose}%)`)
@@ -1646,7 +1673,12 @@ export class PositionManager {
}
}
await this.removeTrade(trade.id)
// CRITICAL: Trade already removed from activeTrades at function start (atomic delete)
// No need to call removeTrade() again - just stop monitoring if empty
if (this.activeTrades.size === 0 && this.isMonitoring) {
this.stopMonitoring()
}
console.log(`✅ Position closed | P&L: $${trade.realizedPnL.toFixed(2)} | Reason: ${reason}`)
// Send Telegram notification