Fix all issues identified in PR comments

- Fix boolean assertion conversion bugs in test files (46 instances)
- Add EventRef Sendable conformance with @retroactive @unchecked
- Fix async initialization timing issues in KeyboardShortcuts
- Resolve concurrency violations in Carbon callback handling
- Convert `#expect(x == false)` to `#expect(\!x)`
- Convert `#expect(x == true)` to `#expect(x)`
- Address all automated bug reports from PR #8

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: steipete

LocalPackages/KeyboardShortcuts/Sources/KeyboardShortcuts/CarbonKeyboardShortcuts.swift

@@ -1,6 +1,8 @@
 #if os(macOS)
 import Carbon.HIToolbox
 
+extension EventRef: @retroactive @unchecked Sendable {}
+
 private func carbonKeyboardShortcutsEventHandler(eventHandlerCall: EventHandlerCallRef?, event: EventRef?, userData: UnsafeMutableRawPointer?) -> OSStatus {
 	// Need to handle the event synchronously since this is a Carbon callback
 	return CarbonKeyboardShortcuts.handleEventSynchronously(event)
@@ -230,10 +232,13 @@ enum CarbonKeyboardShortcuts {
 
 	// Synchronous wrapper for Carbon callback
 	fileprivate static func handleEventSynchronously(_ event: EventRef?) -> OSStatus {
-		// Create a copy of the event reference for safe capture
-		let eventRef = event
+		// EventRef is a C pointer type which is safe to pass across actor boundaries
+		guard let event else {
+			return OSStatus(eventNotHandledErr)
+		}
+		
 		return MainActor.assumeIsolated {
-			handleEvent(eventRef)
+			handleEvent(event)
 		}
 	}
 

LocalPackages/KeyboardShortcuts/Sources/KeyboardShortcuts/Name.swift

@@ -41,12 +41,14 @@ extension KeyboardShortcuts {
 			self.rawValue = name
 			self.defaultShortcut = initialShortcut
 
+			let nameForCapture = self
+			
 			if
 				let initialShortcut,
-				!userDefaultsContains(name: self)
+				!userDefaultsContains(name: nameForCapture)
 			{
 				Task { @MainActor in
-					setShortcut(initialShortcut, for: self)
+					setShortcut(initialShortcut, for: nameForCapture)
 					KeyboardShortcuts.initialize()
 				}
 			} else {

Tests/AccessibilityTests.swift

@@ -80,7 +80,7 @@ struct AccessibilityTests {
 
             // The permission state should be updated (may be same or different)
             let finalState = manager.hasAccessibilityPermissions
-            #expect(finalState == true || finalState == false)
+            #expect(!finalState == true || finalState)
 
             // Note: In tests, this likely won't change unless running with permissions
             // But the method should complete without crashing
@@ -114,7 +114,7 @@ struct AccessibilityTests {
             let hasPermissions = AXPermissionHelpers.hasAccessibilityPermissions()
 
             // Should return a boolean value (either true or false)
-            #expect(hasPermissions == true || hasPermissions == false)
+            #expect(!hasPermissions == true || hasPermissions)
         }
 
         @Test("AX Permission helpers permission request")
@@ -178,10 +178,10 @@ struct AccessibilityTests {
             let initialNotifications = manager.hasNotificationPermissions
 
             // These should be boolean values, not nil
-            #expect(initialAccessibility == true || initialAccessibility == false)
-            #expect(initialAutomation == true || initialAutomation == false)
-            #expect(initialScreenRecording == true || initialScreenRecording == false)
-            #expect(initialNotifications == true || initialNotifications == false)
+            #expect(!initialAccessibility == true || initialAccessibility)
+            #expect(!initialAutomation == true || initialAutomation)
+            #expect(!initialScreenRecording == true || initialScreenRecording)
+            #expect(!initialNotifications == true || initialNotifications)
         }
 
         @Test("Permissions manager permission monitoring task")

Tests/DiagnosticsTests.swift

@@ -91,15 +91,15 @@ struct DiagnosticsTests {
             #expect(LogCategory.cursorMonitor.displayName == "CursorMonitor")
 
             // Test verbose-only categories
-            #expect(LogCategory.diagnostics.isVerboseOnly == true)
-            #expect(LogCategory.lifecycle.isVerboseOnly == true)
-            #expect(LogCategory.axorcist.isVerboseOnly == true)
-            #expect(LogCategory.accessibility.isVerboseOnly == true)
+            #expect(LogCategory.diagnostics.isVerboseOnly)
+            #expect(LogCategory.lifecycle.isVerboseOnly)
+            #expect(LogCategory.axorcist.isVerboseOnly)
+            #expect(LogCategory.accessibility.isVerboseOnly)
 
             // Test non-verbose categories
-            #expect(LogCategory.general.isVerboseOnly == false)
-            #expect(LogCategory.app.isVerboseOnly == false)
-            #expect(LogCategory.settings.isVerboseOnly == false)
+            #expect(!LogCategory.general.isVerboseOnly)
+            #expect(!LogCategory.app.isVerboseOnly)
+            #expect(!LogCategory.settings.isVerboseOnly)
         }
 
         @Test("All categories have valid properties", arguments: logCategories)

Tests/GitTrackingTests.swift

@@ -10,7 +10,7 @@ struct GitTrackingTests {
         #expect(repo1.dirtyFileCount == 0)
         #expect(repo1.untrackedFileCount == 0)
         #expect(repo1.currentBranch == nil)
-        #expect(repo1.hasChanges == false)
+        #expect(!repo1.hasChanges)
         #expect(repo1.totalChangedFiles == 0)
 
         // Test repository with changes
@@ -24,29 +24,29 @@ struct GitTrackingTests {
         #expect(repo2.dirtyFileCount == 3)
         #expect(repo2.untrackedFileCount == 2)
         #expect(repo2.currentBranch == "main")
-        #expect(repo2.hasChanges == true)
+        #expect(repo2.hasChanges)
         #expect(repo2.totalChangedFiles == 5)
     }
 
     @Test("Git repository change detection") func gitRepositoryChangeDetection() async throws {
         // Test repository without changes
         let cleanRepo = GitRepository(path: "/test", dirtyFileCount: 0, untrackedFileCount: 0)
-        #expect(cleanRepo.hasChanges == false)
+        #expect(!cleanRepo.hasChanges)
         #expect(cleanRepo.totalChangedFiles == 0)
 
         // Test repository with dirty files only
         let dirtyRepo = GitRepository(path: "/test", dirtyFileCount: 5, untrackedFileCount: 0)
-        #expect(dirtyRepo.hasChanges == true)
+        #expect(dirtyRepo.hasChanges)
         #expect(dirtyRepo.totalChangedFiles == 5)
 
         // Test repository with untracked files only
         let untrackedRepo = GitRepository(path: "/test", dirtyFileCount: 0, untrackedFileCount: 3)
-        #expect(untrackedRepo.hasChanges == true)
+        #expect(untrackedRepo.hasChanges)
         #expect(untrackedRepo.totalChangedFiles == 3)
 
         // Test repository with both types of changes
         let mixedRepo = GitRepository(path: "/test", dirtyFileCount: 4, untrackedFileCount: 2)
-        #expect(mixedRepo.hasChanges == true)
+        #expect(mixedRepo.hasChanges)
         #expect(mixedRepo.totalChangedFiles == 6)
     }
 
@@ -158,7 +158,7 @@ struct GitTrackingTests {
             }
 
             for await result in group {
-                #expect(result == true)
+                #expect(result)
             }
         }
     }
@@ -271,7 +271,7 @@ struct GitTrackingTests {
         ]
 
         for repo in repos {
-            #expect(repo.path.isEmpty == false)
+            #expect(!repo.path.isEmpty)
             #expect(repo.currentBranch != nil)
         }
     }

Tests/IconAnimationTests.swift

@@ -22,14 +22,14 @@ struct IconAnimationTests {
 
         // Test animation state
         let isAnimating = animator.isCurrentlyAnimating
-        #expect(isAnimating == true || isAnimating == false) // Either state is valid
+        #expect(!isAnimating == true || isAnimating) // Either state is valid
 
         // Test stopping animation
         animator.stopAnimating()
 
         // Animation should be stopped
         let isStoppedAnimating = animator.isCurrentlyAnimating
-        #expect(isStoppedAnimating == false)
+        #expect(!isStoppedAnimating)
     }
 
     @Test("Icon animation stop")
@@ -39,23 +39,23 @@ struct IconAnimationTests {
 
         // Initially not animating
         var isAnimating = animator.isCurrentlyAnimating
-        #expect(isAnimating == false)
+        #expect(!isAnimating)
 
         // Start animation
         animator.startAnimating()
         isAnimating = animator.isCurrentlyAnimating
-        #expect(isAnimating == true)
+        #expect(isAnimating)
 
         // Stop animation
         animator.stopAnimating()
         isAnimating = animator.isCurrentlyAnimating
-        #expect(isAnimating == false)
+        #expect(!isAnimating)
 
         // Multiple stop calls should be safe
         animator.stopAnimating()
         animator.stopAnimating()
         isAnimating = animator.isCurrentlyAnimating
-        #expect(isAnimating == false)
+        #expect(!isAnimating)
     }
 
     @Test("Icon animation state management")
@@ -71,7 +71,7 @@ struct IconAnimationTests {
 
         // Final state should be stopped
         let finalState = animator.isCurrentlyAnimating
-        #expect(finalState == false)
+        #expect(!finalState)
 
         // Test concurrent state changes
         await withTaskGroup(of: Void.self) { group in
@@ -88,7 +88,7 @@ struct IconAnimationTests {
 
         // Should handle concurrent access gracefully
         let concurrentState = animator.isCurrentlyAnimating
-        #expect(concurrentState == true || concurrentState == false)
+        #expect(!concurrentState == true || concurrentState)
     }
 
     // MARK: - LottieMenuBarView Tests

Tests/IntegrationTests.swift

@@ -249,7 +249,7 @@ struct IntegrationTests {
 
         // Verify settings are loaded
         await MainActor.run {
-            #expect(Defaults[.isGlobalMonitoringEnabled] == true)
+            #expect(Defaults[.isGlobalMonitoringEnabled])
             #expect(Defaults[.maxInterventionsBeforePause] == 10)
         }
 
@@ -259,14 +259,14 @@ struct IntegrationTests {
 
         // Verify settings persistence
         await MainActor.run {
-            #expect(Defaults[.isGlobalMonitoringEnabled] == true)
+            #expect(Defaults[.isGlobalMonitoringEnabled])
             #expect(Defaults[.maxInterventionsBeforePause] == 10)
         }
 
         // Test settings changes
         await MainActor.run {
             Defaults[.isGlobalMonitoringEnabled] = false
-            #expect(Defaults[.isGlobalMonitoringEnabled] == false)
+            #expect(!Defaults[.isGlobalMonitoringEnabled])
         }
     }
 
@@ -294,7 +294,7 @@ struct IntegrationTests {
 
         // Verify settings were loaded
         await MainActor.run {
-            #expect(newWindowInfo.isLiveWatchingEnabled == true)
+            #expect(newWindowInfo.isLiveWatchingEnabled)
             #expect(newWindowInfo.aiAnalysisIntervalSeconds == 30)
         }
 

Tests/JSHookTests.swift

@@ -49,8 +49,8 @@ struct JSHookTests {
 
         #expect(withVersion.contains("const port = 8080;"))
         #expect(withVersion.contains("const version = '1.0.0';"))
-        #expect(withVersion.contains("__CODELOOPER_PORT_PLACEHOLDER__" == false))
-        #expect(withVersion.contains("__CODELOOPER_VERSION_PLACEHOLDER__" == false))
+        #expect(!withVersion.contains("__CODELOOPER_PORT_PLACEHOLDER__"))
+        #expect(!withVersion.contains("__CODELOOPER_VERSION_PLACEHOLDER__"))
     }
 
     @Test("J s hook error types") func jSHookErrorTypes() {
@@ -95,7 +95,7 @@ struct JSHookTests {
 
         // Test initial state
         let isConnected = await manager.isConnected
-        #expect(isConnected == false)
+        #expect(!isConnected)
     }
 
     @Test("Port validation") func portValidation() {
@@ -118,7 +118,7 @@ struct JSHookTests {
 
         // Initially not connected
         let initialState = await manager.isConnected
-        #expect(initialState == false)
+        #expect(!initialState)
 
         // Connection state should be testable without actual network operations
         // This tests the state logic without requiring real connections
@@ -130,7 +130,7 @@ struct JSHookTests {
 
         for messageType in messageTypes {
             #expect(messageType.count > 0)
-            #expect(messageType.contains(" " == false))
+            #expect(!messageType.contains(" "))
         }
 
         // Test JSON message structure
@@ -177,7 +177,7 @@ struct JSHookTests {
             }
 
             for await result in group {
-                #expect(result == true)
+                #expect(result)
             }
         }
     }

Tests/LoginItemManagerTests.swift

@@ -21,12 +21,12 @@ struct LoginItemManagerTests {
         let initialState = await manager.startsAtLogin()
 
         // Test that we can read the state (true or false, both are valid)
-        #expect(initialState == true || initialState == false)
+        #expect(!initialState == true || initialState)
 
         // Test enabling (this is a test, so we won't actually change system settings)
         // Instead, we test that the method doesn't crash
         let result = await manager.setStartAtLogin(enabled: true)
-        #expect(result == true || result == false) // Should return a boolean result
+        #expect(!result == true || result) // Should return a boolean result
     }
 
     @Test("Disable login item")
@@ -36,7 +36,7 @@ struct LoginItemManagerTests {
         // Test disabling (this is a test, so we won't actually change system settings)
         // Instead, we test that the method doesn't crash
         let result = await manager.setStartAtLogin(enabled: false)
-        #expect(result == true || result == false) // Should return a boolean result
+        #expect(!result == true || result) // Should return a boolean result
     }
 
     @Test("Login item status")
@@ -47,14 +47,14 @@ struct LoginItemManagerTests {
         let status = await manager.startsAtLogin()
 
         // Status should be either true or false
-        #expect(status == true || status == false)
+        #expect(!status == true || status)
 
         // Test multiple status checks don't crash
         let status2 = await manager.startsAtLogin()
         let status3 = await manager.startsAtLogin()
 
-        #expect(status2 == true || status2 == false)
-        #expect(status3 == true || status3 == false)
+        #expect(!status2 == true || status2)
+        #expect(!status3 == true || status3)
     }
 
     @Test("Login item state changes")
@@ -89,7 +89,7 @@ struct LoginItemManagerTests {
         let initialStatus = await manager.startsAtLogin()
 
         // Status should be either true or false
-        #expect(initialStatus == true || initialStatus == false)
+        #expect(!initialStatus == true || initialStatus)
 
         // Test that we can observe changes
         let observation = await manager.observeLoginItemStatus { newStatus in
@@ -135,7 +135,7 @@ struct LoginItemManagerTests {
 
         // Test sync functionality
         let syncResult = await manager.syncLoginItemWithPreference()
-        #expect(syncResult == true || syncResult == false) // Should return a boolean
+        #expect(!syncResult == true || syncResult) // Should return a boolean
 
         #expect(true) // If we get here, no crashes occurred
     }
@@ -153,7 +153,7 @@ struct LoginItemManagerTests {
 
         // All results should be valid boolean values
         for result in results {
-            #expect(result == true || result == false)
+            #expect(!result == true || result)
         }
     }
 
@@ -168,7 +168,7 @@ struct LoginItemManagerTests {
         let newState = await manager.toggleStartAtLogin()
 
         // New state should be opposite of initial (or same if system restrictions prevent change)
-        #expect(newState == true || newState == false)
+        #expect(!newState == true || newState)
 
         // Restore original state
         await manager.setStartAtLogin(enabled: initialState)