test: Add comprehensive tests for bugs #76, #78, #80

Co-authored-by: mindesbunister <32161838+mindesbunister@users.noreply.github.com>

tests/integration/position-manager/safe-orphan-removal.test.ts

@@ -0,0 +1,266 @@
+/**
+ * Bug #78 Test: Safe Orphan Removal
+ * 
+ * Tests that removeTrade() checks Drift position size before canceling orders
+ * to avoid removing active position protection.
+ * 
+ * Created: Dec 9, 2025
+ * Reason: Bug #78 - cancelAllOrders affects ALL positions on symbol
+ */
+
+import { PositionManager } from '../../../lib/trading/position-manager'
+import { createMockTrade } from '../../helpers/trade-factory'
+
+// Mock dependencies
+jest.mock('../../../lib/drift/client')
+jest.mock('../../../lib/drift/orders')
+jest.mock('../../../lib/pyth/price-monitor')
+jest.mock('../../../lib/database/trades')
+jest.mock('../../../lib/notifications/telegram')
+jest.mock('../../../config/trading')
+
+describe('Bug #78: Safe Orphan Removal', () => {
+  let manager: PositionManager
+  let mockDriftService: any
+  let mockCancelAllOrders: jest.Mock
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+    
+    // Mock Drift service
+    mockDriftService = {
+      getPosition: jest.fn()
+    }
+    
+    const { getDriftService } = require('../../../lib/drift/client')
+    getDriftService.mockReturnValue(mockDriftService)
+    
+    // Mock cancelAllOrders
+    const ordersModule = require('../../../lib/drift/orders')
+    mockCancelAllOrders = jest.fn().mockResolvedValue({ success: true, cancelledCount: 2 })
+    ordersModule.cancelAllOrders = mockCancelAllOrders
+    
+    // Mock Pyth price monitor
+    const { getPythPriceMonitor } = require('../../../lib/pyth/price-monitor')
+    getPythPriceMonitor.mockReturnValue({
+      start: jest.fn().mockResolvedValue(undefined),
+      stop: jest.fn().mockResolvedValue(undefined)
+    })
+    
+    // Mock trading config
+    const { getMarketConfig } = require('../../../config/trading')
+    getMarketConfig.mockReturnValue({
+      driftMarketIndex: 0,
+      minOrderSize: 0.1
+    })
+    
+    manager = new PositionManager()
+  })
+
+  describe('When Drift Position is Closed', () => {
+    it('should cancel orders when Drift confirms position closed (size = 0)', async () => {
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP',
+        direction: 'long'
+      })
+      
+      await manager.addTrade(trade)
+      
+      // Mock Drift position as closed
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 0,
+        side: 'none'
+      })
+      
+      await manager.removeTrade(trade.id)
+      
+      // Should have called cancelAllOrders since position is closed
+      expect(mockCancelAllOrders).toHaveBeenCalledWith('SOL-PERP')
+      expect(mockCancelAllOrders).toHaveBeenCalledTimes(1)
+    })
+
+    it('should cancel orders when Drift position size is negligible (< 0.01)', async () => {
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      // Mock Drift position with negligible size
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 0.005,
+        side: 'long'
+      })
+      
+      await manager.removeTrade(trade.id)
+      
+      // Should cancel since size is negligible
+      expect(mockCancelAllOrders).toHaveBeenCalledWith('SOL-PERP')
+    })
+  })
+
+  describe('When Drift Position is Still Open', () => {
+    it('should NOT cancel orders when Drift shows open position (size >= 0.01)', async () => {
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      // Mock Drift position as still open
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 14.47, // Real position size
+        side: 'long',
+        entryPrice: 138.45
+      })
+      
+      await manager.removeTrade(trade.id)
+      
+      // Should NOT have called cancelAllOrders
+      expect(mockCancelAllOrders).not.toHaveBeenCalled()
+    })
+
+    it('should remove trade from tracking even when skipping order cancellation', async () => {
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      // Verify trade is tracked
+      expect((manager as any).activeTrades.has(trade.id)).toBe(true)
+      
+      // Mock Drift position as open
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 10.5,
+        side: 'long'
+      })
+      
+      await manager.removeTrade(trade.id)
+      
+      // Trade should be removed from tracking
+      expect((manager as any).activeTrades.has(trade.id)).toBe(false)
+      // But orders should not be canceled
+      expect(mockCancelAllOrders).not.toHaveBeenCalled()
+    })
+
+    it('should log warning when skipping cancellation for safety', async () => {
+      const consoleSpy = jest.spyOn(console, 'warn').mockImplementation()
+      
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 12.28,
+        side: 'long'
+      })
+      
+      await manager.removeTrade(trade.id)
+      
+      // Should have logged safety warning
+      expect(consoleSpy).toHaveBeenCalledWith(
+        expect.stringContaining('SAFETY CHECK')
+      )
+      expect(consoleSpy).toHaveBeenCalledWith(
+        expect.stringContaining('Skipping order cancellation')
+      )
+      
+      consoleSpy.mockRestore()
+    })
+  })
+
+  describe('Error Handling', () => {
+    it('should NOT cancel orders on Drift query error (err on side of caution)', async () => {
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      // Mock Drift error
+      mockDriftService.getPosition.mockRejectedValue(new Error('RPC error'))
+      
+      await manager.removeTrade(trade.id)
+      
+      // Should NOT cancel orders on error (safety first)
+      expect(mockCancelAllOrders).not.toHaveBeenCalled()
+      
+      // But should remove from tracking
+      expect((manager as any).activeTrades.has(trade.id)).toBe(false)
+    })
+
+    it('should log error when Drift check fails', async () => {
+      const consoleSpy = jest.spyOn(console, 'error').mockImplementation()
+      const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation()
+      
+      const trade = createMockTrade({ 
+        id: 'trade1',
+        symbol: 'SOL-PERP'
+      })
+      
+      await manager.addTrade(trade)
+      
+      mockDriftService.getPosition.mockRejectedValue(new Error('Network timeout'))
+      
+      await manager.removeTrade(trade.id)
+      
+      expect(consoleSpy).toHaveBeenCalledWith(
+        expect.stringContaining('Error checking Drift position')
+      )
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        expect.stringContaining('Removing from tracking without canceling orders')
+      )
+      
+      consoleSpy.mockRestore()
+      consoleWarnSpy.mockRestore()
+    })
+  })
+
+  describe('Multiple Positions on Same Symbol', () => {
+    it('should preserve orders when removing old position while new position exists', async () => {
+      const oldTrade = createMockTrade({ 
+        id: 'old-trade',
+        symbol: 'SOL-PERP',
+        direction: 'long'
+      })
+      
+      const newTrade = createMockTrade({ 
+        id: 'new-trade',
+        symbol: 'SOL-PERP',
+        direction: 'long'
+      })
+      
+      // Add both trades (simulating orphan detection scenario)
+      await manager.addTrade(oldTrade)
+      await manager.addTrade(newTrade)
+      
+      // Mock Drift showing active position (belongs to new trade)
+      mockDriftService.getPosition.mockResolvedValue({
+        size: 14.47,
+        side: 'long',
+        entryPrice: 138.45
+      })
+      
+      // Remove old trade (orphan cleanup)
+      await manager.removeTrade(oldTrade.id)
+      
+      // Should NOT cancel orders (protects new trade)
+      expect(mockCancelAllOrders).not.toHaveBeenCalled()
+      
+      // Old trade removed
+      expect((manager as any).activeTrades.has(oldTrade.id)).toBe(false)
+      
+      // New trade still tracked
+      expect((manager as any).activeTrades.has(newTrade.id)).toBe(true)
+    })
+  })
+})