diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 60fa9ea..a3f58bc 100644 --- a/.github/copilot-instructions.md +++ b/.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