docs: Add Common Pitfall #67 - Ghost detection race condition

Bug: 23 duplicate Telegram notifications with P&L compounding (-7.96 to -,129.24)
Cause: Multiple monitoring loops passed has() check before any deleted from Map
Fix: Use Map.delete() atomic return value as deduplication lock
Result: First caller deletes and proceeds, subsequent callers return immediately

Related: #48-49 (TP1 P&L compound), #59-61 (external closure duplicates)
Deployed: Dec 2, 2025 17:32:52 UTC (commit 93dd950)

.github/copilot-instructions.md

@@ -5031,6 +5031,89 @@ trade.realizedPnL += actualRealizedPnL  // NOT: result.realizedPnL from SDK
 66. **Smart Entry Validation Queue wrong price display (CRITICAL - Fixed Dec 1, 2025):**
     - **Symptom:** Abandonment notifications in Telegram showing incorrect/impossible prices (e.g., $126.00 → $98.18 = -22.08% drop in 30 seconds)
     - **User Report:** "the abandonend signal shows the wrong price. it never went that low"
+
+67. **Ghost detection race condition causing duplicate notifications with P&L compounding (CRITICAL - Fixed Dec 2, 2025):**
+    - **Symptom:** 23 duplicate Telegram "POSITION CLOSED" notifications for single manual SHORT trade with P&L compounding from -$47.96 to -$1,129.24
+    - **Real incident (Dec 2, 17:20 CET):**
+      * Manual SHORT opened: Entry $138.84, exit ~$140.20
+      * Expected P&L: ~-$48 (single close)
+      * Actual: 23 identical notifications with compounding P&L (-$47.96 → -$102.25 → -$253.40 → ... → -$1,129.24)
+      * Database: Single trade record with final compounded P&L (-$1,129.24)
+      * Hold time: Consistent 27-29 minutes across all notifications
+      * Exit reason: "Ghost detected during monitoring" then "SL"
+    - **Root Cause:** Race condition in ghost detection cleanup handler
+      ```typescript
+      // BUGGY CODE (OLD):
+      async handleExternalClosure(trade: ActiveTrade, reason: string) {
+        const tradeId = trade.id
+        
+        // ❌ Check happens here - AFTER function entry
+        if (!this.activeTrades.has(tradeId)) {
+          console.log('DUPLICATE PREVENTED')
+          return
+        }
+        
+        // Async operations...
+        const estimatedPnL = calculatePnL() // Takes time
+        
+        // Delete happens late
+        this.activeTrades.delete(tradeId) // ❌ TOO LATE
+        
+        // Multiple loops reach here
+        await updateTradeExit({realizedPnL: estimatedPnL}) // P&L compounds
+        await sendTelegram() // Duplicate notification
+      }
+      
+      // RACE CONDITION TIMELINE:
+      // t=0ms:   Loop 1 calls handleExternalClosure()
+      // t=0ms:   Loop 1 checks has(tradeId) → TRUE (exists)
+      // t=2000ms: Loop 2 calls handleExternalClosure()
+      // t=2000ms: Loop 2 checks has(tradeId) → TRUE (still exists!)
+      // t=2010ms: Loop 1 deletes from Map
+      // t=2015ms: Loop 2 also "deletes" (no-op but continues)
+      // t=2100ms: Loop 1 sends Telegram notification
+      // t=2105ms: Loop 2 sends Telegram notification (DUPLICATE)
+      ```
+    - **Fix:** Use Map.delete() atomic return value as deduplication lock
+      ```typescript
+      // FIXED CODE (NEW):
+      async handleExternalClosure(trade: ActiveTrade, reason: string) {
+        const tradeId = trade.id
+        
+        // ✅ Delete IMMEDIATELY - atomic operation
+        // Map.delete() returns true if deleted, false if key didn't exist
+        if (!this.activeTrades.delete(tradeId)) {
+          console.log('DUPLICATE PREVENTED (atomic lock)')
+          return
+        }
+        
+        console.log('Removed from monitoring (lock acquired)')
+        
+        // ONLY first caller reaches here
+        const estimatedPnL = calculatePnL()
+        await updateTradeExit({realizedPnL: estimatedPnL})
+        await sendTelegram() // No duplicates possible
+      }
+      
+      // ATOMIC TIMELINE:
+      // t=0ms:   Loop 1 calls handleExternalClosure()
+      // t=0ms:   Loop 1 calls delete(tradeId) → Returns TRUE (deleted)
+      // t=0ms:   Loop 1 proceeds with cleanup
+      // t=2000ms: Loop 2 calls handleExternalClosure()
+      // t=2000ms: Loop 2 calls delete(tradeId) → Returns FALSE (not found)
+      // t=2000ms: Loop 2 early returns (no notification)
+      ```
+    - **Key Technical Insight:** JavaScript Map.delete() is:
+      * **Atomic:** Single operation, no race condition possible
+      * **Boolean return:** true if key existed and was deleted, false if key didn't exist
+      * **Perfect lock:** First caller gets true, subsequent callers get false
+      * **No async gap:** Deletion happens before any await statements
+    - **Impact:** Every ghost detection event vulnerable to duplicate notifications if multiple monitoring loops detected same ghost
+    - **Files changed:** `lib/trading/position-manager.ts` lines 380-430 (handleExternalClosure method)
+    - **Git commit:** 93dd950 "critical: Fix ghost detection P&L compounding - delete from Map BEFORE check"
+    - **Deployed:** Dec 2, 2025 17:32:52 UTC
+    - **Related:** Common Pitfalls #48-49 (TP1 P&L compound), #59-61 (external closure duplicates)
+    - **Lesson:** When async handler can be called by multiple code paths simultaneously, use atomic operations (like Map.delete()) as locks at function entry, NOT checks (like Map.has()) that can race
     - **Root Cause:** Symbol format mismatch between validation queue storage and market data cache lookup
       * TradingView webhook sends: `"SOLUSDT"`, `"ETHUSDT"`, etc.
       * check-risk endpoint passed: `body.symbol` directly to validation queue without normalization