Fix session log cleanup retention
- commit
- a83b7b9a73414ef93d12d0d16587bc25a7eddbb0
- author
- brice · · 2026-04-24 04:04 UTC
- parent
- 4cc8b0e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +74 -29
M docs/config/index.md +6 -6
| @@ -123,14 +123,14 @@ auto_open_on_new_pane = false | |||
| 123 | 123 | ||
| 124 | 124 | | Key | Type | Default | Valid Values | Description | | |
| 125 | 125 | |-----|------|---------|--------------|-------------| | |
| 126 | - | | `logging.level` | string | `"info"` | `debug`, `info`, `warn`, `error` | Log level | | |
| 127 | - | | `logging.format` | string | `"text"` | `text`, `json` | Log format | | |
| 128 | - | | `logging.max_age` | int | `7` | >= 0 | Days to keep logs | | |
| 129 | - | | `logging.max_files` | int | `100` | >= 0 | Session log files to keep (`0` disables count cleanup) | | |
| 130 | - | | `logging.log_dir` | string | `~/.local/state/dumber/logs/` | any | Log directory | | |
| 126 | + | | `logging.level` | string | `"info"` | `trace`, `debug`, `info`, `warn`, `error`, `fatal` | Log level | | |
| 127 | + | | `logging.format` | string | `"text"` | `text`, `json`, `console` | Log format | | |
| 128 | + | | `logging.max_age` | int | `7` | `>= 0` | Days to keep logs | | |
| 129 | + | | `logging.max_files` | int | `100` | `>= 0` | Session log files to keep (`0` disables count cleanup) | | |
| 130 | + | | `logging.log_dir` | string | `~/.local/state/dumber/logs` | any path | Log directory | | |
| 131 | 131 | | `logging.enable_file_log` | bool | `true` | - | Enable file logging | | |
| 132 | 132 | | `logging.capture_console` | bool | `false` | - | Capture browser console to logs | | |
| 133 | - | | `logging.capture_gtk_webkit_logs` | bool | `false` | - | Capture GTK and WebKit logs for debugging | | |
| 133 | + | | `logging.capture_gtk_logs` | bool | `false` | - | Capture GTK and WebKit logs for debugging. | | |
| 134 | 134 | ||
| 135 | 135 | ## Appearance | |
| 136 | 136 | ||
| @@ -123,14 +123,14 @@ auto_open_on_new_pane = false | |||||
| 123 | 123 | ||||
| 124 | | Key | Type | Default | Valid Values | Description | | 124 | | Key | Type | Default | Valid Values | Description | | ||
| 125 | |-----|------|---------|--------------|-------------| | 125 | |-----|------|---------|--------------|-------------| | ||
| 126 | - | | `logging.level` | string | `"info"` | `debug`, `info`, `warn`, `error` | Log level | | 126 | + | | `logging.level` | string | `"info"` | `trace`, `debug`, `info`, `warn`, `error`, `fatal` | Log level | |
| 127 | - | | `logging.format` | string | `"text"` | `text`, `json` | Log format | | 127 | + | | `logging.format` | string | `"text"` | `text`, `json`, `console` | Log format | |
| 128 | - | | `logging.max_age` | int | `7` | >= 0 | Days to keep logs | | 128 | + | | `logging.max_age` | int | `7` | `>= 0` | Days to keep logs | |
| 129 | - | | `logging.max_files` | int | `100` | >= 0 | Session log files to keep (`0` disables count cleanup) | | 129 | + | | `logging.max_files` | int | `100` | `>= 0` | Session log files to keep (`0` disables count cleanup) | |
| 130 | - | | `logging.log_dir` | string | `~/.local/state/dumber/logs/` | any | Log directory | | 130 | + | | `logging.log_dir` | string | `~/.local/state/dumber/logs` | any path | Log directory | |
| 131 | | `logging.enable_file_log` | bool | `true` | - | Enable file logging | | 131 | | `logging.enable_file_log` | bool | `true` | - | Enable file logging | | ||
| 132 | | `logging.capture_console` | bool | `false` | - | Capture browser console to logs | | 132 | | `logging.capture_console` | bool | `false` | - | Capture browser console to logs | | ||
| 133 | - | | `logging.capture_gtk_webkit_logs` | bool | `false` | - | Capture GTK and WebKit logs for debugging | | 133 | + | | `logging.capture_gtk_logs` | bool | `false` | - | Capture GTK and WebKit logs for debugging. | |
| 134 | 134 | ||||
| 135 | ## Appearance | 135 | ## Appearance | ||
| 136 | 136 | ||||
M docs/reference/configuration.md +5 -5
| @@ -20,14 +20,14 @@ | |||
| 20 | 20 | | `omnibox.initial_behavior` | string | `recent` | `recent`, `most_visited`, `none` | | |
| 21 | 21 | | `omnibox.most_visited_days` | int | `30` | `>= 0` | | |
| 22 | 22 | | `omnibox.auto_open_on_new_pane` | bool | `false` | | | |
| 23 | - | | `logging.level` | string | `info` | `debug`, `info`, `warn`, `error` | | |
| 24 | - | | `logging.format` | string | `text` | `text`, `json` | | |
| 23 | + | | `logging.level` | string | `info` | `trace`, `debug`, `info`, `warn`, `error`, `fatal` | | |
| 24 | + | | `logging.format` | string | `text` | `text`, `json`, `console` | | |
| 25 | 25 | | `logging.max_age` | int | `7` | >= 0 | | |
| 26 | - | | `logging.max_files` | int | `100` | >= 0 | | |
| 27 | - | | `logging.log_dir` | string | `~/.local/state/dumber/logs/` | | | |
| 26 | + | | `logging.max_files` | int | `100` | >= 0 (0 disables count cleanup) | | |
| 27 | + | | `logging.log_dir` | string | `~/.local/state/dumber/logs` | any path | | |
| 28 | 28 | | `logging.enable_file_log` | bool | `true` | | | |
| 29 | 29 | | `logging.capture_console` | bool | `false` | | | |
| 30 | - | | `logging.capture_gtk_webkit_logs` | bool | `false` | | | |
| 30 | + | | `logging.capture_gtk_logs` | bool | `false` | | | |
| 31 | 31 | | `appearance.sans_font` | string | `Fira Sans` | | | |
| 32 | 32 | | `appearance.serif_font` | string | `Fira Sans` | | | |
| 33 | 33 | | `appearance.monospace_font` | string | `Fira Code` | | | |
| @@ -20,14 +20,14 @@ | |||||
| 20 | | `omnibox.initial_behavior` | string | `recent` | `recent`, `most_visited`, `none` | | 20 | | `omnibox.initial_behavior` | string | `recent` | `recent`, `most_visited`, `none` | | ||
| 21 | | `omnibox.most_visited_days` | int | `30` | `>= 0` | | 21 | | `omnibox.most_visited_days` | int | `30` | `>= 0` | | ||
| 22 | | `omnibox.auto_open_on_new_pane` | bool | `false` | | | 22 | | `omnibox.auto_open_on_new_pane` | bool | `false` | | | ||
| 23 | - | | `logging.level` | string | `info` | `debug`, `info`, `warn`, `error` | | 23 | + | | `logging.level` | string | `info` | `trace`, `debug`, `info`, `warn`, `error`, `fatal` | |
| 24 | - | | `logging.format` | string | `text` | `text`, `json` | | 24 | + | | `logging.format` | string | `text` | `text`, `json`, `console` | |
| 25 | | `logging.max_age` | int | `7` | >= 0 | | 25 | | `logging.max_age` | int | `7` | >= 0 | | ||
| 26 | - | | `logging.max_files` | int | `100` | >= 0 | | 26 | + | | `logging.max_files` | int | `100` | >= 0 (0 disables count cleanup) | |
| 27 | - | | `logging.log_dir` | string | `~/.local/state/dumber/logs/` | | | 27 | + | | `logging.log_dir` | string | `~/.local/state/dumber/logs` | any path | |
| 28 | | `logging.enable_file_log` | bool | `true` | | | 28 | | `logging.enable_file_log` | bool | `true` | | | ||
| 29 | | `logging.capture_console` | bool | `false` | | | 29 | | `logging.capture_console` | bool | `false` | | | ||
| 30 | - | | `logging.capture_gtk_webkit_logs` | bool | `false` | | | 30 | + | | `logging.capture_gtk_logs` | bool | `false` | | |
| 31 | | `appearance.sans_font` | string | `Fira Sans` | | | 31 | | `appearance.sans_font` | string | `Fira Sans` | | | ||
| 32 | | `appearance.serif_font` | string | `Fira Sans` | | | 32 | | `appearance.serif_font` | string | `Fira Sans` | | | ||
| 33 | | `appearance.monospace_font` | string | `Fira Code` | | | 33 | | `appearance.monospace_font` | string | `Fira Code` | | | ||
M internal/bootstrap/session.go +5 -4
| @@ -102,7 +102,7 @@ func StartBrowserSession( | |||
| 102 | 102 | Level: cfg.Logging.Level, | |
| 103 | 103 | Format: cfg.Logging.Format, | |
| 104 | 104 | TimeFormat: corelogging.ConsoleTimeFormat, | |
| 105 | - | LogDir: cfg.Logging.LogDir, | |
| 105 | + | LogDir: logDir, | |
| 106 | 106 | WriteToStderr: true, | |
| 107 | 107 | EnableFileLog: cfg.Logging.EnableFileLog, | |
| 108 | 108 | }) | |
| @@ -156,7 +156,7 @@ func StartBrowserSession( | |||
| 156 | 156 | } | |
| 157 | 157 | ||
| 158 | 158 | // Background cleanup of old sessions. | |
| 159 | - | go runSessionCleanup(persistCtx, sessionUC, cleanupUC, cfg, session.ID, log) | |
| 159 | + | go runSessionCleanup(persistCtx, sessionUC, cleanupUC, cfg, logDir, session.ID, log) | |
| 160 | 160 | }) | |
| 161 | 161 | return persistErr | |
| 162 | 162 | } | |
| @@ -310,6 +310,7 @@ func runSessionCleanup( | |||
| 310 | 310 | sessionUC *usecase.ManageSessionUseCase, | |
| 311 | 311 | cleanupUC *usecase.CleanupSessionsUseCase, | |
| 312 | 312 | cfg *config.Config, | |
| 313 | + | logDir string, | |
| 313 | 314 | currentSessionID entity.SessionID, | |
| 314 | 315 | log *zerolog.Logger, | |
| 315 | 316 | ) { | |
| @@ -347,10 +348,10 @@ func runSessionCleanup( | |||
| 347 | 348 | } | |
| 348 | 349 | ||
| 349 | 350 | // Prune old marker files left by previous versions. | |
| 350 | - | sweepLegacyMarkers(cfg.Logging.LogDir, log) | |
| 351 | + | sweepLegacyMarkers(logDir, log) | |
| 351 | 352 | ||
| 352 | 353 | removedLogs, logCleanupErr := corelogging.CleanupSessionLogFiles( | |
| 353 | - | cfg.Logging.LogDir, | |
| 354 | + | logDir, | |
| 354 | 355 | cfg.Logging.MaxFiles, | |
| 355 | 356 | string(currentSessionID), | |
| 356 | 357 | ) | |
| @@ -102,7 +102,7 @@ func StartBrowserSession( | |||||
| 102 | Level: cfg.Logging.Level, | 102 | Level: cfg.Logging.Level, | ||
| 103 | Format: cfg.Logging.Format, | 103 | Format: cfg.Logging.Format, | ||
| 104 | TimeFormat: corelogging.ConsoleTimeFormat, | 104 | TimeFormat: corelogging.ConsoleTimeFormat, | ||
| 105 | - | LogDir: cfg.Logging.LogDir, | 105 | + | LogDir: logDir, |
| 106 | WriteToStderr: true, | 106 | WriteToStderr: true, | ||
| 107 | EnableFileLog: cfg.Logging.EnableFileLog, | 107 | EnableFileLog: cfg.Logging.EnableFileLog, | ||
| 108 | }) | 108 | }) | ||
| @@ -156,7 +156,7 @@ func StartBrowserSession( | |||||
| 156 | } | 156 | } | ||
| 157 | 157 | ||||
| 158 | // Background cleanup of old sessions. | 158 | // Background cleanup of old sessions. | ||
| 159 | - | go runSessionCleanup(persistCtx, sessionUC, cleanupUC, cfg, session.ID, log) | 159 | + | go runSessionCleanup(persistCtx, sessionUC, cleanupUC, cfg, logDir, session.ID, log) |
| 160 | }) | 160 | }) | ||
| 161 | return persistErr | 161 | return persistErr | ||
| 162 | } | 162 | } | ||
| @@ -310,6 +310,7 @@ func runSessionCleanup( | |||||
| 310 | sessionUC *usecase.ManageSessionUseCase, | 310 | sessionUC *usecase.ManageSessionUseCase, | ||
| 311 | cleanupUC *usecase.CleanupSessionsUseCase, | 311 | cleanupUC *usecase.CleanupSessionsUseCase, | ||
| 312 | cfg *config.Config, | 312 | cfg *config.Config, | ||
| 313 | + | logDir string, | |||
| 313 | currentSessionID entity.SessionID, | 314 | currentSessionID entity.SessionID, | ||
| 314 | log *zerolog.Logger, | 315 | log *zerolog.Logger, | ||
| 315 | ) { | 316 | ) { | ||
| @@ -347,10 +348,10 @@ func runSessionCleanup( | |||||
| 347 | } | 348 | } | ||
| 348 | 349 | ||||
| 349 | // Prune old marker files left by previous versions. | 350 | // Prune old marker files left by previous versions. | ||
| 350 | - | sweepLegacyMarkers(cfg.Logging.LogDir, log) | 351 | + | sweepLegacyMarkers(logDir, log) |
| 351 | 352 | ||||
| 352 | removedLogs, logCleanupErr := corelogging.CleanupSessionLogFiles( | 353 | removedLogs, logCleanupErr := corelogging.CleanupSessionLogFiles( | ||
| 353 | - | cfg.Logging.LogDir, | 354 | + | logDir, |
| 354 | cfg.Logging.MaxFiles, | 355 | cfg.Logging.MaxFiles, | ||
| 355 | string(currentSessionID), | 356 | string(currentSessionID), | ||
| 356 | ) | 357 | ) | ||
M internal/bootstrap/session_test.go +2 -2
| @@ -45,7 +45,7 @@ func TestRunSessionCleanup_SkipsCurrentActiveSession(t *testing.T) { | |||
| 45 | 45 | repo.EXPECT().DeleteExitedBefore(mock.Anything, mock.MatchedBy(func(time.Time) bool { return true })).Return(int64(0), nil).Once() | |
| 46 | 46 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | |
| 47 | 47 | ||
| 48 | - | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, current.ID, nil) | |
| 48 | + | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, cfg.Logging.LogDir, current.ID, nil) | |
| 49 | 49 | } | |
| 50 | 50 | ||
| 51 | 51 | func TestRunSessionCleanup_NoPanicOnNilSessions(t *testing.T) { | |
| @@ -62,6 +62,6 @@ func TestRunSessionCleanup_NoPanicOnNilSessions(t *testing.T) { | |||
| 62 | 62 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | |
| 63 | 63 | ||
| 64 | 64 | require.NotPanics(t, func() { | |
| 65 | - | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, entity.SessionID("current"), nil) | |
| 65 | + | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, cfg.Logging.LogDir, entity.SessionID("current"), nil) | |
| 66 | 66 | }) | |
| 67 | 67 | } | |
| @@ -45,7 +45,7 @@ func TestRunSessionCleanup_SkipsCurrentActiveSession(t *testing.T) { | |||||
| 45 | repo.EXPECT().DeleteExitedBefore(mock.Anything, mock.MatchedBy(func(time.Time) bool { return true })).Return(int64(0), nil).Once() | 45 | repo.EXPECT().DeleteExitedBefore(mock.Anything, mock.MatchedBy(func(time.Time) bool { return true })).Return(int64(0), nil).Once() | ||
| 46 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | 46 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | ||
| 47 | 47 | ||||
| 48 | - | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, current.ID, nil) | 48 | + | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, cfg.Logging.LogDir, current.ID, nil) |
| 49 | } | 49 | } | ||
| 50 | 50 | ||||
| 51 | func TestRunSessionCleanup_NoPanicOnNilSessions(t *testing.T) { | 51 | func TestRunSessionCleanup_NoPanicOnNilSessions(t *testing.T) { | ||
| @@ -62,6 +62,6 @@ func TestRunSessionCleanup_NoPanicOnNilSessions(t *testing.T) { | |||||
| 62 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | 62 | repo.EXPECT().DeleteOldestExited(mock.Anything, cfg.Session.MaxExitedSessions).Return(int64(0), nil).Once() | ||
| 63 | 63 | ||||
| 64 | require.NotPanics(t, func() { | 64 | require.NotPanics(t, func() { | ||
| 65 | - | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, entity.SessionID("current"), nil) | 65 | + | runSessionCleanup(ctx, sessionUC, cleanupUC, cfg, cfg.Logging.LogDir, entity.SessionID("current"), nil) |
| 66 | }) | 66 | }) | ||
| 67 | } | 67 | } | ||
M internal/logging/session.go +26 -8
| @@ -63,7 +63,7 @@ type sessionLogFile struct { | |||
| 63 | 63 | } | |
| 64 | 64 | ||
| 65 | 65 | // CleanupSessionLogFiles removes old session log files when their count exceeds maxFiles. | |
| 66 | - | // It keeps the newest files by modification time and never removes the supplied session IDs. | |
| 66 | + | // It keeps up to maxFiles of the newest requested session IDs, then the newest remaining files by modification time. | |
| 67 | 67 | // A maxFiles value of 0 disables count-based cleanup. | |
| 68 | 68 | func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...string) (int, error) { | |
| 69 | 69 | if logDir == "" || maxFiles <= 0 { | |
| @@ -78,12 +78,12 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||
| 78 | 78 | return 0, fmt.Errorf("read log directory: %w", err) | |
| 79 | 79 | } | |
| 80 | 80 | ||
| 81 | - | keep := make(map[string]struct{}, len(keepSessionIDs)) | |
| 81 | + | requestedKeep := make(map[string]struct{}, len(keepSessionIDs)) | |
| 82 | 82 | for _, sessionID := range keepSessionIDs { | |
| 83 | 83 | if sessionID == "" { | |
| 84 | 84 | continue | |
| 85 | 85 | } | |
| 86 | - | keep[sessionID] = struct{}{} | |
| 86 | + | requestedKeep[sessionID] = struct{}{} | |
| 87 | 87 | } | |
| 88 | 88 | ||
| 89 | 89 | files := make([]sessionLogFile, 0, len(entries)) | |
| @@ -119,15 +119,20 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||
| 119 | 119 | return files[i].modTime.After(files[j].modTime) | |
| 120 | 120 | }) | |
| 121 | 121 | ||
| 122 | + | allowedKeep := boundedKeepSessionIDs(files, requestedKeep, maxFiles) | |
| 123 | + | maxNewestFiles := maxFiles - len(allowedKeep) | |
| 122 | 124 | kept := 0 | |
| 125 | + | keptNewest := 0 | |
| 123 | 126 | removed := 0 | |
| 124 | 127 | for _, file := range files { | |
| 125 | - | if _, shouldKeep := keep[file.sessionID]; shouldKeep { | |
| 126 | - | kept++ | |
| 127 | - | continue | |
| 128 | - | } | |
| 129 | - | if kept < maxFiles { | |
| 128 | + | if _, shouldKeep := allowedKeep[file.sessionID]; shouldKeep { | |
| 129 | + | if kept < maxFiles { | |
| 130 | + | kept++ | |
| 131 | + | continue | |
| 132 | + | } | |
| 133 | + | } else if keptNewest < maxNewestFiles && kept < maxFiles { | |
| 130 | 134 | kept++ | |
| 135 | + | keptNewest++ | |
| 131 | 136 | continue | |
| 132 | 137 | } | |
| 133 | 138 | ||
| @@ -140,3 +145,16 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||
| 140 | 145 | ||
| 141 | 146 | return removed, errors.Join(operationErrs...) | |
| 142 | 147 | } | |
| 148 | + | ||
| 149 | + | func boundedKeepSessionIDs(files []sessionLogFile, requestedKeep map[string]struct{}, maxFiles int) map[string]struct{} { | |
| 150 | + | allowedKeep := make(map[string]struct{}, len(requestedKeep)) | |
| 151 | + | for _, file := range files { | |
| 152 | + | if len(allowedKeep) >= maxFiles { | |
| 153 | + | break | |
| 154 | + | } | |
| 155 | + | if _, shouldKeep := requestedKeep[file.sessionID]; shouldKeep { | |
| 156 | + | allowedKeep[file.sessionID] = struct{}{} | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | return allowedKeep | |
| 160 | + | } | |
| @@ -63,7 +63,7 @@ type sessionLogFile struct { | |||||
| 63 | } | 63 | } | ||
| 64 | 64 | ||||
| 65 | // CleanupSessionLogFiles removes old session log files when their count exceeds maxFiles. | 65 | // CleanupSessionLogFiles removes old session log files when their count exceeds maxFiles. | ||
| 66 | - | // It keeps the newest files by modification time and never removes the supplied session IDs. | 66 | + | // It keeps up to maxFiles of the newest requested session IDs, then the newest remaining files by modification time. |
| 67 | // A maxFiles value of 0 disables count-based cleanup. | 67 | // A maxFiles value of 0 disables count-based cleanup. | ||
| 68 | func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...string) (int, error) { | 68 | func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...string) (int, error) { | ||
| 69 | if logDir == "" || maxFiles <= 0 { | 69 | if logDir == "" || maxFiles <= 0 { | ||
| @@ -78,12 +78,12 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||||
| 78 | return 0, fmt.Errorf("read log directory: %w", err) | 78 | return 0, fmt.Errorf("read log directory: %w", err) | ||
| 79 | } | 79 | } | ||
| 80 | 80 | ||||
| 81 | - | keep := make(map[string]struct{}, len(keepSessionIDs)) | 81 | + | requestedKeep := make(map[string]struct{}, len(keepSessionIDs)) |
| 82 | for _, sessionID := range keepSessionIDs { | 82 | for _, sessionID := range keepSessionIDs { | ||
| 83 | if sessionID == "" { | 83 | if sessionID == "" { | ||
| 84 | continue | 84 | continue | ||
| 85 | } | 85 | } | ||
| 86 | - | keep[sessionID] = struct{}{} | 86 | + | requestedKeep[sessionID] = struct{}{} |
| 87 | } | 87 | } | ||
| 88 | 88 | ||||
| 89 | files := make([]sessionLogFile, 0, len(entries)) | 89 | files := make([]sessionLogFile, 0, len(entries)) | ||
| @@ -119,15 +119,20 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||||
| 119 | return files[i].modTime.After(files[j].modTime) | 119 | return files[i].modTime.After(files[j].modTime) | ||
| 120 | }) | 120 | }) | ||
| 121 | 121 | ||||
| 122 | + | allowedKeep := boundedKeepSessionIDs(files, requestedKeep, maxFiles) | |||
| 123 | + | maxNewestFiles := maxFiles - len(allowedKeep) | |||
| 122 | kept := 0 | 124 | kept := 0 | ||
| 125 | + | keptNewest := 0 | |||
| 123 | removed := 0 | 126 | removed := 0 | ||
| 124 | for _, file := range files { | 127 | for _, file := range files { | ||
| 125 | - | if _, shouldKeep := keep[file.sessionID]; shouldKeep { | 128 | + | if _, shouldKeep := allowedKeep[file.sessionID]; shouldKeep { |
| 126 | - | kept++ | 129 | + | if kept < maxFiles { |
| 127 | - | continue | 130 | + | kept++ |
| 128 | - | } | 131 | + | continue |
| 129 | - | if kept < maxFiles { | 132 | + | } |
| 133 | + | } else if keptNewest < maxNewestFiles && kept < maxFiles { | |||
| 130 | kept++ | 134 | kept++ | ||
| 135 | + | keptNewest++ | |||
| 131 | continue | 136 | continue | ||
| 132 | } | 137 | } | ||
| 133 | 138 | ||||
| @@ -140,3 +145,16 @@ func CleanupSessionLogFiles(logDir string, maxFiles int, keepSessionIDs ...strin | |||||
| 140 | 145 | ||||
| 141 | return removed, errors.Join(operationErrs...) | 146 | return removed, errors.Join(operationErrs...) | ||
| 142 | } | 147 | } | ||
| 148 | + | ||||
| 149 | + | func boundedKeepSessionIDs(files []sessionLogFile, requestedKeep map[string]struct{}, maxFiles int) map[string]struct{} { | |||
| 150 | + | allowedKeep := make(map[string]struct{}, len(requestedKeep)) | |||
| 151 | + | for _, file := range files { | |||
| 152 | + | if len(allowedKeep) >= maxFiles { | |||
| 153 | + | break | |||
| 154 | + | } | |||
| 155 | + | if _, shouldKeep := requestedKeep[file.sessionID]; shouldKeep { | |||
| 156 | + | allowedKeep[file.sessionID] = struct{}{} | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | return allowedKeep | |||
| 160 | + | } | |||
M internal/logging/session_test.go +30 -4
| @@ -97,7 +97,7 @@ func TestCleanupSessionLogFiles_UsesFilenameTiebreakerForEqualModTimes(t *testin | |||
| 97 | 97 | assertExists(t, newestByName) | |
| 98 | 98 | } | |
| 99 | 99 | ||
| 100 | - | func TestCleanupSessionLogFiles_KeepsSpecifiedSession(t *testing.T) { | |
| 100 | + | func TestCleanupSessionLogFiles_KeepsSpecifiedSessionWithinLimit(t *testing.T) { | |
| 101 | 101 | t.Parallel() | |
| 102 | 102 | ||
| 103 | 103 | dir := t.TempDir() | |
| @@ -112,13 +112,39 @@ func TestCleanupSessionLogFiles_KeepsSpecifiedSession(t *testing.T) { | |||
| 112 | 112 | if err != nil { | |
| 113 | 113 | t.Fatalf("cleanup session log files: %v", err) | |
| 114 | 114 | } | |
| 115 | - | if removed != 1 { | |
| 116 | - | t.Fatalf("removed = %d, want 1", removed) | |
| 115 | + | if removed != 2 { | |
| 116 | + | t.Fatalf("removed = %d, want 2", removed) | |
| 117 | 117 | } | |
| 118 | 118 | ||
| 119 | 119 | assertExists(t, oldestKept) | |
| 120 | 120 | assertMissing(t, middle) | |
| 121 | - | assertExists(t, newest) | |
| 121 | + | assertMissing(t, newest) | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | func TestCleanupSessionLogFiles_BoundsSpecifiedSessionsToMaxFiles(t *testing.T) { | |
| 125 | + | t.Parallel() | |
| 126 | + | ||
| 127 | + | dir := t.TempDir() | |
| 128 | + | base := time.Date(2026, 4, 24, 12, 0, 0, 0, time.UTC) | |
| 129 | + | ||
| 130 | + | oldestKeptID := "20260424_120000_0001" | |
| 131 | + | middleKeptID := "20260424_120001_0002" | |
| 132 | + | newestKeptID := "20260424_120002_0003" | |
| 133 | + | oldestKept := writeSessionLog(t, dir, oldestKeptID, base) | |
| 134 | + | middleKept := writeSessionLog(t, dir, middleKeptID, base.Add(time.Second)) | |
| 135 | + | newestKept := writeSessionLog(t, dir, newestKeptID, base.Add(2*time.Second)) | |
| 136 | + | ||
| 137 | + | removed, err := CleanupSessionLogFiles(dir, 2, oldestKeptID, middleKeptID, newestKeptID) | |
| 138 | + | if err != nil { | |
| 139 | + | t.Fatalf("cleanup session log files: %v", err) | |
| 140 | + | } | |
| 141 | + | if removed != 1 { | |
| 142 | + | t.Fatalf("removed = %d, want 1", removed) | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | assertMissing(t, oldestKept) | |
| 146 | + | assertExists(t, middleKept) | |
| 147 | + | assertExists(t, newestKept) | |
| 122 | 148 | } | |
| 123 | 149 | ||
| 124 | 150 | func TestCleanupSessionLogFiles_MissingDirectoryIsNoop(t *testing.T) { | |
| @@ -97,7 +97,7 @@ func TestCleanupSessionLogFiles_UsesFilenameTiebreakerForEqualModTimes(t *testin | |||||
| 97 | assertExists(t, newestByName) | 97 | assertExists(t, newestByName) | ||
| 98 | } | 98 | } | ||
| 99 | 99 | ||||
| 100 | - | func TestCleanupSessionLogFiles_KeepsSpecifiedSession(t *testing.T) { | 100 | + | func TestCleanupSessionLogFiles_KeepsSpecifiedSessionWithinLimit(t *testing.T) { |
| 101 | t.Parallel() | 101 | t.Parallel() | ||
| 102 | 102 | ||||
| 103 | dir := t.TempDir() | 103 | dir := t.TempDir() | ||
| @@ -112,13 +112,39 @@ func TestCleanupSessionLogFiles_KeepsSpecifiedSession(t *testing.T) { | |||||
| 112 | if err != nil { | 112 | if err != nil { | ||
| 113 | t.Fatalf("cleanup session log files: %v", err) | 113 | t.Fatalf("cleanup session log files: %v", err) | ||
| 114 | } | 114 | } | ||
| 115 | - | if removed != 1 { | 115 | + | if removed != 2 { |
| 116 | - | t.Fatalf("removed = %d, want 1", removed) | 116 | + | t.Fatalf("removed = %d, want 2", removed) |
| 117 | } | 117 | } | ||
| 118 | 118 | ||||
| 119 | assertExists(t, oldestKept) | 119 | assertExists(t, oldestKept) | ||
| 120 | assertMissing(t, middle) | 120 | assertMissing(t, middle) | ||
| 121 | - | assertExists(t, newest) | 121 | + | assertMissing(t, newest) |
| 122 | + | } | |||
| 123 | + | ||||
| 124 | + | func TestCleanupSessionLogFiles_BoundsSpecifiedSessionsToMaxFiles(t *testing.T) { | |||
| 125 | + | t.Parallel() | |||
| 126 | + | ||||
| 127 | + | dir := t.TempDir() | |||
| 128 | + | base := time.Date(2026, 4, 24, 12, 0, 0, 0, time.UTC) | |||
| 129 | + | ||||
| 130 | + | oldestKeptID := "20260424_120000_0001" | |||
| 131 | + | middleKeptID := "20260424_120001_0002" | |||
| 132 | + | newestKeptID := "20260424_120002_0003" | |||
| 133 | + | oldestKept := writeSessionLog(t, dir, oldestKeptID, base) | |||
| 134 | + | middleKept := writeSessionLog(t, dir, middleKeptID, base.Add(time.Second)) | |||
| 135 | + | newestKept := writeSessionLog(t, dir, newestKeptID, base.Add(2*time.Second)) | |||
| 136 | + | ||||
| 137 | + | removed, err := CleanupSessionLogFiles(dir, 2, oldestKeptID, middleKeptID, newestKeptID) | |||
| 138 | + | if err != nil { | |||
| 139 | + | t.Fatalf("cleanup session log files: %v", err) | |||
| 140 | + | } | |||
| 141 | + | if removed != 1 { | |||
| 142 | + | t.Fatalf("removed = %d, want 1", removed) | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | assertMissing(t, oldestKept) | |||
| 146 | + | assertExists(t, middleKept) | |||
| 147 | + | assertExists(t, newestKept) | |||
| 122 | } | 148 | } | ||
| 123 | 149 | ||||
| 124 | func TestCleanupSessionLogFiles_MissingDirectoryIsNoop(t *testing.T) { | 150 | func TestCleanupSessionLogFiles_MissingDirectoryIsNoop(t *testing.T) { | ||