review fixes apr 2026 (#130)

* Multiple fixes

- refresh coordinator dedup + memory pressure wire
- middleware sse consolidation + timer leak + claim cache
- universal cache sync backfill + isDebug gate
- lazy background task race
- memory monitor stw cached + refresh() api

* fix(auth): suppress OIDC redirects on non-navigation requests

- [x] Add isNonNavigationRequest using Sec-Fetch-Mode and Accept headers
- [x] Add comprehensive TestIsNonNavigationRequest
- [x] Update ServeHTTP to 401 non-navigation and AJAX requests

Fixes #129

* feat(config): add custom CA and insecure skip verify for OIDC TLS

- [x] Add CACertPath, CACertPEM, InsecureSkipVerify to Config
- [x] Implement loadCACertPool for CA bundle loading
- [x] Update HTTPClientConfig with RootCAs and InsecureSkipVerify
- [x] Apply CA pool and skip verify to pooled HTTP clients
- [x] Enhance configKey to distinguish TLS configs
- [x] Add comprehensive ca_cert_test.go

Fixes #125

* feat(oidc): add custom CA certificate support for private OIDC providers

- [x] Add caCertPath, caCertPEM, insecureSkipVerify config options
- [x] Update traefik.yml with new OIDC client config fields
- [x] Add configuration schema descriptions for new options
- [x] Update README table and add Custom CA Certificates section

* Fix the documentation.

* test(redis): add oversized argument rejection test

- [x] Add TestRedisConn_RejectOversizedArgumentBytes
- [x] Import strings package

* Dependencies cleanup

memory_monitor.go

@@ -58,13 +58,21 @@ func (mpl MemoryPressureLevel) String() string {
 	}
 }
 
