Refactor: fix critical bugs, improve code quality and architecture

Critical fixes:
- Add missing shortestSessionDuration property to VPNStatistics
- Fix use-after-free in HotkeyManager cleanup with isRetainedForEventHandler flag
- Fix potential recursion in VPNManager.shared using NotificationCenter
- Fix thread safety issue in StatusBarController deinit with cleanup method

Architecture improvements:
- Split SettingsWindowController into separate view components (GeneralSettingsView, HotkeySettingsView, AboutSettingsView)
- Create centralized ImageCache utility to eliminate image caching duplication
- Optimize ConnectionHistoryManager with in-memory cache to reduce UserDefaults I/O

Code quality:
- Replace unsafeBitCast with safe HandlerWrapper class in VPNConfigurationLoader
- Refactor connectWithRetry: extract attemptConnection, createSessionAndConnect, scheduleRetry, handleConnectionSuccess, handleConnectionError methods
- Eliminate notification content duplication in NotificationManager with buildNotificationContent helper
- Replace deprecated lockFocus/unlockFocus with NSImage(size:flipped:drawingHandler:)
- Fix force unwrap URLs in AppConstants with guard let and fatalError
- Remove useless expression _ = updateInterval
- Remove unused vpnConnectionsDidChange notification
- Simplify NSApplication.isRunning check to NSApp != nil

Author: borzov

Sources/VPNBarApp/AppConstants.swift

@@ -37,8 +37,18 @@ enum AppConstants {
 
     /// URLs used in the application.
     enum URLs {
-        static let repository = URL(string: "https://github.com/borzov/vpn-bar")!
-        static let networkPreferences = URL(string: "x-apple.systempreferences:com.apple.Network-Settings.extension")!
+        static let repository: URL = {
+            guard let url = URL(string: "https://github.com/borzov/vpn-bar") else {
+                fatalError("Invalid repository URL")
+            }
+            return url
+        }()
+        static let networkPreferences: URL = {
+            guard let url = URL(string: "x-apple.systempreferences:com.apple.Network-Settings.extension") else {
+                fatalError("Invalid network preferences URL")
+            }
+            return url
+        }()
     }
 }
 

Sources/VPNBarApp/AppDelegate.swift

@@ -45,6 +45,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
         HotkeyManager.shared.cleanup()
         Task { @MainActor in
             VPNManager.shared.cleanup()
+            statusBarController?.cleanup()
         }
     }
 

Sources/VPNBarApp/Extensions/Notification+Extensions.swift

@@ -9,8 +9,8 @@ extension Notification.Name {
 
     /// Notification about connection name display change.
     static let showConnectionNameDidChange = Notification.Name("ShowConnectionNameDidChange")
-
-    /// Notification about VPN connections list change.
-    static let vpnConnectionsDidChange = Notification.Name("VPNConnectionsDidChange")
+    
+    /// Notification about VPN connection status update.
+    static let vpnConnectionStatusDidUpdate = Notification.Name("VPNConnectionStatusDidUpdate")
 }
 

Sources/VPNBarApp/HotkeyManager.swift

