feat: add new log retention setting (#65)

* feat: Implement configurable log file retention and cleanup, including a verification script.

* test: add `TestCleanupLogs` to verify correct log file cleanup and retention.

* feat: Add log retention count setting to the TUI.

* feat: Introduce Go duration parsing and update download benchmark to use `server start` command with refined time extraction.

* feat: call SyncSessionStart when initializing a fresh download state.

---------

Co-authored-by: Meet Mehta <supercoolpencil@duck.com>

Author: SuperCoolPencil

benchmark.py

@@ -14,6 +14,7 @@
 import tempfile
 import time
 import random
+import re
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Optional
@@ -93,6 +94,7 @@ def get_file_size(path: Path) -> int:
     return 0
 
 
+
 def cleanup_file(path: Path):
     """Remove a file if it exists."""
     try:
@@ -102,6 +104,29 @@ def cleanup_file(path: Path):
         pass
 
 
+def parse_go_duration(s: str) -> float:
+    """Parse Go duration string to seconds."""
+    total = 0.0
+    # Match pairs like 1h, 2m, 3.5s, 500ms, 10µs, 100ns
+    matches = re.findall(r'(\d+(?:\.\d+)?)(ns|us|µs|ms|s|m|h)', s)
+    for val_str, unit in matches:
+        val = float(val_str)
+        if unit == 'h':
+            total += val * 3600
+        elif unit == 'm':
+            total += val * 60
+        elif unit == 's':
+            total += val
+        elif unit == 'ms':
+            total += val / 1000
+        elif unit in ('us', 'µs'):
+            total += val / 1_000_000
+        elif unit == 'ns':
+            total += val / 1_000_000_000
+            
+    return total
+
+
 # =============================================================================
 # SETUP FUNCTIONS
 # =============================================================================
@@ -155,25 +180,25 @@ def benchmark_surge(executable: Path, url: str, output_dir: Path, label: str = "
         return BenchmarkResult(label, False, 0, 0, f"Binary not found: {executable}")
 
     start = time.perf_counter()
+    # Use server start mode with exit-when-done
     success, output = run_command([
-        str(executable), "get", url,
-        "--output", str(output_dir),  # Download directory
+        str(executable), "server", "start", url,
+        "--output", str(output_dir),
+        "--exit-when-done"
     ], timeout=600)
     elapsed = time.perf_counter() - start
 
-    # Try to parse the actual download time from Surge output (excluding probing)
-    # Output format: "Complete: 1.0 GB in 5.2s (196.34 MB/s)" OR "... in 500ms ..."
-    import re
+    # Try to parse the actual download time from Surge output
+    # Output format: "Completed: filename [id] (in 5.2s)"
     actual_time = elapsed
-    match = re.search(r"in ([\d\.]+)(m?s)", output)
+    match = re.search(r"Completed: .*? \[.*?\] \(in (.*?)\)", output)
     if match:
         try:
-            val = float(match.group(1))
-            unit = match.group(2)
-            if unit == "ms":
-                val /= 1000.0
-            actual_time = val
-        except ValueError:
+            duration_str = match.group(1)
+            parsed_time = parse_go_duration(duration_str)
+            if parsed_time > 0:
+                actual_time = parsed_time
+        except Exception:
             pass
 
     # Find downloaded file (surge uses original filename)

cmd/root.go

@@ -729,6 +729,16 @@ func initializeGlobalState() {
 
 	// Config logging
 	utils.ConfigureDebug(logsDir)
+
+	// Clean up old logs
+	settings, err := config.LoadSettings()
+	retention := 5 // default fallback
+	if err == nil {
+		retention = settings.General.LogRetentionCount
+	} else {
+		retention = config.DefaultSettings().General.LogRetentionCount
+	}
+	utils.CleanupLogs(retention)
 }
 
 // convertRuntimeConfig converts config.RuntimeConfig to types.RuntimeConfig

internal/config/settings.go

@@ -25,6 +25,7 @@ type GeneralSettings struct {
 	MaxConcurrentDownloads int    `json:"max_concurrent_downloads"`
 	ClipboardMonitor       bool   `json:"clipboard_monitor"`
 	Theme                  int    `json:"theme"`
+	LogRetentionCount      int    `json:"log_retention_count"`
 }
 
 const (
@@ -77,6 +78,7 @@ func GetSettingsMetadata() map[string][]SettingMeta {
 			{Key: "max_concurrent_downloads", Label: "Max Concurrent Downloads", Description: "Maximum number of downloads running at once (1-10). Requires restart.", Type: "int"},
 			{Key: "clipboard_monitor", Label: "Clipboard Monitor", Description: "Watch clipboard for URLs and prompt to download them.", Type: "bool"},
 			{Key: "theme", Label: "App Theme", Description: "UI Theme (System, Light, Dark).", Type: "int"},
+			{Key: "log_retention_count", Label: "Log Retention Count", Description: "Number of recent log files to keep.", Type: "int"},
 		},
 		"Connections": {
 			{Key: "max_connections_per_host", Label: "Max Connections/Host", Description: "Maximum concurrent connections per host (1-64).", Type: "int"},
@@ -123,6 +125,7 @@ func DefaultSettings() *Settings {
 			MaxConcurrentDownloads: 3,
 			ClipboardMonitor:       true,
 			Theme:                  ThemeAdaptive,
+			LogRetentionCount:      5,
 		},
 		Connections: ConnectionSettings{
 			MaxConnectionsPerHost: 32,

internal/engine/concurrent/downloader.go

@@ -219,6 +219,7 @@ func (d *ConcurrentDownloader) Download(ctx context.Context, rawurl, destPath st
 		// Robustness: ensure state counter starts at 0 for fresh download
 		if d.State != nil {
 			d.State.Downloaded.Store(0)
+			d.State.SyncSessionStart()
 		}
 	}
 	queue := NewTaskQueue()

internal/tui/settings_view.go

@@ -219,6 +219,7 @@ func (m RootModel) getSettingsValues(category string) map[string]interface{} {
 		values["max_concurrent_downloads"] = m.Settings.General.MaxConcurrentDownloads
 		values["clipboard_monitor"] = m.Settings.General.ClipboardMonitor
 		values["theme"] = m.Settings.General.Theme
+		values["log_retention_count"] = m.Settings.General.LogRetentionCount
 
 	case "Connections":
 		values["max_connections_per_host"] = m.Settings.Connections.MaxConnectionsPerHost
@@ -313,6 +314,13 @@ func (m *RootModel) setGeneralSetting(key, value, typ string) error {
 		}
 		m.Settings.General.Theme = theme
 		m.ApplyTheme(theme)
+	case "log_retention_count":
+		if v, err := strconv.Atoi(value); err == nil {
+			if v < 0 {
+				v = 0 // Minimum valid value
+			}
+			m.Settings.General.LogRetentionCount = v
+		}
 	}
 	return nil
 }
@@ -571,6 +579,8 @@ func (m *RootModel) resetSettingToDefault(category, key string, defaults *config
 			m.Settings.General.ClipboardMonitor = defaults.General.ClipboardMonitor
 		case "theme":
 			m.Settings.General.Theme = defaults.General.Theme
+		case "log_retention_count":
+			m.Settings.General.LogRetentionCount = defaults.General.LogRetentionCount
 		}
 
 	case "Connections":

internal/utils/debug.go

@@ -2,8 +2,11 @@ package utils
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"sync"
 	"time"
 )
@@ -45,3 +48,50 @@ func Debug(format string, args ...any) {
 		fmt.Fprintf(debugFile, "[%s] %s\n", timestamp, fmt.Sprintf(format, args...))
 	}
 }
+
+// CleanupLogs removes old log files, keeping only the most recent retentionCount files
+func CleanupLogs(retentionCount int) {
+	if retentionCount < 0 {
+		return // Keep all logs
+	}
+
+	mu.RLock()
+	dir := logsDir
+	mu.RUnlock()
+
+	if dir == "" {
+		return
+	}
+
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		// If directory doesn't exist, nothing to clean
+		return
+	}
+
+	var logs []fs.DirEntry
+	for _, entry := range entries {
+		if !entry.IsDir() && strings.HasPrefix(entry.Name(), "debug-") && strings.HasSuffix(entry.Name(), ".log") {
+			logs = append(logs, entry)
+		}
+	}
+
+	// Sort by modification time (newest first)
+	// Filenames have timestamp: debug-YYYYMMDD-HHMMSS.log, so alphabetical sort is also chronological
+	// But let's rely on ModTime to be safe if possible, or just name since it is consistent
+	sort.Slice(logs, func(i, j int) bool {
+		// Newest first
+		// Since names are YYYYMMDD-HHMMSS, reverse alphabetical works
+		return logs[i].Name() > logs[j].Name()
+	})
+
+	if len(logs) <= retentionCount {
+		return
+	}
+
+	// Remove older logs
+	for _, log := range logs[retentionCount:] {
+		path := filepath.Join(dir, log.Name())
+		os.Remove(path)
+	}
+}

internal/utils/debug_test.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -87,3 +88,76 @@ func TestLogFilePath(t *testing.T) {
 		t.Errorf("Logs directory should be absolute path, got: %s", logsDir)
 	}
 }