-// MemoryMonitor provides comprehensive memory monitoring and alerting
+// MemoryMonitor provides comprehensive memory monitoring and alerting.
+//
+// Memory sampling is expensive: runtime.ReadMemStats is a stop-the-world
+// operation. To keep latency predictable the monitor caches the most recent
+// sample and only refreshes it when the background ticker fires, when TriggerGC
+// is invoked, or when a caller explicitly calls Refresh(). GetCurrentStats is a
+// cheap read of that cached sample.
 type MemoryMonitor struct {
 	lastGCTime         time.Time
 	startTime          time.Time
 	lastStats          *MemoryStats
+	cachedMemStats     runtime.MemStats
 	logger             *Logger
 	alertThresholds    MemoryAlertThresholds
+	config             MemoryMonitorConfig
 	baselineGoroutines int
 	baselineHeap       uint64
 	heapGrowthRate     float64
@@ -84,6 +92,30 @@ type MemoryAlertThresholds struct {
 	GCFrequency         float64 // Alert when GC frequency exceeds this per minute
 }
 
+// MemoryMonitorConfig configures the memory monitor's scheduling behavior.
+// Thresholds are kept separate in MemoryAlertThresholds.
+type MemoryMonitorConfig struct {
+	// Interval between background samples. Must be >= MinMemoryMonitorInterval
+	// (30s). Values below the minimum are clamped when monitoring starts.
+	Interval time.Duration
+}
+
+// Default and minimum interval values. The minimum exists because
+// runtime.ReadMemStats is stop-the-world and hammering it on a hot loop causes
+// noticeable latency spikes, especially under yaegi.
+const (
+	DefaultMemoryMonitorInterval = 60 * time.Second
+	MinMemoryMonitorInterval     = 30 * time.Second
+)
+
+// DefaultMemoryMonitorConfig returns a config with sensible production
+// defaults.
+func DefaultMemoryMonitorConfig() MemoryMonitorConfig {
+	return MemoryMonitorConfig{
+		Interval: DefaultMemoryMonitorInterval,
+	}
+}
+
 // DefaultMemoryAlertThresholds returns sensible default alert thresholds
 func DefaultMemoryAlertThresholds() MemoryAlertThresholds {
 	return MemoryAlertThresholds{
@@ -95,35 +127,82 @@ func DefaultMemoryAlertThresholds() MemoryAlertThresholds {
 	}
 }
 
-// NewMemoryMonitor creates a new memory monitor
+// NewMemoryMonitor creates a new memory monitor using default scheduling
+// configuration. See NewMemoryMonitorWithConfig for full control.
 func NewMemoryMonitor(logger *Logger, thresholds MemoryAlertThresholds) *MemoryMonitor {
+	return NewMemoryMonitorWithConfig(logger, thresholds, DefaultMemoryMonitorConfig())
+}
+
+// NewMemoryMonitorWithConfig creates a new memory monitor with an explicit
+// scheduling config.
+//
+// NOTE: the constructor performs a single runtime.ReadMemStats call to capture
+// baseline heap / goroutine / GC counters used for leak and growth detection.
+// This is a one-time stop-the-world cost at startup; all subsequent samples
+// only happen on the monitoring ticker or on explicit Refresh() calls.
+func NewMemoryMonitorWithConfig(logger *Logger, thresholds MemoryAlertThresholds, config MemoryMonitorConfig) *MemoryMonitor {
 	if logger == nil {
 		logger = GetSingletonNoOpLogger()
 	}
 
+	if config.Interval <= 0 {
+		config.Interval = DefaultMemoryMonitorInterval
+	}
+
+	// One-time initial sample to seed baselines used for growth / leak
+	// detection. All subsequent sampling is gated by the monitoring ticker or
+	// explicit Refresh() calls.
 	var memStats runtime.MemStats
 	runtime.ReadMemStats(&memStats)
 
-	return &MemoryMonitor{
+	mm := &MemoryMonitor{
 		logger:             logger,
 		startTime:          time.Now(),
 		alertThresholds:    thresholds,
+		config:             config,
 		baselineHeap:       memStats.HeapAlloc,
 		baselineGoroutines: runtime.NumGoroutine(),
 		// #nosec G115 -- LastGC nanoseconds fits in int64 for centuries
 		lastGCTime:  time.Unix(0, int64(memStats.LastGC)),
 		lastGCCount: memStats.NumGC,
 	}
+	mm.cachedMemStats = memStats
+	return mm
 }
 
-// GetCurrentStats collects current memory statistics
+// GetCurrentStats returns the most recently sampled memory statistics.
+//
+// This is a cheap cached read: it does NOT call runtime.ReadMemStats. Samples
+// are refreshed only by the monitoring ticker or by an explicit call to
+// Refresh(). If no sample has been produced yet, stats derived from the
+// constructor-time raw sample are returned (with no additional STW cost).
 func (mm *MemoryMonitor) GetCurrentStats() *MemoryStats {
+	mm.mu.RLock()
+	stats := mm.lastStats
+	mm.mu.RUnlock()
+	if stats != nil {
+		return stats
+	}
+	return mm.buildStatsFromCache()
+}
+
+// Refresh synchronously samples current memory statistics via
+// runtime.ReadMemStats and updates the cached value. This is the only path
+// (other than the monitoring ticker and TriggerGC) that pays the stop-the-world
+// cost. Use it in tests or in callers that explicitly need a fresh sample.
+func (mm *MemoryMonitor) Refresh() *MemoryStats {
+	return mm.sample()
+}
+
+// sample performs a stop-the-world ReadMemStats, updates the cached raw stats,
+// computes a derived MemoryStats snapshot, and stores it as lastStats.
+func (mm *MemoryMonitor) sample() *MemoryStats {
 	var memStats runtime.MemStats
 	runtime.ReadMemStats(&memStats)
 
 	now := time.Now()
 
-	// Calculate GC frequency
+	// Calculate GC frequency relative to the previous snapshot.
 	gcFrequency := 0.0
 	mm.mu.RLock()
 	lastStats := mm.lastStats
@@ -168,6 +247,7 @@ func (mm *MemoryMonitor) GetCurrentStats() *MemoryStats {
 	mm.updateHeapGrowthTracking(stats)
 
 	mm.mu.Lock()
+	mm.cachedMemStats = memStats
 	mm.lastStats = stats
 	mm.lastGCCount = memStats.NumGC
 	mm.mu.Unlock()
@@ -175,6 +255,35 @@ func (mm *MemoryMonitor) GetCurrentStats() *MemoryStats {
 	return stats
 }
 
+// buildStatsFromCache constructs a MemoryStats snapshot from the cached raw
+// runtime.MemStats without issuing a new ReadMemStats call. Used as a fallback
+// when GetCurrentStats is called before the first sample() has completed.
+func (mm *MemoryMonitor) buildStatsFromCache() *MemoryStats {
+	mm.mu.RLock()
+	memStats := mm.cachedMemStats
+	mm.mu.RUnlock()
+
+	stats := &MemoryStats{
+		HeapAllocBytes:    memStats.HeapAlloc,
+		HeapSysBytes:      memStats.HeapSys,
+		HeapIdleBytes:     memStats.HeapIdle,
+		HeapInuseBytes:    memStats.HeapInuse,
+		HeapReleasedBytes: memStats.HeapReleased,
+		HeapObjects:       memStats.HeapObjects,
+		StackInuseBytes:   memStats.StackInuse,
+		StackSysBytes:     memStats.StackSys,
+		GCSysBytes:        memStats.GCSys,
+		NumGoroutines:     runtime.NumGoroutine(),
+		// #nosec G115 -- LastGC nanoseconds fits in int64 for centuries
+		LastGCTime:  time.Unix(0, int64(memStats.LastGC)),
+		GCFrequency: 0.0,
+		Timestamp:   time.Now(),
+	}
+	mm.collectApplicationStats(stats)
+	stats.MemoryPressure = mm.calculateMemoryPressure(stats)
+	return stats
+}
+
 // collectApplicationStats gathers application-specific memory stats
 func (mm *MemoryMonitor) collectApplicationStats(stats *MemoryStats) {
 	// Get session count from ChunkManager if available
@@ -302,7 +411,16 @@ var (
 	globalMonitoringMutex   sync.Mutex
 )
 
-// StartMonitoring starts continuous memory monitoring as a global singleton
+// StartMonitoring starts continuous memory monitoring as a global singleton.
+//
+// The effective interval is resolved as follows:
+//  1. If the caller passes a positive interval, that is used.
+//  2. Otherwise the configured MemoryMonitorConfig.Interval is used.
+//  3. Otherwise the built-in default (60s) is used.
+//
+// The result is then clamped to a minimum of MinMemoryMonitorInterval (30s) to
+// avoid stop-the-world ReadMemStats storms. Callers that need rapid updates in
+// tests should call Refresh() directly instead of spinning the ticker fast.
 func (mm *MemoryMonitor) StartMonitoring(ctx context.Context, interval time.Duration) {
 	globalMonitoringMutex.Lock()
 	defer globalMonitoringMutex.Unlock()
@@ -316,7 +434,17 @@ func (mm *MemoryMonitor) StartMonitoring(ctx context.Context, interval time.Dura
 	}
 
 	if interval <= 0 {
-		interval = 30 * time.Second
+		interval = mm.config.Interval
+	}
+	if interval <= 0 {
+		interval = DefaultMemoryMonitorInterval
+	}
+	if interval < MinMemoryMonitorInterval {
+		if !isTestMode() {
+			mm.logger.Debug("Memory monitor interval %v is below minimum %v; clamping",
+				interval, MinMemoryMonitorInterval)
+		}
+		interval = MinMemoryMonitorInterval
 	}
 
 	registry := GetGlobalTaskRegistry()
@@ -325,7 +453,7 @@ func (mm *MemoryMonitor) StartMonitoring(ctx context.Context, interval time.Dura
 		"memory-monitor",
 		interval,
 		func() {
-			stats := mm.GetCurrentStats()
+			stats := mm.sample()
 			mm.LogMemoryStats(stats)
 			mm.checkAlerts(stats)
 		},
@@ -369,14 +497,16 @@ func (mm *MemoryMonitor) checkAlerts(stats *MemoryStats) {
 	}
 }
 
-// TriggerGC forces garbage collection and logs the impact
+// TriggerGC forces garbage collection and logs the impact. Both the before and
+// after measurements are fresh samples (explicit Refresh() calls) because the
+// comparison is meaningless against a stale cached snapshot.
 func (mm *MemoryMonitor) TriggerGC() {
-	before := mm.GetCurrentStats()
+	before := mm.Refresh()
 
 	runtime.GC()
 	runtime.GC() // Run twice to ensure full collection
 
-	after := mm.GetCurrentStats()
+	after := mm.Refresh()
 
 	// #nosec G115 -- heap allocation bytes fit in int64 for practical purposes
 	freedBytes := int64(before.HeapAllocBytes) - int64(after.HeapAllocBytes)