fix(manager,api): fix trader deletion logic to prevent ghost traders and cache issues

[the-dev-z]

Pull Request - Backend | 后端 PR

💡 提示 Tip: 推荐 PR 标题格式 type(scope): description
例如: fix(manager,api): fix trader deletion logic to prevent ghost traders

---

📝 Description | 描述

English:

This PR fixes critical bugs in the trader deletion logic that caused memory leaks and stale data:

Problems Fixed:

1. P0 - Ghost traders: Wrong deletion order (DB first → memory second) caused traders to run indefinitely in memory without database records
2. P1 - Stale competition rankings: Deleted traders still appeared in rankings due to no cache clearing
3. P2 - Duplicate names: No validation prevented users from creating traders with identical names

Solution:

• Added TraderManager.RemoveTrader() method for safe memory cleanup with competition cache clearing
• Fixed handleDeleteTrader() order: memory cleanup first → database deletion second
• Added duplicate name validation in handleCreateTrader()

中文：

本 PR 修復了交易員刪除邏輯中的嚴重 bug，這些 bug 導致內存洩漏和數據陳舊：

修復的問題：

1. P0 - Ghost traders（幽靈交易員）: 錯誤的刪除順序（先刪數據庫 → 後清理內存）導致交易員在內存中無限期運行，但數據庫記錄已刪除
2. P1 - 競賽排名陳舊: 已刪除的交易員仍出現在排行榜中，因為沒有清除緩存
3. P2 - 重名交易員: 沒有驗證，用戶可以創建同名交易員

解決方案：

• 添加 TraderManager.RemoveTrader() 方法，安全清理內存並清除競賽緩存
• 修復 handleDeleteTrader() 順序：先清理內存 → 後刪除數據庫
• 在 handleCreateTrader() 中添加重名驗證

---

🎯 Type of Change | 变更类型

• ✨ New feature | 新功能
• 🎨 Code style update | 代码样式更新
• ♻️ Refactoring | 重构
• 🐛 Bug fix | 修复 Bug
• 💥 Breaking change | 破坏性变更
• ⚡ Performance improvement | 性能优化

---

🔗 Related Issues | 相关 Issue

Fixes:

• Memory leak causing ghost traders running indefinitely
• Competition rankings showing deleted traders
• Users confused by duplicate trader names

Related:

• May help with stability issues reported by users
• Improves system reliability and data consistency

---

📋 Changes Made | 具体变更

1. Add TraderManager.RemoveTrader() method

File: manager/trader_manager.go (lines 428-468)

Implementation:

func (tm *TraderManager) RemoveTrader(traderID string) error {
    tm.mu.Lock()
    defer tm.mu.Unlock()
    
    trader, exists := tm.traders[traderID]
    if !exists {
        return fmt.Errorf("trader ID '%s' 不存在", traderID)
    }
    
    // 1. Stop trader if running (wait up to 5 seconds)
    status := trader.GetStatus()
    if isRunning {
        trader.Stop()
        // Wait with timeout...
    }
    
    // 2. Remove from memory map
    delete(tm.traders, traderID)
    
    // 3. Clear competition cache
    tm.competitionCache.mu.Lock()
    tm.competitionCache.data = nil
    tm.competitionCache.timestamp = time.Time{}
    tm.competitionCache.mu.Unlock()
    
    return nil
}

Features:

• ✅ Thread-safe with mutex
• ✅ Graceful stop with 5-second timeout
• ✅ Auto-clears competition cache
• ✅ Detailed logging

---

2. Fix handleDeleteTrader() order

File: api/server.go (lines 765-784)

Before (❌ Wrong order):

func handleDeleteTrader() {
    // 1. Delete from database first
    s.database.DeleteTrader(userID, traderID)
    
    // 2. Try to stop trader (may fail if DB gone)
    if trader, err := s.traderManager.GetTrader(traderID); err == nil {
        trader.Stop()
    }
}

Problem: If trader is running, it continues running after DB deletion (ghost trader)

After (✅ Correct order):

func handleDeleteTrader() {
    // 1. Stop & remove from memory first
    s.traderManager.RemoveTrader(traderID)
    
    // 2. Delete from database last
    s.database.DeleteTrader(userID, traderID)
}

Benefits:

