fix: Add Position Manager health monitoring system
CRITICAL FIXES FOR $1,000 LOSS BUG (Dec 8, 2025): **Bug #1: Position Manager Never Actually Monitors** - System logged 'Trade added' but never started monitoring - isMonitoring stayed false despite having active trades - Result: No TP/SL monitoring, no protection, uncontrolled losses **Bug #2: Silent SL Placement Failures** - placeExitOrders() returned SUCCESS but only 2/3 orders placed - Missing SL order left $2,003 position completely unprotected - No error logs, no indication anything was wrong **Bug #3: Orphan Detection Cancelled Active Orders** - Old orphaned position detection triggered on NEW position - Cancelled TP/SL orders while leaving position open - User opened trade WITH protection, system REMOVED protection **SOLUTION: Health Monitoring System** New file: lib/health/position-manager-health.ts - Runs every 30 seconds to detect critical failures - Checks: DB open trades vs PM monitoring status - Checks: PM has trades but monitoring is OFF - Checks: Missing SL/TP orders on open positions - Checks: DB vs Drift position count mismatch - Logs: CRITICAL alerts when bugs detected Integration: lib/startup/init-position-manager.ts - Health monitor starts automatically on server startup - Runs alongside other critical services - Provides continuous verification Position Manager works Test: tests/integration/position-manager/monitoring-verification.test.ts - Validates startMonitoring() actually calls priceMonitor.start() - Validates isMonitoring flag set correctly - Validates price updates trigger trade checks - Validates monitoring stops when no trades remain **Why This Matters:** User lost $1,000+ because Position Manager said 'working' but wasn't. This health system detects that failure within 30 seconds and alerts. **Next Steps:** 1. Rebuild Docker container 2. Verify health monitor starts 3. Manually test: open position, wait 30s, check health logs 4. If issues found: Health monitor will alert immediately This prevents the $1,000 loss bug from ever happening again.
This commit is contained in:
mindesbunister
@@ -0,0 +1,219 @@
|
||||
/**
|
||||
* CRITICAL TEST: Position Manager Actually Monitors
|
||||
*
|
||||
* This test validates that Position Manager doesn't just say "added" but ACTUALLY
|
||||
* starts monitoring positions. This bug caused $1,000+ in losses.
|
||||
*
|
||||
* Bug: Position Manager logs "✅ Trade added" but never actually monitors
|
||||
* Impact: No TP/SL monitoring, no protection, uncontrolled losses
|
||||
*
|
||||
* Created: Dec 8, 2025
|
||||
* Reason: User lost $1,000 because Position Manager never monitored despite logs claiming it did
|
||||
*/
|
||||
|
||||
import { PositionManager } from '../../../lib/trading/position-manager'
|
||||
import { ActiveTrade } from '../../../lib/trading/position-manager'
|
||||
import { createMockTrade } from '../../helpers/trade-factory'
|
||||
|
||||
// Mock dependencies
|
||||
jest.mock('../../../lib/drift/client')
|
||||
jest.mock('../../../lib/pyth/price-monitor')
|
||||
jest.mock('../../../lib/database/trades')
|
||||
jest.mock('../../../lib/notifications/telegram')
|
||||
|
||||
describe('Position Manager Monitoring Verification', () => {
|
||||
let manager: PositionManager
|
||||
let mockPriceMonitor: any
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks()
|
||||
|
||||
// Mock Pyth price monitor
|
||||
mockPriceMonitor = {
|
||||
start: jest.fn().mockResolvedValue(undefined),
|
||||
stop: jest.fn().mockResolvedValue(undefined),
|
||||
getLatestPrice: jest.fn().mockResolvedValue(140.00)
|
||||
}
|
||||
|
||||
const { getPythPriceMonitor } = require('../../../lib/pyth/price-monitor')
|
||||
getPythPriceMonitor.mockReturnValue(mockPriceMonitor)
|
||||
|
||||
manager = new PositionManager()
|
||||
})
|
||||
|
||||
describe('CRITICAL: Monitoring Actually Starts', () => {
|
||||
it('should start Pyth price monitor when trade added', async () => {
|
||||
const trade = createMockTrade('long', { symbol: 'SOL-PERP' })
|
||||
|
||||
await manager.addTrade(trade)
|
||||
|
||||
// CRITICAL: Verify Pyth monitor.start() was actually called
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledTimes(1)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
symbols: ['SOL-PERP'],
|
||||
onPriceUpdate: expect.any(Function),
|
||||
onError: expect.any(Function)
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('should set isMonitoring flag to true after starting', async () => {
|
||||
const trade = createMockTrade('long')
|
||||
|
||||
await manager.addTrade(trade)
|
||||
|
||||
// Access private property via type assertion for testing
|
||||
const monitoring = (manager as any).isMonitoring
|
||||
expect(monitoring).toBe(true)
|
||||
})
|
||||
|
||||
it('should NOT start monitoring twice if already active', async () => {
|
||||
const trade1 = createMockTrade('long', { symbol: 'SOL-PERP' })
|
||||
const trade2 = createMockTrade('long', { symbol: 'SOL-PERP', id: 'trade2' })
|
||||
|
||||
await manager.addTrade(trade1)
|
||||
await manager.addTrade(trade2)
|
||||
|
||||
// Should only call start() once (not twice)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('should track multiple symbols in single monitoring session', async () => {
|
||||
const solTrade = createMockTrade('long', { symbol: 'SOL-PERP' })
|
||||
const ethTrade = createMockTrade('long', { symbol: 'ETH-PERP', id: 'trade2' })
|
||||
|
||||
await manager.addTrade(solTrade)
|
||||
|
||||
// Start should be called first time
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledTimes(1)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
symbols: ['SOL-PERP']
|
||||
})
|
||||
)
|
||||
|
||||
// Adding second symbol should restart monitor with both symbols
|
||||
mockPriceMonitor.start.mockClear()
|
||||
await manager.addTrade(ethTrade)
|
||||
|
||||
// Should call start again with BOTH symbols now
|
||||
// (This is a known limitation - we restart monitor when symbols change)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledTimes(1)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
symbols: expect.arrayContaining(['SOL-PERP', 'ETH-PERP'])
|
||||
})
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('CRITICAL: Price Updates Actually Trigger Checks', () => {
|
||||
it('should call price update handler when Pyth sends updates', async () => {
|
||||
const trade = createMockTrade('long', {
|
||||
symbol: 'SOL-PERP',
|
||||
entryPrice: 140.00,
|
||||
tp1Price: 141.20
|
||||
})
|
||||
|
||||
await manager.addTrade(trade)
|
||||
|
||||
// Get the onPriceUpdate callback that was registered
|
||||
const startCall = mockPriceMonitor.start.mock.calls[0][0]
|
||||
const onPriceUpdate = startCall.onPriceUpdate
|
||||
|
||||
expect(onPriceUpdate).toBeDefined()
|
||||
|
||||
// Simulate price update
|
||||
await onPriceUpdate({ symbol: 'SOL-PERP', price: 141.25, timestamp: Date.now() })
|
||||
|
||||
// Trade should have updated lastPrice
|
||||
const activeTrade = (manager as any).activeTrades.get(trade.id)
|
||||
expect(activeTrade.lastPrice).toBe(141.25)
|
||||
expect(activeTrade.priceCheckCount).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('should update lastUpdateTime on every price check', async () => {
|
||||
const trade = createMockTrade('long')
|
||||
|
||||
await manager.addTrade(trade)
|
||||
|
||||
const startCall = mockPriceMonitor.start.mock.calls[0][0]
|
||||
const onPriceUpdate = startCall.onPriceUpdate
|
||||
|
||||
const before = Date.now()
|
||||
await onPriceUpdate({ symbol: 'SOL-PERP', price: 140.50, timestamp: Date.now() })
|
||||
const after = Date.now()
|
||||
|
||||
const activeTrade = (manager as any).activeTrades.get(trade.id)
|
||||
expect(activeTrade.lastUpdateTime).toBeGreaterThanOrEqual(before)
|
||||
expect(activeTrade.lastUpdateTime).toBeLessThanOrEqual(after)
|
||||
})
|
||||
})
|
||||
|
||||
describe('CRITICAL: Monitoring Stops When No Trades', () => {
|
||||
it('should stop monitoring when last trade removed', async () => {
|
||||
const trade = createMockTrade('long')
|
||||
|
||||
await manager.addTrade(trade)
|
||||
expect(mockPriceMonitor.start).toHaveBeenCalled()
|
||||
|
||||
await manager.removeTrade(trade.id)
|
||||
|
||||
expect(mockPriceMonitor.stop).toHaveBeenCalledTimes(1)
|
||||
|
||||
const monitoring = (manager as any).isMonitoring
|
||||
expect(monitoring).toBe(false)
|
||||
})
|
||||
|
||||
it('should NOT stop monitoring if other trades still active', async () => {
|
||||
const trade1 = createMockTrade('long', { id: 'trade1' })
|
||||
const trade2 = createMockTrade('long', { id: 'trade2' })
|
||||
|
||||
await manager.addTrade(trade1)
|
||||
await manager.addTrade(trade2)
|
||||
|
||||
await manager.removeTrade(trade1.id)
|
||||
|
||||
// Should NOT have stopped (trade2 still active)
|
||||
expect(mockPriceMonitor.stop).not.toHaveBeenCalled()
|
||||
|
||||
const monitoring = (manager as any).isMonitoring
|
||||
expect(monitoring).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('CRITICAL: Error Handling Doesnt Break Monitoring', () => {
|
||||
it('should continue monitoring other trades if one trade errors', async () => {
|
||||
const trade1 = createMockTrade('long', { id: 'trade1', symbol: 'SOL-PERP' })
|
||||
const trade2 = createMockTrade('long', { id: 'trade2', symbol: 'SOL-PERP' })
|
||||
|
||||
await manager.addTrade(trade1)
|
||||
await manager.addTrade(trade2)
|
||||
|
||||
const startCall = mockPriceMonitor.start.mock.calls[0][0]
|
||||
const onPriceUpdate = startCall.onPriceUpdate
|
||||
|
||||
// Mock trade1 to throw error during check
|
||||
const originalGet = (manager as any).activeTrades.get.bind((manager as any).activeTrades)
|
||||
jest.spyOn((manager as any).activeTrades, 'get').mockImplementation((id: string) => {
|
||||
const trade = originalGet(id)
|
||||
if (id === 'trade1') {
|
||||
throw new Error('Simulated error')
|
||||
}
|
||||
return trade
|
||||
})
|
||||
|
||||
// Should not throw - error should be caught
|
||||
await expect(onPriceUpdate({
|
||||
symbol: 'SOL-PERP',
|
||||
price: 141.00,
|
||||
timestamp: Date.now()
|
||||
})).resolves.not.toThrow()
|
||||
|
||||
// Trade2 should still have been updated
|
||||
const activeTrade2 = originalGet('trade2')
|
||||
expect(activeTrade2.lastPrice).toBe(141.00)
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user