docs: Add Common Pitfall #60 (stale array snapshot duplicate processing)

Documented Nov 23, 2025 bug where monitoring loop created array snapshot
before async processing, causing removed trades to be processed twice.

Real incident:
- Trade cmibdii4k0004pe07nzfmturo (manual closure)
- 97% size reduction detected
- First iteration removed trade from Map
- Second iteration processed stale reference
- Result: Duplicate Telegram notifications

Fix: Added activeTrades.has() guard at start of checkTradeConditions()
Prevents duplicate processing when trade removed during loop iteration

Also documented:
- Quality threshold .env discrepancy (81 vs 91)
- Settings UI restart requirement
- Why Next.js modules need container restart for env changes

Related to Common Pitfall #59 (Layer 2 ghost detection duplicates)
but different trigger - normal monitoring vs rate limit storm detection

.github/copilot-instructions.md

@@ -3548,6 +3548,82 @@ trade.realizedPnL += actualRealizedPnL  // NOT: result.realizedPnL from SDK
     - **Git commit:** b19f156 "critical: Fix Layer 2 ghost detection causing duplicate Telegram notifications"
     - **Lesson:** ALL code paths that detect external closures must check `closingInProgress` flag, not just the primary close verification path. Rate limit storms can cause monitoring loop to detect same closure dozens of times if flag isn't checked everywhere.
 
+60. **Stale array snapshot in monitoring loop causes duplicate processing (CRITICAL - Fixed Nov 23, 2025):**
+    - **Symptom:** Manual closure sends duplicate "POSITION CLOSED" Telegram notifications with identical content
+    - **Root Cause:** Position Manager creates array snapshot before async processing, removed trades stay in array during iteration
+    - **Real incident (Nov 23, 07:05 CET):**
+      * Trade cmibdii4k0004pe07nzfmturo (SHORT SOL-PERP)
+      * Entry: $128.85, Exit: $128.79, P&L: +$6.44 (+0.05%)
+      * Hold time: 7 seconds
+      * Exit reason: MANUAL
+      * Size reduction: 97% ($12,940.98 → $388.95)
+      * Logs showed "Manual closure recorded" TWICE
+      * Two identical Telegram notifications sent
+    - **Bug mechanism:**
+    ```typescript
+    // handlePriceUpdate (line 531) - BROKEN FLOW:
+    const tradesForSymbol = Array.from(this.activeTrades.values()) // Snapshot created
+    
+    for (const trade of tradesForSymbol) { // Loop over snapshot
+      await this.checkTradeConditions(trade, update.price) // Async call
+      // First iteration: Detects 97% reduction → handleManualClosure()
+      // handleManualClosure: Updates DB, sends Telegram, calls activeTrades.delete()
+      // Second iteration: Same trade object (stale reference) processed AGAIN
+      // No guard to check if still in map → duplicate DB update + Telegram
+    }
+    ```
+    - **Why it happens:**
+      1. `Array.from(activeTrades.values())` creates snapshot at start of loop
+      2. First iteration detects size reduction, calls `handleManualClosure`
+      3. `handleManualClosure` removes trade via `activeTrades.delete(trade.id)`
+      4. But loop continues with original array that still contains removed trade
+      5. Second iteration processes same trade object (stale reference)
+      6. No check if trade still in monitoring → duplicate processing
+    - **Impact:** Every manual closure vulnerable to duplicate notifications if multiple trades in monitoring loop
+    - **Fix (lib/trading/position-manager.ts line 545):**
+    ```typescript
+    private async checkTradeConditions(
+      trade: ActiveTrade,
+      currentPrice: number
+    ): Promise<void> {
+      // CRITICAL FIX (Nov 23, 2025): Check if trade still in monitoring
+      if (!this.activeTrades.has(trade.id)) {
+        console.log(`⏭️ Skipping ${trade.symbol} - already removed from monitoring`)
+        return
+      }
+      
+      // Continue with normal processing...
+      trade.lastPrice = currentPrice
+      trade.lastUpdateTime = Date.now()
+      trade.priceCheckCount++
+      // ... rest of function
+    }
+    ```
+    - **Behavior now:**
+      * First iteration: Processes trade, removes from map
+      * Second iteration: Guard checks map, trade not found, returns immediately
+      * Prevents duplicate database updates and Telegram notifications
+    - **Why Common Pitfall #59 didn't cover this:**
+      * Pitfall #59: Layer 2 ghost detection (failureCount > 20) during rate limit storms
+      * This bug (#60): Normal monitoring loop during manual closures
+      * Different code paths, different triggers, same pattern needed everywhere
+    - **Configuration discrepancy discovered same session:**
+      * User reported Trade #9 (quality 90, -$22.41) should've been blocked
+      * `.env` had `MIN_SIGNAL_QUALITY_SCORE=81` (outdated)
+      * Code raised threshold to 91 on Nov 21, but `.env` not updated
+      * Fixed: `sed -i 's/MIN_SIGNAL_QUALITY_SCORE=81/91/' .env`
+      * Container restart required for settings changes to propagate
+    - **Settings UI enhancement:**
+      * Added console warning: "⚠️ Container restart recommended"
+      * Changed comment from "immediate effect" to "temporary, may not persist"
+      * Future: Need auto-restart trigger or UI notification
+    - **Files changed:**
+      * `lib/trading/position-manager.ts` (guard added)
+      * `app/api/settings/route.ts` (restart warning added)
+    - **Git commit:** a7c5930 "critical: Fix duplicate Telegram notifications + settings UI restart requirement"
+    - **Deployed:** Nov 23, container rebuilt (71.8s), all services running
+    - **Lesson:** When async processing modifies collections during iteration, always guard against stale references. Array snapshots don't protect against this - need explicit membership checks. ALL monitoring code paths need duplicate prevention, not just error scenarios.
+
 ## File Conventions
 
 - **API routes:** `app/api/[feature]/[action]/route.ts` (Next.js 15 App Router)