• Memory always cleaned up (no leaks)
• Competition cache auto-cleared
• Atomic deletion semantics

---

3. Add duplicate name validation

File: api/server.go (lines 488-499)

Implementation:

// Check for duplicate trader names
existingTraders, err := s.database.GetTraders(userID)
if err != nil {
    return HTTP 500
}

for _, existing := range existingTraders {
    if existing.Name == req.Name {
        return HTTP 400 "交易员名称 'XXX' 已存在，请使用其他名称"
    }
}

Benefits:

• Prevents user confusion
• Clear error message
• Better UX

---

🧪 Testing | 测试

Test Environment | 测试环境

• OS | 操作系统: macOS 14.5
• Go Version | Go 版本: 1.21+
• Database: SQLite

Manual Testing | 手动测试

• Delete running trader → correctly stopped and removed
• Delete stopped trader → correctly removed
• Competition rankings update immediately after deletion
• Cannot create traders with duplicate names

Automated Testing | 自动化测试

• Go test compilation: ✅ Passed
• Unit tests: ✅ All passed
• Go formatting: ✅ Applied
• Go vet: ✅ No issues

Test commands:

go fmt ./...                      # ✅ Formatted
go vet ./...                      # ✅ No issues
go test ./api/... ./manager/...   # ✅ All tests passed

---

📊 Impact Analysis | 影响分析

Before (Issues)

Problem	Severity	Impact
Ghost traders	🔴 P0	Memory leak, system instability
Stale rankings	🟡 P1	User confusion, incorrect data
Duplicate names	🟢 P2	Poor UX

After (Fixed)

Area	Improvement
Memory management	No leaks, clean shutdown
Data consistency	Always accurate rankings
User experience	Clear validation, no duplicates
System stability	More reliable, predictable

---

✅ Checklist | 检查清单

Code Quality | 代码质量

• Code follows project style | 代码遵循项目风格
• Self-review completed | 已完成代码自查
• Comments added for complex logic | 已添加必要注释
• Code builds successfully | 代码构建成功
• No compilation errors or warnings | 无编译错误或警告

Testing | 测试

• Unit tests pass | 单元测试通过
• Manual testing completed | 完成手动测试
• Go formatting applied | 已应用 Go 格式化
• Go vet passed | go vet 通过

Documentation | 文档

• Updated relevant documentation | 已更新相关文档 (inline comments)
• Updated type definitions (if needed) | 已更新类型定义 (N/A)
• Added detailed commit message | 已添加详细提交信息

Git

• Commits follow conventional format | 提交遵循 Conventional Commits 格式
• Rebased on latest dev branch | 已 rebase 到最新 dev 分支
• No merge conflicts | 无合并冲突

---

💡 Additional Notes | 补充说明

Why this order matters

Incorrect order (DB first):

1. User clicks "Delete trader"
2. DB record deleted ✅
3. Try to stop trader → ❌ May fail or be skipped
4. Result: Trader runs forever in memory (ghost trader)

Correct order (Memory first):

1. User clicks "Delete trader"
2. Stop & remove from memory ✅ (with RemoveTrader)
3. Clear competition cache ✅
4. Delete DB record ✅
5. Result: Clean, predictable deletion

Future improvements

• Consider adding soft delete (mark as deleted, cleanup later)
• Add metrics for tracking deletion operations
• Add unit tests for RemoveTrader() method specifically

---

By submitting this PR, I confirm | 提交此 PR，我确认：

• I have read the Contributing Guidelines | 已阅读贡献指南
• I agree to the Code of Conduct | 同意行为准则
• My contribution is licensed under AGPL-3.0 | 贡献遵循 AGPL-3.0 许可证

---

🌟 Thank you for reviewing! | 感谢审阅！

This PR fixes critical memory leak issues and improves system reliability.

[github-actions]

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟢 Small (116 lines: +84 -32)

🔧 Backend Checks

Go Formatting: ✅ Good
Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

---

📖 Resources

• Contributing Guidelines
• Migration Guide

Questions? Feel free to ask in the comments! 🙏

---

These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

[hzb1115]

I think we can check status first and then sleep. Otherwise we have to wait everytime

[hzb1115]

Do we need to clear all the cache? Could we add a function to remove this trader from cache?