From a3a6222047f10a6fe37cfcd843f8fff6626eb447 Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Thu, 20 Nov 2025 14:52:29 +0100 Subject: [PATCH] critical: Cancel ghost orders after external closures - Added order cancellation to Position Manager's external closure handler - When on-chain SL/TP orders close position, remaining orders now cancelled automatically - Prevents ghost orders from triggering unintended positions - Real incident: Nov 20 SHORT stop-out left 32 ghost orders on Drift - Risk: Ghost TP1 at $140.66 could fill later, creating unwanted LONG position - Fix: Import cancelAllOrders() and call after trade removed from monitoring - Non-blocking: Logs errors but doesn't fail trade closure if cancellation fails - Files: lib/trading/position-manager.ts (external closure handler ~line 920) - Documented as Common Pitfall #56 --- .github/copilot-instructions.md | 58 +++++++++++++++++++++++++++++++++ lib/trading/position-manager.ts | 17 ++++++++++ 2 files changed, 75 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 66d1f36..ed4d07e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2884,6 +2884,64 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK - **Commits:** 6b00303 "fix: BlockedSignalTracker now uses Drift oracle prices" - **Impact:** Multi-timeframe data collection now operational for Phase 1 analysis (50+ signals per timeframe target) - **Lesson:** Background jobs should use Drift oracle prices (always available) not Pyth cache (real-time only). Always initialize external services before calling their methods. Verify background jobs are actually working by checking database state, not just logs. + +56. **Ghost orders after external closures (CRITICAL - Fixed Nov 20, 2025):** + - **Symptom:** Position closed externally (on-chain SL/TP order filled), but TP/SL orders remain active on Drift + - **Root Cause:** Position Manager's external closure handler didn't call `cancelAllOrders()` before completing trade + - **Real incident (Nov 20, 13:30 CET):** + * SHORT position stopped out at $142.48 + * Position closed successfully on Drift + * TP1 order at $140.66 still active (32 total ghost orders found!) + * Manual cleanup via `/api/trading/cancel-orders` cancelled 32 orders + * Risk: If price dropped to $140.66 later, ghost order would fill → unintended LONG position + - **Impact:** Every external closure (SL/TP fills) leaves ghost orders on exchange + - **Why dangerous:** + * Ghost orders can trigger unintended positions if price moves to those levels + * User may be away, not monitoring ghost order execution + * Creates unlimited risk exposure from positions you don't know exist + * Clogs order management, makes UI confusing + - **Fix (Nov 20, 2025):** + ```typescript + // In lib/trading/position-manager.ts external closure handler (line ~920): + + this.activeTrades.delete(tradeId) + console.log(`🗑️ Removed trade ${tradeId} from monitoring (BEFORE DB update to prevent duplicates)`) + console.log(` Active trades remaining: ${this.activeTrades.size}`) + + // CRITICAL: Cancel all remaining orders for this position (ghost order cleanup) + // When position closes externally (on-chain SL/TP), TP/SL orders may remain active + // These ghost orders can trigger unintended positions if price moves to those levels + console.log(`🗑️ Cancelling remaining orders for ${trade.symbol}...`) + try { + const { cancelAllOrders } = await import('../drift/orders') + const cancelResult = await cancelAllOrders(trade.symbol) + if (cancelResult.success) { + console.log(`✅ Cancelled ${cancelResult.cancelledCount || 0} ghost orders`) + } else { + console.error(`⚠️ Failed to cancel orders: ${cancelResult.error}`) + } + } catch (cancelError) { + console.error('❌ Error cancelling ghost orders:', cancelError) + // Don't fail the trade closure if order cancellation fails + } + + try { + await updateTradeExit({ + // ... database update continues + }) + } + ``` + - **Behavior now:** + * External closure detected (on-chain order filled) + * Trade removed from monitoring + * **IMMEDIATELY cancel all remaining orders** for that symbol + * Update database with exit details + * Stop monitoring if no more trades + * Clean slate - no ghost orders left + - **Why 32 orders:** Drift SDK's `userAccount.orders` array has 32 order slots, bot found SOL-PERP orders in many slots from current + previous trades + - **Files changed:** `lib/trading/position-manager.ts` - Added order cancellation to external closure handler + - **Commit:** [pending] "critical: Cancel ghost orders after external closures" + - **Lesson:** When detecting external closures, always clean up ALL related on-chain state (orders, positions). Ghost orders are financial risks - they can execute when you're not watching. ```typescript // In app/api/settings/route.ts (lines ~150, ~270) // BEFORE (BROKEN): diff --git a/lib/trading/position-manager.ts b/lib/trading/position-manager.ts index 5f7ca8a..193e183 100644 --- a/lib/trading/position-manager.ts +++ b/lib/trading/position-manager.ts @@ -924,6 +924,23 @@ export class PositionManager { console.log(`🗑️ Removed trade ${tradeId} from monitoring (BEFORE DB update to prevent duplicates)`) console.log(` Active trades remaining: ${this.activeTrades.size}`) + // CRITICAL: Cancel all remaining orders for this position (ghost order cleanup) + // When position closes externally (on-chain SL/TP), TP/SL orders may remain active + // These ghost orders can trigger unintended positions if price moves to those levels + console.log(`🗑️ Cancelling remaining orders for ${trade.symbol}...`) + try { + const { cancelAllOrders } = await import('../drift/orders') + const cancelResult = await cancelAllOrders(trade.symbol) + if (cancelResult.success) { + console.log(`✅ Cancelled ${cancelResult.cancelledCount || 0} ghost orders`) + } else { + console.error(`⚠️ Failed to cancel orders: ${cancelResult.error}`) + } + } catch (cancelError) { + console.error('❌ Error cancelling ghost orders:', cancelError) + // Don't fail the trade closure if order cancellation fails + } + try { await updateTradeExit({ positionId: trade.positionId,