+
+func TestCleanupLogs(t *testing.T) {
+	// Use a temporary directory for this test
+	tempDir, err := os.MkdirTemp("", "surge-logs-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	// Configure debug to use this temp dir
+	ConfigureDebug(tempDir)
+
+	// Reset configuration after test (though this changes global state, so might affect other tests potentially)
+	// But in these unit tests, parallelism isn't enabled by default.
+	defer ConfigureDebug(config.GetLogsDir())
+
+	// Create 10 dummy log files
+	baseTime := time.Now()
+	for i := 0; i < 10; i++ {
+		// Use file name format matching debug.go: debug-YYYYMMDD-HHMMSS.log
+		// We add 'i' to time to ensure uniqueness and order
+		ts := baseTime.Add(time.Duration(i) * time.Hour)
+		filename := fmt.Sprintf("debug-%s.log", ts.Format("20060102-150405"))
+		path := filepath.Join(tempDir, filename)
+
+		err := os.WriteFile(path, []byte("dummy log"), 0644)
+		if err != nil {
+			t.Fatalf("Failed to write dummy log: %v", err)
+		}
+	}
+
+	// Verify we created 10
+	entries, err := os.ReadDir(tempDir)
+	if err != nil {
+		t.Fatalf("Failed to read dir: %v", err)
+	}
+	if len(entries) != 10 {
+		t.Fatalf("Expected 10 files, got %d", len(entries))
+	}
+
+	// Test cleanup: Keep 5
+	CleanupLogs(5)
+
+	// Verify we have 5 left
+	entries, err = os.ReadDir(tempDir)
+	if err != nil {
+		t.Fatalf("Failed to read dir after cleanup: %v", err)
+	}
+
+	if len(entries) != 5 {
+		// For debugging failure
+		var names []string
+		for _, e := range entries {
+			names = append(names, e.Name())
+		}
+		t.Errorf("Expected 5 files, got %d. Files: %v", len(entries), names)
+	}
+
+	// Verify we kept the NEWEST ones (indices 5, 6, 7, 8, 9 from loop)
+	// The file created with i=9 should be present
+	newestTS := baseTime.Add(9 * time.Hour).Format("20060102-150405")
+	expectedName := fmt.Sprintf("debug-%s.log", newestTS)
+	found := false
+	for _, e := range entries {
+		if e.Name() == expectedName {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("Expected newest file %s to be present, but it was not", expectedName)
+	}
+}