@@ -17,6 +17,9 @@ class HotkeyManager: HotkeyManagerProtocol {
     /// Set to false in deinit and cleanup to prevent callback execution
     /// after manager memory is deallocated. Checked before each callback invocation.
     private var isValid = true
+    /// Flag to track if self was retained for event handler.
+    /// Used to properly balance retain/release calls.
+    private var isRetainedForEventHandler = false
 
     private init() {
         setupEventHandler()
@@ -38,6 +41,7 @@ class HotkeyManager: HotkeyManagerProtocol {
 
         // Use passRetained to ensure manager stays alive during event handler lifetime
         let userData = Unmanaged.passRetained(self).toOpaque()
+        isRetainedForEventHandler = true
 
         let eventHandlerUPP: EventHandlerUPP = { (nextHandler, theEvent, userData) -> OSStatus in
             guard let userData = userData else {
@@ -150,7 +154,10 @@ class HotkeyManager: HotkeyManagerProtocol {
 
             // Release the retained reference from passRetained
             // This balances the retain from setupEventHandler
-            Unmanaged.passUnretained(self).release()
+            if isRetainedForEventHandler {
+                Unmanaged.passUnretained(self).release()
+                isRetainedForEventHandler = false
+            }
 
             eventHandler = nil
         }

Sources/VPNBarApp/Managers/VPNConfigurationLoader.swift

@@ -2,6 +2,15 @@ import Foundation
 import Darwin
 import os.log
 
+/// Wrapper for handler block to safely pass to Objective-C runtime.
+private class HandlerWrapper: NSObject {
+    let handler: @convention(block) (NSArray?, NSError?) -> Void
+    
+    init(_ handler: @escaping @convention(block) (NSArray?, NSError?) -> Void) {
+        self.handler = handler
+    }
+}
+
 /// Loads VPN configurations from the system.
 @MainActor
 final class VPNConfigurationLoader: VPNConfigurationLoaderProtocol {
@@ -93,8 +102,9 @@ final class VPNConfigurationLoader: VPNConfigurationLoaderProtocol {
             }
         }
 
-        // Use safer approach with NSInvocation instead of unsafeBitCast
-        let handlerObject = unsafeBitCast(handler, to: AnyObject.self)
+        // Use HandlerWrapper to safely pass block to Objective-C runtime
+        let handlerWrapper = HandlerWrapper(handler)
+        let handlerObject = handlerWrapper as AnyObject
 
         // Validate method signature before calling
         let methodSignature = manager.method(for: selector)

Sources/VPNBarApp/MenuController.swift

@@ -7,10 +7,6 @@ class MenuController {
     static let shared = MenuController()
 
     // MARK: - Cached Images
-    private static var cachedActiveImage: NSImage?
-    private static var cachedInactiveImage: NSImage?
-    private static var cachedErrorImage: NSImage?
-    private static var cachedDisconnectAllImage: NSImage?
 
     private var menu: NSMenu?
     private var statusItem: NSStatusItem?
@@ -53,8 +49,8 @@ class MenuController {
     private func buildMenu() {
         let newMenu = NSMenu()
         // Set appearance only if NSApplication is available (not in test environment)
-        if let app = NSApplication.shared as NSApplication?, app.isRunning {
-            newMenu.appearance = app.effectiveAppearance
+        if NSApp != nil {
+            newMenu.appearance = NSApp.effectiveAppearance
         }
 
         if let error = vpnManager.loadingError {
@@ -203,42 +199,18 @@ class MenuController {
     // MARK: - Cached Image Helpers
 
     private static func activeImage() -> NSImage? {
-        if let cached = cachedActiveImage { return cached }
-        guard let image = NSImage(systemSymbolName: "checkmark.circle.fill", accessibilityDescription: nil) else {
-            return nil
-        }
-        image.isTemplate = true
-        cachedActiveImage = image
-        return image
+        return ImageCache.shared.image(systemSymbolName: "checkmark.circle.fill")
     }
 
     private static func inactiveImage() -> NSImage? {
-        if let cached = cachedInactiveImage { return cached }
-        guard let image = NSImage(systemSymbolName: "circle", accessibilityDescription: nil) else {
-            return nil
-        }
-        image.isTemplate = true
-        cachedInactiveImage = image
-        return image
+        return ImageCache.shared.image(systemSymbolName: "circle")
     }
 
     private static func errorImage() -> NSImage? {
-        if let cached = cachedErrorImage { return cached }
-        guard let image = NSImage(systemSymbolName: "exclamationmark.triangle", accessibilityDescription: nil) else {
-            return nil
-        }
-        image.isTemplate = true
-        cachedErrorImage = image
-        return image
+        return ImageCache.shared.image(systemSymbolName: "exclamationmark.triangle")
     }
 
     private static func disconnectAllImage() -> NSImage? {
-        if let cached = cachedDisconnectAllImage { return cached }
-        guard let image = NSImage(systemSymbolName: "xmark.circle", accessibilityDescription: nil) else {
-            return nil
-        }
-        image.isTemplate = true
-        cachedDisconnectAllImage = image
-        return image
+        return ImageCache.shared.image(systemSymbolName: "xmark.circle")
     }
 }

Sources/VPNBarApp/Models/ConnectionHistory.swift

@@ -23,18 +23,48 @@ final class ConnectionHistoryManager {
     private let historyKey = "connectionHistory"
     private let maxHistoryEntries = 100
 
-    private init() {}
+    /// In-memory cache to avoid frequent UserDefaults I/O.
+    private var cachedHistory: [ConnectionHistoryEntry]?
+    private var isDirty = false
+    
+    private init() {
+        loadHistoryFromUserDefaults()
+    }
+    
+    /// Loads history from UserDefaults into cache.
+    private func loadHistoryFromUserDefaults() {
+        guard let data = userDefaults.data(forKey: historyKey),
+              let entries = try? JSONDecoder().decode([ConnectionHistoryEntry].self, from: data) else {
+            cachedHistory = []
+            return
+        }
+        cachedHistory = entries.sorted { $0.timestamp > $1.timestamp }
+    }
+    
+    /// Saves history from cache to UserDefaults.
+    private func saveHistoryToUserDefaults() {
+        guard isDirty, let history = cachedHistory else { return }
+        
+        if let data = try? JSONEncoder().encode(history) {
+            userDefaults.set(data, forKey: historyKey)
+            isDirty = false
+        }
+    }
 
     /// Gets connection history.
     /// - Parameter limit: Maximum number of entries (default is 50).
     /// - Returns: Array of history entries sorted by time (newest first).
     func getHistory(limit: Int = 50) -> [ConnectionHistoryEntry] {
-        guard let data = userDefaults.data(forKey: historyKey),
-              let entries = try? JSONDecoder().decode([ConnectionHistoryEntry].self, from: data) else {
+        // Ensure cache is loaded
+        if cachedHistory == nil {
+            loadHistoryFromUserDefaults()
+        }
+        
+        guard let history = cachedHistory else {
             return []
         }
 
-        return Array(entries.sorted { $0.timestamp > $1.timestamp }.prefix(limit))
+        return Array(history.prefix(limit))
     }
 
     /// Adds entry to connection history.
@@ -51,31 +81,32 @@ final class ConnectionHistoryManager {
             action: action
         )
 
-        // Load existing history without sorting (raw data)
-        guard let data = userDefaults.data(forKey: historyKey),
-              var history = try? JSONDecoder().decode([ConnectionHistoryEntry].self, from: data) else {
-            // No existing history, create new with single entry
-            if let data = try? JSONEncoder().encode([entry]) {
-                userDefaults.set(data, forKey: historyKey)
-            }
-            return
+        // Ensure cache is loaded
+        if cachedHistory == nil {
+            loadHistoryFromUserDefaults()
         }
 
+        var history = cachedHistory ?? []
+        
         // Insert new entry at the beginning (most recent)
         history.insert(entry, at: 0)
 
-        // Trim to max entries if needed (no sorting required, already in order)
+        // Trim to max entries if needed
         if history.count > maxHistoryEntries {
             history = Array(history.prefix(maxHistoryEntries))
         }
 
-        if let data = try? JSONEncoder().encode(history) {
-            userDefaults.set(data, forKey: historyKey)
-        }
+        cachedHistory = history
+        isDirty = true
+        
+        // Save immediately for data persistence
+        saveHistoryToUserDefaults()
     }
 
     /// Clears connection history.
     func clearHistory() {
+        cachedHistory = []
+        isDirty = true
         userDefaults.removeObject(forKey: historyKey)
     }
 }

Sources/VPNBarApp/Models/VPNStatistics.swift

@@ -8,6 +8,7 @@ struct VPNStatistics: Codable, Equatable {
     var lastConnectionDate: Date?
     var lastDisconnectionDate: Date?
     var longestSessionDuration: TimeInterval = 0
+    var shortestSessionDuration: TimeInterval? = nil
     /// Average session duration in seconds.
     var averageSessionDuration: TimeInterval {
         guard totalConnections > 0 else { return 0 }