critical: Fix duplicate Telegram notifications + settings UI restart requirement

Issue #1: Duplicate Telegram Notifications (Nov 23, 2025)
Symptom: Manual closures sent 2x identical notifications
Root Cause: Monitoring loop processes trades from array snapshot, trade removed
during async processing but loop continues with stale reference

Real Incident:
- Trade cmibdii4k0004pe07nzfmturo (SHORT SOL)
- Entry $128.85, Exit $128.79, P&L +$6.44
- Duplicate 'POSITION CLOSED' messages sent
- Logs show 'Manual closure recorded' twice
- Database saved correctly (only once)

Fix (lib/trading/position-manager.ts):
Added guard at start of checkTradeConditions():
```typescript
  console.log(`⏭️ Skipping ${trade.symbol} - already removed`)
  return
}
```

Why needed: handlePriceUpdate() collects trades into array BEFORE async processing
Loop continues even after handleManualClosure() removes trade from Map
Second iteration processes removed trade → duplicate notification

Issue #2: Settings UI Changes Require Container Restart (Nov 23, 2025)
Symptom: Quality threshold raised to 91 via settings UI, but trade with quality 90
still executed (should've been blocked)

Timeline:
- Nov 21 18:55: Threshold raised to 91 in code (commit 08482b4)
- Nov 22 15:08: Container restarted
- Nov 22 16:15: Trade #9 quality 90 executed  (should've blocked)
- .env file had MIN_SIGNAL_QUALITY_SCORE=81 (old value)

Root Cause: Settings API writes to .env but in-memory process.env update doesn't
propagate to all modules. Container restart required for full effect.

Fix (app/api/settings/route.ts):
Added console warning: "⚠️ Container restart recommended"
Changed comment from "immediate effect" to "temporary, may not persist"

User Impact:
- Settings changes via UI now show proper expectations
- Manual .env edit + restart remains required for critical settings
- Future: Add /api/restart call after settings save

Trade #9 Analysis (quality 90, should've been blocked):
- ADX: 17.8 (weak, below 18 minimum)
- Price Position: 98.6% (extreme high, chasing top)
- Loss: -$22.41 (-0.15%)
- Result: Validates quality 91 threshold works correctly

Commits: 08482b4 (threshold raise), this commit (duplicate fix + restart requirement)
This commit is contained in:
mindesbunister
2025-11-23 10:57:32 +01:00
parent 98e954576b
commit a7c593077d
3 changed files with 13 additions and 3 deletions

2
.env
View File

@@ -390,7 +390,7 @@ NEW_RELIC_LICENSE_KEY=
USE_TRAILING_STOP=true
TRAILING_STOP_PERCENT=0.5
TRAILING_STOP_ACTIVATION=0.4
MIN_SIGNAL_QUALITY_SCORE=81
MIN_SIGNAL_QUALITY_SCORE=91
SOLANA_ENABLED=true
SOLANA_POSITION_SIZE=100
SOLANA_LEVERAGE=15

View File

@@ -52,10 +52,13 @@ function updateEnvFile(updates: Record<string, any>) {
fs.writeFileSync(ENV_FILE_PATH, content, 'utf-8')
// Also update in-memory environment so running process sees new values immediately
// Note: Changes to .env require container restart to take full effect
// In-memory updates below are temporary and may not persist across all modules
Object.entries(updates).forEach(([key, value]) => {
process.env[key] = value
process.env[key] = value.toString()
})
console.log('⚠️ Settings updated in .env file. Container restart recommended for all changes to take effect.')
return true
} catch (error) {
console.error('Failed to write .env file:', error)

View File

@@ -547,6 +547,13 @@ export class PositionManager {
trade: ActiveTrade,
currentPrice: number
): Promise<void> {
// CRITICAL FIX (Nov 23, 2025): Check if trade still in monitoring
// Prevents duplicate processing when async operations remove trade during loop
if (!this.activeTrades.has(trade.id)) {
console.log(`⏭️ Skipping ${trade.symbol} - already removed from monitoring`)
return
}
// CRITICAL: Update lastPrice FIRST so /status always shows current price
// (even if function returns early due to position checks)
trade.lastPrice = currentPrice